fix(server): validate expiresAt token value for non existence (#446)

* fix(server): validate expiresAt token value for non existence

---------

Co-authored-by: Olivier Chafik <[email protected]>

Author: christian-bromann

src/server/auth/middleware/bearerAuth.test.ts

@@ -37,6 +37,7 @@ describe("requireBearerAuth middleware", () => {
       token: "valid-token",
       clientId: "client-123",
       scopes: ["read", "write"],
+      expiresAt: Math.floor(Date.now() / 1000) + 3600, // Token expires in an hour
     };
     mockVerifyAccessToken.mockResolvedValue(validAuthInfo);
 
@@ -53,13 +54,17 @@ describe("requireBearerAuth middleware", () => {
     expect(mockResponse.status).not.toHaveBeenCalled();
     expect(mockResponse.json).not.toHaveBeenCalled();
   });
-
-  it("should reject expired tokens", async () => {
+  
+  it.each([
+    [100], // Token expired 100 seconds ago
+    [0], // Token expires at the same time as now
+  ])("should reject expired tokens (expired %s seconds ago)", async (expiredSecondsAgo: number) => {
+    const expiresAt = Math.floor(Date.now() / 1000) - expiredSecondsAgo;
     const expiredAuthInfo: AuthInfo = {
       token: "expired-token",
       clientId: "client-123",
       scopes: ["read", "write"],
-      expiresAt: Math.floor(Date.now() / 1000) - 100, // Token expired 100 seconds ago
+      expiresAt
     };
     mockVerifyAccessToken.mockResolvedValue(expiredAuthInfo);
 
@@ -82,6 +87,37 @@ describe("requireBearerAuth middleware", () => {
     expect(nextFunction).not.toHaveBeenCalled();
   });
 
+  it.each([
+    [undefined], // Token has no expiration time
+    [NaN], // Token has no expiration time
+  ])("should reject tokens with no expiration time (expiresAt: %s)", async (expiresAt: number | undefined) => {
+    const noExpirationAuthInfo: AuthInfo = {
+      token: "no-expiration-token",
+      clientId: "client-123",
+      scopes: ["read", "write"],
+      expiresAt
+    };
+    mockVerifyAccessToken.mockResolvedValue(noExpirationAuthInfo);
+
+    mockRequest.headers = {
+      authorization: "Bearer expired-token",
+    };
+
+    const middleware = requireBearerAuth({ verifier: mockVerifier });
+    await middleware(mockRequest as Request, mockResponse as Response, nextFunction);
+
+    expect(mockVerifyAccessToken).toHaveBeenCalledWith("expired-token");
+    expect(mockResponse.status).toHaveBeenCalledWith(401);
+    expect(mockResponse.set).toHaveBeenCalledWith(
+      "WWW-Authenticate",
+      expect.stringContaining('Bearer error="invalid_token"')
+    );
+    expect(mockResponse.json).toHaveBeenCalledWith(
+      expect.objectContaining({ error: "invalid_token", error_description: "Token has no expiration time" })
+    );
+    expect(nextFunction).not.toHaveBeenCalled();
+  });
+
   it("should accept non-expired tokens", async () => {
     const nonExpiredAuthInfo: AuthInfo = {
       token: "valid-token",
@@ -141,6 +177,7 @@ describe("requireBearerAuth middleware", () => {
       token: "valid-token",
       clientId: "client-123",
       scopes: ["read", "write", "admin"],
+      expiresAt: Math.floor(Date.now() / 1000) + 3600, // Token expires in an hour
     };
     mockVerifyAccessToken.mockResolvedValue(authInfo);
 

src/server/auth/middleware/bearerAuth.ts

@@ -63,8 +63,10 @@ export function requireBearerAuth({ verifier, requiredScopes = [], resourceMetad
         }
       }
 
-      // Check if the token is expired
-      if (!!authInfo.expiresAt && authInfo.expiresAt < Date.now() / 1000) {
+      // Check if the token is set to expire or if it is expired
+      if (typeof authInfo.expiresAt !== 'number' || isNaN(authInfo.expiresAt)) {
+        throw new InvalidTokenError("Token has no expiration time");
+      } else if (authInfo.expiresAt < Date.now() / 1000) {
         throw new InvalidTokenError("Token has expired");
       }