Announcement

Collapse
No announcement yet.
X
  • Filter
  • Time
Clear All
new posts

    Severe (security?) problem with DataSourceField viewRequiresRole and transferred data

    Hi Isomorphic,

    I just started using viewRequiresRole for a new use case, where a customer of mine wants to see some data, others do not want to be seen in the GUI (or even transferred to the client). I'm using v10.1p_2017-07-04/PowerEdition.

    My expection is that there is a different XML returned from DataSourceLoader depending on the roles returned on login by j_security_check / Form-Based Authentication. This is happening:
    Code:
    .ds.xml (using some nativeName because I did not add the actual column to the view, yet):  <field name="CUSTNAME" type="integer" nativeName="MD_LEADTEMPERATURE_NAME" viewRequiresRole="pickLead">
      <title><fmt:message key="leadTemperature" /></title>
      </field>
    
    Result for the field in the DataSourceLoader for a user with "pickLead":
    {name:"CUSTNAME",nativeName:"MD_LEADTEMPERATURE_NAME",type:"integer",title:"Leadtemperatur",validators:[]},
    
    Result for the field in the DataSourceLoader for the same user after changing viewRequiresRole to"false" (which the user does not have):
    {canView:false,name:"CUSTNAME"}
    My 2nd expectation is that in case of viewRequiresRole="false" the column is never present in DSResponses. This is not the case.
    See this screenshot:
    Click image for larger version

