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

Add token refresh endpoints to identity #47228

Closed
Tracked by #47288
halter73 opened this issue Mar 15, 2023 · 6 comments · Fixed by #48595
Closed
Tracked by #47288

Add token refresh endpoints to identity #47228

halter73 opened this issue Mar 15, 2023 · 6 comments · Fixed by #48595
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-identity Includes: Identity and providers enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-token-identity Priority:0 Work that we can't release without
Milestone

Comments

@halter73
Copy link
Member

halter73 commented Mar 15, 2023

Background and Motivation

The default timeout for bearer tokens in the BearerTokenHandler is one hour, and forcing a new login every hour is not acceptable. Unlike with cookies, we cannot implicitly refresh the bearer token by attaching an outgoing header to authenticated request to non-Identity endpoints, so we need to add a refresh token and a new /refresh endpoint that accepts the refresh_token as part of the JSON request body and returns a new bearer token and new refresh token identical to the initial /login response.

As part of this process, when paired with Identity, it will verify the security stamp (if any) and reload the ClaimsPrincipal from the database via the user store. Identity does this via a BearerTokenOptions.OnSigningIn callback.

Proposed API

// namespace Microsoft.AspNetCore.Authentication.Abstractions.dll

namespace Microsoft.AspNetCore.Authentication;

public class AuthenticationProperties
{
+     /// <summary>
+    /// If set, the token must be valid for sign in to continue.
+    /// </summary>
+    [JsonIgnore]
+    public string? RefreshToken { get; set; }
}
// Microsoft.AspNetCore.Identity.dll
namespace Microsoft.Extensions.DependencyInjection;

- public static class IdentityApiEndpointsServiceCollectionExtensions
- {
-    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services)
-        where TUser : class, new();
-    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services, Action<IdentityOptions> configure)
-        where TUser : class, new();
- }

public static class IdentityServiceCollectionExtensions
{
+    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services)
+        where TUser : class, new();
+    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services, Action<IdentityOptions> configure)
+        where TUser : class, new();
}

namespace Microsoft.AspNetCore.Identity;

- public static class IdentityApiEndpointsIdentityBuilderExtensions
- {
-    public static IdentityBuilder AddApiEndpoints(this IdentityBuilder builder);
- }

public static class IdentityBuilderExtensions
{
+    public static IdentityBuilder AddApiEndpoints(this IdentityBuilder builder);
}

+ public static class IdentityAuthenticationBuilderExtensions
+ {
+     public static AuthenticationBuilder AddIdentityBearerToken<TUser>(this AuthenticationBuilder builder)
+         where TUser : class, new()
+    public static AuthenticationBuilder AddIdentityBearerToken<TUser>(this AuthenticationBuilder builder, Action<BearerTokenOptions> configure)
+        where TUser : class, new()
+ }
// Microsoft.AspNetCore.Authentication.BearerToken.dll

namespace Microsoft.AspNetCore.Authentication.BearerToken;

public sealed class BearerTokenOptions : AuthenticationSchemeOptions
{
    public TimeSpan BearerTokenExpiration { get; set; } = TimeSpan.FromHours(1);

+     public TimeSpan RefreshTokenExpiration { get; set; } = TimeSpan.FromDays(14);

-     public ISecureDataFormat<AuthenticationTicket>? BearerTokenProtector { get; set; }\
+     public ISecureDataFormat<AuthenticationTicket>? TokenProtector { get; set; }
}

public class BearerTokenEvents
{
    public Func<SigningInContext, Task> OnMessageReceived { get; set; } = context => Task.CompletedTask;
+   public Func<SigningInContext, Task> OnSigningIn { get; set; } = context => Task.CompletedTask;

    public virtual Task MessageReceivedAsync(MessageReceivedContext context) => OnMessageReceived(context);
+   public virtual Task SigningInAsync(SigningInContext context) => OnSigningIn(context);
}

