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

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

Closed

Conversation

Stratus3D
Copy link
Contributor

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.

@Stratus3D Stratus3D changed the title Don't send the same parameters in query string and JWT for redirect URL Don't send the same parameters in query string and JWT for PAR request Jul 11, 2024
@Stratus3D Stratus3D force-pushed the tb/dont-send-params-in-qs-and-jwt branch from 8f33b6a to 8800d0b Compare July 11, 2024 19:14
@Stratus3D Stratus3D force-pushed the tb/dont-send-params-in-qs-and-jwt branch from 8800d0b to 7666426 Compare July 11, 2024 19:22
@maennchen
Copy link
Member

@Stratus3D Thanks for the detailed look! ❤️

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
Copy link
Member

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

@maennchen
Copy link
Member

Merged manually in 3b0b522

@maennchen maennchen closed this Jul 26, 2024
@Stratus3D
Copy link
Contributor Author

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

@maennchen
Copy link
Member

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

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.

None yet

2 participants