diff --git a/src/D2L.Security.OAuth2.WebApi/Authorization/AuthenticationAttribute.cs b/src/D2L.Security.OAuth2.WebApi/Authorization/AuthenticationAttribute.cs index 5d66c056..d7047cfc 100644 --- a/src/D2L.Security.OAuth2.WebApi/Authorization/AuthenticationAttribute.cs +++ b/src/D2L.Security.OAuth2.WebApi/Authorization/AuthenticationAttribute.cs @@ -1,5 +1,8 @@ using System; +using System.Net; +using System.Net.Http; using System.Web.Http.Controllers; +using D2L.Security.OAuth2.Authorization.Exceptions; using D2L.Security.OAuth2.Principal; namespace D2L.Security.OAuth2.Authorization { @@ -46,10 +49,24 @@ public sealed class AuthenticationAttribute : OAuth2AuthorizeAttribute { return false; case PrincipalType.User: - return m_allowUsers; + if( m_allowUsers ) { + return true; + } + + throw new OAuth2Exception( + error: OAuth2Exception.Type.invalid_token, + errorDescription: "Users are not allowed to access this API." + ); case PrincipalType.Service: - return m_allowServices; + if( m_allowServices ) { + return true; + } + + throw new OAuth2Exception( + error: OAuth2Exception.Type.invalid_token, + errorDescription: "Services are not allowed to access this API." + ); default: throw new NotImplementedException(); diff --git a/src/D2L.Security.OAuth2.WebApi/Authorization/Exceptions/InsufficientScopeException.cs b/src/D2L.Security.OAuth2.WebApi/Authorization/Exceptions/InsufficientScopeException.cs new file mode 100644 index 00000000..e9b229f4 --- /dev/null +++ b/src/D2L.Security.OAuth2.WebApi/Authorization/Exceptions/InsufficientScopeException.cs @@ -0,0 +1,18 @@ +using System; +using D2L.Security.OAuth2.Scopes; + +namespace D2L.Security.OAuth2.Authorization.Exceptions { + internal sealed class InsufficientScopeException : OAuth2Exception { + + internal InsufficientScopeException( Scope scope, Exception innerException = null ) : base( + error: OAuth2Exception.Type.insufficient_scope, + errorDescription: $"Required scope: '{ scope }'", + innerException: innerException + ) { + Scope = scope; + } + + internal Scope Scope { get; } + + } +} diff --git a/src/D2L.Security.OAuth2.WebApi/Authorization/Exceptions/OAuth2Exception.cs b/src/D2L.Security.OAuth2.WebApi/Authorization/Exceptions/OAuth2Exception.cs new file mode 100644 index 00000000..e643ab1b --- /dev/null +++ b/src/D2L.Security.OAuth2.WebApi/Authorization/Exceptions/OAuth2Exception.cs @@ -0,0 +1,32 @@ +using System; +using System.Net; + +namespace D2L.Security.OAuth2.Authorization.Exceptions { + internal class OAuth2Exception : Exception { + // Note: the naming style of these enum values matches the codes from RFC6750 + public enum Type { + invalid_request = HttpStatusCode.BadRequest, + invalid_token = HttpStatusCode.Unauthorized, + insufficient_scope = HttpStatusCode.Forbidden + } + + internal OAuth2Exception( + Type error, + string errorDescription, + Exception innerException = null + ) : base( + message: $"{ error }: { errorDescription }", + innerException: innerException + ) { + Error = error; + ErrorDescription = errorDescription; + + if( errorDescription.Contains( "\"" ) ) { + throw new ArgumentException( nameof( errorDescription ), "Must not contain '\"' character" ); + } + } + + public Type Error { get; } + public string ErrorDescription { get; } + } +} \ No newline at end of file diff --git a/src/D2L.Security.OAuth2.WebApi/Authorization/NoImpersonationAttribute.cs b/src/D2L.Security.OAuth2.WebApi/Authorization/NoImpersonationAttribute.cs index 7fb2e3fc..2a46eefb 100644 --- a/src/D2L.Security.OAuth2.WebApi/Authorization/NoImpersonationAttribute.cs +++ b/src/D2L.Security.OAuth2.WebApi/Authorization/NoImpersonationAttribute.cs @@ -1,7 +1,6 @@ using System; -using System.Net; -using System.Net.Http; using System.Web.Http.Controllers; +using D2L.Security.OAuth2.Authorization.Exceptions; using D2L.Security.OAuth2.Principal; namespace D2L.Security.OAuth2.Authorization { @@ -21,27 +20,21 @@ public sealed class NoImpersonationAttribute : OAuth2AuthorizeAttribute { .Principal as ID2LPrincipal; if( principal == null ) { - return true; + return false; } if( principal.Type != PrincipalType.User ) { return true; } - return principal.ActualUserId == principal.UserId; - } - - protected override void HandleUnauthorizedRequestInternal( - HttpActionContext actionContext - ) { - var response = actionContext - .Request - .CreateErrorResponse( - HttpStatusCode.Forbidden, - "This API is not usable while impersonating. This error message indicates a bug in the client application which is responsible for knowing this." - ); + if( principal.UserId == principal.ActualUserId ) { + return true; + } - actionContext.Response = response; + throw new OAuth2Exception( + error: OAuth2Exception.Type.invalid_token, + errorDescription: "This API is not usable while impersonating. This error message indicates a bug in the client application which is responsible for knowing this." + ); } } } diff --git a/src/D2L.Security.OAuth2.WebApi/Authorization/OAuth2AuthorizeAttribute.cs b/src/D2L.Security.OAuth2.WebApi/Authorization/OAuth2AuthorizeAttribute.cs index 4dc82bbf..7af665f8 100644 --- a/src/D2L.Security.OAuth2.WebApi/Authorization/OAuth2AuthorizeAttribute.cs +++ b/src/D2L.Security.OAuth2.WebApi/Authorization/OAuth2AuthorizeAttribute.cs @@ -1,12 +1,18 @@ using System.Linq; +using System.Net; +using System.Net.Http; +using System.Net.Http.Formatting; +using System.Web; using System.Web.Http; using System.Web.Http.Controllers; +using D2L.Security.OAuth2.Authorization.Exceptions; +using Newtonsoft.Json; namespace D2L.Security.OAuth2.Authorization { public abstract class OAuth2AuthorizeAttribute : AuthorizeAttribute { private const string AUTH_HAS_RUN = "D2L.Security.OAuth2.Authorization.OAuth2AuthorizeAttribute_Auth_Has_Run"; - private const string FAILING_ATTR_PROP = "D2L.Security.OAuth2.Authorization.OAuth2AuthorizeAttribute_Failing_Attribute"; + private const string FAILED_EXCEPTION_PROP = "D2L.Security.OAuth2.Authorization.OAuth2AuthorizeAttribute_Failed_Exception"; protected override bool IsAuthorized( HttpActionContext actionContext ) { var props = actionContext.Request.Properties; @@ -32,8 +38,12 @@ public abstract class OAuth2AuthorizeAttribute : AuthorizeAttribute { .ToArray(); foreach( var attr in oauth2Attributes ) { - if( !attr.IsAuthorizedInternal( actionContext ) ) { - props[ FAILING_ATTR_PROP ] = attr; + try { + if( !attr.IsAuthorizedInternal( actionContext ) ) { + return false; + } + } catch( OAuth2Exception e ) { + props[ FAILED_EXCEPTION_PROP ] = e; return false; } } @@ -44,9 +54,36 @@ public abstract class OAuth2AuthorizeAttribute : AuthorizeAttribute { protected override void HandleUnauthorizedRequest( HttpActionContext actionContext ) { var props = actionContext.Request.Properties; - var attr = ( OAuth2AuthorizeAttribute )props[ FAILING_ATTR_PROP ]; + if( !props.TryGetValue( FAILED_EXCEPTION_PROP, out object exceptionObj ) ) { + HandleNoAuth( actionContext ); + return; + } + + OAuth2Exception exception = exceptionObj as OAuth2Exception; + OAuth2ErrorResponse responseContent = new OAuth2ErrorResponse( + error: exception.Error.ToString(), + errorDescription: exception.ErrorDescription + ); + string authenticateHeader = $"Bearer error=\"{ responseContent.Error }\", error_description=\"{ responseContent.ErrorDescription }\""; + + if( exception is InsufficientScopeException insufficientScopeException ) { + responseContent.Scope = insufficientScopeException.Scope.ToString(); + authenticateHeader += $", scope=\"{ responseContent.Scope }\""; + } - attr.HandleUnauthorizedRequestInternal( actionContext ); + var response = new HttpResponseMessage( (HttpStatusCode)exception.Error ) { + Content = new ObjectContent( + responseContent, + new JsonMediaTypeFormatter() { + SerializerSettings = new JsonSerializerSettings() { + NullValueHandling = NullValueHandling.Ignore + } + } + ) + }; + response.Headers.Add( "WWW-Authenticate", authenticateHeader ); + + actionContext.Response = response; } protected virtual void HandleUnauthorizedRequestInternal( HttpActionContext actionContext ) { @@ -57,5 +94,12 @@ public abstract class OAuth2AuthorizeAttribute : AuthorizeAttribute { protected abstract bool IsAuthorizedInternal( HttpActionContext actionContext ); + private static void HandleNoAuth( HttpActionContext actionContext ) { + var response = new HttpResponseMessage( HttpStatusCode.Unauthorized ); + response.Headers.Add( "WWW-Authenticate", "Bearer" ); + + actionContext.Response = response; + } + } } diff --git a/src/D2L.Security.OAuth2.WebApi/Authorization/OAuth2ErrorResponse.cs b/src/D2L.Security.OAuth2.WebApi/Authorization/OAuth2ErrorResponse.cs new file mode 100644 index 00000000..5a7bb28d --- /dev/null +++ b/src/D2L.Security.OAuth2.WebApi/Authorization/OAuth2ErrorResponse.cs @@ -0,0 +1,24 @@ +using Newtonsoft.Json; + +namespace D2L.Security.OAuth2.Authorization { + internal sealed class OAuth2ErrorResponse { + + public OAuth2ErrorResponse( + string error, + string errorDescription + ) { + Error = error; + ErrorDescription = errorDescription; + } + + [JsonProperty( "error", Required = Required.Always )] + public string Error { get; } + + [JsonProperty( "error_description", Required = Required.Always )] + public string ErrorDescription { get; } + + [JsonProperty( "scope" )] + public string Scope { get; set; } + + } +} diff --git a/src/D2L.Security.OAuth2.WebApi/Authorization/RequireClaimAttribute.cs b/src/D2L.Security.OAuth2.WebApi/Authorization/RequireClaimAttribute.cs index efd445fb..ccba9e44 100644 --- a/src/D2L.Security.OAuth2.WebApi/Authorization/RequireClaimAttribute.cs +++ b/src/D2L.Security.OAuth2.WebApi/Authorization/RequireClaimAttribute.cs @@ -1,7 +1,6 @@ using System; -using System.Net; -using System.Net.Http; using System.Web.Http.Controllers; +using D2L.Security.OAuth2.Authorization.Exceptions; using D2L.Security.OAuth2.Principal; using D2L.Services; @@ -30,25 +29,14 @@ public sealed class RequireClaimAttribute : OAuth2AuthorizeAttribute { return false; } - bool hasClaim = principal - .AccessToken - .Claims - .HasClaim( m_claimType ); - - return hasClaim; - } - - protected override void HandleUnauthorizedRequestInternal( - HttpActionContext actionContext - ) { - var response = actionContext - .Request - .CreateErrorResponse( - HttpStatusCode.Forbidden, - String.Format( "Missing claim: '{0}'", m_claimType ) - ); + if( principal.AccessToken.Claims.HasClaim( m_claimType ) ) { + return true; + } - actionContext.Response = response; + throw new OAuth2Exception( + error: OAuth2Exception.Type.invalid_token, + errorDescription: $"Missing claim: '{ m_claimType }'" + ); } } } diff --git a/src/D2L.Security.OAuth2.WebApi/Authorization/RequireScopeAttribute.cs b/src/D2L.Security.OAuth2.WebApi/Authorization/RequireScopeAttribute.cs index c41cf58d..aee935de 100644 --- a/src/D2L.Security.OAuth2.WebApi/Authorization/RequireScopeAttribute.cs +++ b/src/D2L.Security.OAuth2.WebApi/Authorization/RequireScopeAttribute.cs @@ -1,7 +1,6 @@ using System; -using System.Net; -using System.Net.Http; using System.Web.Http.Controllers; +using D2L.Security.OAuth2.Authorization.Exceptions; using D2L.Security.OAuth2.Principal; using D2L.Security.OAuth2.Scopes; @@ -39,16 +38,11 @@ string permission return false; } - bool isAuthorized = ScopeAuthorizer.IsAuthorized( principal.Scopes, m_requiredScope ); - - return isAuthorized; - } - - protected override void HandleUnauthorizedRequestInternal( HttpActionContext actionContext ) { - var response = actionContext.Request.CreateErrorResponse( HttpStatusCode.Forbidden, "insufficient_scope" ); - response.Headers.Add( "WWW-Authenticate", "Bearer error=\"insufficient_scope\"" ); + if( ScopeAuthorizer.IsAuthorized( principal.Scopes, m_requiredScope ) ) { + return true; + } - actionContext.Response = response; + throw new InsufficientScopeException( m_requiredScope ); } } } diff --git a/src/D2L.Security.OAuth2.WebApi/D2L.Security.OAuth2.WebApi.csproj b/src/D2L.Security.OAuth2.WebApi/D2L.Security.OAuth2.WebApi.csproj index 368e52fa..aa56a7bc 100644 --- a/src/D2L.Security.OAuth2.WebApi/D2L.Security.OAuth2.WebApi.csproj +++ b/src/D2L.Security.OAuth2.WebApi/D2L.Security.OAuth2.WebApi.csproj @@ -87,9 +87,12 @@ + + + diff --git a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/DefaultAuthorizationAttributeTests.cs b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/DefaultAuthorizationAttributeTests.cs index 2efa3f3b..60f6f68d 100644 --- a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/DefaultAuthorizationAttributeTests.cs +++ b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/DefaultAuthorizationAttributeTests.cs @@ -24,7 +24,7 @@ await TestUtilities.RunBasicAuthTest( "/authorization/unspecifiedprincipaltype", [Test] - public async Task Basic_NoAuthentication_403() { + public async Task Basic_NoAuthentication_401() { await TestUtilities.RunBasicAuthTest( "/authorization/basic", HttpStatusCode.Unauthorized ) .SafeAsync(); } diff --git a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/NoImpersonationAttributeTests.cs b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/NoImpersonationAttributeTests.cs index 28bf8543..5e880288 100644 --- a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/NoImpersonationAttributeTests.cs +++ b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/NoImpersonationAttributeTests.cs @@ -29,14 +29,24 @@ await TestUtilities } [Test] - public async Task ImpersonationToken_Forbidden() { + public async Task ImpersonationToken_Unauthorized() { string jwt = await TestUtilities .GetAccessTokenValidForAMinute( userId: 1234, actualUserId: 1235 ) .SafeAsync(); - await TestUtilities - .RunBasicAuthTest( "/authorization/imp", jwt, HttpStatusCode.Forbidden ) + var response = await TestUtilities + .RunBasicAuthTest( "/authorization/imp", jwt, HttpStatusCode.Unauthorized ) .SafeAsync(); + + string expectedError = "invalid_token"; + string expectedErrorDescription = "This API is not usable while impersonating. This error message indicates a bug in the client application which is responsible for knowing this."; + + Assert.AreEqual( expectedError, response.Body.Error ); + Assert.AreEqual( expectedErrorDescription, response.Body.ErrorDescription ); + + string challengeHeader = response.Headers.WwwAuthenticate.ToString(); + StringAssert.Contains( expectedError, challengeHeader ); + StringAssert.Contains( expectedErrorDescription, challengeHeader ); } } } diff --git a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/RequireClaimAttributeTests.cs b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/RequireClaimAttributeTests.cs index 64f62c47..5f6d7f1c 100644 --- a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/RequireClaimAttributeTests.cs +++ b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/RequireClaimAttributeTests.cs @@ -15,16 +15,24 @@ await TestUtilities } [Test] - public async Task ServiceToken_Forbidden() { + public async Task ServiceToken_Unauthorized() { string jwt = await TestUtilities .GetAccessTokenValidForAMinute( userId: null ) .SafeAsync(); - string body = await TestUtilities - .RunBasicAuthTest( RequireClaimAttributeTestsController.ROUTE, jwt, HttpStatusCode.Forbidden ) + var response = await TestUtilities + .RunBasicAuthTest( RequireClaimAttributeTestsController.ROUTE, jwt, HttpStatusCode.Unauthorized ) .SafeAsync(); - StringAssert.Contains( Constants.Claims.USER_ID, body ); + string expectedError = "invalid_token"; + string expectedErrorDescription = $"Missing claim: '{ Constants.Claims.USER_ID }'"; + + Assert.AreEqual( expectedError, response.Body.Error ); + Assert.AreEqual( expectedErrorDescription, response.Body.ErrorDescription ); + + string challengeHeader = response.Headers.WwwAuthenticate.ToString(); + StringAssert.Contains( expectedError, challengeHeader ); + StringAssert.Contains( expectedErrorDescription, challengeHeader ); } [Test] diff --git a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/RequireScopeAttributeTests.cs b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/RequireScopeAttributeTests.cs new file mode 100644 index 00000000..d1275951 --- /dev/null +++ b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/Authorization/RequireScopeAttributeTests.cs @@ -0,0 +1,43 @@ +using System.Net; +using System.Threading.Tasks; +using D2L.Security.OAuth2.TestWebService.Controllers; +using D2L.Services; +using NUnit.Framework; + +namespace D2L.Security.OAuth2.Authorization { + [TestFixture] + internal sealed class RequireScopeAttributeTests { + + [Test] + public async Task Token_Scope_XYZ_403() { + string jwt = await TestUtilities + .GetAccessTokenValidForAMinute( userId: 123, scope: "x:y:z" ) + .SafeAsync(); + + var response = await TestUtilities + .RunBasicAuthTest( RequireScopeAttributeTestsController.ROUTE, jwt, HttpStatusCode.Forbidden ) + .SafeAsync(); + + string expectedError = "insufficient_scope"; + string expectedErrorDescription = $"Required scope: 'a:b:c'"; + + Assert.AreEqual( expectedError, response.Body.Error ); + Assert.AreEqual( expectedErrorDescription, response.Body.ErrorDescription ); + + string challengeHeader = response.Headers.WwwAuthenticate.ToString(); + StringAssert.Contains( expectedError, challengeHeader ); + StringAssert.Contains( expectedErrorDescription, challengeHeader ); + } + + [Test] + public async Task Token_Scope_ABC_Okay() { + string jwt = await TestUtilities + .GetAccessTokenValidForAMinute( userId: 123, scope: "a:b:c" ) + .SafeAsync(); + + await TestUtilities + .RunBasicAuthTest( RequireScopeAttributeTestsController.ROUTE, jwt, HttpStatusCode.NoContent ) + .SafeAsync(); + } + } +} \ No newline at end of file diff --git a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/D2L.Security.OAuth2.WebApi.IntegrationTests.csproj b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/D2L.Security.OAuth2.WebApi.IntegrationTests.csproj index 19489040..5cc33a6e 100644 --- a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/D2L.Security.OAuth2.WebApi.IntegrationTests.csproj +++ b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/D2L.Security.OAuth2.WebApi.IntegrationTests.csproj @@ -105,6 +105,7 @@ + @@ -112,6 +113,7 @@ + diff --git a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/TestUtilities.cs b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/TestUtilities.cs index 9267b73f..4e6c2f3a 100644 --- a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/TestUtilities.cs +++ b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/TestUtilities.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; using System.Net; using System.Net.Http; using System.Net.Http.Headers; @@ -10,6 +11,7 @@ using D2L.Security.OAuth2.Validation.AccessTokens; using D2L.Security.OAuth2.Validation.Request; using D2L.Services; +using Newtonsoft.Json; using NUnit.Framework; namespace D2L.Security.OAuth2 { @@ -67,11 +69,15 @@ internal static class TestUtilities { ).SafeAsync(); } - public static Task RunBasicAuthTest( string route, HttpStatusCode expectedStatusCode ) { + public static Task RunBasicAuthTest( string route, HttpStatusCode expectedStatusCode ) { return RunBasicAuthTest( route, null, expectedStatusCode ); } - public static async Task RunBasicAuthTest( string route, string jwt, HttpStatusCode expectedStatusCode ) { + public static Task RunBasicAuthTest( string route, string jwt, HttpStatusCode expectedStatusCode ) { + return RunBasicAuthTest( route, jwt, expectedStatusCode ); + } + + public static async Task<(T Body, HttpResponseHeaders Headers)> RunBasicAuthTest( string route, string jwt, HttpStatusCode expectedStatusCode ) { using( var client = SetUpFixture.GetHttpClient() ) { var req = new HttpRequestMessage(); @@ -90,7 +96,7 @@ internal static class TestUtilities { .ReadAsStringAsync() .SafeAsync(); - return body; + return (JsonConvert.DeserializeObject( body ), resp.Headers); } } } diff --git a/test/D2L.Security.OAuth2.WebApi.IntegrationTests/TestWebService/Controllers/RequireScopeAttributeTestsController.cs b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/TestWebService/Controllers/RequireScopeAttributeTestsController.cs new file mode 100644 index 00000000..bc28a962 --- /dev/null +++ b/test/D2L.Security.OAuth2.WebApi.IntegrationTests/TestWebService/Controllers/RequireScopeAttributeTestsController.cs @@ -0,0 +1,13 @@ +using System.Web.Http; +using D2L.Security.OAuth2.Authorization; + +namespace D2L.Security.OAuth2.TestWebService.Controllers { + [Authentication( users: true, services: true )] + public sealed class RequireScopeAttributeTestsController : ApiController { + internal const string ROUTE = "authorization/requirescope"; + [HttpGet] + [Route( ROUTE )] + [RequireScope( "a", "b", "c" )] + public void RequireScope() { } + } +}