fix(oidc): fix OIDCAdapter broken flows (#7837)

* fix(oidc): fix OIDCAdapter broken flows

* fix(oidc): fix storage type to include string for userCode index

* test(oidc): add regression tests for OIDCAdapter broken flows

Author: enlightened88

src/node/security/OIDCAdapter.ts

@@ -1,109 +1,110 @@
 import {LRUCache} from 'lru-cache';
 import type {Adapter, AdapterPayload} from "oidc-provider";
 
-
 const options = {
   max: 500,
-  sizeCalculation: (item:any, key:any) => {
-    return 1
-  },
-  // for use with tracking overall storage size
+  sizeCalculation: (item: any, key: any) => 1,
   maxSize: 5000,
-
-  // how long to live in ms
   ttl: 1000 * 60 * 5,
-
-  // return stale items before removing from cache?
   allowStale: false,
-
   updateAgeOnGet: false,
   updateAgeOnHas: false,
 }
 
 const epochTime = (date = Date.now()) => Math.floor(date / 1000);
-
-const storage = new LRUCache<string,AdapterPayload|string[]|string>(options);
+const storage = new LRUCache<string, AdapterPayload | string[] | string>(options);
 
 function grantKeyFor(id: string) {
   return `grant:${id}`;
 }
-
-function userCodeKeyFor(userCode:string) {
+function userCodeKeyFor(userCode: string) {
   return `userCode:${userCode}`;
 }
 
-class MemoryAdapter implements Adapter{
+class MemoryAdapter implements Adapter {
   private readonly name: string;
-  constructor(name:string) {
+
+  constructor(name: string) {
     this.name = name;
   }
 
-  key(id:string) {
+  key(id: string) {
     return `${this.name}:${id}`;
   }
 
-  destroy(id:string) {
+  destroy(id: string) {
     const key = this.key(id);
+    const found = storage.get(key) as AdapterPayload | undefined;
+    const grantId = found?.grantId;
 
-    const found = storage.get(key) as AdapterPayload;
-    const grantId = found && found.grantId;
+    if (found?.userCode) {
+      storage.delete(userCodeKeyFor(found.userCode));
+    }
 
     storage.delete(key);
 
     if (grantId) {
       const grantKey = grantKeyFor(grantId);
-      (storage.get(grantKey) as string[])!.forEach(token => storage.delete(token));
+      const tokens = storage.get(grantKey) as string[] | undefined;
+      tokens?.forEach(token => storage.delete(token));
       storage.delete(grantKey);
     }
 
     return Promise.resolve();
   }
 
   consume(id: string) {
-    (storage.get(this.key(id)) as AdapterPayload)!.consumed = epochTime();
+    const key = this.key(id);
+    const payload = storage.get(key) as AdapterPayload | undefined;
+    if (payload) {
+      payload.consumed = epochTime();
+      storage.set(key, payload);
+    }
     return Promise.resolve();
   }
 
   find(id: string): Promise<AdapterPayload | void | undefined> {
-    if (storage.has(this.key(id))){
-      return Promise.resolve<AdapterPayload>(storage.get(this.key(id)) as AdapterPayload);
+    if (storage.has(this.key(id))) {
+      return Promise.resolve(storage.get(this.key(id)) as AdapterPayload);
     }
-    return Promise.resolve<undefined>(undefined)
+    return Promise.resolve(undefined);
   }
 
   findByUserCode(userCode: string) {
     const id = storage.get(userCodeKeyFor(userCode)) as string;
     return this.find(id);
   }
 
-  upsert(id: string, payload: {
-    iat: number;
-    exp: number;
-    uid: string;
-    kind: string;
-    jti: string;
-    accountId: string;
-    loginTs: number;
-  }, expiresIn: number) {
+  upsert(id: string, payload: AdapterPayload, expiresIn: number) {
     const key = this.key(id);
 
-    storage.set(key, payload, {ttl: expiresIn * 1000});
+    if (payload.grantId) {
+      const grantKey = grantKeyFor(payload.grantId);
+      const grant = (storage.get(grantKey) as string[]) || [];
+      if (!grant.includes(key)) grant.push(key);
+      storage.set(grantKey, grant);
+    }
+
+    if (payload.userCode) {
+      storage.set(userCodeKeyFor(payload.userCode), id);
+    }
 
+    storage.set(key, payload, {ttl: expiresIn * 1000});
     return Promise.resolve();
   }
 
   findByUid(uid: string): Promise<AdapterPayload | void | undefined> {
-    for(const [_, value] of storage.entries()){
-      if(typeof value ==="object" && "uid" in value && value.uid === uid){
-        return Promise.resolve(value);
+    for (const [_, value] of storage.entries()) {
+      if (typeof value === "object" && "uid" in value && value.uid === uid) {
+        return Promise.resolve(value as AdapterPayload);
       }
     }
     return Promise.resolve(undefined);
   }
 
   revokeByGrantId(grantId: string): Promise<void | undefined> {
     const grantKey = grantKeyFor(grantId);
-    const grant = storage.get(grantKey) as string[];
+    const grant = storage.get(grantKey) as string[] | undefined;
     if (grant) {
       grant.forEach((token) => storage.delete(token));
       storage.delete(grantKey);
@@ -112,4 +113,4 @@ class MemoryAdapter implements Adapter{
   }
 }
 
-export default MemoryAdapter
+export default MemoryAdapter;

src/tests/backend/specs/OIDCAdapter.ts

@@ -0,0 +1,57 @@
+/**
+ * Regression tests for OIDCAdapter (MemoryAdapter).
+ *
+ * Covers the four flows that were silently broken before the fix:
+ *   1. upsert → revokeByGrantId actually clears tokens
+ *   2. upsert → findByUserCode returns the payload
+ *   3. destroy clears the userCode mapping
+ *   4. consume on a missing id does not throw
+ */
+
+import {describe, it} from 'mocha';
+import {strict as assert} from 'assert';
+import MemoryAdapter from '../../../node/security/OIDCAdapter';
+
+describe('OIDCAdapter', () => {
+  let adapter: InstanceType<typeof MemoryAdapter>;
+
+  beforeEach(() => {
+    adapter = new MemoryAdapter('Session');
+  });
+
+  it('revokeByGrantId clears all tokens sharing the grant', async () => {
+    const grantId = 'grant-1';
+
+    await adapter.upsert('tok-a', {grantId, jti: 'tok-a'} as any, 60);
+    await adapter.upsert('tok-b', {grantId, jti: 'tok-b'} as any, 60);
+
+    await adapter.revokeByGrantId(grantId);
+
+    assert.equal(await adapter.find('tok-a'), undefined);
+    assert.equal(await adapter.find('tok-b'), undefined);
+  });
+
+  it('findByUserCode returns the payload after upsert', async () => {
+    const userCode = 'USER-CODE-123';
+
+    await adapter.upsert('dc-tok', {userCode, jti: 'dc-tok'} as any, 60);
+
+    const result = await adapter.findByUserCode(userCode);
+    assert.ok(result, 'expected payload to be defined');
+    assert.equal((result as any).jti, 'dc-tok');
+  });
+
+  it('destroy removes the userCode mapping', async () => {
+    const userCode = 'USER-CODE-456';
+
+    await adapter.upsert('dc-tok-2', {userCode, jti: 'dc-tok-2'} as any, 60);
+    await adapter.destroy('dc-tok-2');
+
+    const result = await adapter.findByUserCode(userCode);
+    assert.equal(result, undefined);
+  });
+
+  it('consume on a missing id does not throw', async () => {
+    await assert.doesNotReject(() => adapter.consume('non-existent-id'));
+  });
+});