Skip to content

Commit 66bbf50

Browse files
RotemKirRotem
RotemKir
and
Rotem
authored
Fix usage of cache key prefix - don't encrypt the prefix as part of the cache key, only the token (DuendeArchive#112)
Co-authored-by: Rotem <rotemk@tipalti.local>
1 parent fcfd68c commit 66bbf50

3 files changed

Lines changed: 66 additions & 22 deletions

File tree

‎src/Infrastructure/CacheExtensions.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ static CacheExtensions()
2323
Settings.Converters.Add(new ClaimConverter());
2424
}
2525

26-
public static async Task<IEnumerable<Claim>> GetClaimsAsync(this IDistributedCache cache, string token)
26+
public static async Task<IEnumerable<Claim>> GetClaimsAsync(this IDistributedCache cache, string cacheKeyPrefix, string token)
2727
{
28-
var bytes = await cache.GetAsync(token.Sha256()).ConfigureAwait(false);
28+
var bytes = await cache.GetAsync($"{cacheKeyPrefix}{token.Sha256()}").ConfigureAwait(false);
2929

3030
if (bytes == null)
3131
{
@@ -36,7 +36,7 @@ public static async Task<IEnumerable<Claim>> GetClaimsAsync(this IDistributedCac
3636
return JsonConvert.DeserializeObject<IEnumerable<Claim>>(json, Settings);
3737
}
3838

39-
public static async Task SetClaimsAsync(this IDistributedCache cache, string token, IEnumerable<Claim> claims, TimeSpan duration, ILogger logger)
39+
public static async Task SetClaimsAsync(this IDistributedCache cache, string cacheKeyPrefix, string token, IEnumerable<Claim> claims, TimeSpan duration, ILogger logger)
4040
{
4141
var expClaim = claims.FirstOrDefault(c => c.Type == JwtClaimTypes.Expiration);
4242
if (expClaim == null)
@@ -48,7 +48,7 @@ public static async Task SetClaimsAsync(this IDistributedCache cache, string tok
4848
var now = DateTimeOffset.UtcNow;
4949
var expiration = DateTimeOffset.FromUnixTimeSeconds(long.Parse(expClaim.Value));
5050
logger.LogDebug("Token will expire in {expiration}", expiration);
51-
51+
5252

5353
if (expiration <= now)
5454
{
@@ -70,7 +70,7 @@ public static async Task SetClaimsAsync(this IDistributedCache cache, string tok
7070
var bytes = Encoding.UTF8.GetBytes(json);
7171

7272
logger.LogDebug("Setting cache item expiration to {expiration}", absoluteLifetime);
73-
await cache.SetAsync(token.Sha256(), bytes, new DistributedCacheEntryOptions { AbsoluteExpiration = absoluteLifetime }).ConfigureAwait(false);
73+
await cache.SetAsync($"{cacheKeyPrefix}{token.Sha256()}", bytes, new DistributedCacheEntryOptions { AbsoluteExpiration = absoluteLifetime }).ConfigureAwait(false);
7474
}
7575
}
7676
}

‎src/OAuth2IntrospectionHandler.cs‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class OAuth2IntrospectionHandler : AuthenticationHandler<OAuth2Introspect
2424
{
2525
private readonly IDistributedCache _cache;
2626
private readonly ILogger<OAuth2IntrospectionHandler> _logger;
27-
27+
2828
static readonly ConcurrentDictionary<string, Lazy<Task<AuthenticateResult>>> IntrospectionDictionary =
2929
new ConcurrentDictionary<string, Lazy<Task<AuthenticateResult>>>();
3030

@@ -87,8 +87,7 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
8787
// if caching is enable - let's check if we have a cached introspection
8888
if (Options.EnableCaching)
8989
{
90-
var key = $"{Options.CacheKeyPrefix}{token}";
91-
var claims = await _cache.GetClaimsAsync(key).ConfigureAwait(false);
90+
var claims = await _cache.GetClaimsAsync(Options.CacheKeyPrefix, token).ConfigureAwait(false);
9291
if (claims != null)
9392
{
9493
// find out if it is a cached inactive token
@@ -103,7 +102,7 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
103102

104103
_logger.LogTrace("Token is not cached.");
105104
}
106-
105+
107106
// no cached result - let's make a network roundtrip to the introspection endpoint
108107
// this code block tries to make sure that we only do a single roundtrip, even when multiple requests
109108
// with the same token come in at the same time
@@ -114,19 +113,18 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
114113
return new Lazy<Task<AuthenticateResult>>(async () =>
115114
{
116115
var response = await LoadClaimsForToken(token);
117-
116+
118117
if (response.IsError)
119118
{
120119
_logger.LogError("Error returned from introspection endpoint: " + response.Error);
121120
return await ReportNonSuccessAndReturn("Error returned from introspection endpoint: " + response.Error);
122121
}
123-
122+
124123
if (response.IsActive)
125124
{
126125
if (Options.EnableCaching)
127126
{
128-
var key = $"{Options.CacheKeyPrefix}{token}";
129-
await _cache.SetClaimsAsync(key, response.Claims, Options.CacheDuration, _logger).ConfigureAwait(false);
127+
await _cache.SetClaimsAsync(Options.CacheKeyPrefix, token, response.Claims, Options.CacheDuration, _logger).ConfigureAwait(false);
130128
}
131129

132130
return await CreateTicket(response.Claims, token);
@@ -135,13 +133,11 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
135133
{
136134
if (Options.EnableCaching)
137135
{
138-
var key = $"{Options.CacheKeyPrefix}{token}";
139-
140136
// add an exp claim - otherwise caching will not work
141137
var claimsWithExp = response.Claims.ToList();
142138
claimsWithExp.Add(new Claim("exp",
143139
DateTimeOffset.UtcNow.Add(Options.CacheDuration).ToUnixTimeSeconds().ToString()));
144-
await _cache.SetClaimsAsync(key, claimsWithExp, Options.CacheDuration, _logger)
140+
await _cache.SetClaimsAsync(Options.CacheKeyPrefix, token, claimsWithExp, Options.CacheDuration, _logger)
145141
.ConfigureAwait(false);
146142
}
147143

‎test/Tests/Introspection.cs‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,17 @@
22
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
33

44
using FluentAssertions;
5+
using IdentityModel;
56
using IdentityModel.AspNetCore.OAuth2Introspection;
67
using IdentityModel.Client;
8+
using Microsoft.AspNetCore.TestHost;
9+
using Microsoft.Extensions.Caching.Distributed;
10+
using Microsoft.Extensions.DependencyInjection;
711
using Newtonsoft.Json;
812
using System;
913
using System.Collections.Generic;
1014
using System.Net;
15+
using System.Security.Claims;
1116
using System.Threading.Tasks;
1217
using Tests.Util;
1318
using Xunit;
@@ -207,15 +212,47 @@ public async Task ActiveToken_With_SavedToken_And_Caching()
207212
var expectedToken = "expected_token";
208213
var handler = new IntrospectionEndpointHandler(IntrospectionEndpointHandler.Behavior.Active, TimeSpan.FromHours(1));
209214

210-
var client = PipelineFactory.CreateClient((o) =>
215+
var server = PipelineFactory.CreateServer((o) =>
211216
{
212217
_options(o);
213218

214219
o.SaveToken = true;
215220
o.EnableCaching = true;
216221
o.CacheDuration = TimeSpan.FromMinutes(10);
217222
}, handler, true);
223+
var client = server.CreateClient();
224+
client.SetBearerToken(expectedToken);
225+
226+
var firstResponse = await client.GetAsync("http://test");
227+
firstResponse.StatusCode.Should().Be(HttpStatusCode.OK);
228+
229+
var secondResponse = await client.GetAsync("http://test");
230+
secondResponse.StatusCode.Should().Be(HttpStatusCode.OK);
231+
232+
var responseDataStr = await secondResponse.Content.ReadAsStringAsync();
233+
var responseData = JsonConvert.DeserializeObject<Dictionary<string, string>>(responseDataStr);
234+
235+
responseData.Should().Contain("token", expectedToken);
236+
AssertCacheItemExists(server, string.Empty, expectedToken);
237+
}
218238

239+
[Fact]
240+
public async Task ActiveToken_With_SavedToken_And_Caching_With_Cache_Key_Prefix()
241+
{
242+
var expectedToken = "expected_token";
243+
var cacheKeyPrefix = "KeyPrefix";
244+
var handler = new IntrospectionEndpointHandler(IntrospectionEndpointHandler.Behavior.Active, TimeSpan.FromHours(1));
245+
246+
var server = PipelineFactory.CreateServer((o) =>
247+
{
248+
_options(o);
249+
250+
o.SaveToken = true;
251+
o.EnableCaching = true;
252+
o.CacheKeyPrefix = cacheKeyPrefix;
253+
o.CacheDuration = TimeSpan.FromMinutes(10);
254+
}, handler, true);
255+
var client = server.CreateClient();
219256
client.SetBearerToken(expectedToken);
220257

221258
var firstResponse = await client.GetAsync("http://test");
@@ -228,6 +265,7 @@ public async Task ActiveToken_With_SavedToken_And_Caching()
228265
var responseData = JsonConvert.DeserializeObject<Dictionary<string, string>>(responseDataStr);
229266

230267
responseData.Should().Contain("token", expectedToken);
268+
AssertCacheItemExists(server, cacheKeyPrefix, expectedToken);
231269
}
232270

233271
[Fact]
@@ -236,15 +274,15 @@ public async Task Repeated_active_token_with_caching_enabled_should_hit_cache()
236274
var expectedToken = "expected_token";
237275
var handler = new IntrospectionEndpointHandler(IntrospectionEndpointHandler.Behavior.Active, TimeSpan.FromHours(1));
238276

239-
var client = PipelineFactory.CreateClient((o) =>
277+
var server = PipelineFactory.CreateServer((o) =>
240278
{
241279
_options(o);
242-
280+
243281
o.SaveToken = true;
244282
o.EnableCaching = true;
245283
o.CacheDuration = TimeSpan.FromMinutes(10);
246284
}, handler, true);
247-
285+
var client = server.CreateClient();
248286
client.SetBearerToken(expectedToken);
249287

250288
var firstResponse = await client.GetAsync("http://test");
@@ -255,6 +293,7 @@ public async Task Repeated_active_token_with_caching_enabled_should_hit_cache()
255293
handler.SentIntrospectionRequest = false;
256294
var secondResponse = await client.GetAsync("http://test");
257295
handler.SentIntrospectionRequest.Should().BeFalse();
296+
AssertCacheItemExists(server, string.Empty, expectedToken);
258297
}
259298

260299
[Fact]
@@ -263,15 +302,15 @@ public async Task Repeated_inactive_token_with_caching_enabled_should_hit_cache(
263302
var expectedToken = "expected_token";
264303
var handler = new IntrospectionEndpointHandler(IntrospectionEndpointHandler.Behavior.Inactive);
265304

266-
var client = PipelineFactory.CreateClient((o) =>
305+
var server = PipelineFactory.CreateServer((o) =>
267306
{
268307
_options(o);
269308

270309
o.SaveToken = true;
271310
o.EnableCaching = true;
272311
o.CacheDuration = TimeSpan.FromMinutes(10);
273312
}, handler, true);
274-
313+
var client = server.CreateClient();
275314
client.SetBearerToken(expectedToken);
276315

277316
var firstResponse = await client.GetAsync("http://test");
@@ -283,6 +322,7 @@ public async Task Repeated_inactive_token_with_caching_enabled_should_hit_cache(
283322
var secondResponse = await client.GetAsync("http://test");
284323
secondResponse.StatusCode.Should().Be(HttpStatusCode.Unauthorized);
285324
handler.SentIntrospectionRequest.Should().BeFalse();
325+
AssertCacheItemExists(server, string.Empty, expectedToken);
286326
}
287327

288328
[Fact]
@@ -300,5 +340,13 @@ public async Task ActiveToken_With_Discovery_Unavailable_On_First_Request()
300340
var result = await client.GetAsync("http://test");
301341
result.StatusCode.Should().Be(HttpStatusCode.OK);
302342
}
343+
344+
private void AssertCacheItemExists(TestServer testServer, string cacheKeyPrefix, string token)
345+
{
346+
var cache = testServer.Services.GetService<IDistributedCache>();
347+
var cacheItem = cache.GetString($"{cacheKeyPrefix}{token.ToSha256()}");
348+
349+
cacheItem.Should().NotBeNullOrEmpty();
350+
}
303351
}
304352
}

0 commit comments

Comments
 (0)