Skip to content

fix: clean up login tokens in users.deactivateidle #40570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
julio-rocketchat merged 1 commit into from
May 16, 2026
Merged

fix: clean up login tokens in users.deactivateidle #40570

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/good-rules-lie.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@rocket.chat/model-typings': patch
'@rocket.chat/models': patch
'@rocket.chat/meteor': patch
---

Ensures that deactivated users have their login tokens cleaned up in users.deactivateidle
13 changes: 12 additions & 1 deletion apps/meteor/app/api/server/v1/users.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,9 +420,20 @@ API.v1.addRoute(
const lastLoggedIn = new Date();
lastLoggedIn.setDate(lastLoggedIn.getDate() - daysIdle);

// since we're deactiving users that are not logged in, there is no need to send data through WS
const ids = await Users.findActiveNotLoggedInAfterWithRole(lastLoggedIn, role, { projection: { _id: 1 } })

@cubic-dev-ai cubic-dev-ai Bot May 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The new read-then-update flow is non-atomic and can send watch.users updates for users that were not actually deactivated.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/api/server/v1/users.ts, line 423:

<comment>The new read-then-update flow is non-atomic and can send `watch.users` updates for users that were not actually deactivated.</comment>

<file context>
@@ -420,9 +420,20 @@ API.v1.addRoute(
 			lastLoggedIn.setDate(lastLoggedIn.getDate() - daysIdle);
 
-			// since we're deactiving users that are not logged in, there is no need to send data through WS
+			const ids = await Users.findActiveNotLoggedInAfterWithRole(lastLoggedIn, role, { projection: { _id: 1 } })
+				.map(({ _id }: { _id: string }) => _id)
+				.toArray();
</file context>

.map(({ _id }: { _id: string }) => _id)
.toArray();

const { modifiedCount: count } = await Users.setActiveNotLoggedInAfterWithRole(lastLoggedIn, role, false);

ids.forEach((_id) => {
void notifyOnUserChange({
clientAction: 'updated',
id: _id,
diff: { 'services.resume.loginTokens': [], 'active': false },
});
});

return API.v1.success({
count,
});
Expand Down
23 changes: 23 additions & 0 deletions apps/meteor/tests/end-to-end/api/users.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4012,6 +4012,29 @@ describe('[Users]', () => {
.end(done);
});
});

it('should revoke login tokens of deactivated idle users', async () => {
const idleUser = await createUser();
await request.post(api('roles.addUserToRole')).set(credentials).send({ roleId: testRoleId, username: idleUser.username }).expect(200);

const idleUserCredentials = await login(idleUser.username, password);
await request.get(api('me')).set(idleUserCredentials).expect(200);

await updatePermission('edit-other-user-active-status', ['admin']);
await request
.post(api('users.deactivateIdle'))
.set(credentials)
.send({ daysIdle: 0, role: testRoleId })
.expect(200)
.expect((res: Response) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('count').that.is.greaterThan(0);
});

await request.get(api('me')).set(idleUserCredentials).expect(401);

await deleteUser(idleUser);
});
});

describe('[/users.requestDataDownload]', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/model-typings/src/models/IUsersModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ export interface IUsersModel extends IBaseModel<IUser> {
setUserActive(userId: string, active: boolean): Promise<UpdateResult>;
setAllUsersActive(active: boolean): Promise<UpdateResult | Document>;
setActiveNotLoggedInAfterWithRole(latestLastLoginDate: Date, role?: string, active?: boolean): Promise<UpdateResult | Document>;
findActiveNotLoggedInAfterWithRole(latestLastLoginDate: Date, role?: string, options?: FindOptions<IUser>): FindCursor<IUser>;
unsetRequirePasswordChange(userId: string): Promise<UpdateResult>;
resetPasswordAndSetRequirePasswordChange(
userId: string,
Expand Down
16 changes: 15 additions & 1 deletion packages/models/src/models/Users.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2982,15 +2982,29 @@ export class UsersRaw extends BaseRaw<IUser, DefaultFields<IUser>> implements IU
roles: role,
};

const update = {
const update: UpdateFilter<IUser> = {
$set: {
active,
...(!active && { 'services.resume.loginTokens': [] }),
},
};

return this.updateMany(query, update);
}

findActiveNotLoggedInAfterWithRole(latestLastLoginDate: Date, role: IRole['_id'] = 'user', options: FindOptions<IUser> = {}) {
const neverActive = { lastLogin: { $exists: false }, createdAt: { $lte: latestLastLoginDate } };
const idleTooLong = { lastLogin: { $lte: latestLastLoginDate } };

const query = {
$or: [neverActive, idleTooLong],
active: true,
roles: role,
};

return this.find(query, options);
}

unsetRequirePasswordChange(_id: IUser['_id']) {
const update: UpdateFilter<IUser> = {
$unset: {
Expand Down
Loading