fix(core): Fix issue that prevents owner logging in when using ldap (#7408)

This PR prioritises the internal email account over LDAP for the Owner.

---------

Co-authored-by: ricardo <ricardoespinoza105@gmail.com>
This commit is contained in:
Jon 2023-11-02 03:02:49 +00:00 committed by GitHub
parent 437c95e84e
commit 479f90231d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 45 additions and 6 deletions

View file

@ -29,7 +29,7 @@ import {
isLdapCurrentAuthenticationMethod,
setCurrentAuthenticationMethod,
} from '@/sso/ssoHelpers';
import { InternalServerError } from '../ResponseHelper';
import { BadRequestError, InternalServerError } from '../ResponseHelper';
import { RoleService } from '@/services/role.service';
import { Logger } from '@/Logger';
@ -155,6 +155,10 @@ export const updateLdapConfig = async (ldapConfig: LdapConfig): Promise<void> =>
throw new Error(message);
}
if (ldapConfig.loginEnabled && getCurrentAuthenticationMethod() === 'saml') {
throw new BadRequestError('LDAP cannot be enabled if SSO in enabled');
}
LdapManager.updateConfig({ ...ldapConfig });
ldapConfig.bindingAdminPassword = Container.get(Cipher).encrypt(ldapConfig.bindingAdminPassword);

View file

@ -4,6 +4,7 @@ import { compareHash } from '@/UserManagement/UserManagementHelper';
import * as ResponseHelper from '@/ResponseHelper';
import { Container } from 'typedi';
import { InternalHooks } from '@/InternalHooks';
import { isLdapLoginEnabled } from '@/Ldap/helpers';
export const handleEmailLogin = async (
email: string,
@ -21,7 +22,7 @@ export const handleEmailLogin = async (
// At this point if the user has a LDAP ID, means it was previously an LDAP user,
// so suggest to reset the password to gain access to the instance.
const ldapIdentity = user?.authIdentities?.find((i) => i.providerType === 'ldap');
if (user && ldapIdentity) {
if (user && ldapIdentity && !isLdapLoginEnabled()) {
void Container.get(InternalHooks).userLoginFailedDueToLdapDisabled({
user_id: user.id,
});

View file

@ -66,7 +66,13 @@ export class AuthController {
throw new AuthError('SSO is enabled, please log in with SSO');
}
} else if (isLdapCurrentAuthenticationMethod()) {
user = await handleLdapLogin(email, password);
const preliminaryUser = await handleEmailLogin(email, password);
if (preliminaryUser?.globalRole?.name === 'owner') {
user = preliminaryUser;
usedAuthenticationMethod = 'email';
} else {
user = await handleLdapLogin(email, password);
}
} else {
user = await handleEmailLogin(email, password);
}

View file

@ -50,14 +50,12 @@ beforeAll(async () => {
globalMemberRole = fetchedGlobalMemberRole;
owner = await testDb.createUser({ globalRole: globalOwnerRole });
owner = await testDb.createUser({ globalRole: globalOwnerRole, password: 'password' });
authOwnerAgent = testServer.authAgentFor(owner);
defaultLdapConfig.bindingAdminPassword = Container.get(Cipher).encrypt(
defaultLdapConfig.bindingAdminPassword,
);
await setCurrentAuthenticationMethod('email');
});
beforeEach(async () => {
@ -75,6 +73,8 @@ beforeEach(async () => {
jest.mock('@/telemetry');
config.set('userManagement.isInstanceOwnerSetUp', true);
await setCurrentAuthenticationMethod('email');
});
const createLdapConfig = async (attributes: Partial<LdapConfig> = {}): Promise<LdapConfig> => {
@ -145,6 +145,19 @@ describe('PUT /ldap/config', () => {
expect(response.body.data.loginLabel).toBe('');
});
test('route should fail due to trying to enable LDAP login with SSO as current authentication method', async () => {
const validPayload = {
...LDAP_DEFAULT_CONFIGURATION,
loginEnabled: true,
};
config.set('userManagement.authenticationMethod', 'saml');
const response = await authOwnerAgent.put('/ldap/config').send(validPayload);
expect(response.statusCode).toBe(400);
});
test('should apply "Convert all LDAP users to email users" strategy when LDAP login disabled', async () => {
const ldapConfig = await createLdapConfig();
LdapManager.updateConfig(ldapConfig);
@ -471,6 +484,8 @@ describe('POST /login', () => {
const ldapConfig = await createLdapConfig();
LdapManager.updateConfig(ldapConfig);
await setCurrentAuthenticationMethod('ldap');
jest.spyOn(LdapService.prototype, 'searchWithAdminBinding').mockResolvedValue([ldapUser]);
jest.spyOn(LdapService.prototype, 'validUser').mockResolvedValue();
@ -528,6 +543,19 @@ describe('POST /login', () => {
await runTest(ldapUser);
});
test('should allow instance owner to sign in with email/password when LDAP is enabled', async () => {
const ldapConfig = await createLdapConfig();
LdapManager.updateConfig(ldapConfig);
const response = await testServer.authlessAgent
.post('/login')
.send({ email: owner.email, password: 'password' });
expect(response.status).toBe(200);
expect(response.body.data?.signInType).toBeDefined();
expect(response.body.data?.signInType).toBe('email');
});
test('should transform email user into LDAP user when match found', async () => {
const ldapUser = {
mail: randomEmail(),