From 221645576087e4cd828b34ea33e874e1bff5f34a Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Thu, 30 Mar 2023 12:44:53 +0200 Subject: [PATCH] feat(core): Prevent non owners password reset when saml is enabled (#5788) * prevent non owners from pw reset when saml is enabled * improve tests * change error type --- .../controllers/passwordReset.controller.ts | 13 +++++++- .../integration/passwordReset.api.test.ts | 30 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/controllers/passwordReset.controller.ts b/packages/cli/src/controllers/passwordReset.controller.ts index 4b0ec6c9df..53e8401a4a 100644 --- a/packages/cli/src/controllers/passwordReset.controller.ts +++ b/packages/cli/src/controllers/passwordReset.controller.ts @@ -7,6 +7,7 @@ import { BadRequestError, InternalServerError, NotFoundError, + UnauthorizedError, UnprocessableRequestError, } from '@/ResponseHelper'; import { @@ -24,6 +25,7 @@ import { PasswordResetRequest } from '@/requests'; import type { IDatabaseCollections, IExternalHooksClass, IInternalHooksClass } from '@/Interfaces'; import { issueCookie } from '@/auth/jwt'; import { isLdapEnabled } from '@/Ldap/helpers'; +import { isSamlCurrentAuthenticationMethod } from '../sso/ssoHelpers'; @RestController() export class PasswordResetController { @@ -100,9 +102,18 @@ export class PasswordResetController { email, password: Not(IsNull()), }, - relations: ['authIdentities'], + relations: ['authIdentities', 'globalRole'], }); + if (isSamlCurrentAuthenticationMethod() && user?.globalRole.name !== 'owner') { + this.logger.debug( + 'Request to send password reset email failed because login is handled by SAML', + ); + throw new UnauthorizedError( + 'Login is handled by SAML. Please contact your Identity Provider to reset your password.', + ); + } + const ldapIdentity = user?.authIdentities?.find((i) => i.providerType === 'ldap'); if (!user?.password || (ldapIdentity && user.disabled)) { diff --git a/packages/cli/test/integration/passwordReset.api.test.ts b/packages/cli/test/integration/passwordReset.api.test.ts index 38c9042ddf..4ecc12f5f8 100644 --- a/packages/cli/test/integration/passwordReset.api.test.ts +++ b/packages/cli/test/integration/passwordReset.api.test.ts @@ -14,6 +14,7 @@ import { randomValidPassword, } from './shared/random'; import * as testDb from './shared/testDb'; +import { setCurrentAuthenticationMethod } from '@/sso/ssoHelpers'; jest.mock('@/UserManagement/email/NodeMailer'); @@ -74,6 +75,35 @@ describe('POST /forgot-password', () => { expect(storedOwner.resetPasswordToken).toBeNull(); }); + test('should fail if SAML is authentication method', async () => { + await setCurrentAuthenticationMethod('saml'); + config.set('userManagement.emails.mode', 'smtp'); + const member = await testDb.createUser({ + email: 'test@test.com', + globalRole: globalMemberRole, + }); + + await authlessAgent.post('/forgot-password').send({ email: member.email }).expect(403); + + const storedOwner = await Db.collections.User.findOneByOrFail({ email: member.email }); + expect(storedOwner.resetPasswordToken).toBeNull(); + await setCurrentAuthenticationMethod('email'); + }); + + test('should succeed if SAML is authentication method and requestor is owner', async () => { + await setCurrentAuthenticationMethod('saml'); + config.set('userManagement.emails.mode', 'smtp'); + + const response = await authlessAgent.post('/forgot-password').send({ email: owner.email }); + + expect(response.statusCode).toBe(200); + expect(response.body).toEqual({}); + + const storedOwner = await Db.collections.User.findOneByOrFail({ email: owner.email }); + expect(storedOwner.resetPasswordToken).not.toBeNull(); + await setCurrentAuthenticationMethod('email'); + }); + test('should fail with invalid inputs', async () => { config.set('userManagement.emails.mode', 'smtp');