Skip to content

Commit 7e14b05

Browse files
kevinchaletTratcher
kevinchalet
authored and
Tratcher
committed
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
1 parent 1c4a695 commit 7e14b05

File tree

17 files changed

+510
-36
lines changed

17 files changed

+510
-36
lines changed

‎samples/OpenIdConnectSample/Startup.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ public void ConfigureServices(IServiceCollection services)
6363
o.ResponseType = OpenIdConnectResponseType.CodeIdToken;
6464
o.SaveTokens = true;
6565
o.GetClaimsFromUserInfoEndpoint = true;
66+
o.AccessDeniedPath = "/access-denied-from-remote";
6667

6768
o.ClaimActions.MapAllExcept("aud", "iss", "iat", "nbf", "exp", "aio", "c_hash", "uti", "nonce");
6869

@@ -126,6 +127,16 @@ await WriteHtmlAsync(response, async res =>
126127
return;
127128
}
128129

130+
if (context.Request.Path.Equals("/access-denied-from-remote"))
131+
{
132+
await WriteHtmlAsync(response, async res =>
133+
{
134+
await res.WriteAsync($"<h1>Access Denied error received from the remote authorization server</h1>");
135+
await res.WriteAsync("<a class=\"btn btn-default\" href=\"/\">Home</a>");
136+
});
137+
return;
138+
}
139+
129140
if (context.Request.Path.Equals("/Account/AccessDenied"))
130141
{
131142
await context.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);

