TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2024-12-12 02:54:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: allow usernames without @ when UserMustBeDomain false (#4852)

Browse Source 
* feat: allow usernames without @ when UserMustBeDomain false

* e2e

* test(e2e): table driven tests for humans and machines

* cleanup

* fix(e2e): ensure there are no username conflicts

* e2e: make awaitDesired async

* rm settings mapping

* e2e: make awaitDesired async

* e2e: parse sequence as int

* e2e: ensure test fails if awaitDesired fails

Co-authored-by: Max Peintner <max@caos.ch>
This commit is contained in:
Livio Spring 2022-12-22 12:16:17 +01:00 committed by GitHub
parent 7d9fc2c6e7
commit 0530f19d94
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 498 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
e2e/cypress/e2e/humans/humans.cy.ts
View File

@ -1,19 +1,24 @@
import { apiAuth } from '../../support/api/apiauth';
import { ensureHumanUserExists, ensureUserDoesntExist } from '../../support/api/users';
import { loginname } from '../../support/login/users';
import { ensureDomainPolicy } from '../../support/api/policies';
describe('humans', () => {
const humansPath = `/users?type=human`;
const testHumanUserNameAdd = 'e2ehumanusernameadd';
const testHumanUserNameRemove = 'e2ehumanusernameremove';
[
{ mustBeDomain: false, addName: 'e2ehumanusernameaddGlobal', removeName: 'e2ehumanusernameremoveGlobal' },
{ mustBeDomain: false, addName: 'e2ehumanusernameadd@test.com', removeName: 'e2ehumanusernameremove@test.com' },
{ mustBeDomain: true, addName: 'e2ehumanusernameadd', removeName: 'e2ehumanusernameremove' },
].forEach((user) => {
beforeEach(() => {
apiAuth().as('api');
});
describe('add', () => {
describe(`add "${user.addName}" with domain setting "${user.mustBeDomain}"`, () => {
beforeEach(`ensure it doesn't exist already`, function () {
ensureUserDoesntExist(this.api, loginname(testHumanUserNameAdd, Cypress.env('ORGANIZATION')));
ensureDomainPolicy(this.api, user.mustBeDomain, true, false);
ensureUserDoesntExist(this.api, user.addName);
cy.visit(humansPath);
});
@ -22,35 +27,41 @@ describe('humans', () => {
cy.url().should('contain', 'users/create');
cy.get('[formcontrolname="email"]').type('dummy@dummy.com');
//force needed due to the prefilled username prefix
cy.get('[formcontrolname="userName"]').type(loginname(testHumanUserNameAdd, Cypress.env('ORGANIZATION')));
cy.get('[formcontrolname="userName"]').type(user.addName);
cy.get('[formcontrolname="firstName"]').type('e2ehumanfirstname');
cy.get('[formcontrolname="lastName"]').type('e2ehumanlastname');
cy.get('[formcontrolname="phone"]').type('+41 123456789');
cy.get('[data-e2e="create-button"]').click();
cy.get('.data-e2e-success');
const loginName = loginname(testHumanUserNameAdd, Cypress.env('ORGANIZATION'));
let loginName = user.addName;
if (user.mustBeDomain) {
loginName = loginname(user.addName, Cypress.env('ORGANIZATION'));
}
cy.contains('[data-e2e="copy-loginname"]', loginName).click();
cy.clipboardMatches(loginName);
cy.shouldNotExist({ selector: '.data-e2e-failure' });
});
});
describe('remove', () => {
describe(`remove "${user.removeName}" with domain setting "${user.mustBeDomain}"`, () => {
beforeEach('ensure it exists', function () {
ensureHumanUserExists(this.api, loginname(testHumanUserNameRemove, Cypress.env('ORGANIZATION')));
ensureHumanUserExists(this.api, user.removeName);
cy.visit(humansPath);
});
let loginName = user.removeName;
if (user.mustBeDomain) {
loginName = loginname(user.removeName, Cypress.env('ORGANIZATION'));
}
it('should delete a human user', () => {
const rowSelector = `tr:contains(${testHumanUserNameRemove})`;
const rowSelector = `tr:contains(${user.removeName})`;
cy.get(rowSelector).find('[data-e2e="enabled-delete-button"]').click({ force: true });
cy.get('[data-e2e="confirm-dialog-input"]')
.focus()
.type(loginname(testHumanUserNameRemove, Cypress.env('ORGANIZATION')));
cy.get('[data-e2e="confirm-dialog-input"]').focus().type(loginName);
cy.get('[data-e2e="confirm-dialog-button"]').click();
cy.get('.data-e2e-success');
cy.shouldNotExist({ selector: rowSelector, timeout: 2000 });
cy.shouldNotExist({ selector: '.data-e2e-failure' });
});
});
});
});

40
e2e/cypress/e2e/machines/machines.cy.ts
View File

@ -1,19 +1,24 @@
import { apiAuth } from '../../support/api/apiauth';
import { ensureMachineUserExists, ensureUserDoesntExist } from '../../support/api/users';
import { loginname } from '../../support/login/users';
import { ensureDomainPolicy } from '../../support/api/policies';
describe('machines', () => {
const machinesPath = `/users?type=machine`;
beforeEach(() => {
apiAuth().as('api');
});
const machinesPath = `/users?type=machine`;
const testMachineUserNameAdd = 'e2emachineusernameadd';
const testMachineUserNameRemove = 'e2emachineusernameremove';
describe('add', () => {
[
{ mustBeDomain: false, addName: 'e2emachineusernameaddGlobal', removeName: 'e2emachineusernameremoveGlobal' },
{ mustBeDomain: false, addName: 'e2emachineusernameadd@test.com', removeName: 'e2emachineusernameremove@test.com' },
{ mustBeDomain: true, addName: 'e2emachineusernameadd', removeName: 'e2emachineusernameremove' },
].forEach((machine) => {
describe(`add "${machine.addName}" with domain setting "${machine.mustBeDomain}"`, () => {
beforeEach(`ensure it doesn't exist already`, function () {
ensureUserDoesntExist(this.api, testMachineUserNameAdd);
ensureDomainPolicy(this.api, machine.mustBeDomain, false, false);
ensureUserDoesntExist(this.api, machine.addName);
cy.visit(machinesPath);
});
@ -21,27 +26,35 @@ describe('machines', () => {
cy.get('[data-e2e="create-user-button"]').click();
cy.url().should('contain', 'users/create-machine');
//force needed due to the prefilled username prefix
cy.get('[formcontrolname="userName"]').type(testMachineUserNameAdd);
cy.get('[formcontrolname="userName"]').type(machine.addName);
cy.get('[formcontrolname="name"]').type('e2emachinename');
cy.get('[formcontrolname="description"]').type('e2emachinedescription');
cy.get('[data-e2e="create-button"]').click();
cy.get('.data-e2e-success');
cy.contains('[data-e2e="copy-loginname"]', testMachineUserNameAdd).click();
cy.clipboardMatches(testMachineUserNameAdd);
let loginName = machine.addName;
if (machine.mustBeDomain) {
loginName = loginname(machine.addName, Cypress.env('ORGANIZATION'));
}
cy.contains('[data-e2e="copy-loginname"]', loginName).click();
cy.clipboardMatches(loginName);
cy.shouldNotExist({ selector: '.data-e2e-failure' });
});
});
describe('edit', () => {
describe(`remove "${machine.removeName}" with domain setting "${machine.mustBeDomain}"`, () => {
beforeEach('ensure it exists', function () {
ensureMachineUserExists(this.api, testMachineUserNameRemove);
ensureMachineUserExists(this.api, machine.removeName);
cy.visit(machinesPath);
});
let loginName = machine.removeName;
if (machine.mustBeDomain) {
loginName = loginname(machine.removeName, Cypress.env('ORGANIZATION'));
}
it('should delete a machine', () => {
const rowSelector = `tr:contains(${testMachineUserNameRemove})`;
const rowSelector = `tr:contains(${machine.removeName})`;
cy.get(rowSelector).find('[data-e2e="enabled-delete-button"]').click({ force: true });
cy.get('[data-e2e="confirm-dialog-input"]').focus().type(testMachineUserNameRemove);
cy.get('[data-e2e="confirm-dialog-input"]').focus().type(loginName);
cy.get('[data-e2e="confirm-dialog-button"]').click();
cy.get('.data-e2e-success');
cy.shouldNotExist({ selector: rowSelector, timeout: 2000 });
@ -50,4 +63,5 @@ describe('machines', () => {
it('should create a personal access token');
});
});
});

11
e2e/cypress/support/api/ensure.ts
View File

@ -56,7 +56,6 @@ export function ensureSetting(
'PUT',
body,
(entity) => !!entity,
(body) => body?.settings?.id,
);
}
@ -66,14 +65,15 @@ function awaitDesired(
search: () => Cypress.Chainable<SearchResult>,
initialSequence?: number,
) {
search().then((resp) => {
return search().then((resp) => {
const foundExpectedEntity = expectEntity(resp.entity);
const foundExpectedSequence = !initialSequence || resp.sequence >= initialSequence;
if (!foundExpectedEntity || !foundExpectedSequence) {
const check = !foundExpectedEntity || !foundExpectedSequence;
if (check) {
expect(trials, `trying ${trials} more times`).to.be.greaterThan(0);
cy.wait(1000);
awaitDesired(trials - 1, expectEntity, search, initialSequence);
return awaitDesired(trials - 1, expectEntity, search, initialSequence);
}
});
}
@ -117,7 +117,8 @@ export function ensureSomething(
});
})
.then((data) => {
awaitDesired(90, expectEntity, search, data.sequence);
return awaitDesired(90, expectEntity, search, data.sequence).then(() => {
return cy.wrap<number>(data.id);
});
});
}

4
e2e/cypress/support/api/orgs.ts
View File

@ -12,7 +12,7 @@ export function ensureOrgExists(api: API, name: string): Cypress.Chainable<numbe
encodeURI(`${api.mgmtBaseURL}/global/orgs/_by_domain?domain=${name}.${host(Cypress.config('baseUrl'))}`),
'GET',
(res) => {
return { entity: res.org, id: res.org?.id, sequence: res.org?.details?.sequence };
return { entity: res.org, id: res.org?.id, sequence: parseInt(<string>res.org?.details?.sequence) };
},
),
() => `${api.mgmtBaseURL}/orgs`,
@ -25,6 +25,6 @@ export function ensureOrgExists(api: API, name: string): Cypress.Chainable<numbe
export function getOrgUnderTest(api: API): Cypress.Chainable<number> {
return searchSomething(api, `${api.mgmtBaseURL}/orgs/me`, 'GET', (res) => {
return { entity: res.org, id: res.org.id, sequence: res.org.details.sequence };
return { entity: res.org, id: res.org.id, sequence: parseInt(<string>res.org.details.sequence) };
}).then((res) => res.entity.id);
}

36
e2e/cypress/support/api/policies.ts
View File

@ -1,5 +1,6 @@
import { requestHeaders } from './apiauth';
import { API } from './types';
import { ensureSetting } from './ensure';
export enum Policy {
Label = 'label',
@ -15,3 +16,38 @@ export function resetPolicy(api: API, policy: Policy) {
return null;
});
}
export function ensureDomainPolicy(
api: API,
userLoginMustBeDomain: boolean,
validateOrgDomains: boolean,
smtpSenderAddressMatchesInstanceDomain: boolean,
): Cypress.Chainable<number> {
return ensureSetting(
api,
`${api.adminBaseURL}/policies/domain`,
(body: any) => {
const result = {
sequence: parseInt(<string>body.policy?.details?.sequence),
id: body.policy?.details?.resourceOwner,
entity: null,
};
if (
body.policy &&
(body.policy.userLoginMustBeDomain ? body.policy.userLoginMustBeDomain : false) == userLoginMustBeDomain &&
(body.policy.validateOrgDomains ? body.policy.validateOrgDomains : false) == validateOrgDomains &&
(body.policy.smtpSenderAddressMatchesInstanceDomain ? body.policy.smtpSenderAddressMatchesInstanceDomain : false) ==
smtpSenderAddressMatchesInstanceDomain
) {
return { ...result, entity: body.policy };
}
return result;
},
`${api.adminBaseURL}/policies/domain`,
{
userLoginMustBeDomain: userLoginMustBeDomain,
validateOrgDomains: validateOrgDomains,
smtpSenderAddressMatchesInstanceDomain: smtpSenderAddressMatchesInstanceDomain,
},
);
}

6
internal/api/ui/login/external_login_handler.go
View File

@ -474,9 +474,9 @@ func (l *Login) mapExternalUserToLoginUser(orgIamPolicy *query.DomainPolicy, lin
}
if orgIamPolicy.UserLoginMustBeDomain {
splittedUsername := strings.Split(username, "@")
if len(splittedUsername) > 1 {
username = splittedUsername[0]
index := strings.LastIndex(username, "@")
if index > 1 {
username = username[:index]
}
}

6
internal/auth/repository/eventsourcing/eventstore/auth_request.go
View File

@ -700,12 +700,12 @@ func (repo *AuthRequestRepo) checkLoginName(ctx context.Context, request *domain
func (repo *AuthRequestRepo) checkDomainDiscovery(ctx context.Context, request *domain.AuthRequest, loginName string) bool {
// check if there's a suffix in the loginname
split := strings.Split(loginName, "@")
if len(split) < 2 {
index := strings.LastIndex(loginName, "@")
if index < 0 {
return false
}
// check if the suffix matches a verified domain
org, err := repo.Query.OrgByVerifiedDomain(ctx, split[len(split)-1])
org, err := repo.Query.OrgByVerifiedDomain(ctx, loginName[index+1:])
if err != nil {
return false
}

17
internal/command/user_human.go
View File

@ -235,12 +235,12 @@ func userValidateDomain(ctx context.Context, a *user.Aggregate, username string,
return nil
}
usernameSplit := strings.Split(username, "@")
if len(usernameSplit) != 2 {
return errors.ThrowInvalidArgument(nil, "COMMAND-Dfd21", "Errors.User.Invalid")
index := strings.LastIndex(username, "@")
if index < 0 {
return nil
}
domainCheck := NewOrgDomainVerifiedWriteModel(usernameSplit[1])
domainCheck := NewOrgDomainVerifiedWriteModel(username[index+1:])
events, err := filter(ctx, domainCheck.Query())
if err != nil {
return err
@ -443,11 +443,9 @@ func (c *Commands) createHuman(ctx context.Context, orgID string, human *domain.
human.Username = strings.TrimSpace(human.Username)
human.EmailAddress = strings.TrimSpace(human.EmailAddress)
if !domainPolicy.UserLoginMustBeDomain {
usernameSplit := strings.Split(human.Username, "@")
if len(usernameSplit) != 2 {
return nil, nil, errors.ThrowInvalidArgument(nil, "COMMAND-Dfd21", "Errors.User.Invalid")
}
domainCheck := NewOrgDomainVerifiedWriteModel(usernameSplit[1])
index := strings.LastIndex(human.Username, "@")
if index > 1 {
domainCheck := NewOrgDomainVerifiedWriteModel(human.Username[index+1:])
if err := c.eventstore.FilterToQueryReducer(ctx, domainCheck); err != nil {
return nil, nil, err
}
@ -455,6 +453,7 @@ func (c *Commands) createHuman(ctx context.Context, orgID string, human *domain.
return nil, nil, errors.ThrowInvalidArgument(nil, "COMMAND-SFd21", "Errors.User.DomainNotAllowedAsUsername")
}
}
}
if human.AggregateID == "" {
userID, err := c.idGenerator.Next()

330
internal/command/user_human_test.go
View File

@ -480,6 +480,224 @@ func TestCommandSide_AddHuman(t *testing.T) {
},
},
},
{
name: "add human, email verified, userLoginMustBeDomain false, ok",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
org.NewDomainPolicyAddedEvent(context.Background(),
&userAgg.Aggregate,
false,
true,
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewPasswordComplexityPolicyAddedEvent(context.Background(),
&userAgg.Aggregate,
1,
false,
false,
false,
false,
),
),
),
expectPush(
[]*repository.Event{
eventFromEventPusher(
newAddHumanEvent("password", true, ""),
),
eventFromEventPusher(
user.NewHumanEmailVerifiedEvent(context.Background(),
&userAgg.Aggregate),
),
},
uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("username", "org1", false)),
),
),
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"),
userPasswordAlg: crypto.CreateMockHashAlg(gomock.NewController(t)),
codeAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args: args{
ctx: context.Background(),
orgID: "org1",
human: &AddHuman{
Username: "username",
Password: "password",
FirstName: "firstname",
LastName: "lastname",
Email: Email{
Address: "email@test.ch",
Verified: true,
},
PreferredLanguage: language.English,
PasswordChangeRequired: true,
},
secretGenerator: GetMockSecretGenerator(t),
},
res: res{
want: &domain.HumanDetails{
ID: "user1",
ObjectDetails: domain.ObjectDetails{
ResourceOwner: "org1",
},
},
},
},
{
name: "add human claimed domain, userLoginMustBeDomain false, error",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
org.NewDomainPolicyAddedEvent(context.Background(),
&userAgg.Aggregate,
false,
true,
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewDomainVerifiedEvent(context.Background(),
&org.NewAggregate("org2").Aggregate,
"test.ch",
),
),
),
),
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"),
userPasswordAlg: crypto.CreateMockHashAlg(gomock.NewController(t)),
codeAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args: args{
ctx: context.Background(),
orgID: "org1",
human: &AddHuman{
Username: "username@test.ch",
Password: "password",
FirstName: "firstname",
LastName: "lastname",
Email: Email{
Address: "email@test.ch",
Verified: true,
},
PreferredLanguage: language.English,
PasswordChangeRequired: true,
},
secretGenerator: GetMockSecretGenerator(t),
},
res: res{
err: errors.IsErrorInvalidArgument,
},
},
{
name: "add human domain, userLoginMustBeDomain false, ok",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
org.NewDomainPolicyAddedEvent(context.Background(),
&userAgg.Aggregate,
false,
true,
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewDomainVerifiedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
"test.ch",
),
),
),
expectFilter(
eventFromEventPusher(
org.NewPasswordComplexityPolicyAddedEvent(context.Background(),
&userAgg.Aggregate,
1,
false,
false,
false,
false,
),
),
),
expectPush(
[]*repository.Event{
eventFromEventPusher(
func() eventstore.Command {
event := user.NewHumanAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
"username@test.ch",
"firstname",
"lastname",
"",
"firstname lastname",
language.English,
domain.GenderUnspecified,
"email@test.ch",
true,
)
event.AddPasswordData(&crypto.CryptoValue{
CryptoType: crypto.TypeHash,
Algorithm: "hash",
KeyID: "",
Crypted: []byte("password"),
}, true)
return event
}(),
),
eventFromEventPusher(
user.NewHumanEmailVerifiedEvent(context.Background(),
&userAgg.Aggregate),
),
},
uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("username@test.ch", "org1", false)),
),
),
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"),
userPasswordAlg: crypto.CreateMockHashAlg(gomock.NewController(t)),
codeAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)),
},
args: args{
ctx: context.Background(),
orgID: "org1",
human: &AddHuman{
Username: "username@test.ch",
Password: "password",
FirstName: "firstname",
LastName: "lastname",
Email: Email{
Address: "email@test.ch",
Verified: true,
},
PreferredLanguage: language.English,
PasswordChangeRequired: true,
},
secretGenerator: GetMockSecretGenerator(t),
},
res: res{
want: &domain.HumanDetails{
ID: "user1",
ObjectDetails: domain.ObjectDetails{
ResourceOwner: "org1",
},
},
},
},
{
name: "add human (with phone), ok",
fields: fields{
@ -2140,6 +2358,118 @@ func TestCommandSide_RegisterHuman(t *testing.T) {
},
},
},
{
name: "username without @, ok",
fields: fields{
eventstore: eventstoreExpect(
t,
expectFilter(
eventFromEventPusher(
org.NewDomainPolicyAddedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
false,
true,
true,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewPasswordComplexityPolicyAddedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
1,
false,
false,
false,
false,
),
),
),
expectFilter(
eventFromEventPusher(
org.NewLoginPolicyAddedEvent(context.Background(),
&org.NewAggregate("org1").Aggregate,
false,
true,
false,
false,
false,
false,
false,
false,
false,
domain.PasswordlessTypeNotAllowed,
"",
time.Hour*1,
time.Hour*2,
time.Hour*3,
time.Hour*4,
time.Hour*5,
),
),
),
expectPush(
[]*repository.Event{
eventFromEventPusher(
newRegisterHumanEvent("username", "password", false, ""),
),
eventFromEventPusher(
user.NewHumanInitialCodeAddedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
&crypto.CryptoValue{
CryptoType: crypto.TypeEncryption,
Algorithm: "enc",
KeyID: "id",
Crypted: []byte("a"),
},
time.Hour*1,
),
),
},
uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("username", "org1", false)),
),
),
idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "user1"),
userPasswordAlg: crypto.CreateMockHashAlg(gomock.NewController(t)),
},
args: args{
ctx: context.Background(),
orgID: "org1",
human: &domain.Human{
Password: &domain.Password{
SecretString: "password",
},
Profile: &domain.Profile{
FirstName: "firstname",
LastName: "lastname",
},
Email: &domain.Email{
EmailAddress: "email@test.ch",
},
Username: "username",
},
secretGenerator: GetMockSecretGenerator(t),
},
res: res{
want: &domain.Human{
ObjectRoot: models.ObjectRoot{
AggregateID: "user1",
ResourceOwner: "org1",
},
Username: "username",
Profile: &domain.Profile{
FirstName: "firstname",
LastName: "lastname",
DisplayName: "firstname lastname",
PreferredLanguage: language.Und,
},
Email: &domain.Email{
EmailAddress: "email@test.ch",
},
State: domain.UserStateInitial,
},
},
},
{
name: "add human (with password and initial code), ok",
fields: fields{

3
internal/notification/types/domain_claimed.go
View File

@ -10,8 +10,9 @@ import (
func (notify Notify) SendDomainClaimed(user *query.NotifyUser, origin, username string) error {
url := login.LoginLink(origin, user.ResourceOwner)
index := strings.LastIndex(user.LastEmail, "@")
args := make(map[string]interface{})
args["TempUsername"] = username
args["Domain"] = strings.Split(user.LastEmail, "@")[1]
args["Domain"] = user.LastEmail[index+1:]
return notify(url, args, domain.DomainClaimedMessageType, true)
}