diff --git a/src/Exceptionless.Core/Services/OAuthService.cs b/src/Exceptionless.Core/Services/OAuthService.cs index fb0d7ce3c4..d8f7aec1cd 100644 --- a/src/Exceptionless.Core/Services/OAuthService.cs +++ b/src/Exceptionless.Core/Services/OAuthService.cs @@ -72,7 +72,6 @@ public class OAuthService(OAuthServerOptions options, ICacheClient cacheClient, private const string RefreshTokenLockPrefix = "oauth:refresh:"; private const string AccessTokenClientValidityCachePrefix = "oauth:client-valid:"; private const int OAuthGrantFamilyPageLimit = 1000; - private static readonly TimeSpan AccessTokenClientValidityCacheLifetime = TimeSpan.FromSeconds(30); private const string ClientMetadataNotes = "Discovered from OAuth client metadata document."; private const string DynamicClientRegistrationNotes = "Registered through OAuth dynamic client registration."; private static readonly Regex CodeChallengeRegex = new("^[A-Za-z0-9_-]{43}$", RegexOptions.Compiled | RegexOptions.CultureInvariant); @@ -175,15 +174,10 @@ public async Task IsAccessTokenClientValidAsync(string? clientId) return false; clientId = clientId.Trim(); - string cacheKey = GetAccessTokenClientValidityCacheKey(clientId); - bool? cached = await cacheClient.GetAsync(cacheKey, null); - if (cached.HasValue) - return cached.Value; - - var application = await oauthApplicationRepository.GetByClientIdAsync(clientId); - bool isValid = application is { IsDisabled: false }; - await cacheClient.SetAsync(cacheKey, isValid, AccessTokenClientValidityCacheLifetime); - return isValid; + // An older lookup can complete after the application was disabled and its + // cache entry removed. Current application state authorizes the request. + var application = await oauthApplicationRepository.GetByClientIdAsync(clientId, o => o.Cache(false)); + return application is { IsDisabled: false }; } public Task ClearAccessTokenClientValidityCacheAsync(string? clientId) diff --git a/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs b/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs index 99d1f44c4f..fc01ccfc0f 100644 --- a/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs +++ b/src/Exceptionless.Web/Security/ApiKeyAuthenticationHandler.cs @@ -268,9 +268,13 @@ private async Task AuthenticateOAuthBearerAsync(string token return AuthenticateResult.Success(CreateOAuthUserAuthenticationTicket(user, tokenRecord, activeOAuthOrganizationIds)); } - private Task GetTokenRecordAsync(string token) + private async Task GetTokenRecordAsync(string token) { - return _tokenRepository.GetByIdAsync(token, o => o.Cache()); + var record = await _tokenRepository.GetByIdAsync(token, o => o.Cache()); + // Delayed reads may repopulate a snapshot after logout or revocation. + if (record is not null && (record.Type == TokenType.Authentication || !String.IsNullOrEmpty(record.UserId))) + return await _tokenRepository.GetByIdAsync(token, o => o.Cache(false)); + return record; } private async Task GetOAuthTokenRecordAsync(string token) @@ -283,14 +287,14 @@ private async Task AuthenticateOAuthBearerAsync(string token if (!String.IsNullOrEmpty(tokenId)) { - var cachedToken = await _oauthTokenRepository.GetByIdAsync(tokenId, o => o.Cache()); + var cachedToken = await _oauthTokenRepository.GetByIdAsync(tokenId, o => o.Cache(false)); if (cachedToken is not null && String.Equals(cachedToken.AccessTokenHash, accessTokenHash, StringComparison.Ordinal)) return cachedToken; await _cacheClient.RemoveAsync(cacheKey); } - var results = await _oauthTokenRepository.GetByAccessTokenHashAsync(accessTokenHash); + var results = await _oauthTokenRepository.GetByAccessTokenHashAsync(accessTokenHash, o => o.Cache(false)); var tokenRecord = results.Documents.FirstOrDefault(); await _cacheClient.SetAsync(cacheKey, tokenRecord?.Id ?? OAuthAccessTokenCacheMiss, TimeSpan.FromMinutes(5)); diff --git a/tests/Exceptionless.Tests/Api/Endpoints/UserTokenStateEndpointTests.cs b/tests/Exceptionless.Tests/Api/Endpoints/UserTokenStateEndpointTests.cs new file mode 100644 index 0000000000..495f971e7e --- /dev/null +++ b/tests/Exceptionless.Tests/Api/Endpoints/UserTokenStateEndpointTests.cs @@ -0,0 +1,187 @@ +using System.Reflection; +using Exceptionless.Core; +using Exceptionless.Core.Authorization; +using Exceptionless.Core.Extensions; +using Exceptionless.Core.Models; +using Exceptionless.Core.Repositories; +using Exceptionless.Core.Repositories.Configuration; +using Exceptionless.Core.Services; +using Exceptionless.Core.Utility; +using Exceptionless.Core.Validation; +using Exceptionless.Tests.Extensions; +using Exceptionless.Web.Models; +using Exceptionless.Web.Models.Admin; +using FluentRest; +using Foundatio.Repositories; +using Foundatio.Repositories.Utility; +using Xunit; + +namespace Exceptionless.Tests.Api.Endpoints; + +public sealed class UserTokenStateEndpointTests(ITestOutputHelper output, AppWebHostFactory factory) : IntegrationTestsBase(output, factory) +{ + protected override void RegisterServices(IServiceCollection services) + { + base.RegisterServices(services); + services.ReplaceSingleton(); + services.ReplaceSingleton(); + services.ReplaceSingleton(provider => + { + var repository = new OAuthApplicationRepository(provider.GetRequiredService(), + provider.GetRequiredService(), provider.GetRequiredService()); + var proxy = DispatchProxy.Create(); + ((PausingApplicationRepository)(object)proxy).Repository = repository; + return proxy; + }); + } + + protected override async Task ResetDataAsync() + { + await base.ResetDataAsync(); + await GetService().CreateDataAsync(); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task AuthenticateAsync_ChangedUserTokenWithLateCacheWrite_ReturnsUnauthorized(bool logout) + { + var repository = (CachingTokenRepository)GetService(); + var login = await SendRequestAsAsync(r => r.Post().AppendPath("auth/login") + .Content(new Login { Email = SampleDataService.TEST_USER_EMAIL, Password = SampleDataService.TEST_USER_PASSWORD }) + .StatusCodeShouldBeOk()); + Assert.NotNull(login); + if (!logout) + { + var access = await repository.GetByIdAsync(login.Token, o => o.Cache(false)); + Assert.NotNull(access); + access.Type = TokenType.Access; + await repository.SaveAsync(access, o => o.ImmediateConsistency()); + } + await SendRequestAsync(r => r.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeOk()); + var stale = await repository.GetByIdAsync(login.Token, o => o.Cache(false)); + Assert.NotNull(stale); + if (logout) + { + await SendRequestAsync(r => r.BearerToken(login.Token).AppendPath("auth/logout").StatusCodeShouldBeOk()); + Assert.Null(await repository.GetByIdAsync(login.Token, o => o.Cache(false))); + } + else + { + var current = await repository.GetByIdAsync(login.Token, o => o.Cache(false)); + Assert.NotNull(current); + current.IsDisabled = true; + await repository.SaveAsync(current, o => o.ImmediateConsistency()); + } + await repository.CacheTokenAsync(stale); + await SendRequestAsync(r => r.BearerToken(login.Token).AppendPath("users/me").StatusCodeShouldBeUnauthorized()); + await SendRequestAsync(r => r.BasicAuthorization("client", login.Token).AppendPath("users/me").StatusCodeShouldBeUnauthorized()); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task AuthenticateAsync_ChangedOAuthStateWithLateCacheWrite_ReturnsUnauthorized(bool disableApplication) + { + var applications = GetService(); + var tokens = (CachingOAuthTokenRepository)GetService(); + var user = await GetService().GetByEmailAddressAsync(SampleDataService.TEST_USER_EMAIL); + Assert.NotNull(user); + var now = TimeProvider.GetUtcNow().UtcDateTime; + var application = new OAuthApplication + { + Id = ObjectId.GenerateNewId().ToString(), ClientId = "late-cache-client", Name = "Late cache client", + RedirectUris = ["http://localhost/callback"], Scopes = [AuthorizationRoles.ProjectsRead], + CreatedByUserId = user.Id, UpdatedByUserId = user.Id, CreatedUtc = now, UpdatedUtc = now + }; + await applications.AddAsync(application, o => o.ImmediateConsistency()); + string accessToken = StringExtensions.GetRandomString(OAuthService.OAuthTokenLength); + var stale = new OAuthToken + { + Id = ObjectId.GenerateNewId().ToString(), UserId = user.Id, AuthenticationVersion = user.AuthenticationVersion, + ClientId = application.ClientId, GrantId = "late-cache-grant", Resource = "http://localhost:7110/api/v2", + AccessTokenHash = OAuthService.CreateTokenHash(accessToken), Scopes = [AuthorizationRoles.ProjectsRead], + OrganizationIds = [SampleDataService.TEST_ORG_ID], ExpiresUtc = now.AddHours(1), + CreatedBy = user.Id, CreatedUtc = now, UpdatedUtc = now + }; + await tokens.AddAsync(stale, o => o.ImmediateConsistency()); + await SendRequestAsync(r => r.BearerToken(accessToken).AppendPath("projects").StatusCodeShouldBeOk()); + + if (disableApplication) + { + var service = GetService(); + await service.ClearAccessTokenClientValidityCacheAsync(application.ClientId); + var pause = ((PausingApplicationRepository)(object)applications).PauseNextRead(); + var validation = service.IsAccessTokenClientValidAsync(application.ClientId); + await pause.Read.Task.WaitAsync(TimeSpan.FromSeconds(10), TestCancellationToken); + try + { + await SendRequestAsync(r => r.Put().AsGlobalAdminUser().AppendPaths("admin", "oauth-applications", application.Id) + .Content(new UpdateOAuthApplication + { + ClientId = application.ClientId, Name = application.Name, RedirectUris = application.RedirectUris, + Scopes = application.Scopes, IsDisabled = true + }).StatusCodeShouldBeOk()); + } + finally + { + pause.Resume.TrySetResult(); + } + await validation; + } + else + { + // Revoke through the endpoint after the access-token lookup was warmed. + await SendRequestAsync(r => r.Post().AppendPath("oauth/revoke") + .Content(new FormUrlEncodedContent(new Dictionary + { + ["token"] = accessToken, ["client_id"] = application.ClientId + })).StatusCodeShouldBeOk()); + var stored = await tokens.GetByIdAsync(stale.Id, o => o.Cache(false)); + Assert.NotNull(stored); + Assert.True(stored.IsDisabled); + await tokens.CacheTokenAsync(stale); + } + await SendRequestAsync(r => r.BearerToken(accessToken).AppendPath("projects").StatusCodeShouldBeUnauthorized()); + } + + private sealed class CachingTokenRepository(ExceptionlessElasticConfiguration configuration, MiniValidationValidator validator, AppOptions options) + : TokenRepository(configuration, validator, options) + { + public Task CacheTokenAsync(Token token) => AddDocumentsToCacheAsync(token, new CommandOptions().Cache(), false); + } + + private sealed class CachingOAuthTokenRepository(ExceptionlessElasticConfiguration configuration, MiniValidationValidator validator, AppOptions options) + : OAuthTokenRepository(configuration, validator, options) + { + public Task CacheTokenAsync(OAuthToken token) => AddDocumentsToCacheAsync(token, new CommandOptions().Cache(), false); + } + + public class PausingApplicationRepository : DispatchProxy + { + public IOAuthApplicationRepository Repository { get; set; } = null!; + private ReadPause? _pause; + public ReadPause PauseNextRead() => _pause = new ReadPause(); + protected override object? Invoke(MethodInfo? method, object?[]? args) + { + var result = method!.Invoke(Repository, args); + if (method.Name == nameof(IOAuthApplicationRepository.GetByClientIdAsync) && Interlocked.Exchange(ref _pause, null) is { } pause) + return WaitForReadAsync((Task)result!, pause); + return result; + } + + private static async Task WaitForReadAsync(Task read, ReadPause pause) + { + var application = await read; + pause.Read.TrySetResult(); + await pause.Resume.Task; + return application; + } + } + + public sealed class ReadPause + { + public TaskCompletionSource Read { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + public TaskCompletionSource Resume { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + } +} diff --git a/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs b/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs index 4b104a4d30..1023088976 100644 --- a/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs +++ b/tests/Exceptionless.Tests/Security/ApiKeyAuthenticationHandlerTests.cs @@ -63,7 +63,7 @@ public async Task AuthenticateAsync_OAuthCachedUser_RejectsMismatchWithoutRevoki oauthTokens.Setup(repository => repository.GetByIdAsync(tokenId, It.IsAny>())).ReturnsAsync(token); oauthTokens.Setup(repository => repository.SaveAsync(token, It.IsAny>())).ReturnsAsync(token); var applications = new Mock(MockBehavior.Strict); - applications.Setup(repository => repository.GetByClientIdAsync(clientId, null)).ReturnsAsync(new OAuthApplication { ClientId = clientId }); + applications.Setup(repository => repository.GetByClientIdAsync(clientId, It.IsAny>())).ReturnsAsync(new OAuthApplication { ClientId = clientId }); var oauthService = new OAuthService(new OAuthServerOptions(), cache, GetService(), applications.Object, GetService(), oauthTokens.Object, users.Object, TimeProvider); var options = new Mock>(MockBehavior.Strict); @@ -88,7 +88,7 @@ public async Task AuthenticateAsync_OAuthCachedUser_RejectsMismatchWithoutRevoki oauthTokens.Verify(repository => repository.SaveAsync(token, It.IsAny>()), Times.Exactly(isActive ? 0 : 1)); oauthTokens.Verify(repository => repository.GetByIdAsync(tokenId, It.IsAny>()), Times.Once); users.Verify(repository => repository.GetByIdAsync(user.Id, It.IsAny>()), Times.Once); - applications.Verify(repository => repository.GetByClientIdAsync(clientId, null), Times.Once); + applications.Verify(repository => repository.GetByClientIdAsync(clientId, It.IsAny>()), Times.Once); oauthTokens.VerifyNoOtherCalls(); users.VerifyNoOtherCalls(); applications.VerifyNoOtherCalls(); @@ -149,7 +149,7 @@ public async Task AuthenticateAsync_AuthenticationVersion_RequiresActiveUserAndM // Assert Assert.Equal(expectedSuccess, result.Succeeded); users.Verify(repository => repository.GetByIdAsync(user.Id, It.IsAny>()), Times.Once); - tokens.Verify(repository => repository.GetByIdAsync(tokenId, It.IsAny>()), Times.Once); + tokens.Verify(repository => repository.GetByIdAsync(tokenId, It.IsAny>()), Times.Exactly(2)); users.VerifyNoOtherCalls(); tokens.VerifyNoOtherCalls(); oauthTokens.VerifyNoOtherCalls(); diff --git a/tests/http/oauth.http b/tests/http/oauth.http index 82c975250c..68e805ed78 100644 --- a/tests/http/oauth.http +++ b/tests/http/oauth.http @@ -5,6 +5,7 @@ # Copy code from the authorization response's redirect_uri into authorizationCode. Each code can be exchanged once. @rootUrl = http://localhost:7110 @apiUrl = {{rootUrl}}/api/v2 +# Bearer authentication checks current OAuth token and application state after revocation or disablement. @email = admin@exceptionless.test @password = tester @organizationId = 537650f3b77efe23a47914f3