From 7dfa1925cc00d1fc1b36f0c8674d8a6e1a237e66 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Fri, 23 Sep 2022 14:08:10 +0200 Subject: [PATCH] feat: restrict login to specific org by id (scope) (#4294) * feat: add new org scope * change default of UserLoginMustBeDomain to false * return resource owner claims * fix: use email style for first user * fix: ensure email style for default users (backwards compatibility) * change to external domain (as it was before UserLoginMustBeDomain change) * update e2e tests to use email style usernames * document new scope * lint e2e Co-authored-by: Fabi <38692350+hifabienne@users.noreply.github.com> --- cmd/defaults.yaml | 5 +++- cmd/setup/03.go | 11 ++++++- cmd/setup/steps.yaml | 5 +++- cmd/start/start.go | 2 +- docs/docs/apis/openidoauth/scopes.md | 1 + e2e/cypress/e2e/humans/humans.cy.ts | 4 +-- e2e/cypress/e2e/machines/machines.cy.ts | 4 +-- internal/api/grpc/system/instance.go | 2 +- .../api/grpc/system/instance_converter.go | 10 ++++++- internal/api/grpc/system/server.go | 10 +++++-- internal/api/oidc/client.go | 30 ++++++++++++++++++- internal/api/oidc/client_converter.go | 3 ++ internal/api/ui/login/renderer.go | 2 +- .../eventsourcing/eventstore/auth_request.go | 27 +++++++++++++++-- internal/domain/auth_request.go | 13 ++++++++ internal/domain/org_domain.go | 2 +- internal/domain/request.go | 2 ++ 17 files changed, 114 insertions(+), 19 deletions(-) diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index 05eeb58519..f93142aacc 100644 --- a/cmd/defaults.yaml +++ b/cmd/defaults.yaml @@ -310,6 +310,9 @@ DefaultInstance: Org: Name: Human: + # in case that UserLoginMustBeDomain is false (default) and if you don't overwrite the username with an email, + # it will be suffixed by the org domain (org-name + domain from config). + # for example: zitadel-admin in org `My Org` on domain.tld -> zitadel-admin@my-org.domain.tld UserName: zitadel-admin FirstName: ZITADEL LastName: Admin @@ -383,7 +386,7 @@ DefaultInstance: ExpireWarnDays: 0 MaxAgeDays: 0 DomainPolicy: - UserLoginMustBeDomain: true + UserLoginMustBeDomain: false ValidateOrgDomains: true SMTPSenderAddressMatchesInstanceDomain: false LoginPolicy: diff --git a/cmd/setup/03.go b/cmd/setup/03.go index 65aebf0481..3da04cc0fa 100644 --- a/cmd/setup/03.go +++ b/cmd/setup/03.go @@ -13,6 +13,7 @@ import ( "github.com/zitadel/zitadel/internal/config/systemdefaults" "github.com/zitadel/zitadel/internal/crypto" crypto_db "github.com/zitadel/zitadel/internal/crypto/database" + "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" ) @@ -83,9 +84,17 @@ func (mig *FirstInstance) Execute(ctx context.Context) error { mig.instanceSetup.CustomDomain = mig.externalDomain mig.instanceSetup.DefaultLanguage = mig.DefaultLanguage mig.instanceSetup.Org = mig.Org + // check if username is email style or else append @. + //this way we have the same value as before changing `UserLoginMustBeDomain` to false + if !mig.instanceSetup.DomainPolicy.UserLoginMustBeDomain && !strings.Contains(mig.instanceSetup.Org.Human.Username, "@") { + mig.instanceSetup.Org.Human.Username = mig.instanceSetup.Org.Human.Username + "@" + domain.NewIAMDomainName(mig.instanceSetup.Org.Name, mig.instanceSetup.CustomDomain) + } mig.instanceSetup.Org.Human.Email.Address = strings.TrimSpace(mig.instanceSetup.Org.Human.Email.Address) if mig.instanceSetup.Org.Human.Email.Address == "" { - mig.instanceSetup.Org.Human.Email.Address = "admin@" + mig.instanceSetup.CustomDomain + mig.instanceSetup.Org.Human.Email.Address = mig.instanceSetup.Org.Human.Username + if !strings.Contains(mig.instanceSetup.Org.Human.Email.Address, "@") { + mig.instanceSetup.Org.Human.Email.Address = mig.instanceSetup.Org.Human.Username + "@" + domain.NewIAMDomainName(mig.instanceSetup.Org.Name, mig.instanceSetup.CustomDomain) + } } _, _, err = cmd.SetUpInstance(ctx, &mig.instanceSetup) diff --git a/cmd/setup/steps.yaml b/cmd/setup/steps.yaml index bf1f086eb2..75335ba71a 100644 --- a/cmd/setup/steps.yaml +++ b/cmd/setup/steps.yaml @@ -4,13 +4,16 @@ FirstInstance: Org: Name: ZITADEL Human: + # in case that UserLoginMustBeDomain is false (default) and you don't overwrite the username with an email, + # it will be suffixed by the org domain (org-name + domain from config). + # for example: zitadel-admin in org ZITADEL on domain.tld -> zitadel-admin@zitadel.domain.tld UserName: zitadel-admin FirstName: ZITADEL LastName: Admin NickName: DisplayName: Email: - Address: #autogenerated if empty. uses domain from config and prefixes admin@. for example: admin@domain.tdl + Address: #uses the username if empty Verified: true PreferredLanguage: en Gender: diff --git a/cmd/start/start.go b/cmd/start/start.go index 9efeb90777..3ce7d759ac 100644 --- a/cmd/start/start.go +++ b/cmd/start/start.go @@ -181,7 +181,7 @@ func startAPIs(ctx context.Context, router *mux.Router, commands *command.Comman if err != nil { return fmt.Errorf("error starting admin repo: %w", err) } - if err := apis.RegisterServer(ctx, system.CreateServer(commands, queries, adminRepo, config.Database.Database(), config.DefaultInstance)); err != nil { + if err := apis.RegisterServer(ctx, system.CreateServer(commands, queries, adminRepo, config.Database.Database(), config.DefaultInstance, config.ExternalDomain)); err != nil { return err } if err := apis.RegisterServer(ctx, admin.CreateServer(config.Database.Database(), commands, queries, config.SystemDefaults, adminRepo, config.ExternalSecure, keys.User)); err != nil { diff --git a/docs/docs/apis/openidoauth/scopes.md b/docs/docs/apis/openidoauth/scopes.md index 595e8321fc..2fc8d32601 100644 --- a/docs/docs/apis/openidoauth/scopes.md +++ b/docs/docs/apis/openidoauth/scopes.md @@ -25,6 +25,7 @@ In addition to the standard compliant scopes we utilize the following scopes. | Scopes | Example | Description | |:--------------------------------------------------|:-------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `urn:zitadel:iam:org:project:role:{rolekey}` | `urn:zitadel:iam:org:project:role:user` | By using this scope a client can request the claim urn:zitadel:iam:roles to be asserted when possible. As an alternative approach you can enable all roles to be asserted from the [project](../../guides/manage/console/projects) a client belongs to. | +| `urn:zitadel:iam:org:id:{id}` | `urn:zitadel:iam:org:id:178204173316174381` | When requesting this scope **ZITADEL** will enforce that the user is a member of the selected organization. If the organization does not exist a failure is displayed. It will assert the `urn:zitadel:iam:user:resourceowner` claims. | | `urn:zitadel:iam:org:domain:primary:{domainname}` | `urn:zitadel:iam:org:domain:primary:acme.ch` | When requesting this scope **ZITADEL** will enforce that the user is a member of the selected organization. If the organization does not exist a failure is displayed | | `urn:zitadel:iam:role:{rolename}` | | | | `urn:zitadel:iam:org:project:id:{projectid}:aud` | `urn:zitadel:iam:org:project:id:69234237810729019:aud` | By adding this scope, the requested projectid will be added to the audience of the access token | diff --git a/e2e/cypress/e2e/humans/humans.cy.ts b/e2e/cypress/e2e/humans/humans.cy.ts index 112ae3f990..d48277132f 100644 --- a/e2e/cypress/e2e/humans/humans.cy.ts +++ b/e2e/cypress/e2e/humans/humans.cy.ts @@ -21,7 +21,7 @@ describe('humans', () => { cy.url().should('contain', 'users/create'); cy.get('[formcontrolname="email"]').type(loginname('e2ehuman', Cypress.env('ORGANIZATION'))); //force needed due to the prefilled username prefix - cy.get('[formcontrolname="userName"]').type(testHumanUserNameAdd); + cy.get('[formcontrolname="userName"]').type(loginname(testHumanUserNameAdd, Cypress.env('ORGANIZATION'))); cy.get('[formcontrolname="firstName"]').type('e2ehumanfirstname'); cy.get('[formcontrolname="lastName"]').type('e2ehumanlastname'); cy.get('[formcontrolname="phone"]').type('+41 123456789'); @@ -35,7 +35,7 @@ describe('humans', () => { describe('remove', () => { before('ensure it exists', () => { apiAuth().then((api) => { - ensureHumanUserExists(api, testHumanUserNameRemove).then(() => { + ensureHumanUserExists(api, loginname(testHumanUserNameRemove, Cypress.env('ORGANIZATION'))).then(() => { cy.visit(humansPath); }); }); diff --git a/e2e/cypress/e2e/machines/machines.cy.ts b/e2e/cypress/e2e/machines/machines.cy.ts index b9181e9c39..58ad186982 100644 --- a/e2e/cypress/e2e/machines/machines.cy.ts +++ b/e2e/cypress/e2e/machines/machines.cy.ts @@ -45,9 +45,7 @@ describe('machines', () => { // .trigger('mouseover') .find('[data-e2e="enabled-delete-button"]') .click({ force: true }); - cy.get('[data-e2e="confirm-dialog-input"]') - .focus() - .type(loginname(testMachineUserNameRemove, Cypress.env('ORGANIZATION'))); + cy.get('[data-e2e="confirm-dialog-input"]').focus().type(testMachineUserNameRemove); cy.get('[data-e2e="confirm-dialog-button"]').click(); cy.get('.data-e2e-success'); cy.wait(200); diff --git a/internal/api/grpc/system/instance.go b/internal/api/grpc/system/instance.go index 135d41c6c0..daeaf54cab 100644 --- a/internal/api/grpc/system/instance.go +++ b/internal/api/grpc/system/instance.go @@ -41,7 +41,7 @@ func (s *Server) GetInstance(ctx context.Context, req *system_pb.GetInstanceRequ } func (s *Server) AddInstance(ctx context.Context, req *system_pb.AddInstanceRequest) (*system_pb.AddInstanceResponse, error) { - id, details, err := s.command.SetUpInstance(ctx, AddInstancePbToSetupInstance(req, s.DefaultInstance)) + id, details, err := s.command.SetUpInstance(ctx, AddInstancePbToSetupInstance(req, s.defaultInstance, s.externalDomain)) if err != nil { return nil, err } diff --git a/internal/api/grpc/system/instance_converter.go b/internal/api/grpc/system/instance_converter.go index fb812b62fe..7c452c789b 100644 --- a/internal/api/grpc/system/instance_converter.go +++ b/internal/api/grpc/system/instance_converter.go @@ -1,17 +1,20 @@ package system import ( + "strings" + "golang.org/x/text/language" instance_grpc "github.com/zitadel/zitadel/internal/api/grpc/instance" "github.com/zitadel/zitadel/internal/api/grpc/object" "github.com/zitadel/zitadel/internal/command" + "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/query" instance_pb "github.com/zitadel/zitadel/pkg/grpc/instance" system_pb "github.com/zitadel/zitadel/pkg/grpc/system" ) -func AddInstancePbToSetupInstance(req *system_pb.AddInstanceRequest, defaultInstance command.InstanceSetup) *command.InstanceSetup { +func AddInstancePbToSetupInstance(req *system_pb.AddInstanceRequest, defaultInstance command.InstanceSetup, externalDomain string) *command.InstanceSetup { if req.InstanceName != "" { defaultInstance.InstanceName = req.InstanceName defaultInstance.Org.Name = req.InstanceName @@ -40,6 +43,11 @@ func AddInstancePbToSetupInstance(req *system_pb.AddInstanceRequest, defaultInst } } } + // check if default username is email style or else append @. + // this way we have the same value as before changing `UserLoginMustBeDomain` to false + if !defaultInstance.DomainPolicy.UserLoginMustBeDomain && !strings.Contains(defaultInstance.Org.Human.Username, "@") { + defaultInstance.Org.Human.Username = defaultInstance.Org.Human.Username + "@" + domain.NewIAMDomainName(defaultInstance.Org.Name, externalDomain) + } if req.OwnerUserName != "" { defaultInstance.Org.Human.Username = req.OwnerUserName } diff --git a/internal/api/grpc/system/server.go b/internal/api/grpc/system/server.go index fca6b0a77e..2d9745181a 100644 --- a/internal/api/grpc/system/server.go +++ b/internal/api/grpc/system/server.go @@ -25,25 +25,29 @@ type Server struct { command *command.Commands query *query.Queries administrator repository.AdministratorRepository - DefaultInstance command.InstanceSetup + defaultInstance command.InstanceSetup + externalDomain string } type Config struct { Repository eventsourcing.Config } -func CreateServer(command *command.Commands, +func CreateServer( + command *command.Commands, query *query.Queries, repo repository.Repository, database string, defaultInstance command.InstanceSetup, + externalDomain string, ) *Server { return &Server{ command: command, query: query, administrator: repo, database: database, - DefaultInstance: defaultInstance, + defaultInstance: defaultInstance, + externalDomain: externalDomain, } } diff --git a/internal/api/oidc/client.go b/internal/api/oidc/client.go index 0ad114979c..b73f549eb8 100644 --- a/internal/api/oidc/client.go +++ b/internal/api/oidc/client.go @@ -95,6 +95,13 @@ func (o *OPStorage) ValidateJWTProfileScopes(ctx context.Context, subject string scopes = scopes[:len(scopes)-1] } } + if strings.HasPrefix(scope, domain.OrgIDScope) { + if strings.TrimPrefix(scope, domain.OrgIDScope) != user.ResourceOwner { + scopes[i] = scopes[len(scopes)-1] + scopes[len(scopes)-1] = "" + scopes = scopes[:len(scopes)-1] + } + } } return scopes, nil } @@ -251,6 +258,16 @@ func (o *OPStorage) setUserinfo(ctx context.Context, userInfo oidc.UserInfoSette if strings.HasPrefix(scope, domain.OrgDomainPrimaryScope) { userInfo.AppendClaims(domain.OrgDomainPrimaryClaim, strings.TrimPrefix(scope, domain.OrgDomainPrimaryScope)) } + if strings.HasPrefix(scope, domain.OrgIDScope) { + userInfo.AppendClaims(domain.OrgIDClaim, strings.TrimPrefix(scope, domain.OrgIDScope)) + resourceOwnerClaims, err := o.assertUserResourceOwner(ctx, userID) + if err != nil { + return err + } + for claim, value := range resourceOwnerClaims { + userInfo.AppendClaims(claim, value) + } + } } } if len(roles) == 0 || applicationID == "" { @@ -289,9 +306,20 @@ func (o *OPStorage) GetPrivateClaimsFromScopes(ctx context.Context, userID, clie } if strings.HasPrefix(scope, ScopeProjectRolePrefix) { roles = append(roles, strings.TrimPrefix(scope, ScopeProjectRolePrefix)) - } else if strings.HasPrefix(scope, domain.OrgDomainPrimaryScope) { + } + if strings.HasPrefix(scope, domain.OrgDomainPrimaryScope) { claims = appendClaim(claims, domain.OrgDomainPrimaryClaim, strings.TrimPrefix(scope, domain.OrgDomainPrimaryScope)) } + if strings.HasPrefix(scope, domain.OrgIDScope) { + claims = appendClaim(claims, domain.OrgIDClaim, strings.TrimPrefix(scope, domain.OrgIDScope)) + resourceOwnerClaims, err := o.assertUserResourceOwner(ctx, userID) + if err != nil { + return nil, err + } + for claim, value := range resourceOwnerClaims { + claims = appendClaim(claims, claim, value) + } + } } if len(roles) == 0 || clientID == "" { return claims, nil diff --git a/internal/api/oidc/client_converter.go b/internal/api/oidc/client_converter.go index 90e45a8d4f..b5c843e1f5 100644 --- a/internal/api/oidc/client_converter.go +++ b/internal/api/oidc/client_converter.go @@ -103,6 +103,9 @@ func (c *Client) IsScopeAllowed(scope string) bool { if strings.HasPrefix(scope, domain.OrgDomainPrimaryScope) { return true } + if strings.HasPrefix(scope, domain.OrgIDScope) { + return true + } if strings.HasPrefix(scope, domain.ProjectIDScope) { return true } diff --git a/internal/api/ui/login/renderer.go b/internal/api/ui/login/renderer.go index 1255ff740d..d3c4ef3d4e 100644 --- a/internal/api/ui/login/renderer.go +++ b/internal/api/ui/login/renderer.go @@ -507,7 +507,7 @@ func (l *Login) isDisplayLoginNameSuffix(authReq *domain.AuthRequest) bool { if authReq == nil { return false } - if authReq.RequestedOrgID == "" { + if authReq.RequestedOrgID == "" || !authReq.RequestedOrgDomain { return false } return authReq.LabelPolicy != nil && !authReq.LabelPolicy.HideLoginNameSuffix diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index 2d4eb9366f..2ba00d9701 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -632,8 +632,14 @@ func (repo *AuthRequestRepo) checkLoginName(ctx context.Context, request *domain loginName = strings.TrimSpace(loginName) preferredLoginName := loginName if request.RequestedOrgID != "" { - if request.RequestedOrgID != "" { - preferredLoginName += "@" + request.RequestedPrimaryDomain + if request.RequestedOrgDomain { + domainPolicy, err := repo.getDomainPolicy(ctx, request.RequestedOrgID) + if err != nil { + return err + } + if domainPolicy.UserLoginMustBeDomain { + preferredLoginName += "@" + request.RequestedPrimaryDomain + } } user, err = repo.View.UserByLoginNameAndResourceOwner(preferredLoginName, request.RequestedOrgID, request.InstanceID) } else { @@ -1058,7 +1064,23 @@ func (repo *AuthRequestRepo) hasSucceededPage(ctx context.Context, request *doma return app.OIDCConfig.AppType == domain.OIDCApplicationTypeNative, nil } +func (repo *AuthRequestRepo) getDomainPolicy(ctx context.Context, orgID string) (*query.DomainPolicy, error) { + return repo.Query.DomainPolicyByOrg(ctx, false, orgID) +} + func setOrgID(ctx context.Context, orgViewProvider orgViewProvider, request *domain.AuthRequest) error { + orgID := request.GetScopeOrgID() + if orgID != "" { + org, err := orgViewProvider.OrgByID(ctx, false, orgID) + if err != nil { + return err + } + request.RequestedOrgID = org.ID + request.RequestedOrgName = org.Name + request.RequestedPrimaryDomain = org.Domain + return nil + } + primaryDomain := request.GetScopeOrgPrimaryDomain() if primaryDomain == "" { return nil @@ -1071,6 +1093,7 @@ func setOrgID(ctx context.Context, orgViewProvider orgViewProvider, request *dom request.RequestedOrgID = org.ID request.RequestedOrgName = org.Name request.RequestedPrimaryDomain = primaryDomain + request.RequestedOrgDomain = true return nil } diff --git a/internal/domain/auth_request.go b/internal/domain/auth_request.go index d3e5778430..1862584ac6 100644 --- a/internal/domain/auth_request.go +++ b/internal/domain/auth_request.go @@ -37,6 +37,7 @@ type AuthRequest struct { RequestedOrgID string RequestedOrgName string RequestedPrimaryDomain string + RequestedOrgDomain bool ApplicationResourceOwner string PrivateLabelingSetting PrivateLabelingSetting SelectedIDPConfigID string @@ -164,3 +165,15 @@ func (a *AuthRequest) GetScopeOrgPrimaryDomain() string { } return "" } + +func (a *AuthRequest) GetScopeOrgID() string { + switch request := a.Request.(type) { + case *AuthRequestOIDC: + for _, scope := range request.Scopes { + if strings.HasPrefix(scope, OrgIDScope) { + return strings.TrimPrefix(scope, OrgIDScope) + } + } + } + return "" +} diff --git a/internal/domain/org_domain.go b/internal/domain/org_domain.go index 6b81a6a7b1..6009639edc 100644 --- a/internal/domain/org_domain.go +++ b/internal/domain/org_domain.go @@ -32,7 +32,7 @@ func (domain *OrgDomain) GenerateVerificationCode(codeGenerator crypto.Generator } func NewIAMDomainName(orgName, iamDomain string) string { - return strings.ToLower(strings.ReplaceAll(orgName, " ", "-") + "." + iamDomain) + return strings.ToLower(strings.ReplaceAll(strings.TrimSpace(orgName), " ", "-") + "." + iamDomain) } type OrgDomainValidationType int32 diff --git a/internal/domain/request.go b/internal/domain/request.go index 69bace2573..4dafcf0ca4 100644 --- a/internal/domain/request.go +++ b/internal/domain/request.go @@ -2,7 +2,9 @@ package domain const ( OrgDomainPrimaryScope = "urn:zitadel:iam:org:domain:primary:" + OrgIDScope = "urn:zitadel:iam:org:id:" OrgDomainPrimaryClaim = "urn:zitadel:iam:org:domain:primary" + OrgIDClaim = "urn:zitadel:iam:org:id" ProjectIDScope = "urn:zitadel:iam:org:project:id:" ProjectIDScopeZITADEL = "zitadel" AudSuffix = ":aud"