From ab6c4331df3b233a53b75185e91eb19a69a70055 Mon Sep 17 00:00:00 2001 From: Elio Bischof Date: Fri, 20 Dec 2024 11:31:03 +0100 Subject: [PATCH 01/21] fix(login): avoid disallowed languages with custom texts (#9094) # Which Problems Are Solved If a browsers default language is not allowed by instance restrictions, the login still renders it if it finds any custom texts for this language. In that case, the login tries to render all texts on all screens in this language using custom texts, even for texts that are not customized. ![image](https://github.com/user-attachments/assets/1038ecac-90c9-4352-b75d-e7466a639711) ![image](https://github.com/user-attachments/assets/e4cbd0fb-a60e-41c5-a404-23e6d144de6c) ![image](https://github.com/user-attachments/assets/98d8b0b9-e082-48ae-9540-66792341fe1c) # How the Problems Are Solved If a custom messages language is not allowed, it is not added to the i18n library's translations bundle. The library correctly falls back to the instances default language. ![image](https://github.com/user-attachments/assets/fadac92e-bdea-4f8c-b6c2-2aa6476b89b3) This library method only receives messages for allowed languages ![image](https://github.com/user-attachments/assets/33081929-d3a5-4b0f-b838-7b69f88c13bc) # Additional Context Reported via support request --- internal/i18n/translator.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/internal/i18n/translator.go b/internal/i18n/translator.go index 74dd65663a..d9d385576d 100644 --- a/internal/i18n/translator.go +++ b/internal/i18n/translator.go @@ -64,10 +64,22 @@ func (t *Translator) SupportedLanguages() []language.Tag { return t.allowedLanguages } +// AddMessages adds messages to the translator for the given language tag. +// If the tag is not in the allowed languages, the messages are not added. func (t *Translator) AddMessages(tag language.Tag, messages ...Message) error { if len(messages) == 0 { return nil } + var isAllowed bool + for _, allowed := range t.allowedLanguages { + if allowed == tag { + isAllowed = true + break + } + } + if !isAllowed { + return nil + } i18nMessages := make([]*i18n.Message, len(messages)) for i, message := range messages { i18nMessages[i] = &i18n.Message{ From bcf416d4cf6448faa7f4d76dcdd1b81e5e3defcb Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Fri, 20 Dec 2024 17:03:06 +0100 Subject: [PATCH 02/21] fix(saml): parse xsd:duration format correctly (#9098) # Which Problems Are Solved SAML IdPs exposing an `EntitiesDescriptor` using an `xsd:duration` time format for the `cacheDuration` property (e.g. `PT5H`) failed parsing. # How the Problems Are Solved Handle the unmarshalling for `EntitiesDescriptor` specifically. [crewjam/saml](https://github.com/crewjam/saml/blob/bbccb7933d5f60512ebc6caec7120c604581983d/metadata.go#L88-L103) already did this for `EntitiyDescriptor` the same way. # Additional Changes None # Additional Context - reported by a customer - needs to be backported to current cloud version (2.66.x) --- internal/idp/providers/saml/saml.go | 25 ++++++++++++++++++++++- internal/idp/providers/saml/saml_test.go | 26 ++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/internal/idp/providers/saml/saml.go b/internal/idp/providers/saml/saml.go index e0391bc099..11b1d36da2 100644 --- a/internal/idp/providers/saml/saml.go +++ b/internal/idp/providers/saml/saml.go @@ -126,7 +126,7 @@ func ParseMetadata(metadata []byte) (*saml.EntityDescriptor, error) { if _, err := reader.Seek(0, io.SeekStart); err != nil { return nil, err } - entities := &saml.EntitiesDescriptor{} + entities := &EntitiesDescriptor{} if err := decoder.Decode(entities); err != nil { return nil, err } @@ -253,3 +253,26 @@ func nameIDFormatFromDomain(format domain.SAMLNameIDFormat) saml.NameIDFormat { return saml.UnspecifiedNameIDFormat } } + +// EntitiesDescriptor is a workaround until we eventually fork the crewjam/saml library, since maintenance on that repo seems to have stopped. +// This is to be able to handle xsd:duration format using the UnmarshalXML method. +// crewjam/saml only implements the xsd:dateTime format for EntityDescriptor, but not EntitiesDescriptor. +type EntitiesDescriptor saml.EntitiesDescriptor + +// UnmarshalXML implements xml.Unmarshaler +func (m *EntitiesDescriptor) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { + type Alias EntitiesDescriptor + aux := &struct { + ValidUntil *saml.RelaxedTime `xml:"validUntil,attr,omitempty"` + CacheDuration *saml.Duration `xml:"cacheDuration,attr,omitempty"` + *Alias + }{ + Alias: (*Alias)(m), + } + if err := d.DecodeElement(aux, &start); err != nil { + return err + } + m.ValidUntil = (*time.Time)(aux.ValidUntil) + m.CacheDuration = (*time.Duration)(aux.CacheDuration) + return nil +} diff --git a/internal/idp/providers/saml/saml_test.go b/internal/idp/providers/saml/saml_test.go index 801ddd36fc..69ff231ccc 100644 --- a/internal/idp/providers/saml/saml_test.go +++ b/internal/idp/providers/saml/saml_test.go @@ -3,6 +3,7 @@ package saml import ( "encoding/xml" "testing" + "time" "github.com/crewjam/saml" "github.com/crewjam/saml/samlsp" @@ -271,6 +272,31 @@ func TestParseMetadata(t *testing.T) { }, nil, }, + { + "valid entities using xsd duration descriptor", + args{ + metadata: []byte(``), + }, + &saml.EntityDescriptor{ + EntityID: "http://localhost:8000/metadata", + CacheDuration: 5 * time.Hour, + IDPSSODescriptors: []saml.IDPSSODescriptor{ + { + XMLName: xml.Name{ + Space: "urn:oasis:names:tc:SAML:2.0:metadata", + Local: "IDPSSODescriptor", + }, + SingleSignOnServices: []saml.Endpoint{ + { + Binding: saml.HTTPRedirectBinding, + Location: "http://localhost:8000/sso", + }, + }, + }, + }, + }, + nil, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 1f8623d3dcbf3abdb6fe3e276af24babc3ff4beb Mon Sep 17 00:00:00 2001 From: Branislav Davidovic Date: Mon, 23 Dec 2024 08:01:35 +0100 Subject: [PATCH 03/21] docs(adopter): XPeditionist (#8984) N/A Co-authored-by: Fabi Co-authored-by: Silvan <27845747+adlerhurst@users.noreply.github.com> Co-authored-by: Livio Spring --- ADOPTERS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ADOPTERS.md b/ADOPTERS.md index cfd1aac134..da37b9ffb0 100644 --- a/ADOPTERS.md +++ b/ADOPTERS.md @@ -12,6 +12,7 @@ If you are using Zitadel, please consider adding yourself as a user with a quick | ----------------------- | -------------------------------------------------------------------- | ----------------------------------------------- | | Zitadel | [@fforootd](https://github.com/fforootd) (and many more) | Zitadel Cloud makes heavy use of of Zitadel ;-) | | Rawkode Academy | [@RawkodeAcademy](https://github.com/RawkodeAcademy) | Rawkode Academy Platform & Zulip use Zitadel for all user and M2M authentication | +| XPeditionist | [@XPeditionistTravel](https://github.com/XPeditionistTravel) | An innovative all-in-one travel solution use Zitadel as complete auth solution. | | devOS: Sanity Edition | [@devOS-Sanity-Edition](https://github.com/devOS-Sanity-Edition) | Uses SSO Auth for every piece of our internal and external infrastructure | | CNAP.tech | [@cnap-tech](https://github.com/cnap-tech) | Using Zitadel for authentication and authorization in cloud-native applications | | Minekube | [@minekube](https://github.com/minekube) | Leveraging Zitadel for secure user authentication in gaming infrastructure | From 8ec099ae280f1abc31af68821554f31462febe39 Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Fri, 27 Dec 2024 16:34:38 +0100 Subject: [PATCH 04/21] fix: restructure resend email code to send email code (#9099) # Which Problems Are Solved There is currently no endpoint to send an email code for verification of the email if you don't change the email itself. # How the Problems Are Solved Endpoint HasEmailCode to get the information that an email code is existing, used by the new login. Endpoint SendEmailCode, if no code is existing to replace ResendEmailCode as there is a check that a code has to be there, before it can be resend. # Additional Changes None # Additional Context Closes #9096 --------- Co-authored-by: Silvan <27845747+adlerhurst@users.noreply.github.com> --- internal/api/grpc/user/v2/email.go | 27 ++ .../user/v2/integration_test/email_test.go | 110 +++++ internal/command/user_v2_email.go | 41 +- internal/command/user_v2_email_test.go | 421 +++++++++++++++--- proto/zitadel/user/v2/user_service.proto | 50 ++- 5 files changed, 573 insertions(+), 76 deletions(-) diff --git a/internal/api/grpc/user/v2/email.go b/internal/api/grpc/user/v2/email.go index 6d0871b26e..4b247ef10f 100644 --- a/internal/api/grpc/user/v2/email.go +++ b/internal/api/grpc/user/v2/email.go @@ -67,6 +67,33 @@ func (s *Server) ResendEmailCode(ctx context.Context, req *user.ResendEmailCodeR }, nil } +func (s *Server) SendEmailCode(ctx context.Context, req *user.SendEmailCodeRequest) (resp *user.SendEmailCodeResponse, err error) { + var email *domain.Email + + switch v := req.GetVerification().(type) { + case *user.SendEmailCodeRequest_SendCode: + email, err = s.command.SendUserEmailCodeURLTemplate(ctx, req.GetUserId(), s.userCodeAlg, v.SendCode.GetUrlTemplate()) + case *user.SendEmailCodeRequest_ReturnCode: + email, err = s.command.SendUserEmailReturnCode(ctx, req.GetUserId(), s.userCodeAlg) + case nil: + email, err = s.command.SendUserEmailCode(ctx, req.GetUserId(), s.userCodeAlg) + default: + err = zerrors.ThrowUnimplementedf(nil, "USERv2-faj0l0nj5x", "verification oneOf %T in method SendEmailCode not implemented", v) + } + if err != nil { + return nil, err + } + + return &user.SendEmailCodeResponse{ + Details: &object.Details{ + Sequence: email.Sequence, + ChangeDate: timestamppb.New(email.ChangeDate), + ResourceOwner: email.ResourceOwner, + }, + VerificationCode: email.PlainCode, + }, nil +} + func (s *Server) VerifyEmail(ctx context.Context, req *user.VerifyEmailRequest) (*user.VerifyEmailResponse, error) { details, err := s.command.VerifyUserEmail(ctx, req.GetUserId(), diff --git a/internal/api/grpc/user/v2/integration_test/email_test.go b/internal/api/grpc/user/v2/integration_test/email_test.go index 37d575016b..de53bc68aa 100644 --- a/internal/api/grpc/user/v2/integration_test/email_test.go +++ b/internal/api/grpc/user/v2/integration_test/email_test.go @@ -249,6 +249,116 @@ func TestServer_ResendEmailCode(t *testing.T) { } } +func TestServer_SendEmailCode(t *testing.T) { + userID := Instance.CreateHumanUser(CTX).GetUserId() + verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email()).GetUserId() + + tests := []struct { + name string + req *user.SendEmailCodeRequest + want *user.SendEmailCodeResponse + wantErr bool + }{ + { + name: "user not existing", + req: &user.SendEmailCodeRequest{ + UserId: "xxx", + }, + wantErr: true, + }, + { + name: "user no code", + req: &user.SendEmailCodeRequest{ + UserId: verifiedUserID, + }, + want: &user.SendEmailCodeResponse{ + Details: &object.Details{ + Sequence: 1, + ChangeDate: timestamppb.Now(), + ResourceOwner: Instance.DefaultOrg.Id, + }, + }, + }, + { + name: "resend", + req: &user.SendEmailCodeRequest{ + UserId: userID, + }, + want: &user.SendEmailCodeResponse{ + Details: &object.Details{ + Sequence: 1, + ChangeDate: timestamppb.Now(), + ResourceOwner: Instance.DefaultOrg.Id, + }, + }, + }, + { + name: "custom url template", + req: &user.SendEmailCodeRequest{ + UserId: userID, + Verification: &user.SendEmailCodeRequest_SendCode{ + SendCode: &user.SendEmailVerificationCode{ + UrlTemplate: gu.Ptr("https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}"), + }, + }, + }, + want: &user.SendEmailCodeResponse{ + Details: &object.Details{ + Sequence: 1, + ChangeDate: timestamppb.Now(), + ResourceOwner: Instance.DefaultOrg.Id, + }, + }, + }, + { + name: "template error", + req: &user.SendEmailCodeRequest{ + UserId: userID, + Verification: &user.SendEmailCodeRequest_SendCode{ + SendCode: &user.SendEmailVerificationCode{ + UrlTemplate: gu.Ptr("{{"), + }, + }, + }, + wantErr: true, + }, + { + name: "return code", + req: &user.SendEmailCodeRequest{ + UserId: userID, + Verification: &user.SendEmailCodeRequest_ReturnCode{ + ReturnCode: &user.ReturnEmailVerificationCode{}, + }, + }, + want: &user.SendEmailCodeResponse{ + Details: &object.Details{ + Sequence: 1, + ChangeDate: timestamppb.Now(), + ResourceOwner: Instance.DefaultOrg.Id, + }, + VerificationCode: gu.Ptr("xxx"), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Client.SendEmailCode(CTX, tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + + integration.AssertDetails(t, tt.want, got) + if tt.want.GetVerificationCode() != "" { + assert.NotEmpty(t, got.GetVerificationCode()) + } else { + assert.Empty(t, got.GetVerificationCode()) + } + }) + } +} + func TestServer_VerifyEmail(t *testing.T) { userResp := Instance.CreateHumanUser(CTX) tests := []struct { diff --git a/internal/command/user_v2_email.go b/internal/command/user_v2_email.go index 1618e2cd48..4aa75d0935 100644 --- a/internal/command/user_v2_email.go +++ b/internal/command/user_v2_email.go @@ -57,6 +57,28 @@ func (c *Commands) ResendUserEmailReturnCode(ctx context.Context, userID string, return c.resendUserEmailCode(ctx, userID, alg, true, "") } +// SendUserEmailCode generates a new code +// and triggers a notification e-mail with the default confirmation URL format. +func (c *Commands) SendUserEmailCode(ctx context.Context, userID string, alg crypto.EncryptionAlgorithm) (*domain.Email, error) { + return c.sendUserEmailCode(ctx, userID, alg, false, "") +} + +// SendUserEmailCodeURLTemplate generates a new code +// and triggers a notification e-mail with the confirmation URL rendered from the passed urlTmpl. +// urlTmpl must be a valid [tmpl.Template]. +func (c *Commands) SendUserEmailCodeURLTemplate(ctx context.Context, userID string, alg crypto.EncryptionAlgorithm, urlTmpl string) (*domain.Email, error) { + if err := domain.RenderConfirmURLTemplate(io.Discard, urlTmpl, userID, "code", "orgID"); err != nil { + return nil, err + } + return c.sendUserEmailCode(ctx, userID, alg, false, urlTmpl) +} + +// SendUserEmailReturnCode generates a new code and does not send a notification email. +// The generated plain text code will be set in the returned Email object. +func (c *Commands) SendUserEmailReturnCode(ctx context.Context, userID string, alg crypto.EncryptionAlgorithm) (*domain.Email, error) { + return c.sendUserEmailCode(ctx, userID, alg, true, "") +} + // ChangeUserEmailVerified sets a user's email address and marks it is verified. // No code is generated and no confirmation e-mail is send. func (c *Commands) ChangeUserEmailVerified(ctx context.Context, userID, email string) (*domain.Email, error) { @@ -89,7 +111,16 @@ func (c *Commands) resendUserEmailCode(ctx context.Context, userID string, alg c return nil, err } gen := crypto.NewEncryptionGenerator(*config, alg) - return c.resendUserEmailCodeWithGenerator(ctx, userID, gen, returnCode, urlTmpl) + return c.sendUserEmailCodeWithGenerator(ctx, userID, gen, returnCode, urlTmpl, true) +} + +func (c *Commands) sendUserEmailCode(ctx context.Context, userID string, alg crypto.EncryptionAlgorithm, returnCode bool, urlTmpl string) (*domain.Email, error) { + config, err := cryptoGeneratorConfig(ctx, c.eventstore.Filter, domain.SecretGeneratorTypeVerifyEmailCode) //nolint:staticcheck + if err != nil { + return nil, err + } + gen := crypto.NewEncryptionGenerator(*config, alg) + return c.sendUserEmailCodeWithGenerator(ctx, userID, gen, returnCode, urlTmpl, false) } // changeUserEmailWithGenerator set a user's email address. @@ -104,8 +135,8 @@ func (c *Commands) changeUserEmailWithGenerator(ctx context.Context, userID, ema return cmd.Push(ctx) } -func (c *Commands) resendUserEmailCodeWithGenerator(ctx context.Context, userID string, gen crypto.Generator, returnCode bool, urlTmpl string) (*domain.Email, error) { - cmd, err := c.resendUserEmailCodeWithGeneratorEvents(ctx, userID, gen, returnCode, urlTmpl) +func (c *Commands) sendUserEmailCodeWithGenerator(ctx context.Context, userID string, gen crypto.Generator, returnCode bool, urlTmpl string, existingCheck bool) (*domain.Email, error) { + cmd, err := c.sendUserEmailCodeWithGeneratorEvents(ctx, userID, gen, returnCode, urlTmpl, existingCheck) if err != nil { return nil, err } @@ -129,7 +160,7 @@ func (c *Commands) changeUserEmailWithGeneratorEvents(ctx context.Context, userI return cmd, nil } -func (c *Commands) resendUserEmailCodeWithGeneratorEvents(ctx context.Context, userID string, gen crypto.Generator, returnCode bool, urlTmpl string) (*UserEmailEvents, error) { +func (c *Commands) sendUserEmailCodeWithGeneratorEvents(ctx context.Context, userID string, gen crypto.Generator, returnCode bool, urlTmpl string, existingCheck bool) (*UserEmailEvents, error) { cmd, err := c.NewUserEmailEvents(ctx, userID) if err != nil { return nil, err @@ -137,7 +168,7 @@ func (c *Commands) resendUserEmailCodeWithGeneratorEvents(ctx context.Context, u if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil { return nil, err } - if cmd.model.Code == nil { + if existingCheck && cmd.model.Code == nil { return nil, zerrors.ThrowPreconditionFailed(err, "EMAIL-5w5ilin4yt", "Errors.User.Code.Empty") } if err = cmd.AddGeneratedCode(ctx, gen, urlTmpl, returnCode); err != nil { diff --git a/internal/command/user_v2_email_test.go b/internal/command/user_v2_email_test.go index 79a53705f8..73ab2e1c4c 100644 --- a/internal/command/user_v2_email_test.go +++ b/internal/command/user_v2_email_test.go @@ -512,6 +512,85 @@ func TestCommands_ResendUserEmailCode(t *testing.T) { } } +func TestCommands_SendUserEmailCode(t *testing.T) { + type fields struct { + eventstore *eventstore.Eventstore + checkPermission domain.PermissionCheck + } + type args struct { + userID string + } + tests := []struct { + name string + fields fields + args args + wantErr error + }{ + { + name: "missing permission", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + instance.NewSecretGeneratorAddedEvent(context.Background(), + &instance.NewAggregate("inst1").Aggregate, + domain.SecretGeneratorTypeVerifyEmailCode, + 12, time.Minute, true, true, true, true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + eventFromEventPusher( + user.NewHumanEmailCodeAddedEventV2(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + time.Hour*1, + "", false, "", + ), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + }, + args: args{ + userID: "user1", + }, + wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Commands{ + eventstore: tt.fields.eventstore, + checkPermission: tt.fields.checkPermission, + } + _, err := c.SendUserEmailCode(context.Background(), tt.args.userID, crypto.CreateMockEncryptionAlg(gomock.NewController(t))) + require.ErrorIs(t, err, tt.wantErr) + // successful cases are tested in TestCommands_sendUserEmailCodeWithGeneratorEvents + }) + } +} + func TestCommands_ResendUserEmailCodeURLTemplate(t *testing.T) { type fields struct { eventstore *eventstore.Eventstore @@ -638,7 +717,99 @@ func TestCommands_ResendUserEmailCodeURLTemplate(t *testing.T) { } _, err := c.ResendUserEmailCodeURLTemplate(context.Background(), tt.args.userID, crypto.CreateMockEncryptionAlg(gomock.NewController(t)), tt.args.urlTmpl) require.ErrorIs(t, err, tt.wantErr) - // successful cases are tested in TestCommands_resendUserEmailCodeWithGenerator + // successful cases are tested in TestCommands_sendUserEmailCodeWithGeneratorEvents + }) + } +} + +func TestCommands_SendUserEmailCodeURLTemplate(t *testing.T) { + type fields struct { + eventstore *eventstore.Eventstore + checkPermission domain.PermissionCheck + } + type args struct { + userID string + urlTmpl string + } + tests := []struct { + name string + fields fields + args args + wantErr error + }{ + { + name: "invalid template", + fields: fields{ + eventstore: eventstoreExpect(t), + }, + args: args{ + userID: "user1", + urlTmpl: "{{", + }, + wantErr: zerrors.ThrowInvalidArgument(nil, "DOMAIN-oGh5e", "Errors.User.InvalidURLTemplate"), + }, + { + name: "permission missing", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + instance.NewSecretGeneratorAddedEvent(context.Background(), + &instance.NewAggregate("inst1").Aggregate, + domain.SecretGeneratorTypeVerifyEmailCode, + 12, time.Minute, true, true, true, true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + eventFromEventPusher( + user.NewHumanEmailCodeAddedEventV2(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + time.Hour*1, + "", false, "", + ), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + }, + args: args{ + userID: "user1", + urlTmpl: "https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}", + }, + wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Commands{ + eventstore: tt.fields.eventstore, + checkPermission: tt.fields.checkPermission, + } + _, err := c.SendUserEmailCodeURLTemplate(context.Background(), tt.args.userID, crypto.CreateMockEncryptionAlg(gomock.NewController(t)), tt.args.urlTmpl) + require.ErrorIs(t, err, tt.wantErr) + // successful cases are tested in TestCommands_sendUserEmailCodeWithGeneratorEvents }) } } @@ -760,6 +931,85 @@ func TestCommands_ResendUserEmailReturnCode(t *testing.T) { } } +func TestCommands_SendUserEmailReturnCode(t *testing.T) { + type fields struct { + eventstore *eventstore.Eventstore + checkPermission domain.PermissionCheck + } + type args struct { + userID string + } + tests := []struct { + name string + fields fields + args args + wantErr error + }{ + { + name: "missing permission", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + instance.NewSecretGeneratorAddedEvent(context.Background(), + &instance.NewAggregate("inst1").Aggregate, + domain.SecretGeneratorTypeVerifyEmailCode, + 12, time.Minute, true, true, true, true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + eventFromEventPusher( + user.NewHumanEmailCodeAddedEventV2(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + time.Hour*1, + "", false, "", + ), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + }, + args: args{ + userID: "user1", + }, + wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Commands{ + eventstore: tt.fields.eventstore, + checkPermission: tt.fields.checkPermission, + } + _, err := c.SendUserEmailReturnCode(context.Background(), tt.args.userID, crypto.CreateMockEncryptionAlg(gomock.NewController(t))) + require.ErrorIs(t, err, tt.wantErr) + // successful cases are tested in TestCommands_sendUserEmailCodeWithGeneratorEvents + }) + } +} + func TestCommands_ChangeUserEmailVerified(t *testing.T) { type fields struct { eventstore *eventstore.Eventstore @@ -1218,15 +1468,16 @@ func TestCommands_changeUserEmailWithGenerator(t *testing.T) { } } -func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { +func TestCommands_sendUserEmailCodeWithGeneratorEvents(t *testing.T) { type fields struct { eventstore *eventstore.Eventstore checkPermission domain.PermissionCheck } type args struct { - userID string - returnCode bool - urlTmpl string + userID string + returnCode bool + urlTmpl string + checkExisting bool } tests := []struct { name string @@ -1247,37 +1498,6 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { }, wantErr: zerrors.ThrowInvalidArgument(nil, "COMMAND-0Gzs3", "Errors.User.Email.IDMissing"), }, - { - name: "resend code, missing code", - fields: fields{ - eventstore: eventstoreExpect( - t, - expectFilter( - eventFromEventPusher( - user.NewHumanAddedEvent(context.Background(), - &user.NewAggregate("user1", "org1").Aggregate, - "username", - "firstname", - "lastname", - "nickname", - "displayname", - language.German, - domain.GenderUnspecified, - "email@test.ch", - true, - ), - ), - ), - ), - checkPermission: newMockPermissionCheckAllowed(), - }, - args: args{ - userID: "user1", - returnCode: false, - urlTmpl: "", - }, - wantErr: zerrors.ThrowPreconditionFailed(nil, "EMAIL-5w5ilin4yt", "Errors.User.Code.Empty"), - }, { name: "missing permission", fields: fields{ @@ -1322,6 +1542,58 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { }, wantErr: zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied"), }, + { + name: "send code", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectPush( + user.NewHumanEmailCodeAddedEventV2(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + time.Hour*1, + "", false, "", + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + userID: "user1", + returnCode: false, + urlTmpl: "", + checkExisting: false, + }, + want: &domain.Email{ + ObjectRoot: models.ObjectRoot{ + AggregateID: "user1", + ResourceOwner: "org1", + }, + EmailAddress: "email@test.ch", + IsEmailVerified: false, + }, + }, { name: "resend code", fields: fields{ @@ -1373,9 +1645,10 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { checkPermission: newMockPermissionCheckAllowed(), }, args: args{ - userID: "user1", - returnCode: false, - urlTmpl: "", + userID: "user1", + returnCode: false, + urlTmpl: "", + checkExisting: true, }, want: &domain.Email{ ObjectRoot: models.ObjectRoot{ @@ -1387,7 +1660,7 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { }, }, { - name: "resend code, return code", + name: "resend code, missing code", fields: fields{ eventstore: eventstoreExpect( t, @@ -1406,17 +1679,36 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { true, ), ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + userID: "user1", + returnCode: false, + urlTmpl: "", + checkExisting: true, + }, + wantErr: zerrors.ThrowPreconditionFailed(nil, "EMAIL-5w5ilin4yt", "Errors.User.Code.Empty"), + }, + { + name: "send code, return code", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( eventFromEventPusher( - user.NewHumanEmailCodeAddedEventV2(context.Background(), + user.NewHumanAddedEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, - &crypto.CryptoValue{ - CryptoType: crypto.TypeEncryption, - Algorithm: "enc", - KeyID: "id", - Crypted: []byte("a"), - }, - time.Hour*1, - "", false, "", + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, ), ), ), @@ -1437,9 +1729,10 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { checkPermission: newMockPermissionCheckAllowed(), }, args: args{ - userID: "user1", - returnCode: true, - urlTmpl: "", + userID: "user1", + returnCode: true, + urlTmpl: "", + checkExisting: false, }, want: &domain.Email{ ObjectRoot: models.ObjectRoot{ @@ -1452,7 +1745,7 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { }, }, { - name: "resend code, URL template", + name: "send code, URL template", fields: fields{ eventstore: eventstoreExpect( t, @@ -1471,19 +1764,6 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { true, ), ), - eventFromEventPusher( - user.NewHumanEmailCodeAddedEventV2(context.Background(), - &user.NewAggregate("user1", "org1").Aggregate, - &crypto.CryptoValue{ - CryptoType: crypto.TypeEncryption, - Algorithm: "enc", - KeyID: "id", - Crypted: []byte("a"), - }, - time.Hour*1, - "", false, "", - ), - ), ), expectPush( user.NewHumanEmailCodeAddedEventV2(context.Background(), @@ -1502,9 +1782,10 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { checkPermission: newMockPermissionCheckAllowed(), }, args: args{ - userID: "user1", - returnCode: false, - urlTmpl: "https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}", + userID: "user1", + returnCode: false, + urlTmpl: "https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}", + checkExisting: false, }, want: &domain.Email{ ObjectRoot: models.ObjectRoot{ @@ -1522,7 +1803,7 @@ func TestCommands_resendUserEmailCodeWithGeneratorEvents(t *testing.T) { eventstore: tt.fields.eventstore, checkPermission: tt.fields.checkPermission, } - got, err := c.resendUserEmailCodeWithGenerator(context.Background(), tt.args.userID, GetMockSecretGenerator(t), tt.args.returnCode, tt.args.urlTmpl) + got, err := c.sendUserEmailCodeWithGenerator(context.Background(), tt.args.userID, GetMockSecretGenerator(t), tt.args.returnCode, tt.args.urlTmpl, tt.args.checkExisting) require.ErrorIs(t, err, tt.wantErr) assert.Equal(t, tt.want, got) }) diff --git a/proto/zitadel/user/v2/user_service.proto b/proto/zitadel/user/v2/user_service.proto index 83b025bf0a..8ae7c1bc08 100644 --- a/proto/zitadel/user/v2/user_service.proto +++ b/proto/zitadel/user/v2/user_service.proto @@ -252,9 +252,34 @@ service UserService { }; } + // Send code to verify user email + // + // Send code to verify user email. + rpc SendEmailCode (SendEmailCodeRequest) returns (SendEmailCodeResponse) { + option (google.api.http) = { + post: "/v2/users/{user_id}/email/send" + body: "*" + }; + + option (zitadel.protoc_gen_zitadel.v2.options) = { + auth_option: { + permission: "authenticated" + } + }; + + option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = { + responses: { + key: "200" + value: { + description: "OK"; + } + }; + }; + } + // Verify the email // - // Verify the email with the generated code.. + // Verify the email with the generated code. rpc VerifyEmail (VerifyEmailRequest) returns (VerifyEmailResponse) { option (google.api.http) = { post: "/v2/users/{user_id}/email/verify" @@ -1310,6 +1335,29 @@ message ResendEmailCodeResponse{ optional string verification_code = 2; } +message SendEmailCodeRequest{ + string user_id = 1 [ + (validate.rules).string = {min_len: 1, max_len: 200}, + (google.api.field_behavior) = REQUIRED, + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + min_length: 1; + max_length: 200; + example: "\"69629026806489455\""; + } + ]; + // if no verification is specified, an email is sent with the default url + oneof verification { + SendEmailVerificationCode send_code = 2; + ReturnEmailVerificationCode return_code = 3; + } +} + +message SendEmailCodeResponse{ + zitadel.object.v2.Details details = 1; + // in case the verification was set to return_code, the code will be returned + optional string verification_code = 2; +} + message VerifyEmailRequest{ string user_id = 1 [ (validate.rules).string = {min_len: 1, max_len: 200}, From e1f0d463933a6cd8432863e625627ea12458e289 Mon Sep 17 00:00:00 2001 From: Harsha Reddy Date: Thu, 2 Jan 2025 15:44:15 +0530 Subject: [PATCH 05/21] fix(listUsers): Add Search User By Phone to User Service V2 (#9052) # Which Problems Are Solved Added search by phone to user Service V2. ``` curl --request POST \ --url https:///v2/users \ --header 'Accept: application/json' \ --header 'Authorization: Bearer ' \ --header 'Content-Type: application/json' \ --header 'content-type: application/json' \ --data '{ "query": { "offset": "0", "limit": 100, "asc": true }, "sortingColumn": "USER_FIELD_NAME_UNSPECIFIED", "queries": [ { "phoneQuery": { "number": "+12011223313", "method": "TEXT_QUERY_METHOD_EQUALS" } } ] }' ``` Why? Searching for a user by phone was missing from User Service V2 and V2 beta. # How the Problems Are Solved * Added to the SearchQuery proto * Added code to filter users by phone # Additional Changes N/A # Additional Context Search by phone is present in V3 User Service --------- Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com> --- .../admin/integration_test/iam_member_test.go | 8 +- .../management/integration_test/org_test.go | 8 +- .../user/v2/integration_test/email_test.go | 4 +- .../user/v2/integration_test/idp_link_test.go | 12 +- .../user/v2/integration_test/phone_test.go | 2 +- .../user/v2/integration_test/query_test.go | 340 ++++++++++-------- internal/api/grpc/user/v2/query.go | 6 + .../v2beta/integration_test/email_test.go | 2 +- .../v2beta/integration_test/phone_test.go | 2 +- .../v2beta/integration_test/query_test.go | 275 +++++++------- internal/api/grpc/user/v2beta/query.go | 6 + internal/integration/client.go | 4 +- proto/zitadel/user/v2/query.proto | 21 ++ proto/zitadel/user/v2beta/query.proto | 21 ++ 14 files changed, 423 insertions(+), 288 deletions(-) diff --git a/internal/api/grpc/admin/integration_test/iam_member_test.go b/internal/api/grpc/admin/integration_test/iam_member_test.go index 035cfa9f70..93d4417cba 100644 --- a/internal/api/grpc/admin/integration_test/iam_member_test.go +++ b/internal/api/grpc/admin/integration_test/iam_member_test.go @@ -35,7 +35,7 @@ func TestServer_ListIAMMemberRoles(t *testing.T) { } func TestServer_ListIAMMembers(t *testing.T) { - user := Instance.CreateHumanUserVerified(AdminCTX, Instance.DefaultOrg.Id, gofakeit.Email()) + user := Instance.CreateHumanUserVerified(AdminCTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()) _, err := Client.AddIAMMember(AdminCTX, &admin_pb.AddIAMMemberRequest{ UserId: user.GetUserId(), Roles: iamRoles, @@ -116,7 +116,7 @@ func TestServer_ListIAMMembers(t *testing.T) { } func TestServer_AddIAMMember(t *testing.T) { - user := Instance.CreateHumanUserVerified(AdminCTX, Instance.DefaultOrg.Id, gofakeit.Email()) + user := Instance.CreateHumanUserVerified(AdminCTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()) type args struct { ctx context.Context req *admin_pb.AddIAMMemberRequest @@ -190,7 +190,7 @@ func TestServer_AddIAMMember(t *testing.T) { } func TestServer_UpdateIAMMember(t *testing.T) { - user := Instance.CreateHumanUserVerified(AdminCTX, Instance.DefaultOrg.Id, gofakeit.Email()) + user := Instance.CreateHumanUserVerified(AdminCTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()) _, err := Client.AddIAMMember(AdminCTX, &admin_pb.AddIAMMemberRequest{ UserId: user.GetUserId(), Roles: []string{"IAM_OWNER"}, @@ -271,7 +271,7 @@ func TestServer_UpdateIAMMember(t *testing.T) { } func TestServer_RemoveIAMMember(t *testing.T) { - user := Instance.CreateHumanUserVerified(AdminCTX, Instance.DefaultOrg.Id, gofakeit.Email()) + user := Instance.CreateHumanUserVerified(AdminCTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()) _, err := Client.AddIAMMember(AdminCTX, &admin_pb.AddIAMMemberRequest{ UserId: user.GetUserId(), Roles: []string{"IAM_OWNER"}, diff --git a/internal/api/grpc/management/integration_test/org_test.go b/internal/api/grpc/management/integration_test/org_test.go index 8288ceb9e9..46693f14d7 100644 --- a/internal/api/grpc/management/integration_test/org_test.go +++ b/internal/api/grpc/management/integration_test/org_test.go @@ -39,7 +39,7 @@ func TestServer_ListOrgMemberRoles(t *testing.T) { } func TestServer_ListOrgMembers(t *testing.T) { - user := Instance.CreateHumanUserVerified(OrgCTX, Instance.DefaultOrg.Id, gofakeit.Email()) + user := Instance.CreateHumanUserVerified(OrgCTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()) _, err := Client.AddOrgMember(OrgCTX, &mgmt_pb.AddOrgMemberRequest{ UserId: user.GetUserId(), Roles: iamRoles[1:], @@ -120,7 +120,7 @@ func TestServer_ListOrgMembers(t *testing.T) { } func TestServer_AddOrgMember(t *testing.T) { - user := Instance.CreateHumanUserVerified(OrgCTX, Instance.DefaultOrg.Id, gofakeit.Email()) + user := Instance.CreateHumanUserVerified(OrgCTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()) type args struct { ctx context.Context req *mgmt_pb.AddOrgMemberRequest @@ -194,7 +194,7 @@ func TestServer_AddOrgMember(t *testing.T) { } func TestServer_UpdateOrgMember(t *testing.T) { - user := Instance.CreateHumanUserVerified(OrgCTX, Instance.DefaultOrg.Id, gofakeit.Email()) + user := Instance.CreateHumanUserVerified(OrgCTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()) _, err := Client.AddOrgMember(OrgCTX, &mgmt_pb.AddOrgMemberRequest{ UserId: user.GetUserId(), Roles: []string{"ORG_OWNER"}, @@ -275,7 +275,7 @@ func TestServer_UpdateOrgMember(t *testing.T) { } func TestServer_RemoveIAMMember(t *testing.T) { - user := Instance.CreateHumanUserVerified(OrgCTX, Instance.DefaultOrg.Id, gofakeit.Email()) + user := Instance.CreateHumanUserVerified(OrgCTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()) _, err := Client.AddOrgMember(OrgCTX, &mgmt_pb.AddOrgMemberRequest{ UserId: user.GetUserId(), Roles: []string{"ORG_OWNER"}, diff --git a/internal/api/grpc/user/v2/integration_test/email_test.go b/internal/api/grpc/user/v2/integration_test/email_test.go index de53bc68aa..ad63c2ce5e 100644 --- a/internal/api/grpc/user/v2/integration_test/email_test.go +++ b/internal/api/grpc/user/v2/integration_test/email_test.go @@ -147,7 +147,7 @@ func TestServer_SetEmail(t *testing.T) { func TestServer_ResendEmailCode(t *testing.T) { userID := Instance.CreateHumanUser(CTX).GetUserId() - verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email()).GetUserId() + verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()).GetUserId() tests := []struct { name string @@ -251,7 +251,7 @@ func TestServer_ResendEmailCode(t *testing.T) { func TestServer_SendEmailCode(t *testing.T) { userID := Instance.CreateHumanUser(CTX).GetUserId() - verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email()).GetUserId() + verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()).GetUserId() tests := []struct { name string diff --git a/internal/api/grpc/user/v2/integration_test/idp_link_test.go b/internal/api/grpc/user/v2/integration_test/idp_link_test.go index 116a095216..9d8160ab74 100644 --- a/internal/api/grpc/user/v2/integration_test/idp_link_test.go +++ b/internal/api/grpc/user/v2/integration_test/idp_link_test.go @@ -102,17 +102,17 @@ func TestServer_ListIDPLinks(t *testing.T) { orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListIDPLinks-%s", gofakeit.AppName()), gofakeit.Email()) instanceIdpResp := Instance.AddGenericOAuthProvider(IamCTX, Instance.DefaultOrg.Id) - userInstanceResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email()) + userInstanceResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email(), gofakeit.Phone()) _, err := Instance.CreateUserIDPlink(IamCTX, userInstanceResp.GetUserId(), "external_instance", instanceIdpResp.Id, "externalUsername_instance") require.NoError(t, err) ctxOrg := metadata.AppendToOutgoingContext(IamCTX, "x-zitadel-orgid", orgResp.GetOrganizationId()) orgIdpResp := Instance.AddOrgGenericOAuthProvider(ctxOrg, orgResp.OrganizationId) - userOrgResp := Instance.CreateHumanUserVerified(ctxOrg, orgResp.OrganizationId, gofakeit.Email()) + userOrgResp := Instance.CreateHumanUserVerified(ctxOrg, orgResp.OrganizationId, gofakeit.Email(), gofakeit.Phone()) _, err = Instance.CreateUserIDPlink(ctxOrg, userOrgResp.GetUserId(), "external_org", orgIdpResp.Id, "externalUsername_org") require.NoError(t, err) - userMultipleResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email()) + userMultipleResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email(), gofakeit.Phone()) _, err = Instance.CreateUserIDPlink(IamCTX, userMultipleResp.GetUserId(), "external_multi", instanceIdpResp.Id, "externalUsername_multi") require.NoError(t, err) _, err = Instance.CreateUserIDPlink(ctxOrg, userMultipleResp.GetUserId(), "external_multi", orgIdpResp.Id, "externalUsername_multi") @@ -256,17 +256,17 @@ func TestServer_RemoveIDPLink(t *testing.T) { orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListIDPLinks-%s", gofakeit.AppName()), gofakeit.Email()) instanceIdpResp := Instance.AddGenericOAuthProvider(IamCTX, Instance.DefaultOrg.Id) - userInstanceResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email()) + userInstanceResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email(), gofakeit.Phone()) _, err := Instance.CreateUserIDPlink(IamCTX, userInstanceResp.GetUserId(), "external_instance", instanceIdpResp.Id, "externalUsername_instance") require.NoError(t, err) ctxOrg := metadata.AppendToOutgoingContext(IamCTX, "x-zitadel-orgid", orgResp.GetOrganizationId()) orgIdpResp := Instance.AddOrgGenericOAuthProvider(ctxOrg, orgResp.OrganizationId) - userOrgResp := Instance.CreateHumanUserVerified(ctxOrg, orgResp.OrganizationId, gofakeit.Email()) + userOrgResp := Instance.CreateHumanUserVerified(ctxOrg, orgResp.OrganizationId, gofakeit.Email(), gofakeit.Phone()) _, err = Instance.CreateUserIDPlink(ctxOrg, userOrgResp.GetUserId(), "external_org", orgIdpResp.Id, "externalUsername_org") require.NoError(t, err) - userNoLinkResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email()) + userNoLinkResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email(), gofakeit.Phone()) type args struct { ctx context.Context diff --git a/internal/api/grpc/user/v2/integration_test/phone_test.go b/internal/api/grpc/user/v2/integration_test/phone_test.go index 1c1f75854d..49050c5fe6 100644 --- a/internal/api/grpc/user/v2/integration_test/phone_test.go +++ b/internal/api/grpc/user/v2/integration_test/phone_test.go @@ -123,7 +123,7 @@ func TestServer_SetPhone(t *testing.T) { func TestServer_ResendPhoneCode(t *testing.T) { userID := Instance.CreateHumanUser(CTX).GetUserId() - verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email()).GetUserId() + verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()).GetUserId() tests := []struct { name string diff --git a/internal/api/grpc/user/v2/integration_test/query_test.go b/internal/api/grpc/user/v2/integration_test/query_test.go index a00d1b1a48..2551a4a833 100644 --- a/internal/api/grpc/user/v2/integration_test/query_test.go +++ b/internal/api/grpc/user/v2/integration_test/query_test.go @@ -5,6 +5,7 @@ package user_test import ( "context" "fmt" + "slices" "testing" "time" @@ -24,7 +25,7 @@ func TestServer_GetUserByID(t *testing.T) { type args struct { ctx context.Context req *user.GetUserByIDRequest - dep func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) + dep func(ctx context.Context, request *user.GetUserByIDRequest) *userAttr } tests := []struct { name string @@ -39,8 +40,8 @@ func TestServer_GetUserByID(t *testing.T) { &user.GetUserByIDRequest{ UserId: "", }, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { - return nil, nil + func(ctx context.Context, request *user.GetUserByIDRequest) *userAttr { + return nil }, }, wantErr: true, @@ -52,8 +53,8 @@ func TestServer_GetUserByID(t *testing.T) { &user.GetUserByIDRequest{ UserId: "unknown", }, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { - return nil, nil + func(ctx context.Context, request *user.GetUserByIDRequest) *userAttr { + return nil }, }, wantErr: true, @@ -63,10 +64,10 @@ func TestServer_GetUserByID(t *testing.T) { args: args{ IamCTX, &user.GetUserByIDRequest{}, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - request.UserId = resp.GetUserId() - return &userAttr{resp.GetUserId(), username, nil, resp.GetDetails()}, nil + func(ctx context.Context, request *user.GetUserByIDRequest) *userAttr { + info := createUser(ctx, orgResp.OrganizationId, false) + request.UserId = info.UserID + return &info }, }, want: &user.GetUserByIDResponse{ @@ -90,7 +91,6 @@ func TestServer_GetUserByID(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -107,11 +107,10 @@ func TestServer_GetUserByID(t *testing.T) { args: args{ IamCTX, &user.GetUserByIDRequest{}, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - request.UserId = resp.GetUserId() - details := Instance.SetUserPassword(ctx, resp.GetUserId(), integration.UserPassword, true) - return &userAttr{resp.GetUserId(), username, details.GetChangeDate(), resp.GetDetails()}, nil + func(ctx context.Context, request *user.GetUserByIDRequest) *userAttr { + info := createUser(ctx, orgResp.OrganizationId, true) + request.UserId = info.UserID + return &info }, }, want: &user.GetUserByIDResponse{ @@ -135,7 +134,6 @@ func TestServer_GetUserByID(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, PasswordChangeRequired: true, @@ -152,9 +150,7 @@ func TestServer_GetUserByID(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - username := gofakeit.Email() - userAttr, err := tt.args.dep(tt.args.ctx, username, tt.args.req) - require.NoError(t, err) + userAttr := tt.args.dep(IamCTX, tt.args.req) retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { @@ -174,11 +170,12 @@ func TestServer_GetUserByID(t *testing.T) { tt.want.User.LoginNames = []string{userAttr.Username} if human := tt.want.User.GetHuman(); human != nil { human.Email.Email = userAttr.Username + human.Phone.Phone = userAttr.Phone if tt.want.User.GetHuman().GetPasswordChanged() != nil { human.PasswordChanged = userAttr.Changed } } - assert.Equal(ttt, tt.want.User, got.User) + assert.EqualExportedValues(ttt, tt.want.User, got.User) integration.AssertDetails(ttt, tt.want, got) }, retryDuration, tick) }) @@ -325,21 +322,60 @@ func TestServer_GetUserByID_Permission(t *testing.T) { } } +type userAttrs []userAttr + +func (u userAttrs) userIDs() []string { + ids := make([]string, len(u)) + for i := range u { + ids[i] = u[i].UserID + } + return ids +} + +func (u userAttrs) emails() []string { + emails := make([]string, len(u)) + for i := range u { + emails[i] = u[i].Username + } + return emails +} + type userAttr struct { UserID string Username string + Phone string Changed *timestamppb.Timestamp Details *object.Details } +func createUsers(ctx context.Context, orgID string, count int, passwordChangeRequired bool) userAttrs { + infos := make([]userAttr, count) + for i := 0; i < count; i++ { + infos[i] = createUser(ctx, orgID, passwordChangeRequired) + } + slices.Reverse(infos) + return infos +} + +func createUser(ctx context.Context, orgID string, passwordChangeRequired bool) userAttr { + username := gofakeit.Email() + // used as default country prefix + phone := "+41" + gofakeit.Phone() + resp := Instance.CreateHumanUserVerified(ctx, orgID, username, phone) + info := userAttr{resp.GetUserId(), username, phone, nil, resp.GetDetails()} + if passwordChangeRequired { + details := Instance.SetUserPassword(ctx, resp.GetUserId(), integration.UserPassword, true) + info.Changed = details.GetChangeDate() + } + return info +} + func TestServer_ListUsers(t *testing.T) { orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListUsersOrg-%s", gofakeit.AppName()), gofakeit.Email()) - userResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email()) type args struct { - ctx context.Context - count int - req *user.ListUsersRequest - dep func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) + ctx context.Context + req *user.ListUsersRequest + dep func(ctx context.Context, request *user.ListUsersRequest) userAttrs } tests := []struct { name string @@ -351,11 +387,11 @@ func TestServer_ListUsers(t *testing.T) { name: "list user by id, no permission", args: args{ UserCTX, - 0, &user.ListUsersRequest{}, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - request.Queries = append(request.Queries, InUserIDsQuery([]string{userResp.UserId})) - return []userAttr{}, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, false) + request.Queries = append(request.Queries, InUserIDsQuery([]string{info.UserID})) + return []userAttr{} }, }, want: &user.ListUsersResponse{ @@ -371,22 +407,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user by id, ok", args: args{ IamCTX, - 1, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - userIDs := make([]string, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - userIDs[i] = resp.GetUserId() - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - } - request.Queries = append(request.Queries, InUserIDsQuery(userIDs)) - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, false) + request.Queries = append(request.Queries, InUserIDsQuery([]string{info.UserID})) + return []userAttr{info} }, }, want: &user.ListUsersResponse{ @@ -412,7 +441,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -425,23 +453,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user by id, passwordChangeRequired, ok", args: args{ IamCTX, - 1, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - userIDs := make([]string, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - userIDs[i] = resp.GetUserId() - details := Instance.SetUserPassword(ctx, resp.GetUserId(), integration.UserPassword, true) - infos[i] = userAttr{resp.GetUserId(), username, details.GetChangeDate(), resp.GetDetails()} - } - request.Queries = append(request.Queries, InUserIDsQuery(userIDs)) - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, true) + request.Queries = append(request.Queries, InUserIDsQuery([]string{info.UserID})) + return []userAttr{info} }, }, want: &user.ListUsersResponse{ @@ -467,7 +487,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, PasswordChangeRequired: true, @@ -482,22 +501,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user by id multiple, ok", args: args{ IamCTX, - 3, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - userIDs := make([]string, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - userIDs[i] = resp.GetUserId() - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - } - request.Queries = append(request.Queries, InUserIDsQuery(userIDs)) - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + infos := createUsers(ctx, orgResp.OrganizationId, 3, false) + request.Queries = append(request.Queries, InUserIDsQuery(infos.userIDs())) + return infos }, }, want: &user.ListUsersResponse{ @@ -523,7 +535,27 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", + IsVerified: true, + }, + }, + }, + }, + { + State: user.UserState_USER_STATE_ACTIVE, + Type: &user.User_Human{ + Human: &user.HumanUser{ + Profile: &user.HumanProfile{ + GivenName: "Mickey", + FamilyName: "Mouse", + NickName: gu.Ptr("Mickey"), + DisplayName: gu.Ptr("Mickey Mouse"), + PreferredLanguage: gu.Ptr("nl"), + Gender: user.Gender_GENDER_MALE.Enum(), + }, + Email: &user.HumanEmail{ + IsVerified: true, + }, + Phone: &user.HumanPhone{ IsVerified: true, }, }, @@ -544,28 +576,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", - IsVerified: true, - }, - }, - }, - }, { - State: user.UserState_USER_STATE_ACTIVE, - Type: &user.User_Human{ - Human: &user.HumanUser{ - Profile: &user.HumanProfile{ - GivenName: "Mickey", - FamilyName: "Mouse", - NickName: gu.Ptr("Mickey"), - DisplayName: gu.Ptr("Mickey Mouse"), - PreferredLanguage: gu.Ptr("nl"), - Gender: user.Gender_GENDER_MALE.Enum(), - }, - Email: &user.HumanEmail{ - IsVerified: true, - }, - Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -578,22 +588,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user by username, ok", args: args{ IamCTX, - 1, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - userIDs := make([]string, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - userIDs[i] = resp.GetUserId() - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - request.Queries = append(request.Queries, UsernameQuery(username)) - } - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, false) + request.Queries = append(request.Queries, UsernameQuery(info.Username)) + return []userAttr{info} }, }, want: &user.ListUsersResponse{ @@ -619,7 +622,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -632,20 +634,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user in emails, ok", args: args{ IamCTX, - 1, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - } - request.Queries = append(request.Queries, InUserEmailsQuery(usernames)) - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, false) + request.Queries = append(request.Queries, InUserEmailsQuery([]string{info.Username})) + return []userAttr{info} }, }, want: &user.ListUsersResponse{ @@ -671,7 +668,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -684,20 +680,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user in emails multiple, ok", args: args{ IamCTX, - 3, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - } - request.Queries = append(request.Queries, InUserEmailsQuery(usernames)) - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + infos := createUsers(ctx, orgResp.OrganizationId, 3, false) + request.Queries = append(request.Queries, InUserEmailsQuery(infos.emails())) + return infos }, }, want: &user.ListUsersResponse{ @@ -723,7 +714,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -744,7 +734,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -765,7 +754,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -778,14 +766,81 @@ func TestServer_ListUsers(t *testing.T) { name: "list user in emails no found, ok", args: args{ IamCTX, - 3, &user.ListUsersRequest{Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), InUserEmailsQuery([]string{"notfound"}), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - return []userAttr{}, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + return []userAttr{} + }, + }, + want: &user.ListUsersResponse{ + Details: &object.ListDetails{ + TotalResult: 0, + Timestamp: timestamppb.Now(), + }, + SortingColumn: 0, + Result: []*user.User{}, + }, + }, + { + name: "list user phone, ok", + args: args{ + IamCTX, + &user.ListUsersRequest{ + Queries: []*user.SearchQuery{ + OrganizationIdQuery(orgResp.OrganizationId), + }, + }, + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, false) + request.Queries = append(request.Queries, PhoneQuery(info.Phone)) + return []userAttr{info} + }, + }, + want: &user.ListUsersResponse{ + Details: &object.ListDetails{ + TotalResult: 1, + Timestamp: timestamppb.Now(), + }, + SortingColumn: 0, + Result: []*user.User{ + { + State: user.UserState_USER_STATE_ACTIVE, + Type: &user.User_Human{ + Human: &user.HumanUser{ + Profile: &user.HumanProfile{ + GivenName: "Mickey", + FamilyName: "Mouse", + NickName: gu.Ptr("Mickey"), + DisplayName: gu.Ptr("Mickey Mouse"), + PreferredLanguage: gu.Ptr("nl"), + Gender: user.Gender_GENDER_MALE.Enum(), + }, + Email: &user.HumanEmail{ + IsVerified: true, + }, + Phone: &user.HumanPhone{ + IsVerified: true, + }, + }, + }, + }, + }, + }, + }, + { + name: "list user in emails no found, ok", + args: args{ + IamCTX, + &user.ListUsersRequest{Queries: []*user.SearchQuery{ + OrganizationIdQuery(orgResp.OrganizationId), + InUserEmailsQuery([]string{"notfound"}), + }, + }, + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + return []userAttr{} }, }, want: &user.ListUsersResponse{ @@ -801,19 +856,14 @@ func TestServer_ListUsers(t *testing.T) { name: "list user resourceowner multiple, ok", args: args{ IamCTX, - 3, &user.ListUsersRequest{}, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { orgResp := Instance.CreateOrganization(ctx, fmt.Sprintf("ListUsersResourceowner-%s", gofakeit.AppName()), gofakeit.Email()) - infos := make([]userAttr, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - } + infos := createUsers(ctx, orgResp.OrganizationId, 3, false) request.Queries = append(request.Queries, OrganizationIdQuery(orgResp.OrganizationId)) - request.Queries = append(request.Queries, InUserEmailsQuery(usernames)) - return infos, nil + request.Queries = append(request.Queries, InUserEmailsQuery(infos.emails())) + return infos }, }, want: &user.ListUsersResponse{ @@ -839,7 +889,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -860,7 +909,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -881,7 +929,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -893,12 +940,7 @@ func TestServer_ListUsers(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - usernames := make([]string, tt.args.count) - for i := 0; i < tt.args.count; i++ { - usernames[i] = gofakeit.Email() - } - infos, err := tt.args.dep(tt.args.ctx, usernames, tt.args.req) - require.NoError(t, err) + infos := tt.args.dep(IamCTX, tt.args.req) retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { @@ -924,6 +966,7 @@ func TestServer_ListUsers(t *testing.T) { tt.want.Result[i].LoginNames = []string{infos[i].Username} if human := tt.want.Result[i].GetHuman(); human != nil { human.Email.Email = infos[i].Username + human.Phone.Phone = infos[i].Phone if tt.want.Result[i].GetHuman().GetPasswordChanged() != nil { human.PasswordChanged = infos[i].Changed } @@ -931,7 +974,7 @@ func TestServer_ListUsers(t *testing.T) { tt.want.Result[i].Details = infos[i].Details } for i := range tt.want.Result { - assert.Contains(ttt, got.Result, tt.want.Result[i]) + assert.EqualExportedValues(ttt, got.Result[i], tt.want.Result[i]) } } integration.AssertListDetails(ttt, tt.want, got) @@ -958,6 +1001,15 @@ func InUserEmailsQuery(emails []string) *user.SearchQuery { } } +func PhoneQuery(number string) *user.SearchQuery { + return &user.SearchQuery{Query: &user.SearchQuery_PhoneQuery{ + PhoneQuery: &user.PhoneQuery{ + Number: number, + }, + }, + } +} + func UsernameQuery(username string) *user.SearchQuery { return &user.SearchQuery{Query: &user.SearchQuery_UserNameQuery{ UserNameQuery: &user.UserNameQuery{ diff --git a/internal/api/grpc/user/v2/query.go b/internal/api/grpc/user/v2/query.go index 564d4c1c0a..4cfbb46a51 100644 --- a/internal/api/grpc/user/v2/query.go +++ b/internal/api/grpc/user/v2/query.go @@ -238,6 +238,8 @@ func userQueryToQuery(query *user.SearchQuery, level uint8) (query.SearchQuery, return displayNameQueryToQuery(q.DisplayNameQuery) case *user.SearchQuery_EmailQuery: return emailQueryToQuery(q.EmailQuery) + case *user.SearchQuery_PhoneQuery: + return phoneQueryToQuery(q.PhoneQuery) case *user.SearchQuery_StateQuery: return stateQueryToQuery(q.StateQuery) case *user.SearchQuery_TypeQuery: @@ -285,6 +287,10 @@ func emailQueryToQuery(q *user.EmailQuery) (query.SearchQuery, error) { return query.NewUserEmailSearchQuery(q.EmailAddress, object.TextMethodToQuery(q.Method)) } +func phoneQueryToQuery(q *user.PhoneQuery) (query.SearchQuery, error) { + return query.NewUserPhoneSearchQuery(q.Number, object.TextMethodToQuery(q.Method)) +} + func stateQueryToQuery(q *user.StateQuery) (query.SearchQuery, error) { return query.NewUserStateSearchQuery(int32(q.State)) } diff --git a/internal/api/grpc/user/v2beta/integration_test/email_test.go b/internal/api/grpc/user/v2beta/integration_test/email_test.go index d22355978a..48957e99d9 100644 --- a/internal/api/grpc/user/v2beta/integration_test/email_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/email_test.go @@ -147,7 +147,7 @@ func TestServer_SetEmail(t *testing.T) { func TestServer_ResendEmailCode(t *testing.T) { userID := Instance.CreateHumanUser(CTX).GetUserId() - verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email()).GetUserId() + verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()).GetUserId() tests := []struct { name string diff --git a/internal/api/grpc/user/v2beta/integration_test/phone_test.go b/internal/api/grpc/user/v2beta/integration_test/phone_test.go index cd7199dcea..73d065231c 100644 --- a/internal/api/grpc/user/v2beta/integration_test/phone_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/phone_test.go @@ -125,7 +125,7 @@ func TestServer_SetPhone(t *testing.T) { func TestServer_ResendPhoneCode(t *testing.T) { userID := Instance.CreateHumanUser(CTX).GetUserId() - verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email()).GetUserId() + verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email(), gofakeit.Phone()).GetUserId() tests := []struct { name string diff --git a/internal/api/grpc/user/v2beta/integration_test/query_test.go b/internal/api/grpc/user/v2beta/integration_test/query_test.go index fc1d71926e..67fc609212 100644 --- a/internal/api/grpc/user/v2beta/integration_test/query_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/query_test.go @@ -5,6 +5,7 @@ package user_test import ( "context" "fmt" + "slices" "testing" "time" @@ -33,7 +34,7 @@ func TestServer_GetUserByID(t *testing.T) { type args struct { ctx context.Context req *user.GetUserByIDRequest - dep func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) + dep func(ctx context.Context, request *user.GetUserByIDRequest) *userAttr } tests := []struct { name string @@ -48,8 +49,8 @@ func TestServer_GetUserByID(t *testing.T) { &user.GetUserByIDRequest{ UserId: "", }, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { - return nil, nil + func(ctx context.Context, request *user.GetUserByIDRequest) *userAttr { + return nil }, }, wantErr: true, @@ -61,8 +62,8 @@ func TestServer_GetUserByID(t *testing.T) { &user.GetUserByIDRequest{ UserId: "unknown", }, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { - return nil, nil + func(ctx context.Context, request *user.GetUserByIDRequest) *userAttr { + return nil }, }, wantErr: true, @@ -72,10 +73,10 @@ func TestServer_GetUserByID(t *testing.T) { args: args{ IamCTX, &user.GetUserByIDRequest{}, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - request.UserId = resp.GetUserId() - return &userAttr{resp.GetUserId(), username, nil, resp.GetDetails()}, nil + func(ctx context.Context, request *user.GetUserByIDRequest) *userAttr { + info := createUser(ctx, orgResp.OrganizationId, false) + request.UserId = info.UserID + return &info }, }, want: &user.GetUserByIDResponse{ @@ -99,7 +100,6 @@ func TestServer_GetUserByID(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -116,11 +116,10 @@ func TestServer_GetUserByID(t *testing.T) { args: args{ IamCTX, &user.GetUserByIDRequest{}, - func(ctx context.Context, username string, request *user.GetUserByIDRequest) (*userAttr, error) { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - request.UserId = resp.GetUserId() - details := Instance.SetUserPassword(ctx, resp.GetUserId(), integration.UserPassword, true) - return &userAttr{resp.GetUserId(), username, details.GetChangeDate(), resp.GetDetails()}, nil + func(ctx context.Context, request *user.GetUserByIDRequest) *userAttr { + info := createUser(ctx, orgResp.OrganizationId, true) + request.UserId = info.UserID + return &info }, }, want: &user.GetUserByIDResponse{ @@ -144,7 +143,6 @@ func TestServer_GetUserByID(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, PasswordChangeRequired: true, @@ -161,9 +159,7 @@ func TestServer_GetUserByID(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - username := gofakeit.Email() - userAttr, err := tt.args.dep(tt.args.ctx, username, tt.args.req) - require.NoError(t, err) + userAttr := tt.args.dep(IamCTX, tt.args.req) retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { @@ -183,6 +179,7 @@ func TestServer_GetUserByID(t *testing.T) { tt.want.User.LoginNames = []string{userAttr.Username} if human := tt.want.User.GetHuman(); human != nil { human.Email.Email = userAttr.Username + human.Phone.Phone = userAttr.Phone if tt.want.User.GetHuman().GetPasswordChanged() != nil { human.PasswordChanged = userAttr.Changed } @@ -335,21 +332,60 @@ func TestServer_GetUserByID_Permission(t *testing.T) { } } +type userAttrs []userAttr + +func (u userAttrs) userIDs() []string { + ids := make([]string, len(u)) + for i := range u { + ids[i] = u[i].UserID + } + return ids +} + +func (u userAttrs) emails() []string { + emails := make([]string, len(u)) + for i := range u { + emails[i] = u[i].Username + } + return emails +} + type userAttr struct { UserID string Username string + Phone string Changed *timestamppb.Timestamp Details *object.Details } +func createUsers(ctx context.Context, orgID string, count int, passwordChangeRequired bool) userAttrs { + infos := make([]userAttr, count) + for i := 0; i < count; i++ { + infos[i] = createUser(ctx, orgID, passwordChangeRequired) + } + slices.Reverse(infos) + return infos +} + +func createUser(ctx context.Context, orgID string, passwordChangeRequired bool) userAttr { + username := gofakeit.Email() + // used as default country prefix + phone := "+41" + gofakeit.Phone() + resp := Instance.CreateHumanUserVerified(ctx, orgID, username, phone) + info := userAttr{resp.GetUserId(), username, phone, nil, resp.GetDetails()} + if passwordChangeRequired { + details := Instance.SetUserPassword(ctx, resp.GetUserId(), integration.UserPassword, true) + info.Changed = details.GetChangeDate() + } + return info +} + func TestServer_ListUsers(t *testing.T) { orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListUsersOrg-%s", gofakeit.AppName()), gofakeit.Email()) - userResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email()) type args struct { - ctx context.Context - count int - req *user.ListUsersRequest - dep func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) + ctx context.Context + req *user.ListUsersRequest + dep func(ctx context.Context, request *user.ListUsersRequest) userAttrs } tests := []struct { name string @@ -361,11 +397,11 @@ func TestServer_ListUsers(t *testing.T) { name: "list user by id, no permission", args: args{ UserCTX, - 0, &user.ListUsersRequest{}, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - request.Queries = append(request.Queries, InUserIDsQuery([]string{userResp.UserId})) - return []userAttr{}, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, false) + request.Queries = append(request.Queries, InUserIDsQuery([]string{info.UserID})) + return []userAttr{} }, }, want: &user.ListUsersResponse{ @@ -381,22 +417,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user by id, ok", args: args{ IamCTX, - 1, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - userIDs := make([]string, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - userIDs[i] = resp.GetUserId() - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - } - request.Queries = append(request.Queries, InUserIDsQuery(userIDs)) - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, false) + request.Queries = append(request.Queries, InUserIDsQuery([]string{info.UserID})) + return []userAttr{info} }, }, want: &user.ListUsersResponse{ @@ -422,7 +451,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -435,23 +463,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user by id, passwordChangeRequired, ok", args: args{ IamCTX, - 1, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - userIDs := make([]string, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - userIDs[i] = resp.GetUserId() - details := Instance.SetUserPassword(ctx, resp.GetUserId(), integration.UserPassword, true) - infos[i] = userAttr{resp.GetUserId(), username, details.GetChangeDate(), resp.GetDetails()} - } - request.Queries = append(request.Queries, InUserIDsQuery(userIDs)) - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, true) + request.Queries = append(request.Queries, InUserIDsQuery([]string{info.UserID})) + return []userAttr{info} }, }, want: &user.ListUsersResponse{ @@ -477,7 +497,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, PasswordChangeRequired: true, @@ -492,22 +511,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user by id multiple, ok", args: args{ IamCTX, - 3, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - userIDs := make([]string, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - userIDs[i] = resp.GetUserId() - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - } - request.Queries = append(request.Queries, InUserIDsQuery(userIDs)) - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + infos := createUsers(ctx, orgResp.OrganizationId, 3, false) + request.Queries = append(request.Queries, InUserIDsQuery(infos.userIDs())) + return infos }, }, want: &user.ListUsersResponse{ @@ -533,7 +545,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -554,7 +565,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -575,7 +585,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -588,22 +597,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user by username, ok", args: args{ IamCTX, - 1, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - userIDs := make([]string, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - userIDs[i] = resp.GetUserId() - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - request.Queries = append(request.Queries, UsernameQuery(username)) - } - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, false) + request.Queries = append(request.Queries, UsernameQuery(info.Username)) + return []userAttr{info} }, }, want: &user.ListUsersResponse{ @@ -629,7 +631,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -642,20 +643,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user in emails, ok", args: args{ IamCTX, - 1, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - } - request.Queries = append(request.Queries, InUserEmailsQuery(usernames)) - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, false) + request.Queries = append(request.Queries, InUserEmailsQuery([]string{info.Username})) + return []userAttr{info} }, }, want: &user.ListUsersResponse{ @@ -681,7 +677,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -694,20 +689,15 @@ func TestServer_ListUsers(t *testing.T) { name: "list user in emails multiple, ok", args: args{ IamCTX, - 3, &user.ListUsersRequest{ Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - infos := make([]userAttr, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - } - request.Queries = append(request.Queries, InUserEmailsQuery(usernames)) - return infos, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + infos := createUsers(ctx, orgResp.OrganizationId, 3, false) + request.Queries = append(request.Queries, InUserEmailsQuery(infos.emails())) + return infos }, }, want: &user.ListUsersResponse{ @@ -733,7 +723,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -754,7 +743,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -775,7 +763,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -788,14 +775,13 @@ func TestServer_ListUsers(t *testing.T) { name: "list user in emails no found, ok", args: args{ IamCTX, - 3, &user.ListUsersRequest{Queries: []*user.SearchQuery{ OrganizationIdQuery(orgResp.OrganizationId), InUserEmailsQuery([]string{"notfound"}), }, }, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - return []userAttr{}, nil + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + return []userAttr{} }, }, want: &user.ListUsersResponse{ @@ -807,23 +793,64 @@ func TestServer_ListUsers(t *testing.T) { Result: []*user.User{}, }, }, + { + name: "list user phone, ok", + args: args{ + IamCTX, + &user.ListUsersRequest{ + Queries: []*user.SearchQuery{ + OrganizationIdQuery(orgResp.OrganizationId), + }, + }, + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { + info := createUser(ctx, orgResp.OrganizationId, false) + request.Queries = append(request.Queries, PhoneQuery(info.Phone)) + return []userAttr{info} + }, + }, + want: &user.ListUsersResponse{ + Details: &object_v2beta.ListDetails{ + TotalResult: 1, + Timestamp: timestamppb.Now(), + }, + SortingColumn: 0, + Result: []*user.User{ + { + State: user.UserState_USER_STATE_ACTIVE, + Type: &user.User_Human{ + Human: &user.HumanUser{ + Profile: &user.HumanProfile{ + GivenName: "Mickey", + FamilyName: "Mouse", + NickName: gu.Ptr("Mickey"), + DisplayName: gu.Ptr("Mickey Mouse"), + PreferredLanguage: gu.Ptr("nl"), + Gender: user.Gender_GENDER_MALE.Enum(), + }, + Email: &user.HumanEmail{ + IsVerified: true, + }, + Phone: &user.HumanPhone{ + IsVerified: true, + }, + }, + }, + }, + }, + }, + }, { name: "list user resourceowner multiple, ok", args: args{ IamCTX, - 3, &user.ListUsersRequest{}, - func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { + func(ctx context.Context, request *user.ListUsersRequest) userAttrs { orgResp := Instance.CreateOrganization(ctx, fmt.Sprintf("ListUsersResourceowner-%s", gofakeit.AppName()), gofakeit.Email()) - infos := make([]userAttr, len(usernames)) - for i, username := range usernames { - resp := Instance.CreateHumanUserVerified(ctx, orgResp.OrganizationId, username) - infos[i] = userAttr{resp.GetUserId(), username, nil, resp.GetDetails()} - } + infos := createUsers(ctx, orgResp.OrganizationId, 3, false) request.Queries = append(request.Queries, OrganizationIdQuery(orgResp.OrganizationId)) - request.Queries = append(request.Queries, InUserEmailsQuery(usernames)) - return infos, nil + request.Queries = append(request.Queries, InUserEmailsQuery(infos.emails())) + return infos }, }, want: &user.ListUsersResponse{ @@ -849,7 +876,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -870,7 +896,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -891,7 +916,6 @@ func TestServer_ListUsers(t *testing.T) { IsVerified: true, }, Phone: &user.HumanPhone{ - Phone: "+41791234567", IsVerified: true, }, }, @@ -903,12 +927,7 @@ func TestServer_ListUsers(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - usernames := make([]string, tt.args.count) - for i := 0; i < tt.args.count; i++ { - usernames[i] = gofakeit.Email() - } - infos, err := tt.args.dep(tt.args.ctx, usernames, tt.args.req) - require.NoError(t, err) + infos := tt.args.dep(IamCTX, tt.args.req) retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { @@ -934,6 +953,7 @@ func TestServer_ListUsers(t *testing.T) { tt.want.Result[i].LoginNames = []string{infos[i].Username} if human := tt.want.Result[i].GetHuman(); human != nil { human.Email.Email = infos[i].Username + human.Phone.Phone = infos[i].Phone if tt.want.Result[i].GetHuman().GetPasswordChanged() != nil { human.PasswordChanged = infos[i].Changed } @@ -941,7 +961,7 @@ func TestServer_ListUsers(t *testing.T) { tt.want.Result[i].Details = detailsV2ToV2beta(infos[i].Details) } for i := range tt.want.Result { - assert.Contains(ttt, got.Result, tt.want.Result[i]) + assert.EqualExportedValues(ttt, got.Result[i], tt.want.Result[i]) } } integration.AssertListDetails(ttt, tt.want, got) @@ -968,6 +988,15 @@ func InUserEmailsQuery(emails []string) *user.SearchQuery { } } +func PhoneQuery(number string) *user.SearchQuery { + return &user.SearchQuery{Query: &user.SearchQuery_PhoneQuery{ + PhoneQuery: &user.PhoneQuery{ + Number: number, + }, + }, + } +} + func UsernameQuery(username string) *user.SearchQuery { return &user.SearchQuery{Query: &user.SearchQuery_UserNameQuery{ UserNameQuery: &user.UserNameQuery{ diff --git a/internal/api/grpc/user/v2beta/query.go b/internal/api/grpc/user/v2beta/query.go index 4567259d15..e3602abc33 100644 --- a/internal/api/grpc/user/v2beta/query.go +++ b/internal/api/grpc/user/v2beta/query.go @@ -238,6 +238,8 @@ func userQueryToQuery(query *user.SearchQuery, level uint8) (query.SearchQuery, return displayNameQueryToQuery(q.DisplayNameQuery) case *user.SearchQuery_EmailQuery: return emailQueryToQuery(q.EmailQuery) + case *user.SearchQuery_PhoneQuery: + return phoneQueryToQuery(q.PhoneQuery) case *user.SearchQuery_StateQuery: return stateQueryToQuery(q.StateQuery) case *user.SearchQuery_TypeQuery: @@ -285,6 +287,10 @@ func emailQueryToQuery(q *user.EmailQuery) (query.SearchQuery, error) { return query.NewUserEmailSearchQuery(q.EmailAddress, object.TextMethodToQuery(q.Method)) } +func phoneQueryToQuery(q *user.PhoneQuery) (query.SearchQuery, error) { + return query.NewUserPhoneSearchQuery(q.Number, object.TextMethodToQuery(q.Method)) +} + func stateQueryToQuery(q *user.StateQuery) (query.SearchQuery, error) { return query.NewUserStateSearchQuery(int32(q.State)) } diff --git a/internal/integration/client.go b/internal/integration/client.go index 34d4302ef4..c2297f7a09 100644 --- a/internal/integration/client.go +++ b/internal/integration/client.go @@ -271,7 +271,7 @@ func (i *Instance) CreateOrganizationWithUserID(ctx context.Context, name, userI return resp } -func (i *Instance) CreateHumanUserVerified(ctx context.Context, org, email string) *user_v2.AddHumanUserResponse { +func (i *Instance) CreateHumanUserVerified(ctx context.Context, org, email, phone string) *user_v2.AddHumanUserResponse { resp, err := i.Client.UserV2.AddHumanUser(ctx, &user_v2.AddHumanUserRequest{ Organization: &object.Organization{ Org: &object.Organization_OrgId{ @@ -292,7 +292,7 @@ func (i *Instance) CreateHumanUserVerified(ctx context.Context, org, email strin }, }, Phone: &user_v2.SetHumanPhone{ - Phone: "+41791234567", + Phone: phone, Verification: &user_v2.SetHumanPhone_IsVerified{ IsVerified: true, }, diff --git a/proto/zitadel/user/v2/query.proto b/proto/zitadel/user/v2/query.proto index 53f3446bca..71bb6dc594 100644 --- a/proto/zitadel/user/v2/query.proto +++ b/proto/zitadel/user/v2/query.proto @@ -30,6 +30,7 @@ message SearchQuery { NotQuery not_query = 13; InUserEmailsQuery in_user_emails_query = 14; OrganizationIdQuery organization_id_query = 15; + PhoneQuery phone_query = 16; } } @@ -184,6 +185,26 @@ message EmailQuery { ]; } +// Query for users with a specific phone. +message PhoneQuery { + string number = 1 [ + (validate.rules).string = {max_len: 20}, + (google.api.field_behavior) = REQUIRED, + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "Phone number of the user" + min_length: 1; + max_length: 20; + example: "\"+41791234567\""; + } + ]; + zitadel.object.v2.TextQueryMethod method = 2 [ + (validate.rules).enum.defined_only = true, + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "defines which text equality method is used"; + } + ]; +} + // Query for users with a specific state. message LoginNameQuery { string login_name = 1 [ diff --git a/proto/zitadel/user/v2beta/query.proto b/proto/zitadel/user/v2beta/query.proto index e339cdde71..caf02df747 100644 --- a/proto/zitadel/user/v2beta/query.proto +++ b/proto/zitadel/user/v2beta/query.proto @@ -30,6 +30,7 @@ message SearchQuery { NotQuery not_query = 13; InUserEmailsQuery in_user_emails_query = 14; OrganizationIdQuery organization_id_query = 15; + PhoneQuery phone_query = 16; } } @@ -184,6 +185,26 @@ message EmailQuery { ]; } +// Query for users with a specific phone. +message PhoneQuery { + string number = 1 [ + (validate.rules).string = {max_len: 20}, + (google.api.field_behavior) = REQUIRED, + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "Phone number of the user" + min_length: 1; + max_length: 20; + example: "\"+41791234567\""; + } + ]; + zitadel.object.v2beta.TextQueryMethod method = 2 [ + (validate.rules).enum.defined_only = true, + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "defines which text equality method is used"; + } + ]; +} + // Query for users with a specific state. message LoginNameQuery { string login_name = 1 [ From a3d80f93ff824241c34d895412141895812544c2 Mon Sep 17 00:00:00 2001 From: conblem Date: Thu, 2 Jan 2025 14:14:49 +0100 Subject: [PATCH 06/21] feat: v2 api add way to list authentication factors (#9065) # Which Problems Are Solved The v2 api currently has no endpoint the get all second factors of a user. # How the Problems Are Solved Our v1 api has the ListHumanAuthFactors which got added to the v2 api under the User resource. # Additional Changes # Additional Context Closes #8833 --------- Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com> --- internal/api/grpc/object/v2/converter.go | 103 ++++++++ .../user/v2/integration_test/user_test.go | 243 ++++++++++++++++++ internal/api/grpc/user/v2/user.go | 33 +++ internal/integration/client.go | 20 +- internal/query/user_auth_method.go | 17 ++ proto/zitadel/user/v2/user.proto | 47 ++++ proto/zitadel/user/v2/user_service.proto | 57 ++++ 7 files changed, 518 insertions(+), 2 deletions(-) diff --git a/internal/api/grpc/object/v2/converter.go b/internal/api/grpc/object/v2/converter.go index fe8aba5d6e..8cf0d8b1fa 100644 --- a/internal/api/grpc/object/v2/converter.go +++ b/internal/api/grpc/object/v2/converter.go @@ -9,6 +9,7 @@ import ( "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/query" "github.com/zitadel/zitadel/pkg/grpc/object/v2" + user_pb "github.com/zitadel/zitadel/pkg/grpc/user/v2" ) func DomainToDetailsPb(objectDetail *domain.ObjectDetails) *object.Details { @@ -70,3 +71,105 @@ func TextMethodToQuery(method object.TextQueryMethod) query.TextComparison { return -1 } } + +func AuthMethodsToPb(mfas *query.AuthMethods) []*user_pb.AuthFactor { + factors := make([]*user_pb.AuthFactor, len(mfas.AuthMethods)) + for i, mfa := range mfas.AuthMethods { + factors[i] = AuthMethodToPb(mfa) + } + return factors +} + +func AuthMethodToPb(mfa *query.AuthMethod) *user_pb.AuthFactor { + factor := &user_pb.AuthFactor{ + State: MFAStateToPb(mfa.State), + } + switch mfa.Type { + case domain.UserAuthMethodTypeTOTP: + factor.Type = &user_pb.AuthFactor_Otp{ + Otp: &user_pb.AuthFactorOTP{}, + } + case domain.UserAuthMethodTypeU2F: + factor.Type = &user_pb.AuthFactor_U2F{ + U2F: &user_pb.AuthFactorU2F{ + Id: mfa.TokenID, + Name: mfa.Name, + }, + } + case domain.UserAuthMethodTypeOTPSMS: + factor.Type = &user_pb.AuthFactor_OtpSms{ + OtpSms: &user_pb.AuthFactorOTPSMS{}, + } + case domain.UserAuthMethodTypeOTPEmail: + factor.Type = &user_pb.AuthFactor_OtpEmail{ + OtpEmail: &user_pb.AuthFactorOTPEmail{}, + } + case domain.UserAuthMethodTypeUnspecified: + case domain.UserAuthMethodTypePasswordless: + case domain.UserAuthMethodTypePassword: + case domain.UserAuthMethodTypeIDP: + case domain.UserAuthMethodTypeOTP: + case domain.UserAuthMethodTypePrivateKey: + } + return factor +} + +func AuthFactorsToPb(authFactors []user_pb.AuthFactors) []domain.UserAuthMethodType { + factors := make([]domain.UserAuthMethodType, len(authFactors)) + for i, authFactor := range authFactors { + factors[i] = AuthFactorToPb(authFactor) + } + return factors +} + +func AuthFactorToPb(authFactor user_pb.AuthFactors) domain.UserAuthMethodType { + switch authFactor { + case user_pb.AuthFactors_OTP: + return domain.UserAuthMethodTypeTOTP + case user_pb.AuthFactors_OTP_SMS: + return domain.UserAuthMethodTypeOTPSMS + case user_pb.AuthFactors_OTP_EMAIL: + return domain.UserAuthMethodTypeOTPEmail + case user_pb.AuthFactors_U2F: + return domain.UserAuthMethodTypeU2F + default: + return domain.UserAuthMethodTypeUnspecified + } +} + +func AuthFactorStatesToPb(authFactorStates []user_pb.AuthFactorState) []domain.MFAState { + factorStates := make([]domain.MFAState, len(authFactorStates)) + for i, authFactorState := range authFactorStates { + factorStates[i] = AuthFactorStateToPb(authFactorState) + } + return factorStates +} + +func AuthFactorStateToPb(authFactorState user_pb.AuthFactorState) domain.MFAState { + switch authFactorState { + case user_pb.AuthFactorState_AUTH_FACTOR_STATE_UNSPECIFIED: + return domain.MFAStateUnspecified + case user_pb.AuthFactorState_AUTH_FACTOR_STATE_NOT_READY: + return domain.MFAStateNotReady + case user_pb.AuthFactorState_AUTH_FACTOR_STATE_READY: + return domain.MFAStateReady + case user_pb.AuthFactorState_AUTH_FACTOR_STATE_REMOVED: + return domain.MFAStateRemoved + default: + return domain.MFAStateUnspecified + } +} + +func MFAStateToPb(state domain.MFAState) user_pb.AuthFactorState { + switch state { + case domain.MFAStateNotReady: + return user_pb.AuthFactorState_AUTH_FACTOR_STATE_NOT_READY + case domain.MFAStateReady: + return user_pb.AuthFactorState_AUTH_FACTOR_STATE_READY + case domain.MFAStateUnspecified, domain.MFAStateRemoved: + // Handle all remaining cases so the linter succeeds + return user_pb.AuthFactorState_AUTH_FACTOR_STATE_UNSPECIFIED + default: + return user_pb.AuthFactorState_AUTH_FACTOR_STATE_UNSPECIFIED + } +} diff --git a/internal/api/grpc/user/v2/integration_test/user_test.go b/internal/api/grpc/user/v2/integration_test/user_test.go index 206183351e..8d4c254c6b 100644 --- a/internal/api/grpc/user/v2/integration_test/user_test.go +++ b/internal/api/grpc/user/v2/integration_test/user_test.go @@ -10,6 +10,8 @@ import ( "testing" "time" + "github.com/zitadel/logging" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" @@ -2629,6 +2631,247 @@ func TestServer_ListAuthenticationMethodTypes(t *testing.T) { } } +func TestServer_ListAuthenticationFactors(t *testing.T) { + tests := []struct { + name string + args *user.ListAuthenticationFactorsRequest + want *user.ListAuthenticationFactorsResponse + dep func(args *user.ListAuthenticationFactorsRequest, want *user.ListAuthenticationFactorsResponse) + wantErr bool + ctx context.Context + }{ + { + name: "no auth", + args: &user.ListAuthenticationFactorsRequest{}, + want: &user.ListAuthenticationFactorsResponse{ + Result: nil, + }, + dep: func(args *user.ListAuthenticationFactorsRequest, want *user.ListAuthenticationFactorsResponse) { + userIDWithoutAuth := Instance.CreateHumanUser(CTX).GetUserId() + args.UserId = userIDWithoutAuth + }, + ctx: CTX, + }, + { + name: "with u2f", + args: &user.ListAuthenticationFactorsRequest{}, + want: &user.ListAuthenticationFactorsResponse{ + Result: []*user.AuthFactor{ + { + State: user.AuthFactorState_AUTH_FACTOR_STATE_READY, + }, + }, + }, + dep: func(args *user.ListAuthenticationFactorsRequest, want *user.ListAuthenticationFactorsResponse) { + userWithU2F := Instance.CreateHumanUser(CTX).GetUserId() + U2FId := Instance.RegisterUserU2F(CTX, userWithU2F) + + args.UserId = userWithU2F + want.Result[0].Type = &user.AuthFactor_U2F{ + U2F: &user.AuthFactorU2F{ + Id: U2FId, + Name: "nice name", + }, + } + }, + ctx: CTX, + }, + { + name: "with totp, u2f", + args: &user.ListAuthenticationFactorsRequest{}, + want: &user.ListAuthenticationFactorsResponse{ + Result: []*user.AuthFactor{ + { + State: user.AuthFactorState_AUTH_FACTOR_STATE_READY, + Type: &user.AuthFactor_Otp{ + Otp: &user.AuthFactorOTP{}, + }, + }, + { + State: user.AuthFactorState_AUTH_FACTOR_STATE_READY, + }, + }, + }, + dep: func(args *user.ListAuthenticationFactorsRequest, want *user.ListAuthenticationFactorsResponse) { + userWithTOTP := Instance.CreateHumanUserWithTOTP(CTX, "secret").GetUserId() + U2FIdWithTOTP := Instance.RegisterUserU2F(CTX, userWithTOTP) + + args.UserId = userWithTOTP + want.Result[1].Type = &user.AuthFactor_U2F{ + U2F: &user.AuthFactorU2F{ + Id: U2FIdWithTOTP, + Name: "nice name", + }, + } + }, + ctx: CTX, + }, + { + name: "with totp, u2f filtered", + args: &user.ListAuthenticationFactorsRequest{ + AuthFactors: []user.AuthFactors{user.AuthFactors_U2F}, + }, + want: &user.ListAuthenticationFactorsResponse{ + Result: []*user.AuthFactor{ + { + State: user.AuthFactorState_AUTH_FACTOR_STATE_READY, + }, + }, + }, + dep: func(args *user.ListAuthenticationFactorsRequest, want *user.ListAuthenticationFactorsResponse) { + userWithTOTP := Instance.CreateHumanUserWithTOTP(CTX, "secret").GetUserId() + U2FIdWithTOTP := Instance.RegisterUserU2F(CTX, userWithTOTP) + + args.UserId = userWithTOTP + want.Result[0].Type = &user.AuthFactor_U2F{ + U2F: &user.AuthFactorU2F{ + Id: U2FIdWithTOTP, + Name: "nice name", + }, + } + }, + ctx: CTX, + }, + { + name: "with sms", + args: &user.ListAuthenticationFactorsRequest{}, + want: &user.ListAuthenticationFactorsResponse{ + Result: []*user.AuthFactor{ + { + State: user.AuthFactorState_AUTH_FACTOR_STATE_READY, + Type: &user.AuthFactor_OtpSms{ + OtpSms: &user.AuthFactorOTPSMS{}, + }, + }, + }, + }, + dep: func(args *user.ListAuthenticationFactorsRequest, want *user.ListAuthenticationFactorsResponse) { + userWithSMS := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.GetId(), gofakeit.Email(), gofakeit.Phone()).GetUserId() + Instance.RegisterUserOTPSMS(CTX, userWithSMS) + + args.UserId = userWithSMS + }, + ctx: CTX, + }, + { + name: "with email", + args: &user.ListAuthenticationFactorsRequest{}, + want: &user.ListAuthenticationFactorsResponse{ + Result: []*user.AuthFactor{ + { + State: user.AuthFactorState_AUTH_FACTOR_STATE_READY, + Type: &user.AuthFactor_OtpEmail{ + OtpEmail: &user.AuthFactorOTPEmail{}, + }, + }, + }, + }, + dep: func(args *user.ListAuthenticationFactorsRequest, want *user.ListAuthenticationFactorsResponse) { + userWithEmail := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.GetId(), gofakeit.Email(), gofakeit.Phone()).GetUserId() + Instance.RegisterUserOTPEmail(CTX, userWithEmail) + + args.UserId = userWithEmail + }, + ctx: CTX, + }, + { + name: "with not ready u2f", + args: &user.ListAuthenticationFactorsRequest{}, + want: &user.ListAuthenticationFactorsResponse{ + Result: []*user.AuthFactor{}, + }, + dep: func(args *user.ListAuthenticationFactorsRequest, want *user.ListAuthenticationFactorsResponse) { + userWithNotReadyU2F := Instance.CreateHumanUser(CTX).GetUserId() + _, err := Instance.Client.UserV2.RegisterU2F(CTX, &user.RegisterU2FRequest{ + UserId: userWithNotReadyU2F, + Domain: Instance.Domain, + }) + logging.OnError(err).Panic("Could not register u2f") + + args.UserId = userWithNotReadyU2F + }, + ctx: CTX, + }, + { + name: "with not ready u2f state filtered", + args: &user.ListAuthenticationFactorsRequest{ + States: []user.AuthFactorState{user.AuthFactorState_AUTH_FACTOR_STATE_NOT_READY}, + }, + want: &user.ListAuthenticationFactorsResponse{ + Result: []*user.AuthFactor{ + { + State: user.AuthFactorState_AUTH_FACTOR_STATE_NOT_READY, + }, + }, + }, + dep: func(args *user.ListAuthenticationFactorsRequest, want *user.ListAuthenticationFactorsResponse) { + userWithNotReadyU2F := Instance.CreateHumanUser(CTX).GetUserId() + U2FNotReady, err := Instance.Client.UserV2.RegisterU2F(CTX, &user.RegisterU2FRequest{ + UserId: userWithNotReadyU2F, + Domain: Instance.Domain, + }) + logging.OnError(err).Panic("Could not register u2f") + + args.UserId = userWithNotReadyU2F + want.Result[0].Type = &user.AuthFactor_U2F{ + U2F: &user.AuthFactorU2F{ + Id: U2FNotReady.GetU2FId(), + Name: "", + }, + } + }, + ctx: CTX, + }, + { + name: "with no userId", + args: &user.ListAuthenticationFactorsRequest{ + UserId: "", + }, + ctx: CTX, + wantErr: true, + }, + { + name: "with no permission", + args: &user.ListAuthenticationFactorsRequest{}, + dep: func(args *user.ListAuthenticationFactorsRequest, want *user.ListAuthenticationFactorsResponse) { + userWithTOTP := Instance.CreateHumanUserWithTOTP(CTX, "totp").GetUserId() + + args.UserId = userWithTOTP + }, + ctx: UserCTX, + wantErr: true, + }, + { + name: "with unknown user", + args: &user.ListAuthenticationFactorsRequest{ + UserId: "unknown", + }, + want: &user.ListAuthenticationFactorsResponse{}, + ctx: CTX, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.dep != nil { + tt.dep(tt.args, tt.want) + } + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + got, err := Client.ListAuthenticationFactors(tt.ctx, tt.args) + if tt.wantErr { + require.Error(ttt, err) + return + } + require.NoError(ttt, err) + + assert.ElementsMatch(t, tt.want.GetResult(), got.GetResult()) + }, retryDuration, tick, "timeout waiting for expected auth methods result") + + }) + } +} + func TestServer_CreateInviteCode(t *testing.T) { type args struct { ctx context.Context diff --git a/internal/api/grpc/user/v2/user.go b/internal/api/grpc/user/v2/user.go index c0416f84aa..9d99f210e5 100644 --- a/internal/api/grpc/user/v2/user.go +++ b/internal/api/grpc/user/v2/user.go @@ -597,6 +597,39 @@ func (s *Server) ListAuthenticationMethodTypes(ctx context.Context, req *user.Li }, nil } +func (s *Server) ListAuthenticationFactors(ctx context.Context, req *user.ListAuthenticationFactorsRequest) (*user.ListAuthenticationFactorsResponse, error) { + query := new(query.UserAuthMethodSearchQueries) + + if err := query.AppendUserIDQuery(req.UserId); err != nil { + return nil, err + } + + authMethodsType := []domain.UserAuthMethodType{domain.UserAuthMethodTypeU2F, domain.UserAuthMethodTypeTOTP, domain.UserAuthMethodTypeOTPSMS, domain.UserAuthMethodTypeOTPEmail} + if len(req.GetAuthFactors()) > 0 { + authMethodsType = object.AuthFactorsToPb(req.GetAuthFactors()) + } + if err := query.AppendAuthMethodsQuery(authMethodsType...); err != nil { + return nil, err + } + + states := []domain.MFAState{domain.MFAStateReady} + if len(req.GetStates()) > 0 { + states = object.AuthFactorStatesToPb(req.GetStates()) + } + if err := query.AppendStatesQuery(states...); err != nil { + return nil, err + } + + authMethods, err := s.query.SearchUserAuthMethods(ctx, query, s.checkPermission) + if err != nil { + return nil, err + } + + return &user.ListAuthenticationFactorsResponse{ + Result: object.AuthMethodsToPb(authMethods), + }, nil +} + func authMethodTypesToPb(methodTypes []domain.UserAuthMethodType) []user.AuthenticationMethodType { methods := make([]user.AuthenticationMethodType, len(methodTypes)) for i, method := range methodTypes { diff --git a/internal/integration/client.go b/internal/integration/client.go index c2297f7a09..af30f0e642 100644 --- a/internal/integration/client.go +++ b/internal/integration/client.go @@ -327,7 +327,7 @@ func (i *Instance) CreateUserIDPlink(ctx context.Context, userID, externalID, id ) } -func (i *Instance) RegisterUserPasskey(ctx context.Context, userID string) { +func (i *Instance) RegisterUserPasskey(ctx context.Context, userID string) string { reg, err := i.Client.UserV2.CreatePasskeyRegistrationLink(ctx, &user_v2.CreatePasskeyRegistrationLinkRequest{ UserId: userID, Medium: &user_v2.CreatePasskeyRegistrationLinkRequest_ReturnCode{}, @@ -350,9 +350,10 @@ func (i *Instance) RegisterUserPasskey(ctx context.Context, userID string) { PasskeyName: "nice name", }) logging.OnError(err).Panic("create user passkey") + return pkr.GetPasskeyId() } -func (i *Instance) RegisterUserU2F(ctx context.Context, userID string) { +func (i *Instance) RegisterUserU2F(ctx context.Context, userID string) string { pkr, err := i.Client.UserV2.RegisterU2F(ctx, &user_v2.RegisterU2FRequest{ UserId: userID, Domain: i.Domain, @@ -368,6 +369,21 @@ func (i *Instance) RegisterUserU2F(ctx context.Context, userID string) { TokenName: "nice name", }) logging.OnError(err).Panic("create user u2f") + return pkr.GetU2FId() +} + +func (i *Instance) RegisterUserOTPSMS(ctx context.Context, userID string) { + _, err := i.Client.UserV2.AddOTPSMS(ctx, &user_v2.AddOTPSMSRequest{ + UserId: userID, + }) + logging.OnError(err).Panic("create user sms") +} + +func (i *Instance) RegisterUserOTPEmail(ctx context.Context, userID string) { + _, err := i.Client.UserV2.AddOTPEmail(ctx, &user_v2.AddOTPEmailRequest{ + UserId: userID, + }) + logging.OnError(err).Panic("create user email") } func (i *Instance) SetUserPassword(ctx context.Context, userID, password string, changeRequired bool) *object.Details { diff --git a/internal/query/user_auth_method.go b/internal/query/user_auth_method.go index 3ba794ee0f..0687545aef 100644 --- a/internal/query/user_auth_method.go +++ b/internal/query/user_auth_method.go @@ -270,6 +270,14 @@ func NewUserAuthMethodTypesSearchQuery(values ...domain.UserAuthMethodType) (Sea return NewListQuery(UserAuthMethodColumnMethodType, list, ListIn) } +func NewUserAuthMethodStatesSearchQuery(values ...domain.MFAState) (SearchQuery, error) { + list := make([]interface{}, len(values)) + for i, value := range values { + list[i] = value + } + return NewListQuery(UserAuthMethodColumnState, list, ListIn) +} + func (r *UserAuthMethodSearchQueries) AppendResourceOwnerQuery(orgID string) error { query, err := NewUserAuthMethodResourceOwnerSearchQuery(orgID) if err != nil { @@ -306,6 +314,15 @@ func (r *UserAuthMethodSearchQueries) AppendStateQuery(state domain.MFAState) er return nil } +func (r *UserAuthMethodSearchQueries) AppendStatesQuery(state ...domain.MFAState) error { + query, err := NewUserAuthMethodStatesSearchQuery(state...) + if err != nil { + return err + } + r.Queries = append(r.Queries, query) + return nil +} + func (r *UserAuthMethodSearchQueries) AppendAuthMethodQuery(authMethod domain.UserAuthMethodType) error { query, err := NewUserAuthMethodTypeSearchQuery(authMethod) if err != nil { diff --git a/proto/zitadel/user/v2/user.proto b/proto/zitadel/user/v2/user.proto index cfeebbf33d..b569b81bbd 100644 --- a/proto/zitadel/user/v2/user.proto +++ b/proto/zitadel/user/v2/user.proto @@ -276,6 +276,36 @@ message Passkey { ]; } +message AuthFactor { + AuthFactorState state = 1 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "current state of the auth factor"; + } + ]; + oneof type { + AuthFactorOTP otp = 2 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "TOTP second factor" + } + ]; + AuthFactorU2F u2f = 3 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "U2F second factor" + } + ]; + AuthFactorOTPSMS otp_sms = 4 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "SMS second factor" + } + ]; + AuthFactorOTPEmail otp_email = 5 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "Email second factor" + } + ]; + } +} + enum AuthFactorState { AUTH_FACTOR_STATE_UNSPECIFIED = 0; AUTH_FACTOR_STATE_NOT_READY = 1; @@ -283,6 +313,23 @@ enum AuthFactorState { AUTH_FACTOR_STATE_REMOVED = 3; } +message AuthFactorOTP {} +message AuthFactorOTPSMS {} +message AuthFactorOTPEmail {} + +message AuthFactorU2F { + string id = 1 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + example: "\"69629023906488334\"" + } + ]; + string name = 2 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + example: "\"fido key\"" + } + ]; +} + message SendInviteCode { // Optionally set a url_template, which will be used in the invite mail sent by ZITADEL to guide the user to your invitation page. // If no template is set, the default ZITADEL url will be used. diff --git a/proto/zitadel/user/v2/user_service.proto b/proto/zitadel/user/v2/user_service.proto index 8ae7c1bc08..7e5b8a02e8 100644 --- a/proto/zitadel/user/v2/user_service.proto +++ b/proto/zitadel/user/v2/user_service.proto @@ -1110,6 +1110,28 @@ service UserService { }; } + rpc ListAuthenticationFactors(ListAuthenticationFactorsRequest) returns (ListAuthenticationFactorsResponse) { + option (google.api.http) = { + post: "/v2/users/{user_id}/authentication_factors/_search" + }; + + option (zitadel.protoc_gen_zitadel.v2.options) = { + auth_option: { + permission: "authenticated" + } + }; + + option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = { + responses: { + key: "200" + value: { + description: "OK"; + } + }; + }; + + } + // Create an invite code for a user // // Create an invite code for a user to initialize their first authentication method (password, passkeys, IdP) depending on the organization's available methods. @@ -2216,6 +2238,41 @@ enum AuthenticationMethodType { AUTHENTICATION_METHOD_TYPE_OTP_EMAIL = 7; } +message ListAuthenticationFactorsRequest{ + string user_id = 1 [ + (validate.rules).string = {min_len: 1, max_len: 200}, + (google.api.field_behavior) = REQUIRED, + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + min_length: 1; + max_length: 200; + example: "\"69629026806489455\""; + } + ]; + repeated AuthFactors auth_factors = 2 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "Specify the Auth Factors you are interested in" + default: "All Auth Factors" + } + ]; + repeated AuthFactorState states = 3 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + description: "Specify the state of the Auth Factors" + default: "Auth Factors that are ready" + } + ]; +} + +enum AuthFactors { + OTP = 0; + OTP_SMS = 1; + OTP_EMAIL = 2; + U2F = 3; +} + +message ListAuthenticationFactorsResponse { + repeated zitadel.user.v2.AuthFactor result = 1; +} + message CreateInviteCodeRequest { string user_id = 1 [ (validate.rules).string = {min_len: 1, max_len: 200}, From 2bfdb72bf359713cbb12dfb03824a9e25844154c Mon Sep 17 00:00:00 2001 From: Elio Bischof Date: Fri, 3 Jan 2025 15:00:27 +0100 Subject: [PATCH 07/21] docs: fix reverse proxy guides (#9118) # Which Problems Are Solved Commands for installing compose stacks with reverse proxies don't work. # How the Problems Are Solved - The `docker compose up` commands are fixed by specifying all necessary services to spin up. This is obviously not (or not with all docker compose versions) resolved by the dependencies declarations. - The initial postgres admin username is postgres. - Fix postgres health check to succeed before the init job created the DB. - A hint tells the user to install the grpcurl binary. # Additional Changes - Passing `--wait` to `docker compose up` doesn't require us to sleep for exactly three seconds. - It looks to me like the order of the depends_on declaration for zitadel matters, but I don't understand why. I changed it so that it's for sure correct. - Silenced some command outputs - Removed the version property from all compose files to avoid the following warning ``` WARN[0000] /tmp/caddy-example/docker-compose-base.yaml: the attribute `version` is obsolete, it will be ignored, please remove it to avoid potential confusion ``` # Additional Context - Closes https://github.com/zitadel/zitadel/issues/9115 This is the easiest way to test the updated docs: ```bash # Use this PR branches files: export ZITADEL_CONFIG_FILES=https://raw.githubusercontent.com/zitadel/zitadel/refs/heads/fix-reverse-proxy-guides/docs/docs/self-hosting/manage/reverseproxy ``` The rest of the commands as described in https://docs-git-fix-reverse-proxy-guides-zitadel.vercel.app/docs/self-hosting/manage/reverseproxy/caddy ![image](https://github.com/user-attachments/assets/949d2c2a-246a-49a2-916a-e77250771074) --- .../loadbalancing-example/docker-compose.yaml | 1 - .../manage/configure/docker-compose.yaml | 2 -- .../reverseproxy/_proxy_guide_tls_mode.mdx | 12 ++++++------ .../reverseproxy/caddy/docker-compose.yaml | 2 -- .../manage/reverseproxy/docker-compose.yaml | 17 +++++++---------- .../reverseproxy/httpd/docker-compose.yaml | 2 -- .../reverseproxy/nginx/docker-compose.yaml | 2 -- .../reverseproxy/traefik/docker-compose.yaml | 2 -- .../host.docker.internal/docker-compose.yaml | 2 -- e2e/config/localhost/docker-compose.yaml | 2 -- e2e/docker-compose.yaml | 2 -- 11 files changed, 13 insertions(+), 33 deletions(-) diff --git a/docs/docs/self-hosting/deploy/loadbalancing-example/docker-compose.yaml b/docs/docs/self-hosting/deploy/loadbalancing-example/docker-compose.yaml index 2b9266c798..94d8f438dc 100644 --- a/docs/docs/self-hosting/deploy/loadbalancing-example/docker-compose.yaml +++ b/docs/docs/self-hosting/deploy/loadbalancing-example/docker-compose.yaml @@ -1,4 +1,3 @@ -version: '3.8' services: traefik: diff --git a/docs/docs/self-hosting/manage/configure/docker-compose.yaml b/docs/docs/self-hosting/manage/configure/docker-compose.yaml index 8e5c9fbc05..abd1818a7b 100644 --- a/docs/docs/self-hosting/manage/configure/docker-compose.yaml +++ b/docs/docs/self-hosting/manage/configure/docker-compose.yaml @@ -1,5 +1,3 @@ -version: "3.8" - services: zitadel: restart: "always" diff --git a/docs/docs/self-hosting/manage/reverseproxy/_proxy_guide_tls_mode.mdx b/docs/docs/self-hosting/manage/reverseproxy/_proxy_guide_tls_mode.mdx index debca2f4f5..1cacf076e5 100644 --- a/docs/docs/self-hosting/manage/reverseproxy/_proxy_guide_tls_mode.mdx +++ b/docs/docs/self-hosting/manage/reverseproxy/_proxy_guide_tls_mode.mdx @@ -24,7 +24,7 @@ export const Description = ({mode, link}) => { } export const Commands = ({mode, name, lower, configfilename}) => { - let genCert = '# Generate a self signed certificate and key.\nopenssl req -x509 -batch -subj "/CN=127.0.0.1.sslip.io/O=ZITADEL Demo" -nodes -newkey rsa:2048 -keyout ./selfsigned.key -out ./selfsigned.crt\n\n'; + let genCert = '# Generate a self signed certificate and key.\nopenssl req -x509 -batch -subj "/CN=127.0.0.1.sslip.io/O=ZITADEL Demo" -nodes -newkey rsa:2048 -keyout ./selfsigned.key -out ./selfsigned.crt 2>/dev/null\n\n'; let connPort = "443" let connInsecureFlag = "--insecure " let connScheme = "https" @@ -42,16 +42,16 @@ export const Commands = ({mode, name, lower, configfilename}) => { {'# Download the configuration files.'}{'\n'} {'export ZITADEL_CONFIG_FILES=https://raw.githubusercontent.com/zitadel/zitadel/main/docs/docs/self-hosting/manage/reverseproxy\n'} - {`wget $\{ZITADEL_CONFIG_FILES\}/docker-compose.yaml -O docker-compose-base.yaml`}{'\n'} - {'wget $\{ZITADEL_CONFIG_FILES\}/'}{lower}{'/docker-compose.yaml -O docker-compose-'}{lower}{'.yaml'}{'\n'} - {'wget $\{ZITADEL_CONFIG_FILES\}/'}{lower}{'/'}{configfilename}{' -O '}{configfilename}{'\n'} + {'wget $\{ZITADEL_CONFIG_FILES\}/docker-compose.yaml -O docker-compose-base.yaml --quiet \n'} + {'wget $\{ZITADEL_CONFIG_FILES\}/'}{lower}{'/docker-compose.yaml -O docker-compose-'}{lower}{'.yaml --quiet \n'} + {'wget $\{ZITADEL_CONFIG_FILES\}/'}{lower}{'/'}{configfilename}{' -O '}{configfilename}{' --quiet \n'} {'\n'} {genCert} {'# Run the database, ZITADEL and '}{name}{'.'}{'\n'} - {'docker compose --file docker-compose-base.yaml --file docker-compose-'}{lower}{'.yaml up --detach proxy-'}{mode}{'-tls'}{'\n'} + {'docker compose --file docker-compose-base.yaml --file docker-compose-'}{lower}{'.yaml up --detach --wait db zitadel-init zitadel-'}{mode}{'-tls proxy-'}{mode}{'-tls'}{'\n'} {'\n'} {'# Test that gRPC and HTTP APIs work. Empty brackets like {} means success.\n'} - {'sleep 3\n'} + {'# Make sure you have the grpcurl cli installed on your machine https://github.com/fullstorydev/grpcurl?tab=readme-ov-file#installation\n'} {'grpcurl '}{connInsecureFlag}{grpcPlainTextFlag}{'127.0.0.1.sslip.io:'}{connPort}{' zitadel.admin.v1.AdminService/Healthz\n'} {'curl '}{connInsecureFlag}{connScheme}{'://127.0.0.1.sslip.io:'}{connPort}{'/admin/v1/healthz\n'} diff --git a/docs/docs/self-hosting/manage/reverseproxy/caddy/docker-compose.yaml b/docs/docs/self-hosting/manage/reverseproxy/caddy/docker-compose.yaml index aa4b7f6869..c5fad6ab7b 100644 --- a/docs/docs/self-hosting/manage/reverseproxy/caddy/docker-compose.yaml +++ b/docs/docs/self-hosting/manage/reverseproxy/caddy/docker-compose.yaml @@ -1,5 +1,3 @@ -version: '3.8' - services: proxy-disabled-tls: diff --git a/docs/docs/self-hosting/manage/reverseproxy/docker-compose.yaml b/docs/docs/self-hosting/manage/reverseproxy/docker-compose.yaml index d7d929fa44..989b620fef 100644 --- a/docs/docs/self-hosting/manage/reverseproxy/docker-compose.yaml +++ b/docs/docs/self-hosting/manage/reverseproxy/docker-compose.yaml @@ -1,5 +1,3 @@ -version: '3.8' - services: zitadel-disabled-tls: @@ -17,7 +15,7 @@ services: ZITADEL_DATABASE_POSTGRES_USER_USERNAME: zitadel_user ZITADEL_DATABASE_POSTGRES_USER_PASSWORD: zitadel_pw ZITADEL_DATABASE_POSTGRES_USER_SSL_MODE: disable - ZITADEL_DATABASE_POSTGRES_ADMIN_USERNAME: root + ZITADEL_DATABASE_POSTGRES_ADMIN_USERNAME: postgres ZITADEL_DATABASE_POSTGRES_ADMIN_PASSWORD: postgres ZITADEL_DATABASE_POSTGRES_ADMIN_SSL_MODE: disable networks: @@ -43,16 +41,16 @@ services: ZITADEL_DATABASE_POSTGRES_USER_USERNAME: zitadel_user ZITADEL_DATABASE_POSTGRES_USER_PASSWORD: zitadel_pw ZITADEL_DATABASE_POSTGRES_USER_SSL_MODE: disable - ZITADEL_DATABASE_POSTGRES_ADMIN_USERNAME: root + ZITADEL_DATABASE_POSTGRES_ADMIN_USERNAME: postgres ZITADEL_DATABASE_POSTGRES_ADMIN_PASSWORD: postgres ZITADEL_DATABASE_POSTGRES_ADMIN_SSL_MODE: disable networks: - 'zitadel' depends_on: - zitadel-init: - condition: 'service_completed_successfully' db: condition: 'service_healthy' + zitadel-init: + condition: 'service_completed_successfully' zitadel-enabled-tls: extends: @@ -71,7 +69,7 @@ services: ZITADEL_DATABASE_POSTGRES_USER_USERNAME: zitadel_user ZITADEL_DATABASE_POSTGRES_USER_PASSWORD: zitadel_pw ZITADEL_DATABASE_POSTGRES_USER_SSL_MODE: disable - ZITADEL_DATABASE_POSTGRES_ADMIN_USERNAME: root + ZITADEL_DATABASE_POSTGRES_ADMIN_USERNAME: postgres ZITADEL_DATABASE_POSTGRES_ADMIN_PASSWORD: postgres ZITADEL_DATABASE_POSTGRES_ADMIN_SSL_MODE: disable volumes: @@ -109,7 +107,7 @@ services: ZITADEL_DATABASE_POSTGRES_USER_USERNAME: zitadel_user ZITADEL_DATABASE_POSTGRES_USER_PASSWORD: zitadel_pw ZITADEL_DATABASE_POSTGRES_USER_SSL_MODE: disable - ZITADEL_DATABASE_POSTGRES_ADMIN_USERNAME: root + ZITADEL_DATABASE_POSTGRES_ADMIN_USERNAME: postgres ZITADEL_DATABASE_POSTGRES_ADMIN_PASSWORD: postgres ZITADEL_DATABASE_POSTGRES_ADMIN_SSL_MODE: disable networks: @@ -125,10 +123,9 @@ services: restart: 'always' image: postgres:16-alpine environment: - PGUSER: root POSTGRES_PASSWORD: postgres healthcheck: - test: ["CMD-SHELL", "pg_isready", "-d", "zitadel", "-U", "postgres"] + test: ["CMD-SHELL", "pg_isready"] interval: 5s timeout: 60s retries: 10 diff --git a/docs/docs/self-hosting/manage/reverseproxy/httpd/docker-compose.yaml b/docs/docs/self-hosting/manage/reverseproxy/httpd/docker-compose.yaml index 72e06b976f..8757758dc3 100644 --- a/docs/docs/self-hosting/manage/reverseproxy/httpd/docker-compose.yaml +++ b/docs/docs/self-hosting/manage/reverseproxy/httpd/docker-compose.yaml @@ -1,5 +1,3 @@ -version: '3.8' - services: proxy-disabled-tls: diff --git a/docs/docs/self-hosting/manage/reverseproxy/nginx/docker-compose.yaml b/docs/docs/self-hosting/manage/reverseproxy/nginx/docker-compose.yaml index 21b3361979..524d50fc30 100644 --- a/docs/docs/self-hosting/manage/reverseproxy/nginx/docker-compose.yaml +++ b/docs/docs/self-hosting/manage/reverseproxy/nginx/docker-compose.yaml @@ -1,5 +1,3 @@ -version: '3.8' - services: proxy-disabled-tls: diff --git a/docs/docs/self-hosting/manage/reverseproxy/traefik/docker-compose.yaml b/docs/docs/self-hosting/manage/reverseproxy/traefik/docker-compose.yaml index aee5cf891d..a2dfab075b 100644 --- a/docs/docs/self-hosting/manage/reverseproxy/traefik/docker-compose.yaml +++ b/docs/docs/self-hosting/manage/reverseproxy/traefik/docker-compose.yaml @@ -1,5 +1,3 @@ -version: '3.8' - services: proxy-disabled-tls: diff --git a/e2e/config/host.docker.internal/docker-compose.yaml b/e2e/config/host.docker.internal/docker-compose.yaml index 8c9d755b02..80ea33b364 100644 --- a/e2e/config/host.docker.internal/docker-compose.yaml +++ b/e2e/config/host.docker.internal/docker-compose.yaml @@ -1,5 +1,3 @@ -version: '3.8' - services: db: diff --git a/e2e/config/localhost/docker-compose.yaml b/e2e/config/localhost/docker-compose.yaml index a14c0dd603..040cbc81c0 100644 --- a/e2e/config/localhost/docker-compose.yaml +++ b/e2e/config/localhost/docker-compose.yaml @@ -1,5 +1,3 @@ -version: '3.8' - services: zitadel: user: '$UID' diff --git a/e2e/docker-compose.yaml b/e2e/docker-compose.yaml index ffcfb65c4d..f03b1fcc46 100644 --- a/e2e/docker-compose.yaml +++ b/e2e/docker-compose.yaml @@ -1,5 +1,3 @@ -version: '3.8' - services: zitadel: extends: From 79af682c9b14bc66059d03f2859432c569626b91 Mon Sep 17 00:00:00 2001 From: Fabi Date: Mon, 6 Jan 2025 10:03:29 +0100 Subject: [PATCH 08/21] fix: Typo in Init MFA OTP screen (#9128) # Which Problems Are Solved Type in the word Microsoft on the init mfa otp screen # How the Problems Are Solved Fix typo --- internal/api/ui/login/static/i18n/de.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/ui/login/static/i18n/de.yaml b/internal/api/ui/login/static/i18n/de.yaml index edbeb652ce..28f4d00a88 100644 --- a/internal/api/ui/login/static/i18n/de.yaml +++ b/internal/api/ui/login/static/i18n/de.yaml @@ -108,7 +108,7 @@ InitMFAPrompt: InitMFAOTP: Title: Zwei-Faktor-Authentifizierung Description: Erstelle deinen Zweitfaktor. Installiere eine Authentifizierungs-App, wenn du noch keine hast. - OTPDescription: Scanne den Code mit einer Authentifizierungs-App (z.B. Google/Mircorsoft Authenticator, Authy) oder kopiere das Secret und gib anschliessend den Code ein. + OTPDescription: Scanne den Code mit einer Authentifizierungs-App (z.B. Google/Microsoft Authenticator, Authy) oder kopiere das Secret und gib anschliessend den Code ein. SecretLabel: Secret CodeLabel: Code NextButtonText: Weiter From fa5e590aabda38bd346f1a41484466aebdd8f903 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Mon, 6 Jan 2025 10:47:46 +0100 Subject: [PATCH 09/21] fix(idp): prevent server errors for idps using form post for callbacks (#9097) # Which Problems Are Solved Some IdP callbacks use HTTP form POST to return their data on callbacks. For handling CSRF in the login after such calls, a 302 Found to the corresponding non form callback (in ZITADEL) is sent. Depending on the size of the initial form body, this could lead to ZITADEL terminating the connection, resulting in the user not getting a response or an intermediate proxy to return them an HTTP 502. # How the Problems Are Solved - the form body is parsed and stored into the ZITADEL cache (using the configured database by default) - the redirect (302 Found) is performed with the request id - the callback retrieves the data from the cache instead of the query parameters (will fallback to latter to handle open uncached requests) # Additional Changes - fixed a typo in the default (cache) configuration: `LastUsage` -> `LastUseAge` # Additional Context - reported by a customer - needs to be backported to current cloud version (2.66.x) --------- Co-authored-by: Silvan <27845747+adlerhurst@users.noreply.github.com> --- cmd/defaults.yaml | 21 ++++++-- cmd/start/start.go | 3 ++ .../api/ui/login/external_provider_handler.go | 30 ++++++++++-- internal/api/ui/login/login.go | 49 +++++++++++++++++++ internal/cache/cache.go | 1 + internal/cache/connector/connector.go | 7 +-- internal/cache/purpose_enumer.go | 12 +++-- 7 files changed, 108 insertions(+), 15 deletions(-) diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index 1e5de1eea1..e993657123 100644 --- a/cmd/defaults.yaml +++ b/cmd/defaults.yaml @@ -198,8 +198,11 @@ Caches: AutoPrune: Interval: 1m TimeOut: 5s + # Postgres connector uses the configured database (postgres or cockraochdb) as cache. + # It is suitable for deployments with multiple containers. + # The cache is enabled by default because it is the default cache states for IdP form callbacks Postgres: - Enabled: false + Enabled: true AutoPrune: Interval: 15m TimeOut: 30s @@ -311,7 +314,7 @@ Caches: # When connector is empty, this cache will be disabled. Connector: "" MaxAge: 1h - LastUsage: 10m + LastUseAge: 10m # Log enables cache-specific logging. Default to error log to stderr when omitted. Log: Level: error @@ -322,7 +325,7 @@ Caches: Milestones: Connector: "" MaxAge: 1h - LastUsage: 10m + LastUseAge: 10m Log: Level: error AddSource: true @@ -332,7 +335,17 @@ Caches: Organization: Connector: "" MaxAge: 1h - LastUsage: 10m + LastUseAge: 10m + Log: + Level: error + AddSource: true + Formatter: + Format: text + # IdP callbacks using form POST cache, required for handling them securely and without possible too big request urls. + IdPFormCallbacks: + Connector: "postgres" + MaxAge: 1h + LastUseAge: 10m Log: Level: error AddSource: true diff --git a/cmd/start/start.go b/cmd/start/start.go index 72ab9ea862..154c683481 100644 --- a/cmd/start/start.go +++ b/cmd/start/start.go @@ -317,6 +317,7 @@ func startZitadel(ctx context.Context, config *Config, masterKey string, server authZRepo, keys, permissionCheck, + cacheConnectors, ) if err != nil { return err @@ -361,6 +362,7 @@ func startAPIs( authZRepo authz_repo.Repository, keys *encryption.EncryptionKeys, permissionCheck domain.PermissionCheck, + cacheConnectors connector.Connectors, ) (*api.API, error) { repo := struct { authz_repo.Repository @@ -542,6 +544,7 @@ func startAPIs( keys.User, keys.IDPConfig, keys.CSRFCookieKey, + cacheConnectors, ) if err != nil { return nil, fmt.Errorf("unable to start login: %w", err) diff --git a/internal/api/ui/login/external_provider_handler.go b/internal/api/ui/login/external_provider_handler.go index 15046d25e8..6b312317be 100644 --- a/internal/api/ui/login/external_provider_handler.go +++ b/internal/api/ui/login/external_provider_handler.go @@ -214,8 +214,20 @@ func (l *Login) handleExternalLoginCallbackForm(w http.ResponseWriter, r *http.R l.renderLogin(w, r, nil, err) return } - r.Form.Add("Method", http.MethodPost) - http.Redirect(w, r, HandlerPrefix+EndpointExternalLoginCallback+"?"+r.Form.Encode(), 302) + state := r.Form.Get("state") + if state == "" { + state = r.Form.Get("RelayState") + } + if state == "" { + l.renderLogin(w, r, nil, zerrors.ThrowInvalidArgument(nil, "LOGIN-dsg3f", "Errors.AuthRequest.NotFound")) + return + } + l.caches.idpFormCallbacks.Set(r.Context(), &idpFormCallback{ + InstanceID: authz.GetInstance(r.Context()).InstanceID(), + State: state, + Form: r.Form, + }) + http.Redirect(w, r, HandlerPrefix+EndpointExternalLoginCallback+"?method=POST&state="+state, 302) } // handleExternalLoginCallback handles the callback from a IDP @@ -232,8 +244,7 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque } // workaround because of CSRF on external identity provider flows if data.Method == http.MethodPost { - r.Method = http.MethodPost - r.PostForm = r.Form + l.setDataFromFormCallback(r, data.State) } userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) @@ -345,6 +356,17 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque l.handleExternalUserAuthenticated(w, r, authReq, identityProvider, session, user, l.renderNextStep) } +func (l *Login) setDataFromFormCallback(r *http.Request, state string) { + r.Method = http.MethodPost + // fallback to the form data in case the request was started before the cache was implemented + r.PostForm = r.Form + idpCallback, ok := l.caches.idpFormCallbacks.Get(r.Context(), idpFormCallbackIndexRequestID, + idpFormCallbackKey(authz.GetInstance(r.Context()).InstanceID(), state)) + if ok { + r.PostForm = idpCallback.Form + } +} + func (l *Login) tryMigrateExternalUserID(r *http.Request, session idp.Session, authReq *domain.AuthRequest, externalUser *domain.ExternalUser) (previousIDMatched bool, err error) { migration, ok := session.(idp.SessionSupportsMigration) if !ok { diff --git a/internal/api/ui/login/login.go b/internal/api/ui/login/login.go index 57f6a5f9a3..444c5aaa85 100644 --- a/internal/api/ui/login/login.go +++ b/internal/api/ui/login/login.go @@ -3,6 +3,7 @@ package login import ( "context" "net/http" + "net/url" "strings" "time" @@ -15,6 +16,8 @@ import ( _ "github.com/zitadel/zitadel/internal/api/ui/login/statik" auth_repository "github.com/zitadel/zitadel/internal/auth/repository" "github.com/zitadel/zitadel/internal/auth/repository/eventsourcing" + "github.com/zitadel/zitadel/internal/cache" + "github.com/zitadel/zitadel/internal/cache/connector" "github.com/zitadel/zitadel/internal/command" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" @@ -38,6 +41,7 @@ type Login struct { samlAuthCallbackURL func(context.Context, string) string idpConfigAlg crypto.EncryptionAlgorithm userCodeAlg crypto.EncryptionAlgorithm + caches *Caches } type Config struct { @@ -74,6 +78,7 @@ func CreateLogin(config Config, userCodeAlg crypto.EncryptionAlgorithm, idpConfigAlg crypto.EncryptionAlgorithm, csrfCookieKey []byte, + cacheConnectors connector.Connectors, ) (*Login, error) { login := &Login{ oidcAuthCallbackURL: oidcAuthCallbackURL, @@ -94,6 +99,12 @@ func CreateLogin(config Config, login.router = CreateRouter(login, middleware.TelemetryHandler(IgnoreInstanceEndpoints...), oidcInstanceHandler, samlInstanceHandler, csrfInterceptor, cacheInterceptor, security, userAgentCookie, issuerInterceptor, accessHandler) login.renderer = CreateRenderer(HandlerPrefix, staticStorage, config.LanguageCookieName) login.parser = form.NewParser() + + var err error + login.caches, err = startCaches(context.Background(), cacheConnectors) + if err != nil { + return nil, err + } return login, nil } @@ -201,3 +212,41 @@ func setUserContext(ctx context.Context, userID, resourceOwner string) context.C func (l *Login) baseURL(ctx context.Context) string { return http_utils.DomainContext(ctx).Origin() + HandlerPrefix } + +type Caches struct { + idpFormCallbacks cache.Cache[idpFormCallbackIndex, string, *idpFormCallback] +} + +func startCaches(background context.Context, connectors connector.Connectors) (_ *Caches, err error) { + caches := new(Caches) + caches.idpFormCallbacks, err = connector.StartCache[idpFormCallbackIndex, string, *idpFormCallback](background, []idpFormCallbackIndex{idpFormCallbackIndexRequestID}, cache.PurposeIdPFormCallback, connectors.Config.IdPFormCallbacks, connectors) + if err != nil { + return nil, err + } + return caches, nil +} + +type idpFormCallbackIndex int + +const ( + idpFormCallbackIndexUnspecified idpFormCallbackIndex = iota + idpFormCallbackIndexRequestID +) + +type idpFormCallback struct { + InstanceID string + State string + Form url.Values +} + +// Keys implements cache.Entry +func (c *idpFormCallback) Keys(i idpFormCallbackIndex) []string { + if i == idpFormCallbackIndexRequestID { + return []string{idpFormCallbackKey(c.InstanceID, c.State)} + } + return nil +} + +func idpFormCallbackKey(instanceID, state string) string { + return instanceID + "-" + state +} diff --git a/internal/cache/cache.go b/internal/cache/cache.go index c7dbad6f2c..dc05208caa 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -17,6 +17,7 @@ const ( PurposeAuthzInstance PurposeMilestones PurposeOrganization + PurposeIdPFormCallback ) // Cache stores objects with a value of type `V`. diff --git a/internal/cache/connector/connector.go b/internal/cache/connector/connector.go index 09298fa688..1a0534759a 100644 --- a/internal/cache/connector/connector.go +++ b/internal/cache/connector/connector.go @@ -19,9 +19,10 @@ type CachesConfig struct { Postgres pg.Config Redis redis.Config } - Instance *cache.Config - Milestones *cache.Config - Organization *cache.Config + Instance *cache.Config + Milestones *cache.Config + Organization *cache.Config + IdPFormCallbacks *cache.Config } type Connectors struct { diff --git a/internal/cache/purpose_enumer.go b/internal/cache/purpose_enumer.go index 47ad167d70..a93a978efb 100644 --- a/internal/cache/purpose_enumer.go +++ b/internal/cache/purpose_enumer.go @@ -7,11 +7,11 @@ import ( "strings" ) -const _PurposeName = "unspecifiedauthz_instancemilestonesorganization" +const _PurposeName = "unspecifiedauthz_instancemilestonesorganizationid_p_form_callback" -var _PurposeIndex = [...]uint8{0, 11, 25, 35, 47} +var _PurposeIndex = [...]uint8{0, 11, 25, 35, 47, 65} -const _PurposeLowerName = "unspecifiedauthz_instancemilestonesorganization" +const _PurposeLowerName = "unspecifiedauthz_instancemilestonesorganizationid_p_form_callback" func (i Purpose) String() string { if i < 0 || i >= Purpose(len(_PurposeIndex)-1) { @@ -28,9 +28,10 @@ func _PurposeNoOp() { _ = x[PurposeAuthzInstance-(1)] _ = x[PurposeMilestones-(2)] _ = x[PurposeOrganization-(3)] + _ = x[PurposeIdPFormCallback-(4)] } -var _PurposeValues = []Purpose{PurposeUnspecified, PurposeAuthzInstance, PurposeMilestones, PurposeOrganization} +var _PurposeValues = []Purpose{PurposeUnspecified, PurposeAuthzInstance, PurposeMilestones, PurposeOrganization, PurposeIdPFormCallback} var _PurposeNameToValueMap = map[string]Purpose{ _PurposeName[0:11]: PurposeUnspecified, @@ -41,6 +42,8 @@ var _PurposeNameToValueMap = map[string]Purpose{ _PurposeLowerName[25:35]: PurposeMilestones, _PurposeName[35:47]: PurposeOrganization, _PurposeLowerName[35:47]: PurposeOrganization, + _PurposeName[47:65]: PurposeIdPFormCallback, + _PurposeLowerName[47:65]: PurposeIdPFormCallback, } var _PurposeNames = []string{ @@ -48,6 +51,7 @@ var _PurposeNames = []string{ _PurposeName[11:25], _PurposeName[25:35], _PurposeName[35:47], + _PurposeName[47:65], } // PurposeString retrieves an enum value from the enum constants string name. From 8d7a1efd4a6f7ab66356c2bc708e51ac451db4a8 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Mon, 6 Jan 2025 14:48:32 +0100 Subject: [PATCH 10/21] fix(idp): correctly get data from cache before parsing (#9134) # Which Problems Are Solved IdPs using form callback were not always correctly handled with the newly introduced cache mechanism (https://github.com/zitadel/zitadel/pull/9097). # How the Problems Are Solved Get the data from cache before parsing it. # Additional Changes None # Additional Context Relates to https://github.com/zitadel/zitadel/pull/9097 --- .../api/ui/login/external_provider_handler.go | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/internal/api/ui/login/external_provider_handler.go b/internal/api/ui/login/external_provider_handler.go index 6b312317be..c60e0eb0bb 100644 --- a/internal/api/ui/login/external_provider_handler.go +++ b/internal/api/ui/login/external_provider_handler.go @@ -3,6 +3,7 @@ package login import ( "context" "net/http" + "net/url" "strings" "github.com/crewjam/saml/samlsp" @@ -36,6 +37,9 @@ import ( const ( queryIDPConfigID = "idpConfigID" + queryState = "state" + queryRelayState = "RelayState" + queryMethod = "method" tmplExternalNotFoundOption = "externalnotfoundoption" ) @@ -214,9 +218,9 @@ func (l *Login) handleExternalLoginCallbackForm(w http.ResponseWriter, r *http.R l.renderLogin(w, r, nil, err) return } - state := r.Form.Get("state") + state := r.Form.Get(queryState) if state == "" { - state = r.Form.Get("RelayState") + state = r.Form.Get(queryRelayState) } if state == "" { l.renderLogin(w, r, nil, zerrors.ThrowInvalidArgument(nil, "LOGIN-dsg3f", "Errors.AuthRequest.NotFound")) @@ -227,12 +231,23 @@ func (l *Login) handleExternalLoginCallbackForm(w http.ResponseWriter, r *http.R State: state, Form: r.Form, }) - http.Redirect(w, r, HandlerPrefix+EndpointExternalLoginCallback+"?method=POST&state="+state, 302) + v := url.Values{} + v.Set(queryMethod, http.MethodPost) + v.Set(queryState, state) + http.Redirect(w, r, HandlerPrefix+EndpointExternalLoginCallback+"?"+v.Encode(), 302) } // handleExternalLoginCallback handles the callback from a IDP // and tries to extract the user with the provided data func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Request) { + // workaround because of CSRF on external identity provider flows using form_post + if r.URL.Query().Get(queryMethod) == http.MethodPost { + if err := l.setDataFromFormCallback(r, r.URL.Query().Get(queryState)); err != nil { + l.renderLogin(w, r, nil, err) + return + } + } + data := new(externalIDPCallbackData) err := l.getParseData(r, data) if err != nil { @@ -242,10 +257,6 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque if data.State == "" { data.State = data.RelayState } - // workaround because of CSRF on external identity provider flows - if data.Method == http.MethodPost { - l.setDataFromFormCallback(r, data.State) - } userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) authReq, err := l.authRepo.AuthRequestByID(r.Context(), data.State, userAgentID) @@ -356,15 +367,23 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque l.handleExternalUserAuthenticated(w, r, authReq, identityProvider, session, user, l.renderNextStep) } -func (l *Login) setDataFromFormCallback(r *http.Request, state string) { +func (l *Login) setDataFromFormCallback(r *http.Request, state string) error { r.Method = http.MethodPost + err := r.ParseForm() + if err != nil { + return err + } // fallback to the form data in case the request was started before the cache was implemented r.PostForm = r.Form idpCallback, ok := l.caches.idpFormCallbacks.Get(r.Context(), idpFormCallbackIndexRequestID, idpFormCallbackKey(authz.GetInstance(r.Context()).InstanceID(), state)) if ok { r.PostForm = idpCallback.Form + // We need to set the form as well to make sure the data is parsed correctly. + // Form precedes PostForm in the parsing order. + r.Form = idpCallback.Form } + return nil } func (l *Login) tryMigrateExternalUserID(r *http.Request, session idp.Session, authReq *domain.AuthRequest, externalUser *domain.ExternalUser) (previousIDMatched bool, err error) { From c687d6769bb922a7c0a4299b64b1dfdaf7cd04c4 Mon Sep 17 00:00:00 2001 From: Oleg Lavrovsky <31819+loleg@users.noreply.github.com> Date: Tue, 7 Jan 2025 00:21:09 +0100 Subject: [PATCH 11/21] docs(adopters):Dribdat (#9021) Added a note on Zitadel support in Dribdat, which explicitly mentions it in the [install notes](https://dribdat.cc/deploy.html#authentication) and soon in a blog post or screencast. --------- Co-authored-by: Swarna Podila --- ADOPTERS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ADOPTERS.md b/ADOPTERS.md index da37b9ffb0..0573099cf9 100644 --- a/ADOPTERS.md +++ b/ADOPTERS.md @@ -16,6 +16,7 @@ If you are using Zitadel, please consider adding yourself as a user with a quick | devOS: Sanity Edition | [@devOS-Sanity-Edition](https://github.com/devOS-Sanity-Edition) | Uses SSO Auth for every piece of our internal and external infrastructure | | CNAP.tech | [@cnap-tech](https://github.com/cnap-tech) | Using Zitadel for authentication and authorization in cloud-native applications | | Minekube | [@minekube](https://github.com/minekube) | Leveraging Zitadel for secure user authentication in gaming infrastructure | +| Dribdat | [@dribdat](https://github.com/dribdat) | Educating people about strong auth and resilient identity at hackathons | | Micromate | [@sschoeb](https://github.com/sschoeb) | Using Zitadel for authentication and authorization for learners and managers in our digital learning assistant as well as in the Micromate manage platform | | Smat.io | [@smatio](https://github.com/smatio) - [@lukasver](https://github.com/lukasver) | Zitadel for authentication in cloud applications while offering B2B portfolio management solutions for professional investors | |hirschengraben | [hirschengraben.io](hirschengraben.io) | Using Zitadel as IDP for a multitenant B2B dispatch app for bike messengers | From a54bb2977b8e8fd0a8a18cfb9e22fadaad3ab00e Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Tue, 7 Jan 2025 10:12:39 +0100 Subject: [PATCH 12/21] docs: change scope for zitadel audience (#9117) # Which Problems Are Solved - This replaces the old aud claim from Zitadel in two places. # Additional Context - Relates to [this discord thread](https://discord.com/channels/927474939156643850/1305853084743766067) --- .../zitadel-apis/example-zitadel-api-with-dot-net.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/docs/guides/integrate/zitadel-apis/example-zitadel-api-with-dot-net.md b/docs/docs/guides/integrate/zitadel-apis/example-zitadel-api-with-dot-net.md index fe1b5a2f2c..7a012f799e 100644 --- a/docs/docs/guides/integrate/zitadel-apis/example-zitadel-api-with-dot-net.md +++ b/docs/docs/guides/integrate/zitadel-apis/example-zitadel-api-with-dot-net.md @@ -43,11 +43,10 @@ dotnet add package Zitadel.Api ### Create example client Change the program.cs file to the content below. This will create a client for the management api and call its `GetMyUsers` function. -The SDK will make sure you will have access to the API by retrieving a Bearer Token using JWT Profile with the provided scopes (`openid` and `urn:zitadel:iam:org:project:id:{projectID}:aud`). +The SDK will make sure you will have access to the API by retrieving a Bearer Token using JWT Profile with the provided scopes (`openid` and `urn:zitadel:iam:org:project:id:zitadel:aud`). -Make sure to fill the const `apiUrl`, `apiProject` and `personalAccessToken` with your own instance data. The used vars below are from a test instance, to show you how it should look. +Make sure to fill the const `apiUrl`, and `personalAccessToken` with your own instance data. The used vars below are from a test instance, to show you how it should look. The apiURL is the domain of your instance you can find it on the instance detail in the Customer Portal or in the Console -The apiProject you will find in the ZITADEL project in the first organization of your instance. ```csharp // This file contains two examples: @@ -66,7 +65,8 @@ var client = Clients.AuthService(new(apiUrl, ITokenProvider.Static(personalAcces var result = await client.GetMyUserAsync(new()); Console.WriteLine($"User: {result.User}"); -const string apiProject = "170078979166961921"; +// This adds the urn:zitadel:iam:org:project:id:zitadel:aud scope to the authorization request, enabling access to ZITADEL APIs. +const string apiProject = "zitadel"; var serviceAccount = ServiceAccount.LoadFromJsonString( @" { From 56427cca50cc990bd168064d55559a0af9be4cdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Tue, 7 Jan 2025 13:51:06 +0200 Subject: [PATCH 13/21] fix(cache): convert expiry to number (#9143) # Which Problems Are Solved When `LastUseAge` was configured properly, the Redis LUA script uses manual cleanup for `MaxAge` based expiry. The expiry obtained from Redis apears to be a string and was compared to an int, resulting in a script error. # How the Problems Are Solved Convert expiry to number. # Additional Changes - none # Additional Context - Introduced in #8822 - LastUseAge was fixed in #9097 - closes https://github.com/zitadel/zitadel/issues/9140 --- internal/cache/connector/redis/get.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/cache/connector/redis/get.lua b/internal/cache/connector/redis/get.lua index cfb3e89d8a..b542ff29d1 100644 --- a/internal/cache/connector/redis/get.lua +++ b/internal/cache/connector/redis/get.lua @@ -13,8 +13,8 @@ end -- max-age must be checked manually local expiry = getCall("HGET", object_id, "expiry") -if not (expiry == nil) and expiry > 0 then - if getTime() > expiry then +if not (expiry == nil) and tonumber(expiry) > 0 then + if getTime() > tonumber(expiry) then remove(object_id) return nil end From 11d36fcd0076316ec0f14da3c769235b3ed4c5fe Mon Sep 17 00:00:00 2001 From: Elio Bischof Date: Tue, 7 Jan 2025 15:38:13 +0100 Subject: [PATCH 14/21] feat(console): allow to configure PostHog (#9135) # Which Problems Are Solved The console has no information about where and how to send PostHog events. # How the Problems Are Solved A PostHog API URL and token are passed through as plain text from the Zitadel runtime config to the environment.json. By default, no values are configured and the keys in the environment.json are omitted. # Additional Context - Closes https://github.com/zitadel/zitadel/issues/9070 - Complements https://github.com/zitadel/zitadel/pull/9077 --- cmd/defaults.yaml | 3 +++ internal/api/ui/console/console.go | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index e993657123..868ee06866 100644 --- a/cmd/defaults.yaml +++ b/cmd/defaults.yaml @@ -611,6 +611,9 @@ Console: # 168h is 7 days, one week SharedMaxAge: 168h # ZITADEL_CONSOLE_LONGCACHE_SHAREDMAXAGE InstanceManagementURL: "" # ZITADEL_CONSOLE_INSTANCEMANAGEMENTURL + PostHog: + URL: "" # ZITADEL_CONSOLE_POSTHOG_URL + Token: "" # ZITADEL_CONSOLE_POSTHOG_TOKEN EncryptionKeys: DomainVerification: diff --git a/internal/api/ui/console/console.go b/internal/api/ui/console/console.go index 515f26db9b..fffbc00d5b 100644 --- a/internal/api/ui/console/console.go +++ b/internal/api/ui/console/console.go @@ -28,6 +28,10 @@ type Config struct { ShortCache middleware.CacheConfig LongCache middleware.CacheConfig InstanceManagementURL string + PostHog struct { + Token string + URL string + } } type spaHandler struct { @@ -117,7 +121,7 @@ func Start(config Config, externalSecure bool, issuer op.IssuerFromRequest, call return } limited := limitingAccessInterceptor.Limit(w, r) - environmentJSON, err := createEnvironmentJSON(url, issuer(r), instance.ConsoleClientID(), customerPortal, instanceMgmtURL, limited) + environmentJSON, err := createEnvironmentJSON(url, issuer(r), instance.ConsoleClientID(), customerPortal, instanceMgmtURL, config.PostHog.URL, config.PostHog.Token, limited) if err != nil { http.Error(w, fmt.Sprintf("unable to marshal env for console: %v", err), http.StatusInternalServerError) return @@ -150,13 +154,15 @@ func csp() *middleware.CSP { return &csp } -func createEnvironmentJSON(api, issuer, clientID, customerPortal, instanceMgmtUrl string, exhausted bool) ([]byte, error) { +func createEnvironmentJSON(api, issuer, clientID, customerPortal, instanceMgmtUrl, postHogURL, postHogToken string, exhausted bool) ([]byte, error) { environment := struct { API string `json:"api,omitempty"` Issuer string `json:"issuer,omitempty"` ClientID string `json:"clientid,omitempty"` CustomerPortal string `json:"customer_portal,omitempty"` InstanceManagementURL string `json:"instance_management_url,omitempty"` + PostHogURL string `json:"posthog_url,omitempty"` + PostHogToken string `json:"posthog_token,omitempty"` Exhausted bool `json:"exhausted,omitempty"` }{ API: api, @@ -164,6 +170,8 @@ func createEnvironmentJSON(api, issuer, clientID, customerPortal, instanceMgmtUr ClientID: clientID, CustomerPortal: customerPortal, InstanceManagementURL: instanceMgmtUrl, + PostHogURL: postHogURL, + PostHogToken: postHogToken, Exhausted: exhausted, } return json.Marshal(environment) From f320d18b1a24b97b67500d471c11068fd09b6c39 Mon Sep 17 00:00:00 2001 From: Silvan <27845747+adlerhurst@users.noreply.github.com> Date: Tue, 7 Jan 2025 17:06:33 +0100 Subject: [PATCH 15/21] perf(fields): create index for instance domain query (#9146) # Which Problems Are Solved get instance by domain cannot provide an instance id because it is not known at that time. This causes a full table scan on the fields table because current indexes always include the `instance_id` column. # How the Problems Are Solved Added a specific index for this query. # Additional Context If a system has many fields and there is no cache hit for the given domain this query can heaviuly influence database CPU usage, the newly added resolves this problem. --- cmd/setup/43.go | 40 +++++++++++++++++++++++++++++++++++ cmd/setup/43/cockroach/43.sql | 3 +++ cmd/setup/43/postgres/43.sql | 3 +++ cmd/setup/config.go | 1 + cmd/setup/setup.go | 2 ++ 5 files changed, 49 insertions(+) create mode 100644 cmd/setup/43.go create mode 100644 cmd/setup/43/cockroach/43.sql create mode 100644 cmd/setup/43/postgres/43.sql diff --git a/cmd/setup/43.go b/cmd/setup/43.go new file mode 100644 index 0000000000..844c25cf24 --- /dev/null +++ b/cmd/setup/43.go @@ -0,0 +1,40 @@ +package setup + +import ( + "context" + "embed" + "fmt" + + "github.com/zitadel/logging" + + "github.com/zitadel/zitadel/internal/database" + "github.com/zitadel/zitadel/internal/eventstore" +) + +var ( + //go:embed 43/cockroach/*.sql + //go:embed 43/postgres/*.sql + createFieldsDomainIndex embed.FS +) + +type CreateFieldsDomainIndex struct { + dbClient *database.DB +} + +func (mig *CreateFieldsDomainIndex) Execute(ctx context.Context, _ eventstore.Event) error { + statements, err := readStatements(createFieldsDomainIndex, "43", mig.dbClient.Type()) + if err != nil { + return err + } + for _, stmt := range statements { + logging.WithFields("file", stmt.file, "migration", mig.String()).Info("execute statement") + if _, err := mig.dbClient.ExecContext(ctx, stmt.query); err != nil { + return fmt.Errorf("%s %s: %w", mig.String(), stmt.file, err) + } + } + return nil +} + +func (mig *CreateFieldsDomainIndex) String() string { + return "43_create_fields_domain_index" +} diff --git a/cmd/setup/43/cockroach/43.sql b/cmd/setup/43/cockroach/43.sql new file mode 100644 index 0000000000..9152130970 --- /dev/null +++ b/cmd/setup/43/cockroach/43.sql @@ -0,0 +1,3 @@ +CREATE INDEX CONCURRENTLY IF NOT EXISTS fields_instance_domains_idx +ON eventstore.fields (object_id) +WHERE object_type = 'instance_domain' AND field_name = 'domain'; \ No newline at end of file diff --git a/cmd/setup/43/postgres/43.sql b/cmd/setup/43/postgres/43.sql new file mode 100644 index 0000000000..2f6f958fdf --- /dev/null +++ b/cmd/setup/43/postgres/43.sql @@ -0,0 +1,3 @@ +CREATE INDEX CONCURRENTLY IF NOT EXISTS fields_instance_domains_idx +ON eventstore.fields (object_id) INCLUDE (instance_id) +WHERE object_type = 'instance_domain' AND field_name = 'domain'; \ No newline at end of file diff --git a/cmd/setup/config.go b/cmd/setup/config.go index ae62728c95..407a9412bb 100644 --- a/cmd/setup/config.go +++ b/cmd/setup/config.go @@ -128,6 +128,7 @@ type Steps struct { s38BackChannelLogoutNotificationStart *BackChannelLogoutNotificationStart s40InitPushFunc *InitPushFunc s42Apps7OIDCConfigsLoginVersion *Apps7OIDCConfigsLoginVersion + s43CreateFieldsDomainIndex *CreateFieldsDomainIndex } func MustNewSteps(v *viper.Viper) *Steps { diff --git a/cmd/setup/setup.go b/cmd/setup/setup.go index 497457ba8f..c803ab55b6 100644 --- a/cmd/setup/setup.go +++ b/cmd/setup/setup.go @@ -171,6 +171,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string) steps.s38BackChannelLogoutNotificationStart = &BackChannelLogoutNotificationStart{dbClient: esPusherDBClient, esClient: eventstoreClient} steps.s40InitPushFunc = &InitPushFunc{dbClient: esPusherDBClient} steps.s42Apps7OIDCConfigsLoginVersion = &Apps7OIDCConfigsLoginVersion{dbClient: esPusherDBClient} + steps.s43CreateFieldsDomainIndex = &CreateFieldsDomainIndex{dbClient: queryDBClient} err = projection.Create(ctx, projectionDBClient, eventstoreClient, config.Projections, nil, nil, nil) logging.OnError(err).Fatal("unable to start projections") @@ -242,6 +243,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string) steps.s33SMSConfigs3TwilioAddVerifyServiceSid, steps.s37Apps7OIDConfigsBackChannelLogoutURI, steps.s42Apps7OIDCConfigsLoginVersion, + steps.s43CreateFieldsDomainIndex, } { mustExecuteMigration(ctx, eventstoreClient, step, "migration failed") } From 8d8f38fb4ca6e767f266ceffa07244c9c55448c2 Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Tue, 7 Jan 2025 17:34:59 +0100 Subject: [PATCH 16/21] fix: only allowed idps in login step (#9136) # Which Problems Are Solved If a not allowed IDP is selected or now not allowed IDP was selected before at login, the login will still try to use it as fallback. The same goes for the linked IDPs which are not necessarily active anymore, or disallowed through policies. # How the Problems Are Solved Check all possible or configured IDPs if they can be used. # Additional Changes None # Additional Context Addition to #6466 --------- Co-authored-by: Livio Spring --- .../eventsourcing/eventstore/auth_request.go | 41 +++- .../eventstore/auth_request_test.go | 226 +++++++++++++++++- 2 files changed, 259 insertions(+), 8 deletions(-) diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index e35e7b5143..813c5668f4 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -1065,8 +1065,10 @@ func (repo *AuthRequestRepo) nextSteps(ctx context.Context, request *domain.Auth return nil, err } noLocalAuth := request.LoginPolicy != nil && !request.LoginPolicy.AllowUsernamePassword - if (!isInternalLogin || len(idps.Links) > 0 || noLocalAuth) && len(request.LinkingUsers) == 0 { - step, err := repo.idpChecked(request, idps.Links, userSession) + + allowedLinkedIDPs := checkForAllowedIDPs(request.AllowedExternalIDPs, idps.Links) + if (!isInternalLogin || len(allowedLinkedIDPs) > 0 || noLocalAuth) && len(request.LinkingUsers) == 0 { + step, err := repo.idpChecked(request, allowedLinkedIDPs, userSession) if err != nil { return nil, err } @@ -1146,6 +1148,19 @@ func (repo *AuthRequestRepo) nextSteps(ctx context.Context, request *domain.Auth return append(steps, &domain.RedirectToCallbackStep{}), nil } +func checkForAllowedIDPs(allowedIDPs []*domain.IDPProvider, idps []*query.IDPUserLink) (_ []string) { + allowedLinkedIDPs := make([]string, 0, len(idps)) + // only use allowed linked idps + for _, idp := range idps { + for _, allowedIdP := range allowedIDPs { + if idp.IDPID == allowedIdP.IDPConfigID { + allowedLinkedIDPs = append(allowedLinkedIDPs, allowedIdP.IDPConfigID) + } + } + } + return allowedLinkedIDPs +} + func passwordAgeChangeRequired(policy *domain.PasswordAgePolicy, changed time.Time) bool { if policy == nil || policy.MaxAgeDays == 0 { return false @@ -1299,7 +1314,7 @@ func (repo *AuthRequestRepo) firstFactorChecked(ctx context.Context, request *do return &domain.PasswordStep{} } -func (repo *AuthRequestRepo) idpChecked(request *domain.AuthRequest, idps []*query.IDPUserLink, userSession *user_model.UserSessionView) (domain.NextStep, error) { +func (repo *AuthRequestRepo) idpChecked(request *domain.AuthRequest, idps []string, userSession *user_model.UserSessionView) (domain.NextStep, error) { if checkVerificationTimeMaxAge(userSession.ExternalLoginVerification, request.LoginPolicy.ExternalLoginCheckLifetime, request) { request.IDPLoginChecked = true request.AuthTime = userSession.ExternalLoginVerification @@ -1307,15 +1322,27 @@ func (repo *AuthRequestRepo) idpChecked(request *domain.AuthRequest, idps []*que } // use the explicitly set IdP first if request.SelectedIDPConfigID != "" { - return &domain.ExternalLoginStep{SelectedIDPConfigID: request.SelectedIDPConfigID}, nil + // only use the explicitly set IdP if allowed + for _, allowedIdP := range request.AllowedExternalIDPs { + if request.SelectedIDPConfigID == allowedIdP.IDPConfigID { + return &domain.ExternalLoginStep{SelectedIDPConfigID: request.SelectedIDPConfigID}, nil + } + } + // error if the explicitly set IdP is not allowed, to avoid misinterpretation with usage of another IdP + return nil, zerrors.ThrowPreconditionFailed(nil, "LOGIN-LWif2", "Errors.Org.IdpNotExisting") } // reuse the previously used IdP from the session if userSession.SelectedIDPConfigID != "" { - return &domain.ExternalLoginStep{SelectedIDPConfigID: userSession.SelectedIDPConfigID}, nil + // only use the previously used IdP if allowed + for _, allowedIdP := range request.AllowedExternalIDPs { + if userSession.SelectedIDPConfigID == allowedIdP.IDPConfigID { + return &domain.ExternalLoginStep{SelectedIDPConfigID: userSession.SelectedIDPConfigID}, nil + } + } } - // then use an existing linked IdP of the user + // then use an existing linked and allowed IdP of the user if len(idps) > 0 { - return &domain.ExternalLoginStep{SelectedIDPConfigID: idps[0].IDPID}, nil + return &domain.ExternalLoginStep{SelectedIDPConfigID: idps[0]}, nil } // if the user did not link one, then just use one of the configured IdPs of the org if len(request.AllowedExternalIDPs) > 0 { diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go index dda8c54872..976ae8d8a9 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request_test.go @@ -1247,6 +1247,7 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { args{&domain.AuthRequest{ UserID: "UserID", SelectedIDPConfigID: "IDPConfigID", + AllowedExternalIDPs: []*domain.IDPProvider{{IDPConfigID: "IDPConfigID"}}, LoginPolicy: &domain.LoginPolicy{ AllowUsernamePassword: false, SecondFactorCheckLifetime: 18 * time.Hour, @@ -1254,6 +1255,193 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { []domain.NextStep{&domain.ExternalLoginStep{SelectedIDPConfigID: "IDPConfigID"}}, nil, }, + { + "external user (idp selected, not allowed, no external verification), error", + fields{ + userSessionViewProvider: &mockViewUserSession{ + SecondFactorVerification: testNow.Add(-5 * time.Minute), + }, + userViewProvider: &mockViewUser{ + IsEmailVerified: true, + MFAMaxSetUp: int32(domain.MFALevelSecondFactor), + }, + userEventProvider: &mockEventUser{}, + lockoutPolicyProvider: &mockLockoutPolicy{ + policy: &query.LockoutPolicy{ + ShowFailures: true, + }, + }, + orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, + loginPolicyProvider: &mockLoginPolicy{ + policy: &query.LoginPolicy{ + SecondFactorCheckLifetime: database.Duration(18 * time.Hour), + }, + }, + idpUserLinksProvider: &mockIDPUserLinks{}, + }, + args{&domain.AuthRequest{ + UserID: "UserID", + SelectedIDPConfigID: "IDPConfigID", + AllowedExternalIDPs: []*domain.IDPProvider{}, + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: false, + SecondFactorCheckLifetime: 18 * time.Hour, + }}, false}, + nil, + zerrors.IsPreconditionFailed, + }, + { + "external user (idp link, no external verification), external login step", + fields{ + userSessionViewProvider: &mockViewUserSession{ + SecondFactorVerification: testNow.Add(-5 * time.Minute), + }, + userViewProvider: &mockViewUser{ + IsEmailVerified: true, + MFAMaxSetUp: int32(domain.MFALevelSecondFactor), + }, + userEventProvider: &mockEventUser{}, + lockoutPolicyProvider: &mockLockoutPolicy{ + policy: &query.LockoutPolicy{ + ShowFailures: true, + }, + }, + orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, + loginPolicyProvider: &mockLoginPolicy{ + policy: &query.LoginPolicy{ + SecondFactorCheckLifetime: database.Duration(18 * time.Hour), + }, + }, + idpUserLinksProvider: &mockIDPUserLinks{ + []*query.IDPUserLink{ + {IDPID: "IDPConfigID"}, + }, + }, + }, + args{&domain.AuthRequest{ + UserID: "UserID", + SelectedIDPConfigID: "", + AllowedExternalIDPs: []*domain.IDPProvider{{IDPConfigID: "IDPConfigID"}}, + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: false, + SecondFactorCheckLifetime: 18 * time.Hour, + }}, false}, + []domain.NextStep{&domain.ExternalLoginStep{SelectedIDPConfigID: "IDPConfigID"}}, + nil, + }, + { + "external user (idp link not allowed, no external verification), external login step", + fields{ + userSessionViewProvider: &mockViewUserSession{ + SecondFactorVerification: testNow.Add(-5 * time.Minute), + }, + userViewProvider: &mockViewUser{ + IsEmailVerified: true, + MFAMaxSetUp: int32(domain.MFALevelSecondFactor), + }, + userEventProvider: &mockEventUser{}, + lockoutPolicyProvider: &mockLockoutPolicy{ + policy: &query.LockoutPolicy{ + ShowFailures: true, + }, + }, + orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, + loginPolicyProvider: &mockLoginPolicy{ + policy: &query.LoginPolicy{ + SecondFactorCheckLifetime: database.Duration(18 * time.Hour), + }, + }, + idpUserLinksProvider: &mockIDPUserLinks{ + []*query.IDPUserLink{ + {IDPID: "IDPConfigID1"}, + }, + }, + }, + args{&domain.AuthRequest{ + UserID: "UserID", + SelectedIDPConfigID: "", + AllowedExternalIDPs: []*domain.IDPProvider{{IDPConfigID: "IDPConfigID2"}}, + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: false, + SecondFactorCheckLifetime: 18 * time.Hour, + }}, false}, + []domain.NextStep{&domain.ExternalLoginStep{SelectedIDPConfigID: "IDPConfigID2"}}, + nil, + }, + { + "external user (idp link not allowed, none allowed, no external verification), external login step", + fields{ + userSessionViewProvider: &mockViewUserSession{ + SecondFactorVerification: testNow.Add(-5 * time.Minute), + }, + userViewProvider: &mockViewUser{ + IsEmailVerified: true, + MFAMaxSetUp: int32(domain.MFALevelSecondFactor), + }, + userEventProvider: &mockEventUser{}, + lockoutPolicyProvider: &mockLockoutPolicy{ + policy: &query.LockoutPolicy{ + ShowFailures: true, + }, + }, + orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, + loginPolicyProvider: &mockLoginPolicy{ + policy: &query.LoginPolicy{ + SecondFactorCheckLifetime: database.Duration(18 * time.Hour), + }, + }, + idpUserLinksProvider: &mockIDPUserLinks{ + []*query.IDPUserLink{ + {IDPID: "IDPConfigID1"}, + }, + }, + }, + args{&domain.AuthRequest{ + UserID: "UserID", + SelectedIDPConfigID: "", + AllowedExternalIDPs: []*domain.IDPProvider{}, + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: false, + SecondFactorCheckLifetime: 18 * time.Hour, + }}, false}, + nil, + zerrors.IsPreconditionFailed, + }, + { + "external user (no idp allowed, no external verification), error", + fields{ + userSessionViewProvider: &mockViewUserSession{ + SecondFactorVerification: testNow.Add(-5 * time.Minute), + }, + userViewProvider: &mockViewUser{ + IsEmailVerified: true, + MFAMaxSetUp: int32(domain.MFALevelSecondFactor), + }, + userEventProvider: &mockEventUser{}, + lockoutPolicyProvider: &mockLockoutPolicy{ + policy: &query.LockoutPolicy{ + ShowFailures: true, + }, + }, + orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, + loginPolicyProvider: &mockLoginPolicy{ + policy: &query.LoginPolicy{ + SecondFactorCheckLifetime: database.Duration(18 * time.Hour), + }, + }, + idpUserLinksProvider: &mockIDPUserLinks{}, + }, + args{&domain.AuthRequest{ + UserID: "UserID", + SelectedIDPConfigID: "", + AllowedExternalIDPs: []*domain.IDPProvider{}, + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: false, + SecondFactorCheckLifetime: 18 * time.Hour, + }}, false}, + nil, + zerrors.IsPreconditionFailed, + }, { "external user (only idp available, no external verification), external login step", fields{ @@ -1281,13 +1469,49 @@ func TestAuthRequestRepo_nextSteps(t *testing.T) { }, }, args{&domain.AuthRequest{ - UserID: "UserID", + UserID: "UserID", + AllowedExternalIDPs: []*domain.IDPProvider{{IDPConfigID: "IDPConfigID"}}, LoginPolicy: &domain.LoginPolicy{ AllowUsernamePassword: false, SecondFactorCheckLifetime: 18 * time.Hour, }}, false}, []domain.NextStep{&domain.ExternalLoginStep{SelectedIDPConfigID: "IDPConfigID"}}, nil, + }, { + "external user (only idp available, no allowed, no external verification), external login step", + fields{ + userSessionViewProvider: &mockViewUserSession{ + SecondFactorVerification: testNow.Add(-5 * time.Minute), + }, + userViewProvider: &mockViewUser{ + IsEmailVerified: true, + MFAMaxSetUp: int32(domain.MFALevelSecondFactor), + }, + userEventProvider: &mockEventUser{}, + lockoutPolicyProvider: &mockLockoutPolicy{ + policy: &query.LockoutPolicy{ + ShowFailures: true, + }, + }, + orgViewProvider: &mockViewOrg{State: domain.OrgStateActive}, + loginPolicyProvider: &mockLoginPolicy{ + policy: &query.LoginPolicy{ + SecondFactorCheckLifetime: database.Duration(18 * time.Hour), + }, + }, + idpUserLinksProvider: &mockIDPUserLinks{ + idps: []*query.IDPUserLink{{IDPID: "IDPConfigID"}}, + }, + }, + args{&domain.AuthRequest{ + UserID: "UserID", + AllowedExternalIDPs: []*domain.IDPProvider{}, + LoginPolicy: &domain.LoginPolicy{ + AllowUsernamePassword: false, + SecondFactorCheckLifetime: 18 * time.Hour, + }}, false}, + nil, + zerrors.IsPreconditionFailed, }, { "external user (external verification set), callback", From 42cc6dce79bb0659956d358d142a5ecf2cac59f3 Mon Sep 17 00:00:00 2001 From: Alexey Morozov Date: Tue, 7 Jan 2025 23:32:19 +0300 Subject: [PATCH 17/21] fix(i18n): typo in Russian login description (#9100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Which Problems Are Solved Typo in RU localization on login page. # How the Problems Are Solved Fixed typo by replacing to correct text. # Additional Changes n/a # Additional Context n/a Co-authored-by: Tim Möhlmann --- internal/api/ui/login/static/i18n/ru.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/ui/login/static/i18n/ru.yaml b/internal/api/ui/login/static/i18n/ru.yaml index 03239e0612..221c20a2e9 100644 --- a/internal/api/ui/login/static/i18n/ru.yaml +++ b/internal/api/ui/login/static/i18n/ru.yaml @@ -1,6 +1,6 @@ Login: Title: Добро пожаловать! - Description: Введите свои данные дял входа. + Description: Введите свои данные для входа. TitleLinking: Вход для привязки пользователей DescriptionLinking: Введите данные для входа, чтобы привязать внешнего пользователя к учётной записи ZITADEL. LoginNameLabel: Логин From db8d794794eba191d1b1f3a79ea5b4ec2c90a821 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Wed, 8 Jan 2025 10:40:33 +0200 Subject: [PATCH 18/21] fix(oidc): ignore algorithm for legacy signer (#9148) # Which Problems Are Solved It was possible to set a diffent algorithm for the legacy signer. This is not supported howerver and breaks the token endpoint. # How the Problems Are Solved Remove the OIDC.SigningKeyAlgorithm config option and hard-code RS256 for the legacy signer. # Additional Changes - none # Additional Context Only RS256 is supported by the legacy signer. It was mentioned in the comment of the config not to use it and use the webkeys resource instead. - closes #9121 --- cmd/defaults.yaml | 3 --- internal/api/oidc/key.go | 10 +++++----- internal/api/oidc/op.go | 4 ---- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index 868ee06866..e376e8a488 100644 --- a/cmd/defaults.yaml +++ b/cmd/defaults.yaml @@ -530,9 +530,6 @@ OIDC: GrantTypeRefreshToken: true # ZITADEL_OIDC_GRANTTYPEREFRESHTOKEN RequestObjectSupported: true # ZITADEL_OIDC_REQUESTOBJECTSUPPORTED - # Deprecated: The signing algorithm is determined by the generated keys. - # Use the web keys resource to generate keys with different algorithms. - SigningKeyAlgorithm: RS256 # ZITADEL_OIDC_SIGNINGKEYALGORITHM # Sets the default values for lifetime and expiration for OIDC # This default can be overwritten in the default instance configuration and for each instance during runtime # !!! Changing this after the initial setup will have no impact without a restart !!! diff --git a/internal/api/oidc/key.go b/internal/api/oidc/key.go index 535aa846b4..6c0599f556 100644 --- a/internal/api/oidc/key.go +++ b/internal/api/oidc/key.go @@ -354,15 +354,15 @@ func (o *OPStorage) getSigningKey(ctx context.Context) (op.SigningKey, error) { if keys.State != nil { position = keys.State.Position } - return nil, o.refreshSigningKey(ctx, o.signingKeyAlgorithm, position) + return nil, o.refreshSigningKey(ctx, position) } -func (o *OPStorage) refreshSigningKey(ctx context.Context, algorithm string, position float64) error { +func (o *OPStorage) refreshSigningKey(ctx context.Context, position float64) error { ok, err := o.ensureIsLatestKey(ctx, position) if err != nil || !ok { return zerrors.ThrowInternal(err, "OIDC-ASfh3", "cannot ensure that projection is up to date") } - err = o.lockAndGenerateSigningKeyPair(ctx, algorithm) + err = o.lockAndGenerateSigningKeyPair(ctx) if err != nil { return zerrors.ThrowInternal(err, "OIDC-ADh31", "could not create signing key") } @@ -393,7 +393,7 @@ func PrivateKeyToSigningKey(key query.PrivateKey, algorithm crypto.EncryptionAlg }, nil } -func (o *OPStorage) lockAndGenerateSigningKeyPair(ctx context.Context, algorithm string) error { +func (o *OPStorage) lockAndGenerateSigningKeyPair(ctx context.Context) error { logging.Info("lock and generate signing key pair") ctx, cancel := context.WithCancel(ctx) @@ -409,7 +409,7 @@ func (o *OPStorage) lockAndGenerateSigningKeyPair(ctx context.Context, algorithm return err } - return o.command.GenerateSigningKeyPair(setOIDCCtx(ctx), algorithm) + return o.command.GenerateSigningKeyPair(setOIDCCtx(ctx), "RS256") } func (o *OPStorage) getMaxKeySequence(ctx context.Context) (float64, error) { diff --git a/internal/api/oidc/op.go b/internal/api/oidc/op.go index 86b89690bf..153a13f06e 100644 --- a/internal/api/oidc/op.go +++ b/internal/api/oidc/op.go @@ -31,7 +31,6 @@ type Config struct { AuthMethodPrivateKeyJWT bool GrantTypeRefreshToken bool RequestObjectSupported bool - SigningKeyAlgorithm string DefaultAccessTokenLifetime time.Duration DefaultIdTokenLifetime time.Duration DefaultRefreshTokenIdleExpiration time.Duration @@ -71,7 +70,6 @@ type OPStorage struct { defaultLogoutURLV2 string defaultAccessTokenLifetime time.Duration defaultIdTokenLifetime time.Duration - signingKeyAlgorithm string defaultRefreshTokenIdleExpiration time.Duration defaultRefreshTokenExpiration time.Duration encAlg crypto.EncryptionAlgorithm @@ -162,7 +160,6 @@ func NewServer( jwksCacheControlMaxAge: config.JWKSCacheControlMaxAge, fallbackLogger: fallbackLogger, hasher: hasher, - signingKeyAlgorithm: config.SigningKeyAlgorithm, encAlg: encryptionAlg, opCrypto: op.NewAESCrypto(opConfig.CryptoKey), assetAPIPrefix: assets.AssetAPI(), @@ -232,7 +229,6 @@ func newStorage(config Config, command *command.Commands, query *query.Queries, defaultLoginURL: fmt.Sprintf("%s%s?%s=", login.HandlerPrefix, login.EndpointLogin, login.QueryAuthRequestID), defaultLoginURLV2: config.DefaultLoginURLV2, defaultLogoutURLV2: config.DefaultLogoutURLV2, - signingKeyAlgorithm: config.SigningKeyAlgorithm, defaultAccessTokenLifetime: config.DefaultAccessTokenLifetime, defaultIdTokenLifetime: config.DefaultIdTokenLifetime, defaultRefreshTokenIdleExpiration: config.DefaultRefreshTokenIdleExpiration, From c966446f803aacfc03fbc0c152e11dbe34e9d64e Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Wed, 8 Jan 2025 10:30:12 +0100 Subject: [PATCH 19/21] fix: correctly get x-forwarded-for for browser info in events (#9149) # Which Problems Are Solved Events like "password check succeeded" store some information about the caller including their IP. The `X-Forwarded-For` was not correctly logged, but instead the RemoteAddress. # How the Problems Are Solved - Correctly get the `X-Forwarded-For` in canonical form. # Additional Changes None # Additional Context closes [#9106](https://github.com/zitadel/zitadel/issues/9106) --- internal/api/http/header.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/internal/api/http/header.go b/internal/api/http/header.go index 16ae7cf48c..982684c77c 100644 --- a/internal/api/http/header.go +++ b/internal/api/http/header.go @@ -108,14 +108,8 @@ func GetOrgID(r *http.Request) string { } func GetForwardedFor(headers http.Header) (string, bool) { - forwarded, ok := headers[ForwardedFor] - if ok { - ip := strings.TrimSpace(strings.Split(forwarded[0], ",")[0]) - if ip != "" { - return ip, true - } - } - return "", false + forwarded := strings.Split(headers.Get(ForwardedFor), ",")[0] + return forwarded, forwarded != "" } func RemoteAddrFromCtx(ctx context.Context) string { From df2c6f1d4c23fe43c6a78d911f65f82dd4594ecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Wed, 8 Jan 2025 13:59:44 +0200 Subject: [PATCH 20/21] perf(eventstore): optimize commands to events function (#9092) # Which Problems Are Solved We were seeing high query costs in a the lateral join executed in the commands_to_events procedural function in the database. The high cost resulted in incremental CPU usage as a load test continued and less req/sec handled, sarting at 836 and ending at 130 req/sec. # How the Problems Are Solved 1. Set `PARALLEL SAFE`. I noticed that this option defaults to `UNSAFE`. But it's actually safe if the function doesn't `INSERT` 2. Set the returned `ROWS 10` parameter. 3. Function is re-written in Pl/PgSQL so that we eliminate expensive joins. 4. Introduced an intermediate state that does `SELECT DISTINCT` for the aggregate so that we don't have to do an expensive lateral join. # Additional Changes Use a `COALESCE` to get the owner from the last event, instead of a `CASE` switch. # Additional Context - Function was introduced in https://github.com/zitadel/zitadel/pull/8816 - Closes https://github.com/zitadel/zitadel/issues/8352 --------- Co-authored-by: Silvan <27845747+adlerhurst@users.noreply.github.com> --- cmd/setup/40.go | 2 +- cmd/setup/40/postgres/02_func.sql | 148 ++++++++++++++++-------------- 2 files changed, 80 insertions(+), 70 deletions(-) diff --git a/cmd/setup/40.go b/cmd/setup/40.go index a0d1afcf54..0a3a116d21 100644 --- a/cmd/setup/40.go +++ b/cmd/setup/40.go @@ -48,5 +48,5 @@ func (mig *InitPushFunc) Execute(ctx context.Context, _ eventstore.Event) (err e } func (mig *InitPushFunc) String() string { - return "40_init_push_func" + return "40_init_push_func_v2" } diff --git a/cmd/setup/40/postgres/02_func.sql b/cmd/setup/40/postgres/02_func.sql index 5f84f3908c..0d566ebb42 100644 --- a/cmd/setup/40/postgres/02_func.sql +++ b/cmd/setup/40/postgres/02_func.sql @@ -1,82 +1,92 @@ -CREATE OR REPLACE FUNCTION eventstore.commands_to_events(commands eventstore.command[]) RETURNS SETOF eventstore.events2 VOLATILE AS $$ -SELECT - c.instance_id - , c.aggregate_type - , c.aggregate_id - , c.command_type AS event_type - , cs.sequence + ROW_NUMBER() OVER (PARTITION BY c.instance_id, c.aggregate_type, c.aggregate_id ORDER BY c.in_tx_order) AS sequence - , c.revision - , NOW() AS created_at - , c.payload - , c.creator - , cs.owner - , EXTRACT(EPOCH FROM NOW()) AS position - , c.in_tx_order -FROM ( - SELECT - c.instance_id - , c.aggregate_type - , c.aggregate_id - , c.command_type - , c.revision - , c.payload - , c.creator - , c.owner - , ROW_NUMBER() OVER () AS in_tx_order - FROM - UNNEST(commands) AS c -) AS c -JOIN ( - SELECT - cmds.instance_id - , cmds.aggregate_type - , cmds.aggregate_id - , CASE WHEN (e.owner IS NOT NULL OR e.owner <> '') THEN e.owner ELSE command_owners.owner END AS owner - , COALESCE(MAX(e.sequence), 0) AS sequence - FROM ( +CREATE OR REPLACE FUNCTION eventstore.latest_aggregate_state( + instance_id TEXT + , aggregate_type TEXT + , aggregate_id TEXT + + , sequence OUT BIGINT + , owner OUT TEXT +) + LANGUAGE 'plpgsql' + STABLE PARALLEL SAFE +AS $$ + BEGIN + SELECT + COALESCE(e.sequence, 0) AS sequence + , e.owner + INTO + sequence + , owner + FROM + eventstore.events2 e + WHERE + e.instance_id = $1 + AND e.aggregate_type = $2 + AND e.aggregate_id = $3 + ORDER BY + e.sequence DESC + LIMIT 1; + + RETURN; + END; +$$; + +CREATE OR REPLACE FUNCTION eventstore.commands_to_events(commands eventstore.command[]) + RETURNS SETOF eventstore.events2 + LANGUAGE 'plpgsql' + STABLE PARALLEL SAFE + ROWS 10 +AS $$ +DECLARE + "aggregate" RECORD; + current_sequence BIGINT; + current_owner TEXT; +BEGIN + FOR "aggregate" IN SELECT DISTINCT instance_id , aggregate_type , aggregate_id - , owner FROM UNNEST(commands) - ) AS cmds - LEFT JOIN eventstore.events2 AS e - ON cmds.instance_id = e.instance_id - AND cmds.aggregate_type = e.aggregate_type - AND cmds.aggregate_id = e.aggregate_id - JOIN ( + LOOP + SELECT + * + INTO + current_sequence + , current_owner + FROM eventstore.latest_aggregate_state( + "aggregate".instance_id + , "aggregate".aggregate_type + , "aggregate".aggregate_id + ); + + RETURN QUERY SELECT - DISTINCT ON ( - instance_id - , aggregate_type - , aggregate_id - ) - instance_id - , aggregate_type - , aggregate_id - , owner + c.instance_id + , c.aggregate_type + , c.aggregate_id + , c.command_type -- AS event_type + , COALESCE(current_sequence, 0) + ROW_NUMBER() OVER () -- AS sequence + , c.revision + , NOW() -- AS created_at + , c.payload + , c.creator + , COALESCE(current_owner, c.owner) -- AS owner + , EXTRACT(EPOCH FROM NOW()) -- AS position + , c.ordinality::INT -- AS in_tx_order FROM - UNNEST(commands) - ) AS command_owners ON - cmds.instance_id = command_owners.instance_id - AND cmds.aggregate_type = command_owners.aggregate_type - AND cmds.aggregate_id = command_owners.aggregate_id - GROUP BY - cmds.instance_id - , cmds.aggregate_type - , cmds.aggregate_id - , 4 -- owner -) AS cs - ON c.instance_id = cs.instance_id - AND c.aggregate_type = cs.aggregate_type - AND c.aggregate_id = cs.aggregate_id -ORDER BY - in_tx_order; -$$ LANGUAGE SQL; + UNNEST(commands) WITH ORDINALITY AS c + WHERE + c.instance_id = aggregate.instance_id + AND c.aggregate_type = aggregate.aggregate_type + AND c.aggregate_id = aggregate.aggregate_id; + END LOOP; + RETURN; +END; +$$; CREATE OR REPLACE FUNCTION eventstore.push(commands eventstore.command[]) RETURNS SETOF eventstore.events2 VOLATILE AS $$ INSERT INTO eventstore.events2 SELECT * FROM eventstore.commands_to_events(commands) +ORDER BY in_tx_order RETURNING * $$ LANGUAGE SQL; From 829f4543da19831cd80f0c4b618a9382aa36b2cf Mon Sep 17 00:00:00 2001 From: Silvan <27845747+adlerhurst@users.noreply.github.com> Date: Wed, 8 Jan 2025 17:54:17 +0100 Subject: [PATCH 21/21] perf(eventstore): redefine current sequences index (#9142) # Which Problems Are Solved On Zitadel cloud we found changing the order of columns in the `eventstore.events2_current_sequence` index improved CPU usage for the `SELECT ... FOR UPDATE` query the pusher executes. # How the Problems Are Solved `eventstore.events2_current_sequence`-index got replaced # Additional Context closes https://github.com/zitadel/zitadel/issues/9082 --- cmd/setup/44.go | 39 ++++++++++++++++++++++++++++++ cmd/setup/44/01_create_index.sql | 3 +++ cmd/setup/44/02_drop_old_index.sql | 1 + cmd/setup/config.go | 1 + cmd/setup/setup.go | 2 ++ 5 files changed, 46 insertions(+) create mode 100644 cmd/setup/44.go create mode 100644 cmd/setup/44/01_create_index.sql create mode 100644 cmd/setup/44/02_drop_old_index.sql diff --git a/cmd/setup/44.go b/cmd/setup/44.go new file mode 100644 index 0000000000..11c355a053 --- /dev/null +++ b/cmd/setup/44.go @@ -0,0 +1,39 @@ +package setup + +import ( + "context" + "embed" + "fmt" + + "github.com/zitadel/logging" + + "github.com/zitadel/zitadel/internal/database" + "github.com/zitadel/zitadel/internal/eventstore" +) + +var ( + //go:embed 44/*.sql + replaceCurrentSequencesIndex embed.FS +) + +type ReplaceCurrentSequencesIndex struct { + dbClient *database.DB +} + +func (mig *ReplaceCurrentSequencesIndex) Execute(ctx context.Context, _ eventstore.Event) error { + statements, err := readStatements(replaceCurrentSequencesIndex, "44", "") + if err != nil { + return err + } + for _, stmt := range statements { + logging.WithFields("file", stmt.file, "migration", mig.String()).Info("execute statement") + if _, err := mig.dbClient.ExecContext(ctx, stmt.query); err != nil { + return fmt.Errorf("%s %s: %w", mig.String(), stmt.file, err) + } + } + return nil +} + +func (mig *ReplaceCurrentSequencesIndex) String() string { + return "44_replace_current_sequences_index" +} diff --git a/cmd/setup/44/01_create_index.sql b/cmd/setup/44/01_create_index.sql new file mode 100644 index 0000000000..105d5b76b6 --- /dev/null +++ b/cmd/setup/44/01_create_index.sql @@ -0,0 +1,3 @@ +CREATE INDEX CONCURRENTLY IF NOT EXISTS events2_current_sequence2 + ON eventstore.events2 USING btree + (aggregate_id ASC, aggregate_type ASC, instance_id ASC, sequence DESC); diff --git a/cmd/setup/44/02_drop_old_index.sql b/cmd/setup/44/02_drop_old_index.sql new file mode 100644 index 0000000000..cf97ff9fc3 --- /dev/null +++ b/cmd/setup/44/02_drop_old_index.sql @@ -0,0 +1 @@ +DROP INDEX IF EXISTS eventstore.events2_current_sequence; \ No newline at end of file diff --git a/cmd/setup/config.go b/cmd/setup/config.go index 407a9412bb..9f34c2baa5 100644 --- a/cmd/setup/config.go +++ b/cmd/setup/config.go @@ -129,6 +129,7 @@ type Steps struct { s40InitPushFunc *InitPushFunc s42Apps7OIDCConfigsLoginVersion *Apps7OIDCConfigsLoginVersion s43CreateFieldsDomainIndex *CreateFieldsDomainIndex + s44ReplaceCurrentSequencesIndex *ReplaceCurrentSequencesIndex } func MustNewSteps(v *viper.Viper) *Steps { diff --git a/cmd/setup/setup.go b/cmd/setup/setup.go index c803ab55b6..4ffef441af 100644 --- a/cmd/setup/setup.go +++ b/cmd/setup/setup.go @@ -172,6 +172,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string) steps.s40InitPushFunc = &InitPushFunc{dbClient: esPusherDBClient} steps.s42Apps7OIDCConfigsLoginVersion = &Apps7OIDCConfigsLoginVersion{dbClient: esPusherDBClient} steps.s43CreateFieldsDomainIndex = &CreateFieldsDomainIndex{dbClient: queryDBClient} + steps.s44ReplaceCurrentSequencesIndex = &ReplaceCurrentSequencesIndex{dbClient: esPusherDBClient} err = projection.Create(ctx, projectionDBClient, eventstoreClient, config.Projections, nil, nil, nil) logging.OnError(err).Fatal("unable to start projections") @@ -225,6 +226,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string) steps.s35AddPositionToIndexEsWm, steps.s36FillV2Milestones, steps.s38BackChannelLogoutNotificationStart, + steps.s44ReplaceCurrentSequencesIndex, } { mustExecuteMigration(ctx, eventstoreClient, step, "migration failed") }