Issue with authorization code flow redirect response "code" parameter length

[pounard]

I am using this API in an environment with OpenAM as client, and experience a blocking issue: when your API sends the redirect response, it sends back as code a serialized, encrypted JSON structure containing the request and other few data, as you can see in src/Grant/AuthCodeGrant.php:

            $response->setRedirectUri(
                $this->makeRedirectUri(
                    $finalRedirectUri,
                    [
                        'code'  => $this->encrypt(
                            json_encode(
                                [
                                    'client_id'               => $authCode->getClient()->getIdentifier(),
                                    'redirect_uri'            => $authCode->getRedirectUri(),
                                    'auth_code_id'            => $authCode->getIdentifier(),
                                    'scopes'                  => $authCode->getScopes(),
                                    'user_id'                 => $authCode->getUserIdentifier(),
                                    'expire_time'             => (new \DateTime())->add($this->authCodeTTL)->format('U'),
                                    'code_challenge'          => $authorizationRequest->getCodeChallenge(),
                                    'code_challenge_method  ' => $authorizationRequest->getCodeChallengeMethod(),
                                ]
                            )
                        ),
                        'state' => $authorizationRequest->getState(),
                    ]
                )
            );

Problem is that OpenAM has an hardcoded request size limit, as you can see there:

ESAPI.validator().isValidInput(PARAM_CODE, code, "HTTPParameterValue", 512, true)

So it is unable to validate any response from this API. Looking at the OpenID Connect 1.0 spec with can see:

To obtain an Access Token, an ID Token, and optionally a Refresh Token, the RP (Client) sends a Token Request to the Token Endpoint to obtain a Token Response, as described in Section 3.2 of OAuth 2.0 [RFC6749], when using the Authorization Code Flow.

And later on in RFC6749, in the code definition:

REQUIRED. The authorization code generated by the authorization server. The authorization code MUST expire shortly after it is issued to mitigate the risk of leaks. A maximum authorization code lifetime of 10 minutes is RECOMMENDED. The client MUST NOT use the authorization code

If I look up a bit deeper in the Open ID Connect spec there, and it states:

When using the Authorization Code or Hybrid flows, an ID Token is returned from the Token Endpoint in response to a Token Request using an Authorization Code. Some implementations may choose to encode state about the ID Token to be returned in the Authorization Code value. Others may use the Authorization Code value as an index into a database storing this state.

So if I understand correctly, the code can be anything, really, and those information you send encrypted should probably just a generated unique identifier instead, stored on the server side.

Is there any way to do this ?

[alexbilbie]

This project doesn't implement the Open ID Connect specification yet (and won't for a while).

The authorization codes are distributed as embedded payloads to reduce DB hits in high request environments when a client converts to an access token.

The only way currently to change this would be to extend the grant and the AuthorizationCodeRepositoryInterface to support saving the value to the database

[pounard]

Yet, this is not OpenID specific, but the RFC doesn't opiniate about what exactly is the code, wouldn't it be useful to provide a repository interface for this, for which the default implementation would be to crypt/uncrypt ?

I'm not that comfortable with extending the API, I'd like my future updates to go smoothly :)

If the idea's of adding another repository + interface + default implementation is OK for you, would you potentially accept a PR ?

[pounard]

Actually I found a way better solution, I am overriding the CryptTrait to something that instead of crypting will load and save into the database.

This probably would worth an injectable component using an clean interface for this instead of the actual trait, so wouldn't have to actually extend the grant implementation in order to override the trait itself.

It would also be cleaner and easier for unit testing, and also for integrating for such use cases.

[alexbilbie]

I'm moving v6 away from using the crypt trait (and removing the requirement for symmetric key pairs too if you don't want to use them). Going forward an auth code is likely to be a JWT which should be smaller than the current encrypted payload

[pounard]

Good news, thanks.