Merge branch 'master' into fix/invalid-argument-error-leak

Author: jankapunkt

docs/api/model.md

@@ -84,6 +84,14 @@ This model function is **required** for all grant types.
 - `refresh_token` grant
 - `password` grant
 
+**Remarks:**
+`clientSecret` is `null`/absent for public clients, and also for
+`authorization_code` requests that authenticate with PKCE (a `code_verifier`)
+instead of a secret — even if the client *has* a `client_secret`. PKCE is
+**not** a substitute for client authentication: your implementation must
+reject (return a falsy value) a confidential client that should have
+presented its `client_secret` but did not.
+
 **Kind**: instance method of [<code>Model</code>](#Model)  
 **Fulfil**: [<code>ClientData</code>](#ClientData) - An `Object` representing the client and associated data, or a falsy value if no such client could be found.  
 **Reject**: <code>Error</code> - An Error type  

docs/api/server.md

@@ -158,7 +158,7 @@ function authorizeHandler(options) {
 Retrieves a new token for an authorized token request.
 **Remarks:**
 If `options.allowExtendedTokenAttributes` is `true` any additional properties set on the object returned from `Model#saveToken() <Model#saveToken>` are copied to the token response sent to the client.
-By default, all grant types require the client to send it's `client_secret` with the token request. `options.requireClientAuthentication` can be used to disable this check for selected grants. If used, this server option must be an object containing properties set to `true` or `false`. Possible keys for the object include all supported values for the token request's `grant_type` field (`authorization_code`, `client_credentials`, `password` and `refresh_token`). Grants that are not specified default to `true` which enables verification of the `client_secret`.
+By default, all grant types require the client to send it's `client_secret` with the token request. `options.requireClientAuthentication` can be used to disable this check for selected grants. If used, this server option must be an object containing properties set to `true` or `false`. Possible keys for the object include all supported values for the token request's `grant_type` field (`authorization_code`, `client_credentials`, `password` and `refresh_token`). Grants that are not specified default to `true` which enables verification of the `client_secret`. Note: setting a grant to `false` disables only the *presence* check of the `client_secret` for that grant, and does so for **all** clients (not just public ones); it does not validate a secret that is sent. The same applies to `authorization_code` requests that use PKCE (a `code_verifier`), where the presence check is skipped. Per-client (public vs confidential) authentication must therefore be enforced in your `Model#getClient() <Model#getClient>` implementation, which should reject a confidential client that fails to present its `client_secret`.
 ```js
 let options = {
   // ...

docs/guide/pkce.md

@@ -130,3 +130,36 @@ The loaded code has to contain `codeChallenge` and `codeChallengeMethod`.
 If `model.saveAuthorizationCode` did not cover these values when saving the code then this step will deny the request.
 
 See `Model#saveAuthorizationCode` and `Model#getAuthorizationCode`
+
+## PKCE, client authentication and refresh tokens
+
+PKCE only protects the `authorization_code` → token exchange. The `code_verifier`
+is sent and verified **once**, when the authorization code is redeemed (step 3
+above); it is **not** a parameter of the `refresh_token` grant and is ignored
+there. A client that obtained its tokens via PKCE refreshes them like any other
+client — by presenting its `refresh_token` (and, if it is a confidential client,
+its `client_secret`).
+
+PKCE is **not** client authentication, and never a substitute for a `client_secret`:
+
+- A **confidential** client (one issued a `client_secret`) must authenticate with
+  its secret on **every** token request, including `refresh_token`. PKCE is
+  additive. Your `model.getClient(clientId, clientSecret)` is responsible for
+  rejecting (returning a falsy value) a confidential client that fails to present
+  its secret.
+- A **public** client has no secret. If you choose to issue refresh tokens to
+  public clients (weigh the security implications first — see
+  [RFC 9700](https://www.rfc-editor.org/rfc/rfc9700)), relax client authentication
+  for that grant:
+
+      const server = new OAuth2Server({
+        model,
+        requireClientAuthentication: { refresh_token: false } // allow refresh without a client_secret
+      })
+
+  `requireClientAuthentication: { refresh_token: false }` disables the
+  `client_secret` **presence** check for the `refresh_token` grant for **all**
+  clients, not just public ones, so per-client (public vs confidential)
+  enforcement must be done in your `model.getClient`. The library does not yet
+  model the public/confidential distinction itself (tracked in
+  [#81](https://github.com/node-oauth/node-oauth2-server/issues/81)).

index.d.ts

@@ -228,6 +228,9 @@ declare namespace OAuth2Server {
 
         /**
          * Require a client secret. Defaults to true for all grant types.
+         * Setting a grant to `false` disables the client_secret presence check for
+         * that grant for ALL clients (not just public ones); per-client
+         * (public vs confidential) enforcement must be done in model.getClient.
          */
         requireClientAuthentication?: Record<string, boolean>;
 

lib/model.js

@@ -96,6 +96,13 @@ class Model {
    * - `refresh_token` grant
    * - `password` grant
    *
+   * **Remarks:**
+   * `clientSecret` is `null`/absent for public clients, and also for
+   * `authorization_code` requests that authenticate with PKCE (a `code_verifier`)
+   * instead of a secret — even if the client *has* a `client_secret`. PKCE is
+   * **not** a substitute for client authentication: your implementation must
+   * reject (return a falsy value) a confidential client that should have
+   * presented its `client_secret` but did not.
    *
    * @async
    * @param clientId {string} The client id of the client to retrieve.

lib/server.js

@@ -183,7 +183,7 @@ class OAuth2Server {
    * Retrieves a new token for an authorized token request.
    * **Remarks:**
    * If `options.allowExtendedTokenAttributes` is `true` any additional properties set on the object returned from `Model#saveToken() <Model#saveToken>` are copied to the token response sent to the client.
-   * By default, all grant types require the client to send it's `client_secret` with the token request. `options.requireClientAuthentication` can be used to disable this check for selected grants. If used, this server option must be an object containing properties set to `true` or `false`. Possible keys for the object include all supported values for the token request's `grant_type` field (`authorization_code`, `client_credentials`, `password` and `refresh_token`). Grants that are not specified default to `true` which enables verification of the `client_secret`.
+   * By default, all grant types require the client to send it's `client_secret` with the token request. `options.requireClientAuthentication` can be used to disable this check for selected grants. If used, this server option must be an object containing properties set to `true` or `false`. Possible keys for the object include all supported values for the token request's `grant_type` field (`authorization_code`, `client_credentials`, `password` and `refresh_token`). Grants that are not specified default to `true` which enables verification of the `client_secret`. Note: setting a grant to `false` disables only the *presence* check of the `client_secret` for that grant, and does so for **all** clients (not just public ones); it does not validate a secret that is sent. The same applies to `authorization_code` requests that use PKCE (a `code_verifier`), where the presence check is skipped. Per-client (public vs confidential) authentication must therefore be enforced in your `Model#getClient() <Model#getClient>` implementation, which should reject a confidential client that fails to present its `client_secret`.
    * ```js
    * let options = {
    *   // ...