diff --git a/sandbox/OpenIddict.Sandbox.AspNet.Client/Startup.cs b/sandbox/OpenIddict.Sandbox.AspNet.Client/Startup.cs index ef1a8b16..ce75bc16 100644 --- a/sandbox/OpenIddict.Sandbox.AspNet.Client/Startup.cs +++ b/sandbox/OpenIddict.Sandbox.AspNet.Client/Startup.cs @@ -22,7 +22,7 @@ namespace OpenIddict.Sandbox.AspNet.Client // Register the Autofac scope injector middleware. app.UseAutofacLifetimeScopeInjector(container); - // Register the cookie middleware responsible of storing the user sessions. + // Register the cookie middleware responsible for storing the user sessions. app.UseCookieAuthentication(new CookieAuthenticationOptions { ExpireTimeSpan = TimeSpan.FromMinutes(50), diff --git a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.Authentication.cs b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.Authentication.cs index 6e0f32ef..89454951 100644 --- a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.Authentication.cs +++ b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.Authentication.cs @@ -33,6 +33,7 @@ public static partial class OpenIddictClientOwinHandlers * Redirection response handling: */ AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachCacheControlHeader.Descriptor, ProcessPassthroughErrorResponse.Descriptor, ProcessLocalErrorResponse.Descriptor); diff --git a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs index 5b39ea4b..d19b8f6a 100644 --- a/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs +++ b/src/OpenIddict.Client.Owin/OpenIddictClientOwinHandlers.cs @@ -616,6 +616,58 @@ public static partial class OpenIddictClientOwinHandlers } } + /// + /// Contains the logic responsible for attaching an OWIN response chalenge to the context, if necessary. + /// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN. + /// + public class AttachOwinResponseChallenge : IOpenIddictClientHandler where TContext : BaseRequestContext + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictClientHandlerDescriptor Descriptor { get; } + = OpenIddictClientHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler>() + .SetOrder(AttachHttpResponseCode.Descriptor.Order + 1_000) + .SetType(OpenIddictClientHandlerType.BuiltIn) + .Build(); + + /// + public ValueTask HandleAsync(TContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + // This handler only applies to OWIN requests. If The OWIN request cannot be resolved, + // this may indicate that the request was incorrectly processed by another server stack. + var response = context.Transaction.GetOwinRequest()?.Context.Response ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0120)); + + // OWIN authentication middleware configured to use active authentication (which is the default mode) + // are known to aggressively intercept 401 responses even if the request is already considered fully + // handled. In practice, this behavior is often seen with the cookies authentication middleware, + // that will rewrite the 401 responses returned by OpenIddict and try to redirect the user agent + // to the login page configured in the options. To prevent this undesirable behavior, a fake + // response challenge pointing to a non-existent middleware is manually added to the OWIN context + // to prevent the active authentication middleware from rewriting OpenIddict's 401 HTTP responses. + // + // Note: while 403 responses are generally not intercepted by the built-in OWIN authentication + // middleware, they are treated the same way as 401 responses to account for custom middleware + // that may potentially use the same interception logic for both 401 and 403 HTTP responses. + if (response.StatusCode is 401 or 403 && + response.Context.Authentication.AuthenticationResponseChallenge is null) + { + response.Context.Authentication.AuthenticationResponseChallenge = + new AuthenticationResponseChallenge(new[] { Guid.NewGuid().ToString() }, null); + } + + return default; + } + } + /// /// Contains the logic responsible for attaching the appropriate HTTP response cache headers. /// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN. @@ -629,7 +681,7 @@ public static partial class OpenIddictClientOwinHandlers = OpenIddictClientHandlerDescriptor.CreateBuilder() .AddFilter() .UseSingletonHandler>() - .SetOrder(AttachHttpResponseCode.Descriptor.Order + 1_000) + .SetOrder(AttachOwinResponseChallenge.Descriptor.Order + 1_000) .SetType(OpenIddictClientHandlerType.BuiltIn) .Build(); diff --git a/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandlers.cs b/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandlers.cs index 599de079..e5d2e89f 100644 --- a/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandlers.cs +++ b/src/OpenIddict.Server.AspNetCore/OpenIddictServerAspNetCoreHandlers.cs @@ -37,6 +37,7 @@ public static partial class OpenIddictServerAspNetCoreHandlers /* * Challenge processing: */ + AttachHostChallengeError.Descriptor, ResolveHostChallengeParameters.Descriptor, /* @@ -277,11 +278,10 @@ public static partial class OpenIddictServerAspNetCoreHandlers } /// - /// Contains the logic responsible for resolving the additional sign-in parameters stored in the ASP.NET - /// Core authentication properties specified by the application that triggered the sign-in operation. + /// Contains the logic responsible for attaching the error details using the ASP.NET Core authentication properties. /// Note: this handler is not used when the OpenID Connect request is not initially handled by ASP.NET Core. /// - public class ResolveHostChallengeParameters : IOpenIddictServerHandler + public class AttachHostChallengeError : IOpenIddictServerHandler { /// /// Gets the default descriptor definition assigned to this handler. @@ -289,7 +289,7 @@ public static partial class OpenIddictServerAspNetCoreHandlers public static OpenIddictServerHandlerDescriptor Descriptor { get; } = OpenIddictServerHandlerDescriptor.CreateBuilder() .AddFilter() - .UseSingletonHandler() + .UseSingletonHandler() .SetOrder(AttachDefaultChallengeError.Descriptor.Order - 500) .SetType(OpenIddictServerHandlerType.BuiltIn) .Build(); @@ -303,33 +303,48 @@ public static partial class OpenIddictServerAspNetCoreHandlers } var properties = context.Transaction.GetProperty(typeof(AuthenticationProperties).FullName!); - if (properties is null) + if (properties is not null) { - return default; + context.Response.Error = properties.GetString(Properties.Error); + context.Response.ErrorDescription = properties.GetString(Properties.ErrorDescription); + context.Response.ErrorUri = properties.GetString(Properties.ErrorUri); + context.Response.Scope = properties.GetString(Properties.Scope); } - if (properties.Items.TryGetValue(Properties.Error, out string? error) && - !string.IsNullOrEmpty(error)) - { - context.Parameters[Parameters.Error] = error; - } + return default; + } + } - if (properties.Items.TryGetValue(Properties.ErrorDescription, out string? description) && - !string.IsNullOrEmpty(description)) - { - context.Parameters[Parameters.ErrorDescription] = description; - } + /// + /// Contains the logic responsible for resolving the additional sign-in parameters stored in the ASP.NET + /// Core authentication properties specified by the application that triggered the sign-in operation. + /// Note: this handler is not used when the OpenID Connect request is not initially handled by ASP.NET Core. + /// + public class ResolveHostChallengeParameters : IOpenIddictServerHandler + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictServerHandlerDescriptor Descriptor { get; } + = OpenIddictServerHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler() + .SetOrder(AttachChallengeParameters.Descriptor.Order - 500) + .SetType(OpenIddictServerHandlerType.BuiltIn) + .Build(); - if (properties.Items.TryGetValue(Properties.ErrorUri, out string? uri) && - !string.IsNullOrEmpty(uri)) + /// + public ValueTask HandleAsync(ProcessChallengeContext context) + { + if (context is null) { - context.Parameters[Parameters.ErrorUri] = uri; + throw new ArgumentNullException(nameof(context)); } - if (properties.Items.TryGetValue(Properties.Scope, out string? scope) && - !string.IsNullOrEmpty(scope)) + var properties = context.Transaction.GetProperty(typeof(AuthenticationProperties).FullName!); + if (properties is null) { - context.Parameters[Parameters.Scope] = scope; + return default; } foreach (var parameter in properties.Parameters) @@ -875,29 +890,37 @@ public static partial class OpenIddictServerAspNetCoreHandlers throw new ArgumentNullException(nameof(context)); } + Debug.Assert(context.Transaction.Response is not null, SR.GetResourceString(SR.ID4007)); + // This handler only applies to ASP.NET Core requests. If the HTTP context cannot be resolved, // this may indicate that the request was incorrectly processed by another server stack. var response = context.Transaction.GetHttpRequest()?.HttpContext.Response ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0114)); - Debug.Assert(context.Transaction.Response is not null, SR.GetResourceString(SR.ID4007)); - - // When client authentication is made using basic authentication, the authorization server MUST return - // a 401 response with a valid WWW-Authenticate header containing the Basic scheme and a non-empty realm. - // A similar error MAY be returned even when basic authentication is not used and MUST also be returned - // when an invalid token is received by the userinfo endpoint using the Bearer authentication scheme. - // To simplify the logic, a 401 response with the Bearer scheme is returned for invalid_token errors - // and a 401 response with the Basic scheme is returned for invalid_client, even if the credentials - // were specified in the request form instead of the HTTP headers, as allowed by the specification. - response.StatusCode = context.Transaction.Response.Error switch + response.StatusCode = (context.EndpointType, context.Transaction.Response.Error) switch { - null => 200, // Note: the default code may be replaced by another handler (e.g when doing redirects). + // Note: the default code may be replaced by another handler (e.g when doing redirects). + (_, null or { Length: 0 }) => 200, + + // Unlike other server endpoints, errors returned by the userinfo endpoint follow the same logic as + // errors returned by API endpoints implementing bearer token authentication and MUST be returned + // as part of the standard WWW-Authenticate header. For more information, see + // https://openid.net/specs/openid-connect-core-1_0.html#UserInfoError. + (OpenIddictServerEndpointType.Userinfo, Errors.InvalidToken or Errors.MissingToken) => 401, + (OpenIddictServerEndpointType.Userinfo, Errors.InsufficientAccess or Errors.InsufficientScope) => 403, - Errors.InvalidClient or Errors.InvalidToken or Errors.MissingToken => 401, + // When client authentication is made using basic authentication, the authorization server + // MUST return a 401 response with a valid WWW-Authenticate header containing the HTTP Basic + // authentication scheme. A similar error MAY be returned even when using client_secret_post. + // To simplify the logic, a 401 response with the Basic scheme is returned for invalid_client + // errors, even if credentials were specified in the form, as allowed by the specification. + (not OpenIddictServerEndpointType.Userinfo, Errors.InvalidClient) => 401, - Errors.InsufficientAccess or Errors.InsufficientScope => 403, + (_, Errors.ServerError) => 500, - _ => 400 + // Note: unless specified otherwise, errors are expected to result in 400 responses. + // See https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 for more information. + _ => 400 }; return default; @@ -973,29 +996,35 @@ public static partial class OpenIddictServerAspNetCoreHandlers throw new ArgumentNullException(nameof(context)); } + Debug.Assert(context.Transaction.Response is not null, SR.GetResourceString(SR.ID4007)); + // This handler only applies to ASP.NET Core requests. If the HTTP context cannot be resolved, // this may indicate that the request was incorrectly processed by another server stack. var response = context.Transaction.GetHttpRequest()?.HttpContext.Response ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0114)); - Debug.Assert(context.Transaction.Response is not null, SR.GetResourceString(SR.ID4007)); + if (string.IsNullOrEmpty(context.Transaction.Response.Error)) + { + return default; + } - // When client authentication is made using basic authentication, the authorization server MUST return - // a 401 response with a valid WWW-Authenticate header containing the HTTP Basic authentication scheme. - // A similar error MAY be returned even when basic authentication is not used and MUST also be returned - // when an invalid token is received by the userinfo endpoint using the Bearer authentication scheme. - // To simplify the logic, a 401 response with the Bearer scheme is returned for invalid_token errors - // and a 401 response with the Basic scheme is returned for invalid_client, even if the credentials - // were specified in the request form instead of the HTTP headers, as allowed by the specification. - var scheme = context.Transaction.Response.Error switch + var scheme = (context.EndpointType, context.Transaction.Response.Error) switch { - Errors.InvalidClient => Schemes.Basic, + // Unlike other server endpoints, errors returned by the userinfo endpoint follow the same + // logic as errors returned by API endpoints implementing bearer token authentication and + // MUST be returned as part of the standard WWW-Authenticate header. For more information, + // see https://openid.net/specs/openid-connect-core-1_0.html#UserInfoError. + (OpenIddictServerEndpointType.Userinfo, _) => Schemes.Bearer, - Errors.InvalidToken or - Errors.MissingToken or - Errors.InsufficientAccess or - Errors.InsufficientScope => Schemes.Bearer, + // When client authentication is made using basic authentication, the authorization server + // MUST return a 401 response with a valid WWW-Authenticate header containing the HTTP Basic + // authentication scheme. A similar error MAY be returned even when using client_secret_post. + // To simplify the logic, a 401 response with the Basic scheme is returned for invalid_client + // errors, even if credentials were specified in the form, as allowed by the specification. + (_, Errors.InvalidClient) => Schemes.Basic, + // For all other errors, don't return a WWW-Authenticate header and return server errors + // as formatted JSON responses, as required by the OAuth 2.0 base specification. _ => null }; @@ -1007,19 +1036,19 @@ public static partial class OpenIddictServerAspNetCoreHandlers var parameters = new Dictionary(StringComparer.Ordinal); // If a realm was configured in the options, attach it to the parameters. - if (!string.IsNullOrEmpty(_options.CurrentValue.Realm)) + if (_options.CurrentValue.Realm is string { Length: > 0 } realm) { - parameters[Parameters.Realm] = _options.CurrentValue.Realm; + parameters[Parameters.Realm] = realm; } foreach (var parameter in context.Transaction.Response.GetParameters()) { // Note: the error details are only included if the error was not caused by a missing token, as recommended // by the OAuth 2.0 bearer specification: https://tools.ietf.org/html/rfc6750#section-3.1. - if (string.Equals(context.Transaction.Response.Error, Errors.MissingToken, StringComparison.Ordinal) && - (string.Equals(parameter.Key, Parameters.Error, StringComparison.Ordinal) || - string.Equals(parameter.Key, Parameters.ErrorDescription, StringComparison.Ordinal) || - string.Equals(parameter.Key, Parameters.ErrorUri, StringComparison.Ordinal))) + if (context.Transaction.Response.Error is Errors.MissingToken && + parameter.Key is Parameters.Error or + Parameters.ErrorDescription or + Parameters.ErrorUri) { continue; } diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Authentication.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Authentication.cs index bc7cb726..b5184d45 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Authentication.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Authentication.cs @@ -44,6 +44,7 @@ public static partial class OpenIddictServerOwinHandlers */ RemoveCachedRequest.Descriptor, AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachCacheControlHeader.Descriptor, ProcessFormPostResponse.Descriptor, ProcessQueryResponse.Descriptor, diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Device.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Device.cs index 82bcd6d6..49fe072e 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Device.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Device.cs @@ -25,6 +25,7 @@ public static partial class OpenIddictServerOwinHandlers * Device response processing: */ AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachCacheControlHeader.Descriptor, AttachWwwAuthenticateHeader.Descriptor, ProcessJsonResponse.Descriptor, @@ -43,6 +44,7 @@ public static partial class OpenIddictServerOwinHandlers * Verification response processing: */ AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachCacheControlHeader.Descriptor, ProcessHostRedirectionResponse.Descriptor, ProcessPassthroughErrorResponse.Descriptor, diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Discovery.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Discovery.cs index 8cf2f73e..b7e76f9c 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Discovery.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Discovery.cs @@ -22,6 +22,7 @@ public static partial class OpenIddictServerOwinHandlers * Configuration response processing: */ AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachWwwAuthenticateHeader.Descriptor, ProcessJsonResponse.Descriptor, @@ -34,6 +35,7 @@ public static partial class OpenIddictServerOwinHandlers * Cryptography response processing: */ AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachWwwAuthenticateHeader.Descriptor, ProcessJsonResponse.Descriptor); } diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Exchange.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Exchange.cs index 174ece72..8de80efd 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Exchange.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Exchange.cs @@ -28,6 +28,7 @@ public static partial class OpenIddictServerOwinHandlers * Token response processing: */ AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachCacheControlHeader.Descriptor, AttachWwwAuthenticateHeader.Descriptor, ProcessJsonResponse.Descriptor); diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Introspection.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Introspection.cs index c9a7175e..9bd7a2d6 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Introspection.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Introspection.cs @@ -23,6 +23,7 @@ public static partial class OpenIddictServerOwinHandlers * Introspection response processing: */ AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachWwwAuthenticateHeader.Descriptor, ProcessJsonResponse.Descriptor); } diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Revocation.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Revocation.cs index db85a8ac..94405ee1 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Revocation.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Revocation.cs @@ -23,6 +23,7 @@ public static partial class OpenIddictServerOwinHandlers * Revocation response processing: */ AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachCacheControlHeader.Descriptor, AttachWwwAuthenticateHeader.Descriptor, ProcessJsonResponse.Descriptor); diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Session.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Session.cs index c8f23b4e..1b15e441 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Session.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Session.cs @@ -42,6 +42,7 @@ public static partial class OpenIddictServerOwinHandlers */ RemoveCachedRequest.Descriptor, AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachCacheControlHeader.Descriptor, ProcessHostRedirectionResponse.Descriptor, ProcessPassthroughErrorResponse.Descriptor, diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Userinfo.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Userinfo.cs index 36cb6fb1..065ca209 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Userinfo.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.Userinfo.cs @@ -28,6 +28,7 @@ public static partial class OpenIddictServerOwinHandlers * Userinfo response processing: */ AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachWwwAuthenticateHeader.Descriptor, ProcessChallengeErrorResponse.Descriptor, ProcessJsonResponse.Descriptor); diff --git a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.cs b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.cs index 409d88bf..90b3a968 100644 --- a/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.cs +++ b/src/OpenIddict.Server.Owin/OpenIddictServerOwinHandlers.cs @@ -32,7 +32,7 @@ public static partial class OpenIddictServerOwinHandlers /* * Challenge processing: */ - ResolveHostChallengeParameters.Descriptor) + AttachHostChallengeError.Descriptor) .AddRange(Authentication.DefaultHandlers) .AddRange(Device.DefaultHandlers) .AddRange(Discovery.DefaultHandlers) @@ -264,11 +264,10 @@ public static partial class OpenIddictServerOwinHandlers } /// - /// Contains the logic responsible for resolving the additional sign-in parameters stored in the OWIN - /// authentication properties specified by the application that triggered the sign-in operation. + /// Contains the logic responsible for attaching the error details using the OWIN authentication properties. /// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN. /// - public class ResolveHostChallengeParameters : IOpenIddictServerHandler + public class AttachHostChallengeError : IOpenIddictServerHandler { /// /// Gets the default descriptor definition assigned to this handler. @@ -276,8 +275,8 @@ public static partial class OpenIddictServerOwinHandlers public static OpenIddictServerHandlerDescriptor Descriptor { get; } = OpenIddictServerHandlerDescriptor.CreateBuilder() .AddFilter() - .UseSingletonHandler() - .SetOrder(AttachChallengeParameters.Descriptor.Order - 500) + .UseSingletonHandler() + .SetOrder(AttachDefaultChallengeError.Descriptor.Order - 500) .SetType(OpenIddictServerHandlerType.BuiltIn) .Build(); @@ -290,36 +289,18 @@ public static partial class OpenIddictServerOwinHandlers } var properties = context.Transaction.GetProperty(typeof(AuthenticationProperties).FullName!); - if (properties is null) + if (properties is not null) { - return default; - } - - if (properties.Dictionary.TryGetValue(Properties.Error, out string? error) && - !string.IsNullOrEmpty(error)) - { - context.Parameters[Parameters.Error] = error; - } - - if (properties.Dictionary.TryGetValue(Properties.ErrorDescription, out string? description) && - !string.IsNullOrEmpty(description)) - { - context.Parameters[Parameters.ErrorDescription] = description; - } - - if (properties.Dictionary.TryGetValue(Properties.ErrorUri, out string? uri) && - !string.IsNullOrEmpty(uri)) - { - context.Parameters[Parameters.ErrorUri] = uri; - } - - if (properties.Dictionary.TryGetValue(Properties.Scope, out string? scope) && - !string.IsNullOrEmpty(scope)) - { - context.Parameters[Parameters.Scope] = scope; + context.Response.Error = GetProperty(properties, Properties.Error); + context.Response.ErrorDescription = GetProperty(properties, Properties.ErrorDescription); + context.Response.ErrorUri = GetProperty(properties, Properties.ErrorUri); + context.Response.Scope = GetProperty(properties, Properties.Scope); } return default; + + static string? GetProperty(AuthenticationProperties properties, string name) + => properties.Dictionary.TryGetValue(name, out string? value) ? value : null; } } @@ -740,23 +721,83 @@ public static partial class OpenIddictServerOwinHandlers var response = context.Transaction.GetOwinRequest()?.Context.Response ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0120)); - // When client authentication is made using basic authentication, the authorization server MUST return - // a 401 response with a valid WWW-Authenticate header containing the Basic scheme and a non-empty realm. - // A similar error MAY be returned even when basic authentication is not used and MUST also be returned - // when an invalid token is received by the userinfo endpoint using the Bearer authentication scheme. - // To simplify the logic, a 401 response with the Bearer scheme is returned for invalid_token errors - // and a 401 response with the Basic scheme is returned for invalid_client, even if the credentials - // were specified in the request form instead of the HTTP headers, as allowed by the specification. - response.StatusCode = context.Transaction.Response.Error switch + response.StatusCode = (context.EndpointType, context.Transaction.Response.Error) switch { - null => 200, // Note: the default code may be replaced by another handler (e.g when doing redirects). + // Note: the default code may be replaced by another handler (e.g when doing redirects). + (_, null or { Length: 0 }) => 200, + + // Unlike other server endpoints, errors returned by the userinfo endpoint follow the same logic as + // errors returned by API endpoints implementing bearer token authentication and MUST be returned + // as part of the standard WWW-Authenticate header. For more information, see + // https://openid.net/specs/openid-connect-core-1_0.html#UserInfoError. + (OpenIddictServerEndpointType.Userinfo, Errors.InvalidToken or Errors.MissingToken) => 401, + (OpenIddictServerEndpointType.Userinfo, Errors.InsufficientAccess or Errors.InsufficientScope) => 403, + + // When client authentication is made using basic authentication, the authorization server + // MUST return a 401 response with a valid WWW-Authenticate header containing the HTTP Basic + // authentication scheme. A similar error MAY be returned even when using client_secret_post. + // To simplify the logic, a 401 response with the Basic scheme is returned for invalid_client + // errors, even if credentials were specified in the form, as allowed by the specification. + (not OpenIddictServerEndpointType.Userinfo, Errors.InvalidClient) => 401, + + (_, Errors.ServerError) => 500, + + // Note: unless specified otherwise, errors are expected to result in 400 responses. + // See https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 for more information. + _ => 400 + }; - Errors.InvalidClient or Errors.InvalidToken or Errors.MissingToken => 401, + return default; + } + } - Errors.InsufficientAccess or Errors.InsufficientScope => 403, + /// + /// Contains the logic responsible for attaching an OWIN response chalenge to the context, if necessary. + /// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN. + /// + public class AttachOwinResponseChallenge : IOpenIddictServerHandler where TContext : BaseRequestContext + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictServerHandlerDescriptor Descriptor { get; } + = OpenIddictServerHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler>() + .SetOrder(AttachHttpResponseCode.Descriptor.Order + 1_000) + .SetType(OpenIddictServerHandlerType.BuiltIn) + .Build(); - _ => 400 - }; + /// + public ValueTask HandleAsync(TContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + // This handler only applies to OWIN requests. If The OWIN request cannot be resolved, + // this may indicate that the request was incorrectly processed by another server stack. + var response = context.Transaction.GetOwinRequest()?.Context.Response ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0120)); + + // OWIN authentication middleware configured to use active authentication (which is the default mode) + // are known to aggressively intercept 401 responses even if the request is already considered fully + // handled. In practice, this behavior is often seen with the cookies authentication middleware, + // that will rewrite the 401 responses returned by OpenIddict and try to redirect the user agent + // to the login page configured in the options. To prevent this undesirable behavior, a fake + // response challenge pointing to a non-existent middleware is manually added to the OWIN context + // to prevent the active authentication middleware from rewriting OpenIddict's 401 HTTP responses. + // + // Note: while 403 responses are generally not intercepted by the built-in OWIN authentication + // middleware, they are treated the same way as 401 responses to account for custom middleware + // that may potentially use the same interception logic for both 401 and 403 HTTP responses. + if (response.StatusCode is 401 or 403 && + response.Context.Authentication.AuthenticationResponseChallenge is null) + { + response.Context.Authentication.AuthenticationResponseChallenge = + new AuthenticationResponseChallenge(new[] { Guid.NewGuid().ToString() }, null); + } return default; } @@ -775,7 +816,7 @@ public static partial class OpenIddictServerOwinHandlers = OpenIddictServerHandlerDescriptor.CreateBuilder() .AddFilter() .UseSingletonHandler>() - .SetOrder(AttachHttpResponseCode.Descriptor.Order + 1_000) + .SetOrder(AttachOwinResponseChallenge.Descriptor.Order + 1_000) .SetType(OpenIddictServerHandlerType.BuiltIn) .Build(); @@ -838,22 +879,28 @@ public static partial class OpenIddictServerOwinHandlers var response = context.Transaction.GetOwinRequest()?.Context.Response ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0120)); - // When client authentication is made using basic authentication, the authorization server MUST return - // a 401 response with a valid WWW-Authenticate header containing the HTTP Basic authentication scheme. - // A similar error MAY be returned even when basic authentication is not used and MUST also be returned - // when an invalid token is received by the userinfo endpoint using the Bearer authentication scheme. - // To simplify the logic, a 401 response with the Bearer scheme is returned for invalid_token errors - // and a 401 response with the Basic scheme is returned for invalid_client, even if the credentials - // were specified in the request form instead of the HTTP headers, as allowed by the specification. - var scheme = context.Transaction.Response.Error switch + if (string.IsNullOrEmpty(context.Transaction.Response.Error)) { - Errors.InvalidClient => Schemes.Basic, - - Errors.InvalidToken or - Errors.MissingToken or - Errors.InsufficientAccess or - Errors.InsufficientScope => Schemes.Bearer, + return default; + } + var scheme = (context.EndpointType, context.Transaction.Response.Error) switch + { + // Unlike other server endpoints, errors returned by the userinfo endpoint follow the same + // logic as errors returned by API endpoints implementing bearer token authentication and + // MUST be returned as part of the standard WWW-Authenticate header. For more information, + // see https://openid.net/specs/openid-connect-core-1_0.html#UserInfoError. + (OpenIddictServerEndpointType.Userinfo, _) => Schemes.Bearer, + + // When client authentication is made using basic authentication, the authorization server + // MUST return a 401 response with a valid WWW-Authenticate header containing the HTTP Basic + // authentication scheme. A similar error MAY be returned even when using client_secret_post. + // To simplify the logic, a 401 response with the Basic scheme is returned for invalid_client + // errors, even if credentials were specified in the form, as allowed by the specification. + (_, Errors.InvalidClient) => Schemes.Basic, + + // For all other errors, don't return a WWW-Authenticate header and return server errors + // as formatted JSON responses, as required by the OAuth 2.0 base specification. _ => null }; @@ -865,19 +912,19 @@ public static partial class OpenIddictServerOwinHandlers var parameters = new Dictionary(StringComparer.Ordinal); // If a realm was configured in the options, attach it to the parameters. - if (!string.IsNullOrEmpty(_options.CurrentValue.Realm)) + if (_options.CurrentValue.Realm is string { Length: > 0 } realm) { - parameters[Parameters.Realm] = _options.CurrentValue.Realm; + parameters[Parameters.Realm] = realm; } foreach (var parameter in context.Transaction.Response.GetParameters()) { // Note: the error details are only included if the error was not caused by a missing token, as recommended // by the OAuth 2.0 bearer specification: https://tools.ietf.org/html/rfc6750#section-3.1. - if (string.Equals(context.Transaction.Response.Error, Errors.MissingToken, StringComparison.Ordinal) && - (string.Equals(parameter.Key, Parameters.Error, StringComparison.Ordinal) || - string.Equals(parameter.Key, Parameters.ErrorDescription, StringComparison.Ordinal) || - string.Equals(parameter.Key, Parameters.ErrorUri, StringComparison.Ordinal))) + if (context.Transaction.Response.Error is Errors.MissingToken && + parameter.Key is Parameters.Error or + Parameters.ErrorDescription or + Parameters.ErrorUri) { continue; } diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.cs index 70d8b13b..aa5cbf1e 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.cs @@ -907,47 +907,38 @@ public static partial class OpenIddictServerHandlers throw new ArgumentNullException(nameof(context)); } - if (!context.Parameters.ContainsKey(Parameters.Error)) + context.Response.Error ??= context.EndpointType switch { - context.Parameters[Parameters.Error] = context.EndpointType switch - { - OpenIddictServerEndpointType.Authorization or OpenIddictServerEndpointType.Verification - => Errors.AccessDenied, + OpenIddictServerEndpointType.Authorization or OpenIddictServerEndpointType.Verification + => Errors.AccessDenied, - OpenIddictServerEndpointType.Token => Errors.InvalidGrant, - OpenIddictServerEndpointType.Userinfo => Errors.InsufficientAccess, + OpenIddictServerEndpointType.Token => Errors.InvalidGrant, + OpenIddictServerEndpointType.Userinfo => Errors.InsufficientAccess, - _ => throw new InvalidOperationException(SR.GetResourceString(SR.ID0006)) - }; - } + _ => throw new InvalidOperationException(SR.GetResourceString(SR.ID0006)) + }; - if (!context.Parameters.ContainsKey(Parameters.ErrorDescription)) + context.Response.ErrorDescription ??= context.EndpointType switch { - context.Parameters[Parameters.ErrorDescription] = context.EndpointType switch - { - OpenIddictServerEndpointType.Authorization or OpenIddictServerEndpointType.Verification - => SR.GetResourceString(SR.ID2015), + OpenIddictServerEndpointType.Authorization or OpenIddictServerEndpointType.Verification + => SR.GetResourceString(SR.ID2015), - OpenIddictServerEndpointType.Token => SR.GetResourceString(SR.ID2024), - OpenIddictServerEndpointType.Userinfo => SR.GetResourceString(SR.ID2025), + OpenIddictServerEndpointType.Token => SR.GetResourceString(SR.ID2024), + OpenIddictServerEndpointType.Userinfo => SR.GetResourceString(SR.ID2025), - _ => throw new InvalidOperationException(SR.GetResourceString(SR.ID0006)) - }; - } + _ => throw new InvalidOperationException(SR.GetResourceString(SR.ID0006)) + }; - if (!context.Parameters.ContainsKey(Parameters.ErrorUri)) + context.Response.ErrorUri ??= context.EndpointType switch { - context.Parameters[Parameters.ErrorUri] = context.EndpointType switch - { - OpenIddictServerEndpointType.Authorization or OpenIddictServerEndpointType.Verification - => SR.FormatID8000(SR.ID2015), + OpenIddictServerEndpointType.Authorization or OpenIddictServerEndpointType.Verification + => SR.FormatID8000(SR.ID2015), - OpenIddictServerEndpointType.Token => SR.FormatID8000(SR.ID2024), - OpenIddictServerEndpointType.Userinfo => SR.FormatID8000(SR.ID2025), + OpenIddictServerEndpointType.Token => SR.FormatID8000(SR.ID2024), + OpenIddictServerEndpointType.Userinfo => SR.FormatID8000(SR.ID2025), - _ => throw new InvalidOperationException(SR.GetResourceString(SR.ID0006)) - }; - } + _ => throw new InvalidOperationException(SR.GetResourceString(SR.ID0006)) + }; return default; } diff --git a/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandlers.cs b/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandlers.cs index 19ab53d0..5343057e 100644 --- a/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandlers.cs +++ b/src/OpenIddict.Validation.AspNetCore/OpenIddictValidationAspNetCoreHandlers.cs @@ -43,6 +43,7 @@ public static partial class OpenIddictValidationAspNetCoreHandlers * Challenge processing: */ AttachHostChallengeError.Descriptor, + ResolveHostChallengeParameters.Descriptor, /* * Response processing: @@ -51,13 +52,16 @@ public static partial class OpenIddictValidationAspNetCoreHandlers AttachCacheControlHeader.Descriptor, AttachWwwAuthenticateHeader.Descriptor, ProcessChallengeErrorResponse.Descriptor, - ProcessJsonResponse.Descriptor, AttachHttpResponseCode.Descriptor, AttachCacheControlHeader.Descriptor, AttachWwwAuthenticateHeader.Descriptor, - ProcessChallengeErrorResponse.Descriptor, - ProcessJsonResponse.Descriptor); + ProcessChallengeErrorResponse.Descriptor, + + /* + * Error processing: + */ + AttachErrorParameters.Descriptor); /// /// Contains the logic responsible for infering the default issuer from the HTTP request host and validating it. @@ -292,7 +296,7 @@ public static partial class OpenIddictValidationAspNetCoreHandlers = OpenIddictValidationHandlerDescriptor.CreateBuilder() .AddFilter() .UseSingletonHandler() - .SetOrder(int.MinValue + 50_000) + .SetOrder(AttachDefaultChallengeError.Descriptor.Order - 500) .SetType(OpenIddictValidationHandlerType.BuiltIn) .Build(); @@ -305,33 +309,48 @@ public static partial class OpenIddictValidationAspNetCoreHandlers } var properties = context.Transaction.GetProperty(typeof(AuthenticationProperties).FullName!); - if (properties is null) + if (properties is not null) { - return default; + context.Response.Error = properties.GetString(Properties.Error); + context.Response.ErrorDescription = properties.GetString(Properties.ErrorDescription); + context.Response.ErrorUri = properties.GetString(Properties.ErrorUri); + context.Response.Scope = properties.GetString(Properties.Scope); } - if (properties.Items.TryGetValue(Properties.Error, out string? error) && - !string.IsNullOrEmpty(error)) - { - context.Parameters[Parameters.Error] = error; - } + return default; + } + } - if (properties.Items.TryGetValue(Properties.ErrorDescription, out string? description) && - !string.IsNullOrEmpty(description)) - { - context.Parameters[Parameters.ErrorDescription] = description; - } + /// + /// Contains the logic responsible for resolving the additional sign-in parameters stored in the ASP.NET + /// Core authentication properties specified by the application that triggered the sign-in operation. + /// Note: this handler is not used when the OpenID Connect request is not initially handled by ASP.NET Core. + /// + public class ResolveHostChallengeParameters : IOpenIddictValidationHandler + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictValidationHandlerDescriptor Descriptor { get; } + = OpenIddictValidationHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler() + .SetOrder(AttachChallengeParameters.Descriptor.Order - 500) + .SetType(OpenIddictValidationHandlerType.BuiltIn) + .Build(); - if (properties.Items.TryGetValue(Properties.ErrorUri, out string? uri) && - !string.IsNullOrEmpty(uri)) + /// + public ValueTask HandleAsync(ProcessChallengeContext context) + { + if (context is null) { - context.Parameters[Parameters.ErrorUri] = uri; + throw new ArgumentNullException(nameof(context)); } - if (properties.Items.TryGetValue(Properties.Scope, out string? scope) && - !string.IsNullOrEmpty(scope)) + var properties = context.Transaction.GetProperty(typeof(AuthenticationProperties).FullName!); + if (properties is null) { - context.Parameters[Parameters.Scope] = scope; + return default; } foreach (var parameter in properties.Parameters) @@ -391,12 +410,15 @@ public static partial class OpenIddictValidationAspNetCoreHandlers response.StatusCode = context.Transaction.Response.Error switch { - null => 200, + // Note: the default code may be replaced by another handler (e.g when doing redirects). + null or { Length: 0 } => 200, Errors.InvalidToken or Errors.MissingToken => 401, Errors.InsufficientAccess or Errors.InsufficientScope => 403, + Errors.ServerError => 500, + _ => 400 }; @@ -480,37 +502,34 @@ public static partial class OpenIddictValidationAspNetCoreHandlers var response = context.Transaction.GetHttpRequest()?.HttpContext.Response ?? throw new InvalidOperationException(SR.GetResourceString(SR.ID0114)); - var scheme = context.Transaction.Response.Error switch - { - Errors.InvalidToken or - Errors.MissingToken or - Errors.InsufficientAccess or - Errors.InsufficientScope => Schemes.Bearer, - - _ => null - }; - - if (string.IsNullOrEmpty(scheme)) + if (string.IsNullOrEmpty(context.Transaction.Response.Error)) { return default; } + // Note: unlike the server stack, the validation stack doesn't expose any endpoint + // and thus never returns responses containing a formatted body (e.g a JSON response). + // + // As such, all errors - even errors indicating an invalid request - are returned + // as part of the standard WWW-Authenticate header, as defined by the specification. + // See https://datatracker.ietf.org/doc/html/rfc6750#section-3 for more information. + var parameters = new Dictionary(StringComparer.Ordinal); // If a realm was configured in the options, attach it to the parameters. - if (!string.IsNullOrEmpty(_options.CurrentValue.Realm)) + if (_options.CurrentValue.Realm is string { Length: > 0 } realm) { - parameters[Parameters.Realm] = _options.CurrentValue.Realm; + parameters[Parameters.Realm] = realm; } foreach (var parameter in context.Transaction.Response.GetParameters()) { // Note: the error details are only included if the error was not caused by a missing token, as recommended // by the OAuth 2.0 bearer specification: https://tools.ietf.org/html/rfc6750#section-3.1. - if (string.Equals(context.Transaction.Response.Error, Errors.MissingToken, StringComparison.Ordinal) && - (string.Equals(parameter.Key, Parameters.Error, StringComparison.Ordinal) || - string.Equals(parameter.Key, Parameters.ErrorDescription, StringComparison.Ordinal) || - string.Equals(parameter.Key, Parameters.ErrorUri, StringComparison.Ordinal))) + if (context.Transaction.Response.Error is Errors.MissingToken && + parameter.Key is Parameters.Error or + Parameters.ErrorDescription or + Parameters.ErrorUri) { continue; } @@ -525,7 +544,7 @@ public static partial class OpenIddictValidationAspNetCoreHandlers parameters[parameter.Key] = value; } - var builder = new StringBuilder(scheme); + var builder = new StringBuilder(Schemes.Bearer); foreach (var parameter in parameters) { @@ -592,58 +611,4 @@ public static partial class OpenIddictValidationAspNetCoreHandlers return default; } } - - /// - /// Contains the logic responsible for processing OpenID Connect responses that must be returned as JSON. - /// Note: this handler is not used when the OpenID Connect request is not initially handled by ASP.NET Core. - /// - public class ProcessJsonResponse : IOpenIddictValidationHandler where TContext : BaseRequestContext - { - /// - /// Gets the default descriptor definition assigned to this handler. - /// - public static OpenIddictValidationHandlerDescriptor Descriptor { get; } - = OpenIddictValidationHandlerDescriptor.CreateBuilder() - .AddFilter() - .UseSingletonHandler>() - .SetOrder(ProcessChallengeErrorResponse.Descriptor.Order + 1_000) - .SetType(OpenIddictValidationHandlerType.BuiltIn) - .Build(); - - /// - public async ValueTask HandleAsync(TContext context) - { - if (context is null) - { - throw new ArgumentNullException(nameof(context)); - } - - Debug.Assert(context.Transaction.Response is not null, SR.GetResourceString(SR.ID4007)); - - // This handler only applies to ASP.NET Core requests. If the HTTP context cannot be resolved, - // this may indicate that the request was incorrectly processed by another server stack. - var response = context.Transaction.GetHttpRequest()?.HttpContext.Response ?? - throw new InvalidOperationException(SR.GetResourceString(SR.ID0114)); - - context.Logger.LogInformation(SR.GetResourceString(SR.ID6142), context.Transaction.Response); - - using var stream = new MemoryStream(); - using var writer = new Utf8JsonWriter(stream, new JsonWriterOptions - { - Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, - Indented = true - }); - - context.Transaction.Response.WriteTo(writer); - writer.Flush(); - - response.ContentLength = stream.Length; - response.ContentType = "application/json;charset=UTF-8"; - - stream.Seek(offset: 0, loc: SeekOrigin.Begin); - await stream.CopyToAsync(response.Body, 4096, response.HttpContext.RequestAborted); - - context.HandleRequest(); - } - } } diff --git a/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandlers.cs b/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandlers.cs index 28433e01..6ce97fd0 100644 --- a/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandlers.cs +++ b/src/OpenIddict.Validation.Owin/OpenIddictValidationOwinHandlers.cs @@ -43,16 +43,21 @@ public static partial class OpenIddictValidationOwinHandlers * Response processing: */ AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachCacheControlHeader.Descriptor, AttachWwwAuthenticateHeader.Descriptor, ProcessChallengeErrorResponse.Descriptor, - ProcessJsonResponse.Descriptor, AttachHttpResponseCode.Descriptor, + AttachOwinResponseChallenge.Descriptor, AttachCacheControlHeader.Descriptor, AttachWwwAuthenticateHeader.Descriptor, ProcessChallengeErrorResponse.Descriptor, - ProcessJsonResponse.Descriptor); + + /* + * Error processing: + */ + AttachErrorParameters.Descriptor); /// /// Contains the logic responsible for infering the default issuer from the HTTP request host and validating it. @@ -289,7 +294,7 @@ public static partial class OpenIddictValidationOwinHandlers = OpenIddictValidationHandlerDescriptor.CreateBuilder() .AddFilter() .UseSingletonHandler() - .SetOrder(int.MinValue + 50_000) + .SetOrder(AttachDefaultChallengeError.Descriptor.Order - 500) .SetType(OpenIddictValidationHandlerType.BuiltIn) .Build(); @@ -302,36 +307,18 @@ public static partial class OpenIddictValidationOwinHandlers } var properties = context.Transaction.GetProperty(typeof(AuthenticationProperties).FullName!); - if (properties is null) - { - return default; - } - - if (properties.Dictionary.TryGetValue(Properties.Error, out string? error) && - !string.IsNullOrEmpty(error)) - { - context.Parameters[Parameters.Error] = error; - } - - if (properties.Dictionary.TryGetValue(Properties.ErrorDescription, out string? description) && - !string.IsNullOrEmpty(description)) - { - context.Parameters[Parameters.ErrorDescription] = description; - } - - if (properties.Dictionary.TryGetValue(Properties.ErrorUri, out string? uri) && - !string.IsNullOrEmpty(uri)) + if (properties is not null) { - context.Parameters[Parameters.ErrorUri] = uri; - } - - if (properties.Dictionary.TryGetValue(Properties.Scope, out string? scope) && - !string.IsNullOrEmpty(scope)) - { - context.Parameters[Parameters.Scope] = scope; + context.Response.Error = GetProperty(properties, Properties.Error); + context.Response.ErrorDescription = GetProperty(properties, Properties.ErrorDescription); + context.Response.ErrorUri = GetProperty(properties, Properties.ErrorUri); + context.Response.Scope = GetProperty(properties, Properties.Scope); } return default; + + static string? GetProperty(AuthenticationProperties properties, string name) + => properties.Dictionary.TryGetValue(name, out string? value) ? value : null; } } @@ -369,12 +356,15 @@ public static partial class OpenIddictValidationOwinHandlers response.StatusCode = context.Transaction.Response.Error switch { - null => 200, + // Note: the default code may be replaced by another handler (e.g when doing redirects). + null or { Length: 0 } => 200, Errors.InvalidToken or Errors.MissingToken => 401, Errors.InsufficientAccess or Errors.InsufficientScope => 403, + Errors.ServerError => 500, + _ => 400 }; @@ -382,6 +372,58 @@ public static partial class OpenIddictValidationOwinHandlers } } + /// + /// Contains the logic responsible for attaching an OWIN response chalenge to the context, if necessary. + /// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN. + /// + public class AttachOwinResponseChallenge : IOpenIddictValidationHandler where TContext : BaseRequestContext + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictValidationHandlerDescriptor Descriptor { get; } + = OpenIddictValidationHandlerDescriptor.CreateBuilder() + .AddFilter() + .UseSingletonHandler>() + .SetOrder(AttachHttpResponseCode.Descriptor.Order + 1_000) + .SetType(OpenIddictValidationHandlerType.BuiltIn) + .Build(); + + /// + public ValueTask HandleAsync(TContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + // This handler only applies to OWIN requests. If The OWIN request cannot be resolved, + // this may indicate that the request was incorrectly processed by another server stack. + var response = context.Transaction.GetOwinRequest()?.Context.Response ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0120)); + + // OWIN authentication middleware configured to use active authentication (which is the default mode) + // are known to aggressively intercept 401 responses even if the request is already considered fully + // handled. In practice, this behavior is often seen with the cookies authentication middleware, + // that will rewrite the 401 responses returned by OpenIddict and try to redirect the user agent + // to the login page configured in the options. To prevent this undesirable behavior, a fake + // response challenge pointing to a non-existent middleware is manually added to the OWIN context + // to prevent the active authentication middleware from rewriting OpenIddict's 401 HTTP responses. + // + // Note: while 403 responses are generally not intercepted by the built-in OWIN authentication + // middleware, they are treated the same way as 401 responses to account for custom middleware + // that may potentially use the same interception logic for both 401 and 403 HTTP responses. + if (response.StatusCode is 401 or 403 && + response.Context.Authentication.AuthenticationResponseChallenge is null) + { + response.Context.Authentication.AuthenticationResponseChallenge = + new AuthenticationResponseChallenge(new[] { Guid.NewGuid().ToString() }, null); + } + + return default; + } + } + /// /// Contains the logic responsible for attaching the appropriate HTTP response cache headers. /// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN. @@ -395,7 +437,7 @@ public static partial class OpenIddictValidationOwinHandlers = OpenIddictValidationHandlerDescriptor.CreateBuilder() .AddFilter() .UseSingletonHandler>() - .SetOrder(AttachHttpResponseCode.Descriptor.Order + 1_000) + .SetOrder(AttachOwinResponseChallenge.Descriptor.Order + 1_000) .SetType(OpenIddictValidationHandlerType.BuiltIn) .Build(); @@ -463,37 +505,29 @@ public static partial class OpenIddictValidationOwinHandlers return default; } - var scheme = context.Transaction.Response.Error switch - { - Errors.InvalidToken or - Errors.MissingToken or - Errors.InsufficientAccess or - Errors.InsufficientScope => Schemes.Bearer, - - _ => null - }; - - if (string.IsNullOrEmpty(scheme)) - { - return default; - } + // Note: unlike the server stack, the validation stack doesn't expose any endpoint + // and thus never returns responses containing a formatted body (e.g a JSON response). + // + // As such, all errors - even errors indicating an invalid request - are returned + // as part of the standard WWW-Authenticate header, as defined by the specification. + // See https://datatracker.ietf.org/doc/html/rfc6750#section-3 for more information. var parameters = new Dictionary(StringComparer.Ordinal); // If a realm was configured in the options, attach it to the parameters. - if (!string.IsNullOrEmpty(_options.CurrentValue.Realm)) + if (_options.CurrentValue.Realm is string { Length: > 0 } realm) { - parameters[Parameters.Realm] = _options.CurrentValue.Realm; + parameters[Parameters.Realm] = realm; } foreach (var parameter in context.Transaction.Response.GetParameters()) { // Note: the error details are only included if the error was not caused by a missing token, as recommended // by the OAuth 2.0 bearer specification: https://tools.ietf.org/html/rfc6750#section-3.1. - if (string.Equals(context.Transaction.Response.Error, Errors.MissingToken, StringComparison.Ordinal) && - (string.Equals(parameter.Key, Parameters.Error, StringComparison.Ordinal) || - string.Equals(parameter.Key, Parameters.ErrorDescription, StringComparison.Ordinal) || - string.Equals(parameter.Key, Parameters.ErrorUri, StringComparison.Ordinal))) + if (context.Transaction.Response.Error is Errors.MissingToken && + parameter.Key is Parameters.Error or + Parameters.ErrorDescription or + Parameters.ErrorUri) { continue; } @@ -508,7 +542,7 @@ public static partial class OpenIddictValidationOwinHandlers parameters[parameter.Key] = value; } - var builder = new StringBuilder(scheme); + var builder = new StringBuilder(Schemes.Bearer); foreach (var parameter in parameters) { @@ -575,58 +609,4 @@ public static partial class OpenIddictValidationOwinHandlers return default; } } - - /// - /// Contains the logic responsible for processing OpenID Connect responses that must be returned as JSON. - /// Note: this handler is not used when the OpenID Connect request is not initially handled by OWIN. - /// - public class ProcessJsonResponse : IOpenIddictValidationHandler where TContext : BaseRequestContext - { - /// - /// Gets the default descriptor definition assigned to this handler. - /// - public static OpenIddictValidationHandlerDescriptor Descriptor { get; } - = OpenIddictValidationHandlerDescriptor.CreateBuilder() - .AddFilter() - .UseSingletonHandler>() - .SetOrder(ProcessChallengeErrorResponse.Descriptor.Order + 1_000) - .SetType(OpenIddictValidationHandlerType.BuiltIn) - .Build(); - - /// - public async ValueTask HandleAsync(TContext context) - { - if (context is null) - { - throw new ArgumentNullException(nameof(context)); - } - - Debug.Assert(context.Transaction.Response is not null, SR.GetResourceString(SR.ID4007)); - - // This handler only applies to OWIN requests. If The OWIN request cannot be resolved, - // this may indicate that the request was incorrectly processed by another server stack. - var response = context.Transaction.GetOwinRequest()?.Context.Response ?? - throw new InvalidOperationException(SR.GetResourceString(SR.ID0120)); - - context.Logger.LogInformation(SR.GetResourceString(SR.ID6142), context.Transaction.Response); - - using var stream = new MemoryStream(); - using var writer = new Utf8JsonWriter(stream, new JsonWriterOptions - { - Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, - Indented = true - }); - - context.Transaction.Response.WriteTo(writer); - writer.Flush(); - - response.ContentLength = stream.Length; - response.ContentType = "application/json;charset=UTF-8"; - - stream.Seek(offset: 0, loc: SeekOrigin.Begin); - await stream.CopyToAsync(response.Body, 4096, response.Context.Request.CallCancelled); - - context.HandleRequest(); - } - } } diff --git a/src/OpenIddict.Validation/OpenIddictValidationHandlers.cs b/src/OpenIddict.Validation/OpenIddictValidationHandlers.cs index 6662d0da..d959443b 100644 --- a/src/OpenIddict.Validation/OpenIddictValidationHandlers.cs +++ b/src/OpenIddict.Validation/OpenIddictValidationHandlers.cs @@ -24,7 +24,8 @@ public static partial class OpenIddictValidationHandlers /* * Challenge processing: */ - AttachDefaultChallengeError.Descriptor) + AttachDefaultChallengeError.Descriptor, + AttachChallengeParameters.Descriptor) .AddRange(Discovery.DefaultHandlers) .AddRange(Introspection.DefaultHandlers) @@ -95,11 +96,7 @@ public static partial class OpenIddictValidationHandlers { // The validation handler is responsible for validating access tokens for endpoints // it doesn't manage (typically, API endpoints using token authentication). - // - // As such, sending an access token is not mandatory: API endpoints that require - // authentication can set up an authorization policy to reject such requests later - // in the request processing pipeline (typically, via the authorization middleware). - OpenIddictValidationEndpointType.Unknown => (true, false, true), + OpenIddictValidationEndpointType.Unknown => (true, true, true), _ => (false, false, false) }; @@ -123,7 +120,7 @@ public static partial class OpenIddictValidationHandlers /// public static OpenIddictValidationHandlerDescriptor Descriptor { get; } = OpenIddictValidationHandlerDescriptor.CreateBuilder() - .UseSingletonHandler() + .UseSingletonHandler() .SetOrder(EvaluateValidatedTokens.Descriptor.Order + 1_000) .SetType(OpenIddictValidationHandlerType.BuiltIn) .Build(); @@ -240,14 +237,6 @@ public static partial class OpenIddictValidationHandlers throw new ArgumentNullException(nameof(context)); } - // If an error was explicitly set by the application, don't override it. - if (!string.IsNullOrEmpty(context.Response.Error) || - !string.IsNullOrEmpty(context.Response.ErrorDescription) || - !string.IsNullOrEmpty(context.Response.ErrorUri)) - { - return default; - } - // Try to retrieve the authentication context from the validation transaction and use // the error details returned during the authentication processing, if available. // If no error is attached to the authentication context, this likely means that @@ -258,18 +247,43 @@ public static partial class OpenIddictValidationHandlers var notification = context.Transaction.GetProperty( typeof(ProcessAuthenticationContext).FullName!); - if (!string.IsNullOrEmpty(notification?.Error)) + context.Response.Error ??= notification?.Error ?? Errors.InsufficientAccess; + context.Response.ErrorDescription ??= notification?.ErrorDescription ?? SR.GetResourceString(SR.ID2095); + context.Response.ErrorUri ??= notification?.ErrorUri ?? SR.FormatID8000(SR.ID2095); + + return default; + } + } + + /// + /// Contains the logic responsible for attaching the appropriate parameters to the challenge response. + /// + public class AttachChallengeParameters : IOpenIddictValidationHandler + { + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictValidationHandlerDescriptor Descriptor { get; } + = OpenIddictValidationHandlerDescriptor.CreateBuilder() + .UseSingletonHandler() + .SetOrder(AttachDefaultChallengeError.Descriptor.Order + 1_000) + .SetType(OpenIddictValidationHandlerType.BuiltIn) + .Build(); + + /// + public ValueTask HandleAsync(ProcessChallengeContext context) + { + if (context is null) { - context.Response.Error = notification.Error; - context.Response.ErrorDescription = notification.ErrorDescription; - context.Response.ErrorUri = notification.ErrorUri; + throw new ArgumentNullException(nameof(context)); } - else + if (context.Parameters.Count > 0) { - context.Response.Error = Errors.InsufficientAccess; - context.Response.ErrorDescription = SR.GetResourceString(SR.ID2095); - context.Response.ErrorUri = SR.FormatID8000(SR.ID2095); + foreach (var parameter in context.Parameters) + { + context.Response.SetParameter(parameter.Key, parameter.Value); + } } return default;