Make AuthCodeGrant::validateAuthorizationCode() visibility protected

[GameplayJDK]

Hello everyone,

I'd like to propose making the AuthCodeGrant::validateAuthorizationCode() method's visibility protected. In addition the method should also carry a @throws tag.

Currently the method is private, which means there is no way to alter (extend or change) the validation logic for the auth code, for example in a sub-class of AuthCode.

The new signature would look like this:

    /**
     * Validate the authorization code.
     *
     * @param stdClass               $authCodePayload
     * @param ClientEntityInterface  $client
     * @param ServerRequestInterface $request
     *
     * @throws OAuthServerException
     */
    protected function validateAuthorizationCode(
        $authCodePayload,
        ClientEntityInterface $client,
        ServerRequestInterface $request
    ) {
        // ...
    }

I could also provide a pull request, if this change is desired by the maintainers.

Best regards

[Sephster]

What kind of change would you want to make to the authorisation mechanism? Can you give an example? Thank you

[GameplayJDK]

The use-case at hand is a sub-class of AuthCode, which must have access to the submitted auth code before the access and refresh tokens are issued. The intent is to link the provided auth code to the issued tokens for later use.

[Sephster]

What is the reason for wanting to link the auth code to the access token? I'm hesitant to make this function non-private as it is a part of core security of the package and a mistake could make implementations less secure.

[GameplayJDK]

It would be pretty complicated to explain the whole concept I decides to use for single sign on here.

For the auth_code grant, when a authentication request is completed and an auth code is issued, a session is created and already associates with it. I'm currently overriding the issueAuthCode() method from the AbstractGrant class for that. In the next step, when the access token request is completed, I'll need to check the session of the supplied auth code against the session id sent with the request to verify the integrity of the request. A good point to do this at would be the validateAuthorizationCode() method in question here, as it is supplied with the decrypted auth code payload and it can safely be assumed, the code itself is valid when calling the parent method returns without throwing.

For the time being I'm limited to loading a hotfix class via the composer autoload, which is far beyond ideal.

[Sephster]

This sounds like quite a niche implementation. We haven't had many calls for overriding the validateAuthorizationCode() method (if any) as I think it fits most people's requirements. I appreciate this might be causing you issues but I feel that for the vast majority, having this method as private is not causing any issues, but opening it up, could lead to accidentally making the system insecure.

We are pretty liberal with customisation in this package but I think the validateAuthorizationCode mechanism is probably something we don't want to open up at this time.

One thing I do note is you mentioned SSO - if you are using OAuth for authentication, it might be better you look at OpenIdConnect. I might not be right with this though as it was just a passing comment but might be something to look into.

Thank you for your PR and sorry we won't move forwards with it this time.

[GameplayJDK]

While it may not cause any issues, keeping it private is really limiting extensibility here.

I understand and accept your point of view and it's definitely good to not always take a liberal opinion on security related topics. To overcome this limitation I now will have to ship a slightly changed implementation of the AuthCode class. It's a pity there is no other way to solve this, but it'll do.

I'll certainly give OpenID Connect a read, but for now the SSO stuff is less of a problem, since it's a custom implentation on top of OAuth 2.0. The aim is to allow SSO internally between multiple applications, while external use-cases work without session functionality. This library provides the core foundation to that concept through its grant types and domain logic.