Skip to content
This repository was archived by the owner on Nov 19, 2024. It is now read-only.

Commit 09c73e3

Browse files
josephdecockjosephdecock
authored
Merge commit from fork
Advisory fix
2 parents 29bef25 + 4aa9cc6 commit 09c73e3

8 files changed

+101
-65
lines changed

‎Directory.Build.targets

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<FrameworkVersion>8.0.1</FrameworkVersion>
44
<ExtensionsVersion>8.0.0</ExtensionsVersion>
55
<WilsonVersion>7.1.2</WilsonVersion>
6-
<IdentityServerVersion>7.0.6</IdentityServerVersion>
6+
<IdentityServerVersion>7.0.8</IdentityServerVersion>
77
</PropertyGroup>
88

99
<ItemGroup>

‎src/Duende.AccessTokenManagement.OpenIdConnect/AuthenticateResultCache.cs

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright (c) Brock Allen & Dominick Baier. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Authentication;
5+
using System.Collections.Generic;
6+
7+
/// <summary>
8+
/// Per-request cache so that if SignInAsync is used, we won't re-read the old/cached AuthenticateResult from the handler.
9+
/// This requires this service to be added as scoped to the DI system.
10+
/// Be VERY CAREFUL to not accidentally capture this service for longer than the appropriate DI scope - e.g., in an HttpClient.
11+
/// </summary>
12+
internal class AuthenticateResultCache: Dictionary<string, AuthenticateResult>
13+
{
14+
}

‎src/Duende.AccessTokenManagement.OpenIdConnect/AuthenticationSessionUserTokenStore.cs

+16-10
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@
44
using Microsoft.AspNetCore.Authentication;
55
using Microsoft.AspNetCore.Http;
66
using System;
7-
using System.Collections.Generic;
87
using System.Security.Claims;
98
using System.Threading.Tasks;
109
using Microsoft.Extensions.Logging;
10+
using Microsoft.Extensions.DependencyInjection;
11+
using IdentityModel;
1112

