erlef / oidcc

OpenId Connect client library in Erlang & Elixir
https://hexdocs.pm/oidcc
Apache License 2.0
183 stars 49 forks source link

Don't send the same parameters in query string and JWT for PAR request #360

Closed Stratus3D closed 3 months ago

Stratus3D commented 4 months ago

I tried using version 3.2.0 of this library along with oidcc_plug to authenticate my users via Okta and got the following error when the library would send the PAR to Okta:

%{"error" => "invalid_request", "error_description" => "The request contained multiple parameters with the same name."}

At first I was confused because I didn't see any duplicate parameters. In the query string I had: client_id, client_secret, redirect_uri, request, response_type, scope.

And the JWT there was: aud, client_id, code_challenge, code_challenge_method, exp, iat, iss, jti, nbf, nonce, redirect_uri, response_type, scope.

No duplicates in either list.

Then I read OAuth 2.0 Pushed Authorization Requests - RFC-9126 section 3:

The rules for processing, signing, and encryption of the Request Object as defined in JAR [RFC9101] apply. Request parameters required by a given client authentication method are included in the application/x-www-form-urlencoded request directly and are the only parameters other than request in the form body (e.g., mutual TLS client authentication [RFC8705] uses the client_id HTTP request parameter, while JWT assertion-based client authentication [RFC7523] uses client_assertion and client_assertion_type). All other request parameters, i.e., those pertaining to the authorization request itself, MUST appear as claims of the JWT representing the authorization request.

So at the very least the query string parameters must be a subset of the fields in the JWT.

Then I looked at the OpenID Connect spec:

When the request parameter is used, the OpenID Connect request parameter values contained in the JWT supersede those passed using the OAuth 2.0 request syntax. However, parameters MAY also be passed using the OAuth 2.0 request syntax even when a Request Object is used; this would typically be done to enable a cached, pre-signed (and possibly pre-encrypted) Request Object value to be used containing the fixed request parameters, while parameters that can vary with each request, such as state and nonce, are passed as OAuth 2.0 parameters.

I'm not familiar with the "OAuth 2.0 request syntax", but this leads me to believe that if a field is present in both the request JWT and the query string the query string parameter is ignored. If that is the case it can be omitted from the query string. However, it appears the scope parameter may always be required:

Even if a scope parameter is present in the Request Object value, a scope parameter MUST always be passed using the OAuth 2.0 request syntax containing the openid scope value to indicate to the underlying OAuth 2.0 logic that this is an OpenID Connect request.

The changes in this PR allow me to send requests to Okta that it accepts as valid. I'm new to the the OIDC spec so I can't say if this is a bug with Okta or this library.

maennchen commented 4 months ago

@Stratus3D Thanks for the detailed look! :heart:

I'll have to make sure that the combination in this PR is correct by re-running the conformance tests. I'm however a bit busy at the moment and didn't have the time yet to have a look. I'll try to get this done until the end of the week.

maennchen commented 3 months ago

Sorry for the long wait. It passes certification. Merging as soon as CI passes.

maennchen commented 3 months ago

Merged manually in 3b0b5221a0c88ad733a3ffc769b5acada70c2afd

Stratus3D commented 3 months ago

@maennchen any idea when a new version of oidcc will be tagged that includes these changes?

maennchen commented 3 months ago

Once I get feedback on #359 (if that happens in a reasonable timeframe)