diff --git a/.changeset/big-corners-tie.md b/.changeset/big-corners-tie.md new file mode 100644 index 0000000000000..d94dbbfb5daca --- /dev/null +++ b/.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 diff --git a/apps/meteor/app/api/server/v1/users.ts b/apps/meteor/app/api/server/v1/users.ts index 899a6f9de7277..2873bc36fc47b 100644 --- a/apps/meteor/app/api/server/v1/users.ts +++ b/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, @@ -554,6 +554,12 @@ API.v1.post( 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', diff --git a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts index b8fa5c86b3b1f..6734c9bbb781f 100644 --- a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts +++ b/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 } }); diff --git a/apps/meteor/app/oauth2-server-config/server/oauth/oauth2-server.ts b/apps/meteor/app/oauth2-server-config/server/oauth/oauth2-server.ts index 2a2ac65a5f4eb..c5302beb8bfec 100644 --- a/apps/meteor/app/oauth2-server-config/server/oauth/oauth2-server.ts +++ b/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({ diff --git a/apps/meteor/server/oauth2-server/model.ts b/apps/meteor/server/oauth2-server/model.ts index 48e7895f41579..a9ba672854d20 100644 --- a/apps/meteor/server/oauth2-server/model.ts +++ b/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!, diff --git a/apps/meteor/tests/end-to-end/api/oauth-server.ts b/apps/meteor/tests/end-to-end/api/oauth-server.ts index 44a9af2be7323..c35707343a8d3 100644 --- a/apps/meteor/tests/end-to-end/api/oauth-server.ts +++ b/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>; + 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>; + 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'); + }); + }); + }); }); diff --git a/packages/model-typings/src/models/IOAuthAccessTokensModel.ts b/packages/model-typings/src/models/IOAuthAccessTokensModel.ts index 3a7a2d0af9026..5a3ac3ebda02f 100644 --- a/packages/model-typings/src/models/IOAuthAccessTokensModel.ts +++ b/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 { findOneByAccessToken(accessToken: string, options?: FindOptions): Promise; findOneByRefreshToken(refreshToken: string, options?: FindOptions): Promise; + deleteByUserId(userId: string): Promise; + deleteByUserIds(userIds: string[]): Promise; } diff --git a/packages/model-typings/src/models/IOAuthAuthCodesModel.ts b/packages/model-typings/src/models/IOAuthAuthCodesModel.ts index fb457a2686030..01c3da630e632 100644 --- a/packages/model-typings/src/models/IOAuthAuthCodesModel.ts +++ b/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 { findOneByAuthCode(authCode: string, options?: FindOptions): Promise; + deleteByUserId(userId: string): Promise; + deleteByUserIds(userIds: string[]): Promise; } diff --git a/packages/model-typings/src/models/IOAuthRefreshTokensModel.ts b/packages/model-typings/src/models/IOAuthRefreshTokensModel.ts index 03f2b80f5f094..e3692b844df6a 100644 --- a/packages/model-typings/src/models/IOAuthRefreshTokensModel.ts +++ b/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 { findOneByRefreshToken(refreshToken: string, options?: FindOptions): Promise; + deleteByUserId(userId: string): Promise; + deleteByUserIds(userIds: string[]): Promise; } diff --git a/packages/models/src/models/OAuthAccessTokens.ts b/packages/models/src/models/OAuthAccessTokens.ts index d7d9e77000eed..efcf37b40a73e 100644 --- a/packages/models/src/models/OAuthAccessTokens.ts +++ b/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 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 implements } return this.findOne({ refreshToken }, options); } + + async deleteByUserId(userId: string): Promise { + return this.deleteMany({ userId }); + } + + async deleteByUserIds(userIds: string[]): Promise { + return this.deleteMany({ userId: { $in: userIds } }); + } } diff --git a/packages/models/src/models/OAuthAuthCodes.ts b/packages/models/src/models/OAuthAuthCodes.ts index 30124f488686c..e2eef02a39587 100644 --- a/packages/models/src/models/OAuthAuthCodes.ts +++ b/packages/models/src/models/OAuthAuthCodes.ts @@ -1,6 +1,6 @@ import type { IOAuthAuthCode, RocketChatRecordDeleted } from '@rocket.chat/core-typings'; import type { IOAuthAuthCodesModel } 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'; @@ -10,7 +10,7 @@ export class OAuthAuthCodesRaw extends BaseRaw implements IOAuth } override modelIndexes(): IndexDescription[] { - return [{ key: { authCode: 1 } }, { key: { expires: 1 }, expireAfterSeconds: 60 * 5 }]; + return [{ key: { authCode: 1 } }, { key: { userId: 1 } }, { key: { expires: 1 }, expireAfterSeconds: 60 * 5 }]; } findOneByAuthCode(authCode: string, options?: FindOptions): Promise { @@ -19,4 +19,12 @@ export class OAuthAuthCodesRaw extends BaseRaw implements IOAuth } return this.findOne({ authCode }, options); } + + async deleteByUserId(userId: string): Promise { + return this.deleteMany({ userId }); + } + + async deleteByUserIds(userIds: string[]): Promise { + return this.deleteMany({ userId: { $in: userIds } }); + } } diff --git a/packages/models/src/models/OAuthRefreshTokens.ts b/packages/models/src/models/OAuthRefreshTokens.ts index 031dc0fa9dd75..7b48c0ddec7b1 100644 --- a/packages/models/src/models/OAuthRefreshTokens.ts +++ b/packages/models/src/models/OAuthRefreshTokens.ts @@ -1,6 +1,6 @@ import type { IOAuthRefreshToken, RocketChatRecordDeleted } from '@rocket.chat/core-typings'; import type { IOAuthRefreshTokensModel } 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'; @@ -10,7 +10,7 @@ export class OAuthRefreshTokensRaw extends BaseRaw implement } override modelIndexes(): IndexDescription[] { - return [{ key: { refreshToken: 1 } }, { key: { expires: 1 }, expireAfterSeconds: 60 * 60 * 24 * 30 }]; + return [{ key: { refreshToken: 1 } }, { key: { userId: 1 } }, { key: { expires: 1 }, expireAfterSeconds: 60 * 60 * 24 * 30 }]; } findOneByRefreshToken(refreshToken: string, options?: FindOptions): Promise { @@ -19,4 +19,12 @@ export class OAuthRefreshTokensRaw extends BaseRaw implement } return this.findOne({ refreshToken }, options); } + + async deleteByUserId(userId: string): Promise { + return this.deleteMany({ userId }); + } + + async deleteByUserIds(userIds: string[]): Promise { + return this.deleteMany({ userId: { $in: userIds } }); + } }