1213
namespace Duende.AccessTokenManagement.OpenIdConnect
1314
{
@@ -20,10 +21,6 @@ public class AuthenticationSessionUserAccessTokenStore : IUserTokenStore
2021
private readonly IStoreTokensInAuthenticationProperties _tokensInProps;
2122
private readonly ILogger<AuthenticationSessionUserAccessTokenStore> _logger;
2223

23-
// per-request cache so that if SignInAsync is used, we won't re-read the old/cached AuthenticateResult from the handler
24-
// this requires this service to be added as scoped to the DI system
25-
private readonly Dictionary<string, AuthenticateResult> _cache = new Dictionary<string, AuthenticateResult>();
26-
2724
/// <summary>
2825
/// ctor
2926
/// </summary>
@@ -46,10 +43,14 @@ public async Task<UserToken> GetTokenAsync(
4643
UserTokenRequestParameters? parameters = null)
4744
{
4845
parameters ??= new();
49-
46+
// Resolve the cache here because it needs to have a per-request
47+
// lifetime. Sometimes the store itself is captured for longer than
48+
// that inside an HttpClient.
49+
var cache = _contextAccessor.HttpContext?.RequestServices.GetRequiredService<AuthenticateResultCache>();
50+
5051
// check the cache in case the cookie was re-issued via StoreTokenAsync
5152
// we use String.Empty as the key for a null SignInScheme
52-
if (!_cache.TryGetValue(parameters.SignInScheme ?? String.Empty, out var result))
53+
if (!cache!.TryGetValue(parameters.SignInScheme ?? String.Empty, out var result))
5354
{
5455
result = await _contextAccessor!.HttpContext!.AuthenticateAsync(parameters.SignInScheme).ConfigureAwait(false);
5556
}
@@ -80,9 +81,14 @@ public async Task StoreTokenAsync(
8081
{
8182
parameters ??= new();
8283

84+
// Resolve the cache here because it needs to have a per-request
85+
// lifetime. Sometimes the store itself is captured for longer than
86+
// that inside an HttpClient.
87+
var cache = _contextAccessor.HttpContext?.RequestServices.GetRequiredService<AuthenticateResultCache>();
88+
8389
// check the cache in case the cookie was re-issued via StoreTokenAsync
8490
// we use String.Empty as the key for a null SignInScheme
85-
if (!_cache.TryGetValue(parameters.SignInScheme ?? String.Empty, out var result))
91+
if (!cache!.TryGetValue(parameters.SignInScheme ?? String.Empty, out var result))
8692
{
8793
result = await _contextAccessor.HttpContext!.AuthenticateAsync(parameters.SignInScheme)!.ConfigureAwait(false);
8894
}
@@ -103,7 +109,7 @@ public async Task StoreTokenAsync(
103109

104110
// add to the cache so if GetTokenAsync is called again, we will use the updated property values
105111
// we use String.Empty as the key for a null SignInScheme
106-
_cache[parameters.SignInScheme ?? String.Empty] = AuthenticateResult.Success(new AuthenticationTicket(transformedPrincipal, result.Properties, scheme!));
112+
cache[parameters.SignInScheme ?? String.Empty] = AuthenticateResult.Success(new AuthenticationTicket(transformedPrincipal, result.Properties, scheme!));
107113
}
108114

109115
/// <inheritdoc/>
@@ -125,4 +131,4 @@ protected virtual Task<ClaimsPrincipal> FilterPrincipalAsync(ClaimsPrincipal pri
125131
return Task.FromResult(principal);
126132
}
127133
}
128-
}
134+
}

‎src/Duende.AccessTokenManagement.OpenIdConnect/OpenIdConnectTokenManagementServiceCollectionExtensions.cs

+5-1
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
33

44
using System;
5+
using System.Collections.Generic;
56
using System.Net.Http;
67
using Duende.AccessTokenManagement;
78
using Duende.AccessTokenManagement.OpenIdConnect;
9+
using Microsoft.AspNetCore.Authentication;
810
using Microsoft.AspNetCore.Http;
911
using Microsoft.Extensions.DependencyInjection.Extensions;
1012
using Microsoft.Extensions.Logging;
@@ -46,8 +48,10 @@ public static IServiceCollection AddOpenIdConnectAccessTokenManagement(this ISer
4648
// context, and we register different ones in blazor
4749

4850
services.TryAddScoped<IUserAccessor, HttpContextUserAccessor>();
49-
// scoped since it will be caching per-request authentication results
5051
services.TryAddScoped<IUserTokenStore, AuthenticationSessionUserAccessTokenStore>();
52+
53+
// scoped since it will be caching per-request authentication results
54+
services.AddScoped<AuthenticateResultCache>();
5155

5256
return services;
5357
}

‎test/Tests/Framework/ApiHost.cs

+8-37
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using Duende.IdentityServer.Models;
55
using Microsoft.AspNetCore.Builder;
6+
using Microsoft.AspNetCore.Http;
67
using Microsoft.Extensions.DependencyInjection;
78

89
namespace Duende.AccessTokenManagement.Tests;
@@ -37,6 +38,7 @@ private void ConfigureServices(IServiceCollection services)
3738
options.Audience = _identityServerHost.Url("/resources");
3839
options.MapInboundClaims = false;
3940
options.BackchannelHttpHandler = _identityServerHost.Server.CreateHandler();
41+
options.TokenValidationParameters.NameClaimType = "sub";
4042
});
4143
}
4244

@@ -62,43 +64,12 @@ private void Configure(IApplicationBuilder app)
6264

6365
app.UseEndpoints(endpoints =>
6466
{
65-
// endpoints.Map("/{**catch-all}", async context =>
66-
// {
67-
// // capture body if present
68-
// var body = default(string);
69-
// if (context.Request.HasJsonContentType())
70-
// {
71-
// using (var sr = new StreamReader(context.Request.Body))
72-
// {
73-
// body = await sr.ReadToEndAsync();
74-
// }
75-
// }
76-
//
77-
// // capture request headers
78-
// var requestHeaders = new Dictionary<string, List<string>>();
79-
// foreach (var header in context.Request.Headers)
80-
// {
81-
// var values = new List<string>(header.Value.Select(v => v));
82-
// requestHeaders.Add(header.Key, values);
83-
// }
84-
//
85-
// var response = new ApiResponse(
86-
// context.Request.Method,
87-
// context.Request.Path.Value,
88-
// context.User.FindFirst(("sub"))?.Value,
89-
// context.User.FindFirst(("client_id"))?.Value,
90-
// context.User.Claims.Select(x => new ClaimRecord(x.Type, x.Value)).ToArray())
91-
// {
92-
// Body = body,
93-
// RequestHeaders = requestHeaders
94-
// };
95-
//
96-
// context.Response.StatusCode = ApiStatusCodeToReturn ?? 200;
97-
// ApiStatusCodeToReturn = null;
98-
//
99-
// context.Response.ContentType = "application/json";
100-
// await context.Response.WriteAsync(JsonSerializer.Serialize(response));
101-
// });
67+
endpoints.Map("/{**catch-all}", (HttpContext context) =>
68+
{
69+
return new TokenEchoResponse(
70+
context.User.Identity?.Name ?? "missing sub",
71+
context.Request.Headers.Authorization.First() ?? "missing token");
72+
});
10273
});
10374
}
10475
}

