From e3ac1c2c40f56653b95ee0a8968ec795366987ca Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Thu, 14 Nov 2024 11:11:49 +0000 Subject: [PATCH 1/4] Add no-store and no-cache to the Cache-Control header on auth responses --- .../Orchestrator.Tests/Integration/AuthHandlingTests.cs | 6 ++++++ .../Orchestrator/Features/Auth/AuthController.cs | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs b/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs index aa7818981..d991fe9ee 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs @@ -54,6 +54,8 @@ public async Task Get_Clickthrough_UnknownCustomer_Returns400() // Assert response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); } [Fact] @@ -67,6 +69,8 @@ public async Task Get_UnknownRole_Returns404() // Assert response.StatusCode.Should().Be(HttpStatusCode.NotFound); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); } [Fact] @@ -215,6 +219,8 @@ public async Task Get_Token_Returns200_WithAccessToken_IfSuccess_AndMessageIdNot var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["accessToken"].Value().Should().Be(token.Entity.BearerToken); responseBody["expiresIn"].Value().Should().Be(token.Entity.Ttl); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); } #endregion diff --git a/src/protagonist/Orchestrator/Features/Auth/AuthController.cs b/src/protagonist/Orchestrator/Features/Auth/AuthController.cs index bba81c5b7..73e6b8542 100644 --- a/src/protagonist/Orchestrator/Features/Auth/AuthController.cs +++ b/src/protagonist/Orchestrator/Features/Auth/AuthController.cs @@ -29,6 +29,7 @@ public AuthController(IMediator mediator, IOptions cacheSettings, /// Handle clickthrough auth request - create a new auth cookie and return View for user to close /// [Route("{customer}/clickthrough")] + [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] [HttpGet] public async Task Clickthrough(int customer) { @@ -47,6 +48,7 @@ public async Task Clickthrough(int customer) /// See https://iiif.io/api/auth/1.0/#access-token-service /// [Route("{customer}/token")] + [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] [HttpGet] public async Task Token(int customer, string? messageId, string? origin) { @@ -87,6 +89,7 @@ public async Task Token(int customer, string? messageId, string? /// Name of authService to initiate. /// Redirect to downstream role-provider login service [Route("{customer}/{authService}")] + [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] [HttpGet] public async Task InitiateAuthService(int customer, string authService) { @@ -104,6 +107,7 @@ public async Task InitiateAuthService(int customer, string authSe /// Name of authService. /// Role-provider token [Route("{customer}/{authService}")] + [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] [HttpGet] public async Task RoleProviderToken(int customer, string authService, [RequiredFromQuery] string token) @@ -125,6 +129,7 @@ public async Task RoleProviderToken(int customer, string authServ /// Name of authService. /// [Route("{customer}/{authService}/logout")] + [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] [HttpGet] public async Task Logout(int customer, string authService) { From 9437b6c8d9090c5ffb6e2977913e2c23d86dc203 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Thu, 14 Nov 2024 15:10:54 +0000 Subject: [PATCH 2/4] Update to use custom action attribute that doesn't send headers on failure + more checks on tests --- .../Integration/AuthHandlingTests.cs | 24 ++++++++++++++--- .../Features/Auth/AuthController.cs | 10 +++---- .../ResponseNoCacheOnSuccessAttribute.cs | 27 +++++++++++++++++++ 3 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs diff --git a/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs b/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs index d991fe9ee..7a829e398 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs @@ -54,8 +54,7 @@ public async Task Get_Clickthrough_UnknownCustomer_Returns400() // Assert response.StatusCode.Should().Be(HttpStatusCode.BadRequest); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl.Should().BeNull(); } [Fact] @@ -69,8 +68,7 @@ public async Task Get_UnknownRole_Returns404() // Assert response.StatusCode.Should().Be(HttpStatusCode.NotFound); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl.Should().BeNull(); } [Fact] @@ -114,6 +112,7 @@ public async Task Get_Token_Returns401_WithErrorJson_IfNoCookie_AndMessageIdNotP // Assert response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + response.Headers.CacheControl.Should().BeNull(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("missingCredentials"); @@ -133,6 +132,7 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieDoesNotContainId_An // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); + response.Headers.CacheControl.Should().BeNull(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -152,6 +152,7 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieDoesNotContainKnown // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); + response.Headers.CacheControl.Should().BeNull(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -173,6 +174,7 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieContainsId_ForDiffe // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); + response.Headers.CacheControl.Should().BeNull(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -194,6 +196,7 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieContainsExpiredId_A // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); + response.Headers.CacheControl.Should().BeNull(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -240,6 +243,8 @@ public async Task Get_Token_ReturnsView_WithErrorJson_IfNoCookie() var responseBody = await ParseHtmlTokenReponse(response); responseBody["error"].Value().Should().Be("missingCredentials"); responseBody["description"].Value().Should().Be("Required cookie missing"); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); } [Fact] @@ -259,6 +264,8 @@ public async Task Get_Token_ReturnsView_WithErrorJson_IfCookieDoesNotContainId() var responseBody = await ParseHtmlTokenReponse(response); responseBody["error"].Value().Should().Be("invalidCredentials"); responseBody["description"].Value().Should().Be("Id not found in cookie"); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); } [Fact] @@ -278,6 +285,8 @@ public async Task Get_Token_ReturnsView_WithErrorJson_IfCookieDoesNotContainKnow var responseBody = await ParseHtmlTokenReponse(response); responseBody["error"].Value().Should().Be("invalidCredentials"); responseBody["description"].Value().Should().Be("Credentials provided unknown or expired"); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); } [Fact] @@ -299,6 +308,8 @@ public async Task Get_Token_ReturnsView_WithErrorJson_IfCookieContainsId_ForDiff var responseBody = await ParseHtmlTokenReponse(response); responseBody["error"].Value().Should().Be("invalidCredentials"); responseBody["description"].Value().Should().Be("Credentials provided unknown or expired"); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); } [Fact] @@ -320,6 +331,8 @@ public async Task Get_Token_ReturnsView_WithErrorJson_IfCookieContainsExpiredId( var responseBody = await ParseHtmlTokenReponse(response); responseBody["error"].Value().Should().Be("invalidCredentials"); responseBody["description"].Value().Should().Be("Credentials provided unknown or expired"); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); } [Fact] @@ -342,6 +355,8 @@ public async Task Get_Token_ReturnsView_WithAccessToken_IfSuccess() responseBody["accessToken"].Value().Should().Be(token.Entity.BearerToken); responseBody["expiresIn"].Value().Should().Be(token.Entity.Ttl); responseBody["messageId"].Value().Should().Be("123"); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); } #endregion @@ -375,6 +390,7 @@ public async Task ProbeService_404_IfAssetNotFound() // Assert result.StatusCode.Should().Be(HttpStatusCode.NotFound); + result.Headers.CacheControl.Should().BeNull(); result.Content.Headers.ContentType.MediaType .Should().Be("application/problem+json", "this isn't an AuthProbeResult2"); } diff --git a/src/protagonist/Orchestrator/Features/Auth/AuthController.cs b/src/protagonist/Orchestrator/Features/Auth/AuthController.cs index 73e6b8542..76a1aa8d5 100644 --- a/src/protagonist/Orchestrator/Features/Auth/AuthController.cs +++ b/src/protagonist/Orchestrator/Features/Auth/AuthController.cs @@ -29,7 +29,7 @@ public AuthController(IMediator mediator, IOptions cacheSettings, /// Handle clickthrough auth request - create a new auth cookie and return View for user to close /// [Route("{customer}/clickthrough")] - [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] + [ResponseNoCacheOnSuccess] [HttpGet] public async Task Clickthrough(int customer) { @@ -48,7 +48,7 @@ public async Task Clickthrough(int customer) /// See https://iiif.io/api/auth/1.0/#access-token-service /// [Route("{customer}/token")] - [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] + [ResponseNoCacheOnSuccess] [HttpGet] public async Task Token(int customer, string? messageId, string? origin) { @@ -89,7 +89,7 @@ public async Task Token(int customer, string? messageId, string? /// Name of authService to initiate. /// Redirect to downstream role-provider login service [Route("{customer}/{authService}")] - [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] + [ResponseNoCacheOnSuccess] [HttpGet] public async Task InitiateAuthService(int customer, string authService) { @@ -107,7 +107,7 @@ public async Task InitiateAuthService(int customer, string authSe /// Name of authService. /// Role-provider token [Route("{customer}/{authService}")] - [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] + [ResponseNoCacheOnSuccess] [HttpGet] public async Task RoleProviderToken(int customer, string authService, [RequiredFromQuery] string token) @@ -129,7 +129,7 @@ public async Task RoleProviderToken(int customer, string authServ /// Name of authService. /// [Route("{customer}/{authService}/logout")] - [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] + [ResponseNoCacheOnSuccess] [HttpGet] public async Task Logout(int customer, string authService) { diff --git a/src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs b/src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs new file mode 100644 index 000000000..e4d1f1ab9 --- /dev/null +++ b/src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs @@ -0,0 +1,27 @@ +using System; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace Orchestrator.Infrastructure; + +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] +public class ResponseNoCacheOnSuccessAttribute : ActionFilterAttribute +{ + public override void OnResultExecuting(ResultExecutingContext filterContext) + { + var result = filterContext.Result as ObjectResult; + + if ((result != null && IsSuccessStatusCode(result.StatusCode!.Value)) || + (result == null && IsSuccessStatusCode(filterContext.HttpContext.Response.StatusCode))) // avoids issues with ContentResult types + { + filterContext.HttpContext.Response.Headers.CacheControl = "no-cache,no-store"; + } + + base.OnResultExecuting(filterContext); + } + + private bool IsSuccessStatusCode(int statusCode) + { + return statusCode is >= 200 and <= 299; + } +} \ No newline at end of file From 526eb94ad168556141f1dcc41d5d84ffe2bf068d Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Thu, 14 Nov 2024 15:44:11 +0000 Subject: [PATCH 3/4] allow caching only on BadRequest types, rather than all failures --- .../Integration/AuthHandlingTests.cs | 18 ++++++++++++------ .../ResponseNoCacheOnSuccessAttribute.cs | 15 +++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs b/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs index 7a829e398..98cb8d341 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs @@ -68,7 +68,8 @@ public async Task Get_UnknownRole_Returns404() // Assert response.StatusCode.Should().Be(HttpStatusCode.NotFound); - response.Headers.CacheControl.Should().BeNull(); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); } [Fact] @@ -112,7 +113,8 @@ public async Task Get_Token_Returns401_WithErrorJson_IfNoCookie_AndMessageIdNotP // Assert response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); - response.Headers.CacheControl.Should().BeNull(); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("missingCredentials"); @@ -132,7 +134,8 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieDoesNotContainId_An // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); - response.Headers.CacheControl.Should().BeNull(); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -152,7 +155,8 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieDoesNotContainKnown // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); - response.Headers.CacheControl.Should().BeNull(); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -174,7 +178,8 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieContainsId_ForDiffe // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); - response.Headers.CacheControl.Should().BeNull(); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -196,7 +201,8 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieContainsExpiredId_A // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); - response.Headers.CacheControl.Should().BeNull(); + response.Headers.CacheControl!.NoCache.Should().BeTrue(); + response.Headers.CacheControl.NoStore.Should().BeTrue(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); diff --git a/src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs b/src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs index e4d1f1ab9..03c31279f 100644 --- a/src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs +++ b/src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.Linq; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; @@ -7,12 +9,17 @@ namespace Orchestrator.Infrastructure; [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] public class ResponseNoCacheOnSuccessAttribute : ActionFilterAttribute { + private readonly List disallowedStatusCodes = new () + { + 400 + }; + public override void OnResultExecuting(ResultExecutingContext filterContext) { var result = filterContext.Result as ObjectResult; - if ((result != null && IsSuccessStatusCode(result.StatusCode!.Value)) || - (result == null && IsSuccessStatusCode(filterContext.HttpContext.Response.StatusCode))) // avoids issues with ContentResult types + if ((result != null && IsAllowedStatusCode(result.StatusCode!.Value)) || + (result == null && IsAllowedStatusCode(filterContext.HttpContext.Response.StatusCode))) // avoids issues with ContentResult types { filterContext.HttpContext.Response.Headers.CacheControl = "no-cache,no-store"; } @@ -20,8 +27,8 @@ public override void OnResultExecuting(ResultExecutingContext filterContext) base.OnResultExecuting(filterContext); } - private bool IsSuccessStatusCode(int statusCode) + private bool IsAllowedStatusCode(int statusCode) { - return statusCode is >= 200 and <= 299; + return disallowedStatusCodes.All(s => s != statusCode); } } \ No newline at end of file From e7c86551600bfdfe7f2a0577d77e74ec3bdb5b16 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Thu, 14 Nov 2024 17:05:01 +0000 Subject: [PATCH 4/4] move back to using inbuilt response cache --- .../Integration/AuthHandlingTests.cs | 41 +++++++------------ .../Features/Auth/AuthController.cs | 10 ++--- .../ResponseNoCacheOnSuccessAttribute.cs | 34 --------------- 3 files changed, 19 insertions(+), 66 deletions(-) delete mode 100644 src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs diff --git a/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs b/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs index 98cb8d341..fce9c556f 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/AuthHandlingTests.cs @@ -54,7 +54,7 @@ public async Task Get_Clickthrough_UnknownCustomer_Returns400() // Assert response.StatusCode.Should().Be(HttpStatusCode.BadRequest); - response.Headers.CacheControl.Should().BeNull(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); } [Fact] @@ -68,8 +68,7 @@ public async Task Get_UnknownRole_Returns404() // Assert response.StatusCode.Should().Be(HttpStatusCode.NotFound); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); } [Fact] @@ -113,8 +112,7 @@ public async Task Get_Token_Returns401_WithErrorJson_IfNoCookie_AndMessageIdNotP // Assert response.StatusCode.Should().Be(HttpStatusCode.Unauthorized); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("missingCredentials"); @@ -134,8 +132,7 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieDoesNotContainId_An // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -155,8 +152,7 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieDoesNotContainKnown // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -178,8 +174,7 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieContainsId_ForDiffe // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -201,8 +196,7 @@ public async Task Get_Token_Returns403_WithErrorJson_IfCookieContainsExpiredId_A // Assert response.StatusCode.Should().Be(HttpStatusCode.Forbidden); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["error"].Value().Should().Be("invalidCredentials"); @@ -228,8 +222,7 @@ public async Task Get_Token_Returns200_WithAccessToken_IfSuccess_AndMessageIdNot var responseBody = JObject.Parse(await response.Content.ReadAsStringAsync()); responseBody["accessToken"].Value().Should().Be(token.Entity.BearerToken); responseBody["expiresIn"].Value().Should().Be(token.Entity.Ttl); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); } #endregion @@ -249,8 +242,7 @@ public async Task Get_Token_ReturnsView_WithErrorJson_IfNoCookie() var responseBody = await ParseHtmlTokenReponse(response); responseBody["error"].Value().Should().Be("missingCredentials"); responseBody["description"].Value().Should().Be("Required cookie missing"); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); } [Fact] @@ -270,8 +262,7 @@ public async Task Get_Token_ReturnsView_WithErrorJson_IfCookieDoesNotContainId() var responseBody = await ParseHtmlTokenReponse(response); responseBody["error"].Value().Should().Be("invalidCredentials"); responseBody["description"].Value().Should().Be("Id not found in cookie"); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); } [Fact] @@ -291,8 +282,7 @@ public async Task Get_Token_ReturnsView_WithErrorJson_IfCookieDoesNotContainKnow var responseBody = await ParseHtmlTokenReponse(response); responseBody["error"].Value().Should().Be("invalidCredentials"); responseBody["description"].Value().Should().Be("Credentials provided unknown or expired"); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); } [Fact] @@ -314,8 +304,7 @@ public async Task Get_Token_ReturnsView_WithErrorJson_IfCookieContainsId_ForDiff var responseBody = await ParseHtmlTokenReponse(response); responseBody["error"].Value().Should().Be("invalidCredentials"); responseBody["description"].Value().Should().Be("Credentials provided unknown or expired"); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); } [Fact] @@ -337,8 +326,7 @@ public async Task Get_Token_ReturnsView_WithErrorJson_IfCookieContainsExpiredId( var responseBody = await ParseHtmlTokenReponse(response); responseBody["error"].Value().Should().Be("invalidCredentials"); responseBody["description"].Value().Should().Be("Credentials provided unknown or expired"); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); } [Fact] @@ -361,8 +349,7 @@ public async Task Get_Token_ReturnsView_WithAccessToken_IfSuccess() responseBody["accessToken"].Value().Should().Be(token.Entity.BearerToken); responseBody["expiresIn"].Value().Should().Be(token.Entity.Ttl); responseBody["messageId"].Value().Should().Be("123"); - response.Headers.CacheControl!.NoCache.Should().BeTrue(); - response.Headers.CacheControl.NoStore.Should().BeTrue(); + response.Headers.CacheControl!.NoStore.Should().BeTrue(); } #endregion diff --git a/src/protagonist/Orchestrator/Features/Auth/AuthController.cs b/src/protagonist/Orchestrator/Features/Auth/AuthController.cs index 76a1aa8d5..42f7f1d7c 100644 --- a/src/protagonist/Orchestrator/Features/Auth/AuthController.cs +++ b/src/protagonist/Orchestrator/Features/Auth/AuthController.cs @@ -29,7 +29,7 @@ public AuthController(IMediator mediator, IOptions cacheSettings, /// Handle clickthrough auth request - create a new auth cookie and return View for user to close /// [Route("{customer}/clickthrough")] - [ResponseNoCacheOnSuccess] + [ResponseCache(NoStore = true)] [HttpGet] public async Task Clickthrough(int customer) { @@ -48,7 +48,7 @@ public async Task Clickthrough(int customer) /// See https://iiif.io/api/auth/1.0/#access-token-service /// [Route("{customer}/token")] - [ResponseNoCacheOnSuccess] + [ResponseCache(NoStore = true)] [HttpGet] public async Task Token(int customer, string? messageId, string? origin) { @@ -89,7 +89,7 @@ public async Task Token(int customer, string? messageId, string? /// Name of authService to initiate. /// Redirect to downstream role-provider login service [Route("{customer}/{authService}")] - [ResponseNoCacheOnSuccess] + [ResponseCache(NoStore = true)] [HttpGet] public async Task InitiateAuthService(int customer, string authService) { @@ -107,7 +107,7 @@ public async Task InitiateAuthService(int customer, string authSe /// Name of authService. /// Role-provider token [Route("{customer}/{authService}")] - [ResponseNoCacheOnSuccess] + [ResponseCache(NoStore = true)] [HttpGet] public async Task RoleProviderToken(int customer, string authService, [RequiredFromQuery] string token) @@ -129,7 +129,7 @@ public async Task RoleProviderToken(int customer, string authServ /// Name of authService. /// [Route("{customer}/{authService}/logout")] - [ResponseNoCacheOnSuccess] + [ResponseCache(NoStore = true)] [HttpGet] public async Task Logout(int customer, string authService) { diff --git a/src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs b/src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs deleted file mode 100644 index 03c31279f..000000000 --- a/src/protagonist/Orchestrator/Infrastructure/ResponseNoCacheOnSuccessAttribute.cs +++ /dev/null @@ -1,34 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.Filters; - -namespace Orchestrator.Infrastructure; - -[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] -public class ResponseNoCacheOnSuccessAttribute : ActionFilterAttribute -{ - private readonly List disallowedStatusCodes = new () - { - 400 - }; - - public override void OnResultExecuting(ResultExecutingContext filterContext) - { - var result = filterContext.Result as ObjectResult; - - if ((result != null && IsAllowedStatusCode(result.StatusCode!.Value)) || - (result == null && IsAllowedStatusCode(filterContext.HttpContext.Response.StatusCode))) // avoids issues with ContentResult types - { - filterContext.HttpContext.Response.Headers.CacheControl = "no-cache,no-store"; - } - - base.OnResultExecuting(filterContext); - } - - private bool IsAllowedStatusCode(int statusCode) - { - return disallowedStatusCodes.All(s => s != statusCode); - } -} \ No newline at end of file