facile-it / php-openid-client

PHP OpenID Client
35 stars 7 forks source link

Fix token request body building #2

Closed drupol closed 3 years ago

drupol commented 3 years ago

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 commented 3 years ago

Thank you @drupol,

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

https://github.com/facile-it/php-openid-client/blob/59bb8ebd026abcc6db7632c5c0bdddccaf04297d/src/Service/RevocationService.php#L51-L54

drupol commented 3 years ago

Fixed!

thomasvargiu commented 3 years ago

Thanks @drupol