+ public class SigningInContext : PrincipalContext<BearerTokenOptions>
+ {
+     public SigningInContext(HttpContext context, AuthenticationScheme scheme, BearerTokenOptions options, ClaimsPrincipal principal, AuthenticationProperties? properties);
+
+     /// <summary>
+     /// The opaque bearer token to be written to the JSON response body as the "access_token".
+     /// If left unset, one will be generated automatically.
+     /// This should later be sent as part of the Authorization request header.
+     /// </summary>
+     public string? AccessToken { get; set; }
+
+     /// <summary>
+     /// The opaque refresh token written the to the JSON response body as the "refresh_token".
+     /// If left unset, it will be generated automatically.
+     /// This should later be sent as part of a request to refresh the <see cref="AccessToken"/>
+     /// </summary>
+     public string? RefreshToken { get; set; }
+ }

Usage Examples

In the default cookie + bearer token case where you call the AddIdentityApiEndpoints IServiceCollection extension method, the server code is unchanged from the #47227 example modulo the API renames.

// ...
builder.Services.AddIdentityApiEndpoints<IdentityUser>()
    .AddEntityFrameworkStores<ApplicationDbContext>();

var app = builder.Build();

app.MapGroup("/identity").MapIdentityApi<IdentityUser>();

app.MapGet("/requires-auth", (ClaimsPrincipal user) => $"Hello, {user.Identity?.Name}!").RequireAuthorization();
// ...

But now, if you want to enable only bearer tokens but not cookies, you should call the new AddIdentityBearerToken<TUser>() AuthenticationBuilder extension method rather than calling AddBearerToken(IdentityConstants.BearerScheme) like before so that identity can wire up the security stamp validation and the user store reloading during refresh as part of the BearerTokenOptions.OnSigningIn callback.

// ...
builder.Services.AddIdentityCore<ApplicationUser>()
    .AddEntityFrameworkStores<ApplicationDbContext>()
    .AddApiEndpoints();

builder.Services.AddAuthentication()
    .AddIdentityBearerToken<ApplicationUser>();

var app = builder.Build();
// ...

If you want to manually refresh the access token with a refresh token without the MapIdentityApi endpoints, you can call HttpContext.SignIn yourself with AuthenticationProperties.RefreshToken set:

app.MapPost("/refresh", (RefreshRequest refreshRequest) =>
{
    // This is the minimal principal that IsAuthenticated. The BearerTokenHander will recreate the full principal
    // from the refresh token if it is able. The sign in will fail without an identity name.
    var refreshPrincipal =  new ClaimsPrincipal(new ClaimsIdentity(IdentityConstants.BearerScheme));
    var properties = new AuthenticationProperties
    {
        RefreshToken = refreshRequest.RefreshToken
    };
    return TypedResults.SignIn(refreshPrincipal, properties, IdentityConstants.BearerScheme);
});

And if you manually want to do something like security stamp validation, or updating the ClaimsPrincipal to something different than when it was created, you can hook OnSigningIn manually:

authBuilder.AddBearerToken(IdentityConstants.BearerScheme, bearerOptions =>
{
    bearerOptions.Events.OnSigningIn = async signInContext =>
    {
        // Only validate the security stamp and refresh the user from the store during /refresh
        // not during the initial /login when the Principal is already newly created from the store.
        if (signInContext.Properties.RefreshToken is null)
        {
            return;
        }

        var signInManager = signInContext.HttpContext.RequestServices.GetRequiredService<SignInManager<ApplicationUser>>();

        // Reject the /refresh attempt if the security stamp validation fails which will result in a 401 challenge.
        if (await signInManager.ValidateSecurityStampAsync(signInContext.Principal) is not TUser user)
        {
            signInContext.Principal = null;
            return;
        }

        signInContext.Principal = await signInManager.CreateUserPrincipalAsync(user)
    }
});

Or the OnSigningIn can go full manual mode and create the tokens itself:

