mirror of
https://github.com/n8n-io/n8n.git
synced 2024-11-09 22:24:05 -08:00
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:
parent
437c95e84e
commit
479f90231d
|
@ -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);
|
||||
|
|
|
@ -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,
|
||||
});
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
|
|
@ -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(),
|
||||
|
|
Loading…
Reference in a new issue