Uploaded image for project: 'OpenAM'
  1. OpenAM
  2. OPENAM-15712

Incomplete authentication to say return 400 Unauthorized (if incomplete callbacks is passed) or the attempts of tries is done



    • Bug
    • Status: Open
    • Major
    • Resolution: Unresolved
    • None
    • oauth2


      The code AM password flow does not handle complicated tree flows. This is due to the Retry Node going back to ask for more NameCallback. When one is using a Tree node with many Nodes or have a Retry Node or some node that does not fit with UserName/password input gathering needed for the password flow, return 500 Internal Error when failing password grant


      Above is an example of a bad tree (using Retry and lockout)


      1. Setup a OAuth client for password grant
      2. Let say you use the default service=Example for this and access the password grant with correct password. This should work and get an access token.
      3. Similar try this out with a wrong password. On as long you should get 400 Error (with OPENAM-15323 fix in
      4. Now change the default service on this realm or pass the password grant with &auth_chain=<Yourchain_with_Retry>
      5. Test with correct password with the &auth_chain= <Yourchain_with_Retry>, and this works
      6. Now try <Yourchain_with_Retry> with a wrong password. This get 500 Internal error.

      Current behaviour
      Currently throws the following error after user account lockout with AM Build 512c5a2f00:

       [support@localhost ~]$ curl --request POST --data "grant_type=password" --data "username=demo" --data "password=changeit" --data "scope=profile" --data "client_id=MyClientID" --data "client_secret=password" --data "auth_chain=MyAuthTree" 
      {"error_description":"Internal Server Error (500) - The server encountered an unexpected condition which prevented it from fulfilling the request","error":"server_error"}

      Expected behaviour

      400 Error 
      {    "error_description": "Authentication Failed",    "error": "invalid_grant"}



      for (int i = 0; i < 3; i++) {
          Response response = authenticationHandler.authenticate(servletRequest, servletResponse, payload,
                  indexType, service, null);
          payload = getPayload(response);
      String universalId = (String) servletRequest.getAttribute(PRINCIPAL_UID_REQUEST_ATTR); <--- THIS is NULL (when the LOOP exit)
      Integer authLevel = (Integer) servletRequest.getAttribute(AUTH_LEVEL_REQUEST_ATTR);
      try {
          SSOToken adminToken = coreWrapper.getAdminToken();
       final AMIdentity id = coreWrapper.getIdentity(adminToken, universalId);
       return new ResourceOwner(id.getName(), id, currentTimeMillis(), authLevel);

      The problem is AM has will evaluate the Tree internally with a maximum 3 callback evaluation. It is possible that it may cause triggering issues as if the retry node is used it may call the flow with the same wrong password and hence likely to trigger cascaded issue (like account lockout prematurely). In the above example if the Retry node has set a high retry, the tree evaluation will not end to the "Failure" Node and hence still inside tree evaluation,

      Anyway, the main issue here is that it exhausted all the tries and yes there is a bug hence since on line 176: 

      String universalId = (String) servletRequest.getAttribute(PRINCIPAL_UID_REQUEST_ATTR);

      universalId is null (as the value is only set on Success) but we are rerouted by the retry for more Callbacks (so the state is in transition). Probably we should just return InvalidGrant for this.

      So this bug is for handling the case where the tree evaluation is incomplete (from the Tree which did not get a SUCESSS or FAILURE node). It would be best to log "debug" level and return the error as 400 Invalid instead of 500 error. Unfortunately since the tree is not completely evaluated, it is unlikely that we get any exact error from the Tree (if say there is).

      You may want to use a auth_chain=<another_tree> that does not suffer or fails due to asking for more user data since the password grant passes username/password authentication and work on those and these tree should not have retry or more UI input.




            Unassigned Unassigned
            tom.white Tom White
            0 Vote for this issue
            9 Start watching this issue