‎src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationHandler.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ protected override async Task HandleForbiddenAsync(AuthenticationProperties prop
422422
var returnUrl = properties.RedirectUri;
423423
if (string.IsNullOrEmpty(returnUrl))
424424
{
425-
returnUrl = OriginalPathBase + Request.Path + Request.QueryString;
425+
returnUrl = OriginalPathBase + OriginalPath + Request.QueryString;
426426
}
427427
var accessDeniedUri = Options.AccessDeniedPath + QueryString.Create(Options.ReturnUrlParameter, returnUrl);
428428
var redirectContext = new RedirectContext<CookieAuthenticationOptions>(Context, Scheme, Options, properties, BuildRedirectUri(accessDeniedUri));
@@ -434,7 +434,7 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
434434
var redirectUri = properties.RedirectUri;
435435
if (string.IsNullOrEmpty(redirectUri))
436436
{
437-
redirectUri = OriginalPathBase + Request.Path + Request.QueryString;
437+
redirectUri = OriginalPathBase + OriginalPath + Request.QueryString;
438438
}
439439

440440
var loginUri = Options.LoginPath + QueryString.Create(Options.ReturnUrlParameter, redirectUri);

‎src/Microsoft.AspNetCore.Authentication.OAuth/OAuthHandler.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
6363
var error = query["error"];
6464
if (!StringValues.IsNullOrEmpty(error))
6565
{
66+
// Note: access_denied errors are special protocol errors indicating the user didn't
67+
// approve the authorization demand requested by the remote authorization server.
68+
// Since it's a frequent scenario (that is not caused by incorrect configuration),
69+
// denied errors are handled differently using HandleAccessDeniedErrorAsync().
70+
// Visit https://tools.ietf.org/html/rfc6749#section-4.1.2.1 for more information.
71+
if (StringValues.Equals(error, "access_denied"))
72+
{
73+
return await HandleAccessDeniedErrorAsync(properties);
74+
}
75+
6676
var failureMessage = new StringBuilder();
6777
failureMessage.Append(error);
6878
var errorDescription = query["error_description"];
@@ -194,7 +204,7 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
194204
{
195205
if (string.IsNullOrEmpty(properties.RedirectUri))
196206
{
197-
properties.RedirectUri = CurrentUri;
207+
properties.RedirectUri = OriginalPathBase + OriginalPath + Request.QueryString;
198208
}
199209

200210
// OAuth2 10.12 CSRF

‎src/Microsoft.AspNetCore.Authentication.OAuth/OAuthOptions.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,9 @@
33

44
using System;
55
using System.Collections.Generic;
6-
using Microsoft.AspNetCore.Authentication;
7-
using Microsoft.AspNetCore.Authentication.OAuth;
8-
using Microsoft.AspNetCore.Authentication.OAuth.Claims;
9-
using Microsoft.AspNetCore.Http.Authentication;
106
using System.Globalization;
7+
using Microsoft.AspNetCore.Authentication.OAuth.Claims;
8+
using Microsoft.AspNetCore.Http;
119

1210
namespace Microsoft.AspNetCore.Authentication.OAuth
1311
{

‎src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ public async virtual Task SignOutAsync(AuthenticationProperties properties)
186186
properties.RedirectUri = BuildRedirectUriIfRelative(Options.SignedOutRedirectUri);
187187
if (string.IsNullOrWhiteSpace(properties.RedirectUri))
188188
{
189-
properties.RedirectUri = CurrentUri;
189+
properties.RedirectUri = OriginalPathBase + OriginalPath + Request.QueryString;
190190
}
191191
}
192192
Logger.PostSignOutRedirect(properties.RedirectUri);
@@ -312,7 +312,7 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
312312
// 2. CurrentUri if RedirectUri is not set)
313313
if (string.IsNullOrEmpty(properties.RedirectUri))
314314
{
315-
properties.RedirectUri = CurrentUri;
315+
properties.RedirectUri = OriginalPathBase + OriginalPath + Request.QueryString;
316316
}
317317
Logger.PostAuthenticationLocalRedirect(properties.RedirectUri);
318318

@@ -520,6 +520,16 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
520520
// if any of the error fields are set, throw error null
521521
if (!string.IsNullOrEmpty(authorizationResponse.Error))
522522
{
523+
// Note: access_denied errors are special protocol errors indicating the user didn't
524+
// approve the authorization demand requested by the remote authorization server.
525+
// Since it's a frequent scenario (that is not caused by incorrect configuration),
526+
// denied errors are handled differently using HandleAccessDeniedErrorAsync().
527+
// Visit https://tools.ietf.org/html/rfc6749#section-4.1.2.1 for more information.
528+
if (string.Equals(authorizationResponse.Error, "access_denied", StringComparison.Ordinal))
529+
{
530+
return await HandleAccessDeniedErrorAsync(properties);
531+
}
532+
523533
return HandleRequestResult.Fail(CreateOpenIdConnectProtocolException(authorizationResponse, response: null), properties);
524534
}
525535

‎src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,14 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
5555

5656
var properties = requestToken.Properties;
5757

58-
// REVIEW: see which of these are really errors
59-
6058
var denied = query["denied"];
6159
if (!StringValues.IsNullOrEmpty(denied))
6260
{
63-
return HandleRequestResult.Fail("The user denied permissions.", properties);
61+
// Note: denied errors are special protocol errors indicating the user didn't
62+
// approve the authorization demand requested by the remote authorization server.
63+
// Since it's a frequent scenario (that is not caused by incorrect configuration),
64+
// denied errors are handled differently using HandleAccessDeniedErrorAsync().
65+
return await HandleAccessDeniedErrorAsync(properties);
6466
}
6567

6668
var returnedToken = query["oauth_token"];
@@ -130,7 +132,7 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
130132
{
131133
if (string.IsNullOrEmpty(properties.RedirectUri))
132134
{
133-
properties.RedirectUri = CurrentUri;
135+
properties.RedirectUri = OriginalPathBase + OriginalPath + Request.QueryString;
134136
}
135137

136138
// If CallbackConfirmed is false, this will throw

‎src/Microsoft.AspNetCore.Authentication.Twitter/TwitterOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Security.Claims;
65
using System.Globalization;
6+
using System.Security.Claims;
77
using Microsoft.AspNetCore.Authentication.OAuth.Claims;
88
using Microsoft.AspNetCore.Http;
99

‎src/Microsoft.AspNetCore.Authentication.WsFederation/WsFederationHandler.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
8383
// Save the original challenge URI so we can redirect back to it when we're done.
8484
if (string.IsNullOrEmpty(properties.RedirectUri))
8585
{
86-
properties.RedirectUri = CurrentUri;
86+
properties.RedirectUri = OriginalPathBase + OriginalPath + Request.QueryString;
8787
}
8888

8989
var wsFederationMessage = new WsFederationMessage()

‎src/Microsoft.AspNetCore.Authentication/Events/AccessDeniedContext.cs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Http;
5+
6+
namespace Microsoft.AspNetCore.Authentication
7+
{
8+
/// <summary>
9+
/// Provides access denied failure context information to handler providers.
10+
/// </summary>
11+
public class AccessDeniedContext : HandleRequestContext<RemoteAuthenticationOptions>
12+
{
13+
public AccessDeniedContext(
14+
HttpContext context,
15+
AuthenticationScheme scheme,
16+
RemoteAuthenticationOptions options)
17+
: base(context, scheme, options)
18+
{
19+
}
20+
21+
/// <summary>
22+
/// Gets or sets the endpoint path the user agent will be redirected to.
23+
/// By default, this property is set to <see cref="RemoteAuthenticationOptions.AccessDeniedPath"/>.
24+
/// </summary>
25+
public PathString AccessDeniedPath { get; set; }
26+
27+
/// <summary>
28+
/// Additional state values for the authentication session.
29+
/// </summary>
30+
public AuthenticationProperties Properties { get; set; }
31+
32+
/// <summary>
33+
/// Gets or sets the return URL that will be flowed up to the access denied page.
34+
/// If <see cref="ReturnUrlParameter"/> is not set, this property is not used.
35+
/// </summary>
36+
public string ReturnUrl { get; set; }
37+
38+
/// <summary>
39+
/// Gets or sets the parameter name that will be used to flow the return URL.
40+
/// By default, this property is set to <see cref="RemoteAuthenticationOptions.ReturnUrlParameter"/>.
41+
/// </summary>
42+
public string ReturnUrlParameter { get; set; }
43+
}
44+
}

‎src/Microsoft.AspNetCore.Authentication/Events/RemoteAuthenticationEvents.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,18 @@ namespace Microsoft.AspNetCore.Authentication
88
{
99
public class RemoteAuthenticationEvents
1010
{
11+
public Func<AccessDeniedContext, Task> OnAccessDenied { get; set; } = context => Task.CompletedTask;
1112
public Func<RemoteFailureContext, Task> OnRemoteFailure { get; set; } = context => Task.CompletedTask;
1213

1314
public Func<TicketReceivedContext, Task> OnTicketReceived { get; set; } = context => Task.CompletedTask;
1415

1516
/// <summary>
16-
/// Invoked when there is a remote failure
17+
/// Invoked when an access denied error was returned by the remote server.
18+
/// </summary>
19+
public virtual Task AccessDenied(AccessDeniedContext context) => OnAccessDenied(context);
20+
21+
/// <summary>
22+
/// Invoked when there is a remote failure.
1723
/// </summary>
1824
public virtual Task RemoteFailure(RemoteFailureContext context) => OnRemoteFailure(context);
1925

‎src/Microsoft.AspNetCore.Authentication/LoggingExtensions.cs

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,20 @@ namespace Microsoft.Extensions.Logging
77
{
88
internal static class LoggingExtensions
99
{
10-
private static Action<ILogger, string, Exception> _authSchemeAuthenticated;
11-
private static Action<ILogger, string, Exception> _authSchemeNotAuthenticated;
12-
private static Action<ILogger, string, string, Exception> _authSchemeNotAuthenticatedWithFailure;
13-
private static Action<ILogger, string, Exception> _authSchemeChallenged;
14-
private static Action<ILogger, string, Exception> _authSchemeForbidden;
15-
private static Action<ILogger, string, Exception> _remoteAuthenticationError;
16-
private static Action<ILogger, Exception> _signInHandled;
17-
private static Action<ILogger, Exception> _signInSkipped;
18-
private static Action<ILogger, string, Exception> _correlationPropertyNotFound;
19-
private static Action<ILogger, string, Exception> _correlationCookieNotFound;
20-
private static Action<ILogger, string, string, Exception> _unexpectedCorrelationCookieValue;
10+
private static readonly Action<ILogger, string, Exception> _authSchemeAuthenticated;
11+
private static readonly Action<ILogger, string, Exception> _authSchemeNotAuthenticated;
12+
private static readonly Action<ILogger, string, string, Exception> _authSchemeNotAuthenticatedWithFailure;
13+
private static readonly Action<ILogger, string, Exception> _authSchemeChallenged;
14+
private static readonly Action<ILogger, string, Exception> _authSchemeForbidden;
15+
private static readonly Action<ILogger, string, Exception> _remoteAuthenticationError;
16+
private static readonly Action<ILogger, Exception> _signInHandled;
17+
private static readonly Action<ILogger, Exception> _signInSkipped;
18+
private static readonly Action<ILogger, string, Exception> _correlationPropertyNotFound;
19+
private static readonly Action<ILogger, string, Exception> _correlationCookieNotFound;
20+
private static readonly Action<ILogger, string, string, Exception> _unexpectedCorrelationCookieValue;
21+
private static readonly Action<ILogger, Exception> _accessDeniedError;
22+
private static readonly Action<ILogger, Exception> _accessDeniedContextHandled;
23+
private static readonly Action<ILogger, Exception> _accessDeniedContextSkipped;
2124

2225
static LoggingExtensions()
2326
{
@@ -65,6 +68,18 @@ static LoggingExtensions()
6568
eventId: 16,
6669
logLevel: LogLevel.Warning,
6770
formatString: "The correlation cookie value '{CorrelationCookieName}' did not match the expected value '{CorrelationCookieValue}'.");
71+
_accessDeniedError = LoggerMessage.Define(
72+
eventId: 17,
73+
logLevel: LogLevel.Information,
74+
formatString: "Access was denied by the resource owner or by the remote server.");
75+
_accessDeniedContextHandled = LoggerMessage.Define(
76+
eventId: 18,
77+
logLevel: LogLevel.Debug,
78+
formatString: "The AccessDenied event returned Handled.");
79+
_accessDeniedContextSkipped = LoggerMessage.Define(
80+
eventId: 19,
81+
logLevel: LogLevel.Debug,
82+
formatString: "The AccessDenied event returned Skipped.");
6883
}
6984

7085
public static void AuthenticationSchemeAuthenticated(this ILogger logger, string authenticationScheme)
@@ -121,5 +136,20 @@ public static void UnexpectedCorrelationCookieValue(this ILogger logger, string
121136
{
122137
_unexpectedCorrelationCookieValue(logger, cookieName, cookieValue, null);
123138
}
139+
140+
public static void AccessDeniedError(this ILogger logger)
141+
{
142+
_accessDeniedError(logger, null);
143+
}
144+
145+
public static void AccessDeniedContextHandled(this ILogger logger)
146+
{
147+
_accessDeniedContextHandled(logger, null);
148+
}
149+
150+
public static void AccessDeniedContextSkipped(this ILogger logger)
151+
{
152+
_accessDeniedContextSkipped(logger, null);
153+
}
124154
}
125155
}

‎src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Security.Cryptography;
66
using System.Text.Encodings.Web;
77
using System.Threading.Tasks;
8+
using Microsoft.AspNetCore.WebUtilities;
89
using Microsoft.Extensions.Logging;
910
using Microsoft.Extensions.Options;
1011

@@ -241,5 +242,48 @@ protected virtual bool ValidateCorrelationId(AuthenticationProperties properties
241242

242243
return true;
243244
}
245+
246+
protected virtual async Task<HandleRequestResult> HandleAccessDeniedErrorAsync(AuthenticationProperties properties)
247+
{
248+
Logger.AccessDeniedError();
249+
var context = new AccessDeniedContext(Context, Scheme, Options)
250+
{
251+
AccessDeniedPath = Options.AccessDeniedPath,
252+
Properties = properties,
253+
ReturnUrl = properties?.RedirectUri,
254+
ReturnUrlParameter = Options.ReturnUrlParameter
255+
};
256+
await Events.AccessDenied(context);
257+
258+
if (context.Result != null)
259+
{
260+
if (context.Result.Handled)
261+
{
262+
Logger.AccessDeniedContextHandled();
263+
}
264+
else if (context.Result.Skipped)
265+
{
266+
Logger.AccessDeniedContextSkipped();
267+
}
268+
269+
return context.Result;
270+
}
271+
272+
// If an access denied endpoint was specified, redirect the user agent.
273+
// Otherwise, invoke the RemoteFailure event for further processing.
274+
if (context.AccessDeniedPath.HasValue)
275+
{
276+
string uri = context.AccessDeniedPath;
277+
if (!string.IsNullOrEmpty(context.ReturnUrlParameter) && !string.IsNullOrEmpty(context.ReturnUrl))
278+
{
279+
uri = QueryHelpers.AddQueryString(uri, context.ReturnUrlParameter, context.ReturnUrl);
280+
}
281+
Response.Redirect(uri);
282+
283+
return HandleRequestResult.Handle();
284+
}
285+
286+
return HandleRequestResult.Fail("Access was denied by the resource owner or by the remote server.", properties);
287+
}
244288
}
245289
}

‎src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,22 @@ public override void Validate()
8989
/// </summary>
9090
public PathString CallbackPath { get; set; }
9191

92+
/// <summary>
93+
/// Gets or sets the optional path the user agent is redirected to if the user
94+
/// doesn't approve the authorization demand requested by the remote server.
95+
/// This property is not set by default. In this case, an exception is thrown
96+
/// if an access_denied response is returned by the remote authorization server.
97+
/// </summary>
98+
public PathString AccessDeniedPath { get; set; }
99+
100+
/// <summary>
101+
/// Gets or sets the name of the parameter used to convey the original location
102+
/// of the user before the remote challenge was triggered up to the access denied page.
103+
/// This property is only used when the <see cref="AccessDeniedPath"/> is explicitly specified.
104+
/// </summary>
105+
// Note: this deliberately matches the default parameter name used by the cookie handler.
106+
public string ReturnUrlParameter { get; set; } = "ReturnUrl";
107+
92108
/// <summary>
93109
/// Gets or sets the authentication scheme corresponding to the middleware
94110
/// responsible of persisting user's identity after a successful authentication.

0 commit comments

Comments
 (0)