Skip to content

Commit ba7d550

Browse files
kjaciOvergaardandr317c
authored
Move access/refresh tokens to secure cookies (V17) (#20820)
* Move access/refresh tokens to secure cookies (#20779) * feat: adds the `credentials: include` header to all manual requests * feat: adds `credentials: include` as a configurable option to xhr requests (and sets it by default to true) * feat: configures the auto-generated fetch client from hey-api to include credentials by default * Add OpenIddict handler to hide tokens from the back-office client * Make back-office token redaction optional (default false) * Clear back-office token cookies on logout * Add configuration for backoffice cookie settings * Make cookies forcefully secure + move cookie handler enabling to the BackOfficeTokenCookieSettings * Use the "__Host-" prefix for cookie names * docs: adds documentation on cookie settings * build: sets up launch profile for vscode with new cookie recommended settings * docs: adds extra note around SameSite settings * docs: adds extra note around SameSite settings * Respect sites that do not use HTTPS * Explicitly invalidate potentially valid, old refresh tokens that should no longer be used * Removed obsolete const --------- Co-authored-by: Jacob Overgaard <[email protected]> * Remove configuration option * Invalidate all existing access tokens on upgrade * docs: updates recommended settings for development * build: removes non-existing variable * Skip flaky test * Bumped version of our test helpers to fix failing tests --------- Co-authored-by: Jacob Overgaard <[email protected]> Co-authored-by: Andreas Zerbst <[email protected]>
1 parent 0600df4 commit ba7d550

File tree

20 files changed

+307
-14
lines changed

20 files changed

+307
-14
lines changed

.github/BUILD.md

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ In order to work with the Umbraco source code locally, first make sure you have
3737

3838
### Familiarizing yourself with the code
3939

40-
Umbraco is a .NET application using C#. The solution is broken down into multiple projects. There are several class libraries. The `Umbraco.Web.UI` project is the main project that hosts the back office and login screen. This is the project you will want to run to see your changes.
40+
Umbraco is a .NET application using C#. The solution is broken down into multiple projects. There are several class libraries. The `Umbraco.Web.UI` project is the main project that hosts the back office and login screen. This is the project you will want to run to see your changes.
4141

4242
There are two web projects in the solution with client-side assets based on TypeScript, `Umbraco.Web.UI.Client` and `Umbraco.Web.UI.Login`.
4343

@@ -73,13 +73,19 @@ Just be careful not to include this change in your PR.
7373

7474
Conversely, if you are working on front-end only, you want to build the back-end once and then run it. Before you do so, update the configuration in `appSettings.json` to add the following under `Umbraco:Cms:Security`:
7575

76-
```
76+
```json
7777
"BackOfficeHost": "http://localhost:5173",
7878
"AuthorizeCallbackPathName": "/oauth_complete",
7979
"AuthorizeCallbackLogoutPathName": "/logout",
80-
"AuthorizeCallbackErrorPathName": "/error"
80+
"AuthorizeCallbackErrorPathName": "/error",
81+
"BackOfficeTokenCookie": {
82+
"SameSite": "None"
83+
}
8184
```
8285

86+
> [!NOTE]
87+
> If you get stuck in a login loop, try clearing your browser cookies for localhost, and make sure that the `Umbraco:Cms:Security:BackOfficeTokenCookie:SameSite` setting is set to `None`.
88+
8389
Then run Umbraco from the command line.
8490

8591
```

.github/copilot-instructions.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ Bootstrap, build, and test the repository:
3131
- `cd src/Umbraco.Web.UI`
3232
- `dotnet run --no-build` -- Application runs on https://localhost:44339 and http://localhost:11000
3333

34+
Check out [BUILD.md](./BUILD.md) for more detailed instructions.
35+
3436
## Validation
3537

3638
- ALWAYS run through at least one complete end-to-end scenario after making changes.
@@ -103,7 +105,10 @@ For frontend-only changes:
103105
"BackOfficeHost": "http://localhost:5173",
104106
"AuthorizeCallbackPathName": "/oauth_complete",
105107
"AuthorizeCallbackLogoutPathName": "/logout",
106-
"AuthorizeCallbackErrorPathName": "/error"
108+
"AuthorizeCallbackErrorPathName": "/error",
109+
"BackOfficeTokenCookie": {
110+
"SameSite": "None"
111+
}
107112
```
108113
2. Run backend: `cd src/Umbraco.Web.UI && dotnet run --no-build`
109114
3. Run frontend dev server: `cd src/Umbraco.Web.UI.Client && npm run dev:server`

.vscode/launch.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,9 @@
104104
"UMBRACO__CMS__SECURITY__BACKOFFICEHOST": "http://localhost:5173",
105105
"UMBRACO__CMS__SECURITY__AUTHORIZECALLBACKPATHNAME": "/oauth_complete",
106106
"UMBRACO__CMS__SECURITY__AUTHORIZECALLBACKLOGOUTPATHNAME": "/logout",
107-
"UMBRACO__CMS__SECURITY__AUTHORIZECALLBACKERRORPATHNAME": "/error"
107+
"UMBRACO__CMS__SECURITY__AUTHORIZECALLBACKERRORPATHNAME": "/error",
108+
"UMBRACO__CMS__SECURITY__KEEPUSERLOGGEDIN": "true",
109+
"UMBRACO__CMS__SECURITY__BACKOFFICETOKENCOOKIE__SAMESITE": "None"
108110
},
109111
"sourceFileMap": {
110112
"/Views": "${workspaceFolder}/Umbraco.Web.UI/Views"
Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
using System.Diagnostics.CodeAnalysis;
2+
using Microsoft.AspNetCore.DataProtection;
3+
using Microsoft.AspNetCore.Http;
4+
using Microsoft.Extensions.Options;
5+
using OpenIddict.Server;
6+
using OpenIddict.Validation;
7+
using Umbraco.Cms.Core;
8+
using Umbraco.Cms.Core.Configuration.Models;
9+
using Umbraco.Cms.Core.Events;
10+
using Umbraco.Cms.Core.Notifications;
11+
using Umbraco.Cms.Web.Common.Security;
12+
using Umbraco.Extensions;
13+
14+
namespace Umbraco.Cms.Api.Common.DependencyInjection;
15+
16+
internal sealed class HideBackOfficeTokensHandler
17+
: IOpenIddictServerHandler<OpenIddictServerEvents.ApplyTokenResponseContext>,
18+
IOpenIddictServerHandler<OpenIddictServerEvents.ExtractTokenRequestContext>,
19+
IOpenIddictValidationHandler<OpenIddictValidationEvents.ProcessAuthenticationContext>,
20+
INotificationHandler<UserLogoutSuccessNotification>
21+
{
22+
private const string RedactedTokenValue = "[redacted]";
23+
private const string AccessTokenCookieKey = "__Host-umbAccessToken";
24+
private const string RefreshTokenCookieKey = "__Host-umbRefreshToken";
25+
26+
private readonly IHttpContextAccessor _httpContextAccessor;
27+
private readonly IDataProtectionProvider _dataProtectionProvider;
28+
private readonly BackOfficeTokenCookieSettings _backOfficeTokenCookieSettings;
29+
private readonly GlobalSettings _globalSettings;
30+
31+
public HideBackOfficeTokensHandler(
32+
IHttpContextAccessor httpContextAccessor,
33+
IDataProtectionProvider dataProtectionProvider,
34+
IOptions<BackOfficeTokenCookieSettings> backOfficeTokenCookieSettings,
35+
IOptions<GlobalSettings> globalSettings)
36+
{
37+
_httpContextAccessor = httpContextAccessor;
38+
_dataProtectionProvider = dataProtectionProvider;
39+
_backOfficeTokenCookieSettings = backOfficeTokenCookieSettings.Value;
40+
_globalSettings = globalSettings.Value;
41+
}
42+
43+
/// <summary>
44+
/// This is invoked when tokens (access and refresh tokens) are issued to a client. For the back-office client,
45+
/// we will intercept the response, write the tokens from the response into HTTP-only cookies, and redact the
46+
/// tokens from the response, so they are not exposed to the client.
47+
/// </summary>
48+
public ValueTask HandleAsync(OpenIddictServerEvents.ApplyTokenResponseContext context)
49+
{
50+
if (context.Request?.ClientId is not Constants.OAuthClientIds.BackOffice)
51+
{
52+
// Only ever handle the back-office client.
53+
return ValueTask.CompletedTask;
54+
}
55+
56+
HttpContext httpContext = GetHttpContext();
57+
58+
if (context.Response.AccessToken is not null)
59+
{
60+
SetCookie(httpContext, AccessTokenCookieKey, context.Response.AccessToken);
61+
context.Response.AccessToken = RedactedTokenValue;
62+
}
63+
64+
if (context.Response.RefreshToken is not null)
65+
{
66+
SetCookie(httpContext, RefreshTokenCookieKey, context.Response.RefreshToken);
67+
context.Response.RefreshToken = RedactedTokenValue;
68+
}
69+
70+
return ValueTask.CompletedTask;
71+
}
72+
73+
/// <summary>
74+
/// This is invoked when requesting new tokens.
75+
/// </summary>
76+
public ValueTask HandleAsync(OpenIddictServerEvents.ExtractTokenRequestContext context)
77+
{
78+
if (context.Request?.ClientId != Constants.OAuthClientIds.BackOffice)
79+
{
80+
// Only ever handle the back-office client.
81+
return ValueTask.CompletedTask;
82+
}
83+
84+
// For the back-office client, this only happens when a refresh token is being exchanged for a new access token.
85+
if (context.Request.RefreshToken == RedactedTokenValue
86+
&& TryGetCookie(RefreshTokenCookieKey, out var refreshToken))
87+
{
88+
context.Request.RefreshToken = refreshToken;
89+
}
90+
else
91+
{
92+
// If we got here, either the refresh token was not redacted, or nothing was found in the refresh token cookie.
93+
// If OpenIddict found a refresh token, it could be an old token that is potentially still valid. For security
94+
// reasons, we cannot accept that; at this point, we expect the refresh tokens to be explicitly redacted.
95+
context.Request.RefreshToken = null;
96+
}
97+
98+
99+
return ValueTask.CompletedTask;
100+
}
101+
102+
/// <summary>
103+
/// This is invoked when extracting the auth context for a client request.
104+
/// </summary>
105+
public ValueTask HandleAsync(OpenIddictValidationEvents.ProcessAuthenticationContext context)
106+
{
107+
// For the back-office client, this only happens when an access token is sent to the API.
108+
if (context.AccessToken != RedactedTokenValue)
109+
{
110+
return ValueTask.CompletedTask;
111+
}
112+
113+
if (TryGetCookie(AccessTokenCookieKey, out var accessToken))
114+
{
115+
context.AccessToken = accessToken;
116+
}
117+
118+
return ValueTask.CompletedTask;
119+
}
120+
121+
public void Handle(UserLogoutSuccessNotification notification)
122+
{
123+
HttpContext? context = _httpContextAccessor.HttpContext;
124+
if (context is null)
125+
{
126+
// For some reason there is no ambient HTTP context, so we can't clean up the cookies.
127+
// This is OK, because the tokens in the cookies have already been revoked at user sign-out,
128+
// so the cookie clean-up is mostly cosmetic.
129+
return;
130+
}
131+
132+
context.Response.Cookies.Delete(AccessTokenCookieKey);
133+
context.Response.Cookies.Delete(RefreshTokenCookieKey);
134+
}
135+
136+
private HttpContext GetHttpContext()
137+
=> _httpContextAccessor.GetRequiredHttpContext();
138+
139+
private void SetCookie(HttpContext httpContext, string key, string value)
140+
{
141+
var cookieValue = EncryptionHelper.Encrypt(value, _dataProtectionProvider);
142+
143+
var cookieOptions = new CookieOptions
144+
{
145+
// Prevent the client-side scripts from accessing the cookie.
146+
HttpOnly = true,
147+
148+
// Mark the cookie as essential to the application, to enforce it despite any
149+
// data collection consent options. This aligns with how ASP.NET Core Identity
150+
// does when writing cookies for cookie authentication.
151+
IsEssential = true,
152+
153+
// Cookie path must be root for optimal security.
154+
Path = "/",
155+
156+
// For optimal security, the cooke must be secure. However, Umbraco allows for running development
157+
// environments over HTTP, so we need to take that into account here.
158+
// Thus, we will make the cookie secure if:
159+
// - HTTPS is explicitly enabled by config (default for production environments), or
160+
// - The current request is over HTTPS (meaning the environment supports it regardless of config).
161+
Secure = _globalSettings.UseHttps || httpContext.Request.IsHttps,
162+
163+
// SameSite is configurable (see BackOfficeTokenCookieSettings for defaults):
164+
SameSite = ParseSameSiteMode(_backOfficeTokenCookieSettings.SameSite),
165+
};
166+
167+
httpContext.Response.Cookies.Delete(key, cookieOptions);
168+
httpContext.Response.Cookies.Append(key, cookieValue, cookieOptions);
169+
}
170+
171+
private bool TryGetCookie(string key, [NotNullWhen(true)] out string? value)
172+
{
173+
if (GetHttpContext().Request.Cookies.TryGetValue(key, out var cookieValue))
174+
{
175+
value = EncryptionHelper.Decrypt(cookieValue, _dataProtectionProvider);
176+
return true;
177+
}
178+
179+
value = null;
180+
return false;
181+
}
182+
183+
private static SameSiteMode ParseSameSiteMode(string sameSiteMode) =>
184+
Enum.TryParse(sameSiteMode, ignoreCase: true, out SameSiteMode result)
185+
? result
186+
: throw new ArgumentException($"The provided {nameof(sameSiteMode)} value could not be parsed into as SameSiteMode value.", nameof(sameSiteMode));
187+
}

src/Umbraco.Cms.Api.Common/DependencyInjection/UmbracoBuilderAuthExtensions.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Umbraco.Cms.Core.DependencyInjection;
1212
using Umbraco.Cms.Infrastructure.BackgroundJobs;
1313
using Umbraco.Cms.Infrastructure.BackgroundJobs.Jobs.DistributedJobs;
14+
using Umbraco.Cms.Core.Notifications;
1415

1516
namespace Umbraco.Cms.Api.Common.DependencyInjection;
1617

@@ -113,6 +114,19 @@ private static void ConfigureOpenIddict(IUmbracoBuilder builder)
113114
{
114115
configuration.UseSingletonHandler<ProcessRequestContextHandler>().SetOrder(OpenIddict.Server.AspNetCore.OpenIddictServerAspNetCoreHandlers.ResolveRequestUri.Descriptor.Order - 1);
115116
});
117+
118+
options.AddEventHandler<OpenIddictServerEvents.ApplyTokenResponseContext>(configuration =>
119+
{
120+
configuration
121+
.UseSingletonHandler<HideBackOfficeTokensHandler>()
122+
.SetOrder(OpenIddict.Server.AspNetCore.OpenIddictServerAspNetCoreHandlers.ProcessJsonResponse<OpenIddictServerEvents.ApplyTokenResponseContext>.Descriptor.Order - 1);
123+
});
124+
options.AddEventHandler<OpenIddictServerEvents.ExtractTokenRequestContext>(configuration =>
125+
{
126+
configuration
127+
.UseSingletonHandler<HideBackOfficeTokensHandler>()
128+
.SetOrder(OpenIddict.Server.AspNetCore.OpenIddictServerAspNetCoreHandlers.ExtractPostRequest<OpenIddictServerEvents.ExtractTokenRequestContext>.Descriptor.Order + 1);
129+
});
116130
})
117131

118132
// Register the OpenIddict validation components.
@@ -137,9 +151,19 @@ private static void ConfigureOpenIddict(IUmbracoBuilder builder)
137151
{
138152
configuration.UseSingletonHandler<ProcessRequestContextHandler>().SetOrder(OpenIddict.Validation.AspNetCore.OpenIddictValidationAspNetCoreHandlers.ResolveRequestUri.Descriptor.Order - 1);
139153
});
154+
155+
options.AddEventHandler<OpenIddictValidationEvents.ProcessAuthenticationContext>(configuration =>
156+
{
157+
configuration
158+
.UseSingletonHandler<HideBackOfficeTokensHandler>()
159+
// IMPORTANT: the handler must be AFTER the built-in query string handler, because the client-side SignalR library sometimes appends access tokens to the query string.
160+
.SetOrder(OpenIddict.Validation.AspNetCore.OpenIddictValidationAspNetCoreHandlers.ExtractAccessTokenFromQueryString.Descriptor.Order + 1);
161+
});
140162
});
141163

142164
builder.Services.AddSingleton<IDistributedBackgroundJob, OpenIddictCleanupJob>();
143165
builder.Services.ConfigureOptions<ConfigureOpenIddict>();
166+
167+
builder.AddNotificationHandler<UserLogoutSuccessNotification, HideBackOfficeTokensHandler>();
144168
}
145169
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
using System.ComponentModel;
2+
3+
namespace Umbraco.Cms.Core.Configuration.Models;
4+
5+
/// <summary>
6+
/// Typed configuration options for back-office token cookie settings.
7+
/// </summary>
8+
[UmbracoOptions(Constants.Configuration.ConfigBackOfficeTokenCookie)]
9+
[Obsolete("This will be replaced with a different authentication scheme. Scheduled for removal in Umbraco 18.")]
10+
public class BackOfficeTokenCookieSettings
11+
{
12+
private const string StaticSameSite = "Strict";
13+
14+
/// <summary>
15+
/// Gets or sets a value indicating whether the cookie SameSite configuration.
16+
/// </summary>
17+
/// <remarks>
18+
/// Valid values are "Unspecified", "None", "Lax" and "Strict" (default).
19+
/// </remarks>
20+
[DefaultValue(StaticSameSite)]
21+
public string SameSite { get; set; } = StaticSameSite;
22+
}

src/Umbraco.Core/Constants-Configuration.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ public static class Configuration
6767
public const string ConfigWebhookPayloadType = ConfigWebhook + ":PayloadType";
6868
public const string ConfigCache = ConfigPrefix + "Cache";
6969
public const string ConfigDistributedJobs = ConfigPrefix + "DistributedJobs";
70+
public const string ConfigBackOfficeTokenCookie = ConfigSecurity + ":BackOfficeTokenCookie";
7071

7172
public static class NamedOptions
7273
{

src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ public static IUmbracoBuilder AddConfiguration(this IUmbracoBuilder builder)
8888
.AddUmbracoOptions<WebhookSettings>()
8989
.AddUmbracoOptions<CacheSettings>()
9090
.AddUmbracoOptions<SystemDateMigrationSettings>()
91-
.AddUmbracoOptions<DistributedJobSettings>();
91+
.AddUmbracoOptions<DistributedJobSettings>()
92+
.AddUmbracoOptions<BackOfficeTokenCookieSettings>();
9293

9394
// Configure connection string and ensure it's updated when the configuration changes
9495
builder.Services.AddSingleton<IConfigureOptions<ConnectionStrings>, ConfigureConnectionStrings>();

src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,5 +139,6 @@ protected virtual void DefinePlan()
139139
To<V_17_0_0.AddDistributedJobLock>("{263075BF-F18A-480D-92B4-4947D2EAB772}");
140140
To<V_17_0_0.AddLastSyncedTable>("26179D88-58CE-4C92-B4A4-3CBA6E7188AC");
141141
To<V_17_0_0.EnsureDefaultMediaFolderHasDefaultCollection>("{8B2C830A-4FFB-4433-8337-8649B0BF52C8}");
142+
To<V_17_0_0.InvalidateBackofficeUserAccess>("{1C38D589-26BB-4A46-9ABE-E4A0DF548A87}");
142143
}
143144
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_17_0_0;
2+
3+
public class InvalidateBackofficeUserAccess : AsyncMigrationBase
4+
{
5+
public InvalidateBackofficeUserAccess(IMigrationContext context)
6+
: base(context)
7+
{
8+
}
9+
10+
protected override Task MigrateAsync()
11+
{
12+
InvalidateBackofficeUserAccess = true;
13+
return Task.CompletedTask;
14+
}
15+
}

0 commit comments

Comments
 (0)