Merge pull request #1565 from DuendeSoftware/joe/max-age-0

Prevent infinite loop when max_age=0

src/IdentityServer/Constants.cs

@@ -131,6 +131,12 @@ public static class SigningAlgorithms
     /// </summary>
     public const string ProcessedPrompt = "suppressed_" + OidcConstants.AuthorizeRequest.Prompt;
 
+    /// <summary>
+    /// The name of the parameter passed to the authorize callback to indicate
+    /// max age that have already been used.
+    /// </summary>
+    public const string ProcessedMaxAge = "suppressed_" + OidcConstants.AuthorizeRequest.MaxAge;
+
     public static class KnownAcrValues
     {
         public const string HomeRealm = "idp:";

src/IdentityServer/Extensions/ValidatedAuthorizeRequestExtensions.cs

@@ -10,6 +10,7 @@
 using System.Security.Cryptography;
 using System.Text;
 using System.Collections.Specialized;
+using System.Globalization;
 
 #pragma warning disable 1591
 
@@ -49,6 +50,15 @@ public static void RemovePrompt(this ValidatedAuthorizeRequest request)
         }).ToArray();
     }
 
+    public static void RemoveMaxAge(this ValidatedAuthorizeRequest request)
+    {
+        if (request.MaxAge.HasValue)
+        {
+            request.Raw.Add(Constants.ProcessedMaxAge, request.MaxAge.Value.ToString(CultureInfo.InvariantCulture));
+            request.MaxAge = null;
+        }
+    }
+
     public static string GetPrefixedAcrValue(this ValidatedAuthorizeRequest request, string prefix)
     {
         var value = request.AuthenticationContextReferenceClasses

src/IdentityServer/ResponseHandling/Default/AuthorizeInteractionResponseGenerator.cs

@@ -168,16 +168,34 @@ protected internal virtual Task<InteractionResponse> ProcessCreateAccountAsync(V
     protected internal virtual async Task<InteractionResponse> ProcessLoginAsync(ValidatedAuthorizeRequest request)
     {
         using var activity = Tracing.BasicActivitySource.StartActivity("AuthorizeInteractionResponseGenerator.ProcessLogin");
-
+
+        // prompt=login, prompt=select_account, and max_age=0
+        //
+        // These parameters are all treated the same, and force a login.
+        // max_age=0 being equivalent to prompt=login is explicitly in the spec,
+        // and while selecting from multiple accounts would take a significant
+        // amount of work to implement, the user interaction would happen on the
+        // login page.
+        bool showLoginBecauseOfPrompt = false; // we need this flag because we want to check for (and suppress) either OR BOTH of the prompt and max_age params
         if (request.PromptModes.Contains(OidcConstants.PromptModes.Login) ||
             request.PromptModes.Contains(OidcConstants.PromptModes.SelectAccount))
         {
             Logger.LogInformation("Showing login: request contains prompt={0}", request.PromptModes.ToSpaceSeparatedString());
-
             // remove prompt so when we redirect back in from login page
             // we won't think we need to force a prompt again
             request.RemovePrompt();
-
+            showLoginBecauseOfPrompt = true;
+        }
+        if (request.MaxAge == 0)
+        {
+            Logger.LogInformation("Showing login: request contains max_age=0.");
+            // remove max_age=0 so when we redirect back in from login page
+            // we won't think we need to force a prompt again
+            request.RemoveMaxAge();
+            showLoginBecauseOfPrompt = true;
+        }
+        if (showLoginBecauseOfPrompt)
+        {
             return new InteractionResponse { IsLogin = true };
         }
 

src/IdentityServer/Validation/Default/AuthorizeRequestValidator.cs

@@ -696,6 +696,13 @@ private async Task<AuthorizeRequestValidationResult> ValidateOptionalParametersA
             }
         }
 
+        var processed_max_age = request.Raw.Get(Constants.ProcessedMaxAge);
+        if(processed_max_age.IsPresent())
+        {
+            request.MaxAge = null;
+            // TODO - Consider adding an OriginalMaxAge property for consistency with prompt.
+        }
+
         //////////////////////////////////////////////////////////
         // check login_hint
         //////////////////////////////////////////////////////////

test/IdentityServer.IntegrationTests/Endpoints/Authorize/AuthorizeTests.cs

@@ -1238,7 +1238,6 @@ public async Task code_flow_with_fragment_response_type_should_be_allowed()
         _mockPipeline.LoginWasCalled.Should().BeTrue();
     }
 
-
     [Fact]
     [Trait("Category", Category)]
     public async Task prompt_login_should_show_login_page_and_preserve_prompt_values()
@@ -1259,7 +1258,27 @@ public async Task prompt_login_should_show_login_page_and_preserve_prompt_values
         _mockPipeline.LoginWasCalled.Should().BeTrue();
         _mockPipeline.LoginRequest.PromptModes.Should().Contain("login");
     }
-
+
+    [Fact]
+    [Trait("Category", Category)]
+    public async Task max_age_0_should_show_login_page()
+    {
+        await _mockPipeline.LoginAsync("bob");
+
+        var url = _mockPipeline.CreateAuthorizeUrl(
+            clientId: "client3",
+            responseType: "id_token",
+            scope: "openid profile",
+            redirectUri: "https://client3/callback",
+            state: "123_state",
+            nonce: "123_nonce",
+            extra:new { max_age = "0" }
+        );
+        var response = await _mockPipeline.BrowserClient.GetAsync(url);
+
+        _mockPipeline.LoginWasCalled.Should().BeTrue();
+    }
+
     [Fact]
     [Trait("Category", Category)]
     public async Task prompt_login_should_allow_user_to_login_and_complete_authorization()
@@ -1278,7 +1297,33 @@ public async Task prompt_login_should_allow_user_to_login_and_complete_authoriza
 
         var response = await _mockPipeline.BrowserClient.GetAsync(url);
 
-        // this simulates the login page returning to the returnUrl whichi is the authorize callback page
+        // this simulates the login page returning to the returnUrl which is the authorize callback page
+        _mockPipeline.BrowserClient.AllowAutoRedirect = false;
+        response = await _mockPipeline.BrowserClient.GetAsync(IdentityServerPipeline.BaseUrl + _mockPipeline.LoginReturnUrl);
+        response.StatusCode.Should().Be(HttpStatusCode.Redirect);
+        response.Headers.Location.ToString().Should().StartWith("https://client1/callback");
+        response.Headers.Location.ToString().Should().Contain("id_token=");
+    }
+
+    [Fact]
+    [Trait("Category", Category)]
+    public async Task max_age_0_should_allow_user_to_login_and_complete_authorization()
+    {
+        await _mockPipeline.LoginAsync("bob");
+
+        var url = _mockPipeline.CreateAuthorizeUrl(
+            clientId: "client1",
+            responseType: "id_token",
+            scope: "openid profile",
+            redirectUri: "https://client1/callback",
+            state: "123_state",
+            nonce: "123_nonce",
+            extra: new { max_age = "0" }
+        );
+
+        var response = await _mockPipeline.BrowserClient.GetAsync(url);
+
+        // this simulates the login page returning to the returnUrl which is the authorize callback page
         _mockPipeline.BrowserClient.AllowAutoRedirect = false;
         response = await _mockPipeline.BrowserClient.GetAsync(IdentityServerPipeline.BaseUrl + _mockPipeline.LoginReturnUrl);
         response.StatusCode.Should().Be(HttpStatusCode.Redirect);