From 76f56674248f396e7c087eff9123fe3919fc4a58 Mon Sep 17 00:00:00 2001
From: Marcus Olsson <8396880+marcusolsson@users.noreply.github.com>
Date: Thu, 21 May 2026 11:45:47 +0200
Subject: [PATCH] fix: remove double token expiry buffer (#5)

---
 src/plugin/auth.test.ts      | 65 ++++++++++++++++++++++++++++++++++++
 src/plugin/pkce-flow.test.ts |  4 ++-
 src/plugin/pkce-flow.ts      |  3 +-
 src/plugin/token.test.ts     |  4 ++-
 src/plugin/token.ts          |  4 +--
 5 files changed, 74 insertions(+), 6 deletions(-)
 create mode 100644 src/plugin/auth.test.ts

diff --git a/src/plugin/auth.test.ts b/src/plugin/auth.test.ts
new file mode 100644
index 0000000..2c3d124
--- /dev/null
+++ b/src/plugin/auth.test.ts
@@ -0,0 +1,65 @@
+import { describe, expect, it } from 'vitest';
+
+import { ACCESS_TOKEN_EXPIRY_BUFFER_MS } from '../constants';
+import { accessTokenExpired } from './auth';
+import type { OAuthAuthDetails } from './types';
+
+describe('accessTokenExpired - Issue #5', () => {
+  // With raw expiry storage, the buffer lives ONLY in the check.
+  // A token with expires_in=300 (raw expiry = now + 300_000) should be
+  // considered expired at: now + 300_000 - 60_000 = now + 240_000.
+  it('returns false when token lifetime minus buffer remains', () => {
+    const auth: OAuthAuthDetails = {
+      access: 'token',
+      expires: Date.now() + 300_000, // raw 5-minute expiry
+      refresh: 'refresh',
+      type: 'oauth',
+    };
+
+    expect(accessTokenExpired(auth)).toBe(false);
+  });
+
+  it('returns true when only the buffer remains', () => {
+    const auth: OAuthAuthDetails = {
+      access: 'token',
+      expires: Date.now() + ACCESS_TOKEN_EXPIRY_BUFFER_MS - 1, // raw expiry < now + buffer
+      refresh: 'refresh',
+      type: 'oauth',
+    };
+
+    expect(accessTokenExpired(auth)).toBe(true);
+  });
+
+  it('returns true when the raw expiry has passed', () => {
+    const auth: OAuthAuthDetails = {
+      access: 'token',
+      expires: Date.now() - 1,
+      refresh: 'refresh',
+      type: 'oauth',
+    };
+
+    expect(accessTokenExpired(auth)).toBe(true);
+  });
+
+  it('returns true when access token is missing', () => {
+    const auth: OAuthAuthDetails = {
+      access: undefined,
+      expires: Date.now() + 300_000,
+      refresh: 'refresh',
+      type: 'oauth',
+    };
+
+    expect(accessTokenExpired(auth)).toBe(true);
+  });
+
+  it('returns true when expires is not a number', () => {
+    const auth = {
+      access: 'token',
+      expires: 'not-a-number',
+      refresh: 'refresh',
+      type: 'oauth',
+    } as unknown as OAuthAuthDetails;
+
+    expect(accessTokenExpired(auth)).toBe(true);
+  });
+});
diff --git a/src/plugin/pkce-flow.test.ts b/src/plugin/pkce-flow.test.ts
index 7fbb281..0050932 100644
--- a/src/plugin/pkce-flow.test.ts
+++ b/src/plugin/pkce-flow.test.ts
@@ -191,7 +191,9 @@ describe('exchangeCodeForTokens - Issue #3', () => {
       throw new Error('expected success with access token');
     }
     expect(result.access).toBe('access-123');
-    expect(typeof result.expires).toBe('number');
+    // Issue #5: stored expiry must be the raw timestamp, NOT pre-reduced by buffer
+    expect(result.expires).toBeGreaterThanOrEqual(Date.now() + 300_000 - 2_000);
+    expect(result.expires).toBeLessThanOrEqual(Date.now() + 300_000 + 2_000);
     expect(result.refresh).toBe('refresh-456');
   });
 
diff --git a/src/plugin/pkce-flow.ts b/src/plugin/pkce-flow.ts
index ca50f35..2435f52 100644
--- a/src/plugin/pkce-flow.ts
+++ b/src/plugin/pkce-flow.ts
@@ -15,7 +15,6 @@ import * as url from 'node:url';
 import type { AuthOAuthResult, AuthorizeResult } from './types';
 
 import {
-  ACCESS_TOKEN_EXPIRY_BUFFER_MS,
   getKeycloakRealm,
   getKeycloakUrl,
   KEYCLOAK_CLIENT_ID,
@@ -213,7 +212,7 @@ export async function exchangeCodeForTokens(
     };
   }
 
-  const expires = Date.now() + tokenData.expires_in * 1000 - ACCESS_TOKEN_EXPIRY_BUFFER_MS;
+  const expires = Date.now() + tokenData.expires_in * 1000;
 
   logDebug('Successfully obtained tokens via PKCE');
 
diff --git a/src/plugin/token.test.ts b/src/plugin/token.test.ts
index abc142e..badfeef 100644
--- a/src/plugin/token.test.ts
+++ b/src/plugin/token.test.ts
@@ -77,7 +77,9 @@ describe('refreshAccessTokenDirect', () => {
     if (!result.success) throw new Error('result should be success');
     expect(result.auth.access).toBe(newToken);
     expect(result.auth.refresh).toBe('refresh-token-1');
-    expect(result.auth.expires).toBeGreaterThan(Date.now());
+    // Issue #5: stored expiry must be the raw timestamp, NOT pre-reduced by buffer
+    expect(result.auth.expires).toBeGreaterThanOrEqual(Date.now() + expiresIn * 1000 - 2_000);
+    expect(result.auth.expires).toBeLessThanOrEqual(Date.now() + expiresIn * 1000 + 2_000);
 
     expect(client.auth.set).toHaveBeenCalledTimes(1);
     expect(client.auth.set).toHaveBeenCalledWith({
diff --git a/src/plugin/token.ts b/src/plugin/token.ts
index a9bbae3..3e183c6 100644
--- a/src/plugin/token.ts
+++ b/src/plugin/token.ts
@@ -4,7 +4,7 @@
 
 import type { OAuthAuthDetails, PluginInput, RefreshResult } from './types';
 
-import { ACCESS_TOKEN_EXPIRY_BUFFER_MS, getTokenRefreshEndpoint } from '../constants';
+import { getTokenRefreshEndpoint } from '../constants';
 import { logDebug } from './debug';
 
 // Track in-flight refresh requests to prevent duplicates
@@ -117,7 +117,7 @@ async function refreshAccessTokenInternal(
     const updatedAuth: OAuthAuthDetails = {
       ...auth,
       access: data.token,
-      expires: Date.now() + data.expires_in * 1000 - ACCESS_TOKEN_EXPIRY_BUFFER_MS,
+      expires: Date.now() + data.expires_in * 1000,
       refresh: data.refresh_token || refreshToken, // Use new refresh token if rotated
     };