fix: clean up OAuth tokens after user deactivation

.changeset/big-corners-tie.md

@@ -0,0 +1,7 @@
+---
+'@rocket.chat/model-typings': patch
+'@rocket.chat/models': patch
+'@rocket.chat/meteor': patch
+---
+
+Ensures OAuth tokens are cleaned up after user deactivation

apps/meteor/app/api/server/v1/users.ts

@@ -1,6 +1,6 @@
 import { MeteorError, Team, api, Calendar } from '@rocket.chat/core-services';
 import type { IExportOperation, ILoginToken, IPersonalAccessToken, IUser, UserStatus } from '@rocket.chat/core-typings';
-import { Users, Subscriptions, Sessions } from '@rocket.chat/models';
+import { Users, Subscriptions, Sessions, OAuthAccessTokens, OAuthRefreshTokens, OAuthAuthCodes } from '@rocket.chat/models';
 import {
 	isUserCreateParamsPOST,
 	isUserSetActiveStatusParamsPOST,
@@ -434,6 +434,12 @@ API.v1.addRoute(
 
 			const { modifiedCount: count } = await Users.setActiveNotLoggedInAfterWithRole(lastLoggedIn, role, false);
 
+			await Promise.all([
+				OAuthAccessTokens.deleteByUserIds(ids),
+				OAuthRefreshTokens.deleteByUserIds(ids),
+				OAuthAuthCodes.deleteByUserIds(ids),
+			]);
+
 			ids.forEach((_id) => {
 				void notifyOnUserChange({
 					clientAction: 'updated',

apps/meteor/app/lib/server/functions/setUserActiveStatus.ts

@@ -1,6 +1,6 @@
 import type { IUser, IUserEmail } from '@rocket.chat/core-typings';
 import { isUserFederated, isDirectMessageRoom } from '@rocket.chat/core-typings';
-import { Rooms, Users, Subscriptions } from '@rocket.chat/models';
+import { Rooms, Users, Subscriptions, OAuthAccessTokens, OAuthRefreshTokens, OAuthAuthCodes } from '@rocket.chat/models';
 import { Accounts } from 'meteor/accounts-base';
 import { check } from 'meteor/check';
 import { Meteor } from 'meteor/meteor';
@@ -121,6 +121,11 @@ export async function setUserActiveStatus(
 
 	if (active === false) {
 		await Users.unsetLoginTokens(userId);
+		await Promise.all([
+			OAuthAccessTokens.deleteByUserId(userId),
+			OAuthRefreshTokens.deleteByUserId(userId),
+			OAuthAuthCodes.deleteByUserId(userId),
+		]);
 		await Rooms.setDmReadOnlyByUserId(userId, undefined, true, false);
 
 		void notifyOnUserChange({ clientAction: 'updated', id: userId, diff: { 'services.resume.loginTokens': [], active } });

apps/meteor/app/oauth2-server-config/server/oauth/oauth2-server.ts

@@ -34,9 +34,9 @@ export async function oAuth2ServerAuth(partialRequest: { authorization?: string;
 		return;
 	}
 
-	const user = await Users.findOneById(accessToken.userId);
+	const user = await Users.findOneActiveById(accessToken.userId);
 
-	if (user == null) {
+	if (!user) {
 		return;
 	}
 
@@ -54,8 +54,8 @@ oauth2server.app.get('/oauth/userinfo', async (req: Request, res: Response) => {
 	if (token == null) {
 		return res.status(401).send('Invalid Token');
 	}
-	const user = await Users.findOneById(token.userId);
-	if (user == null) {
+	const user = await Users.findOneActiveById(token.userId);
+	if (!user) {
 		return res.status(401).send('Invalid Token');
 	}
 	return res.send({

apps/meteor/server/oauth2-server/model.ts

@@ -8,7 +8,7 @@ import type {
 	Token,
 	User,
 } from '@node-oauth/oauth2-server';
-import { OAuthApps, OAuthAuthCodes, OAuthAccessTokens, OAuthRefreshTokens } from '@rocket.chat/models';
+import { OAuthApps, OAuthAuthCodes, OAuthAccessTokens, OAuthRefreshTokens, Users } from '@rocket.chat/models';
 
 export type ModelConfig = {
 	debug?: boolean;
@@ -53,6 +53,11 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {
 			throw new Error('Invalid clientId');
 		}
 
+		const user = await Users.findOneActiveById(token.userId, { projection: { _id: 1 } });
+		if (!user) {
+			return;
+		}
+
 		const result: Token = {
 			accessToken: token.accessToken,
 			client,
@@ -228,6 +233,11 @@ export class Model implements AuthorizationCodeModel, RefreshTokenModel {
 			throw new Error('Invalid clientId');
 		}
 
+		const user = await Users.findOneActiveById(token.userId, { projection: { _id: 1 } });
+		if (!user) {
+			throw new Error('Invalid token');
+		}
+
 		const result: RefreshToken = {
 			// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 			refreshToken: token.refreshToken!,

apps/meteor/tests/end-to-end/api/oauth-server.ts

@@ -3,6 +3,40 @@ import { after, before, describe, it } from 'mocha';
 import type { Response } from 'supertest';
 
 import { getCredentials, api, request, credentials } from '../../data/api-data';
+import { password } from '../../data/user';
+import { createUser, deleteUser, login } from '../../data/users.helper';
+
+async function authorizeAndExchange(loginToken: string, cId: string, cSecret: string, redirectUri: string) {
+	const authRes = await request
+		.post(`/oauth/authorize`)
+		.type('form')
+		.send({
+			token: loginToken,
+			client_id: cId,
+			response_type: 'code',
+			redirect_uri: redirectUri,
+			state: 'test-state',
+			allow: 'yes',
+		})
+		.expect(302);
+
+	const location = new URL(authRes.headers.location);
+	const code = location.searchParams.get('code') as string;
+
+	const tokenRes = await request
+		.post(`/oauth/token`)
+		.type('form')
+		.send({
+			grant_type: 'authorization_code',
+			code,
+			client_id: cId,
+			client_secret: cSecret,
+			redirect_uri: redirectUri,
+		})
+		.expect(200);
+
+	return { accessToken: tokenRes.body.access_token as string, refreshToken: tokenRes.body.refresh_token as string };
+}
 
 describe('[OAuth Server]', () => {
 	let oAuthAppId: string;
@@ -228,4 +262,165 @@ describe('[OAuth Server]', () => {
 			});
 		});
 	});
+
+	describe('[user deactivation revokes OAuth tokens]', () => {
+		let testUser: Awaited<ReturnType<typeof createUser>>;
+		let testUserCredentials: { 'X-Auth-Token': string; 'X-User-Id': string };
+		let deactivationClientId: string;
+		let deactivationClientSecret: string;
+		let deactivationAppId: string;
+		let userAccessToken: string;
+		let userRefreshToken: string;
+		const redirectUri = 'http://asd.com';
+
+		before(async () => {
+			testUser = await createUser();
+			testUserCredentials = await login(testUser.username, password);
+
+			const appRes = await request
+				.post(api('oauth-apps.create'))
+				.set(credentials)
+				.send({ name: 'deactivation-test-app', redirectUri: `http://test.com,${redirectUri}`, active: true })
+				.expect(200);
+
+			deactivationAppId = appRes.body.application._id;
+			deactivationClientId = appRes.body.application.clientId;
+			deactivationClientSecret = appRes.body.application.clientSecret;
+
+			const tokens = await authorizeAndExchange(
+				testUserCredentials['X-Auth-Token'],
+				deactivationClientId,
+				deactivationClientSecret,
+				redirectUri,
+			);
+			userAccessToken = tokens.accessToken;
+			userRefreshToken = tokens.refreshToken;
+
+			// Verify tokens work before deactivation
+			await request.get(api('me')).auth(userAccessToken, { type: 'bearer' }).expect(200);
+
+			// Deactivate the user
+			await request.post(api('users.setActiveStatus')).set(credentials).send({ userId: testUser._id, activeStatus: false }).expect(200);
+		});
+
+		after(async () => {
+			await request.post(api('oauth-apps.delete')).set(credentials).send({ appId: deactivationAppId }).expect(200);
+			await deleteUser(testUser);
+		});
+
+		it('should reject the access token after user deactivation', async () => {
+			await request.get(api('me')).auth(userAccessToken, { type: 'bearer' }).expect(401);
+		});
+
+		it('should reject the access token on /oauth/userinfo after user deactivation', async () => {
+			await request.get(`/oauth/userinfo`).auth(userAccessToken, { type: 'bearer' }).expect(401);
+		});
+
+		it('should reject the refresh token grant after user deactivation', async () => {
+			await request
+				.post(`/oauth/token`)
+				.type('form')
+				.send({
+					grant_type: 'refresh_token',
+					refresh_token: userRefreshToken,
+					client_id: deactivationClientId,
+					client_secret: deactivationClientSecret,
+				})
+				.expect((res: Response) => {
+					expect(res.status).to.not.equal(200);
+					expect(res.body).to.have.property('error');
+					expect(res.body).to.not.have.property('access_token');
+				});
+		});
+
+		it('should still reject the access token after user reactivation (token was deleted, not just blocked)', async () => {
+			await request.post(api('users.setActiveStatus')).set(credentials).send({ userId: testUser._id, activeStatus: true }).expect(200);
+
+			await request.get(api('me')).auth(userAccessToken, { type: 'bearer' }).expect(401);
+
+			// Reactivated user can obtain new tokens via a fresh OAuth flow
+			const reactivatedCredentials = await login(testUser.username, password);
+			const newTokens = await authorizeAndExchange(
+				reactivatedCredentials['X-Auth-Token'],
+				deactivationClientId,
+				deactivationClientSecret,
+				redirectUri,
+			);
+
+			await request
+				.get(api('me'))
+				.auth(newTokens.accessToken, { type: 'bearer' })
+				.expect(200)
+				.expect((res: Response) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('_id', testUser._id);
+				});
+		});
+	});
+
+	describe('[users.deactivateIdle revokes OAuth tokens]', () => {
+		let idleUser: Awaited<ReturnType<typeof createUser>>;
+		let idleUserCredentials: { 'X-Auth-Token': string; 'X-User-Id': string };
+		let idleClientId: string;
+		let idleClientSecret: string;
+		let idleAppId: string;
+		let idleAccessToken: string;
+		let idleRefreshToken: string;
+		const redirectUri = 'http://asd.com';
+
+		before(async () => {
+			idleUser = await createUser();
+			idleUserCredentials = await login(idleUser.username, password);
+
+			const appRes = await request
+				.post(api('oauth-apps.create'))
+				.set(credentials)
+				.send({ name: 'idle-deactivation-test-app', redirectUri: `http://test.com,${redirectUri}`, active: true })
+				.expect(200);
+
+			idleAppId = appRes.body.application._id;
+			idleClientId = appRes.body.application.clientId;
+			idleClientSecret = appRes.body.application.clientSecret;
+
+			const tokens = await authorizeAndExchange(idleUserCredentials['X-Auth-Token'], idleClientId, idleClientSecret, redirectUri);
+			idleAccessToken = tokens.accessToken;
+			idleRefreshToken = tokens.refreshToken;
+
+			// Verify tokens work before deactivation
+			await request.get(api('me')).auth(idleAccessToken, { type: 'bearer' }).expect(200);
+
+			// Deactivate via deactivateIdle using daysIdle=0 to catch all users with no recent login
+			await request
+				.post(api('users.deactivateIdle'))
+				.set(credentials)
+				.send({ daysIdle: 0, role: idleUser.roles?.[0] ?? 'user' })
+				.expect(200);
+		});
+
+		after(async () => {
+			await request.post(api('oauth-apps.delete')).set(credentials).send({ appId: idleAppId }).expect(200);
+			await deleteUser(idleUser);
+		});
+
+		it('should reject the access token after idle deactivation', async () => {
+			await request.get(api('me')).auth(idleAccessToken, { type: 'bearer' }).expect(401);
+		});
+
+		it('should reject the refresh token grant after idle deactivation', async () => {
+			await request
+				.post(`/oauth/token`)
+				.type('form')
+				.send({
+					grant_type: 'refresh_token',
+					refresh_token: idleRefreshToken,
+					client_id: idleClientId,
+					client_secret: idleClientSecret,
+				})
+				.expect((res: Response) => {
+					expect(res.status).to.not.equal(200);
+					expect(res.body).to.have.property('error');
+					expect(res.body).to.not.have.property('access_token');
+				});
+		});
+	});
 });

packages/model-typings/src/models/IOAuthAccessTokensModel.ts

@@ -1,9 +1,11 @@
 import type { IOAuthAccessToken } from '@rocket.chat/core-typings';
-import type { FindOptions } from 'mongodb';
+import type { DeleteResult, FindOptions } from 'mongodb';
 
 import type { IBaseModel } from './IBaseModel';
 
 export interface IOAuthAccessTokensModel extends IBaseModel<IOAuthAccessToken> {
 	findOneByAccessToken(accessToken: string, options?: FindOptions<IOAuthAccessToken>): Promise<IOAuthAccessToken | null>;
 	findOneByRefreshToken(refreshToken: string, options?: FindOptions<IOAuthAccessToken>): Promise<IOAuthAccessToken | null>;
+	deleteByUserId(userId: string): Promise<DeleteResult>;
+	deleteByUserIds(userIds: string[]): Promise<DeleteResult>;
 }

packages/model-typings/src/models/IOAuthAuthCodesModel.ts

@@ -1,8 +1,10 @@
 import type { IOAuthAuthCode } from '@rocket.chat/core-typings';
-import type { FindOptions } from 'mongodb';
+import type { DeleteResult, FindOptions } from 'mongodb';
 
 import type { IBaseModel } from './IBaseModel';
 
 export interface IOAuthAuthCodesModel extends IBaseModel<IOAuthAuthCode> {
 	findOneByAuthCode(authCode: string, options?: FindOptions<IOAuthAuthCode>): Promise<IOAuthAuthCode | null>;
+	deleteByUserId(userId: string): Promise<DeleteResult>;
+	deleteByUserIds(userIds: string[]): Promise<DeleteResult>;
 }

packages/model-typings/src/models/IOAuthRefreshTokensModel.ts

@@ -1,8 +1,10 @@
 import type { IOAuthRefreshToken } from '@rocket.chat/core-typings';
-import type { FindOptions } from 'mongodb';
+import type { DeleteResult, FindOptions } from 'mongodb';
 
 import type { IBaseModel } from './IBaseModel';
 
 export interface IOAuthRefreshTokensModel extends IBaseModel<IOAuthRefreshToken> {
 	findOneByRefreshToken(refreshToken: string, options?: FindOptions<IOAuthRefreshToken>): Promise<IOAuthRefreshToken | null>;
+	deleteByUserId(userId: string): Promise<DeleteResult>;
+	deleteByUserIds(userIds: string[]): Promise<DeleteResult>;
 }

packages/models/src/models/OAuthAccessTokens.ts

@@ -1,6 +1,6 @@
 import type { IOAuthAccessToken, RocketChatRecordDeleted } from '@rocket.chat/core-typings';
 import type { IOAuthAccessTokensModel } from '@rocket.chat/model-typings';
-import type { Db, Collection, FindOptions, IndexDescription } from 'mongodb';
+import type { Db, Collection, DeleteResult, FindOptions, IndexDescription } from 'mongodb';
 
 import { BaseRaw } from './BaseRaw';
 
@@ -13,6 +13,7 @@ export class OAuthAccessTokensRaw extends BaseRaw<IOAuthAccessToken> implements
 		return [
 			{ key: { accessToken: 1 } },
 			{ key: { refreshToken: 1 } },
+			{ key: { userId: 1 } },
 			{ key: { expires: 1 }, expireAfterSeconds: 60 * 60 * 24 * 30 },
 			{ key: { refreshTokenExpiresAt: 1 }, expireAfterSeconds: 60 * 60 * 24 * 30 },
 		];
@@ -31,4 +32,12 @@ export class OAuthAccessTokensRaw extends BaseRaw<IOAuthAccessToken> implements
 		}
 		return this.findOne({ refreshToken }, options);
 	}
+
+	async deleteByUserId(userId: string): Promise<DeleteResult> {
+		return this.deleteMany({ userId });
+	}
+
+	async deleteByUserIds(userIds: string[]): Promise<DeleteResult> {
+		return this.deleteMany({ userId: { $in: userIds } });
+	}
 }