authBuilder.AddBearerToken(IdentityConstants.BearerScheme, bearerOptions =>
{
    bearerOptions.Events.OnSigningIn = signInContext =>
    {
        // In this case since the callback creates the tokens, it can be expected to validate the token and create the final
        // ClaimsPrincipal before calling SignInAsync. So it doesn't ever need to use AuthenticationProperties.RefreshToken.
        // It could continue to use AuthenticationProperties.RefreshToken by overriding the TokenProtector instead        
        signInContext.AccessToken = CreateAccessToken(signInContext.Principal, signInContext.ExpiresUtc);
        signInContext.RefreshToken = CreateRefreshToken(signInContext.Principal);
    }
});

Client

Assume httpClient, username and password are already initialized.

// The request body is: { "username": "<username>", "password": "<password>" }
var loginResponse = await httpClient.PostAsJsonAsync("/identity/login", new { username, password });

// loginResponse is similar to the "Access Token Response" defined in the OAuth 2 spec
// {
//   "token_type": "Bearer",
//   "access_token": "...",
//   "expires_in": 3600
//   "refresh_token": "...",
// }
var loginContent = await loginResponse.Content.ReadFromJsonAsync<JsonElement>();
var accessToken = loginContent.GetProperty("access_token").GetString();
var refreshToken = loginContent.GetProperty("refresh_token").GetString();

httpClient.DefaultRequestHeaders.Authorization = new("Bearer", accessToken);

// Writes "Hello, <username>!"
Console.WriteLine(await httpClient.GetStringAsync("/requires-auth"));

await Task.Delay(TimeSpan.FromHours(2));

// The bearer token should expire in 1 hour by default. We just waited 2 hours.
var unauthorizedResponse = await httpClient.GetAsync("/requires-auth");
Trace.Assert(unauthorizedResponse.StatusCode == HttpStatusCode.Unauthorized);

// The request body is: { "refreshtoken": "{refreshToken}" }
var refreshResponse = await httpClient.PostAsJsonAsync("/identity/refresh", new { refreshToken });

// refreshResponse is identical to loginResponse with new tokens
var refreshContent = await refreshResponse.Content.ReadFromJsonAsync<JsonElement>();
var refreshedAccessToken = refreshContent.GetProperty("access_token").GetString();

// Writes "Hello, <possibly updated username>!"
Console.WriteLine(await httpClient.GetStringAsync("/requires-auth"));

Alternative Designs

We could create a new derived class for AuthenticationProperties similar to OAuthChallengeProperties or GoogleChallengeProperties called BearerTokenProperties as suggested by @kevinchalet in the PR at https://github.com/dotnet/aspnetcore/pull/48595/files#r1220367158. We'd probably want to add a public static readonly string RefereshTokenKey for consistency and to be able to look for tokens on arbitrary AuthenticationProperties.

We could then consider adding public new BearerTokenProperties Properties { get; set; } to SigningInContext to hide the property with the same name from the ProperitesContext grandparent type, but that'd probably require at least shallow copying the properties from a passed-in AuthenticationProperties. The other handlers with custom properties don't appear to do this.

We could consider not approving SigningInContext.AccessToken and SigningInContext.RefreshToken and say the one true way to customize tokens is with BearerTokenOptions.TokenProtector. It's a little less flexible because you don't have easy access to the HttpContext in an ISecureDataFormat<AuthenticationTicket>, but identity doesn't need this functionality. We could always add it later.

Risks

I think the risk is minimal. This is all either moving API added in preview4 or adding new API to support the refresh token scenario.

@halter73 halter73 added area-identity Includes: Identity and providers feature-token-identity labels Mar 15, 2023
@mkArtakMSFT mkArtakMSFT added enhancement This issue represents an ask for new feature or an enhancement to an existing one and removed untriaged labels Mar 15, 2023
@mkArtakMSFT mkArtakMSFT added this to the 8.0-preview4 milestone Mar 15, 2023
@mkArtakMSFT mkArtakMSFT added the Priority:0 Work that we can't release without label Mar 17, 2023
@halter73 halter73 added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Jun 8, 2023
@ghost
Copy link

