spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.72k stars 5.87k forks source link

Consider adding support for pushed authorization requests (PAR, RFC 9126) #11301

Open dpulrichth opened 2 years ago

dpulrichth commented 2 years ago

Expected Behavior

RFC 9126 introduces pushed authorization requests (PAR) for OAuth. In essence, pushed authorization requests allow the client to send authorization request information to the authorization server through back channel communication rather than through the user agent. Not exposing the authorization request data to the user agent makes it harder for an attacker to manipulate said info, e.g. change the requested scopes or tamper with any of the additional query parameters defined in one of the many OAuth enhancement RFCs. Once the client has successfully pushed the authorization request data, the AS responds with a unique identifier that the client must then send as a query parameter during a normal authorization request. From there everything works as usual.

PAR are part of the upcoming FAPI 2.0 and more and more AS/OP products and frameworks such as Keycloak, Forgerock and Nimbus OAuth are adding support for PAR. It would be great if the Spring Security OAuth Client would support PAR out of the box. PAR endpoints can be discovered from the AS/OP published metadata. Ideally it would be as simple as setting a boolean flag in the client configuration to configure the client to use PAR.

Current Behavior

PAR not supported in Spring Security OAuth Client.

Context

Any and all information being transported through the user agent during an authorization request can be manipulated. JWT-Secured Authorization Request (JAR) address this but can lead to quite long query strings which can cause problems with some AS or the web server on which they are hosted. Signed and encrypted JWTs are required to add tamper proofing and confidentiality, respectively. This adds more complexity on the client side. PAR finds a middle ground as it adds a good degree of confidentiality and tamper proofing by not passing the sensitive authorization data through the user agent. For added security, PAR can be combined with JAR to alleviate the long request string problem.

sjohnr commented 2 years ago

Hi @dpulrichth, welcome to the project!

I'm a bit unfamiliar as of yet with RFC 9126, so I may ask a few obvious questions. Just for context, can you say more about what you mean here:

Any and all information being transported through the user agent during an authorization request can be manipulated.

If this is covered in the spec, can you provide that reference? Also:

In essence, pushed authorization requests allow the client to send authorization request information to the authorization server through back channel communication rather than through the user agent.

Is the spec at all clear on the details of this back channel? For example, is there an official endpoint for exchanging this information, or is it out of scope of the specification to provide some/all of the mechanism for "pushing" the authorization data?

Lastly, do you have an idea of what might be required to support this feature? For example, would a new API need to be introduced to persist information on the client side?

If the above question is not easy to answer at this time, one thing that would be helpful for discovering the scope of the changes necessary to support the feature would be to build a proof of concept using existing customization options. If you or another community member were able to provide a working sample that adds support for PAR on top of Spring Security, that would be very informative. If for some reason it's not possible, that would also be fairly informative.

dpulrichth commented 2 years ago

Hi @sjohnr

thanks for the feedback. I'll try and clarify

I'm a bit unfamiliar as of yet with RFC 9126, so I may ask a few obvious questions. Just for context, can you say more about what you mean here: Any and all information being transported through the user agent during an authorization request can be manipulated.

Since the authorization request data is usually sent by a HTTP-GET request to the authorization server it is easily read by anyone who controls the browser, e.g. using the developer tools or just copying the URL with the authorization request. Given tools such as Burp Suite or a compromised user agent an attacker could even manipulate the original request to change some of its content such as the requested scopes.

Even without manipulation, accidental leaking of information (for instance by logging it in HTTP-Accesslogs, browser console or by being visible in the browser history) can already be problematic if the request data is sensitive.

The spec gives a brief overview of the challenges of sending authorization request data through the user agent in the first section.

Is the spec at all clear on the details of this back channel? For example, is there an official endpoint for exchanging this information, or is it out of scope of the specification to provide some/all of the mechanism for "pushing" the authorization data?

Yes, the spec adds the pushed_authorization_request_endpoint entry to the Authorization Server metadata defined in RFC 8414 in section 5. It also gives a detailed description for both the request and responses in sections 2 to 4.

Lastly, do you have an idea of what might be required to support this feature? For example, would a new API need to be introduced to persist information on the client side?

I am very much new to both Spring and Spring Security and I really couldn't say if a new API would be required. The client would need the ability to

From there on it's back to standard OAuth.

If you or another community member were able to provide a working sample that adds support for PAR on top of Spring Security, that would be very informative. If for some reason it's not possible, that would also be fairly informative.

I could give it a try but seeing as I'm fairly new to Spring it would take me quite some time. That is not to say it's not possible using customizations, just that I lack the knowledge on how to wield the customization options to achieve this. If anyone else has an idea where to start I'd appreciate it.

sjohnr commented 2 years ago

