From a78865b5e0569d91c2cf3db9a97a218e54edb5d5 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 21 Mar 2026 14:45:53 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=84=20refactor:=20Update=20Token=20Del?= =?UTF-8?q?etion=20Logic=20to=20Use=20AND=20Semantics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Refactored the `deleteTokens` method to delete tokens based on all provided fields using AND conditions instead of OR, enhancing precision in token management. - Updated related tests to reflect the new logic, ensuring that only tokens matching all specified criteria are deleted. - Added new test cases for deleting tokens by type and userId, and for preventing cross-user token deletions, improving overall test coverage and robustness. - Introduced a new `type` field in the `TokenQuery` interface to support the updated deletion functionality. --- .../data-schemas/src/methods/token.spec.ts | 84 +++++++++++++++++-- packages/data-schemas/src/methods/token.ts | 16 ++-- packages/data-schemas/src/types/token.ts | 1 + 3 files changed, 85 insertions(+), 16 deletions(-) diff --git a/packages/data-schemas/src/methods/token.spec.ts b/packages/data-schemas/src/methods/token.spec.ts index e6cf56d18d..87c3916bf8 100644 --- a/packages/data-schemas/src/methods/token.spec.ts +++ b/packages/data-schemas/src/methods/token.spec.ts @@ -566,26 +566,94 @@ describe('Token Methods - Detailed Tests', () => { expect(remainingTokens).toHaveLength(3); }); - test('should delete multiple tokens when they match OR conditions', async () => { - // Create tokens that will match multiple conditions + test('should only delete tokens matching ALL provided fields (AND semantics)', async () => { await Token.create({ - token: 'multi-match', - userId: user2Id, // Will match userId condition + token: 'extra-user2-token', + userId: user2Id, email: 'different@example.com', createdAt: new Date(), expiresAt: new Date(Date.now() + 3600000), }); const result = await methods.deleteTokens({ - token: 'verify-token-1', + token: 'verify-token-2', userId: user2Id.toString(), }); - // Should delete: verify-token-1 (by token) + verify-token-2 (by userId) + multi-match (by userId) - expect(result.deletedCount).toBe(3); + expect(result.deletedCount).toBe(1); const remainingTokens = await Token.find({}); - expect(remainingTokens).toHaveLength(2); + expect(remainingTokens).toHaveLength(4); + expect(remainingTokens.find((t) => t.token === 'verify-token-1')).toBeDefined(); + expect(remainingTokens.find((t) => t.token === 'extra-user2-token')).toBeDefined(); + }); + + test('should delete tokens by type and userId together', async () => { + await Token.create([ + { + token: 'mcp-oauth-token', + userId: oauthUserId, + type: 'mcp_oauth', + identifier: 'mcp:test-server', + createdAt: new Date(), + expiresAt: new Date(Date.now() + 3600000), + }, + { + token: 'mcp-refresh-token', + userId: oauthUserId, + type: 'mcp_oauth_refresh', + identifier: 'mcp:test-server:refresh', + createdAt: new Date(), + expiresAt: new Date(Date.now() + 3600000), + }, + ]); + + const result = await methods.deleteTokens({ + userId: oauthUserId.toString(), + type: 'mcp_oauth', + identifier: 'mcp:test-server', + }); + + expect(result.deletedCount).toBe(1); + + const remaining = await Token.find({ userId: oauthUserId }); + expect(remaining).toHaveLength(2); + expect(remaining.find((t) => t.type === 'mcp_oauth')).toBeUndefined(); + expect(remaining.find((t) => t.type === 'mcp_oauth_refresh')).toBeDefined(); + }); + + test('should not delete cross-user tokens with matching identifier', async () => { + const userAId = new mongoose.Types.ObjectId(); + const userBId = new mongoose.Types.ObjectId(); + + await Token.create([ + { + token: 'user-a-mcp', + userId: userAId, + type: 'mcp_oauth', + identifier: 'mcp:shared-server', + createdAt: new Date(), + expiresAt: new Date(Date.now() + 3600000), + }, + { + token: 'user-b-mcp', + userId: userBId, + type: 'mcp_oauth', + identifier: 'mcp:shared-server', + createdAt: new Date(), + expiresAt: new Date(Date.now() + 3600000), + }, + ]); + + await methods.deleteTokens({ + userId: userAId.toString(), + type: 'mcp_oauth', + identifier: 'mcp:shared-server', + }); + + const userBTokens = await Token.find({ userId: userBId }); + expect(userBTokens).toHaveLength(1); + expect(userBTokens[0].token).toBe('user-b-mcp'); }); test('should throw error when no query parameters provided', async () => { diff --git a/packages/data-schemas/src/methods/token.ts b/packages/data-schemas/src/methods/token.ts index 95fb57e426..a5de3d2a5d 100644 --- a/packages/data-schemas/src/methods/token.ts +++ b/packages/data-schemas/src/methods/token.ts @@ -48,10 +48,7 @@ export function createTokenMethods(mongoose: typeof import('mongoose')) { } } - /** - * Deletes all Token documents that match the provided token, user ID, or email. - * Email is automatically normalized to lowercase for case-insensitive matching. - */ + /** Deletes all Token documents matching every provided field (AND semantics). */ async function deleteTokens(query: TokenQuery): Promise { try { const Token = mongoose.models.Token; @@ -66,19 +63,19 @@ export function createTokenMethods(mongoose: typeof import('mongoose')) { if (query.email !== undefined) { conditions.push({ email: query.email.trim().toLowerCase() }); } + if (query.type !== undefined) { + conditions.push({ type: query.type }); + } if (query.identifier !== undefined) { conditions.push({ identifier: query.identifier }); } - /** - * If no conditions are specified, throw an error to prevent accidental deletion of all tokens - */ if (conditions.length === 0) { throw new Error('At least one query parameter must be provided'); } return await Token.deleteMany({ - $or: conditions, + $and: conditions, }); } catch (error) { logger.debug('An error occurred while deleting tokens:', error); @@ -104,6 +101,9 @@ export function createTokenMethods(mongoose: typeof import('mongoose')) { if (query.email) { conditions.push({ email: query.email.trim().toLowerCase() }); } + if (query.type) { + conditions.push({ type: query.type }); + } if (query.identifier) { conditions.push({ identifier: query.identifier }); } diff --git a/packages/data-schemas/src/types/token.ts b/packages/data-schemas/src/types/token.ts index 063e18a7c9..fd5bfa3a8a 100644 --- a/packages/data-schemas/src/types/token.ts +++ b/packages/data-schemas/src/types/token.ts @@ -26,6 +26,7 @@ export interface TokenQuery { userId?: Types.ObjectId | string; token?: string; email?: string; + type?: string; identifier?: string; }