ghost commented Jun 8, 2023

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@halter73
Copy link
Member Author

API Review Notes:

  • Is it weird to use SignIn for refresh?
    • It is weird, but it is similar to normal sign in.
    • Cookies refresh the cookie during normal OnAuthenticationAsync handler if the sliding expiration.
  • Is the /refresh endpoint authenticated?
    • No. @Tratcher wonders if a second hander could authenticate this endpoint using the refresh token.
    • We provide the refresh token and access token on a single response, which makes it simpler to implement as one handler.
    • We wouldn't need the second handler to validate anything other than the /refresh endpoint.
  • How does the /refresh endpoint read to refresh token?
    • From the RefreshToken property on the JSON body.

We ran out of time before we could discuss more minor details like where the AuthenticationProperties.RefereshToken should live (assuming we keep passing the refresh token to sign in this way). I'll experiment with extracting the ClaimsPrincipal from the refresh token as part of the /refresh endpoint implementation. I like having all the token creation and validation logic in the handler, but we'll see.

@halter73 halter73 added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Jun 12, 2023
@halter73
Copy link
Member Author

halter73 commented Jun 20, 2023

Updated API Proposal:

Proposed API

// Microsoft.AspNetCore.Identity.dll
namespace Microsoft.Extensions.DependencyInjection;

- public static class IdentityApiEndpointsServiceCollectionExtensions
- {
-    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services)
-        where TUser : class, new();
-    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services, Action<IdentityOptions> configure)
-        where TUser : class, new();
- }

public static class IdentityServiceCollectionExtensions
{
+    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services)
+        where TUser : class, new();
+    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services, Action<IdentityOptions> configure)
+        where TUser : class, new();
}

namespace Microsoft.AspNetCore.Identity;

- public static class IdentityApiEndpointsIdentityBuilderExtensions
- {
-    public static IdentityBuilder AddApiEndpoints(this IdentityBuilder builder);
- }

public static class IdentityBuilderExtensions
{
+    public static IdentityBuilder AddApiEndpoints(this IdentityBuilder builder);
}

+ public static class IdentityAuthenticationBuilderExtensions
+ {
+     public static AuthenticationBuilder AddIdentityBearerToken<TUser>(this AuthenticationBuilder builder)
+         where TUser : class, new()
+    public static AuthenticationBuilder AddIdentityBearerToken<TUser>(this AuthenticationBuilder builder, Action<BearerTokenOptions> configure)
+        where TUser : class, new()
+ }
// Microsoft.AspNetCore.Authentication.BearerToken.dll

namespace Microsoft.AspNetCore.Authentication.BearerToken;

public sealed class BearerTokenOptions : AuthenticationSchemeOptions
{
    public TimeSpan BearerTokenExpiration { get; set; } = TimeSpan.FromHours(1);
+     public TimeSpan RefreshTokenExpiration { get; set; } = TimeSpan.FromDays(14);

     public ISecureDataFormat<AuthenticationTicket>? BearerTokenProtector { get; set; }
+     public ISecureDataFormat<AuthenticationTicket>? RefreshTokenProtector { get; set; }
}

Usage examples

The implementation of /refresh becomes more complicated, but it doesn't do the nameless ClaimsPrincipal hack anymore.

