Update the non-interactive endpoints to return a 401 response for invalid client credentials when basic authentication is used

Author: kevinchalet

build/dependencies.props

@@ -2,7 +2,7 @@
 
   <PropertyGroup Label="Package Versions">
     <AngleSharpVersion>0.9.9</AngleSharpVersion>
-    <AspNetContribOpenIdExtensionsVersion>2.0.0-rc2-final</AspNetContribOpenIdExtensionsVersion>
+    <AspNetContribOpenIdExtensionsVersion>2.0.0-rc3-final</AspNetContribOpenIdExtensionsVersion>
     <AspNetCoreVersion>2.0.0</AspNetCoreVersion>
     <CoreFxVersion>4.4.0</CoreFxVersion>
     <EntityFrameworkVersion>6.1.3</EntityFrameworkVersion>

src/AspNet.Security.OpenIdConnect.Server/OpenIdConnectServerHandler.Userinfo.cs

@@ -191,14 +191,9 @@ private async Task<bool> InvokeUserinfoEndpointAsync()
             {
                 Logger.LogError("The userinfo request was rejected because the access token was invalid.");
 
-                // Note: an invalid token should result in an unauthorized response
-                // but returning a 401 status would invoke the previously registered
-                // authentication middleware and potentially replace it by a 302 response.
-                // To work around this limitation, a 400 error is returned instead.
-                // See http://openid.net/specs/openid-connect-core-1_0.html#UserInfoError
                 return await SendUserinfoResponseAsync(new OpenIdConnectResponse
                 {
-                    Error = OpenIdConnectConstants.Errors.InvalidGrant,
+                    Error = OpenIdConnectConstants.Errors.InvalidToken,
                     ErrorDescription = "The specified access token is not valid."
                 });
             }
@@ -208,14 +203,9 @@ private async Task<bool> InvokeUserinfoEndpointAsync()
             {
                 Logger.LogError("The userinfo request was rejected because the access token was expired.");
 
-                // Note: an invalid token should result in an unauthorized response
-                // but returning a 401 status would invoke the previously registered
-                // authentication middleware and potentially replace it by a 302 response.
-                // To work around this limitation, a 400 error is returned instead.
-                // See http://openid.net/specs/openid-connect-core-1_0.html#UserInfoError
                 return await SendUserinfoResponseAsync(new OpenIdConnectResponse
                 {
-                    Error = OpenIdConnectConstants.Errors.InvalidGrant,
+                    Error = OpenIdConnectConstants.Errors.InvalidToken,
                     ErrorDescription = "The specified access token is no longer valid."
                 });
             }

src/AspNet.Security.OpenIdConnect.Server/OpenIdConnectServerHandler.cs

@@ -8,6 +8,7 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Security.Claims;
+using System.Text;
 using System.Text.Encodings.Web;
 using System.Threading.Tasks;
 using AspNet.Security.OpenIdConnect.Extensions;
@@ -17,6 +18,7 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
+using Microsoft.Extensions.Primitives;
 using Microsoft.Net.Http.Headers;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
@@ -736,7 +738,42 @@ private async Task<bool> SendPayloadAsync(OpenIdConnectResponse response)
 
                 if (!string.IsNullOrEmpty(response.Error))
                 {
-                    Response.StatusCode = 400;
+                    // 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.
+                    string GetAuthenticationScheme()
+                    {
+                        switch (response.Error)
+                        {
+                            case OpenIdConnectConstants.Errors.InvalidClient: return OpenIdConnectConstants.Schemes.Basic;
+                            case OpenIdConnectConstants.Errors.InvalidToken:  return OpenIdConnectConstants.Schemes.Bearer;
+                            default:                                          return null;
+                        }
+                    }
+
+                    var scheme = GetAuthenticationScheme();
+                    if (!string.IsNullOrEmpty(scheme))
+                    {
+                        Response.StatusCode = 401;
+
+                        Response.Headers[HeaderNames.WWWAuthenticate] = new StringBuilder()
+                            .Append(scheme)
+                            .Append(' ')
+                            .Append(OpenIdConnectConstants.Parameters.Realm)
+                            .Append("=\"")
+                            .Append(Context.GetIssuer(Options))
+                            .Append('"')
+                            .ToString();
+                    }
+
+                    else
+                    {
+                        Response.StatusCode = 400;
+                    }
                 }
 
                 Response.ContentLength = buffer.Length;

