Skip to content

Provide Cookie implementation of AuthorizationRequestRepository #6374

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

Closed
jgrandja opened this issue Jan 8, 2019 · 17 comments
Closed

Provide Cookie implementation of AuthorizationRequestRepository #6374

jgrandja opened this issue Jan 8, 2019 · 17 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply

Comments

@jgrandja
Copy link
Contributor

jgrandja commented Jan 8, 2019

We should consider providing a Cookie based implementation of AuthorizationRequestRepository.

@afrancoc2000
Copy link

Hi @jgrandja, How many votes do you need?

@jgrandja
Copy link
Contributor Author

jgrandja commented May 6, 2019

Hi @afrancoc2000. There is no specific number of votes we target but this one has enough to start prioritizing. However, we are pretty jammed with higher priority tasks that need to get into the upcoming 5.2 release so this one will take lower priority. It would be helpful if someone from the community would contribute a PR for this as I believe it would be fairly trivial. Would you be interested in submitting a PR?

@jgrandja
Copy link
Contributor Author

jgrandja commented May 6, 2019

@afrancoc2000 Can you provide details on why you require a cookie implementation of AuthorizationRequestRepository? I'd like to understand your use case before we proceed with any work.

Anyone else interested in this feature, I'd like to hear from you as well and your use case so I can understand the reasoning for needing such an implementation.

@afrancoc2000
Copy link

Sure the reason is microservices applications, today the best way of handling this type of apps is making them stateless so any app can respond to any request.

The authorization can be easily saved and restored from a jwt cookie thats better than having a session because for sessions I need an extra component like a redis cache, that means more infrastructure, more points of failure and the posiblility of conflicts between sessions, problems that could be easily solved by replacing a couple lines of code.

Also that allows me to link an external frontend more easily just by passing the jwt cookie that is a standard in the industry.

I would love to help I was looking at the example of @naturalprogrammer in this links: link 1, link 2 but found out that storing the OAuth2AuthorizationRequest is not enough and I would prefer to do the implementation with a jwt cookie saving and restoring the security context, haven't got till there yet.

@jgrandja
Copy link
Contributor Author

jgrandja commented May 6, 2019

@afrancoc2000 The AuthorizationRequestRepository is responsible for persisting the OAuth2AuthorizationRequest as part of the authorization_code grant flow. Please see Authorization Request for more details.

It does not save a Jwt as it seems that is your understanding?

The authorization can be easily saved and restored from a jwt cookie

Please correct me if I'm misunderstanding your comments?

@fnaeem78
Copy link

I was hoping to use the cookie based approach, because I am trying to use spring-oidc to authenticate to multiple providers, but wanted to do it in a seemless manner. If I implement a cookie-base request repository, and maybe write a cookie from javascript, when the response comes back, it will have a request in the cookie format and will let the flow go to my endpoint instead of intercepting it and returning me a the oidc login page.

@jgrandja jgrandja added this to the 5.3.x milestone Nov 18, 2019
@jgrandja jgrandja modified the milestones: 5.3.x, 5.4.x Feb 10, 2020
@jgrandja jgrandja removed this from the 5.4.x milestone Mar 12, 2020
@Gittenburg
Copy link

Gittenburg commented May 21, 2020

Would simply saving the serialized OAuth2AuthorizationRequest to a cookie and deserializing it on load be save? Or does leaking / allowing tampering of the OAuth2AuthorizationRequest introduce security issues?

@rwinch
Copy link
Member

rwinch commented May 27, 2020

We would not want to perform Java serialization/deserialization as that can lead to a lot of different types of attacks. The cookie would need to be written as a String that did not allow attackers to control the Java objects that were being created.

@Gittenburg
Copy link

Alright, is there a need to encrypt or sign the cookie?

@rwinch
Copy link
Member

rwinch commented May 28, 2020

I cannot think of any reason to do this. The user can make changes to the cookie, but it only impacts their own security and not much different than tampering with redirect URLs sent from the server.

@Gittenburg
Copy link

If we want to avoid (de)serialization, how do we deal with attributes and additionalParameters of OAuth2AuthorizationRequest? Because both can contain java.lang.Object?

@rwinch
Copy link
Member

rwinch commented Jun 1, 2020

You would need to use Spring's Conversion APIs to convert it to/from a String. You would avoid including the type information in the String as that is what is used for deserialization attacks.

@milanov
Copy link

milanov commented Dec 10, 2020

Any updates on this issue?

@jgrandja
Copy link
Contributor Author

jgrandja commented Dec 11, 2020

@milanov There are no updates. Quite honestly, I'm reconsidering on NOT providing this feature. There are a few vulnerabilities around Cookie tampering so I'm hesitant to proceed with this feature.

If the main motivation for this feature is to achieve statelessness, then providing a Cookie based implementation of AuthorizationRequestRepository will not be sufficient enough. Access tokens are associated with OAuth2AuthorizedClient, which are stored via OAuth2AuthorizedClientService. The current implementations are in-memory and HttpSession - so this would need to be addressed to achieved statelessness. There are other areas that would need to be addressed as well, e.g. SecurityContext etc.

The key point I want to emphasize is that storing secret/private information on the server is the most secure way and transporting secret/private information over secure and/or un-secure channels is NOT the most secure in certain scenarios.

I'd like to hear more from the community on the motivation for this feature, in case I'm missing something.

@milanov
Copy link

milanov commented Dec 22, 2020

In our case the motivation is exactly statelessness. If login is attempted on one of our instances, then performed at an IdP and the user is redirected back, we'll have to resort to special measures to ensure that the same instance handles the request, in order to have matching state and complete the login process.

I guess we can use a shared storage-backed AuthorizationRequestRepository (for example in Redis), which we can implement ourselves. On the other hand, I assume that this is a fairly common case in a multi-instance environment (as indicated by the number of votes as well), and so a standard solution could exist out-of-the-box.

@jgrandja
Copy link
Contributor Author

jgrandja commented Jan 5, 2021

@milanov

I guess we can use a shared storage-backed AuthorizationRequestRepository (for example in Redis)

This can be achieved today with the help of Spring Session.

Please see gh-7889 for complete details on how to setup and configure to support application clustering.

@jgrandja
Copy link
Contributor Author

jgrandja commented May 18, 2021

This feature will not be implemented for the reasons mentioned in this comment. If an application requires this, it would be fairly trivial to implement a custom AuthorizationRequestRepository, please see gh-8621.

@jgrandja jgrandja added status: declined A suggestion or change that we don't feel we should currently apply and removed type: enhancement A general enhancement labels May 18, 2021
@jgrandja jgrandja self-assigned this May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants