🔄 refactor: Update Token Deletion Logic to Use AND Semantics

- 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.
This commit is contained in:
Danny Avila 2026-03-21 14:45:53 -04:00
parent 150361273f
commit a78865b5e0
No known key found for this signature in database
GPG key ID: BF31EEB2C5CA0956
3 changed files with 85 additions and 16 deletions

View file

@ -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 () => {

View file

@ -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<TokenDeleteResult> {
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 });
}

View file

@ -26,6 +26,7 @@ export interface TokenQuery {
userId?: Types.ObjectId | string;
token?: string;
email?: string;
type?: string;
identifier?: string;
}