‎test/Tests/Framework/AppHost.cs

+19-5
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,15 @@
1010
using IdentityModel;
1111
using Duende.AccessTokenManagement.OpenIdConnect;
1212
using RichardSzalay.MockHttp;
13+
using System.Net.Http.Json;
1314

1415
namespace Duende.AccessTokenManagement.Tests;
1516

1617
public class AppHost : GenericHost
1718
{
1819
private readonly IdentityServerHost _identityServerHost;
1920
private readonly ApiHost _apiHost;
20-
private readonly string _clientId;
21+
public string ClientId;
2122
private readonly Action<UserTokenManagementOptions>? _configureUserTokenManagementOptions;
2223

2324
public AppHost(
@@ -30,7 +31,7 @@ public AppHost(
3031
{
3132
_identityServerHost = identityServerHost;
3233
_apiHost = apiHost;
33-
_clientId = clientId;
34+
ClientId = clientId;
3435
_configureUserTokenManagementOptions = configureUserTokenManagementOptions;
3536
OnConfigureServices += ConfigureServices;
3637
OnConfigure += Configure;
@@ -58,7 +59,7 @@ private void ConfigureServices(IServiceCollection services)
5859
{
5960
options.Authority = _identityServerHost.Url();
6061

61-
options.ClientId = _clientId;
62+
options.ClientId = ClientId;
6263
options.ClientSecret = "secret";
6364
options.ResponseType = "code";
6465
options.ResponseMode = "query";
@@ -68,7 +69,7 @@ private void ConfigureServices(IServiceCollection services)
6869
options.SaveTokens = true;
6970

7071
options.Scope.Clear();
71-
var client = _identityServerHost.Clients.Single(x => x.ClientId == _clientId);
72+
var client = _identityServerHost.Clients.Single(x => x.ClientId == ClientId);
7273
foreach (var scope in client.AllowedScopes)
7374
{
7475
options.Scope.Add(scope);
@@ -107,6 +108,10 @@ private void ConfigureServices(IServiceCollection services)
107108
}
108109
});
109110

111+
services.AddUserAccessTokenHttpClient("callApi", configureClient: client => {
112+
client.BaseAddress = new Uri(_apiHost.Url());
113+
})
114+
.ConfigurePrimaryHttpMessageHandler(() => _apiHost.HttpMessageHandler);
110115
}
111116

112117
private void Configure(IApplicationBuilder app)
@@ -136,6 +141,13 @@ await context.ChallengeAsync(new AuthenticationProperties
136141
await context.Response.WriteAsJsonAsync(token);
137142
});
138143

144+
endpoints.MapGet("/call_api", async (IHttpClientFactory factory, HttpContext context) =>
145+
{
146+
var http = factory.CreateClient("callApi");
147+
var response = await http.GetAsync("test");
148+
return await response.Content.ReadFromJsonAsync<TokenEchoResponse>();
149+
});
150+
139151
endpoints.MapGet("/user_token_with_resource/{resource}", async (string resource, HttpContext context) =>
140152
{
141153
var token = await context.GetUserAccessTokenAsync(new UserTokenRequestParameters
@@ -204,4 +216,6 @@ public async Task<HttpResponseMessage> LogoutAsync(string? sid = null)
204216
response = await BrowserClient.GetAsync(Url(response.Headers.Location.ToString()));
205217
return response;
206218
}
207-
}
219+
}
220+
221+
public record TokenEchoResponse(string sub, string token);

‎test/Tests/Framework/GenericHost.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,9 @@ public GenericHost(string baseAddress = "https://server")
3232
public TestServer Server { get; private set; } = default!;
3333
public TestBrowserClient BrowserClient { get; set; } = default!;
3434
public HttpClient HttpClient { get; set; } = default!;
35-
35+
public HttpMessageHandler HttpMessageHandler { get; set; } = default!;
3636
public TestLoggerProvider Logger { get; set; } = new TestLoggerProvider();
3737

38-
3938
public T Resolve<T>()
4039
where T : notnull
4140
{
@@ -84,6 +83,7 @@ public async Task InitializeAsync()
8483
Server = host.GetTestServer();
8584
BrowserClient = new TestBrowserClient(Server.CreateHandler());
8685
HttpClient = Server.CreateClient();
86+
HttpMessageHandler = Server.CreateHandler();
8787
}
8888

8989
public event Action<IServiceCollection> OnConfigureServices = services => { };

‎test/Tests/UserTokenManagementTests.cs

+36-9
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,18 @@ public async Task Missing_initial_refresh_token_and_expired_access_token_should_
183183
[Fact]
184184
public async Task Short_token_lifetime_should_trigger_refresh()
185185
{
186+
// This test makes an initial token request using code flow and then
187+
// refreshes the token a couple of times.
188+
189+
// We mock the expiration of the first few token responses to be short
190+
// enough that we will automatically refresh immediately when attempting
191+
// to use the tokens, while the final response gets a long refresh time,
192+
// allowing us to verify that the token is not refreshed.
193+
186194
var mockHttp = new MockHttpMessageHandler();
187195
AppHost.IdentityServerHttpHandler = mockHttp;
188196

189-
// short token lifetime should trigger refresh on 1st use
197+
// Respond to code flow with a short token lifetime so that we trigger refresh on 1st use
190198
var initialTokenResponse = new
191199
{
192200
id_token = IdentityServerHost.CreateIdToken("1", "web"),
@@ -195,37 +203,31 @@ public async Task Short_token_lifetime_should_trigger_refresh()
195203
expires_in = 10,
196204
refresh_token = "initial_refresh_token",
197205
};
198-
199-
// response for re-deeming code
200206
mockHttp.When("/connect/token")
201207
.WithFormData("grant_type", "authorization_code")
202208
.Respond("application/json", JsonSerializer.Serialize(initialTokenResponse));
203209

204-
// short token lifetime should trigger refresh on 1st use
210+
// Respond to refresh with a short token lifetime so that we trigger another refresh on 2nd use
205211
var refreshTokenResponse = new
206212
{
207213
access_token = "refreshed1_access_token",
208214
token_type = "token_type1",
209215
expires_in = 10,
210216
refresh_token = "refreshed1_refresh_token",
211217
};
212-
213-
// response for refresh 1
214218
mockHttp.When("/connect/token")
215219
.WithFormData("grant_type", "refresh_token")
216220
.WithFormData("refresh_token", "initial_refresh_token")
217221
.Respond("application/json", JsonSerializer.Serialize(refreshTokenResponse));
218222

219-
// short token lifetime should trigger refresh on 2nd use
223+
// Respond to second refresh with a long token lifetime so that we don't trigger another refresh on 3rd use
220224
var refreshTokenResponse2 = new
221225
{
222226
access_token = "refreshed2_access_token",
223227
token_type = "token_type2",
224228
expires_in = 3600,
225229
refresh_token = "refreshed2_refresh_token",
226230
};
227-
228-
// response for refresh 1
229231
mockHttp.When("/connect/token")
230232
.WithFormData("grant_type", "refresh_token")
231233
.WithFormData("refresh_token", "refreshed1_refresh_token")
@@ -397,4 +399,29 @@ public async Task Refresh_responses_without_refresh_token_use_old_refresh_token(
397399
token.IsError.ShouldBeFalse();
398400
token.RefreshToken.ShouldBe("initial_refresh_token");
399401
}
402+
403+
[Fact]
404+
public async Task Multiple_users_have_distinct_tokens_across_refreshes()
405+
{
406+
// setup host
407+
AppHost.ClientId = "web.short";
408+
await AppHost.InitializeAsync();
409+
await AppHost.LoginAsync("alice");
410+
411+
var firstResponse = await AppHost.BrowserClient.GetAsync(AppHost.Url("/call_api"));
412+
var firstToken = await firstResponse.Content.ReadFromJsonAsync<TokenEchoResponse>();
413+
var secondResponse = await AppHost.BrowserClient.GetAsync(AppHost.Url("/call_api"));
414+
var secondToken = await secondResponse.Content.ReadFromJsonAsync<TokenEchoResponse>();
415+
firstToken.ShouldNotBeNull();
416+
secondToken.ShouldNotBeNull();
417+
secondToken.sub.ShouldBe(firstToken.sub);
418+
secondToken.token.ShouldNotBe(firstToken.token);
419+
420+
await AppHost.LoginAsync("bob");
421+
var thirdResponse = await AppHost.BrowserClient.GetAsync(AppHost.Url("/call_api"));
422+
var thirdToken = await thirdResponse.Content.ReadFromJsonAsync<TokenEchoResponse>();
423+
thirdToken.ShouldNotBeNull();
424+
thirdToken.sub.ShouldNotBe(secondToken.sub);
425+
thirdToken.token.ShouldNotBe(firstToken.token);
426+
}
400427
}

0 commit comments

Comments
 (0)