From 7e14b052ea9cb935ec4f5cb0485b4edb5d41297a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Fri, 16 Nov 2018 00:12:42 +0100 Subject: [PATCH] Add AccessDeniedPath support to the OIDC/OAuth2/Twitter providers (#1887) * Add AccessDeniedPath support to the OIDC/OAuth2/Twitter providers * Update the code documentation and remove an unnecessary call to SignOutAsync() * Introduce a new AccessDenied event and move most of the access denied handling logic to RemoteAuthenticationHandler * Add ReturnUrlParameter support to RemoteAuthenticationHandler * Remove AccessDeniedException and introduce RemoteAuthenticationHandler.HandleAccessDeniedErrorAsync() * Use OriginalPath instead of Request.Path * Update obsolete code comments * Add unit tests for the new AccessDenied event * Allow customizing the access denied path/return URL/return URL parameter from the AccessDenied event --- samples/OpenIdConnectSample/Startup.cs | 11 ++ .../CookieAuthenticationHandler.cs | 4 +- .../OAuthHandler.cs | 12 +- .../OAuthOptions.cs | 6 +- .../OpenIdConnectHandler.cs | 14 ++- .../TwitterHandler.cs | 10 +- .../TwitterOptions.cs | 2 +- .../WsFederationHandler.cs | 2 +- .../Events/AccessDeniedContext.cs | 44 ++++++++ .../Events/RemoteAuthenticationEvents.cs | 8 +- .../LoggingExtensions.cs | 52 +++++++-- .../RemoteAuthenticationHandler.cs | 44 ++++++++ .../RemoteAuthenticationOptions.cs | 16 +++ .../GoogleTests.cs | 68 +++++++++++- .../OAuthTests.cs | 103 +++++++++++++++++- .../OpenIdConnect/OpenIdConnectEventTests.cs | 60 +++++++++- .../TwitterTests.cs | 90 ++++++++++++++- 17 files changed, 510 insertions(+), 36 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Authentication/Events/AccessDeniedContext.cs diff --git a/samples/OpenIdConnectSample/Startup.cs b/samples/OpenIdConnectSample/Startup.cs index 1aa7625cb0..a2b5a9a5ca 100644 --- a/samples/OpenIdConnectSample/Startup.cs +++ b/samples/OpenIdConnectSample/Startup.cs @@ -63,6 +63,7 @@ namespace OpenIdConnectSample o.ResponseType = OpenIdConnectResponseType.CodeIdToken; o.SaveTokens = true; o.GetClaimsFromUserInfoEndpoint = true; + o.AccessDeniedPath = "/access-denied-from-remote"; o.ClaimActions.MapAllExcept("aud", "iss", "iat", "nbf", "exp", "aio", "c_hash", "uti", "nonce"); @@ -126,6 +127,16 @@ namespace OpenIdConnectSample return; } + if (context.Request.Path.Equals("/access-denied-from-remote")) + { + await WriteHtmlAsync(response, async res => + { + await res.WriteAsync($"

Access Denied error received from the remote authorization server

"); + await res.WriteAsync("Home"); + }); + return; + } + if (context.Request.Path.Equals("/Account/AccessDenied")) { await context.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme); diff --git a/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationHandler.cs b/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationHandler.cs index b77a51ef4f..ff6bb3cbca 100644 --- a/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationHandler.cs @@ -422,7 +422,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies var returnUrl = properties.RedirectUri; if (string.IsNullOrEmpty(returnUrl)) { - returnUrl = OriginalPathBase + Request.Path + Request.QueryString; + returnUrl = OriginalPathBase + OriginalPath + Request.QueryString; } var accessDeniedUri = Options.AccessDeniedPath + QueryString.Create(Options.ReturnUrlParameter, returnUrl); var redirectContext = new RedirectContext(Context, Scheme, Options, properties, BuildRedirectUri(accessDeniedUri)); @@ -434,7 +434,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies var redirectUri = properties.RedirectUri; if (string.IsNullOrEmpty(redirectUri)) { - redirectUri = OriginalPathBase + Request.Path + Request.QueryString; + redirectUri = OriginalPathBase + OriginalPath + Request.QueryString; } var loginUri = Options.LoginPath + QueryString.Create(Options.ReturnUrlParameter, redirectUri); diff --git a/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthHandler.cs b/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthHandler.cs index 808e0f9039..05ba4df68d 100644 --- a/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthHandler.cs @@ -63,6 +63,16 @@ namespace Microsoft.AspNetCore.Authentication.OAuth var error = query["error"]; if (!StringValues.IsNullOrEmpty(error)) { + // Note: access_denied errors are special protocol errors indicating the user didn't + // approve the authorization demand requested by the remote authorization server. + // 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. + if (StringValues.Equals(error, "access_denied")) + { + return await HandleAccessDeniedErrorAsync(properties); + } + var failureMessage = new StringBuilder(); failureMessage.Append(error); var errorDescription = query["error_description"]; @@ -194,7 +204,7 @@ namespace Microsoft.AspNetCore.Authentication.OAuth { if (string.IsNullOrEmpty(properties.RedirectUri)) { - properties.RedirectUri = CurrentUri; + properties.RedirectUri = OriginalPathBase + OriginalPath + Request.QueryString; } // OAuth2 10.12 CSRF diff --git a/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthOptions.cs b/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthOptions.cs index 3c71f055f5..4eacbd0b30 100644 --- a/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthOptions.cs +++ b/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthOptions.cs @@ -3,11 +3,9 @@ using System; using System.Collections.Generic; -using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Authentication.OAuth; -using Microsoft.AspNetCore.Authentication.OAuth.Claims; -using Microsoft.AspNetCore.Http.Authentication; using System.Globalization; +using Microsoft.AspNetCore.Authentication.OAuth.Claims; +using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Authentication.OAuth { diff --git a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs index 029cf541b7..8b0b72428a 100644 --- a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs @@ -186,7 +186,7 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect properties.RedirectUri = BuildRedirectUriIfRelative(Options.SignedOutRedirectUri); if (string.IsNullOrWhiteSpace(properties.RedirectUri)) { - properties.RedirectUri = CurrentUri; + properties.RedirectUri = OriginalPathBase + OriginalPath + Request.QueryString; } } Logger.PostSignOutRedirect(properties.RedirectUri); @@ -312,7 +312,7 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect // 2. CurrentUri if RedirectUri is not set) if (string.IsNullOrEmpty(properties.RedirectUri)) { - properties.RedirectUri = CurrentUri; + properties.RedirectUri = OriginalPathBase + OriginalPath + Request.QueryString; } Logger.PostAuthenticationLocalRedirect(properties.RedirectUri); @@ -520,6 +520,16 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect // if any of the error fields are set, throw error null if (!string.IsNullOrEmpty(authorizationResponse.Error)) { + // Note: access_denied errors are special protocol errors indicating the user didn't + // approve the authorization demand requested by the remote authorization server. + // 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. + if (string.Equals(authorizationResponse.Error, "access_denied", StringComparison.Ordinal)) + { + return await HandleAccessDeniedErrorAsync(properties); + } + return HandleRequestResult.Fail(CreateOpenIdConnectProtocolException(authorizationResponse, response: null), properties); } diff --git a/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs b/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs index 51dabbd99c..cc8591ed32 100644 --- a/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs @@ -55,12 +55,14 @@ namespace Microsoft.AspNetCore.Authentication.Twitter var properties = requestToken.Properties; - // REVIEW: see which of these are really errors - var denied = query["denied"]; if (!StringValues.IsNullOrEmpty(denied)) { - return HandleRequestResult.Fail("The user denied permissions.", properties); + // Note: denied errors are special protocol errors indicating the user didn't + // approve the authorization demand requested by the remote authorization server. + // Since it's a frequent scenario (that is not caused by incorrect configuration), + // denied errors are handled differently using HandleAccessDeniedErrorAsync(). + return await HandleAccessDeniedErrorAsync(properties); } var returnedToken = query["oauth_token"]; @@ -130,7 +132,7 @@ namespace Microsoft.AspNetCore.Authentication.Twitter { if (string.IsNullOrEmpty(properties.RedirectUri)) { - properties.RedirectUri = CurrentUri; + properties.RedirectUri = OriginalPathBase + OriginalPath + Request.QueryString; } // If CallbackConfirmed is false, this will throw diff --git a/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterOptions.cs b/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterOptions.cs index 03396807ee..269516ae1a 100644 --- a/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterOptions.cs +++ b/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterOptions.cs @@ -2,8 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Security.Claims; using System.Globalization; +using System.Security.Claims; using Microsoft.AspNetCore.Authentication.OAuth.Claims; using Microsoft.AspNetCore.Http; diff --git a/src/Microsoft.AspNetCore.Authentication.WsFederation/WsFederationHandler.cs b/src/Microsoft.AspNetCore.Authentication.WsFederation/WsFederationHandler.cs index e47f8431f9..c4bb8fff74 100644 --- a/src/Microsoft.AspNetCore.Authentication.WsFederation/WsFederationHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.WsFederation/WsFederationHandler.cs @@ -83,7 +83,7 @@ namespace Microsoft.AspNetCore.Authentication.WsFederation // Save the original challenge URI so we can redirect back to it when we're done. if (string.IsNullOrEmpty(properties.RedirectUri)) { - properties.RedirectUri = CurrentUri; + properties.RedirectUri = OriginalPathBase + OriginalPath + Request.QueryString; } var wsFederationMessage = new WsFederationMessage() diff --git a/src/Microsoft.AspNetCore.Authentication/Events/AccessDeniedContext.cs b/src/Microsoft.AspNetCore.Authentication/Events/AccessDeniedContext.cs new file mode 100644 index 0000000000..f01d69453b --- /dev/null +++ b/src/Microsoft.AspNetCore.Authentication/Events/AccessDeniedContext.cs @@ -0,0 +1,44 @@ +// 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 Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Authentication +{ + /// + /// Provides access denied failure context information to handler providers. + /// + public class AccessDeniedContext : HandleRequestContext + { + public AccessDeniedContext( + HttpContext context, + AuthenticationScheme scheme, + RemoteAuthenticationOptions options) + : base(context, scheme, options) + { + } + + /// + /// Gets or sets the endpoint path the user agent will be redirected to. + /// By default, this property is set to . + /// + public PathString AccessDeniedPath { get; set; } + + /// + /// Additional state values for the authentication session. + /// + public AuthenticationProperties Properties { get; set; } + + /// + /// Gets or sets the return URL that will be flowed up to the access denied page. + /// If is not set, this property is not used. + /// + public string ReturnUrl { get; set; } + + /// + /// Gets or sets the parameter name that will be used to flow the return URL. + /// By default, this property is set to . + /// + public string ReturnUrlParameter { get; set; } + } +} diff --git a/src/Microsoft.AspNetCore.Authentication/Events/RemoteAuthenticationEvents.cs b/src/Microsoft.AspNetCore.Authentication/Events/RemoteAuthenticationEvents.cs index ca0f4a5c01..be4c6ab49e 100644 --- a/src/Microsoft.AspNetCore.Authentication/Events/RemoteAuthenticationEvents.cs +++ b/src/Microsoft.AspNetCore.Authentication/Events/RemoteAuthenticationEvents.cs @@ -8,12 +8,18 @@ namespace Microsoft.AspNetCore.Authentication { public class RemoteAuthenticationEvents { + public Func OnAccessDenied { get; set; } = context => Task.CompletedTask; public Func OnRemoteFailure { get; set; } = context => Task.CompletedTask; public Func OnTicketReceived { get; set; } = context => Task.CompletedTask; /// - /// Invoked when there is a remote failure + /// Invoked when an access denied error was returned by the remote server. + /// + public virtual Task AccessDenied(AccessDeniedContext context) => OnAccessDenied(context); + + /// + /// Invoked when there is a remote failure. /// public virtual Task RemoteFailure(RemoteFailureContext context) => OnRemoteFailure(context); diff --git a/src/Microsoft.AspNetCore.Authentication/LoggingExtensions.cs b/src/Microsoft.AspNetCore.Authentication/LoggingExtensions.cs index 8cba6c0d5e..042bfabca3 100644 --- a/src/Microsoft.AspNetCore.Authentication/LoggingExtensions.cs +++ b/src/Microsoft.AspNetCore.Authentication/LoggingExtensions.cs @@ -7,17 +7,20 @@ namespace Microsoft.Extensions.Logging { internal static class LoggingExtensions { - private static Action _authSchemeAuthenticated; - private static Action _authSchemeNotAuthenticated; - private static Action _authSchemeNotAuthenticatedWithFailure; - private static Action _authSchemeChallenged; - private static Action _authSchemeForbidden; - private static Action _remoteAuthenticationError; - private static Action _signInHandled; - private static Action _signInSkipped; - private static Action _correlationPropertyNotFound; - private static Action _correlationCookieNotFound; - private static Action _unexpectedCorrelationCookieValue; + private static readonly Action _authSchemeAuthenticated; + private static readonly Action _authSchemeNotAuthenticated; + private static readonly Action _authSchemeNotAuthenticatedWithFailure; + private static readonly Action _authSchemeChallenged; + private static readonly Action _authSchemeForbidden; + private static readonly Action _remoteAuthenticationError; + private static readonly Action _signInHandled; + private static readonly Action _signInSkipped; + private static readonly Action _correlationPropertyNotFound; + private static readonly Action _correlationCookieNotFound; + private static readonly Action _unexpectedCorrelationCookieValue; + private static readonly Action _accessDeniedError; + private static readonly Action _accessDeniedContextHandled; + private static readonly Action _accessDeniedContextSkipped; static LoggingExtensions() { @@ -65,6 +68,18 @@ namespace Microsoft.Extensions.Logging eventId: 16, logLevel: LogLevel.Warning, formatString: "The correlation cookie value '{CorrelationCookieName}' did not match the expected value '{CorrelationCookieValue}'."); + _accessDeniedError = LoggerMessage.Define( + eventId: 17, + logLevel: LogLevel.Information, + formatString: "Access was denied by the resource owner or by the remote server."); + _accessDeniedContextHandled = LoggerMessage.Define( + eventId: 18, + logLevel: LogLevel.Debug, + formatString: "The AccessDenied event returned Handled."); + _accessDeniedContextSkipped = LoggerMessage.Define( + eventId: 19, + logLevel: LogLevel.Debug, + formatString: "The AccessDenied event returned Skipped."); } public static void AuthenticationSchemeAuthenticated(this ILogger logger, string authenticationScheme) @@ -121,5 +136,20 @@ namespace Microsoft.Extensions.Logging { _unexpectedCorrelationCookieValue(logger, cookieName, cookieValue, null); } + + public static void AccessDeniedError(this ILogger logger) + { + _accessDeniedError(logger, null); + } + + public static void AccessDeniedContextHandled(this ILogger logger) + { + _accessDeniedContextHandled(logger, null); + } + + public static void AccessDeniedContextSkipped(this ILogger logger) + { + _accessDeniedContextSkipped(logger, null); + } } } diff --git a/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs b/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs index bea4895d62..c72583b5ce 100644 --- a/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs @@ -5,6 +5,7 @@ using System; using System.Security.Cryptography; using System.Text.Encodings.Web; using System.Threading.Tasks; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -241,5 +242,48 @@ namespace Microsoft.AspNetCore.Authentication return true; } + + protected virtual async Task HandleAccessDeniedErrorAsync(AuthenticationProperties properties) + { + Logger.AccessDeniedError(); + var context = new AccessDeniedContext(Context, Scheme, Options) + { + AccessDeniedPath = Options.AccessDeniedPath, + Properties = properties, + ReturnUrl = properties?.RedirectUri, + ReturnUrlParameter = Options.ReturnUrlParameter + }; + await Events.AccessDenied(context); + + if (context.Result != null) + { + if (context.Result.Handled) + { + Logger.AccessDeniedContextHandled(); + } + else if (context.Result.Skipped) + { + Logger.AccessDeniedContextSkipped(); + } + + return context.Result; + } + + // If an access denied endpoint was specified, redirect the user agent. + // Otherwise, invoke the RemoteFailure event for further processing. + if (context.AccessDeniedPath.HasValue) + { + string uri = context.AccessDeniedPath; + if (!string.IsNullOrEmpty(context.ReturnUrlParameter) && !string.IsNullOrEmpty(context.ReturnUrl)) + { + uri = QueryHelpers.AddQueryString(uri, context.ReturnUrlParameter, context.ReturnUrl); + } + Response.Redirect(uri); + + return HandleRequestResult.Handle(); + } + + return HandleRequestResult.Fail("Access was denied by the resource owner or by the remote server.", properties); + } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs b/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs index 1bd3b210e5..188b7a9917 100644 --- a/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs +++ b/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs @@ -89,6 +89,22 @@ namespace Microsoft.AspNetCore.Authentication /// public PathString CallbackPath { get; set; } + /// + /// Gets or sets the optional path the user agent is redirected to if the user + /// doesn't approve the authorization demand requested by the remote server. + /// This property is not set by default. In this case, an exception is thrown + /// if an access_denied response is returned by the remote authorization server. + /// + public PathString AccessDeniedPath { get; set; } + + /// + /// Gets or sets the name of the parameter used to convey the original location + /// of the user before the remote challenge was triggered up to the access denied page. + /// This property is only used when the is explicitly specified. + /// + // Note: this deliberately matches the default parameter name used by the cookie handler. + public string ReturnUrlParameter { get; set; } = "ReturnUrl"; + /// /// Gets or sets the authentication scheme corresponding to the middleware /// responsible of persisting user's identity after a successful authentication. diff --git a/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs index 19d7d898b7..ce158eaf20 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs @@ -357,6 +357,70 @@ namespace Microsoft.AspNetCore.Authentication.Google Assert.Equal("The oauth state was missing or invalid.", error.GetBaseException().Message); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ReplyPathWithAccessDeniedErrorFails(bool redirect) + { + var server = CreateServer(o => + { + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + o.StateDataFormat = new TestStateDataFormat(); + o.Events = redirect ? new OAuthEvents() + { + OnAccessDenied = ctx => + { + ctx.Response.Redirect("/error?FailureMessage=AccessDenied"); + ctx.HandleResponse(); + return Task.FromResult(0); + } + } : new OAuthEvents(); + }); + var sendTask = server.SendAsync("https://example.com/signin-google?error=access_denied&error_description=SoBad&error_uri=foobar&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=AccessDenied", transaction.Response.Headers.GetValues("Location").First()); + } + else + { + var error = await Assert.ThrowsAnyAsync(() => sendTask); + Assert.Equal("Access was denied by the resource owner or by the remote server.", error.GetBaseException().Message); + } + } + + [Fact] + public async Task ReplyPathWithAccessDeniedError_AllowsCustomizingPath() + { + var server = CreateServer(o => + { + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + o.StateDataFormat = new TestStateDataFormat(); + o.AccessDeniedPath = "/access-denied"; + o.Events = new OAuthEvents() + { + OnAccessDenied = ctx => + { + Assert.Equal("/access-denied", ctx.AccessDeniedPath.Value); + Assert.Equal("http://testhost/redirect", ctx.ReturnUrl); + Assert.Equal("ReturnUrl", ctx.ReturnUrlParameter); + ctx.AccessDeniedPath = "/custom-denied-page"; + ctx.ReturnUrl = "http://www.google.com/"; + ctx.ReturnUrlParameter = "rurl"; + return Task.FromResult(0); + } + }; + }); + var transaction = await server.SendAsync("https://example.com/signin-google?error=access_denied&error_description=SoBad&error_uri=foobar&state=protected_state", + ".AspNetCore.Correlation.Google.correlationId=N"); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + Assert.Equal("/custom-denied-page?rurl=http%3A%2F%2Fwww.google.com%2F", transaction.Response.Headers.GetValues("Location").First()); + } + [Theory] [InlineData(true)] [InlineData(false)] @@ -378,7 +442,7 @@ namespace Microsoft.AspNetCore.Authentication.Google } : new OAuthEvents(); }); var sendTask = server.SendAsync("https://example.com/signin-google?error=OMG&error_description=SoBad&error_uri=foobar&state=protected_state", - ".AspNetCore.Correlation.Google.corrilationId=N"); + ".AspNetCore.Correlation.Google.correlationId=N"); if (redirect) { var transaction = await sendTask; @@ -1205,7 +1269,7 @@ namespace Microsoft.AspNetCore.Authentication.Google Assert.Equal("protected_state", protectedText); var properties = new AuthenticationProperties(new Dictionary() { - { ".xsrf", "corrilationId" }, + { ".xsrf", "correlationId" }, { "testkey", "testvalue" } }); properties.RedirectUri = "http://testhost/redirect"; diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs index 87131b8d7b..838798ceaf 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs @@ -251,6 +251,101 @@ namespace Microsoft.AspNetCore.Authentication.OAuth o.CallbackPath = "/oauth-callback"; } + [Fact] + public async Task HandleRequestAsync_RedirectsToAccessDeniedPathWhenExplicitlySet() + { + var server = CreateServer( + s => s.AddAuthentication().AddOAuth( + "Weblie", + opt => + { + opt.ClientId = "Test Id"; + opt.ClientSecret = "secret"; + opt.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme; + opt.AuthorizationEndpoint = "https://example.com/provider/login"; + opt.TokenEndpoint = "https://example.com/provider/token"; + opt.CallbackPath = "/oauth-callback"; + opt.AccessDeniedPath = "/access-denied"; + opt.StateDataFormat = new TestStateDataFormat(); + opt.Events.OnRemoteFailure = context => throw new InvalidOperationException("This event should not be called."); + })); + + var transaction = await server.SendAsync("https://www.example.com/oauth-callback?error=access_denied&state=protected_state", + ".AspNetCore.Correlation.Weblie.correlationId=N"); + + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + Assert.Equal("/access-denied?ReturnUrl=http%3A%2F%2Ftesthost%2Fredirect", transaction.Response.Headers.Location.ToString()); + } + + [Fact] + public async Task HandleRequestAsync_InvokesAccessDeniedEvent() + { + var server = CreateServer( + s => s.AddAuthentication().AddOAuth( + "Weblie", + opt => + { + opt.ClientId = "Test Id"; + opt.ClientSecret = "secret"; + opt.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme; + opt.AuthorizationEndpoint = "https://example.com/provider/login"; + opt.TokenEndpoint = "https://example.com/provider/token"; + opt.CallbackPath = "/oauth-callback"; + opt.StateDataFormat = new TestStateDataFormat(); + opt.Events = new OAuthEvents() + { + OnAccessDenied = context => + { + Assert.Equal("testvalue", context.Properties.Items["testkey"]); + context.Response.StatusCode = StatusCodes.Status406NotAcceptable; + context.HandleResponse(); + return Task.CompletedTask; + } + }; + })); + + var transaction = await server.SendAsync("https://www.example.com/oauth-callback?error=access_denied&state=protected_state", + ".AspNetCore.Correlation.Weblie.correlationId=N"); + + Assert.Equal(HttpStatusCode.NotAcceptable, transaction.Response.StatusCode); + Assert.Null(transaction.Response.Headers.Location); + } + + [Fact] + public async Task HandleRequestAsync_InvokesRemoteFailureEventWhenAccessDeniedPathIsNotExplicitlySet() + { + var server = CreateServer( + s => s.AddAuthentication().AddOAuth( + "Weblie", + opt => + { + opt.ClientId = "Test Id"; + opt.ClientSecret = "secret"; + opt.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme; + opt.AuthorizationEndpoint = "https://example.com/provider/login"; + opt.TokenEndpoint = "https://example.com/provider/token"; + opt.CallbackPath = "/oauth-callback"; + opt.StateDataFormat = new TestStateDataFormat(); + opt.Events = new OAuthEvents() + { + OnRemoteFailure = context => + { + Assert.Equal("Access was denied by the resource owner or by the remote server.", context.Failure.Message); + Assert.Equal("testvalue", context.Properties.Items["testkey"]); + context.Response.StatusCode = StatusCodes.Status406NotAcceptable; + context.HandleResponse(); + return Task.CompletedTask; + } + }; + })); + + var transaction = await server.SendAsync("https://www.example.com/oauth-callback?error=access_denied&state=protected_state", + ".AspNetCore.Correlation.Weblie.correlationId=N"); + + Assert.Equal(HttpStatusCode.NotAcceptable, transaction.Response.StatusCode); + Assert.Null(transaction.Response.Headers.Location); + } + [Fact] public async Task RemoteAuthenticationFailed_OAuthError_IncludesProperties() { @@ -270,7 +365,7 @@ namespace Microsoft.AspNetCore.Authentication.OAuth { OnRemoteFailure = context => { - Assert.Contains("declined", context.Failure.Message); + Assert.Contains("custom_error", context.Failure.Message); Assert.Equal("testvalue", context.Properties.Items["testkey"]); context.Response.StatusCode = StatusCodes.Status406NotAcceptable; context.HandleResponse(); @@ -279,8 +374,8 @@ namespace Microsoft.AspNetCore.Authentication.OAuth }; })); - var transaction = await server.SendAsync("https://www.example.com/oauth-callback?error=declined&state=protected_state", - ".AspNetCore.Correlation.Weblie.corrilationId=N"); + var transaction = await server.SendAsync("https://www.example.com/oauth-callback?error=custom_error&state=protected_state", + ".AspNetCore.Correlation.Weblie.correlationId=N"); Assert.Equal(HttpStatusCode.NotAcceptable, transaction.Response.StatusCode); Assert.Null(transaction.Response.Headers.Location); @@ -323,7 +418,7 @@ namespace Microsoft.AspNetCore.Authentication.OAuth Assert.Equal("protected_state", protectedText); var properties = new AuthenticationProperties(new Dictionary() { - { ".xsrf", "corrilationId" }, + { ".xsrf", "correlationId" }, { "testkey", "testvalue" } }); properties.RedirectUri = "http://testhost/redirect"; diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectEventTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectEventTests.cs index 7530b00c31..090bf3dec4 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectEventTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectEventTests.cs @@ -783,6 +783,52 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect events.ValidateExpectations(); } + [Fact] + public async Task OnAccessDenied_Skip_NoMoreEventsRun() + { + var events = new ExpectedOidcEvents() + { + ExpectMessageReceived = true, + ExpectAccessDenied = true + }; + events.OnAccessDenied = context => + { + context.SkipHandler(); + return Task.FromResult(0); + }; + var server = CreateServer(events, AppWritePath); + + var response = await PostAsync(server, "signin-oidc", "error=access_denied&state=protected_state"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("/signin-oidc", await response.Content.ReadAsStringAsync()); + events.ValidateExpectations(); + } + + [Fact] + public async Task OnAccessDenied_Handled_NoMoreEventsRun() + { + var events = new ExpectedOidcEvents() + { + ExpectMessageReceived = true, + ExpectAccessDenied = true + }; + events.OnAccessDenied = context => + { + Assert.Equal("testvalue", context.Properties.Items["testkey"]); + context.HandleResponse(); + context.Response.StatusCode = StatusCodes.Status202Accepted; + return Task.FromResult(0); + }; + var server = CreateServer(events, AppNotImpl); + + var response = await PostAsync(server, "signin-oidc", "error=access_denied&state=protected_state"); + + Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); + Assert.Equal("", await response.Content.ReadAsStringAsync()); + events.ValidateExpectations(); + } + [Fact] public async Task OnRemoteFailure_Skip_NoMoreEventsRun() { @@ -1099,6 +1145,9 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect public bool ExpectTokenValidated { get; set; } public bool InvokedTokenValidated { get; set; } + public bool ExpectAccessDenied { get; set; } + public bool InvokedAccessDenied { get; set; } + public bool ExpectRemoteFailure { get; set; } public bool InvokedRemoteFailure { get; set; } @@ -1168,6 +1217,12 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect return base.TicketReceived(context); } + public override Task AccessDenied(AccessDeniedContext context) + { + InvokedAccessDenied = true; + return base.AccessDenied(context); + } + public override Task RemoteFailure(RemoteFailureContext context) { InvokedRemoteFailure = true; @@ -1201,6 +1256,7 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect Assert.Equal(ExpectUserInfoReceived, InvokedUserInfoReceived); Assert.Equal(ExpectAuthenticationFailed, InvokeAuthenticationFailed); Assert.Equal(ExpectTicketReceived, InvokedTicketReceived); + Assert.Equal(ExpectAccessDenied, InvokedAccessDenied); Assert.Equal(ExpectRemoteFailure, InvokedRemoteFailure); Assert.Equal(ExpectRedirectForSignOut, InvokedRedirectForSignOut); Assert.Equal(ExpectRemoteSignOut, InvokedRemoteSignOut); @@ -1248,7 +1304,7 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect private Task PostAsync(TestServer server, string path, string form) { var client = server.CreateClient(); - var cookie = ".AspNetCore.Correlation." + OpenIdConnectDefaults.AuthenticationScheme + ".corrilationId=N"; + var cookie = ".AspNetCore.Correlation." + OpenIdConnectDefaults.AuthenticationScheme + ".correlationId=N"; client.DefaultRequestHeaders.Add("Cookie", cookie); return client.PostAsync("signin-oidc", new StringContent(form, Encoding.ASCII, "application/x-www-form-urlencoded")); @@ -1273,7 +1329,7 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect Assert.Equal("protected_state", protectedText); var properties = new AuthenticationProperties(new Dictionary() { - { ".xsrf", "corrilationId" }, + { ".xsrf", "correlationId" }, { OpenIdConnectDefaults.RedirectUriForCodePropertiesKey, "redirect_uri" }, { "testkey", "testvalue" } }); diff --git a/test/Microsoft.AspNetCore.Authentication.Test/TwitterTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/TwitterTests.cs index c438b1f3f7..8eb7a5cd7d 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/TwitterTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/TwitterTests.cs @@ -174,6 +174,94 @@ namespace Microsoft.AspNetCore.Authentication.Twitter Assert.Contains("https://api.twitter.com/oauth/authenticate?oauth_token=", location); } + [Fact] + public async Task HandleRequestAsync_RedirectsToAccessDeniedPathWhenExplicitlySet() + { + var server = CreateServer(o => + { + o.ConsumerKey = "Test Consumer Key"; + o.ConsumerSecret = "Test Consumer Secret"; + o.BackchannelHttpHandler = new TestHttpMessageHandler + { + Sender = BackchannelRequestToken + }; + o.AccessDeniedPath = "/access-denied"; + o.Events.OnRemoteFailure = context => throw new InvalidOperationException("This event should not be called."); + }, + async context => + { + var properties = new AuthenticationProperties(); + properties.Items["testkey"] = "testvalue"; + await context.ChallengeAsync("Twitter", properties); + return true; + }); + var transaction = await server.SendAsync("http://example.com/challenge"); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + var location = transaction.Response.Headers.Location.AbsoluteUri; + Assert.Contains("https://api.twitter.com/oauth/authenticate?oauth_token=", location); + Assert.True(transaction.Response.Headers.TryGetValues(HeaderNames.SetCookie, out var setCookie)); + Assert.True(SetCookieHeaderValue.TryParseList(setCookie.ToList(), out var setCookieValues)); + Assert.Single(setCookieValues); + var setCookieValue = setCookieValues.Single(); + var cookie = new CookieHeaderValue(setCookieValue.Name, setCookieValue.Value); + + var request = new HttpRequestMessage(HttpMethod.Get, "/signin-twitter?denied=ABCDEFG"); + request.Headers.Add(HeaderNames.Cookie, cookie.ToString()); + var client = server.CreateClient(); + var response = await client.SendAsync(request); + + Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); + Assert.Equal("/access-denied?ReturnUrl=%2Fchallenge", response.Headers.Location.ToString()); + } + + [Fact] + public async Task BadCallbackCallsAccessDeniedWithState() + { + var server = CreateServer(o => + { + o.ConsumerKey = "Test Consumer Key"; + o.ConsumerSecret = "Test Consumer Secret"; + o.BackchannelHttpHandler = new TestHttpMessageHandler + { + Sender = BackchannelRequestToken + }; + o.Events = new TwitterEvents() + { + OnAccessDenied = context => + { + Assert.NotNull(context.Properties); + Assert.Equal("testvalue", context.Properties.Items["testkey"]); + context.Response.StatusCode = StatusCodes.Status406NotAcceptable; + context.HandleResponse(); + return Task.CompletedTask; + } + }; + }, + async context => + { + var properties = new AuthenticationProperties(); + properties.Items["testkey"] = "testvalue"; + await context.ChallengeAsync("Twitter", properties); + return true; + }); + var transaction = await server.SendAsync("http://example.com/challenge"); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + var location = transaction.Response.Headers.Location.AbsoluteUri; + Assert.Contains("https://api.twitter.com/oauth/authenticate?oauth_token=", location); + Assert.True(transaction.Response.Headers.TryGetValues(HeaderNames.SetCookie, out var setCookie)); + Assert.True(SetCookieHeaderValue.TryParseList(setCookie.ToList(), out var setCookieValues)); + Assert.Single(setCookieValues); + var setCookieValue = setCookieValues.Single(); + var cookie = new CookieHeaderValue(setCookieValue.Name, setCookieValue.Value); + + var request = new HttpRequestMessage(HttpMethod.Get, "/signin-twitter?denied=ABCDEFG"); + request.Headers.Add(HeaderNames.Cookie, cookie.ToString()); + var client = server.CreateClient(); + var response = await client.SendAsync(request); + + Assert.Equal(HttpStatusCode.NotAcceptable, response.StatusCode); + } + [Fact] public async Task BadCallbackCallsRemoteAuthFailedWithState() { @@ -190,7 +278,7 @@ namespace Microsoft.AspNetCore.Authentication.Twitter OnRemoteFailure = context => { Assert.NotNull(context.Failure); - Assert.Equal("The user denied permissions.", context.Failure.Message); + Assert.Equal("Access was denied by the resource owner or by the remote server.", context.Failure.Message); Assert.NotNull(context.Properties); Assert.Equal("testvalue", context.Properties.Items["testkey"]); context.Response.StatusCode = StatusCodes.Status406NotAcceptable;