Thanks for the additional info @dpulrichth!

If anyone else has an idea where to start I'd appreciate it.

Yes, I think it's perfectly fine to open this up for a community member to help out and prototype something.

erlendfg commented 1 year ago

Thanks for opening up this issue. This is actually something several of Norwegian IdPs highly recommend for clients with high security requirements, for instance ID-porten: https://docs.digdir.no/docs/idporten/oidc/oidc_protocol_par

I'm trying to implement this in an AuthorizationRequestResolver, which implements OAuth2AuthorizationRequestResolver. I guess this is the correct starting point.

erlendfg commented 1 year ago

I'm trying to implement this in an AuthorizationRequestResolver, which implements OAuth2AuthorizationRequestResolver. I guess this is the correct starting point.

It seems that this should be added to OAuth2AuthorizationRequestRedirectFilter instead. I managed to get a PAR response when I tried this out in my AuthorizationRequestResolver, but it didn't work to add the request_uri param this way:

String requestUri = getRequestUriFromParEndpoint(authorizationRequest, authorizationRequestAttributes,
                    request);
    return OAuth2AuthorizationRequest.from(authorizationRequest)
        .authorizationRequestUri(requestUri)
        .attributes(authorizationRequestAttributes)
        .build();

I tried to extend OAuth2AuthorizationRequestRedirectFilter and build the redirectUrl in my own sub class, but I haven't found a proper way to configure this class yet. Note sure if it's possible to extend it at all. This is how the redirect uri should look like: GET /authorize?request_uri=urn:idporten:JF38qJvAge0yvmYC4Hw3P0NXCahVkqlpeVCm_4K0paw&client_id=s6BhdRkqt3

jgrandja commented 1 year ago

@erlendfg The correct customization point is OAuth2AuthorizationRequestResolver. The DefaultOAuth2AuthorizationRequestResolver provides the capability to completely override OAuth2AuthorizationRequest.getAuthorizationRequestUri() using OAuth2AuthorizationRequest.Builder.authorizationRequestUri(String) OR OAuth2AuthorizationRequest.Builder.authorizationRequestUri(Function<UriBuilder, URI>). See the reference documentation for an example.

erlendfg commented 1 year ago

@jgrandja, I only managed to access the builder this way:

OAuth2AuthorizationRequest.Builder builder = OAuth2AuthorizationRequest.authorizationCode()
                        .authorizationRequestUri(authorizationRequestUri);
return builder.build();

But the validation failed, so I had to add state, clientId and authorizationUri, even thought these parameters are not used. After I added these, it still didn't work until I also added: .attributes((attrs) -> attrs.put(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()))

Now I get redirected to the IdP, but something fails when the token endpoint is being called. Not sure why, but I'm trying to figure it out. Maybe because the nonce, which was posted to the PAR endpoint, is not stored for the authentication part (not sure).

erlendfg commented 5 months ago

@jgrandja, I'm afraid that this is not so simple. First of all, this is how an authorize request should look like if the other parameters have been sent to a PAR endpoint: /authorize?request_uri=abcdef&client_id=1234

This is explained here with an example from one of our IdPs: https://docs.digdir.no/docs/idporten/oidc/oidc_protocol_par

So I tried to do what you suggested:

String requestUri = getRequestUriFromParEndpoint(…);
parameters.put("request_uri", requestUri);

return OAuth2AuthorizationRequest
  .authorizationCode() // Does not compile without, but we shouldn't add it!
  .authorizationUri(authorizationRequest.getAuthorizationUri())
  .state(authorizationRequest.getState()) // Shouldn't be added!
  .additionalParameters(parameters)
  .clientId(authorizationRequest.getClientId())
  .build();

This constructs the following authorize url, which is almost correct: /authorize?response_type=code&client_id=*******&state=*****&nonce=*****&request_uri=****

First, response_type=code shouldn't be added to the request, but the code above will not compile without an .authorizationCode(). Second, HttpSessionOAuth2AuthorizationRequestRepository complains about missing state, and this shouldn't be added as well. This is supposed to be sent to the PAR endpoint instead. nonce is also added for some reason.

When I tried to return the above, I'm getting the following error: java.lang.IllegalArgumentException: registrationId cannot be empty. This is from HttpSessionOAuth2AuthorizationRequestRepository.

adrien-dedecker commented 3 months ago

Any update on this ?

netroms commented 6 days ago

Hi @erlendfg! Is it possible to contact you regarding your test implementation? We at DHIS2 really need this feature too, in order to use ID-porten.

erlendfg commented 3 days ago

Hi @erlendfg! Is it possible to contact you regarding your test implementation? We at DHIS2 really need this feature too, in order to use ID-porten.

Yes, send an email to erlendfg (at) uio.no.