Repository: badgateway/oauth2-client (ts)

Issue:
Public client without authentication method
I want to use OAuth public client with code flow and PKCE. The issue is with token request, which must include client_id in this case according to the https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.3. This library adds the client_id to the token request only if the "settins.authenticationMethod" equals to "client_secret_post" which is wrong IMO. It should be added even if no authenticationMethod is used.
Another issue is if I do not provide "authenticationMethod" in settings, this library will use the first authentication method from OAuth server metadata (token_endpoint_auth_methods_supported). For example, if the server metadata contains ["client_secret_basic"], then the library will include Authorization HTTP header with basic authentication even if I do not intend to use any.

Pull request description:
If there's no secret, we should never use Basic auth to encode the client_id.
Fixes #128.

Patch:
diff --git a/changelog.md b/changelog.md
index 2fd45da..33a574c 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,6 +1,13 @@
 Changelog
 =========
 
+2.2.5 (????-??-??)
+------------------
+
+* Fix for #128: If there's no secret, we should never use Basic auth to encode
+  the `client_id`.
+
+
 2.2.4 (2023-09-05)
 ------------------
 
diff --git a/src/client.ts b/src/client.ts
index 424a071..74b1917 100644
--- a/src/client.ts
+++ b/src/client.ts
@@ -296,8 +296,18 @@ export class OAuth2Client {
     };
 
     let authMethod = this.settings.authenticationMethod;
+
+    if (!this.settings.clientSecret) {
+      // Basic auth should only be used when there's a client_secret, for
+      // non-confidential clients we may only have a client_id, which
+      // always gets added to the body.
+      authMethod = 'client_secret_post';
+    }
     if (!authMethod) {
-      authMethod = this.settings.clientSecret ? 'client_secret_basic' : 'client_secret_post';
+      // If we got here, it means no preference was provided by anything,
+      // and we have a secret. In this case its preferred to embed
+      // authentication in the Authorization header.
+      authMethod = 'client_secret_basic';
     }
 
     switch(authMethod) {


Test patch:
diff --git a/test/authorization-code.ts b/test/authorization-code.ts
index 4477e60..e81c0e9 100644
--- a/test/authorization-code.ts
+++ b/test/authorization-code.ts
@@ -235,6 +235,38 @@ describe('authorization-code', () => {
       });
 
     });
+    it('should not use Basic Auth if no secret is provided, even if client_secret_basic is set.', async() => {
+
+      const server = testServer();
+
+      const client = new OAuth2Client({
+        server: server.url,
+        tokenEndpoint: '/token',
+        clientId: 'test-client-id',
+        authenticationMethod: 'client_secret_basic',
+      });
+
+      const result = await client.authorizationCode.getToken({
+        code: 'code_000',
+        redirectUri: 'http://example/redirect',
+      });
+
+      expect(result.accessToken).to.equal('access_token_000');
+      expect(result.refreshToken).to.equal('refresh_token_000');
+      expect(result.expiresAt).to.be.lessThanOrEqual(Date.now() + 3600_000);
+      expect(result.expiresAt).to.be.greaterThanOrEqual(Date.now() + 3500_000);
+
+      const request = server.lastRequest();
+      expect(request.headers.get('Authorization')).to.equal(null);
+
+      expect(request.body).to.eql({
+        client_id: 'test-client-id',
+        grant_type: 'authorization_code',
+        code: 'code_000',
+        redirect_uri: 'http://example/redirect',
+      });
+
+    });
 
   });
 });