[API Proposal]: AuthenticationProperties.RotateSessionKey, to rotate the cookie session identifier on sign-in

Open
#70,373 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Feature
Clarity
Clearly specified
Activity status
Active
Tech stack
csharp

Research direction

This is an API design proposal covering the AuthenticationProperties.RotateSessionKey member and its interaction with cookie session storage. Start with docs/APIReviewProcess.md and the proposal’s linked implementation PR #70105, especially CookieAuthenticationHandler.cs and PublicAPI.Unshipped.txt. Done means an area owner reviews the API shape and the issue reaches the repository’s API review process; the proposal notes that an owner must pick it up.

Written by the indexing model from the issue text.

Description

api-proposal area-auth

Background and Motivation

Filed as the separate API proposal that docs/APIReviewProcess.md asks for when an implementation pull request adds public API. It covers the one new member in #70105, which implements #47503.

When CookieAuthenticationOptions.SessionStore is configured, CookieAuthenticationHandler.ReadCookieTicket caches the session key that arrived on the request, and HandleSignInAsync then renews the ticket under that same key instead of minting a new one. That behaviour was introduced deliberately, for #22135, so that concurrent requests carrying the same cookie keep working.

The consequence is that a sign-in which changes who the principal is stores the new ticket under a key an earlier cookie already holds, so the earlier cookie keeps resolving to the new ticket. #47503 reports this as a session fixation concern, and OWASP's session management guidance asks for the session identifier to be renewed on a change of privilege level.

#67049 fixed the sign-out leg by clearing the cached key, which covers SignOutAsync followed by SignInAsync in one request. It does not cover a bare SignInAsync, which is the path ASP.NET Core Identity takes on password sign-in: SignInManager.SignInWithClaimsAsync calls Context.SignInAsync without signing the application scheme out first.

The two cases cannot be told apart by inspecting the principals. The privilege change in #47503 signs in the same subject with different claims, and so does the refresh case in #22135. Any automatic rule based on comparing the incoming and outgoing principal would either miss the privilege change or break the refresh, so the caller has to say which one it means. An opt-in flag on AuthenticationProperties is what #67049 suggested leaving to this design discussion.

Proposed API

 namespace Microsoft.AspNetCore.Authentication;

 public class AuthenticationProperties
 {
+    [JsonIgnore]
+    public bool? RotateSessionKey { get; set; }
 }

PublicAPI.Unshipped.txt for Microsoft.AspNetCore.Authentication.Abstractions (src/Http/Authentication.Abstractions/src/PublicAPI.Unshipped.txt):

 #nullable enable
+Microsoft.AspNetCore.Authentication.AuthenticationProperties.RotateSessionKey.get -> bool?
+Microsoft.AspNetCore.Authentication.AuthenticationProperties.RotateSessionKey.set -> void

The shape follows the existing bool? properties on the same type. Like AllowRefresh, it is backed by the Items dictionary through the existing GetBool/SetBool helpers under a new .rotate key, and carries [JsonIgnore] as RedirectUri, IssuedUtc, ExpiresUtc and AllowRefresh already do.

null and false leave today's behaviour untouched: the key is still renewed in place and #22135 stays fixed. No existing caller and no existing test changes behaviour.

Usage Examples

Rotating the session identifier on an Identity password sign-in, so a cookie captured before the sign-in stops resolving:

await _signInManager.SignInWithClaimsAsync(
    user,
    new AuthenticationProperties { RotateSessionKey = true },
    additionalClaims: []);

Directly on the scheme:

await HttpContext.SignInAsync(
    CookieAuthenticationDefaults.AuthenticationScheme,
    principal,
    new AuthenticationProperties { RotateSessionKey = true });

Leaving it unset keeps the renew-in-place behaviour that concurrent requests depend on:

// Refresh of the same principal: the session key is reused, as it is today.
await HttpContext.SignInAsync(CookieAuthenticationDefaults.AuthenticationScheme, principal);

Alternative Designs

