0
Fork 0
mirror of https://github.com/logto-io/logto.git synced 2024-12-16 20:26:19 -05:00

fix(core): should sign out user after deletion or suspension (#5857)

fixed #5572
This commit is contained in:
wangsijie 2024-05-14 16:10:31 +08:00 committed by GitHub
parent 1e24843a28
commit 5660c54cb5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 44 additions and 9 deletions

View file

@ -0,0 +1,7 @@
---
"@logto/core": patch
---
Sign out user after deletion or suspension
When a user is deleted or suspended through Management API, they should be signed out immediately, including sessions and refresh tokens.

View file

@ -89,6 +89,7 @@ export const createUserLibrary = (queries: Queries) => {
rolesScopes: { findRolesScopesByRoleIds }, rolesScopes: { findRolesScopesByRoleIds },
scopes: { findScopesByIdsAndResourceIndicator }, scopes: { findScopesByIdsAndResourceIndicator },
organizations, organizations,
oidcModelInstances: { revokeInstanceByUserId },
} = queries; } = queries;
const generateUserId = async (retries = 500) => const generateUserId = async (retries = 500) =>
@ -270,6 +271,14 @@ export const createUserLibrary = (queries: Queries) => {
return user; return user;
}; };
const signOutUser = async (userId: string) => {
await Promise.all([
revokeInstanceByUserId('AccessToken', userId),
revokeInstanceByUserId('RefreshToken', userId),
revokeInstanceByUserId('Session', userId),
]);
};
return { return {
generateUserId, generateUserId,
insertUser, insertUser,
@ -279,5 +288,6 @@ export const createUserLibrary = (queries: Queries) => {
findUserRoles, findUserRoles,
addUserMfaVerification, addUserMfaVerification,
verifyUserPassword, verifyUserPassword,
signOutUser,
}; };
}; };

View file

@ -14,7 +14,6 @@ const { jest } = import.meta;
const { mockEsmWithActual } = createMockUtils(jest); const { mockEsmWithActual } = createMockUtils(jest);
const mockedQueries = { const mockedQueries = {
oidcModelInstances: { revokeInstanceByUserId: jest.fn() },
signInExperiences: { signInExperiences: {
findDefaultSignInExperience: jest.fn( findDefaultSignInExperience: jest.fn(
async () => async () =>
@ -65,7 +64,6 @@ const mockHasUser = jest.fn(async () => false);
const mockHasUserWithEmail = jest.fn(async () => false); const mockHasUserWithEmail = jest.fn(async () => false);
const mockHasUserWithPhone = jest.fn(async () => false); const mockHasUserWithPhone = jest.fn(async () => false);
const { revokeInstanceByUserId } = mockedQueries.oidcModelInstances;
const { hasUser, findUserById, updateUserById, deleteUserIdentity, deleteUserById } = const { hasUser, findUserById, updateUserById, deleteUserIdentity, deleteUserById } =
mockedQueries.users; mockedQueries.users;
@ -77,6 +75,7 @@ const { encryptUserPassword } = await mockEsmWithActual('#src/libraries/user.js'
})); }));
const verifyUserPassword = jest.fn(); const verifyUserPassword = jest.fn();
const signOutUser = jest.fn();
const usersLibraries = { const usersLibraries = {
generateUserId: jest.fn(async () => 'fooId'), generateUserId: jest.fn(async () => 'fooId'),
insertUser: jest.fn( insertUser: jest.fn(
@ -86,6 +85,7 @@ const usersLibraries = {
}) })
), ),
verifyUserPassword, verifyUserPassword,
signOutUser,
} satisfies Partial<Libraries['users']>; } satisfies Partial<Libraries['users']>;
const adminUserRoutes = await pickDefault(import('./basics.js')); const adminUserRoutes = await pickDefault(import('./basics.js'));
@ -377,7 +377,7 @@ describe('adminUserRoutes', () => {
.patch(`/users/${mockedUserId}/is-suspended`) .patch(`/users/${mockedUserId}/is-suspended`)
.send({ isSuspended: true }); .send({ isSuspended: true });
expect(updateUserById).toHaveBeenCalledWith(mockedUserId, { isSuspended: true }); expect(updateUserById).toHaveBeenCalledWith(mockedUserId, { isSuspended: true });
expect(revokeInstanceByUserId).toHaveBeenCalledWith('refreshToken', mockedUserId); expect(signOutUser).toHaveBeenCalledWith(mockedUserId);
expect(response.status).toEqual(200); expect(response.status).toEqual(200);
expect(response.body).toEqual({ expect(response.body).toEqual({
...mockUserResponse, ...mockUserResponse,
@ -389,6 +389,7 @@ describe('adminUserRoutes', () => {
const userId = 'fooUser'; const userId = 'fooUser';
const response = await userRequest.delete(`/users/${userId}`); const response = await userRequest.delete(`/users/${userId}`);
expect(response.status).toEqual(204); expect(response.status).toEqual(204);
expect(signOutUser).toHaveBeenCalledWith(userId);
}); });
it('DELETE /users/:userId should throw if user is deleting self', async () => { it('DELETE /users/:userId should throw if user is deleting self', async () => {

View file

@ -1,3 +1,4 @@
/* eslint-disable max-lines */
import { emailRegEx, phoneRegEx, usernameRegEx } from '@logto/core-kit'; import { emailRegEx, phoneRegEx, usernameRegEx } from '@logto/core-kit';
import { import {
UsersPasswordEncryptionMethod, UsersPasswordEncryptionMethod,
@ -21,7 +22,6 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
) { ) {
const [router, { queries, libraries }] = args; const [router, { queries, libraries }] = args;
const { const {
oidcModelInstances: { revokeInstanceByUserId },
users: { users: {
deleteUserById, deleteUserById,
findUserById, findUserById,
@ -33,7 +33,13 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
userSsoIdentities, userSsoIdentities,
} = queries; } = queries;
const { const {
users: { checkIdentifierCollision, generateUserId, insertUser, verifyUserPassword }, users: {
checkIdentifierCollision,
generateUserId,
insertUser,
verifyUserPassword,
signOutUser,
},
} = libraries; } = libraries;
router.get( router.get(
@ -343,12 +349,11 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
}); });
if (isSuspended) { if (isSuspended) {
await revokeInstanceByUserId('refreshToken', user.id); await signOutUser(user.id);
} }
ctx.body = pick(user, ...userInfoSelectFields); ctx.body = pick(user, ...userInfoSelectFields);
// eslint-disable-next-line max-lines
return next(); return next();
} }
); );
@ -368,6 +373,7 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
throw new RequestError('user.cannot_delete_self'); throw new RequestError('user.cannot_delete_self');
} }
await signOutUser(userId);
await deleteUserById(userId); await deleteUserById(userId);
ctx.status = 204; ctx.status = 204;
@ -376,3 +382,4 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
} }
); );
} }
/* eslint-enable max-lines */

