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

OAuth 2 refresh token support #8175

Open
nelle775 opened this issue Mar 4, 2019 · 3 comments
Open

OAuth 2 refresh token support #8175

nelle775 opened this issue Mar 4, 2019 · 3 comments
Labels
affected-medium This issue impacts approximately half of our customers area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-major This label is used by an internal tool
Milestone

Comments

@nelle775
Copy link

nelle775 commented Mar 4, 2019

The OAuthHandler class does not provide any option to expire the underlying Cookie ticket upon expiry of the Bearer Token, also it does not have any support for Refresh tokens other than storing the value in AuthenticationProperties.

I Suggest the following:
Add a property: bool ExpireUponTokenExpiry to RemoteAuthenticationOptions

On authenticating ticket, check this property
if false just return AuthenticateResult.Success upon validating the ticket
if true and bearer token is not expired return AuthenticateResult.Success
if true and bearer token is expired
-> if refresh_token isSet in AuthenticationProperties, Exhange the refresh token for new bearer token through http backchannel,
-> if no refresh_token is set, start a new OAuth authentication flow, with RedirectResult

@blowdart blowdart added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Mar 4, 2019
@Tratcher
Copy link
Member

Tratcher commented Mar 4, 2019

The UseTokenLifetime OIDC option already does part of this, tying the cookie lifetime to the token lifetime, though most people don't like it and have turned it off.

The biggest gap in the suggested design is that RemoteAuth.AuthenticateAsync doesn't run every request, it only runs during sign-in. After sign-in it's CookieAuth.AuthenticateAsync that runs every request and it doesn't know about the upstream details.

The other big gap is that refresh is not a well standardized behavior for OAuth. I tried it with a few of our common providers and only two out of five were able to share any of their logic. This was my last experiment at refresh: https://github.com/aspnet/AspNetCore/blob/39e52578d354a6a7abb3f6169d5ac2174ffe4551/src/Security/Authentication/samples/SocialSample/Startup.cs

We do need to provide better refresh support, but I doubt it would ever be completely automatic. A big reason is concurrency. If you have any parallel requests from the same user they would all start attempting the refresh as soon as the token expired. A better pattern I've seen is to only refresh the token when you go to use it. That cuts down the concurrency as you only use that token on a small subset of requests.

@Eilon Eilon added this to the Backlog milestone Mar 26, 2019
@Eilon
Copy link
Member

Eilon commented Mar 26, 2019

Placing in backlog so we can consider in a future version.

@Tratcher Tratcher added affected-medium This issue impacts approximately half of our customers enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-major This label is used by an internal tool labels Nov 10, 2020 — with ASP.NET Core Issue Ranking
@AdamWyzgol
Copy link

really still in backlog? since 2019?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-medium This issue impacts approximately half of our customers area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-major This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

5 participants