diff --git a/frontend/src/component/admin/users/UsersList/ChangePassword/ChangePassword.tsx b/frontend/src/component/admin/users/UsersList/ChangePassword/ChangePassword.tsx index 5253426212..c0ebb44fa5 100644 --- a/frontend/src/component/admin/users/UsersList/ChangePassword/ChangePassword.tsx +++ b/frontend/src/component/admin/users/UsersList/ChangePassword/ChangePassword.tsx @@ -13,6 +13,7 @@ import PasswordMatcher from 'component/user/common/ResetPasswordForm/PasswordMat import type { IUser } from 'interfaces/user'; import useAdminUsersApi from 'hooks/api/actions/useAdminUsersApi/useAdminUsersApi'; import { UserAvatar } from 'component/common/UserAvatar/UserAvatar'; +import useToast from 'hooks/useToast'; const StyledUserAvatar = styled(UserAvatar)(({ theme }) => ({ width: theme.spacing(5), @@ -36,6 +37,7 @@ const ChangePassword = ({ const [validPassword, setValidPassword] = useState(false); const { classes: themeStyles } = useThemeStyles(); const { changePassword } = useAdminUsersApi(); + const { setToastData } = useToast(); const updateField: React.ChangeEventHandler = (event) => { setError(undefined); @@ -58,6 +60,11 @@ const ChangePassword = ({ await changePassword(user.id, data.password); setData({}); closeDialog(); + setToastData({ + title: 'Password changed successfully', + text: 'The user can now sign in using the new password.', + type: 'success', + }); } catch (error: unknown) { console.warn(error); setError(PASSWORD_FORMAT_MESSAGE); diff --git a/frontend/src/hooks/api/actions/useAdminUsersApi/useAdminUsersApi.ts b/frontend/src/hooks/api/actions/useAdminUsersApi/useAdminUsersApi.ts index 9c49931354..ec71c06ded 100644 --- a/frontend/src/hooks/api/actions/useAdminUsersApi/useAdminUsersApi.ts +++ b/frontend/src/hooks/api/actions/useAdminUsersApi/useAdminUsersApi.ts @@ -1,10 +1,4 @@ import useAPI from '../useApi/useApi'; -import { - handleBadRequest, - handleForbidden, - handleNotFound, - handleUnauthorized, -} from './errorHandlers'; interface IUserPayload { name: string; @@ -16,10 +10,7 @@ export const REMOVE_USER_ERROR = 'removeUser'; const useAdminUsersApi = () => { const { loading, makeRequest, createRequest, errors } = useAPI({ - handleBadRequest, - handleNotFound, - handleUnauthorized, - handleForbidden, + propagateErrors: true, }); const addUser = async (user: IUserPayload) => { diff --git a/src/lib/db/user-store.ts b/src/lib/db/user-store.ts index 73a4e1e2ef..f8bf03eaf5 100644 --- a/src/lib/db/user-store.ts +++ b/src/lib/db/user-store.ts @@ -22,6 +22,7 @@ const USER_COLUMNS_PUBLIC = [ 'image_url', 'seen_at', 'is_service', + 'scim_id', ]; const USER_COLUMNS = [...USER_COLUMNS_PUBLIC, 'login_attempts', 'created_at']; @@ -56,6 +57,7 @@ const rowToUser = (row) => { seenAt: row.seen_at, createdAt: row.created_at, isService: row.is_service, + scimId: row.scim_id, }); }; diff --git a/src/lib/openapi/spec/user-schema.ts b/src/lib/openapi/spec/user-schema.ts index ecb092d4df..a3ee176195 100644 --- a/src/lib/openapi/spec/user-schema.ts +++ b/src/lib/openapi/spec/user-schema.ts @@ -92,6 +92,13 @@ export const userSchema = { type: 'string', }, }, + scimId: { + description: + 'The SCIM ID of the user, only present if managed by SCIM', + type: 'string', + nullable: true, + example: '01HTMEXAMPLESCIMID7SWWGHN6', + }, }, components: {}, } as const; diff --git a/src/lib/routes/admin-api/user-admin.ts b/src/lib/routes/admin-api/user-admin.ts index 7aaf145b3d..68406fa150 100644 --- a/src/lib/routes/admin-api/user-admin.ts +++ b/src/lib/routes/admin-api/user-admin.ts @@ -51,7 +51,7 @@ import { type AdminCountSchema, adminCountSchema, } from '../../openapi/spec/admin-count-schema'; -import { BadDataError } from '../../error'; +import { BadDataError, ForbiddenError } from '../../error'; import { createUserResponseSchema, type CreateUserResponseSchema, @@ -81,6 +81,8 @@ export default class UserAdminController extends Controller { readonly unleashUrl: string; + readonly isEnterprise: boolean; + constructor( config: IUnleashConfig, { @@ -116,6 +118,7 @@ export default class UserAdminController extends Controller { this.logger = config.getLogger('routes/user-controller.ts'); this.unleashUrl = config.server.unleashUrl; this.flagResolver = config.flagResolver; + this.isEnterprise = config.isEnterprise; this.route({ method: 'post', @@ -402,6 +405,8 @@ export default class UserAdminController extends Controller { ): Promise { const { user } = req; const receiver = req.body.id; + const receiverUser = await this.userService.getByEmail(receiver); + await this.throwIfScimUser(receiverUser); const resetPasswordUrl = await this.userService.createResetPasswordEmail(receiver, user); @@ -605,6 +610,7 @@ export default class UserAdminController extends Controller { if (!Number.isInteger(Number(id))) { throw new BadDataError('User id should be an integer'); } + await this.throwIfScimUser({ id: Number(id) }); const normalizedRootRole = Number.isInteger(Number(rootRole)) ? Number(rootRole) : (rootRole as RoleName); @@ -636,6 +642,7 @@ export default class UserAdminController extends Controller { if (!Number.isInteger(Number(id))) { throw new BadDataError('User id should be an integer'); } + await this.throwIfScimUser({ id: Number(id) }); await this.userService.deleteUser(+id, user); res.status(200).send(); @@ -658,6 +665,8 @@ export default class UserAdminController extends Controller { const { id } = req.params; const { password } = req.body; + await this.throwIfScimUser({ id: Number(id) }); + await this.userService.changePassword(+id, password); res.status(200).send(); } @@ -716,4 +725,34 @@ export default class UserAdminController extends Controller { projectRoles, }); } + + async throwIfScimUser({ + id, + scimId, + }: Pick): Promise { + if (!this.isEnterprise) return; + if (!this.flagResolver.isEnabled('scimApi')) return; + + const isScimUser = await this.isScimUser({ id, scimId }); + if (!isScimUser) return; + + const { enabled } = await this.settingService.getWithDefault('scim', { + enabled: false, + }); + if (!enabled) return; + + throw new ForbiddenError( + 'This user is managed by your SCIM provider and cannot be changed manually', + ); + } + + async isScimUser({ + id, + scimId, + }: Pick): Promise { + return ( + Boolean(scimId) || + Boolean((await this.userService.getUser(id)).scimId) + ); + } } diff --git a/src/lib/types/user.ts b/src/lib/types/user.ts index e8ac49d2a2..dd5dd15ee2 100644 --- a/src/lib/types/user.ts +++ b/src/lib/types/user.ts @@ -14,6 +14,7 @@ export interface UserData { loginAttempts?: number; createdAt?: Date; isService?: boolean; + scimId?: string; } export interface IUser { @@ -29,6 +30,7 @@ export interface IUser { isAPI: boolean; imageUrl?: string; accountType?: AccountType; + scimId?: string; } export interface IProjectUser extends IUser { @@ -58,6 +60,8 @@ export default class User implements IUser { accountType?: AccountType = 'User'; + scimId?: string; + constructor({ id, name, @@ -68,6 +72,7 @@ export default class User implements IUser { loginAttempts, createdAt, isService, + scimId, }: UserData) { if (!id) { throw new ValidationError('Id is required', [], undefined); @@ -85,6 +90,7 @@ export default class User implements IUser { this.loginAttempts = loginAttempts; this.createdAt = createdAt; this.accountType = isService ? 'Service Account' : 'User'; + this.scimId = scimId; } generateImageUrl(): string { diff --git a/src/test/e2e/api/admin/user-admin.scim.e2e.test.ts b/src/test/e2e/api/admin/user-admin.scim.e2e.test.ts new file mode 100644 index 0000000000..3461ab1f66 --- /dev/null +++ b/src/test/e2e/api/admin/user-admin.scim.e2e.test.ts @@ -0,0 +1,116 @@ +import { + type IUnleashTest, + setupAppWithCustomConfig, +} from '../../helpers/test-helper'; +import dbInit, { type ITestDb } from '../../helpers/database-init'; +import getLogger from '../../../fixtures/no-logger'; +import type { IUnleashStores } from '../../../../lib/types'; + +let stores: IUnleashStores; +let db: ITestDb; +let app: IUnleashTest; + +let scimUserId: number; +let regularUserId: number; + +const scimUser = { + email: 'scim-user@test.com', + name: 'SCIM User', + scim_id: 'some-random-scim-id', +}; + +const regularUser = { + email: 'regular-user@test.com', + name: 'Regular User', +}; + +const scimGuardErrorMessage = + 'This user is managed by your SCIM provider and cannot be changed manually'; + +beforeAll(async () => { + db = await dbInit('user_admin_scim', getLogger); + stores = db.stores; + app = await setupAppWithCustomConfig(stores, { + enterpriseVersion: 'enterprise', + experimental: { + flags: { + strictSchemaValidation: true, + scimApi: true, + }, + }, + }); + + await stores.settingStore.insert('scim', { + enabled: true, + }); + + scimUserId = ( + await db.rawDatabase('users').insert(scimUser).returning('id') + )[0].id; + + regularUserId = ( + await db.rawDatabase('users').insert(regularUser).returning('id') + )[0].id; +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +test('fetching a SCIM user should include scimId', async () => { + const { body } = await app.request + .get(`/api/admin/user-admin/${scimUserId}`) + .expect(200); + + expect(body.email).toBe(scimUser.email); + expect(body.scimId).toBe('some-random-scim-id'); +}); + +test('fetching a regular user should not include scimId', async () => { + const { body } = await app.request + .get(`/api/admin/user-admin/${regularUserId}`) + .expect(200); + + expect(body.email).toBe(regularUser.email); + expect(body.scimId).toBeFalsy(); +}); + +test('should prevent editing a SCIM user', async () => { + const { body } = await app.request + .put(`/api/admin/user-admin/${scimUserId}`) + .send({ + name: 'New name', + }) + .expect(403); + + expect(body.details[0].message).toBe(scimGuardErrorMessage); +}); + +test('should prevent deleting a SCIM user', async () => { + const { body } = await app.request + .delete(`/api/admin/user-admin/${scimUserId}`) + .expect(403); + + expect(body.details[0].message).toBe(scimGuardErrorMessage); +}); + +test('should prevent changing password for a SCIM user', async () => { + const { body } = await app.request + .post(`/api/admin/user-admin/${scimUserId}/change-password`) + .send({ + password: 'new-password', + }) + .expect(403); + + expect(body.details[0].message).toBe(scimGuardErrorMessage); +}); + +test('should prevent resetting password for a SCIM user', async () => { + const { body } = await app.request + .post(`/api/admin/user-admin/reset-password`) + .send({ id: scimUser.email }) + .expect(403); + + expect(body.details[0].message).toBe(scimGuardErrorMessage); +});