View file

@ -22,7 +22,11 @@ import {
} from '#src/api/index.js'; } from '#src/api/index.js';
import { clearConnectorsByTypes } from '#src/helpers/connector.js'; import { clearConnectorsByTypes } from '#src/helpers/connector.js';
import { createUserByAdmin, expectRejects } from '#src/helpers/index.js'; import { createUserByAdmin, expectRejects } from '#src/helpers/index.js';
import { createNewSocialUserWithUsernameAndPassword } from '#src/helpers/interactions.js'; import {
createNewSocialUserWithUsernameAndPassword,
signInWithPassword,
} from '#src/helpers/interactions.js';
import { enableAllPasswordSignInMethods } from '#src/helpers/sign-in-experience.js';
import { import {
generateUsername, generateUsername,
generateEmail, generateEmail,
@ -177,7 +181,9 @@ describe('admin console user management', () => {
}); });
it('should delete user successfully', async () => { it('should delete user successfully', async () => {
const user = await createUserByAdmin(); const username = generateUsername();
const password = 'password';
const user = await createUserByAdmin({ username, password });
const userEntity = await getUser(user.id); const userEntity = await getUser(user.id);
expect(userEntity).toMatchObject(user); expect(userEntity).toMatchObject(user);
@ -186,6 +192,10 @@ describe('admin console user management', () => {
const response = await getUser(user.id).catch((error: unknown) => error); const response = await getUser(user.id).catch((error: unknown) => error);
expect(response instanceof HTTPError && response.response.status === 404).toBe(true); expect(response instanceof HTTPError && response.response.status === 404).toBe(true);
await enableAllPasswordSignInMethods();
// Sign in with deleted user should throw error
await expect(signInWithPassword({ username, password })).rejects.toThrowError();
}); });
it('should update user password successfully', async () => { it('should update user password successfully', async () => {