src/Owin.Security.OpenIdConnect.Server/OpenIdConnectServerHandler.Userinfo.cs

@@ -189,7 +189,7 @@ private async Task<bool> InvokeUserinfoEndpointAsync()
                 // See http://openid.net/specs/openid-connect-core-1_0.html#UserInfoError
                 return await SendUserinfoResponseAsync(new OpenIdConnectResponse
                 {
-                    Error = OpenIdConnectConstants.Errors.InvalidGrant,
+                    Error = OpenIdConnectConstants.Errors.InvalidToken,
                     ErrorDescription = "The specified access token is not valid."
                 });
             }
@@ -206,7 +206,7 @@ private async Task<bool> InvokeUserinfoEndpointAsync()
                 // See http://openid.net/specs/openid-connect-core-1_0.html#UserInfoError
                 return await SendUserinfoResponseAsync(new OpenIdConnectResponse
                 {
-                    Error = OpenIdConnectConstants.Errors.InvalidGrant,
+                    Error = OpenIdConnectConstants.Errors.InvalidToken,
                     ErrorDescription = "The specified access token is no longer valid."
                 });
             }

src/Owin.Security.OpenIdConnect.Server/OpenIdConnectServerHandler.cs

@@ -745,6 +745,10 @@ private async Task<bool> SendPayloadAsync(OpenIdConnectResponse response)
 
                 writer.Flush();
 
+                // Note: when using basic authentication, returning an invalid_client error MUST result in
+                // an unauthorized response but returning a 401 status code would invoke the previously
+                // registered authentication middleware and potentially replace it by a 302 response.
+                // To work around this OWIN/Katana limitation, a 400 response code is always returned.
                 if (!string.IsNullOrEmpty(response.Error))
                 {
                     Response.StatusCode = 400;

test/AspNet.Security.OpenIdConnect.Server.Tests/OpenIdConnectServerHandlerTests.Userinfo.cs

@@ -252,7 +252,7 @@ public async Task InvokeUserinfoEndpointAsync_InvalidTokenCausesAnError()
             });
 
             // Assert
-            Assert.Equal(OpenIdConnectConstants.Errors.InvalidGrant, response.Error);
+            Assert.Equal(OpenIdConnectConstants.Errors.InvalidToken, response.Error);
             Assert.Equal("The specified access token is not valid.", response.ErrorDescription);
         }
 
@@ -286,7 +286,7 @@ public async Task InvokeUserinfoEndpointAsync_ExpiredTokenCausesAnError()
             });
 
             // Assert
-            Assert.Equal(OpenIdConnectConstants.Errors.InvalidGrant, response.Error);
+            Assert.Equal(OpenIdConnectConstants.Errors.InvalidToken, response.Error);
             Assert.Equal("The specified access token is no longer valid.", response.ErrorDescription);
         }
 

test/Owin.Security.OpenIdConnect.Server.Tests/OpenIdConnectServerHandlerTests.Userinfo.cs

@@ -250,7 +250,7 @@ public async Task InvokeUserinfoEndpointAsync_InvalidTokenCausesAnError()
             });
 
             // Assert
-            Assert.Equal(OpenIdConnectConstants.Errors.InvalidGrant, response.Error);
+            Assert.Equal(OpenIdConnectConstants.Errors.InvalidToken, response.Error);
             Assert.Equal("The specified access token is not valid.", response.ErrorDescription);
         }
 
@@ -283,7 +283,7 @@ public async Task InvokeUserinfoEndpointAsync_ExpiredTokenCausesAnError()
             });
 
             // Assert
-            Assert.Equal(OpenIdConnectConstants.Errors.InvalidGrant, response.Error);
+            Assert.Equal(OpenIdConnectConstants.Errors.InvalidToken, response.Error);
             Assert.Equal("The specified access token is no longer valid.", response.ErrorDescription);
         }