Name:	security-problem.png
Views:	2
Size:	16.2 KB
ID:	245875

    My expectation comes from this Quick Start Guide excerpt:
    PDF page 70:
    Similarly, the viewRequiresRole attribute will cause DataBound Components to avoiding showing the field at all, and values for the field will be automatically omitted from server responses. This behavior is automatic even if you build a custom DataSource or write DMI logic that returns data for the field, so it can be used regardless of how persistence is implemented.
    I'm using v10.1p_2017-07-04 with Tomcat 8.

    Best regards
    Blama

    #2
    Hi Isomorphic,

    FYI: I just retested with v10.1p_2017-07-20 and the behavior is the same.

    Best regards
    Blama

    Comment


      #3
      We have a large suite of automated tests that exercise field-level security, including tests for unauthenticated, role-free and wrong-role users trying to fetch a field with viewRequiresRole. These are all passing with current 10.1 builds, so there is no "it just doesn't work" situation. Could you provide us with a testcase that demonstrates this problem? We would need to see a case that doesn't depend on a specific authentication or role-based authorization technology beyond the mechanisms provided by the servlet API - the HttpServletRequest methods getRemoteUser() and isUserInRole()

      Comment


        #4
        Hi Isomorphic,

        OK, I'll try to create a testcase based on Tomcat and BuiltInDS tomorrow.
        I'm following closely your Realm approach from the wiki.

        Best regards
        Blama

        Comment


          #5
          Hi Isomorphic,

          all good, sorry for the false alarm. After seeing that it worked as expected in BuiltInDS I tried to simplify my .ds.xml with no success.
          It turned out to be related to my processRequest() override in my IDACall-subclass.

          This is working:
          Code:
           public class LMSIDACall extends IDACall {
            private static final long serialVersionUID = 4543651445782711736L;
             
            /*
            * See http://www.smartclient.com/smartgwtee/javadoc/com/smartgwt/client/docs/serverds/OperationBinding.html#requiresRole
            * Basically the result you get from rpc.getUserId() before this is the login user-*name*, while for the rest of the application
            * the user-*Id* (number) is needed (for fields of type "creator", "modifier").
            */
            @Override
            public void processRequest(HttpServletRequest servletRequest, HttpServletResponse servletResponse) throws ServletException, IOException {
            Long userId = User.getUserId(servletRequest);
            try {
            RequestContext context = RequestContext.instance(this, servletRequest, servletResponse);
            RPCManager rpc = new RPCManager(servletRequest, servletResponse);
            String currentId = rpc.getUserId();
            List currentRoles = rpc.getUserRoles();
             
            rpc.setUserId(userId.toString());
            rpc.setUserRoles(User.getUserCapabilities(servletRequest)); //Did not have this row, before
             
            currentId = rpc.getUserId();
            currentRoles = rpc.getUserRoles();
             
            // call processRPCTransaction() to iterate through all RPCRequests and DSRequests and execute them
            processRPCTransaction(rpc, context);
             
            } catch (Throwable e) {
            handleError(servletResponse, e);
            }
            }
            }
          Without the row rpc.setUserRoles(User.getUserCapabilities(servletRequest)), rpc.getUserRoles() returns null.
          Nevertheless operationBindung-level requiresRole is working.
          Code:
          <operationBinding operationType="fetch" requiresRole="myRole">....
          Only field level security is not. I don't know if this is a (different) bug, but now that I know the reason, I'll just add the line.

          The reason I have this code in the 1st place is that I do these things:
          • Username conversion from servlet username to ID, as this ID is needed for database fields of type creator
          • User session duration logging
          The concept itself, which does mention the use of setUserRoles() comes from these docs.

          Thank you & Best regards
          Blama

          Comment


            #6
            Thanks for the clarification. We will look at why this seems to be required at field level but not operation level - there may be a good reason, but on the face of it they should work the same way

            Comment


              #7
              Reviewing the code in this area, we see no obvious difference between the "viewRequiresRole" case and the "requiresRole" case. It appears that both should work correctly without explicitly adding roles to the RPCManager, so long as isUserInRole() returns the expected value. Based on your LMSIDACall class above, could you tell us the results of:
              • Calling servletRequest.isUserInRole("myRole") as the very first thing that method does
              • Calling rpc.getUserRoles() immediately after creating the RPCManager
              Thanks,
              Isomorphic Software Support

              Comment


                #8
                Hi Isomorphic,

                now testing with v11.1p_2017-08-16. Result:
                • servletRequest.isUserInRole("editCampaign") as 1st call: true (as expected)
                • rpc.getUserRoles() directly after RPCManager rpc = new RPCManager(servletRequest, servletResponse): null
                • rpc.getUserId() directly after RPCManager rpc = new RPCManager(servletRequest, servletResponse): the expected user name
                Perhaps there is something missing in the RPCManager constructor?

                Best regards
                Blama

                Comment


                  #9
                  No, this is as expected. If RPCManager.getUserRoles() returns null, we call the isUserInRole() method instead. Likewise, you should not have to call setUserId(), because we fall back to getRemoteUser() if the user is not explicitly set (this is why getUserId() returns the expected user even though you have not explicitly set it). So your findings agree with our review; unfortunately, this does not explain why your code does not work unless you explicitly provide roles to the RPCManager. Maybe your "User" class is doing something that is interfering with SmartClient?

                  Comment


                    #10
                    Hi Isomorphic,

                    does this
                    Originally posted by Isomorphic View Post
                    If RPCManager.getUserRoles() returns null, we call the isUserInRole() method instead.
                    also happen if setUserId() was called? Because then I should not need my line with rpc.setUserRoles().

                    User class does not do anything fancy, but I need the call to setUserId() with an ID-int value as Tomcat returns the username of the logged in user. I do need the ID though, as all creator and modifier fields are number fields in the DB, of course. See this post where I had the problem first.

                    Thinking about it now, I will look up if I can get Tomcat to return the ID instead of the login name.

                    Best regards
                    Blama

                    Comment


                      #11
                      Yes, the logic that decides whether to fall back to isUserInRole() is simply, "does getUserRoles() return null?" It is independent of what you have done with the user. One thought we originally had is that you may have two users in play here - the one that getRemouteUser() would return, which is the one that would apply to isUserInRole(), and some other user that is coming back from your code. This could mislead you into thinking that the user has a particular role; it didn't seem very likely, to be honest, and your additional findings seem to show that this is not what's happening (your initial call to isUserInRole() is exactly what SmartClient will do if there is no local list of roles to consult, and you are getting back true). But we don't really have any other theories to offer: we tested this exact use case of setting the user via RPCManager.setUserId(), but not calling RPCManager.setUserRoles(), and everything worked as expected.

                      Comment


                        #12
                        Hi Isomorphic,

                        strange, but ok for me as it is working with that line as it should.

                        One last comment: Without setUserRoles(), Declarative security was working for me all the time for Operation Bindings. It just was not for field-level Declarative security, where it allowed the access always, see post #5.

                        Originally posted by Blama View Post
                        Without the row rpc.setUserRoles(User.getUserCapabilities(servletRequest)), rpc.getUserRoles() returns null.
                        Nevertheless operationBindung-level requiresRole is working.
                        Code:
                        <operationBinding operationType="fetch" requiresRole="myRole">....
                        Only field level security is not. I don't know if this is a (different) bug, but now that I know the reason, I'll just add the line.
                        Best regards
                        Blama

                        Comment

                        Working...
                        X