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.

Sign up for GitHub

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

Jump to bottom

Oauth2 Middleware scalability limit and resilience issues with code grant flow and token storage #2635

Open
drewby opened this issue Mar 6, 2023 · 5 comments · May be fixed by #2967
Open

Oauth2 Middleware scalability limit and resilience issues with code grant flow and token storage #2635

drewby opened this issue Mar 6, 2023 · 5 comments · May be fixed by #2967
Labels
kind/bug Something isn't working P1 pinned Issue does not get stale
Milestone
v1.14

Comments

@drewby
Copy link
Contributor

drewby commented Mar 6, 2023 •
edited

Expected Behavior

Oauth2 Middleware should scale to multiple instances without the use of request affinity. It should also be resilient to restarts.

Actual Behavior

Oauth2 Middleware uses in-memory session state to store information during auth code grant flow and to store the client token at the end of authorization. This requires requests to always return to the same instance of the dapr sidecar and offers no resilience in the case of a restart of the daprd instance.

Steps to Reproduce the Problem

For scale:

  1. Configure daprd with a scaled deployment, no affinity on ingress
  2. After authorization, make multiple requests
  3. Some requests will require new authentication

For resilience:

  1. Use Oauth2 component to authenticate
  2. Restart daprd
  3. Next request will require authorization

Proposals

Use cookies instead of session state to store data during auth code grant flow and the client token.
Enable session state to be stored in a cache (Redis, etc).

Release Note

RELEASE NOTE: FIX Oauth2 Middleware resilient storage of client token
@drewby drewby added the kind/bug Something isn't working label Mar 6, 2023
@drewby drewby changed the title Oauth2 Middleware uses in-memory session state for code grant flow and token storage Oauth2 Middleware scalability limit and resilience issues with code grant flow and token storage Mar 6, 2023
@ItalyPaleAle ItalyPaleAle added the pinned Issue does not get stale label Mar 6, 2023
@ItalyPaleAle ItalyPaleAle added this to the v1.11 milestone Mar 6, 2023
@ItalyPaleAle ItalyPaleAle added the P1 label Mar 6, 2023
@ItalyPaleAle
Copy link
Contributor

This should be a blocker to make the components stable (#2621 and #2622)

However, I don't think we should persist the tokens anywhere, as that requires a data store and to query the data store on every request.

Instead, we should issue our own JWT, which is a bearer token so can be verified by Dapr without querying a database. The JWT will contain the token issued by the OAuth2 server and will be encrypted (JWE) in case the token issued by the server contains confidential information.

This was referenced Mar 6, 2023
Open
Open
@ItalyPaleAle ItalyPaleAle mentioned this issue Mar 26, 2023
Open
@berndverst berndverst modified the milestones: v1.11, v1.12 May 30, 2023
This was referenced Jun 30, 2023
Open
Closed
@ItalyPaleAle
Copy link
Contributor

@drewby I have a POC for the OAuth2 middleware that stores tokens in cookies self-contained. Although it works, I'm running into limitations due to the fact that Azure AD tokens can be very large. Would love your feedback too on #2963

This was referenced Jul 3, 2023
Closed
Draft
@drewby
Copy link
Contributor Author

drewby commented Jul 6, 2023 •
edited

@drewby I have a POC for the OAuth2 middleware that stores tokens in cookies self-contained. Although it works, I'm running into limitations due to the fact that Azure AD tokens can be very large. Would love your feedback too on #2963

I started to comment here and then saw your link. I will comment on the PR.

@ItalyPaleAle
Copy link
Contributor

Final one is #2967

as for splitting, we can’t do that because the 4KB limit is per-domain.

regarding safety, the tokens are stored in cookies as encrypted JWTs

@drewby
Copy link
Contributor Author

drewby commented Jul 6, 2023

Got it. I saw my question/comment were all addressed in the PR.

@ItalyPaleAle ItalyPaleAle modified the milestones: v1.12, v1.13 Aug 2, 2023
@ItalyPaleAle ItalyPaleAle modified the milestones: v1.13, v1.14 Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/bug Something isn't working P1 pinned Issue does not get stale
Projects
None yet
Milestone
v1.14
3 participants
@ItalyPaleAle @drewby @berndverst