Optimization: reject grant_type=authorization_code requests that don'…

…t specify a redirect_uri directly from ValidateTokenRequest

src/OpenIddict/OpenIddictProvider.Exchange.cs

@@ -50,6 +50,20 @@ public override async Task ValidateTokenRequest([NotNull] ValidateTokenRequestCo
                 return;
             }
 
+            // Optimization: the OpenID Connect server middleware automatically rejects grant_type=authorization_code
+            // requests missing the redirect_uri parameter if one was specified in the initial authorization request.
+            // Since OpenIddict doesn't allow redirect_uri-less authorization requests, an earlier check can be made here,
+            // which saves the OpenID Connect server middleware from having to deserialize the authorization code ticket.
+            // See http://openid.net/specs/openid-connect-core-1_0.html#TokenRequestValidation for more information.
+            if (context.Request.IsAuthorizationCodeGrantType() && string.IsNullOrEmpty(context.Request.RedirectUri))
+            {
+                context.Reject(
+                    error: OpenIdConnectConstants.Errors.InvalidRequest,
+                    description: "The mandatory 'redirect_uri' parameter was missing.");
+
+                return;
+            }
+
             // Note: the OpenID Connect server middleware allows returning a refresh token with grant_type=client_credentials,
             // though it's usually not recommended by the OAuth2 specification. To encourage developers to make a new
             // grant_type=client_credentials request instead of using refresh tokens, OpenIddict uses a stricter policy
@@ -65,28 +79,19 @@ public override async Task ValidateTokenRequest([NotNull] ValidateTokenRequestCo
                 return;
             }
 
-            // Note: the OpenID Connect server middleware rejects grant_type=client_credentials requests
-            // when validation is skipped but an early check is made here to avoid making unnecessary
+            // Optimization: the OpenID Connect server middleware automatically rejects grant_type=client_credentials
+            // requests when validation is skipped but an earlier check is made here to avoid making unnecessary
             // database roundtrips to retrieve the client application corresponding to the client_id.
             if (context.Request.IsClientCredentialsGrantType() && (string.IsNullOrEmpty(context.Request.ClientId) ||
                                                                    string.IsNullOrEmpty(context.Request.ClientSecret)))
             {
-                logger.LogError("The token request was rejected because the client credentials were missing.");
-
                 context.Reject(
                     error: OpenIdConnectConstants.Errors.InvalidRequest,
                     description: "Client applications must be authenticated to use the client credentials grant.");
 
                 return;
             }
 
-            // Note: though required by the OpenID Connect specification for the refresh token grant,
-            // client authentication is not mandatory for non-confidential client applications in OAuth2.
-            // To avoid breaking OAuth2 scenarios, OpenIddict uses a relaxed policy that allows
-            // public applications to use the refresh token grant without having to authenticate.
-            // See http://openid.net/specs/openid-connect-core-1_0.html#RefreshingAccessToken
-            // and https://tools.ietf.org/html/rfc6749#section-6 for more information.
-
             // At this stage, skip client authentication if the client identifier is missing
             // or reject the token request if client identification is set as required.
             // Note: the OpenID Connect server middleware will automatically ensure that
@@ -97,18 +102,15 @@ public override async Task ValidateTokenRequest([NotNull] ValidateTokenRequestCo
                 // Reject the request if client identification is mandatory.
                 if (options.Value.RequireClientIdentification)
                 {
-                    logger.LogError("The token request was rejected becaused the " +
-                                    "mandatory client_id parameter was missing or empty.");
-
                     context.Reject(
                         error: OpenIdConnectConstants.Errors.InvalidRequest,
                         description: "The mandatory 'client_id' parameter was missing.");
 
                     return;
                 }
 
-                logger.LogInformation("The token request validation process was skipped " +
-                                      "because the client_id parameter was missing or empty.");
+                logger.LogDebug("The token request validation process was partially skipped " +
+                                "because the 'client_id' parameter was missing or empty.");
 
                 context.Skip();
 

test/OpenIddict.Tests/OpenIddictProviderTests.Exchange.cs

@@ -74,6 +74,28 @@ public async Task ValidateTokenRequest_RequestWithOfflineAccessScopeIsRejectedWh
             Assert.Equal("The 'offline_access' scope is not allowed.", response.ErrorDescription);
         }
 
+        [Fact]
+        public async Task ValidateTokenRequest_AuthorizationCodeRequestIsRejectedWhenRedirectUriIsMissing()
+        {
+            // Arrange
+            var server = CreateAuthorizationServer();
+
+            var client = new OpenIdConnectClient(server.CreateClient());
+
+            // Act
+            var response = await client.PostAsync(TokenEndpoint, new OpenIdConnectRequest
+            {
+                ClientId = "Fabrikam",
+                Code = "SplxlOBeZQQYbYS6WxSbIA",
+                GrantType = OpenIdConnectConstants.GrantTypes.AuthorizationCode,
+                RedirectUri = null
+            });
+
+            // Assert
+            Assert.Equal(OpenIdConnectConstants.Errors.InvalidRequest, response.Error);
+            Assert.Equal("The mandatory 'redirect_uri' parameter was missing.", response.ErrorDescription);
+        }
+
         [Fact]
         public async Task ValidateTokenRequest_ClientCredentialsRequestWithOfflineAccessScopeIsRejected()
         {
@@ -378,7 +400,8 @@ public async Task HandleTokenRequest_AuthorizationCodeRevocationIsIgnoredWhenTok
             {
                 ClientId = "Fabrikam",
                 Code = "SplxlOBeZQQYbYS6WxSbIA",
-                GrantType = OpenIdConnectConstants.GrantTypes.AuthorizationCode
+                GrantType = OpenIdConnectConstants.GrantTypes.AuthorizationCode,
+                RedirectUri = "http://www.fabrikam.com/path"
             });
 
             // Assert
@@ -483,7 +506,8 @@ public async Task HandleTokenRequest_RequestIsRejectedWhenAuthorizationCodeIsExp
             {
                 ClientId = "Fabrikam",
                 Code = "SplxlOBeZQQYbYS6WxSbIA",
-                GrantType = OpenIdConnectConstants.GrantTypes.AuthorizationCode
+                GrantType = OpenIdConnectConstants.GrantTypes.AuthorizationCode,
+                RedirectUri = "http://www.fabrikam.com/path"
             });
 
             // Assert
@@ -604,7 +628,8 @@ public async Task HandleTokenRequest_AuthorizationCodeIsAutomaticallyRevoked()
             {
                 ClientId = "Fabrikam",
                 Code = "SplxlOBeZQQYbYS6WxSbIA",
-                GrantType = OpenIdConnectConstants.GrantTypes.AuthorizationCode
+                GrantType = OpenIdConnectConstants.GrantTypes.AuthorizationCode,
+                RedirectUri = "http://www.fabrikam.com/path"
             });
 
             // Assert
@@ -749,6 +774,7 @@ public async Task HandleTokenRequest_RequestsAreNotHandledLocally(string flow)
                 ClientSecret = "7Fjfp0ZBr1KtDRbnfVdmIw",
                 Code = "8xLOxBtZp8",
                 GrantType = flow,
+                RedirectUri = "http://www.fabrikam.com/path",
                 RefreshToken = "8xLOxBtZp8",
                 Username = "johndoe",
                 Password = "A3ddj3w"