Announcement Announcement Module
Collapse
No announcement yet.
Oauth 1.0.5 AccountStatusException caught in ResourceOwnerPasswordTokenGranter Page Title Module
Move Remove Collapse
X
Conversation Detail Module
Collapse
  • Filter
  • Time
  • Show
Clear All
new posts

  • Oauth 1.0.5 AccountStatusException caught in ResourceOwnerPasswordTokenGranter

    Current 1.0.5 now catches the AccountStatusException in ResourceOwnerPasswordTokenGranter.getOAuth2Authent ication()

    Even thought the change is perfectly fine for returning "invalid_grant", but at list
    the exception should be set as the cause for the exception.
    1.0.5:
    catch (AccountStatusException ase) {
    //covers expired, locked, disabled cases (mentioned in section 5.2, draft 31)
    throw new InvalidGrantException(ase.getMessage());
    }

    It should be:
    catch (AccountStatusException ase) {
    //covers expired, locked, disabled cases (mentioned in section 5.2, draft 31)
    throw new InvalidGrantException(ase.getMessage(), ase);
    }

    My application has been throwing 403 for this type of exception; now it becomes a 400. I would suggest that at least the original exception in this case is set as the cause of the one returned, so the application can figure out what to do, decorate the response appropriately, with metadata.

    In regular security, you have the various account statuses.

  • #2
    I'd say that maybe this isn't your application sending a 400 though: it's the Authorization Server (which perhaps you embedded in your application, but really it's a separate concern). I'm happy to consider a pull request, but what would you do with the cause in this case? I can see that comment indicating the granter is trying to follow explicit instructions from the spec, but those are always changing and probably open to interpretation, so we can see if this case can be treated differently if you have some more detailed arguments.

    I think the main reason the ResourceOwnerPasswordTokenGranter uses its own AuthenticationManager is to be able to step outside the "normal" flow for exceptions and HTTP status codes because the spec seemed to be idiosyncratic in that area, and require us to handle exceptions differently. I'd also like to change that (and maybe give you more flexibility to handle the exceptions how you prefer), but that would be a more long term (1.1?) effort.

    Comment


    • #3
      thank you for your response.

      There is only one Authentication Service, and the authentication goes through the OAuth2 granter.When the user's account is locked, or expired, the application would like to know, so it can stop prompting the user to enter the pwd again. As you can see this is different from an invalid password, which according to OAuth2 falls also under 'invalid_grant". In the former case, I would like to throw a 403, and not having to rely on the text message since it can be localized.

      At this point, it appears I cannot use 1.0.5, because of this. Any suggestions?? I do not want to call authentication manager twice because of this.

      Comment


      • #4
        Maybe you do have to rely on the error message (which isn't localized). But also not sure what you mean by calling the authentication manager twice, and I don't know what an Authentication Service is, so I'm a bit in the dark really about what you need to do and why it isn't working, when all we do in Spring OAuth is try and follow the spec. Is this actually a change in 1.0.5 that you need to revert? Confused, but willing to try and understand if it helps.

        Comment


        • #5
          The change I am referring to was introduced in 1.0.1. We are currently using 1.0.0., so it is not due to 1.0.5.
          I just checked the OAuth releases' code.
          I know that you are following the specs. Regarding a refresh token, invalid_grant suffices, but regarding a user's own account, when using this grant for authentication and authorization, the invalid_grant error is too generic. I really would need to know the cause for the exception. So that is why I proposed to set the invalid_grant exception cause to the exception originator.
          Regular spring security makes the distinction between the different statuses the user is in.

          I am thinking I can look at the UserDetails probably where these flags, e.g., accountLocked, etc. are set to change the final http error sent back.



          THis is the RFC:
          invalid_grant
          The provided authorization grant (e.g., authorization
          code, resource owner credentials) or refresh token is
          invalid, expired, revoked, does not match the redirection
          URI used in the authorization request, or was issued to
          another client.

          The refresh token is treated the same as a user's credentials, regarding errors. For a token it makes sense, but not for a user's authentication.

          Comment


          • #6
            Can you open a JIRA ticket, please. I want to refactor the ResourceOwnerPasswordTokenGranter anyway? Your best workaround for now is to replace that token granter I guess.

            Comment

            Working...
            X