Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix token request body building #2

Merged
merged 1 commit into from
Mar 11, 2021
Merged

Fix token request body building #2

merged 1 commit into from
Mar 11, 2021

Conversation

drupol
Copy link
Contributor

@drupol drupol commented Mar 11, 2021

This PR:

The bug has been discovered by my colleage @ileanatudoran, props to her!

How to reproduce the issue?

When you want to introspect the access token using client_secret_jwt, the IntrospectionService seems to be your friend.
Inside IntrospectionService::introspect(), the ClientSecretJwt AuthMethodInterface calls AbstractJwtAuth::createRequest()

$tokenRequest = $authMethod->createRequest($tokenRequest, $client, $params);

and do:

$clientId = $client->getMetadata()->getClientId();

$claims = array_merge([
    'client_id' => $clientId,
    'client_assertion_type' => 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer',
    'client_assertion' => $this->createAuthJwt($client, $claims),
], $claims);

$request->getBody()->write(http_build_query($claims));

Then, that $request object is returned back in IntrospectionService::introspect() and the body of the $request is modified:

$tokenRequest = $authMethod->createRequest($tokenRequest, $client, $params);
$tokenRequest->getBody()->write(http_build_query(array_merge($params, [
    'token' => $token,
])));

The issue is when we do:

$tokenRequest->getBody()->write(http_build_query(array_merge($params, [
    'token' => $token,
])));

By using the write() method, it will append the generated query string to the $request body.

So, you could end up having a request body as such:

client_assertion=tokenvalue&client_id=value1&client_assertion_type=urn:ietf:params:oauth:client-assertion-type:jwt-bearertoken=token

There is a missing & betweem the original request body client_assertion=tokenvalue&client_id=value1&client_assertion_type=urn:ietf:params:oauth:client-assertion-type:jwt-bearer and token=token.

@thomasvargiu
Copy link
Member

Thank you @drupol,

the same problem is in the RevocationService. Can you fix it?

$tokenRequest = $authMethod->createRequest($tokenRequest, $client, $params);
$tokenRequest->getBody()->write(http_build_query(array_merge($params, [
'token' => $token,
])));

@drupol
Copy link
Contributor Author

drupol commented Mar 11, 2021

Fixed!

@thomasvargiu thomasvargiu merged commit 20d075e into facile-it:master Mar 11, 2021
@thomasvargiu
Copy link
Member

Thanks @drupol

@drupol drupol deleted the fix-request-building branch March 11, 2021 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants