Repository: ory/fosite (go)

Issue:
Failing to fetch a PKCE request session fails requests even when PKCE is not enforced
### Preflight checklist

- [X] I could not find a solution in the existing issues, docs, nor discussions.
- [X] I agree to follow this project's [Code of Conduct](https://github.com/ory/fosite/blob/master/CODE_OF_CONDUCT.md).
- [X] I have read and am following this repository's [Contribution Guidelines](https://github.com/ory/fosite/blob/master/CONTRIBUTING.md).
- [ ] This issue affects my [Ory Network](https://www.ory.sh/) project.
- [ ] I have joined the [Ory Community Slack](https://slack.ory.sh).
- [ ] I am signed up to the [Ory Security Patch Newsletter](https://ory.us10.list-manage.com/subscribe?u=ffb1a878e4ec6c0ed312a3480&id=f605a41b53).

### Describe the bug

`GetPKCERequestSession` is always called when the `OAuth2PKCEFactory` is enabled, however returning `NotFound` from this function will fail a request even if `EnforcePKCE` is set to `false`. 

This means that it is not possible to enable this factory with existing sessions that do not use PKCE without returning a stub from `GetPKCERequestSession` - a workaround that means a real NotFound error from a PKCE enabled session will be masked.

The problem line being: https://github.com/ory/fosite/blob/45a6785cc54fcbd9195b0de4b821bb5fed6a41be/handler/pkce/handler.go#L127

### Reproducing the bug

1. Implement an `authorization_code` flow without the `OAuth2PKCEFactory`
2. Have a client start an auth code flow
3. Enable the `OAuth2PKCEFactory`, returning `fosite.NotFound` if a session does not exist
4. Finish the auth code flow with the created client

I would expect the client to complete the flow successfully when it is not using PKCE, however just having the middleware enabled causes the flow to fail.

### Relevant log output

_No response_

### Relevant configuration

_No response_

### Version

0.42.2

### On which operating system are you observing this issue?

Linux

### In which environment are you deploying?

None

### Additional Context

We have been running a service implemented with fosite and want to rollout PKCE support but cannot do so without invalidating all existing sessions because of this.

Pull request description:
fix(pkce): session generated needlessly
This fixes an issue where the PKCE sessions are generated needlessly which also leads to errors in some circumstances.

## Related Issue or Design Document

Fixes #743 

## Checklist

- [x] I have read the [contributing guidelines](../blob/master/CONTRIBUTING.md) and signed the CLA.
- [x] I have referenced an issue containing the design document if my change introduces a new feature.
- [x] I have read the [security policy](../security/policy).
- [x] I confirm that this pull request does not address a security vulnerability. 
      If this pull request addresses a security vulnerability, 
      I confirm that I got approval (please contact [security@ory.sh](mailto:security@ory.sh)) from the maintainers to push the changes.
- [x] I have added tests that prove my fix is effective or that my feature works.
- [x] I have added the necessary documentation within the code base (if appropriate).

## Further comments

<!--
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution
you did and what alternatives you considered, etc...
-->


Patch:
diff --git a/authorize_helper.go b/authorize_helper.go
index 7d2a10b..4514a4a 100644
--- a/authorize_helper.go
+++ b/authorize_helper.go
@@ -180,7 +180,7 @@ func IsRedirectURISecure(ctx context.Context, redirectURI *url.URL) bool {
 // IsRedirectURISecureStrict is stricter than IsRedirectURISecure and it does not allow custom-scheme
 // URLs because they can be hijacked for native apps. Use claimed HTTPS redirects instead.
 // See discussion in https://github.com/ory/fosite/pull/489.
-func IsRedirectURISecureStrict(redirectURI *url.URL) bool {
+func IsRedirectURISecureStrict(ctx context.Context, redirectURI *url.URL) bool {
 	return redirectURI.Scheme == "https" || (redirectURI.Scheme == "http" && IsLocalhost(redirectURI))
 }
 
diff --git a/handler/oauth2/flow_refresh.go b/handler/oauth2/flow_refresh.go
index de20b59..5bf6807 100644
--- a/handler/oauth2/flow_refresh.go
+++ b/handler/oauth2/flow_refresh.go
@@ -77,6 +77,7 @@ func (c *RefreshTokenGrantHandler) HandleTokenEndpointRequest(ctx context.Contex
 		return errorsx.WithStack(fosite.ErrInvalidGrant.WithHint("The OAuth 2.0 Client ID from this request does not match the ID during the initial token issuance."))
 	}
 
+	request.SetID(originalRequest.GetID())
 	request.SetSession(originalRequest.GetSession().Clone())
 	request.SetRequestedScopes(originalRequest.GetRequestedScopes())
 	request.SetRequestedAudience(originalRequest.GetRequestedAudience())
diff --git a/handler/pkce/handler.go b/handler/pkce/handler.go
index 1d92b8c..3deb94b 100644
--- a/handler/pkce/handler.go
+++ b/handler/pkce/handler.go
@@ -47,6 +47,11 @@ func (c *Handler) HandleAuthorizeEndpointRequest(ctx context.Context, ar fosite.
 		return err
 	}
 
+	// We don't need a session if it's not enforced and the PKCE parameters are not provided by the client.
+	if challenge == "" && method == "" {
+		return nil
+	}
+
 	code := resp.GetCode()
 	if len(code) == 0 {
 		return errorsx.WithStack(fosite.ErrServerError.WithDebug("The PKCE handler must be loaded after the authorize code handler."))
@@ -64,24 +69,14 @@ func (c *Handler) HandleAuthorizeEndpointRequest(ctx context.Context, ar fosite.
 }
 
 func (c *Handler) validate(ctx context.Context, challenge, method string, client fosite.Client) error {
-	if challenge == "" {
+	if len(challenge) == 0 {
 		// If the server requires Proof Key for Code Exchange (PKCE) by OAuth
 		// clients and the client does not send the "code_challenge" in
 		// the request, the authorization endpoint MUST return the authorization
 		// error response with the "error" value set to "invalid_request".  The
 		// "error_description" or the response of "error_uri" SHOULD explain the
 		// nature of error, e.g., code challenge required.
-		if c.Config.GetEnforcePKCE(ctx) {
-			return errorsx.WithStack(fosite.ErrInvalidRequest.
-				WithHint("Clients must include a code_challenge when performing the authorize code flow, but it is missing.").
-				WithDebug("The server is configured in a way that enforces PKCE for clients."))
-		}
-		if c.Config.GetEnforcePKCEForPublicClients(ctx) && client.IsPublic() {
-			return errorsx.WithStack(fosite.ErrInvalidRequest.
-				WithHint("This client must include a code_challenge when performing the authorize code flow, but it is missing.").
-				WithDebug("The server is configured in a way that enforces PKCE for this client."))
-		}
-		return nil
+		return c.validateNoPKCE(ctx, client)
 	}
 
 	// If the server supporting PKCE does not support the requested
@@ -108,6 +103,20 @@ func (c *Handler) validate(ctx context.Context, challenge, method string, client
 	return nil
 }
 
+func (c *Handler) validateNoPKCE(ctx context.Context, client fosite.Client) error {
+	if c.Config.GetEnforcePKCE(ctx) {
+		return errorsx.WithStack(fosite.ErrInvalidRequest.
+			WithHint("Clients must include a code_challenge when performing the authorize code flow, but it is missing.").
+			WithDebug("The server is configured in a way that enforces PKCE for clients."))
+	}
+	if c.Config.GetEnforcePKCEForPublicClients(ctx) && client.IsPublic() {
+		return errorsx.WithStack(fosite.ErrInvalidRequest.
+			WithHint("This client must include a code_challenge when performing the authorize code flow, but it is missing.").
+			WithDebug("The server is configured in a way that enforces PKCE for this client."))
+	}
+	return nil
+}
+
 func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite.AccessRequester) error {
 	if !c.CanHandleTokenEndpointRequest(ctx, request) {
 		return errorsx.WithStack(fosite.ErrUnknownRequest)
@@ -123,8 +132,15 @@ func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite
 
 	code := request.GetRequestForm().Get("code")
 	signature := c.AuthorizeCodeStrategy.AuthorizeCodeSignature(ctx, code)
-	authorizeRequest, err := c.Storage.GetPKCERequestSession(ctx, signature, request.GetSession())
+	pkceRequest, err := c.Storage.GetPKCERequestSession(ctx, signature, request.GetSession())
+
+	nv := len(verifier)
+
 	if errors.Is(err, fosite.ErrNotFound) {
+		if nv == 0 {
+			return c.validateNoPKCE(ctx, request.GetClient())
+		}
+
 		return errorsx.WithStack(fosite.ErrInvalidGrant.WithHint("Unable to find initial PKCE data tied to this request").WithWrap(err).WithDebug(err.Error()))
 	} else if err != nil {
 		return errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error()))
@@ -134,14 +150,16 @@ func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite
 		return errorsx.WithStack(fosite.ErrServerError.WithWrap(err).WithDebug(err.Error()))
 	}
 
-	challenge := authorizeRequest.GetRequestForm().Get("code_challenge")
-	method := authorizeRequest.GetRequestForm().Get("code_challenge_method")
-	client := authorizeRequest.GetClient()
+	challenge := pkceRequest.GetRequestForm().Get("code_challenge")
+	method := pkceRequest.GetRequestForm().Get("code_challenge_method")
+	client := pkceRequest.GetClient()
 	if err := c.validate(ctx, challenge, method, client); err != nil {
 		return err
 	}
 
-	if !c.Config.GetEnforcePKCE(ctx) && challenge == "" && verifier == "" {
+	nc := len(challenge)
+
+	if !c.Config.GetEnforcePKCE(ctx) && nc == 0 && nv == 0 {
 		return nil
 	}
 
@@ -152,15 +170,18 @@ func (c *Handler) HandleTokenEndpointRequest(ctx context.Context, request fosite
 	// 	43-octet URL safe string to use as the code verifier.
 
 	// Validation
-	if len(verifier) < 43 {
+	if nv < 43 {
 		return errorsx.WithStack(fosite.ErrInvalidGrant.
 			WithHint("The PKCE code verifier must be at least 43 characters."))
-	} else if len(verifier) > 128 {
+	} else if nv > 128 {
 		return errorsx.WithStack(fosite.ErrInvalidGrant.
 			WithHint("The PKCE code verifier can not be longer than 128 characters."))
 	} else if verifierWrongFormat.MatchString(verifier) {
 		return errorsx.WithStack(fosite.ErrInvalidGrant.
 			WithHint("The PKCE code verifier must only contain [a-Z], [0-9], '-', '.', '_', '~'."))
+	} else if nc == 0 {
+		return errorsx.WithStack(fosite.ErrInvalidGrant.
+			WithHint("The PKCE code verifier was provided but the code challenge was absent from the authorization request."))
 	}
 
 	// Upon receipt of the request at the token endpoint, the server
diff --git a/request.go b/request.go
index 6bcfac7..26fe736 100644
--- a/request.go
+++ b/request.go
@@ -159,10 +159,12 @@ func (a *Request) Merge(request Requester) {
 	}
 }
 
+var defaultAllowedParameters = []string{"grant_type", "response_type", "scope", "client_id"}
+
 func (a *Request) Sanitize(allowedParameters []string) Requester {
 	b := new(Request)
 	allowed := map[string]bool{}
-	for _, v := range allowedParameters {
+	for _, v := range append(allowedParameters, defaultAllowedParameters...) {
 		allowed[v] = true
 	}
 


Test patch:
diff --git a/authorize_helper_test.go b/authorize_helper_test.go
index fcb7993..637c6bc 100644
--- a/authorize_helper_test.go
+++ b/authorize_helper_test.go
@@ -309,7 +309,7 @@ func TestIsRedirectURISecureStrict(t *testing.T) {
 	} {
 		uu, err := url.Parse(c.u)
 		require.NoError(t, err)
-		assert.Equal(t, !c.err, fosite.IsRedirectURISecureStrict(uu), "case %d", d)
+		assert.Equal(t, !c.err, fosite.IsRedirectURISecureStrict(context.Background(), uu), "case %d", d)
 	}
 }
 
diff --git a/handler/oauth2/flow_refresh_test.go b/handler/oauth2/flow_refresh_test.go
index 986ae91..54df6cd 100644
--- a/handler/oauth2/flow_refresh_test.go
+++ b/handler/oauth2/flow_refresh_test.go
@@ -159,7 +159,11 @@ func TestRefreshFlow_HandleTokenEndpointRequest(t *testing.T) {
 						require.NoError(t, err)
 
 						areq.Form.Add("refresh_token", token)
+
+						orReqID := areq.GetID() + "_OR"
+						areq.Form.Add("or_request_id", orReqID)
 						err = store.CreateRefreshTokenSession(context.Background(), sig, &fosite.Request{
+							ID:             orReqID,
 							Client:         areq.Client,
 							GrantedScope:   fosite.Arguments{"foo", "offline"},
 							RequestedScope: fosite.Arguments{"foo", "bar", "offline"},
@@ -177,6 +181,7 @@ func TestRefreshFlow_HandleTokenEndpointRequest(t *testing.T) {
 						assert.NotEqual(t, url.Values{"foo": []string{"bar"}}, areq.Form)
 						assert.Equal(t, time.Now().Add(time.Hour).UTC().Round(time.Second), areq.GetSession().GetExpiresAt(fosite.AccessToken))
 						assert.Equal(t, time.Now().Add(time.Hour).UTC().Round(time.Second), areq.GetSession().GetExpiresAt(fosite.RefreshToken))
+						assert.EqualValues(t, areq.Form.Get("or_request_id"), areq.GetID(), "Requester ID should be replaced based on the refresh token session")
 					},
 				},
 				{
diff --git a/handler/pkce/handler_test.go b/handler/pkce/handler_test.go
index ced8ce8..bdd5065 100644
--- a/handler/pkce/handler_test.go
+++ b/handler/pkce/handler_test.go
@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -93,21 +94,23 @@ func TestPKCEHandlerValidate(t *testing.T) {
 	s256challenge := base64.RawURLEncoding.EncodeToString(hash.Sum([]byte{}))
 
 	for k, tc := range []struct {
-		d           string
-		grant       string
-		force       bool
-		enablePlain bool
-		challenge   string
-		method      string
-		verifier    string
-		code        string
-		expectErr   error
-		client      *fosite.DefaultClient
+		d             string
+		grant         string
+		force         bool
+		enablePlain   bool
+		challenge     string
+		method        string
+		verifier      string
+		code          string
+		expectErr     error
+		expectErrDesc string
+		client        *fosite.DefaultClient
 	}{
 		{
-			d:         "fails because not auth code flow",
-			grant:     "not_authorization_code",
-			expectErr: fosite.ErrUnknownRequest,
+			d:             "fails because not auth code flow",
+			grant:         "not_authorization_code",
+			expectErr:     fosite.ErrUnknownRequest,
+			expectErrDesc: "The handler is not responsible for this request.",
 		},
 		{
 			d:           "passes with private client",
@@ -121,11 +124,13 @@ func TestPKCEHandlerValidate(t *testing.T) {
 			code:        "valid-code-1",
 		},
 		{
-			d:         "fails because invalid code",
-			grant:     "authorization_code",
-			expectErr: fosite.ErrInvalidGrant,
-			client:    pc,
-			code:      "invalid-code-2",
+			d:             "fails because invalid code",
+			grant:         "authorization_code",
+			client:        pc,
+			code:          "invalid-code-2",
+			verifier:      "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo",
+			expectErr:     fosite.ErrInvalidGrant,
+			expectErrDesc: "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client. Unable to find initial PKCE data tied to this request not_found",
 		},
 		{
 			d:      "passes because auth code flow but pkce is not forced and no challenge given",
@@ -134,12 +139,13 @@ func TestPKCEHandlerValidate(t *testing.T) {
 			code:   "valid-code-3",
 		},
 		{
-			d:         "fails because auth code flow and pkce challenge given but plain is disabled",
-			grant:     "authorization_code",
-			challenge: "foo",
-			client:    pc,
-			expectErr: fosite.ErrInvalidRequest,
-			code:      "valid-code-4",
+			d:             "fails because auth code flow and pkce challenge given but plain is disabled",
+			grant:         "authorization_code",
+			challenge:     "foo",
+			client:        pc,
+			code:          "valid-code-4",
+			expectErr:     fosite.ErrInvalidRequest,
+			expectErrDesc: "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Clients must use code_challenge_method=S256, plain is not allowed. The server is configured in a way that enforces PKCE S256 as challenge method for clients.",
 		},
 		{
 			d:           "passes",
@@ -163,69 +169,75 @@ func TestPKCEHandlerValidate(t *testing.T) {
 			code:        "valid-code-6",
 		},
 		{
-			d:           "fails because challenge and verifier do not match",
-			grant:       "authorization_code",
-			challenge:   "not-foo",
-			verifier:    "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo",
-			method:      "plain",
-			client:      pc,
-			enablePlain: true,
-			code:        "valid-code-7",
-			expectErr:   fosite.ErrInvalidGrant,
+			d:             "fails because challenge and verifier do not match",
+			grant:         "authorization_code",
+			challenge:     "not-foo",
+			verifier:      "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo",
+			method:        "plain",
+			client:        pc,
+			enablePlain:   true,
+			code:          "valid-code-7",
+			expectErr:     fosite.ErrInvalidGrant,
+			expectErrDesc: "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client. The PKCE code challenge did not match the code verifier.",
 		},
 		{
-			d:           "fails because challenge and verifier do not match",
-			grant:       "authorization_code",
-			challenge:   "not-foonot-foonot-foonot-foonot-foonot-foonot-foonot-foo",
-			verifier:    "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo",
-			client:      pc,
-			enablePlain: true,
-			code:        "valid-code-8",
-			expectErr:   fosite.ErrInvalidGrant,
+			d:             "fails because challenge and verifier do not match",
+			grant:         "authorization_code",
+			challenge:     "not-foonot-foonot-foonot-foonot-foonot-foonot-foonot-foo",
+			verifier:      "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo",
+			client:        pc,
+			enablePlain:   true,
+			code:          "valid-code-8",
+			expectErr:     fosite.ErrInvalidGrant,
+			expectErrDesc: "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client. The PKCE code challenge did not match the code verifier.",
 		},
 		{
-			d:         "fails because verifier is too short",
-			grant:     "authorization_code",
-			challenge: "foo",
-			verifier:  "foo",
-			method:    "S256",
-			client:    pc,
-			force:     true,
-			code:      "valid-code-9a",
-			expectErr: fosite.ErrInvalidGrant,
+			d:             "fails because verifier is too short",
+			grant:         "authorization_code",
+			challenge:     "foo",
+			verifier:      "foo",
+			method:        "S256",
+			client:        pc,
+			force:         true,
+			code:          "valid-code-9",
+			expectErr:     fosite.ErrInvalidGrant,
+			expectErrDesc: "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client. The PKCE code verifier must be at least 43 characters.",
 		},
 		{
-			d:         "fails because verifier is too long",
-			grant:     "authorization_code",
-			challenge: "foo",
-			verifier:  "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo",
-			method:    "S256",
-			client:    pc,
-			force:     true,
-			code:      "valid-code-10",
-			expectErr: fosite.ErrInvalidGrant,
+			d:             "fails because verifier is too long",
+			grant:         "authorization_code",
+			challenge:     "foo",
+			verifier:      "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo",
+			method:        "S256",
+			client:        pc,
+			force:         true,
+			code:          "valid-code-10",
+			expectErr:     fosite.ErrInvalidGrant,
+			expectErrDesc: "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client. The PKCE code verifier can not be longer than 128 characters.",
 		},
 		{
-			d:         "fails because verifier is malformed",
-			grant:     "authorization_code",
-			challenge: "foo",
-			verifier:  `(!"/$%Z&$T()/)OUZI>$"&=/T(PUOI>"%/)TUOI&/(O/()RGTE>=/(%"/()="$/)(=()=/R/()=))`,
-			method:    "S256",
-			client:    pc,
-			force:     true,
-			code:      "valid-code-11",
-			expectErr: fosite.ErrInvalidGrant,
+			d:             "fails because verifier is malformed",
+			grant:         "authorization_code",
+			challenge:     "foo",
+			verifier:      `(!"/$%Z&$T()/)OUZI>$"&=/T(PUOI>"%/)TUOI&/(O/()RGTE>=/(%"/()="$/)(=()=/R/()=))`,
+			method:        "S256",
+			client:        pc,
+			force:         true,
+			code:          "valid-code-11",
+			expectErr:     fosite.ErrInvalidGrant,
+			expectErrDesc: "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client. The PKCE code verifier must only contain [a-Z], [0-9], '-', '.', '_', '~'.",
 		},
 		{
-			d:         "fails because challenge and verifier do not match",
-			grant:     "authorization_code",
-			challenge: "Zm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9v",
-			verifier:  "Zm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9v",
-			method:    "S256",
-			client:    pc,
-			force:     true,
-			code:      "valid-code-12",
-			expectErr: fosite.ErrInvalidGrant,
+			d:             "fails because challenge and verifier do not match",
+			grant:         "authorization_code",
+			challenge:     "Zm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9v",
+			verifier:      "Zm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9v",
+			method:        "S256",
+			client:        pc,
+			force:         true,
+			code:          "valid-code-12",
+			expectErr:     fosite.ErrInvalidGrant,
+			expectErrDesc: "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client. The PKCE code challenge did not match the code verifier.",
 		},
 		{
 			d:         "passes because challenge and verifier match",
@@ -237,25 +249,58 @@ func TestPKCEHandlerValidate(t *testing.T) {
 			force:     true,
 			code:      "valid-code-13",
 		},
+		{
+			d:      "passes when not forced because no challenge or verifier",
+			grant:  "authorization_code",
+			client: pc,
+			code:   "valid-code-14",
+		},
+		{
+			d:             "fails when not forced because verifier provided when no challenge",
+			grant:         "authorization_code",
+			client:        pc,
+			code:          "valid-code-15",
+			verifier:      "Zm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9vZm9v",
+			expectErr:     fosite.ErrInvalidGrant,
+			expectErrDesc: "The provided authorization grant (e.g., authorization code, resource owner credentials) or refresh token is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client. The PKCE code verifier was provided but the code challenge was absent from the authorization request.",
+		},
 	} {
 		t.Run(fmt.Sprintf("case=%d/description=%s", k, tc.d), func(t *testing.T) {
 			config.EnablePKCEPlainChallengeMethod = tc.enablePlain
 			config.EnforcePKCE = tc.force
 			ms.signature = tc.code
 			ar := fosite.NewAuthorizeRequest()
-			ar.Form.Add("code_challenge", tc.challenge)
-			ar.Form.Add("code_challenge_method", tc.method)
+
+			if len(tc.challenge) != 0 {
+				ar.Form.Add("code_challenge", tc.challenge)
+			}
+
+			if len(tc.method) != 0 {
+				ar.Form.Add("code_challenge_method", tc.method)
+			}
+
+			ar.Client = tc.client
+
 			require.NoError(t, s.CreatePKCERequestSession(context.Background(), fmt.Sprintf("valid-code-%d", k), ar))
 
 			r := fosite.NewAccessRequest(nil)
 			r.Client = tc.client
 			r.GrantTypes = fosite.Arguments{tc.grant}
-			r.Form.Add("code_verifier", tc.verifier)
+
+			if len(tc.verifier) != 0 {
+				r.Form.Add("code_verifier", tc.verifier)
+			}
+
+			err := h.HandleTokenEndpointRequest(context.Background(), r)
+
 			if tc.expectErr == nil {
-				require.NoError(t, h.HandleTokenEndpointRequest(context.Background(), r))
+				assert.NoError(t, err)
 			} else {
-				err := h.HandleTokenEndpointRequest(context.Background(), r)
-				require.EqualError(t, err, tc.expectErr.Error(), "%+v", err)
+				assert.EqualError(t, err, tc.expectErr.Error(), "%+v", err)
+
+				if len(tc.expectErrDesc) != 0 {
+					assert.EqualError(t, newtesterr(err), tc.expectErrDesc)
+				}
 			}
 		})
 	}
@@ -345,3 +390,24 @@ func TestPKCEHandleTokenEndpointRequest(t *testing.T) {
 		})
 	}
 }
+
+func newtesterr(err error) error {
+	if err == nil {
+		return nil
+	}
+
+	var e *fosite.RFC6749Error
+	if errors.As(err, &e) {
+		return &testerr{e}
+	}
+
+	return err
+}
+
+type testerr struct {
+	*fosite.RFC6749Error
+}
+
+func (e *testerr) Error() string {
+	return e.RFC6749Error.WithExposeDebug(true).GetDescription()
+}
diff --git a/request_test.go b/request_test.go
index 5abe1da..459d69c 100644
--- a/request_test.go
+++ b/request_test.go
@@ -73,9 +73,13 @@ func TestSanitizeRequest(t *testing.T) {
 		RequestedScope: Arguments{"asdff"},
 		GrantedScope:   []string{"asdf"},
 		Form: url.Values{
-			"foo": []string{"fasdf"},
-			"bar": []string{"fasdf", "faaaa"},
-			"baz": []string{"fasdf"},
+			"foo":           []string{"fasdf"},
+			"bar":           []string{"fasdf", "faaaa"},
+			"baz":           []string{"fasdf"},
+			"grant_type":    []string{"code"},
+			"response_type": []string{"id_token"},
+			"client_id":     []string{"1234"},
+			"scope":         []string{"read"},
 		},
 		Session: new(DefaultSession),
 	}
@@ -92,6 +96,10 @@ func TestSanitizeRequest(t *testing.T) {
 	assert.Equal(t, "fasdf", a.GetRequestForm().Get("bar"))
 	assert.Equal(t, []string{"fasdf", "faaaa"}, a.GetRequestForm()["bar"])
 	assert.Equal(t, "fasdf", a.GetRequestForm().Get("baz"))
+	assert.Equal(t, "code", a.GetRequestForm().Get("grant_type"))
+	assert.Equal(t, "id_token", a.GetRequestForm().Get("response_type"))
+	assert.Equal(t, "1234", a.GetRequestForm().Get("client_id"))
+	assert.Equal(t, "read", a.GetRequestForm().Get("scope"))
 }
 
 func TestIdentifyRequest(t *testing.T) {