Repository: badgateway/oauth2-client (ts)

Issue:
Fetch wrapper breaks token for some providers when refreshing
I'm using Cognito as the OAuth provider. In this case, the tokens returned to the client when providing a refresh token do not include a new refresh token (nor the old one - no refresh token comes back at all). This means that https://github.com/badgateway/oauth2-client/blob/dfaa4622ff37217a62a8daa551257c385e8bd7b4/src/client.ts#L184 wipes out the refresh token and subsequent refreshes will fail. I've mentioned the fetch wrapper because that's where I noticed the behaviour, but it appears this will happen for any refresh.

The solution might be to check the returned token and add the old refresh token back if the new one is null. I can PR for this if you're happy with the approach.

Pull request description:
Re-use old refresh token if no new one was issued.
Fixes #161


Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 38d3352..6b59827 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -7,7 +7,7 @@ on:
   push:
     branches: [ main ]
   pull_request:
-    branches: [ main ]
+    branches: [ main, 'version-2.x' ]
 
 jobs:
   test:
diff --git a/changelog.md b/changelog.md
index f0ae596..496819e 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,6 +1,12 @@
 Changelog
 =========
 
+2.4.2 (????-??-??)
+------------------
+
+* #161: Re-use old refresh_token if no new one was issued after a refresh.
+
+
 2.4.1 (2024-08-22)
 ------------------
 
diff --git a/src/client.ts b/src/client.ts
index b035e65..6332dd0 100644
--- a/src/client.ts
+++ b/src/client.ts
@@ -181,7 +181,12 @@ export class OAuth2Client {
     if (params?.scope) body.scope = params.scope.join(' ');
     if (params?.resource) body.resource = params.resource;
 
-    return this.tokenResponseToOAuth2Token(this.request('tokenEndpoint', body));
+    const newToken = await this.tokenResponseToOAuth2Token(this.request('tokenEndpoint', body));
+    if (!newToken.refreshToken && token.refreshToken) {
+      // Reuse old refresh token if we didn't get a new one.
+      newToken.refreshToken = token.refreshToken;
+    }
+    return newToken;
 
   }
 


Test patch:
diff --git a/test/refresh.ts b/test/refresh.ts
new file mode 100644
index 0000000..26891d6
--- /dev/null
+++ b/test/refresh.ts
@@ -0,0 +1,70 @@
+import { testServer } from './test-server';
+import { OAuth2Client } from '../src';
+import { expect } from 'chai';
+
+describe('refreshing tokens', () => {
+
+  it('should work', async () => {
+
+    const server = testServer();
+
+    const client = new OAuth2Client({
+      server: server.url,
+      tokenEndpoint: '/token',
+      clientId: 'test-client-id',
+      clientSecret: 'test-client-secret',
+    });
+
+    const result = await client.refreshToken({
+      refreshToken: 'refresh_token_000',
+      accessToken: 'access_token_000',
+      expiresAt: null,
+    });
+
+    expect(result.accessToken).to.equal('access_token_001');
+    expect(result.refreshToken).to.equal('refresh_token_001');
+    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('Basic ' + btoa('test-client-id:test-client-secret'));
+    expect(request.headers.get('Accept')).to.equal('application/json');
+
+    expect(request.body).to.eql({
+      grant_type: 'refresh_token',
+      refresh_token: 'refresh_token_000',
+    });
+  });
+
+  it('should re-use the old refresh token if no new one was issued', async () => {
+
+    const server = testServer();
+
+    const client = new OAuth2Client({
+      server: server.url,
+      tokenEndpoint: '/token',
+      clientId: 'test-client-id',
+      clientSecret: 'test-client-secret',
+    });
+
+    const result = await client.refreshToken({
+      refreshToken: 'refresh_token_001',
+      accessToken: 'access_token_001',
+      expiresAt: null,
+    });
+
+    expect(result.accessToken).to.equal('access_token_002');
+    expect(result.refreshToken).to.equal('refresh_token_001');
+    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('Basic ' + btoa('test-client-id:test-client-secret'));
+    expect(request.headers.get('Accept')).to.equal('application/json');
+
+    expect(request.body).to.eql({
+      grant_type: 'refresh_token',
+      refresh_token: 'refresh_token_001',
+    });
+  });
+});
diff --git a/test/test-server.ts b/test/test-server.ts
index 2ed52fd..5a8697b 100644
--- a/test/test-server.ts
+++ b/test/test-server.ts
@@ -105,11 +105,29 @@ const issueToken: Middleware = (ctx, next) => {
   }
 
   ctx.response.type = 'application/json';
-  ctx.response.body = {
-    access_token: 'access_token_000',
-    refresh_token: 'refresh_token_000',
-    expires_in: 3600,
-  };
+  if (ctx.request.body.refresh_token === 'refresh_token_000') {
+
+    ctx.response.body = {
+      access_token: 'access_token_001',
+      refresh_token: 'refresh_token_001',
+      expires_in: 3600,
+    };
+
+  } else if (ctx.request.body.refresh_token === 'refresh_token_001') {
+
+    ctx.response.body = {
+      access_token: 'access_token_002',
+      expires_in: 3600,
+    };
+
+  } else {
+
+    ctx.response.body = {
+      access_token: 'access_token_000',
+      refresh_token: 'refresh_token_000',
+      expires_in: 3600,
+    };
+  }
 
 };