Two were considered in #70105 and not taken:

  1. Rotate unconditionally on SignInAsync when a SessionStore is configured. This is the behaviour OWASP asks for and needs no new API, but it is a breaking change for anyone relying on the renew semantics #22135 introduced, for the reason given above: the refresh case and the privilege-change case are indistinguishable from the principals alone.

  2. Put the switch on CookieAuthenticationOptions rather than on AuthenticationProperties. Simpler, and it needs no change to Microsoft.AspNetCore.Authentication.Abstractions, but it cannot be scoped to the one sign-in that changes the principal: it would apply to every sign-in on the scheme, including the refreshes, which is alternative 1 behind an option.

A third shape worth the reviewers' attention, which the implementation does not take: the property lives on AuthenticationProperties, which is shared by every handler, while only handlers that keep server-side session state can honour it. The implementation documents that in the <remarks>. If the review prefers the flag to live somewhere cookie-specific, the implementation can follow.

Risks

  • Naming and placement. AuthenticationProperties is scheme-agnostic; this member is meaningful only where there is server-side session state to rotate. A handler that ignores it fails silently rather than throwing, which is how the other properties on the type behave but is still worth a decision.
  • ITicketStore contract. ITicketStore.StoreAsync promises to return the key the ticket was stored under, not a key that has never been used. The implementation therefore keeps the incoming key reserved, stores the new ticket, and removes the old key only once StoreAsync has returned a different one, so a conforming store that recycles released identifiers cannot hand the incoming key straight back. This was found by the automated review on #70105 and is covered by a test using a key-recycling store. If the API review would rather tighten the ITicketStore contract to require a fresh key, that is a larger change than this member and would belong in its own proposal.
  • No breaking change. With the property unset the handler takes the path it took before.
  • Not addressed here. The flag only removes the old entry from the store. Rotating a SessionStore-less cookie, or revoking other outstanding cookies for the same subject, is out of scope.

Links

  • Originating issue: #47503
  • Implementation pull request: #70105
  • Prior art and the suggestion to use an AuthenticationProperties flag: #67049
  • Behaviour this must not break: #22135

Champion

I am the author of #70105 and can attend the API review meeting to represent the proposal, but I am a community contributor rather than the area owner, so an area-auth owner would need to pick this up before it can be marked api-ready-for-review. I have not applied api-suggestion, api-proposal or the Feature issue type, because I do not have write access to this repository; whoever triages this, please add them.

Source justifications

The renew-in-place behaviour and its reason: CookieAuthenticationHandler.cs, quoted in #70105 — "// Renew the ticket in cases of multiple requests see: https://github.com/dotnet/aspnetcore/issues/22135"

The Identity sign-in path: #70105 — "which is what ASP.NET Core Identity does on password sign-in, since SignInManager.SignInWithClaimsAsync calls Context.SignInAsync without signing the application scheme out first."

Why opt-in rather than automatic: #70105 — "Any automatic rule based on comparing the incoming and outgoing principal would either miss the privilege change or break the refresh, so the caller has to say which one it means."

The suggestion to use an AuthenticationProperties flag: #67049, quoted in #70105 — "The larger question — whether SignInAsync should always rotate the session key (and how to preserve the refresh case from #22135) — is left for that issue, where it needs its own design discussion (likely an opt-in AuthenticationProperties flag)."

The alternatives considered: #70105 — "doing the rotation unconditionally on SignInAsync when a SessionStore is configured, which is the behaviour OWASP asks for but would be a breaking change for anyone relying on the current renew semantics from #22135" and "putting the switch on CookieAuthenticationOptions rather than on AuthenticationProperties, which is simpler but cannot be scoped to the sign-in that changes the principal"

The store-key-reuse risk: automated review on #70105 — "ITicketStore.StoreAsync only promises to return the associated key; it does not promise a fresh one."

The declared public API: src/Http/Authentication.Abstractions/src/PublicAPI.Unshipped.txt in #70105.

The process this issue follows: docs/APIReviewProcess.md — "API changes are reviewed in a separate API proposal issue from the issue that tracks the feature or implementation."

Dominant language
C#
Stars
38.5k
Forks
11.6k
Avg merge
3d 1h
Merged PRs (30d)
289

Getting set up

Open in Codespaces

Starts the project's dev container in your browser, under your own GitHub account.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from dotnet/aspnetcore

All issues in dotnet/aspnetcore

Similar issues

More C# issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.