app.MapPost("/refresh", async (RefreshRequest refreshRequest, IOptionsMonitor<BearerTokenOptions> optionsMonitor, TimeProvider timeProvider, SignInManager<TUser> signInManager) =>
{
    var identityBearerOptions = optionsMonitor.Get(IdentityConstants.BearerAndApplicationScheme);
    var refreshTokenProtector = identityBearerOptions.RefreshTokenProtector ?? throw new ArgumentException();
    var refreshTicket = refreshTokenProtector.Unprotect(refreshRequest.RefreshToken);

    if (refreshTicket?.Properties?.ExpiresUtc is not { } expiresUtc || timeProvider.GetUtcNow() >= expiresUtc)
    {
        return TypedResults.Challenge();
    }

    // Reject the /refresh attempt if the security stamp validation fails which will result in a 401 challenge.
    if (await signInManager.ValidateSecurityStampAsync(refreshTicket.Principal) is not TUser user)
    {
        return TypedResults.Challenge();
    }

    var newPrincipal = await signInManager.CreateUserPrincipalAsync(user);

    return TypedResults.SignIn(newPrincipal, authenticationScheme: IdentityConstants.BearerScheme);
});

Alternative Designs

  • The original proposal.
  • Make both BearerTokenProtector and RefreshTokenProtector non-nullable and set it in IConfigureNamedOptions early instead of IPostConfigureOptions.

@halter73 halter73 added api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews and removed api-needs-work API needs work before it is approved, it is NOT ready for implementation labels Jun 20, 2023
@ghost
Copy link

ghost commented Jun 20, 2023

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@halter73
Copy link
Member Author

halter73 commented Jun 20, 2023

API Review Note:

  • We could make the protectors non-nullable and have the getters throw if it hasn't been set yet.
  • Overall, this feels a lot cleaner than having the /refresh endpoint create a fake ClaimsPrincipal to do a refresh sign in.
  • We're also fine with the extension method moves.

API Approved!

// Microsoft.AspNetCore.Identity.dll
namespace Microsoft.Extensions.DependencyInjection;

- public static class IdentityApiEndpointsServiceCollectionExtensions
- {
-    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services)
-        where TUser : class, new();
-    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services, Action<IdentityOptions> configure)
-        where TUser : class, new();
- }

public static class IdentityServiceCollectionExtensions
{
+    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services)
+        where TUser : class, new();
+    public static IdentityBuilder AddIdentityApiEndpoints<TUser>(this IServiceCollection services, Action<IdentityOptions> configure)
+        where TUser : class, new();
}

namespace Microsoft.AspNetCore.Identity;

- public static class IdentityApiEndpointsIdentityBuilderExtensions
- {
-    public static IdentityBuilder AddApiEndpoints(this IdentityBuilder builder);
- }

public static class IdentityBuilderExtensions
{
+    public static IdentityBuilder AddApiEndpoints(this IdentityBuilder builder);
}

+ public static class IdentityAuthenticationBuilderExtensions
+ {
+     public static AuthenticationBuilder AddIdentityBearerToken<TUser>(this AuthenticationBuilder builder)
+         where TUser : class, new()
+    public static AuthenticationBuilder AddIdentityBearerToken<TUser>(this AuthenticationBuilder builder, Action<BearerTokenOptions> configure)
+        where TUser : class, new()
+ }
// Microsoft.AspNetCore.Authentication.BearerToken.dll

namespace Microsoft.AspNetCore.Authentication.BearerToken;

public sealed class BearerTokenOptions : AuthenticationSchemeOptions
{
    public TimeSpan BearerTokenExpiration { get; set; } = TimeSpan.FromHours(1);
+     public TimeSpan RefreshTokenExpiration { get; set; } = TimeSpan.FromDays(14);

-     public ISecureDataFormat<AuthenticationTicket>? BearerTokenProtector { get; set; }
+     public ISecureDataFormat<AuthenticationTicket> BearerTokenProtector { get; set; }
+     public ISecureDataFormat<AuthenticationTicket> RefreshTokenProtector { get; set; }
}

@halter73 halter73 added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Jun 20, 2023
@davidfowl
Copy link
Member

Very nice!

@ghost ghost locked as resolved and limited conversation to collaborators Jul 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-identity Includes: Identity and providers enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-token-identity Priority:0 Work that we can't release without
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@halter73 @davidfowl @mkArtakMSFT and others