From 4fa5a228cfeb52926b30a2741b99112a64454b36 Mon Sep 17 00:00:00 2001 From: Chris Ross Date: Wed, 12 Jun 2019 11:32:40 -0700 Subject: [PATCH] Add error details for OAuth and OIDC remote errors #4682 (#11002) --- .../Authentication/OAuth/src/OAuthHandler.cs | 23 ++++-- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 6 +- .../Authentication/test/GoogleTests.cs | 57 ++++++++++++++- .../OpenIdConnectAuthenticateTests.cs | 73 +++++++++++++++++++ 4 files changed, 150 insertions(+), 9 deletions(-) diff --git a/src/Security/Authentication/OAuth/src/OAuthHandler.cs b/src/Security/Authentication/OAuth/src/OAuthHandler.cs index 0424020db2..a6bfbbc550 100644 --- a/src/Security/Authentication/OAuth/src/OAuthHandler.cs +++ b/src/Security/Authentication/OAuth/src/OAuthHandler.cs @@ -71,27 +71,40 @@ namespace Microsoft.AspNetCore.Authentication.OAuth // Since it's a frequent scenario (that is not caused by incorrect configuration), // denied errors are handled differently using HandleAccessDeniedErrorAsync(). // Visit https://tools.ietf.org/html/rfc6749#section-4.1.2.1 for more information. + var errorDescription = query["error_description"]; + var errorUri = query["error_uri"]; if (StringValues.Equals(error, "access_denied")) { var result = await HandleAccessDeniedErrorAsync(properties); - return !result.None ? result - : HandleRequestResult.Fail("Access was denied by the resource owner or by the remote server.", properties); + if (!result.None) + { + return result; + } + var deniedEx = new Exception("Access was denied by the resource owner or by the remote server."); + deniedEx.Data["error"] = error.ToString(); + deniedEx.Data["error_description"] = errorDescription.ToString(); + deniedEx.Data["error_uri"] = errorUri.ToString(); + + return HandleRequestResult.Fail(deniedEx, properties); } var failureMessage = new StringBuilder(); failureMessage.Append(error); - var errorDescription = query["error_description"]; if (!StringValues.IsNullOrEmpty(errorDescription)) { failureMessage.Append(";Description=").Append(errorDescription); } - var errorUri = query["error_uri"]; if (!StringValues.IsNullOrEmpty(errorUri)) { failureMessage.Append(";Uri=").Append(errorUri); } - return HandleRequestResult.Fail(failureMessage.ToString(), properties); + var ex = new Exception(failureMessage.ToString()); + ex.Data["error"] = error.ToString(); + ex.Data["error_description"] = errorDescription.ToString(); + ex.Data["error_uri"] = errorUri.ToString(); + + return HandleRequestResult.Fail(ex, properties); } var code = query["code"]; diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index 4a9a259398..65ad366a50 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -1309,12 +1309,16 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect Logger.ResponseError(message.Error, description, errorUri); } - return new OpenIdConnectProtocolException(string.Format( + var ex = new OpenIdConnectProtocolException(string.Format( CultureInfo.InvariantCulture, Resources.MessageContainsError, message.Error, description, errorUri)); + ex.Data["error"] = message.Error; + ex.Data["error_description"] = description; + ex.Data["error_uri"] = errorUri; + return ex; } } } diff --git a/src/Security/Authentication/test/GoogleTests.cs b/src/Security/Authentication/test/GoogleTests.cs index 8d5e635fd0..473f669c65 100644 --- a/src/Security/Authentication/test/GoogleTests.cs +++ b/src/Security/Authentication/test/GoogleTests.cs @@ -420,6 +420,50 @@ namespace Microsoft.AspNetCore.Authentication.Google Assert.Equal("/custom-denied-page?rurl=http%3A%2F%2Fwww.google.com%2F", transaction.Response.Headers.GetValues("Location").First()); } + [Fact] + public async Task ReplyPathWithAccessDeniedErrorAndNoAccessDeniedPath_FallsBackToRemoteError() + { + var accessDeniedCalled = false; + var remoteFailureCalled = false; + var server = CreateServer(o => + { + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + o.StateDataFormat = new TestStateDataFormat(); + o.Events = new OAuthEvents() + { + OnAccessDenied = ctx => + { + Assert.Null(ctx.AccessDeniedPath.Value); + Assert.Equal("http://testhost/redirect", ctx.ReturnUrl); + Assert.Equal("ReturnUrl", ctx.ReturnUrlParameter); + accessDeniedCalled = true; + return Task.FromResult(0); + }, + OnRemoteFailure = ctx => + { + var ex = ctx.Failure; + Assert.True(ex.Data.Contains("error"), "error"); + Assert.True(ex.Data.Contains("error_description"), "error_description"); + Assert.True(ex.Data.Contains("error_uri"), "error_uri"); + Assert.Equal("access_denied", ex.Data["error"]); + Assert.Equal("whyitfailed", ex.Data["error_description"]); + Assert.Equal("https://example.com/fail", ex.Data["error_uri"]); + remoteFailureCalled = true; + ctx.Response.Redirect("/error?FailureMessage=" + UrlEncoder.Default.Encode(ctx.Failure.Message)); + ctx.HandleResponse(); + return Task.FromResult(0); + } + }; + }); + var transaction = await server.SendAsync("https://example.com/signin-google?error=access_denied&error_description=whyitfailed&error_uri=https://example.com/fail&state=protected_state", + ".AspNetCore.Correlation.Google.correlationId=N"); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + Assert.StartsWith("/error?FailureMessage=", transaction.Response.Headers.GetValues("Location").First()); + Assert.True(accessDeniedCalled); + Assert.True(remoteFailureCalled); + } + [Theory] [InlineData(true)] [InlineData(false)] @@ -434,24 +478,31 @@ namespace Microsoft.AspNetCore.Authentication.Google { OnRemoteFailure = ctx => { + var ex = ctx.Failure; + Assert.True(ex.Data.Contains("error"), "error"); + Assert.True(ex.Data.Contains("error_description"), "error_description"); + Assert.True(ex.Data.Contains("error_uri"), "error_uri"); + Assert.Equal("itfailed", ex.Data["error"]); + Assert.Equal("whyitfailed", ex.Data["error_description"]); + Assert.Equal("https://example.com/fail", ex.Data["error_uri"]); ctx.Response.Redirect("/error?FailureMessage=" + UrlEncoder.Default.Encode(ctx.Failure.Message)); ctx.HandleResponse(); return Task.FromResult(0); } } : new OAuthEvents(); }); - var sendTask = server.SendAsync("https://example.com/signin-google?error=OMG&error_description=SoBad&error_uri=foobar&state=protected_state", + var sendTask = server.SendAsync("https://example.com/signin-google?error=itfailed&error_description=whyitfailed&error_uri=https://example.com/fail&state=protected_state", ".AspNetCore.Correlation.Google.correlationId=N"); if (redirect) { var transaction = await sendTask; Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); - Assert.Equal("/error?FailureMessage=OMG" + UrlEncoder.Default.Encode(";Description=SoBad;Uri=foobar"), transaction.Response.Headers.GetValues("Location").First()); + Assert.Equal("/error?FailureMessage=itfailed" + UrlEncoder.Default.Encode(";Description=whyitfailed;Uri=https://example.com/fail"), transaction.Response.Headers.GetValues("Location").First()); } else { var error = await Assert.ThrowsAnyAsync(() => sendTask); - Assert.Equal("OMG;Description=SoBad;Uri=foobar", error.GetBaseException().Message); + Assert.Equal("itfailed;Description=whyitfailed;Uri=https://example.com/fail", error.GetBaseException().Message); } } diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectAuthenticateTests.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectAuthenticateTests.cs index 02b22d4fb8..8502b0670a 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectAuthenticateTests.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectAuthenticateTests.cs @@ -1,9 +1,14 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; +using System.Linq; +using System.Net; using System.Net.Http; +using System.Text.Encodings.Web; using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication.OpenIdConnect; using Microsoft.AspNetCore.Http; using Xunit; @@ -63,5 +68,73 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect // Assert Assert.Equal("Hi from the callback path", transaction.ResponseText); } + + [Fact] + public async Task ErrorResponseWithDetails() + { + var settings = new TestSettings( + opt => + { + opt.StateDataFormat = new TestStateDataFormat(); + opt.Authority = TestServerBuilder.DefaultAuthority; + opt.ClientId = "Test Id"; + opt.Events = new OpenIdConnectEvents() + { + OnRemoteFailure = ctx => + { + var ex = ctx.Failure; + Assert.True(ex.Data.Contains("error"), "error"); + Assert.True(ex.Data.Contains("error_description"), "error_description"); + Assert.True(ex.Data.Contains("error_uri"), "error_uri"); + Assert.Equal("itfailed", ex.Data["error"]); + Assert.Equal("whyitfailed", ex.Data["error_description"]); + Assert.Equal("https://example.com/fail", ex.Data["error_uri"]); + ctx.Response.Redirect("/error?FailureMessage=" + UrlEncoder.Default.Encode(ctx.Failure.Message)); + ctx.HandleResponse(); + return Task.FromResult(0); + } + }; + }); + + var server = settings.CreateTestServer(); + + var transaction = await server.SendAsync( + "https://example.com/signin-oidc?error=itfailed&error_description=whyitfailed&error_uri=https://example.com/fail&state=protected_state", + ".AspNetCore.Correlation.OpenIdConnect.correlationId=N"); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + Assert.StartsWith("/error?FailureMessage=", transaction.Response.Headers.GetValues("Location").First()); + } + + private class TestStateDataFormat : ISecureDataFormat + { + private AuthenticationProperties Data { get; set; } + + public string Protect(AuthenticationProperties data) + { + return "protected_state"; + } + + public string Protect(AuthenticationProperties data, string purpose) + { + throw new NotImplementedException(); + } + + public AuthenticationProperties Unprotect(string protectedText) + { + Assert.Equal("protected_state", protectedText); + var properties = new AuthenticationProperties(new Dictionary() + { + { ".xsrf", "correlationId" }, + { "testkey", "testvalue" } + }); + properties.RedirectUri = "http://testhost/redirect"; + return properties; + } + + public AuthenticationProperties Unprotect(string protectedText, string purpose) + { + throw new NotImplementedException(); + } + } } }