From b6fad5ac83b6b53d6ef1d66d5674ce4f91d9e679 Mon Sep 17 00:00:00 2001 From: Shubham Singh Sugara <37795429+shubhamsugara22@users.noreply.github.com> Date: Mon, 14 Oct 2024 13:12:08 +0530 Subject: [PATCH 1/4] fix: Update Defaults.yaml (#8731) # Which Problems Are Solved The primary issue addressed in this PR is that the defaults.yaml file contains escaped characters (like `<` for < and `>` for >) in message texts, which prevents valid HTML rendering in certain parts of the Zitadel platform. These escaped characters are used in user-facing content (e.g., email templates or notifications), resulting in improperly displayed text, where the HTML elements like line breaks or bold text don't render correctly. # How the Problems Are Solved The solution involves replacing the escaped characters with their corresponding HTML tags in the defaults.yaml file, ensuring that the HTML renders correctly in the emails or user interfaces where these messages are displayed. This update ensures that: - The HTML in these message templates is rendered properly, improving the user experience. - The content looks professional and adheres to web standards for displaying HTML content. # Additional Changes N/A # Additional Context N/A - Closes #8531 Co-authored-by: Max Peintner (cherry picked from commit 7eb54e4c7beb0203cdbc82b0688252c9e98a6e16) --- cmd/defaults.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index 81c312137c..90c2db1f01 100644 --- a/cmd/defaults.yaml +++ b/cmd/defaults.yaml @@ -854,7 +854,7 @@ DefaultInstance: PreHeader: User initialisieren Subject: User initialisieren Greeting: Hallo {{.DisplayName}}, - Text: Dieser Benutzer wurde soeben im Zitadel erstellt. Mit dem Benutzernamen <br><strong>{{.PreferredLoginName}}</strong><br> kannst du dich anmelden. Nutze den untenstehenden Button, um die Initialisierung abzuschliessen <br>(Code <strong>{{.Code}}</strong>).<br> Falls du dieses Mail nicht angefordert hast, kannst du es einfach ignorieren. + Text: Dieser Benutzer wurde soeben im Zitadel erstellt. Mit dem Benutzernamen
{{.PreferredLoginName}}
kannst du dich anmelden. Nutze den untenstehenden Button, um die Initialisierung abzuschliessen
(Code {{.Code}}).
Falls du dieses Mail nicht angefordert hast, kannst du es einfach ignorieren. ButtonText: Initialisierung abschliessen - MessageTextType: PasswordReset Language: de @@ -862,7 +862,7 @@ DefaultInstance: PreHeader: Passwort zurücksetzen Subject: Passwort zurücksetzen Greeting: Hallo {{.DisplayName}}, - Text: Wir haben eine Anfrage für das Zurücksetzen deines Passwortes bekommen. Du kannst den untenstehenden Button verwenden, um dein Passwort zurückzusetzen <br>(Code <strong>{{.Code}}</strong>).<br> Falls du dieses Mail nicht angefordert hast, kannst du es ignorieren. + Text: Wir haben eine Anfrage für das Zurücksetzen deines Passwortes bekommen. Du kannst den untenstehenden Button verwenden, um dein Passwort zurückzusetzen
(Code {{.Code}}).
Falls du dieses Mail nicht angefordert hast, kannst du es ignorieren. ButtonText: Passwort zurücksetzen - MessageTextType: VerifyEmail Language: de @@ -870,7 +870,7 @@ DefaultInstance: PreHeader: Email verifizieren Subject: Email verifizieren Greeting: Hallo {{.DisplayName}}, - Text: Eine neue E-Mail Adresse wurde hinzugefügt. Bitte verwende den untenstehenden Button um diese zu verifizieren <br>(Code <strong>{{.Code}}</strong>).<br> Falls du deine E-Mail Adresse nicht selber hinzugefügt hast, kannst du dieses E-Mail ignorieren. + Text: Eine neue E-Mail Adresse wurde hinzugefügt. Bitte verwende den untenstehenden Button um diese zu verifizieren
(Code {{.Code}}).
Falls du deine E-Mail Adresse nicht selber hinzugefügt hast, kannst du dieses E-Mail ignorieren. ButtonText: Email verifizieren - MessageTextType: VerifyPhone Language: de From 63a9312a8e3d4e82222052cf94fcd85e47f0d4b7 Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Thu, 17 Oct 2024 23:20:57 +0200 Subject: [PATCH 2/4] chore: improve integration tests (#8727) Improve integration tests: - spliting the tests in TokenExchange to isolated instances and in parallel - corrected some test structure so that the check for Details is no done anymore if the test already failed - replace required-calls with assert-calls to not stop the testing - add gofakeit for application, project and usernames(emails) - add eventually checks for testing in actions v2, so the request only get called when the execution is defined - check for length of results in list/search endpoints to avoid index errors (cherry picked from commit 8d973636428484193b5c9b5fe6c9ba8f70622662) --- .../admin/integration_test/iam_member_test.go | 22 +- .../integration_test/iam_settings_test.go | 26 +- .../admin/integration_test/import_test.go | 3 +- .../admin/integration_test/server_test.go | 9 +- .../v2/integration_test/feature_test.go | 12 +- .../v2beta/integration_test/feature_test.go | 6 +- .../idp/v2/integration_test/query_test.go | 25 +- .../management/integration_test/org_test.go | 11 +- .../grpc/org/v2/integration_test/org_test.go | 12 +- .../org/v2/integration_test/query_test.go | 22 +- internal/api/grpc/org/v2/org_test.go | 5 +- .../org/v2beta/integration_test/org_test.go | 12 +- .../integration_test/execution_target_test.go | 46 ++- .../integration_test/execution_test.go | 1 - .../v3alpha/integration_test/query_test.go | 64 ++- .../v3alpha/integration_test/server_test.go | 9 +- .../v3alpha/integration_test/email_test.go | 12 +- .../v3alpha/integration_test/phone_test.go | 12 +- .../v3alpha/integration_test/server_test.go | 10 +- .../v3alpha/integration_test/user_test.go | 25 +- .../v3alpha/integration_test/query_test.go | 50 +-- .../v3alpha/integration_test/server_test.go | 11 +- .../webkey_integration_test.go | 8 +- .../v2/integration_test/session_test.go | 5 +- .../v2beta/integration_test/session_test.go | 5 +- .../v2/integration_test/settings_test.go | 7 +- .../v2beta/integration_test/settings_test.go | 7 +- .../system/integration_test/instance_test.go | 2 +- .../integration_test/limits_block_test.go | 4 +- .../user/v2/integration_test/email_test.go | 23 +- .../user/v2/integration_test/idp_link_test.go | 44 +-- .../user/v2/integration_test/passkey_test.go | 18 +- .../user/v2/integration_test/password_test.go | 5 +- .../user/v2/integration_test/phone_test.go | 26 +- .../user/v2/integration_test/query_test.go | 84 ++-- .../user/v2/integration_test/user_test.go | 86 +++-- .../v2beta/integration_test/email_test.go | 23 +- .../v2beta/integration_test/password_test.go | 7 +- .../v2beta/integration_test/phone_test.go | 28 +- .../v2beta/integration_test/query_test.go | 76 ++-- .../user/v2beta/integration_test/user_test.go | 74 ++-- internal/api/idp/integration_test/idp_test.go | 21 +- .../api/oidc/integration_test/oidc_test.go | 3 +- .../integration_test/token_exchange_test.go | 365 +++++++++--------- .../oidc/integration_test/userinfo_test.go | 10 +- internal/integration/context.go | 30 ++ .../integration_test/telemetry_pusher_test.go | 2 + 47 files changed, 719 insertions(+), 649 deletions(-) create mode 100644 internal/integration/context.go 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 c65ede3d26..1b6440923e 100644 --- a/internal/api/grpc/admin/integration_test/iam_member_test.go +++ b/internal/api/grpc/admin/integration_test/iam_member_test.go @@ -29,7 +29,7 @@ var iamRoles = []string{ func TestServer_ListIAMMemberRoles(t *testing.T) { got, err := Client.ListIAMMemberRoles(AdminCTX, &admin_pb.ListIAMMemberRolesRequest{}) - require.NoError(t, err) + assert.NoError(t, err) assert.ElementsMatch(t, iamRoles, got.GetRoles()) } @@ -92,23 +92,23 @@ func TestServer_ListIAMMembers(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, 20*time.Second) assert.EventuallyWithT(t, func(ct *assert.CollectT) { got, err := Client.ListIAMMembers(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(ct, err) + require.Error(ct, err) return } require.NoError(ct, err) wantResult := tt.want.GetResult() gotResult := got.GetResult() - if assert.Len(ct, gotResult, len(wantResult)) { - for i, want := range wantResult { - assert.Equal(ct, want.GetUserId(), gotResult[i].GetUserId()) - assert.ElementsMatch(ct, want.GetRoles(), gotResult[i].GetRoles()) - } + require.Len(ct, gotResult, len(wantResult)) + for i, want := range wantResult { + assert.Equal(ct, want.GetUserId(), gotResult[i].GetUserId()) + assert.ElementsMatch(ct, want.GetRoles(), gotResult[i].GetRoles()) } - }, time.Minute, time.Second) + }, retryDuration, tick) }) } } @@ -178,7 +178,7 @@ func TestServer_AddIAMMember(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.AddIAMMember(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -259,7 +259,7 @@ func TestServer_UpdateIAMMember(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.UpdateIAMMember(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -316,7 +316,7 @@ func TestServer_RemoveIAMMember(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.RemoveIAMMember(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) diff --git a/internal/api/grpc/admin/integration_test/iam_settings_test.go b/internal/api/grpc/admin/integration_test/iam_settings_test.go index 2787f94755..93da4aed8a 100644 --- a/internal/api/grpc/admin/integration_test/iam_settings_test.go +++ b/internal/api/grpc/admin/integration_test/iam_settings_test.go @@ -5,6 +5,7 @@ package admin_test import ( "context" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -53,16 +54,19 @@ func TestServer_GetSecurityPolicy(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - resp, err := instance.Client.Admin.GetSecurityPolicy(tt.ctx, &admin_pb.GetSecurityPolicyRequest{}) - if tt.wantErr { - assert.Error(t, err) - return - } - require.NoError(t, err) - got, want := resp.GetPolicy(), tt.want.GetPolicy() - assert.Equal(t, want.GetEnableIframeEmbedding(), got.GetEnableIframeEmbedding(), "enable iframe embedding") - assert.Equal(t, want.GetAllowedOrigins(), got.GetAllowedOrigins(), "allowed origins") - assert.Equal(t, want.GetEnableImpersonation(), got.GetEnableImpersonation(), "enable impersonation") + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.ctx, 5*time.Second) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + resp, err := instance.Client.Admin.GetSecurityPolicy(tt.ctx, &admin_pb.GetSecurityPolicyRequest{}) + if tt.wantErr { + require.Error(ttt, err) + return + } + require.NoError(ttt, err) + got, want := resp.GetPolicy(), tt.want.GetPolicy() + assert.Equal(ttt, want.GetEnableIframeEmbedding(), got.GetEnableIframeEmbedding(), "enable iframe embedding") + assert.Equal(ttt, want.GetAllowedOrigins(), got.GetAllowedOrigins(), "allowed origins") + assert.Equal(ttt, want.GetEnableImpersonation(), got.GetEnableImpersonation(), "enable impersonation") + }, retryDuration, tick, "timeout waiting for expected target result") }) } } @@ -162,7 +166,7 @@ func TestServer_SetSecurityPolicy(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := instance.Client.Admin.SetSecurityPolicy(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) diff --git a/internal/api/grpc/admin/integration_test/import_test.go b/internal/api/grpc/admin/integration_test/import_test.go index 1ee7d7d88e..7d323e5ab8 100644 --- a/internal/api/grpc/admin/integration_test/import_test.go +++ b/internal/api/grpc/admin/integration_test/import_test.go @@ -8,7 +8,6 @@ import ( "github.com/brianvoe/gofakeit/v6" "github.com/google/uuid" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/zitadel/zitadel/internal/integration" @@ -474,7 +473,7 @@ func TestServer_ImportData(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.ImportData(AdminCTX, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) diff --git a/internal/api/grpc/admin/integration_test/server_test.go b/internal/api/grpc/admin/integration_test/server_test.go index 5d751fb485..e29b4d5c78 100644 --- a/internal/api/grpc/admin/integration_test/server_test.go +++ b/internal/api/grpc/admin/integration_test/server_test.go @@ -35,19 +35,18 @@ func TestMain(m *testing.M) { } func await(t *testing.T, ctx context.Context, cb func(*assert.CollectT)) { - deadline, ok := ctx.Deadline() - require.True(t, ok, "context must have deadline") + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) require.EventuallyWithT( t, func(tt *assert.CollectT) { defer func() { // Panics are not recovered and don't mark the test as failed, so we need to do that ourselves - require.Nil(t, recover(), "panic in await callback") + assert.Nil(tt, recover(), "panic in await callback") }() cb(tt) }, - time.Until(deadline), - time.Second, + retryDuration, + tick, "awaiting successful callback failed", ) } diff --git a/internal/api/grpc/feature/v2/integration_test/feature_test.go b/internal/api/grpc/feature/v2/integration_test/feature_test.go index a97306fdae..2af4f642c4 100644 --- a/internal/api/grpc/feature/v2/integration_test/feature_test.go +++ b/internal/api/grpc/feature/v2/integration_test/feature_test.go @@ -96,7 +96,7 @@ func TestServer_SetSystemFeatures(t *testing.T) { }) got, err := Client.SetSystemFeatures(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -137,7 +137,7 @@ func TestServer_ResetSystemFeatures(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.ResetSystemFeatures(tt.ctx, &feature.ResetSystemFeaturesRequest{}) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -211,7 +211,7 @@ func TestServer_GetSystemFeatures(t *testing.T) { } got, err := Client.GetSystemFeatures(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -278,7 +278,7 @@ func TestServer_SetInstanceFeatures(t *testing.T) { }) got, err := Client.SetInstanceFeatures(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -319,7 +319,7 @@ func TestServer_ResetInstanceFeatures(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.ResetInstanceFeatures(tt.ctx, &feature.ResetInstanceFeaturesRequest{}) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -480,7 +480,7 @@ func TestServer_GetInstanceFeatures(t *testing.T) { } got, err := Client.GetInstanceFeatures(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) diff --git a/internal/api/grpc/feature/v2beta/integration_test/feature_test.go b/internal/api/grpc/feature/v2beta/integration_test/feature_test.go index ecb8b5a993..69e05352d0 100644 --- a/internal/api/grpc/feature/v2beta/integration_test/feature_test.go +++ b/internal/api/grpc/feature/v2beta/integration_test/feature_test.go @@ -99,7 +99,7 @@ func TestServer_SetInstanceFeatures(t *testing.T) { }) got, err := Client.SetInstanceFeatures(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -140,7 +140,7 @@ func TestServer_ResetInstanceFeatures(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.ResetInstanceFeatures(tt.ctx, &feature.ResetInstanceFeaturesRequest{}) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -292,7 +292,7 @@ func TestServer_GetInstanceFeatures(t *testing.T) { } got, err := Client.GetInstanceFeatures(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) diff --git a/internal/api/grpc/idp/v2/integration_test/query_test.go b/internal/api/grpc/idp/v2/integration_test/query_test.go index c1288f537e..7bfa286b5e 100644 --- a/internal/api/grpc/idp/v2/integration_test/query_test.go +++ b/internal/api/grpc/idp/v2/integration_test/query_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/timestamppb" @@ -67,7 +68,7 @@ func TestServer_GetIDPByID(t *testing.T) { IamCTX, &idp.GetIDPByIDRequest{}, func(ctx context.Context, request *idp.GetIDPByIDRequest) *idpAttr { - name := fmt.Sprintf("GetIDPByID%d", time.Now().UnixNano()) + name := fmt.Sprintf("GetIDPByID-%s", gofakeit.AppName()) resp := Instance.AddGenericOAuthProvider(ctx, name) request.Id = resp.Id return &idpAttr{ @@ -115,7 +116,7 @@ func TestServer_GetIDPByID(t *testing.T) { UserCTX, &idp.GetIDPByIDRequest{}, func(ctx context.Context, request *idp.GetIDPByIDRequest) *idpAttr { - name := fmt.Sprintf("GetIDPByID%d", time.Now().UnixNano()) + name := fmt.Sprintf("GetIDPByID-%s", gofakeit.AppName()) resp := Instance.AddGenericOAuthProvider(IamCTX, name) request.Id = resp.Id return &idpAttr{ @@ -136,7 +137,7 @@ func TestServer_GetIDPByID(t *testing.T) { CTX, &idp.GetIDPByIDRequest{}, func(ctx context.Context, request *idp.GetIDPByIDRequest) *idpAttr { - name := fmt.Sprintf("GetIDPByID%d", time.Now().UnixNano()) + name := fmt.Sprintf("GetIDPByID-%s", gofakeit.AppName()) resp := Instance.AddOrgGenericOAuthProvider(ctx, name) request.Id = resp.Id return &idpAttr{ @@ -184,7 +185,7 @@ func TestServer_GetIDPByID(t *testing.T) { UserCTX, &idp.GetIDPByIDRequest{}, func(ctx context.Context, request *idp.GetIDPByIDRequest) *idpAttr { - name := fmt.Sprintf("GetIDPByID%d", time.Now().UnixNano()) + name := fmt.Sprintf("GetIDPByID-%s", gofakeit.AppName()) resp := Instance.AddOrgGenericOAuthProvider(CTX, name) request.Id = resp.Id return &idpAttr{ @@ -203,20 +204,14 @@ func TestServer_GetIDPByID(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { idpAttr := tt.args.dep(tt.args.ctx, tt.args.req) - retryDuration := time.Minute - if ctxDeadline, ok := CTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { - got, getErr := Client.GetIDPByID(tt.args.ctx, tt.args.req) - assertErr := assert.NoError + got, err := Client.GetIDPByID(tt.args.ctx, tt.args.req) if tt.wantErr { - assertErr = assert.Error - } - assertErr(ttt, getErr) - if getErr != nil { + require.Error(ttt, err) return } + require.NoError(ttt, err) // set provided info from creation tt.want.Idp.Details = idpAttr.Details @@ -229,7 +224,7 @@ func TestServer_GetIDPByID(t *testing.T) { tt.want.Idp.Details = got.Idp.Details // to check the rest of the content assert.Equal(ttt, tt.want.Idp, got.Idp) - }, retryDuration, time.Second) + }, retryDuration, tick) }) } } diff --git a/internal/api/grpc/management/integration_test/org_test.go b/internal/api/grpc/management/integration_test/org_test.go index 310fcf94b4..8288ceb9e9 100644 --- a/internal/api/grpc/management/integration_test/org_test.go +++ b/internal/api/grpc/management/integration_test/org_test.go @@ -97,10 +97,11 @@ func TestServer_ListOrgMembers(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) assert.EventuallyWithT(t, func(ct *assert.CollectT) { got, err := Client.ListOrgMembers(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(ct, err) + require.Error(ct, err) return } require.NoError(ct, err) @@ -113,7 +114,7 @@ func TestServer_ListOrgMembers(t *testing.T) { assert.ElementsMatch(ct, want.GetRoles(), gotResult[i].GetRoles()) } } - }, time.Minute, time.Second) + }, retryDuration, tick) }) } } @@ -183,7 +184,7 @@ func TestServer_AddOrgMember(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.AddOrgMember(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -264,7 +265,7 @@ func TestServer_UpdateOrgMember(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.UpdateOrgMember(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) @@ -321,7 +322,7 @@ func TestServer_RemoveIAMMember(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.RemoveOrgMember(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) diff --git a/internal/api/grpc/org/v2/integration_test/org_test.go b/internal/api/grpc/org/v2/integration_test/org_test.go index e84da7a811..165eb1471f 100644 --- a/internal/api/grpc/org/v2/integration_test/org_test.go +++ b/internal/api/grpc/org/v2/integration_test/org_test.go @@ -4,11 +4,11 @@ package org_test import ( "context" - "fmt" "os" "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -75,7 +75,7 @@ func TestServer_AddOrganization(t *testing.T) { name: "invalid admin type", ctx: CTX, req: &org.AddOrganizationRequest{ - Name: fmt.Sprintf("%d", time.Now().UnixNano()), + Name: gofakeit.AppName(), Admins: []*org.AddOrganizationRequest_Admin{ {}, }, @@ -86,7 +86,7 @@ func TestServer_AddOrganization(t *testing.T) { name: "admin with init", ctx: CTX, req: &org.AddOrganizationRequest{ - Name: fmt.Sprintf("%d", time.Now().UnixNano()), + Name: gofakeit.AppName(), Admins: []*org.AddOrganizationRequest_Admin{ { UserType: &org.AddOrganizationRequest_Admin_Human{ @@ -96,7 +96,7 @@ func TestServer_AddOrganization(t *testing.T) { FamilyName: "lastname", }, Email: &user.SetHumanEmail{ - Email: fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()), + Email: gofakeit.Email(), Verification: &user.SetHumanEmail_ReturnCode{ ReturnCode: &user.ReturnEmailVerificationCode{}, }, @@ -121,7 +121,7 @@ func TestServer_AddOrganization(t *testing.T) { name: "existing user and new human with idp", ctx: CTX, req: &org.AddOrganizationRequest{ - Name: fmt.Sprintf("%d", time.Now().UnixNano()), + Name: gofakeit.AppName(), Admins: []*org.AddOrganizationRequest_Admin{ { UserType: &org.AddOrganizationRequest_Admin_UserId{UserId: User.GetUserId()}, @@ -134,7 +134,7 @@ func TestServer_AddOrganization(t *testing.T) { FamilyName: "lastname", }, Email: &user.SetHumanEmail{ - Email: fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()), + Email: gofakeit.Email(), Verification: &user.SetHumanEmail_IsVerified{ IsVerified: true, }, diff --git a/internal/api/grpc/org/v2/integration_test/query_test.go b/internal/api/grpc/org/v2/integration_test/query_test.go index e075ba66ea..e52ea40018 100644 --- a/internal/api/grpc/org/v2/integration_test/query_test.go +++ b/internal/api/grpc/org/v2/integration_test/query_test.go @@ -83,10 +83,10 @@ func TestServer_ListOrganizations(t *testing.T) { func(ctx context.Context, request *org.ListOrganizationsRequest) ([]orgAttr, error) { count := 3 orgs := make([]orgAttr, count) - prefix := fmt.Sprintf("ListOrgs%d", time.Now().UnixNano()) + prefix := fmt.Sprintf("ListOrgs-%s", gofakeit.AppName()) for i := 0; i < count; i++ { name := prefix + strconv.Itoa(i) - orgResp := Instance.CreateOrganization(ctx, name, fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + orgResp := Instance.CreateOrganization(ctx, name, gofakeit.Email()) orgs[i] = orgAttr{ ID: orgResp.GetOrganizationId(), Name: name, @@ -399,25 +399,19 @@ func TestServer_ListOrganizations(t *testing.T) { } } - retryDuration := time.Minute - if ctxDeadline, ok := CTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { - got, listErr := Client.ListOrganizations(tt.args.ctx, tt.args.req) - assertErr := assert.NoError + got, err := Client.ListOrganizations(tt.args.ctx, tt.args.req) if tt.wantErr { - assertErr = assert.Error - } - assertErr(ttt, listErr) - if listErr != nil { + require.Error(ttt, err) return } + require.NoError(ttt, err) // totalResult is unrelated to the tests here so gets carried over, can vary from the count of results due to permissions tt.want.Details.TotalResult = got.Details.TotalResult // always first check length, otherwise its failed anyway - assert.Len(ttt, got.Result, len(tt.want.Result)) + require.Len(ttt, got.Result, len(tt.want.Result)) for i := range tt.want.Result { // domain from result, as it is generated though the create @@ -430,7 +424,7 @@ func TestServer_ListOrganizations(t *testing.T) { assert.Contains(ttt, got.Result, tt.want.Result[i]) } integration.AssertListDetails(t, tt.want, got) - }, retryDuration, time.Millisecond*100, "timeout waiting for expected user result") + }, retryDuration, tick, "timeout waiting for expected user result") }) } } diff --git a/internal/api/grpc/org/v2/org_test.go b/internal/api/grpc/org/v2/org_test.go index 451c4006b3..b384f858de 100644 --- a/internal/api/grpc/org/v2/org_test.go +++ b/internal/api/grpc/org/v2/org_test.go @@ -6,7 +6,6 @@ import ( "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/timestamppb" "github.com/zitadel/zitadel/internal/command" @@ -110,7 +109,7 @@ func Test_addOrganizationRequestToCommand(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := addOrganizationRequestToCommand(tt.args.request) - require.ErrorIs(t, err, tt.wantErr) + assert.ErrorIs(t, err, tt.wantErr) assert.Equal(t, tt.want, got) }) } @@ -165,7 +164,7 @@ func Test_createdOrganizationToPb(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := createdOrganizationToPb(tt.args.createdOrg) - require.ErrorIs(t, err, tt.wantErr) + assert.ErrorIs(t, err, tt.wantErr) assert.Equal(t, tt.want, got) }) } diff --git a/internal/api/grpc/org/v2beta/integration_test/org_test.go b/internal/api/grpc/org/v2beta/integration_test/org_test.go index 95b2bed3b2..5998b17a71 100644 --- a/internal/api/grpc/org/v2beta/integration_test/org_test.go +++ b/internal/api/grpc/org/v2beta/integration_test/org_test.go @@ -4,11 +4,11 @@ package org_test import ( "context" - "fmt" "os" "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -72,7 +72,7 @@ func TestServer_AddOrganization(t *testing.T) { name: "invalid admin type", ctx: CTX, req: &org.AddOrganizationRequest{ - Name: fmt.Sprintf("%d", time.Now().UnixNano()), + Name: gofakeit.AppName(), Admins: []*org.AddOrganizationRequest_Admin{ {}, }, @@ -83,7 +83,7 @@ func TestServer_AddOrganization(t *testing.T) { name: "admin with init", ctx: CTX, req: &org.AddOrganizationRequest{ - Name: fmt.Sprintf("%d", time.Now().UnixNano()), + Name: gofakeit.AppName(), Admins: []*org.AddOrganizationRequest_Admin{ { UserType: &org.AddOrganizationRequest_Admin_Human{ @@ -93,7 +93,7 @@ func TestServer_AddOrganization(t *testing.T) { FamilyName: "lastname", }, Email: &user_v2beta.SetHumanEmail{ - Email: fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()), + Email: gofakeit.Email(), Verification: &user_v2beta.SetHumanEmail_ReturnCode{ ReturnCode: &user_v2beta.ReturnEmailVerificationCode{}, }, @@ -118,7 +118,7 @@ func TestServer_AddOrganization(t *testing.T) { name: "existing user and new human with idp", ctx: CTX, req: &org.AddOrganizationRequest{ - Name: fmt.Sprintf("%d", time.Now().UnixNano()), + Name: gofakeit.AppName(), Admins: []*org.AddOrganizationRequest_Admin{ { UserType: &org.AddOrganizationRequest_Admin_UserId{UserId: User.GetUserId()}, @@ -131,7 +131,7 @@ func TestServer_AddOrganization(t *testing.T) { FamilyName: "lastname", }, Email: &user_v2beta.SetHumanEmail{ - Email: fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()), + Email: gofakeit.Email(), Verification: &user_v2beta.SetHumanEmail_IsVerified{ IsVerified: true, }, diff --git a/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go b/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go index 169ee0e5d2..042b7a416e 100644 --- a/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go +++ b/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go @@ -65,6 +65,8 @@ func TestServer_ExecutionTarget(t *testing.T) { targetRequest := instance.CreateTarget(ctx, t, "", urlRequest, domain.TargetTypeCall, false) instance.SetExecution(ctx, t, conditionRequestFullMethod(fullMethod), executionTargetsSingleTarget(targetRequest.GetDetails().GetId())) + waitForExecutionOnCondition(ctx, t, instance, conditionRequestFullMethod(fullMethod)) + // expected response from the GetTarget expectedResponse := &action.GetTargetResponse{ Target: &action.GetTarget{ @@ -120,6 +122,7 @@ func TestServer_ExecutionTarget(t *testing.T) { targetResponse := instance.CreateTarget(ctx, t, "", targetResponseURL, domain.TargetTypeCall, false) instance.SetExecution(ctx, t, conditionResponseFullMethod(fullMethod), executionTargetsSingleTarget(targetResponse.GetDetails().GetId())) + waitForExecutionOnCondition(ctx, t, instance, conditionResponseFullMethod(fullMethod)) return func() { closeRequest() closeResponse() @@ -163,6 +166,7 @@ func TestServer_ExecutionTarget(t *testing.T) { // GetTarget with used target request.Id = targetRequest.GetDetails().GetId() + waitForExecutionOnCondition(ctx, t, instance, conditionRequestFullMethod(fullMethod)) return func() { closeRequest() }, nil @@ -232,6 +236,7 @@ func TestServer_ExecutionTarget(t *testing.T) { targetResponse := instance.CreateTarget(ctx, t, "", targetResponseURL, domain.TargetTypeCall, true) instance.SetExecution(ctx, t, conditionResponseFullMethod(fullMethod), executionTargetsSingleTarget(targetResponse.GetDetails().GetId())) + waitForExecutionOnCondition(ctx, t, instance, conditionResponseFullMethod(fullMethod)) return func() { closeResponse() }, nil @@ -250,25 +255,20 @@ func TestServer_ExecutionTarget(t *testing.T) { require.NoError(t, err) defer close() } - retryDuration := 5 * time.Second - if ctxDeadline, ok := isolatedIAMOwnerCTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } - + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 5*time.Second) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := instance.Client.ActionV3Alpha.GetTarget(tt.ctx, tt.req) if tt.wantErr { - assert.Error(ttt, err, "Error: "+err.Error()) - } else { - assert.NoError(ttt, err) - } - if err != nil { + require.Error(ttt, err) return } + require.NoError(ttt, err) - integration.AssertResourceDetails(t, tt.want.GetTarget().GetDetails(), got.GetTarget().GetDetails()) - assert.Equal(t, tt.want.GetTarget().GetConfig(), got.GetTarget().GetConfig()) - }, retryDuration, time.Millisecond*100, "timeout waiting for expected execution result") + integration.AssertResourceDetails(ttt, tt.want.GetTarget().GetDetails(), got.GetTarget().GetDetails()) + tt.want.Target.Details = got.GetTarget().GetDetails() + assert.EqualExportedValues(ttt, tt.want.GetTarget().GetConfig(), got.GetTarget().GetConfig()) + + }, retryDuration, tick, "timeout waiting for expected execution result") if tt.clean != nil { tt.clean(tt.ctx) @@ -277,6 +277,26 @@ func TestServer_ExecutionTarget(t *testing.T) { } } +func waitForExecutionOnCondition(ctx context.Context, t *testing.T, instance *integration.Instance, condition *action.Condition) { + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, 5*time.Second) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + got, err := instance.Client.ActionV3Alpha.SearchExecutions(ctx, &action.SearchExecutionsRequest{ + Filters: []*action.ExecutionSearchFilter{ + {Filter: &action.ExecutionSearchFilter_InConditionsFilter{ + InConditionsFilter: &action.InConditionsFilter{Conditions: []*action.Condition{condition}}, + }}, + }, + }) + if !assert.NoError(ttt, err) { + return + } + if assert.Len(ttt, got.GetResult(), 1) { + return + } + }, retryDuration, tick, "timeout waiting for expected execution result") + return +} + func conditionRequestFullMethod(fullMethod string) *action.Condition { return &action.Condition{ ConditionType: &action.Condition_Request{ diff --git a/internal/api/grpc/resources/action/v3alpha/integration_test/execution_test.go b/internal/api/grpc/resources/action/v3alpha/integration_test/execution_test.go index 9ab8ebc7ef..19d97c3857 100644 --- a/internal/api/grpc/resources/action/v3alpha/integration_test/execution_test.go +++ b/internal/api/grpc/resources/action/v3alpha/integration_test/execution_test.go @@ -196,7 +196,6 @@ func TestServer_SetExecution_Request(t *testing.T) { require.Error(t, err) return } - require.NoError(t, err) integration.AssertResourceDetails(t, tt.want.Details, got.Details) diff --git a/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go b/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go index 37d84eec05..3756a144d8 100644 --- a/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go +++ b/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go @@ -216,16 +216,20 @@ func TestServer_GetTarget(t *testing.T) { err := tt.args.dep(tt.args.ctx, tt.args.req, tt.want) require.NoError(t, err) } - got, getErr := instance.Client.ActionV3Alpha.GetTarget(tt.args.ctx, tt.args.req) - if tt.wantErr { - assert.Error(t, getErr, "Error: "+getErr.Error()) - } else { - assert.NoError(t, getErr) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 5*time.Second) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + got, err := instance.Client.ActionV3Alpha.GetTarget(tt.args.ctx, tt.args.req) + if tt.wantErr { + require.Error(ttt, err, "Error: "+err.Error()) + return + } + require.NoError(ttt, err) + wantTarget := tt.want.GetTarget() gotTarget := got.GetTarget() - integration.AssertResourceDetails(t, wantTarget.GetDetails(), gotTarget.GetDetails()) - assert.Equal(t, wantTarget.GetConfig(), gotTarget.GetConfig()) - } + integration.AssertResourceDetails(ttt, wantTarget.GetDetails(), gotTarget.GetDetails()) + assert.EqualExportedValues(ttt, wantTarget.GetConfig(), gotTarget.GetConfig()) + }, retryDuration, tick, "timeout waiting for expected target result") }) } } @@ -474,31 +478,24 @@ func TestServer_ListTargets(t *testing.T) { require.NoError(t, err) } - retryDuration := 5 * time.Second - if ctxDeadline, ok := isolatedIAMOwnerCTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } - + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 5*time.Second) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, listErr := instance.Client.ActionV3Alpha.SearchTargets(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(ttt, listErr, "Error: "+listErr.Error()) - } else { - assert.NoError(ttt, listErr) - } - if listErr != nil { + require.Error(ttt, listErr, "Error: "+listErr.Error()) return } + require.NoError(ttt, listErr) + // always first check length, otherwise its failed anyway - if !assert.Len(ttt, got.Result, len(tt.want.Result)) { - return - } + require.Len(ttt, got.Result, len(tt.want.Result)) + for i := range tt.want.Result { integration.AssertResourceDetails(ttt, tt.want.Result[i].GetDetails(), got.Result[i].GetDetails()) - assert.Equal(ttt, tt.want.Result[i].GetConfig(), got.Result[i].GetConfig()) + assert.EqualExportedValues(ttt, tt.want.Result[i].GetConfig(), got.Result[i].GetConfig()) } integration.AssertResourceListDetails(ttt, tt.want, got) - }, retryDuration, time.Millisecond*100, "timeout waiting for expected execution result") + }, retryDuration, tick, "timeout waiting for expected execution result") }) } } @@ -866,32 +863,27 @@ func TestServer_SearchExecutions(t *testing.T) { require.NoError(t, err) } - retryDuration := 5 * time.Second - if ctxDeadline, ok := isolatedIAMOwnerCTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } - + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 5*time.Second) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, listErr := instance.Client.ActionV3Alpha.SearchExecutions(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(ttt, listErr, "Error: "+listErr.Error()) - } else { - assert.NoError(ttt, listErr) - } - if listErr != nil { + require.Error(ttt, listErr, "Error: "+listErr.Error()) return } + require.NoError(ttt, listErr) // always first check length, otherwise its failed anyway - assert.Len(ttt, got.Result, len(tt.want.Result)) + require.Len(ttt, got.Result, len(tt.want.Result)) for i := range tt.want.Result { // as not sorted, all elements have to be checked // workaround as oneof elements can only be checked with assert.EqualExportedValues() if j, found := containExecution(got.Result, tt.want.Result[i]); found { - assert.EqualExportedValues(t, tt.want.Result[i], got.Result[j]) + integration.AssertResourceDetails(ttt, tt.want.Result[i].GetDetails(), got.Result[j].GetDetails()) + got.Result[j].Details = tt.want.Result[i].GetDetails() + assert.EqualExportedValues(ttt, tt.want.Result[i], got.Result[j]) } } integration.AssertResourceListDetails(ttt, tt.want, got) - }, retryDuration, time.Millisecond*100, "timeout waiting for expected execution result") + }, retryDuration, tick, "timeout waiting for expected execution result") }) } } diff --git a/internal/api/grpc/resources/action/v3alpha/integration_test/server_test.go b/internal/api/grpc/resources/action/v3alpha/integration_test/server_test.go index 357f532356..3c1c32062d 100644 --- a/internal/api/grpc/resources/action/v3alpha/integration_test/server_test.go +++ b/internal/api/grpc/resources/action/v3alpha/integration_test/server_test.go @@ -43,10 +43,8 @@ func ensureFeatureEnabled(t *testing.T, instance *integration.Instance) { Actions: gu.Ptr(true), }) require.NoError(t, err) - retryDuration := time.Minute - if ctxDeadline, ok := ctx.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { f, err := instance.Client.FeatureV2.GetInstanceFeatures(ctx, &feature.GetInstanceFeaturesRequest{ @@ -59,12 +57,13 @@ func ensureFeatureEnabled(t *testing.T, instance *integration.Instance) { time.Second, "timed out waiting for ensuring instance feature") + retryDuration, tick = integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { _, err := instance.Client.ActionV3Alpha.ListExecutionMethods(ctx, &action.ListExecutionMethodsRequest{}) assert.NoError(ttt, err) }, retryDuration, - time.Second, + tick, "timed out waiting for ensuring instance feature call") } diff --git a/internal/api/grpc/resources/user/v3alpha/integration_test/email_test.go b/internal/api/grpc/resources/user/v3alpha/integration_test/email_test.go index c5bec9008e..4b5a342905 100644 --- a/internal/api/grpc/resources/user/v3alpha/integration_test/email_test.go +++ b/internal/api/grpc/resources/user/v3alpha/integration_test/email_test.go @@ -350,10 +350,10 @@ func TestServer_SetContactEmail(t *testing.T) { } got, err := instance.Client.UserV3Alpha.SetContactEmail(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.res.want, got.Details) if tt.res.returnCode { assert.NotNil(t, got.VerificationCode) @@ -545,10 +545,10 @@ func TestServer_VerifyContactEmail(t *testing.T) { } got, err := instance.Client.UserV3Alpha.VerifyContactEmail(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.res.want, got.Details) }) } @@ -757,10 +757,10 @@ func TestServer_ResendContactEmailCode(t *testing.T) { } got, err := instance.Client.UserV3Alpha.ResendContactEmailCode(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.res.want, got.Details) if tt.res.returnCode { assert.NotNil(t, got.VerificationCode) diff --git a/internal/api/grpc/resources/user/v3alpha/integration_test/phone_test.go b/internal/api/grpc/resources/user/v3alpha/integration_test/phone_test.go index d61135d30d..9fcacd7457 100644 --- a/internal/api/grpc/resources/user/v3alpha/integration_test/phone_test.go +++ b/internal/api/grpc/resources/user/v3alpha/integration_test/phone_test.go @@ -277,10 +277,10 @@ func TestServer_SetContactPhone(t *testing.T) { } got, err := instance.Client.UserV3Alpha.SetContactPhone(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.res.want, got.Details) if tt.res.returnCode { assert.NotNil(t, got.VerificationCode) @@ -474,10 +474,10 @@ func TestServer_VerifyContactPhone(t *testing.T) { } got, err := instance.Client.UserV3Alpha.VerifyContactPhone(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.res.want, got.Details) }) } @@ -686,10 +686,10 @@ func TestServer_ResendContactPhoneCode(t *testing.T) { } got, err := instance.Client.UserV3Alpha.ResendContactPhoneCode(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.res.want, got.Details) if tt.res.returnCode { assert.NotNil(t, got.VerificationCode) diff --git a/internal/api/grpc/resources/user/v3alpha/integration_test/server_test.go b/internal/api/grpc/resources/user/v3alpha/integration_test/server_test.go index fee1a38430..467a563dcf 100644 --- a/internal/api/grpc/resources/user/v3alpha/integration_test/server_test.go +++ b/internal/api/grpc/resources/user/v3alpha/integration_test/server_test.go @@ -43,10 +43,7 @@ func ensureFeatureEnabled(t *testing.T, instance *integration.Instance) { UserSchema: gu.Ptr(true), }) require.NoError(t, err) - retryDuration := time.Minute - if ctxDeadline, ok := ctx.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, 5*time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { f, err := instance.Client.FeatureV2.GetInstanceFeatures(ctx, &feature.GetInstanceFeaturesRequest{ @@ -58,15 +55,16 @@ func ensureFeatureEnabled(t *testing.T, instance *integration.Instance) { } }, retryDuration, - time.Second, + tick, "timed out waiting for ensuring instance feature") + retryDuration, tick = integration.WaitForAndTickWithMaxDuration(ctx, 5*time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { _, err := instance.Client.UserV3Alpha.SearchUsers(ctx, &user.SearchUsersRequest{}) assert.NoError(ttt, err) }, retryDuration, - time.Second, + tick, "timed out waiting for ensuring instance feature call") } diff --git a/internal/api/grpc/resources/user/v3alpha/integration_test/user_test.go b/internal/api/grpc/resources/user/v3alpha/integration_test/user_test.go index 95e98b1a9e..34d12446c5 100644 --- a/internal/api/grpc/resources/user/v3alpha/integration_test/user_test.go +++ b/internal/api/grpc/resources/user/v3alpha/integration_test/user_test.go @@ -224,6 +224,7 @@ func TestServer_CreateUser(t *testing.T) { if tt.res.returnCodePhone { require.NotNil(t, got.PhoneCode) } + }) } } @@ -629,10 +630,10 @@ func TestServer_PatchUser(t *testing.T) { } got, err := instance.Client.UserV3Alpha.PatchUser(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.res.want, got.Details) if tt.res.returnCodeEmail { assert.NotNil(t, got.EmailCode) @@ -848,10 +849,10 @@ func TestServer_DeleteUser(t *testing.T) { } got, err := instance.Client.UserV3Alpha.DeleteUser(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.want, got.Details) }) } @@ -1059,10 +1060,10 @@ func TestServer_LockUser(t *testing.T) { } got, err := instance.Client.UserV3Alpha.LockUser(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.want, got.Details) }) } @@ -1242,10 +1243,10 @@ func TestServer_UnlockUser(t *testing.T) { } got, err := instance.Client.UserV3Alpha.UnlockUser(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.want, got.Details) }) } @@ -1444,10 +1445,10 @@ func TestServer_DeactivateUser(t *testing.T) { } got, err := instance.Client.UserV3Alpha.DeactivateUser(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.want, got.Details) }) } @@ -1627,10 +1628,10 @@ func TestServer_ActivateUser(t *testing.T) { } got, err := instance.Client.UserV3Alpha.ActivateUser(tt.ctx, tt.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } - assert.NoError(t, err) + require.NoError(t, err) integration.AssertResourceDetails(t, tt.want, got.Details) }) } diff --git a/internal/api/grpc/resources/userschema/v3alpha/integration_test/query_test.go b/internal/api/grpc/resources/userschema/v3alpha/integration_test/query_test.go index 36cf032660..436af3bc6f 100644 --- a/internal/api/grpc/resources/userschema/v3alpha/integration_test/query_test.go +++ b/internal/api/grpc/resources/userschema/v3alpha/integration_test/query_test.go @@ -188,31 +188,26 @@ func TestServer_ListUserSchemas(t *testing.T) { require.NoError(t, err) } - retryDuration := 20 * time.Second - if ctxDeadline, ok := isolatedIAMOwnerCTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } - + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 20*time.Second) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := instance.Client.UserSchemaV3.SearchUserSchemas(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(ttt, err) return } - assert.NoError(ttt, err) - + require.NoError(ttt, err) // always first check length, otherwise its failed anyway - assert.Len(ttt, got.Result, len(tt.want.Result)) + require.Len(ttt, got.Result, len(tt.want.Result)) for i := range tt.want.Result { - want := tt.want.Result[i] - got := got.Result[i] + wantSchema := tt.want.Result[i] + gotSchema := got.Result[i] - integration.AssertResourceDetails(t, want.GetDetails(), got.GetDetails()) - want.Details = got.Details - grpc.AllFieldsEqual(t, want.ProtoReflect(), got.ProtoReflect(), grpc.CustomMappers) + integration.AssertResourceDetails(ttt, wantSchema.GetDetails(), gotSchema.GetDetails()) + wantSchema.Details = gotSchema.GetDetails() + grpc.AllFieldsEqual(ttt, wantSchema.ProtoReflect(), gotSchema.ProtoReflect(), grpc.CustomMappers) } - integration.AssertListDetails(t, tt.want, got) - }, retryDuration, time.Millisecond*100, "timeout waiting for expected user schema result") + integration.AssertListDetails(ttt, tt.want, got) + }, retryDuration, tick, "timeout waiting for expected user schema result") }) } } @@ -300,24 +295,21 @@ func TestServer_GetUserSchema(t *testing.T) { require.NoError(t, err) } - retryDuration := 5 * time.Second - if ctxDeadline, ok := isolatedIAMOwnerCTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } - + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 5*time.Second) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := instance.Client.UserSchemaV3.GetUserSchema(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err, "Error: "+err.Error()) - } else { - assert.NoError(t, err) - wantSchema := tt.want.GetUserSchema() - gotSchema := got.GetUserSchema() - integration.AssertResourceDetails(t, wantSchema.GetDetails(), gotSchema.GetDetails()) - tt.want.UserSchema.Details = got.GetUserSchema().GetDetails() - grpc.AllFieldsEqual(t, tt.want.ProtoReflect(), got.ProtoReflect(), grpc.CustomMappers) + require.Error(ttt, err, "Error: "+err.Error()) + return } - }, retryDuration, time.Millisecond*100, "timeout waiting for expected user schema result") + require.NoError(ttt, err) + + wantSchema := tt.want.GetUserSchema() + gotSchema := got.GetUserSchema() + integration.AssertResourceDetails(ttt, wantSchema.GetDetails(), gotSchema.GetDetails()) + wantSchema.Details = got.GetUserSchema().GetDetails() + grpc.AllFieldsEqual(ttt, wantSchema.ProtoReflect(), gotSchema.ProtoReflect(), grpc.CustomMappers) + }, retryDuration, tick, "timeout waiting for expected user schema result") }) } } diff --git a/internal/api/grpc/resources/userschema/v3alpha/integration_test/server_test.go b/internal/api/grpc/resources/userschema/v3alpha/integration_test/server_test.go index c562f9613a..3bab8d8c05 100644 --- a/internal/api/grpc/resources/userschema/v3alpha/integration_test/server_test.go +++ b/internal/api/grpc/resources/userschema/v3alpha/integration_test/server_test.go @@ -43,10 +43,8 @@ func ensureFeatureEnabled(t *testing.T, instance *integration.Instance) { UserSchema: gu.Ptr(true), }) require.NoError(t, err) - retryDuration := time.Minute - if ctxDeadline, ok := ctx.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, 5*time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { f, err := instance.Client.FeatureV2.GetInstanceFeatures(ctx, &feature.GetInstanceFeaturesRequest{ @@ -58,15 +56,16 @@ func ensureFeatureEnabled(t *testing.T, instance *integration.Instance) { } }, retryDuration, - time.Second, + tick, "timed out waiting for ensuring instance feature") + retryDuration, tick = integration.WaitForAndTickWithMaxDuration(ctx, 5*time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { _, err := instance.Client.UserSchemaV3.SearchUserSchemas(ctx, &schema.SearchUserSchemasRequest{}) assert.NoError(ttt, err) }, retryDuration, - time.Second, + tick, "timed out waiting for ensuring instance feature call") } diff --git a/internal/api/grpc/resources/webkey/v3/integration_test/webkey_integration_test.go b/internal/api/grpc/resources/webkey/v3/integration_test/webkey_integration_test.go index a545f1fb06..2a178ea285 100644 --- a/internal/api/grpc/resources/webkey/v3/integration_test/webkey_integration_test.go +++ b/internal/api/grpc/resources/webkey/v3/integration_test/webkey_integration_test.go @@ -191,6 +191,8 @@ func createInstance(t *testing.T, enableFeature bool) (*integration.Instance, co }) require.NoError(t, err) } + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(iamCTX, time.Minute) assert.EventuallyWithT(t, func(collect *assert.CollectT) { resp, err := instance.Client.WebKeyV3Alpha.ListWebKeys(iamCTX, &webkey.ListWebKeysRequest{}) if enableFeature { @@ -199,7 +201,7 @@ func createInstance(t *testing.T, enableFeature bool) (*integration.Instance, co } else { assert.Error(collect, err) } - }, time.Minute, time.Second) + }, retryDuration, tick) return instance, iamCTX } @@ -213,6 +215,8 @@ func assertFeatureDisabledError(t *testing.T, err error) { } func checkWebKeyListState(ctx context.Context, t *testing.T, instance *integration.Instance, nKeys int, expectActiveKeyID string, config any) { + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) assert.EventuallyWithT(t, func(collect *assert.CollectT) { resp, err := instance.Client.WebKeyV3Alpha.ListWebKeys(ctx, &webkey.ListWebKeysRequest{}) require.NoError(collect, err) @@ -243,5 +247,5 @@ func checkWebKeyListState(ctx context.Context, t *testing.T, instance *integrati if expectActiveKeyID != "" { assert.Equal(collect, expectActiveKeyID, gotActiveKeyID) } - }, time.Minute, time.Second) + }, retryDuration, tick) } diff --git a/internal/api/grpc/session/v2/integration_test/session_test.go b/internal/api/grpc/session/v2/integration_test/session_test.go index 32aa1adf55..ccd08f3471 100644 --- a/internal/api/grpc/session/v2/integration_test/session_test.go +++ b/internal/api/grpc/session/v2/integration_test/session_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/pquerna/otp/totp" "github.com/stretchr/testify/assert" @@ -348,8 +349,8 @@ func TestServer_CreateSession(t *testing.T) { func TestServer_CreateSession_lock_user(t *testing.T) { // create a separate org so we don't interfere with any other test org := Instance.CreateOrganization(IAMOwnerCTX, - fmt.Sprintf("TestServer_CreateSession_lock_user_%d", time.Now().UnixNano()), - fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()), + fmt.Sprintf("TestServer_CreateSession_lock_user_%s", gofakeit.AppName()), + gofakeit.Email(), ) userID := org.CreatedAdmins[0].GetUserId() Instance.SetUserPassword(IAMOwnerCTX, userID, integration.UserPassword, false) diff --git a/internal/api/grpc/session/v2beta/integration_test/session_test.go b/internal/api/grpc/session/v2beta/integration_test/session_test.go index 11ed9f7ed4..52e355204d 100644 --- a/internal/api/grpc/session/v2beta/integration_test/session_test.go +++ b/internal/api/grpc/session/v2beta/integration_test/session_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/pquerna/otp/totp" "github.com/stretchr/testify/assert" @@ -348,8 +349,8 @@ func TestServer_CreateSession(t *testing.T) { func TestServer_CreateSession_lock_user(t *testing.T) { // create a separate org so we don't interfere with any other test org := Instance.CreateOrganization(IAMOwnerCTX, - fmt.Sprintf("TestServer_CreateSession_lock_user_%d", time.Now().UnixNano()), - fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()), + fmt.Sprintf("TestServer_CreateSession_lock_user_%s", gofakeit.AppName()), + gofakeit.Email(), ) userID := org.CreatedAdmins[0].GetUserId() Instance.SetUserPassword(IAMOwnerCTX, userID, integration.UserPassword, false) diff --git a/internal/api/grpc/settings/v2/integration_test/settings_test.go b/internal/api/grpc/settings/v2/integration_test/settings_test.go index 99af36a004..8ae576d104 100644 --- a/internal/api/grpc/settings/v2/integration_test/settings_test.go +++ b/internal/api/grpc/settings/v2/integration_test/settings_test.go @@ -53,10 +53,11 @@ func TestServer_GetSecuritySettings(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.ctx, 20*time.Second) assert.EventuallyWithT(t, func(ct *assert.CollectT) { resp, err := Client.GetSecuritySettings(tt.ctx, &settings.GetSecuritySettingsRequest{}) if tt.wantErr { - assert.Error(ct, err) + require.Error(ct, err) return } require.NoError(ct, err) @@ -64,7 +65,7 @@ func TestServer_GetSecuritySettings(t *testing.T) { assert.Equal(ct, want.GetEmbeddedIframe().GetEnabled(), got.GetEmbeddedIframe().GetEnabled(), "enable iframe embedding") assert.Equal(ct, want.GetEmbeddedIframe().GetAllowedOrigins(), got.GetEmbeddedIframe().GetAllowedOrigins(), "allowed origins") assert.Equal(ct, want.GetEnableImpersonation(), got.GetEnableImpersonation(), "enable impersonation") - }, time.Minute, time.Second/10) + }, retryDuration, tick) }) } } @@ -167,7 +168,7 @@ func TestServer_SetSecuritySettings(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.SetSecuritySettings(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) diff --git a/internal/api/grpc/settings/v2beta/integration_test/settings_test.go b/internal/api/grpc/settings/v2beta/integration_test/settings_test.go index d53dec1a1c..9f6968f5e0 100644 --- a/internal/api/grpc/settings/v2beta/integration_test/settings_test.go +++ b/internal/api/grpc/settings/v2beta/integration_test/settings_test.go @@ -53,10 +53,11 @@ func TestServer_GetSecuritySettings(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.ctx, time.Minute) assert.EventuallyWithT(t, func(ct *assert.CollectT) { resp, err := Client.GetSecuritySettings(tt.ctx, &settings.GetSecuritySettingsRequest{}) if tt.wantErr { - assert.Error(ct, err) + require.Error(ct, err) return } require.NoError(ct, err) @@ -64,7 +65,7 @@ func TestServer_GetSecuritySettings(t *testing.T) { assert.Equal(ct, want.GetEmbeddedIframe().GetEnabled(), got.GetEmbeddedIframe().GetEnabled(), "enable iframe embedding") assert.Equal(ct, want.GetEmbeddedIframe().GetAllowedOrigins(), got.GetEmbeddedIframe().GetAllowedOrigins(), "allowed origins") assert.Equal(ct, want.GetEnableImpersonation(), got.GetEnableImpersonation(), "enable impersonation") - }, time.Minute, time.Second/10) + }, retryDuration, tick) }) } } @@ -167,7 +168,7 @@ func TestServer_SetSecuritySettings(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := Client.SetSecuritySettings(tt.args.ctx, tt.args.req) if tt.wantErr { - assert.Error(t, err) + require.Error(t, err) return } require.NoError(t, err) diff --git a/internal/api/grpc/system/integration_test/instance_test.go b/internal/api/grpc/system/integration_test/instance_test.go index c90201a00b..18d2f68bb9 100644 --- a/internal/api/grpc/system/integration_test/instance_test.go +++ b/internal/api/grpc/system/integration_test/instance_test.go @@ -104,7 +104,7 @@ func TestServer_ListInstances(t *testing.T) { } require.NoError(t, err) got := resp.GetResult() - assert.Len(t, got, len(tt.want)) + require.Len(t, got, len(tt.want)) for i := 0; i < len(tt.want); i++ { assert.Equalf(t, tt.want[i].GetId(), got[i].GetId(), "instance[%d] id", i) } diff --git a/internal/api/grpc/system/integration_test/limits_block_test.go b/internal/api/grpc/system/integration_test/limits_block_test.go index b8d17a1167..46b213f603 100644 --- a/internal/api/grpc/system/integration_test/limits_block_test.go +++ b/internal/api/grpc/system/integration_test/limits_block_test.go @@ -140,13 +140,13 @@ func TestServer_Limits_Block(t *testing.T) { InstanceId: isoInstance.ID(), Block: gu.Ptr(true), }) - require.NoError(t, err) + assert.NoError(t, err) // The following call ensures that an undefined bool is not deserialized to false _, err = integration.SystemClient().SetLimits(CTX, &system.SetLimitsRequest{ InstanceId: isoInstance.ID(), AuditLogRetention: durationpb.New(time.Hour), }) - require.NoError(t, err) + assert.NoError(t, err) for _, tt := range tests { var isFirst bool t.Run(tt.name+" with blocking", func(t *testing.T) { 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 cbb82d2636..5092dbf40d 100644 --- a/internal/api/grpc/user/v2/integration_test/email_test.go +++ b/internal/api/grpc/user/v2/integration_test/email_test.go @@ -3,10 +3,9 @@ package user_test import ( - "fmt" "testing" - "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -134,12 +133,15 @@ func TestServer_SetEmail(t *testing.T) { got, err := Client.SetEmail(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(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()) } }) } @@ -149,7 +151,7 @@ func TestServer_ResendEmailCode(t *testing.T) { t.Parallel() userID := Instance.CreateHumanUser(CTX).GetUserId() - verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())).GetUserId() + verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email()).GetUserId() tests := []struct { name string @@ -237,12 +239,15 @@ func TestServer_ResendEmailCode(t *testing.T) { got, err := Client.ResendEmailCode(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(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()) } }) } @@ -294,9 +299,9 @@ func TestServer_VerifyEmail(t *testing.T) { got, err := Client.VerifyEmail(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } 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 cee42c2ae6..e9022f31f8 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 @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc/metadata" @@ -91,10 +92,9 @@ func TestServer_AddIDPLink(t *testing.T) { got, err := Client.AddIDPLink(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } - + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -103,20 +103,20 @@ func TestServer_AddIDPLink(t *testing.T) { func TestServer_ListIDPLinks(t *testing.T) { t.Parallel() - orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListIDPLinks%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + 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, fmt.Sprintf("%d@listidplinks.com", time.Now().UnixNano())) + userInstanceResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email()) _, 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, fmt.Sprintf("%d@listidplinks.com", time.Now().UnixNano())) + userOrgResp := Instance.CreateHumanUserVerified(ctxOrg, orgResp.OrganizationId, gofakeit.Email()) _, err = Instance.CreateUserIDPlink(ctxOrg, userOrgResp.GetUserId(), "external_org", orgIdpResp.Id, "externalUsername_org") require.NoError(t, err) - userMultipleResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, fmt.Sprintf("%d@listidplinks.com", time.Now().UnixNano())) + userMultipleResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email()) _, 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") @@ -236,27 +236,21 @@ func TestServer_ListIDPLinks(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - retryDuration := time.Minute - if ctxDeadline, ok := CTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { - got, listErr := Client.ListIDPLinks(tt.args.ctx, tt.args.req) - assertErr := assert.NoError + got, err := Client.ListIDPLinks(tt.args.ctx, tt.args.req) if tt.wantErr { - assertErr = assert.Error - } - assertErr(ttt, listErr) - if listErr != nil { + require.Error(ttt, err) return } + require.NoError(ttt, err) // always first check length, otherwise its failed anyway - assert.Len(ttt, got.Result, len(tt.want.Result)) + require.Len(ttt, got.Result, len(tt.want.Result)) for i := range tt.want.Result { assert.Contains(ttt, got.Result, tt.want.Result[i]) } integration.AssertListDetails(t, tt.want, got) - }, retryDuration, time.Millisecond*100, "timeout waiting for expected idplinks result") + }, retryDuration, tick, "timeout waiting for expected idplinks result") }) } } @@ -264,20 +258,20 @@ func TestServer_ListIDPLinks(t *testing.T) { func TestServer_RemoveIDPLink(t *testing.T) { t.Parallel() - orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListIDPLinks%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + 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, fmt.Sprintf("%d@listidplinks.com", time.Now().UnixNano())) + userInstanceResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email()) _, 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, fmt.Sprintf("%d@listidplinks.com", time.Now().UnixNano())) + userOrgResp := Instance.CreateHumanUserVerified(ctxOrg, orgResp.OrganizationId, gofakeit.Email()) _, err = Instance.CreateUserIDPlink(ctxOrg, userOrgResp.GetUserId(), "external_org", orgIdpResp.Id, "externalUsername_org") require.NoError(t, err) - userNoLinkResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, fmt.Sprintf("%d@listidplinks.com", time.Now().UnixNano())) + userNoLinkResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, gofakeit.Email()) type args struct { ctx context.Context @@ -363,9 +357,9 @@ func TestServer_RemoveIDPLink(t *testing.T) { got, err := Client.RemoveIDPLink(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) diff --git a/internal/api/grpc/user/v2/integration_test/passkey_test.go b/internal/api/grpc/user/v2/integration_test/passkey_test.go index bba548dfca..585e5ba413 100644 --- a/internal/api/grpc/user/v2/integration_test/passkey_test.go +++ b/internal/api/grpc/user/v2/integration_test/passkey_test.go @@ -584,27 +584,21 @@ func TestServer_ListPasskeys(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - retryDuration := time.Minute - if ctxDeadline, ok := CTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { - got, listErr := Client.ListPasskeys(tt.args.ctx, tt.args.req) - assertErr := assert.NoError + got, err := Client.ListPasskeys(tt.args.ctx, tt.args.req) if tt.wantErr { - assertErr = assert.Error - } - assertErr(ttt, listErr) - if listErr != nil { + require.Error(ttt, err) return } + require.NoError(ttt, err) // always first check length, otherwise its failed anyway assert.Len(ttt, got.Result, len(tt.want.Result)) for i := range tt.want.Result { assert.Contains(ttt, got.Result, tt.want.Result[i]) } - integration.AssertListDetails(t, tt.want, got) - }, retryDuration, time.Millisecond*100, "timeout waiting for expected idplinks result") + integration.AssertListDetails(ttt, tt.want, got) + }, retryDuration, tick, "timeout waiting for expected idplinks result") }) } } diff --git a/internal/api/grpc/user/v2/integration_test/password_test.go b/internal/api/grpc/user/v2/integration_test/password_test.go index 00ac0bda8d..7707537653 100644 --- a/internal/api/grpc/user/v2/integration_test/password_test.go +++ b/internal/api/grpc/user/v2/integration_test/password_test.go @@ -94,9 +94,10 @@ func TestServer_RequestPasswordReset(t *testing.T) { got, err := Client.PasswordReset(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + integration.AssertDetails(t, tt.want, got) if tt.want.GetVerificationCode() != "" { assert.NotEmpty(t, got.GetVerificationCode()) 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 f757b0acb4..47590b6d67 100644 --- a/internal/api/grpc/user/v2/integration_test/phone_test.go +++ b/internal/api/grpc/user/v2/integration_test/phone_test.go @@ -4,10 +4,9 @@ package user_test import ( "context" - "fmt" "testing" - "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -112,9 +111,10 @@ func TestServer_SetPhone(t *testing.T) { got, err := Client.SetPhone(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + integration.AssertDetails(t, tt.want, got) if tt.want.GetVerificationCode() != "" { assert.NotEmpty(t, got.GetVerificationCode()) @@ -127,7 +127,7 @@ func TestServer_ResendPhoneCode(t *testing.T) { t.Parallel() userID := Instance.CreateHumanUser(CTX).GetUserId() - verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())).GetUserId() + verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email()).GetUserId() tests := []struct { name string @@ -188,9 +188,10 @@ func TestServer_ResendPhoneCode(t *testing.T) { got, err := Client.ResendPhoneCode(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + integration.AssertDetails(t, tt.want, got) if tt.want.GetVerificationCode() != "" { assert.NotEmpty(t, got.GetVerificationCode()) @@ -245,9 +246,10 @@ func TestServer_VerifyPhone(t *testing.T) { got, err := Client.VerifyPhone(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + integration.AssertDetails(t, tt.want, got) }) } @@ -340,12 +342,12 @@ func TestServer_RemovePhone(t *testing.T) { require.NoError(t, depErr) got, err := Client.RemovePhone(tt.ctx, tt.req) - if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + integration.AssertDetails(t, tt.want, got) }) } 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 b4d332e6c5..fc2104d62e 100644 --- a/internal/api/grpc/user/v2/integration_test/query_test.go +++ b/internal/api/grpc/user/v2/integration_test/query_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -21,7 +22,7 @@ import ( func TestServer_GetUserByID(t *testing.T) { t.Parallel() - orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("GetUserByIDOrg%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("GetUserByIDOrg-%s", gofakeit.AppName()), gofakeit.Email()) type args struct { ctx context.Context req *user.GetUserByIDRequest @@ -153,23 +154,19 @@ func TestServer_GetUserByID(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - username := fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()) + username := gofakeit.Email() userAttr, err := tt.args.dep(tt.args.ctx, username, tt.args.req) require.NoError(t, err) - retryDuration := time.Minute - if ctxDeadline, ok := CTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { - got, getErr := Client.GetUserByID(tt.args.ctx, tt.args.req) - assertErr := assert.NoError + got, err := Client.GetUserByID(tt.args.ctx, tt.args.req) if tt.wantErr { - assertErr = assert.Error - } - assertErr(ttt, getErr) - if getErr != nil { + require.Error(ttt, err) return } + require.NoError(ttt, err) + tt.want.User.Details = userAttr.Details tt.want.User.UserId = userAttr.UserID tt.want.User.Username = userAttr.Username @@ -183,7 +180,7 @@ func TestServer_GetUserByID(t *testing.T) { } assert.Equal(ttt, tt.want.User, got.User) integration.AssertDetails(ttt, tt.want, got) - }, retryDuration, time.Second) + }, retryDuration, tick) }) } } @@ -192,8 +189,8 @@ func TestServer_GetUserByID_Permission(t *testing.T) { t.Parallel() timeNow := time.Now().UTC() - newOrgOwnerEmail := fmt.Sprintf("%d@permission.get.com", timeNow.UnixNano()) - newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("GetHuman%d", time.Now().UnixNano()), newOrgOwnerEmail) + newOrgOwnerEmail := gofakeit.Email() + newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("GetHuman-%s", gofakeit.AppName()), newOrgOwnerEmail) newUserID := newOrg.CreatedAdmins[0].GetUserId() type args struct { ctx context.Context @@ -307,20 +304,21 @@ func TestServer_GetUserByID_Permission(t *testing.T) { got, err := Client.GetUserByID(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) - tt.want.User.UserId = tt.args.req.GetUserId() - tt.want.User.Username = newOrgOwnerEmail - tt.want.User.PreferredLoginName = newOrgOwnerEmail - tt.want.User.LoginNames = []string{newOrgOwnerEmail} - if human := tt.want.User.GetHuman(); human != nil { - human.Email.Email = newOrgOwnerEmail - } - // details tested in GetUserByID - tt.want.User.Details = got.User.GetDetails() - - assert.Equal(t, tt.want.User, got.User) + return } + require.NoError(t, err) + + tt.want.User.UserId = tt.args.req.GetUserId() + tt.want.User.Username = newOrgOwnerEmail + tt.want.User.PreferredLoginName = newOrgOwnerEmail + tt.want.User.LoginNames = []string{newOrgOwnerEmail} + if human := tt.want.User.GetHuman(); human != nil { + human.Email.Email = newOrgOwnerEmail + } + // details tested in GetUserByID + tt.want.User.Details = got.User.GetDetails() + + assert.Equal(t, tt.want.User, got.User) }) } } @@ -335,8 +333,8 @@ type userAttr struct { func TestServer_ListUsers(t *testing.T) { t.Parallel() - orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListUsersOrg%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) - userResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, fmt.Sprintf("%d@listusers.com", time.Now().UnixNano())) + 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 @@ -806,7 +804,7 @@ func TestServer_ListUsers(t *testing.T) { 3, &user.ListUsersRequest{}, func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - orgResp := Instance.CreateOrganization(ctx, fmt.Sprintf("ListUsersResourceowner%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + orgResp := Instance.CreateOrganization(ctx, fmt.Sprintf("ListUsersResourceowner-%s", gofakeit.AppName()), gofakeit.Email()) infos := make([]userAttr, len(usernames)) for i, username := range usernames { @@ -897,28 +895,24 @@ func TestServer_ListUsers(t *testing.T) { t.Run(tt.name, func(t *testing.T) { usernames := make([]string, tt.args.count) for i := 0; i < tt.args.count; i++ { - usernames[i] = fmt.Sprintf("%d%d@mouse.com", time.Now().UnixNano(), i) + usernames[i] = gofakeit.Email() } infos, err := tt.args.dep(tt.args.ctx, usernames, tt.args.req) require.NoError(t, err) - retryDuration := time.Minute - if ctxDeadline, ok := CTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { - got, listErr := Client.ListUsers(tt.args.ctx, tt.args.req) - assertErr := assert.NoError + got, err := Client.ListUsers(tt.args.ctx, tt.args.req) if tt.wantErr { - assertErr = assert.Error - } - assertErr(ttt, listErr) - if listErr != nil { + require.Error(ttt, err) return } + require.NoError(ttt, err) + // always only give back dependency infos which are required for the response - assert.Len(ttt, tt.want.Result, len(infos)) + require.Len(ttt, tt.want.Result, len(infos)) // always first check length, otherwise its failed anyway - assert.Len(ttt, got.Result, len(tt.want.Result)) + require.Len(ttt, got.Result, len(tt.want.Result)) // totalResult is unrelated to the tests here so gets carried over, can vary from the count of results due to permissions tt.want.Details.TotalResult = got.Details.TotalResult @@ -941,7 +935,7 @@ func TestServer_ListUsers(t *testing.T) { assert.Contains(ttt, got.Result, tt.want.Result[i]) } integration.AssertListDetails(ttt, tt.want, got) - }, retryDuration, time.Millisecond*100, "timeout waiting for expected user result") + }, retryDuration, tick, "timeout waiting for expected user result") }) } } 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 8790030b54..3ebe760d91 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,7 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -659,16 +660,20 @@ func TestServer_AddHumanUser(t *testing.T) { got, err := Client.AddHumanUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) assert.Equal(t, tt.want.GetUserId(), got.GetUserId()) if tt.want.GetEmailCode() != "" { assert.NotEmpty(t, got.GetEmailCode()) + } else { + assert.Empty(t, got.GetEmailCode()) } if tt.want.GetPhoneCode() != "" { assert.NotEmpty(t, got.GetPhoneCode()) + } else { + assert.Empty(t, got.GetPhoneCode()) } integration.AssertDetails(t, tt.want, got) }) @@ -678,8 +683,8 @@ func TestServer_AddHumanUser(t *testing.T) { func TestServer_AddHumanUser_Permission(t *testing.T) { t.Parallel() - newOrgOwnerEmail := fmt.Sprintf("%d@permission.com", time.Now().UnixNano()) - newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("AddHuman%d", time.Now().UnixNano()), newOrgOwnerEmail) + newOrgOwnerEmail := gofakeit.Email() + newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("AddHuman-%s", gofakeit.AppName()), newOrgOwnerEmail) type args struct { ctx context.Context req *user.AddHumanUserRequest @@ -860,9 +865,9 @@ func TestServer_AddHumanUser_Permission(t *testing.T) { got, err := Client.AddHumanUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) assert.Equal(t, tt.want.GetUserId(), got.GetUserId()) integration.AssertDetails(t, tt.want, got) @@ -908,7 +913,7 @@ func TestServer_UpdateHumanUser(t *testing.T) { args: args{ CTX, &user.UpdateHumanUserRequest{ - Username: gu.Ptr(fmt.Sprint(time.Now().UnixNano() + 1)), + Username: gu.Ptr(gofakeit.Username()), }, }, want: &user.UpdateHumanUserResponse{ @@ -1214,14 +1219,19 @@ func TestServer_UpdateHumanUser(t *testing.T) { got, err := Client.UpdateHumanUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + if tt.want.GetEmailCode() != "" { assert.NotEmpty(t, got.GetEmailCode()) + } else { + assert.Empty(t, got.GetEmailCode()) } if tt.want.GetPhoneCode() != "" { assert.NotEmpty(t, got.GetPhoneCode()) + } else { + assert.Empty(t, got.GetPhoneCode()) } integration.AssertDetails(t, tt.want, got) }) @@ -1231,8 +1241,8 @@ func TestServer_UpdateHumanUser(t *testing.T) { func TestServer_UpdateHumanUser_Permission(t *testing.T) { t.Parallel() - newOrgOwnerEmail := fmt.Sprintf("%d@permission.update.com", time.Now().UnixNano()) - newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("UpdateHuman%d", time.Now().UnixNano()), newOrgOwnerEmail) + newOrgOwnerEmail := gofakeit.Email() + newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("UpdateHuman-%s", gofakeit.AppName()), newOrgOwnerEmail) newUserID := newOrg.CreatedAdmins[0].GetUserId() type args struct { ctx context.Context @@ -1250,7 +1260,7 @@ func TestServer_UpdateHumanUser_Permission(t *testing.T) { SystemCTX, &user.UpdateHumanUserRequest{ UserId: newUserID, - Username: gu.Ptr(fmt.Sprint("system", time.Now().UnixNano()+1)), + Username: gu.Ptr(gofakeit.Username()), }, }, want: &user.UpdateHumanUserResponse{ @@ -1266,7 +1276,7 @@ func TestServer_UpdateHumanUser_Permission(t *testing.T) { IamCTX, &user.UpdateHumanUserRequest{ UserId: newUserID, - Username: gu.Ptr(fmt.Sprint("instance", time.Now().UnixNano()+1)), + Username: gu.Ptr(gofakeit.Username()), }, }, want: &user.UpdateHumanUserResponse{ @@ -1282,7 +1292,7 @@ func TestServer_UpdateHumanUser_Permission(t *testing.T) { CTX, &user.UpdateHumanUserRequest{ UserId: newUserID, - Username: gu.Ptr(fmt.Sprint("org", time.Now().UnixNano()+1)), + Username: gu.Ptr(gofakeit.Username()), }, }, wantErr: true, @@ -1293,7 +1303,7 @@ func TestServer_UpdateHumanUser_Permission(t *testing.T) { UserCTX, &user.UpdateHumanUserRequest{ UserId: newUserID, - Username: gu.Ptr(fmt.Sprint("user", time.Now().UnixNano()+1)), + Username: gu.Ptr(gofakeit.Username()), }, }, wantErr: true, @@ -1415,9 +1425,9 @@ func TestServer_LockUser(t *testing.T) { got, err := Client.LockUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -1525,9 +1535,9 @@ func TestServer_UnLockUser(t *testing.T) { got, err := Client.UnlockUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -1635,9 +1645,10 @@ func TestServer_DeactivateUser(t *testing.T) { got, err := Client.DeactivateUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + integration.AssertDetails(t, tt.want, got) }) } @@ -1745,9 +1756,9 @@ func TestServer_ReactivateUser(t *testing.T) { got, err := Client.ReactivateUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -1846,9 +1857,9 @@ func TestServer_DeleteUser(t *testing.T) { got, err := Client.DeleteUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -1859,7 +1870,7 @@ func TestServer_StartIdentityProviderIntent(t *testing.T) { idpResp := Instance.AddGenericOAuthProvider(IamCTX, Instance.DefaultOrg.Id) orgIdpResp := Instance.AddOrgGenericOAuthProvider(CTX, Instance.DefaultOrg.Id) - orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("NotDefaultOrg%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("NotDefaultOrg-%s", gofakeit.AppName()), gofakeit.Email()) notDefaultOrgIdpResp := Instance.AddOrgGenericOAuthProvider(IamCTX, orgResp.OrganizationId) samlIdpID := Instance.AddSAMLProvider(IamCTX) samlRedirectIdpID := Instance.AddSAMLRedirectProvider(IamCTX, "") @@ -2092,15 +2103,14 @@ func TestServer_StartIdentityProviderIntent(t *testing.T) { got, err := Client.StartIdentityProviderIntent(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) if tt.want.url != "" { authUrl, err := url.Parse(got.GetAuthUrl()) - assert.NoError(t, err) - - assert.Len(t, authUrl.Query(), len(tt.want.parametersEqual)+len(tt.want.parametersExisting)) + require.NoError(t, err) + require.Len(t, authUrl.Query(), len(tt.want.parametersEqual)+len(tt.want.parametersExisting)) for _, existing := range tt.want.parametersExisting { assert.True(t, authUrl.Query().Has(existing)) @@ -2771,9 +2781,10 @@ func TestServer_CreateInviteCode(t *testing.T) { got, err := Client.CreateInviteCode(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + integration.AssertDetails(t, tt.want, got) if tt.want.GetInviteCode() != "" { assert.NotEmpty(t, got.GetInviteCode()) @@ -2866,9 +2877,10 @@ func TestServer_ResendInviteCode(t *testing.T) { got, err := Client.ResendInviteCode(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + integration.AssertDetails(t, tt.want, got) }) } @@ -2957,9 +2969,9 @@ func TestServer_VerifyInviteCode(t *testing.T) { got, err := Client.VerifyInviteCode(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } 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 073a432078..ebace6daa2 100644 --- a/internal/api/grpc/user/v2beta/integration_test/email_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/email_test.go @@ -3,10 +3,9 @@ package user_test import ( - "fmt" "testing" - "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -134,12 +133,15 @@ func TestServer_SetEmail(t *testing.T) { got, err := Client.SetEmail(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(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()) } }) } @@ -149,7 +151,7 @@ func TestServer_ResendEmailCode(t *testing.T) { t.Parallel() userID := Instance.CreateHumanUser(CTX).GetUserId() - verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())).GetUserId() + verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email()).GetUserId() tests := []struct { name string @@ -237,12 +239,15 @@ func TestServer_ResendEmailCode(t *testing.T) { got, err := Client.ResendEmailCode(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(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()) } }) } @@ -294,9 +299,9 @@ func TestServer_VerifyEmail(t *testing.T) { got, err := Client.VerifyEmail(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } diff --git a/internal/api/grpc/user/v2beta/integration_test/password_test.go b/internal/api/grpc/user/v2beta/integration_test/password_test.go index 254a13a53e..5995f87c7f 100644 --- a/internal/api/grpc/user/v2beta/integration_test/password_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/password_test.go @@ -94,12 +94,15 @@ func TestServer_RequestPasswordReset(t *testing.T) { got, err := Client.PasswordReset(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(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()) } }) } 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 afcbc35c4a..03567f4023 100644 --- a/internal/api/grpc/user/v2beta/integration_test/phone_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/phone_test.go @@ -4,10 +4,9 @@ package user_test import ( "context" - "fmt" "testing" - "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -112,12 +111,15 @@ func TestServer_SetPhone(t *testing.T) { got, err := Client.SetPhone(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(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()) } }) } @@ -127,7 +129,7 @@ func TestServer_ResendPhoneCode(t *testing.T) { t.Parallel() userID := Instance.CreateHumanUser(CTX).GetUserId() - verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())).GetUserId() + verifiedUserID := Instance.CreateHumanUserVerified(CTX, Instance.DefaultOrg.Id, gofakeit.Email()).GetUserId() tests := []struct { name string @@ -188,12 +190,14 @@ func TestServer_ResendPhoneCode(t *testing.T) { got, err := Client.ResendPhoneCode(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(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()) } }) } @@ -245,9 +249,9 @@ func TestServer_VerifyPhone(t *testing.T) { got, err := Client.VerifyPhone(CTX, tt.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -340,12 +344,12 @@ func TestServer_RemovePhone(t *testing.T) { require.NoError(t, depErr) got, err := Client.RemovePhone(tt.ctx, tt.req) - if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + integration.AssertDetails(t, tt.want, got) }) } 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 f7c12fd03a..4ea911727c 100644 --- a/internal/api/grpc/user/v2beta/integration_test/query_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/query_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -30,7 +31,7 @@ func detailsV2ToV2beta(obj *object.Details) *object_v2beta.Details { func TestServer_GetUserByID(t *testing.T) { t.Parallel() - orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("GetUserByIDOrg%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("GetUserByIDOrg-%s", gofakeit.AppName()), gofakeit.Email()) type args struct { ctx context.Context req *user.GetUserByIDRequest @@ -162,23 +163,19 @@ func TestServer_GetUserByID(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - username := fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()) + username := gofakeit.Email() userAttr, err := tt.args.dep(tt.args.ctx, username, tt.args.req) require.NoError(t, err) - retryDuration := time.Minute - if ctxDeadline, ok := CTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { - got, getErr := Client.GetUserByID(tt.args.ctx, tt.args.req) - assertErr := assert.NoError + got, err := Client.GetUserByID(tt.args.ctx, tt.args.req) if tt.wantErr { - assertErr = assert.Error - } - assertErr(ttt, getErr) - if getErr != nil { + require.Error(t, err) return } + require.NoError(t, err) + tt.want.User.Details = detailsV2ToV2beta(userAttr.Details) tt.want.User.UserId = userAttr.UserID tt.want.User.Username = userAttr.Username @@ -192,7 +189,7 @@ func TestServer_GetUserByID(t *testing.T) { } assert.Equal(ttt, tt.want.User, got.User) integration.AssertDetails(t, tt.want, got) - }, retryDuration, time.Second) + }, retryDuration, tick) }) } } @@ -201,8 +198,8 @@ func TestServer_GetUserByID_Permission(t *testing.T) { t.Parallel() timeNow := time.Now().UTC() - newOrgOwnerEmail := fmt.Sprintf("%d@permission.get.com", timeNow.UnixNano()) - newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("GetHuman%d", time.Now().UnixNano()), newOrgOwnerEmail) + newOrgOwnerEmail := gofakeit.Email() + newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("GetHuman-%s", gofakeit.AppName()), newOrgOwnerEmail) newUserID := newOrg.CreatedAdmins[0].GetUserId() type args struct { ctx context.Context @@ -313,11 +310,14 @@ func TestServer_GetUserByID_Permission(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := Client.GetUserByID(tt.args.ctx, tt.args.req) - if tt.wantErr { - require.Error(t, err) - } else { - require.NoError(t, err) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + got, err := Client.GetUserByID(tt.args.ctx, tt.args.req) + if tt.wantErr { + require.Error(ttt, err) + return + } + require.NoError(ttt, err) tt.want.User.UserId = tt.args.req.GetUserId() tt.want.User.Username = newOrgOwnerEmail tt.want.User.PreferredLoginName = newOrgOwnerEmail @@ -328,8 +328,8 @@ func TestServer_GetUserByID_Permission(t *testing.T) { // details tested in GetUserByID tt.want.User.Details = got.User.GetDetails() - assert.Equal(t, tt.want.User, got.User) - } + assert.Equal(ttt, tt.want.User, got.User) + }, retryDuration, tick, "timeout waiting for expected user result") }) } } @@ -344,8 +344,8 @@ type userAttr struct { func TestServer_ListUsers(t *testing.T) { t.Parallel() - orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListUsersOrg%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) - userResp := Instance.CreateHumanUserVerified(IamCTX, orgResp.OrganizationId, fmt.Sprintf("%d@listusers.com", time.Now().UnixNano())) + 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 @@ -815,7 +815,7 @@ func TestServer_ListUsers(t *testing.T) { 3, &user.ListUsersRequest{}, func(ctx context.Context, usernames []string, request *user.ListUsersRequest) ([]userAttr, error) { - orgResp := Instance.CreateOrganization(ctx, fmt.Sprintf("ListUsersResourceowner%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + orgResp := Instance.CreateOrganization(ctx, fmt.Sprintf("ListUsersResourceowner-%s", gofakeit.AppName()), gofakeit.Email()) infos := make([]userAttr, len(usernames)) for i, username := range usernames { @@ -906,28 +906,24 @@ func TestServer_ListUsers(t *testing.T) { t.Run(tt.name, func(t *testing.T) { usernames := make([]string, tt.args.count) for i := 0; i < tt.args.count; i++ { - usernames[i] = fmt.Sprintf("%d%d@mouse.com", time.Now().UnixNano(), i) + usernames[i] = gofakeit.Email() } infos, err := tt.args.dep(tt.args.ctx, usernames, tt.args.req) require.NoError(t, err) - retryDuration := time.Minute - if ctxDeadline, ok := CTX.Deadline(); ok { - retryDuration = time.Until(ctxDeadline) - } + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { - got, listErr := Client.ListUsers(tt.args.ctx, tt.args.req) - assertErr := assert.NoError + got, err := Client.ListUsers(tt.args.ctx, tt.args.req) if tt.wantErr { - assertErr = assert.Error - } - assertErr(ttt, listErr) - if listErr != nil { + require.Error(ttt, err) return } + require.NoError(ttt, err) + // always only give back dependency infos which are required for the response - assert.Len(ttt, tt.want.Result, len(infos)) + require.Len(ttt, tt.want.Result, len(infos)) // always first check length, otherwise its failed anyway - assert.Len(ttt, got.Result, len(tt.want.Result)) + require.Len(ttt, got.Result, len(tt.want.Result)) // fill in userid and username as it is generated // totalResult is unrelated to the tests here so gets carried over, can vary from the count of results due to permissions @@ -949,8 +945,8 @@ func TestServer_ListUsers(t *testing.T) { for i := range tt.want.Result { assert.Contains(ttt, got.Result, tt.want.Result[i]) } - integration.AssertListDetails(t, tt.want, got) - }, retryDuration, time.Millisecond*100, "timeout waiting for expected user result") + integration.AssertListDetails(ttt, tt.want, got) + }, retryDuration, tick, "timeout waiting for expected user result") }) } } diff --git a/internal/api/grpc/user/v2beta/integration_test/user_test.go b/internal/api/grpc/user/v2beta/integration_test/user_test.go index 587d6b5fe3..7b158f0d68 100644 --- a/internal/api/grpc/user/v2beta/integration_test/user_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/user_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -616,16 +617,20 @@ func TestServer_AddHumanUser(t *testing.T) { got, err := Client.AddHumanUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) assert.Equal(t, tt.want.GetUserId(), got.GetUserId()) if tt.want.GetEmailCode() != "" { assert.NotEmpty(t, got.GetEmailCode()) + } else { + assert.Empty(t, got.GetEmailCode()) } if tt.want.GetPhoneCode() != "" { assert.NotEmpty(t, got.GetPhoneCode()) + } else { + assert.Empty(t, got.GetPhoneCode()) } integration.AssertDetails(t, tt.want, got) }) @@ -635,8 +640,7 @@ func TestServer_AddHumanUser(t *testing.T) { func TestServer_AddHumanUser_Permission(t *testing.T) { t.Parallel() - newOrgOwnerEmail := fmt.Sprintf("%d@permission.com", time.Now().UnixNano()) - newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("AddHuman%d", time.Now().UnixNano()), newOrgOwnerEmail) + newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("AddHuman-%s", gofakeit.AppName()), gofakeit.Email()) type args struct { ctx context.Context req *user.AddHumanUserRequest @@ -817,9 +821,9 @@ func TestServer_AddHumanUser_Permission(t *testing.T) { got, err := Client.AddHumanUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) assert.Equal(t, tt.want.GetUserId(), got.GetUserId()) integration.AssertDetails(t, tt.want, got) @@ -865,7 +869,7 @@ func TestServer_UpdateHumanUser(t *testing.T) { args: args{ CTX, &user.UpdateHumanUserRequest{ - Username: gu.Ptr(fmt.Sprint(time.Now().UnixNano() + 1)), + Username: gu.Ptr(gofakeit.Username()), }, }, want: &user.UpdateHumanUserResponse{ @@ -1171,14 +1175,19 @@ func TestServer_UpdateHumanUser(t *testing.T) { got, err := Client.UpdateHumanUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) + if tt.want.GetEmailCode() != "" { assert.NotEmpty(t, got.GetEmailCode()) + } else { + assert.Empty(t, got.GetEmailCode()) } if tt.want.GetPhoneCode() != "" { assert.NotEmpty(t, got.GetPhoneCode()) + } else { + assert.Empty(t, got.GetPhoneCode()) } integration.AssertDetails(t, tt.want, got) }) @@ -1188,8 +1197,7 @@ func TestServer_UpdateHumanUser(t *testing.T) { func TestServer_UpdateHumanUser_Permission(t *testing.T) { t.Parallel() - newOrgOwnerEmail := fmt.Sprintf("%d@permission.update.com", time.Now().UnixNano()) - newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("UpdateHuman%d", time.Now().UnixNano()), newOrgOwnerEmail) + newOrg := Instance.CreateOrganization(IamCTX, fmt.Sprintf("UpdateHuman-%s", gofakeit.AppName()), gofakeit.Email()) newUserID := newOrg.CreatedAdmins[0].GetUserId() type args struct { ctx context.Context @@ -1207,7 +1215,7 @@ func TestServer_UpdateHumanUser_Permission(t *testing.T) { SystemCTX, &user.UpdateHumanUserRequest{ UserId: newUserID, - Username: gu.Ptr(fmt.Sprint("system", time.Now().UnixNano()+1)), + Username: gu.Ptr(gofakeit.Username()), }, }, want: &user.UpdateHumanUserResponse{ @@ -1223,7 +1231,7 @@ func TestServer_UpdateHumanUser_Permission(t *testing.T) { IamCTX, &user.UpdateHumanUserRequest{ UserId: newUserID, - Username: gu.Ptr(fmt.Sprint("instance", time.Now().UnixNano()+1)), + Username: gu.Ptr(gofakeit.Username()), }, }, want: &user.UpdateHumanUserResponse{ @@ -1239,7 +1247,7 @@ func TestServer_UpdateHumanUser_Permission(t *testing.T) { CTX, &user.UpdateHumanUserRequest{ UserId: newUserID, - Username: gu.Ptr(fmt.Sprint("org", time.Now().UnixNano()+1)), + Username: gu.Ptr(gofakeit.Username()), }, }, wantErr: true, @@ -1250,7 +1258,7 @@ func TestServer_UpdateHumanUser_Permission(t *testing.T) { UserCTX, &user.UpdateHumanUserRequest{ UserId: newUserID, - Username: gu.Ptr(fmt.Sprint("user", time.Now().UnixNano()+1)), + Username: gu.Ptr(gofakeit.Username()), }, }, wantErr: true, @@ -1262,9 +1270,9 @@ func TestServer_UpdateHumanUser_Permission(t *testing.T) { got, err := Client.UpdateHumanUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -1482,9 +1490,9 @@ func TestServer_UnLockUser(t *testing.T) { got, err := Client.UnlockUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -1592,9 +1600,9 @@ func TestServer_DeactivateUser(t *testing.T) { got, err := Client.DeactivateUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -1702,9 +1710,9 @@ func TestServer_ReactivateUser(t *testing.T) { got, err := Client.ReactivateUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -1803,9 +1811,9 @@ func TestServer_DeleteUser(t *testing.T) { got, err := Client.DeleteUser(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -1884,10 +1892,9 @@ func TestServer_AddIDPLink(t *testing.T) { got, err := Client.AddIDPLink(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } - + require.NoError(t, err) integration.AssertDetails(t, tt.want, got) }) } @@ -1898,7 +1905,7 @@ func TestServer_StartIdentityProviderIntent(t *testing.T) { idpResp := Instance.AddGenericOAuthProvider(IamCTX, Instance.DefaultOrg.Id) orgIdpID := Instance.AddOrgGenericOAuthProvider(CTX, Instance.DefaultOrg.Id) - orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("NotDefaultOrg%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("NotDefaultOrg-%s", gofakeit.AppName()), gofakeit.Email()) notDefaultOrgIdpID := Instance.AddOrgGenericOAuthProvider(IamCTX, orgResp.OrganizationId) samlIdpID := Instance.AddSAMLProvider(IamCTX) samlRedirectIdpID := Instance.AddSAMLRedirectProvider(IamCTX, "") @@ -2131,15 +2138,14 @@ func TestServer_StartIdentityProviderIntent(t *testing.T) { got, err := Client.StartIdentityProviderIntent(tt.args.ctx, tt.args.req) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) + return } + require.NoError(t, err) if tt.want.url != "" { authUrl, err := url.Parse(got.GetAuthUrl()) - assert.NoError(t, err) - - assert.Len(t, authUrl.Query(), len(tt.want.parametersEqual)+len(tt.want.parametersExisting)) + require.NoError(t, err) + require.Len(t, authUrl.Query(), len(tt.want.parametersEqual)+len(tt.want.parametersExisting)) for _, existing := range tt.want.parametersExisting { assert.True(t, authUrl.Query().Has(existing)) diff --git a/internal/api/idp/integration_test/idp_test.go b/internal/api/idp/integration_test/idp_test.go index 609b98262b..8e7141271a 100644 --- a/internal/api/idp/integration_test/idp_test.go +++ b/internal/api/idp/integration_test/idp_test.go @@ -335,17 +335,18 @@ func TestServer_SAMLACS(t *testing.T) { location, err := integration.CheckPost(callbackURL, httpPostFormRequest(relayState, response)) if tt.wantErr { require.Error(t, err) - } else { - require.NoError(t, err) - assert.Equal(t, relayState, location.Query().Get("id")) - if tt.want.successful { - assert.True(t, strings.HasPrefix(location.String(), tt.args.successURL)) - assert.NotEmpty(t, location.Query().Get("token")) - assert.Equal(t, tt.want.user, location.Query().Get("user")) - } else { - assert.True(t, strings.HasPrefix(location.String(), tt.args.failureURL)) - } + return } + require.NoError(t, err) + assert.Equal(t, relayState, location.Query().Get("id")) + if tt.want.successful { + assert.True(t, strings.HasPrefix(location.String(), tt.args.successURL)) + assert.NotEmpty(t, location.Query().Get("token")) + assert.Equal(t, tt.want.user, location.Query().Get("user")) + } else { + assert.True(t, strings.HasPrefix(location.String(), tt.args.failureURL)) + } + }) } } diff --git a/internal/api/oidc/integration_test/oidc_test.go b/internal/api/oidc/integration_test/oidc_test.go index 016745cf56..86754aab0e 100644 --- a/internal/api/oidc/integration_test/oidc_test.go +++ b/internal/api/oidc/integration_test/oidc_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/zitadel/oidc/v3/pkg/client/rp" @@ -121,7 +122,7 @@ func Test_ZITADEL_API_missing_authentication(t *testing.T) { func Test_ZITADEL_API_missing_mfa_policy(t *testing.T) { clientID, _ := createClient(t, Instance) - org := Instance.CreateOrganization(CTXIAM, fmt.Sprintf("ZITADEL_API_MISSING_MFA_%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + org := Instance.CreateOrganization(CTXIAM, fmt.Sprintf("ZITADEL_API_MISSING_MFA_%s", gofakeit.AppName()), gofakeit.Email()) userID := org.CreatedAdmins[0].GetUserId() Instance.SetUserPassword(CTXIAM, userID, integration.UserPassword, false) authRequestID := createAuthRequest(t, Instance, clientID, redirectURI, oidc.ScopeOpenID, zitadelAudienceScope) diff --git a/internal/api/oidc/integration_test/token_exchange_test.go b/internal/api/oidc/integration_test/token_exchange_test.go index 26c4475024..5b0b86f0ec 100644 --- a/internal/api/oidc/integration_test/token_exchange_test.go +++ b/internal/api/oidc/integration_test/token_exchange_test.go @@ -25,40 +25,71 @@ import ( "github.com/zitadel/zitadel/pkg/grpc/feature/v2" ) -func setTokenExchangeFeature(t *testing.T, value bool) { - iamCTX := Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) +func setTokenExchangeFeature(t *testing.T, instance *integration.Instance, value bool) { + iamCTX := instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) - _, err := Instance.Client.FeatureV2.SetInstanceFeatures(iamCTX, &feature.SetInstanceFeaturesRequest{ + _, err := instance.Client.FeatureV2.SetInstanceFeatures(iamCTX, &feature.SetInstanceFeaturesRequest{ OidcTokenExchange: proto.Bool(value), }) require.NoError(t, err) + retryDuration := time.Minute + if ctxDeadline, ok := iamCTX.Deadline(); ok { + retryDuration = time.Until(ctxDeadline) + } + require.EventuallyWithT(t, + func(ttt *assert.CollectT) { + f, err := instance.Client.FeatureV2.GetInstanceFeatures(iamCTX, &feature.GetInstanceFeaturesRequest{ + Inheritance: true, + }) + assert.NoError(ttt, err) + if f.OidcTokenExchange.GetEnabled() { + return + } + }, + retryDuration, + time.Second, + "timed out waiting for ensuring instance feature") time.Sleep(time.Second) } -func resetFeatures(t *testing.T) { - iamCTX := Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) - _, err := Instance.Client.FeatureV2.ResetInstanceFeatures(iamCTX, &feature.ResetInstanceFeaturesRequest{}) +func resetFeatures(t *testing.T, instance *integration.Instance) { + iamCTX := instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) + _, err := instance.Client.FeatureV2.ResetInstanceFeatures(iamCTX, &feature.ResetInstanceFeaturesRequest{}) require.NoError(t, err) time.Sleep(time.Second) } -func setImpersonationPolicy(t *testing.T, value bool) { - iamCTX := Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) +func setImpersonationPolicy(t *testing.T, instance *integration.Instance, value bool) { + iamCTX := instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) - policy, err := Instance.Client.Admin.GetSecurityPolicy(iamCTX, &admin.GetSecurityPolicyRequest{}) + policy, err := instance.Client.Admin.GetSecurityPolicy(iamCTX, &admin.GetSecurityPolicyRequest{}) require.NoError(t, err) if policy.GetPolicy().GetEnableImpersonation() != value { - _, err = Instance.Client.Admin.SetSecurityPolicy(iamCTX, &admin.SetSecurityPolicyRequest{ + _, err = instance.Client.Admin.SetSecurityPolicy(iamCTX, &admin.SetSecurityPolicyRequest{ EnableImpersonation: value, }) require.NoError(t, err) } - time.Sleep(time.Second) + + retryDuration := time.Minute + if ctxDeadline, ok := iamCTX.Deadline(); ok { + retryDuration = time.Until(ctxDeadline) + } + require.EventuallyWithT(t, + func(ttt *assert.CollectT) { + f, err := instance.Client.Admin.GetSecurityPolicy(iamCTX, &admin.GetSecurityPolicyRequest{}) + assert.NoError(ttt, err) + if f.GetPolicy().GetEnableImpersonation() != value { + return + } + }, + retryDuration, + time.Second, + "timed out waiting for ensuring impersonation policy") } -func createMachineUserPATWithMembership(t *testing.T, roles ...string) (userID, pat string) { - iamCTX := Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) - userID, pat, err := Instance.CreateMachineUserPATWithMembership(iamCTX, roles...) +func createMachineUserPATWithMembership(ctx context.Context, t *testing.T, instance *integration.Instance, roles ...string) (userID, pat string) { + userID, pat, err := instance.CreateMachineUserPATWithMembership(ctx, roles...) require.NoError(t, err) return userID, pat } @@ -114,40 +145,34 @@ func refreshTokenVerifier(ctx context.Context, provider rp.RelyingParty, subject func TestServer_TokenExchange(t *testing.T) { t.Parallel() - t.Cleanup(func() { - resetFeatures(t) - setImpersonationPolicy(t, false) - }) + instance := integration.NewInstance(CTX) + ctx := instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) + userResp := instance.CreateHumanUser(ctx) - client, keyData, err := Instance.CreateOIDCTokenExchangeClient(CTX) + client, keyData, err := instance.CreateOIDCTokenExchangeClient(ctx) require.NoError(t, err) signer, err := rp.SignerFromKeyFile(keyData)() require.NoError(t, err) - exchanger, err := tokenexchange.NewTokenExchangerJWTProfile(CTX, Instance.OIDCIssuer(), client.GetClientId(), signer) + exchanger, err := tokenexchange.NewTokenExchangerJWTProfile(ctx, instance.OIDCIssuer(), client.GetClientId(), signer) require.NoError(t, err) - time.Sleep(time.Second) + _, orgImpersonatorPAT := createMachineUserPATWithMembership(ctx, t, instance, "ORG_ADMIN_IMPERSONATOR") + serviceUserID, noPermPAT := createMachineUserPATWithMembership(ctx, t, instance) - iamUserID, iamImpersonatorPAT := createMachineUserPATWithMembership(t, "IAM_ADMIN_IMPERSONATOR") - orgUserID, orgImpersonatorPAT := createMachineUserPATWithMembership(t, "ORG_ADMIN_IMPERSONATOR") - serviceUserID, noPermPAT := createMachineUserPATWithMembership(t) - - // exchange some tokens for later use - setTokenExchangeFeature(t, true) - teResp, err := tokenexchange.ExchangeToken(CTX, exchanger, noPermPAT, oidc.AccessTokenType, "", "", nil, nil, nil, oidc.AccessTokenType) + // test that feature is disabled per default + teResp, err := tokenexchange.ExchangeToken(ctx, exchanger, noPermPAT, oidc.AccessTokenType, "", "", nil, nil, nil, oidc.AccessTokenType) + require.Error(t, err) + setTokenExchangeFeature(t, instance, true) + teResp, err = tokenexchange.ExchangeToken(ctx, exchanger, noPermPAT, oidc.AccessTokenType, "", "", nil, nil, nil, oidc.AccessTokenType) require.NoError(t, err) patScopes := oidc.SpaceDelimitedArray{"openid", "profile", "urn:zitadel:iam:user:metadata", "urn:zitadel:iam:user:resourceowner"} - relyingParty, err := rp.NewRelyingPartyOIDC(CTX, Instance.OIDCIssuer(), client.GetClientId(), "", "", []string{"openid"}, rp.WithJWTProfile(rp.SignerFromKeyFile(keyData))) + relyingParty, err := rp.NewRelyingPartyOIDC(ctx, instance.OIDCIssuer(), client.GetClientId(), "", "", []string{"openid"}, rp.WithJWTProfile(rp.SignerFromKeyFile(keyData))) require.NoError(t, err) - resourceServer, err := Instance.CreateResourceServerJWTProfile(CTX, keyData) + resourceServer, err := instance.CreateResourceServerJWTProfile(ctx, keyData) require.NoError(t, err) - type settings struct { - tokenExchangeFeature bool - impersonationPolicy bool - } type args struct { SubjectToken string SubjectTokenType oidc.TokenType @@ -168,30 +193,13 @@ func TestServer_TokenExchange(t *testing.T) { verifyIDToken func(t *testing.T, token string) } tests := []struct { - name string - settings settings - args args - want result - wantErr bool + name string + args args + want result + wantErr bool }{ - { - name: "feature disabled error", - settings: settings{ - tokenExchangeFeature: false, - impersonationPolicy: false, - }, - args: args{ - SubjectToken: noPermPAT, - SubjectTokenType: oidc.AccessTokenType, - }, - wantErr: true, - }, { name: "unsupported resource parameter", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ SubjectToken: noPermPAT, SubjectTokenType: oidc.AccessTokenType, @@ -201,10 +209,6 @@ func TestServer_TokenExchange(t *testing.T) { }, { name: "invalid subject token", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ SubjectToken: "foo", SubjectTokenType: oidc.AccessTokenType, @@ -213,10 +217,6 @@ func TestServer_TokenExchange(t *testing.T) { }, { name: "EXCHANGE: access token to default", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ SubjectToken: noPermPAT, SubjectTokenType: oidc.AccessTokenType, @@ -226,16 +226,12 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: oidc.BearerToken, expiresIn: 43100, scopes: patScopes, - verifyAccessToken: accessTokenVerifier(CTX, resourceServer, serviceUserID, ""), - verifyIDToken: idTokenVerifier(CTX, relyingParty, serviceUserID, ""), + verifyAccessToken: accessTokenVerifier(ctx, resourceServer, serviceUserID, ""), + verifyIDToken: idTokenVerifier(ctx, relyingParty, serviceUserID, ""), }, }, { name: "EXCHANGE: access token to access token", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ SubjectToken: noPermPAT, SubjectTokenType: oidc.AccessTokenType, @@ -246,16 +242,12 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: oidc.BearerToken, expiresIn: 43100, scopes: patScopes, - verifyAccessToken: accessTokenVerifier(CTX, resourceServer, serviceUserID, ""), - verifyIDToken: idTokenVerifier(CTX, relyingParty, serviceUserID, ""), + verifyAccessToken: accessTokenVerifier(ctx, resourceServer, serviceUserID, ""), + verifyIDToken: idTokenVerifier(ctx, relyingParty, serviceUserID, ""), }, }, { name: "EXCHANGE: access token to JWT", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ SubjectToken: noPermPAT, SubjectTokenType: oidc.AccessTokenType, @@ -266,16 +258,12 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: oidc.BearerToken, expiresIn: 43100, scopes: patScopes, - verifyAccessToken: accessTokenVerifier(CTX, resourceServer, serviceUserID, ""), - verifyIDToken: idTokenVerifier(CTX, relyingParty, serviceUserID, ""), + verifyAccessToken: accessTokenVerifier(ctx, resourceServer, serviceUserID, ""), + verifyIDToken: idTokenVerifier(ctx, relyingParty, serviceUserID, ""), }, }, { name: "EXCHANGE: access token to ID Token", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ SubjectToken: noPermPAT, SubjectTokenType: oidc.AccessTokenType, @@ -286,7 +274,7 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: "N_A", expiresIn: 43100, scopes: patScopes, - verifyAccessToken: idTokenVerifier(CTX, relyingParty, serviceUserID, ""), + verifyAccessToken: idTokenVerifier(ctx, relyingParty, serviceUserID, ""), verifyIDToken: func(t *testing.T, token string) { assert.Empty(t, token) }, @@ -294,10 +282,6 @@ func TestServer_TokenExchange(t *testing.T) { }, { name: "EXCHANGE: refresh token not allowed", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ SubjectToken: teResp.RefreshToken, SubjectTokenType: oidc.RefreshTokenType, @@ -307,10 +291,6 @@ func TestServer_TokenExchange(t *testing.T) { }, { name: "EXCHANGE: alternate scope for refresh token", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ SubjectToken: noPermPAT, SubjectTokenType: oidc.AccessTokenType, @@ -322,17 +302,13 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: oidc.BearerToken, expiresIn: 43100, scopes: []string{oidc.ScopeOpenID, oidc.ScopeOfflineAccess, "profile"}, - verifyAccessToken: accessTokenVerifier(CTX, resourceServer, serviceUserID, ""), - verifyIDToken: idTokenVerifier(CTX, relyingParty, serviceUserID, ""), - verifyRefreshToken: refreshTokenVerifier(CTX, relyingParty, "", ""), + verifyAccessToken: accessTokenVerifier(ctx, resourceServer, serviceUserID, ""), + verifyIDToken: idTokenVerifier(ctx, relyingParty, serviceUserID, ""), + verifyRefreshToken: refreshTokenVerifier(ctx, relyingParty, "", ""), }, }, { name: "EXCHANGE: access token, requested token type not supported error", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ SubjectToken: noPermPAT, SubjectTokenType: oidc.AccessTokenType, @@ -342,10 +318,6 @@ func TestServer_TokenExchange(t *testing.T) { }, { name: "EXCHANGE: access token, invalid audience", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ SubjectToken: noPermPAT, SubjectTokenType: oidc.AccessTokenType, @@ -356,12 +328,8 @@ func TestServer_TokenExchange(t *testing.T) { }, { name: "IMPERSONATION: subject: userID, actor: access token, policy disabled error", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: false, - }, args: args{ - SubjectToken: User.GetUserId(), + SubjectToken: userResp.GetUserId(), SubjectTokenType: oidc_api.UserIDTokenType, RequestedTokenType: oidc.AccessTokenType, ActorToken: orgImpersonatorPAT, @@ -369,14 +337,94 @@ func TestServer_TokenExchange(t *testing.T) { }, wantErr: true, }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tokenexchange.ExchangeToken(ctx, exchanger, tt.args.SubjectToken, tt.args.SubjectTokenType, tt.args.ActorToken, tt.args.ActorTokenType, tt.args.Resource, tt.args.Audience, tt.args.Scopes, tt.args.RequestedTokenType) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want.issuedTokenType, got.IssuedTokenType) + assert.Equal(t, tt.want.tokenType, got.TokenType) + assert.Greater(t, got.ExpiresIn, tt.want.expiresIn) + assert.Equal(t, tt.want.scopes, got.Scopes) + if tt.want.verifyAccessToken != nil { + tt.want.verifyAccessToken(t, got.AccessToken) + } + if tt.want.verifyRefreshToken != nil { + tt.want.verifyRefreshToken(t, got.RefreshToken) + } + if tt.want.verifyIDToken != nil { + tt.want.verifyIDToken(t, got.IDToken) + } + }) + } +} + +func TestServer_TokenExchangeImpersonation(t *testing.T) { + t.Parallel() + + instance := integration.NewInstance(CTX) + ctx := instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) + userResp := instance.CreateHumanUser(ctx) + + // exchange some tokens for later use + setTokenExchangeFeature(t, instance, true) + setImpersonationPolicy(t, instance, true) + + client, keyData, err := instance.CreateOIDCTokenExchangeClient(ctx) + require.NoError(t, err) + signer, err := rp.SignerFromKeyFile(keyData)() + require.NoError(t, err) + exchanger, err := tokenexchange.NewTokenExchangerJWTProfile(ctx, instance.OIDCIssuer(), client.GetClientId(), signer) + require.NoError(t, err) + + iamUserID, iamImpersonatorPAT := createMachineUserPATWithMembership(ctx, t, instance, "IAM_ADMIN_IMPERSONATOR") + orgUserID, orgImpersonatorPAT := createMachineUserPATWithMembership(ctx, t, instance, "ORG_ADMIN_IMPERSONATOR") + serviceUserID, noPermPAT := createMachineUserPATWithMembership(ctx, t, instance) + + teResp, err := tokenexchange.ExchangeToken(ctx, exchanger, noPermPAT, oidc.AccessTokenType, "", "", nil, nil, nil, oidc.AccessTokenType) + require.NoError(t, err) + + patScopes := oidc.SpaceDelimitedArray{"openid", "profile", "urn:zitadel:iam:user:metadata", "urn:zitadel:iam:user:resourceowner"} + + relyingParty, err := rp.NewRelyingPartyOIDC(ctx, instance.OIDCIssuer(), client.GetClientId(), "", "", []string{"openid"}, rp.WithJWTProfile(rp.SignerFromKeyFile(keyData))) + require.NoError(t, err) + resourceServer, err := instance.CreateResourceServerJWTProfile(ctx, keyData) + require.NoError(t, err) + + type args struct { + SubjectToken string + SubjectTokenType oidc.TokenType + ActorToken string + ActorTokenType oidc.TokenType + Resource []string + Audience []string + Scopes []string + RequestedTokenType oidc.TokenType + } + type result struct { + issuedTokenType oidc.TokenType + tokenType string + expiresIn uint64 + scopes oidc.SpaceDelimitedArray + verifyAccessToken func(t *testing.T, token string) + verifyRefreshToken func(t *testing.T, token string) + verifyIDToken func(t *testing.T, token string) + } + tests := []struct { + name string + args args + want result + wantErr bool + }{ { name: "IMPERSONATION: subject: userID, actor: access token, membership not found error", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: true, - }, args: args{ - SubjectToken: User.GetUserId(), + SubjectToken: userResp.GetUserId(), SubjectTokenType: oidc_api.UserIDTokenType, RequestedTokenType: oidc.AccessTokenType, ActorToken: noPermPAT, @@ -386,12 +434,8 @@ func TestServer_TokenExchange(t *testing.T) { }, { name: "IAM IMPERSONATION: subject: userID, actor: access token, success", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: true, - }, args: args{ - SubjectToken: User.GetUserId(), + SubjectToken: userResp.GetUserId(), SubjectTokenType: oidc_api.UserIDTokenType, RequestedTokenType: oidc.AccessTokenType, ActorToken: iamImpersonatorPAT, @@ -402,18 +446,14 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: oidc.BearerToken, expiresIn: 43100, scopes: patScopes, - verifyAccessToken: accessTokenVerifier(CTX, resourceServer, User.GetUserId(), iamUserID), - verifyIDToken: idTokenVerifier(CTX, relyingParty, User.GetUserId(), iamUserID), + verifyAccessToken: accessTokenVerifier(ctx, resourceServer, userResp.GetUserId(), iamUserID), + verifyIDToken: idTokenVerifier(ctx, relyingParty, userResp.GetUserId(), iamUserID), }, }, { name: "ORG IMPERSONATION: subject: userID, actor: access token, success", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: true, - }, args: args{ - SubjectToken: User.GetUserId(), + SubjectToken: userResp.GetUserId(), SubjectTokenType: oidc_api.UserIDTokenType, RequestedTokenType: oidc.AccessTokenType, ActorToken: orgImpersonatorPAT, @@ -424,16 +464,12 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: oidc.BearerToken, expiresIn: 43100, scopes: patScopes, - verifyAccessToken: accessTokenVerifier(CTX, resourceServer, User.GetUserId(), orgUserID), - verifyIDToken: idTokenVerifier(CTX, relyingParty, User.GetUserId(), orgUserID), + verifyAccessToken: accessTokenVerifier(ctx, resourceServer, userResp.GetUserId(), orgUserID), + verifyIDToken: idTokenVerifier(ctx, relyingParty, userResp.GetUserId(), orgUserID), }, }, { name: "ORG IMPERSONATION: subject: access token, actor: access token, success", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: true, - }, args: args{ SubjectToken: teResp.AccessToken, SubjectTokenType: oidc.AccessTokenType, @@ -446,16 +482,12 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: oidc.BearerToken, expiresIn: 43100, scopes: patScopes, - verifyAccessToken: accessTokenVerifier(CTX, resourceServer, serviceUserID, orgUserID), - verifyIDToken: idTokenVerifier(CTX, relyingParty, serviceUserID, orgUserID), + verifyAccessToken: accessTokenVerifier(ctx, resourceServer, serviceUserID, orgUserID), + verifyIDToken: idTokenVerifier(ctx, relyingParty, serviceUserID, orgUserID), }, }, { name: "ORG IMPERSONATION: subject: ID token, actor: access token, success", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: true, - }, args: args{ SubjectToken: teResp.IDToken, SubjectTokenType: oidc.IDTokenType, @@ -468,22 +500,18 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: oidc.BearerToken, expiresIn: 43100, scopes: patScopes, - verifyAccessToken: accessTokenVerifier(CTX, resourceServer, serviceUserID, orgUserID), - verifyIDToken: idTokenVerifier(CTX, relyingParty, serviceUserID, orgUserID), + verifyAccessToken: accessTokenVerifier(ctx, resourceServer, serviceUserID, orgUserID), + verifyIDToken: idTokenVerifier(ctx, relyingParty, serviceUserID, orgUserID), }, }, { name: "ORG IMPERSONATION: subject: JWT, actor: access token, success", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: true, - }, args: args{ SubjectToken: func() string { token, err := crypto.Sign(&oidc.JWTTokenRequest{ Issuer: client.GetClientId(), - Subject: User.GetUserId(), - Audience: oidc.Audience{Instance.OIDCIssuer()}, + Subject: userResp.GetUserId(), + Audience: oidc.Audience{instance.OIDCIssuer()}, ExpiresAt: oidc.FromTime(time.Now().Add(time.Hour)), IssuedAt: oidc.FromTime(time.Now().Add(-time.Second)), }, signer) @@ -500,16 +528,12 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: oidc.BearerToken, expiresIn: 43100, scopes: patScopes, - verifyAccessToken: accessTokenVerifier(CTX, resourceServer, User.GetUserId(), orgUserID), - verifyIDToken: idTokenVerifier(CTX, relyingParty, User.GetUserId(), orgUserID), + verifyAccessToken: accessTokenVerifier(ctx, resourceServer, userResp.GetUserId(), orgUserID), + verifyIDToken: idTokenVerifier(ctx, relyingParty, userResp.GetUserId(), orgUserID), }, }, { name: "ORG IMPERSONATION: subject: access token, actor: access token, with refresh token, success", - settings: settings{ - tokenExchangeFeature: true, - impersonationPolicy: true, - }, args: args{ SubjectToken: teResp.AccessToken, SubjectTokenType: oidc.AccessTokenType, @@ -523,19 +547,15 @@ func TestServer_TokenExchange(t *testing.T) { tokenType: oidc.BearerToken, expiresIn: 43100, scopes: []string{oidc.ScopeOpenID, oidc.ScopeOfflineAccess}, - verifyAccessToken: accessTokenVerifier(CTX, resourceServer, serviceUserID, orgUserID), - verifyIDToken: idTokenVerifier(CTX, relyingParty, serviceUserID, orgUserID), - verifyRefreshToken: refreshTokenVerifier(CTX, relyingParty, serviceUserID, orgUserID), + verifyAccessToken: accessTokenVerifier(ctx, resourceServer, serviceUserID, orgUserID), + verifyIDToken: idTokenVerifier(ctx, relyingParty, serviceUserID, orgUserID), + verifyRefreshToken: refreshTokenVerifier(ctx, relyingParty, serviceUserID, orgUserID), }, }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - setTokenExchangeFeature(t, tt.settings.tokenExchangeFeature) - setImpersonationPolicy(t, tt.settings.impersonationPolicy) - - got, err := tokenexchange.ExchangeToken(CTX, exchanger, tt.args.SubjectToken, tt.args.SubjectTokenType, tt.args.ActorToken, tt.args.ActorTokenType, tt.args.Resource, tt.args.Audience, tt.args.Scopes, tt.args.RequestedTokenType) + got, err := tokenexchange.ExchangeToken(ctx, exchanger, tt.args.SubjectToken, tt.args.SubjectTokenType, tt.args.ActorToken, tt.args.ActorTokenType, tt.args.Resource, tt.args.Audience, tt.args.Scopes, tt.args.RequestedTokenType) if tt.wantErr { assert.Error(t, err) return @@ -561,32 +581,33 @@ func TestServer_TokenExchange(t *testing.T) { // This test tries to call the zitadel API with an impersonated token, // which should fail. func TestImpersonation_API_Call(t *testing.T) { - client, keyData, err := Instance.CreateOIDCTokenExchangeClient(CTX) + t.Parallel() + + instance := integration.NewInstance(CTX) + ctx := instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) + + client, keyData, err := instance.CreateOIDCTokenExchangeClient(ctx) require.NoError(t, err) signer, err := rp.SignerFromKeyFile(keyData)() require.NoError(t, err) - exchanger, err := tokenexchange.NewTokenExchangerJWTProfile(CTX, Instance.OIDCIssuer(), client.GetClientId(), signer) + exchanger, err := tokenexchange.NewTokenExchangerJWTProfile(ctx, instance.OIDCIssuer(), client.GetClientId(), signer) require.NoError(t, err) - resourceServer, err := Instance.CreateResourceServerJWTProfile(CTX, keyData) + resourceServer, err := instance.CreateResourceServerJWTProfile(ctx, keyData) require.NoError(t, err) - setTokenExchangeFeature(t, true) - setImpersonationPolicy(t, true) - t.Cleanup(func() { - resetFeatures(t) - setImpersonationPolicy(t, false) - }) + setTokenExchangeFeature(t, instance, true) + setImpersonationPolicy(t, instance, true) - iamUserID, iamImpersonatorPAT := createMachineUserPATWithMembership(t, "IAM_ADMIN_IMPERSONATOR") - iamOwner := Instance.Users.Get(integration.UserTypeIAMOwner) + iamUserID, iamImpersonatorPAT := createMachineUserPATWithMembership(ctx, t, instance, "IAM_ADMIN_IMPERSONATOR") + iamOwner := instance.Users.Get(integration.UserTypeIAMOwner) // impersonating the IAM owner! - resp, err := tokenexchange.ExchangeToken(CTX, exchanger, iamOwner.Token, oidc.AccessTokenType, iamImpersonatorPAT, oidc.AccessTokenType, nil, nil, nil, oidc.AccessTokenType) + resp, err := tokenexchange.ExchangeToken(ctx, exchanger, iamOwner.Token, oidc.AccessTokenType, iamImpersonatorPAT, oidc.AccessTokenType, nil, nil, nil, oidc.AccessTokenType) require.NoError(t, err) - accessTokenVerifier(CTX, resourceServer, iamOwner.ID, iamUserID) + accessTokenVerifier(ctx, resourceServer, iamOwner.ID, iamUserID) - impersonatedCTX := integration.WithAuthorizationToken(CTX, resp.AccessToken) - _, err = Instance.Client.Admin.GetAllowedLanguages(impersonatedCTX, &admin.GetAllowedLanguagesRequest{}) + impersonatedCTX := integration.WithAuthorizationToken(ctx, resp.AccessToken) + _, err = instance.Client.Admin.GetAllowedLanguages(impersonatedCTX, &admin.GetAllowedLanguagesRequest{}) status := status.Convert(err) assert.Equal(t, codes.PermissionDenied, status.Code()) assert.Equal(t, "Errors.TokenExchange.Token.NotForAPI (APP-Shi0J)", status.Message()) diff --git a/internal/api/oidc/integration_test/userinfo_test.go b/internal/api/oidc/integration_test/userinfo_test.go index c2ad3be3ec..da1dc6b1e3 100644 --- a/internal/api/oidc/integration_test/userinfo_test.go +++ b/internal/api/oidc/integration_test/userinfo_test.go @@ -6,8 +6,8 @@ import ( "fmt" "strings" "testing" - "time" + "github.com/brianvoe/gofakeit/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/zitadel/oidc/v3/pkg/client/rp" @@ -135,14 +135,14 @@ func testServer_UserInfo(t *testing.T) { prepare: func(t *testing.T, clientID string, scope []string) *oidc.Tokens[*oidc.IDTokenClaims] { _, err := Instance.Client.Mgmt.UpdateProject(CTX, &management.UpdateProjectRequest{ Id: projectID, - Name: fmt.Sprintf("project-%d", time.Now().UnixNano()), + Name: fmt.Sprintf("project-%s", gofakeit.AppName()), ProjectRoleAssertion: true, }) require.NoError(t, err) t.Cleanup(func() { _, err := Instance.Client.Mgmt.UpdateProject(CTX, &management.UpdateProjectRequest{ Id: projectID, - Name: fmt.Sprintf("project-%d", time.Now().UnixNano()), + Name: fmt.Sprintf("project-%s", gofakeit.AppName()), ProjectRoleAssertion: false, }) require.NoError(t, err) @@ -245,7 +245,7 @@ func TestServer_UserInfo_OrgIDRoles(t *testing.T) { _, err := Instance.Client.Mgmt.UpdateProject(CTX, &management.UpdateProjectRequest{ Id: projectID, - Name: fmt.Sprintf("project-%d", time.Now().UnixNano()), + Name: fmt.Sprintf("project-%s", gofakeit.AppName()), ProjectRoleAssertion: true, }) require.NoError(t, err) @@ -356,7 +356,7 @@ func addProjectRolesGrants(t *testing.T, userID, projectID string, roles ...stri // addProjectOrgGrant adds a new organization which will be granted on the projectID with the specified roles. // The userID will be granted in the new organization to the project with the same roles. func addProjectOrgGrant(t *testing.T, userID, projectID string, roles ...string) (grantedOrgID string) { - grantedOrg := Instance.CreateOrganization(CTXIAM, fmt.Sprintf("ZITADEL_GRANTED_%d", time.Now().UnixNano()), fmt.Sprintf("%d@mouse.com", time.Now().UnixNano())) + grantedOrg := Instance.CreateOrganization(CTXIAM, fmt.Sprintf("ZITADEL_GRANTED_%s", gofakeit.AppName()), gofakeit.Email()) projectGrant, err := Instance.Client.Mgmt.AddProjectGrant(CTX, &management.AddProjectGrantRequest{ ProjectId: projectID, GrantedOrgId: grantedOrg.GetOrganizationId(), diff --git a/internal/integration/context.go b/internal/integration/context.go new file mode 100644 index 0000000000..8ba4cbe204 --- /dev/null +++ b/internal/integration/context.go @@ -0,0 +1,30 @@ +package integration + +import ( + "context" + "time" +) + +// WaitForAndTickWithMaxDuration determine a duration and interval for EventuallyWithT-tests from context timeout and desired max duration +func WaitForAndTickWithMaxDuration(ctx context.Context, max time.Duration) (time.Duration, time.Duration) { + // interval which is used to retry the test + tick := time.Millisecond * 100 + // tolerance which is used to stop the test for the timeout + tolerance := tick * 5 + // default of the WaitFor is always a defined duration, shortened if the context would time out before + waitFor := max + + if ctxDeadline, ok := ctx.Deadline(); ok { + // if the context has a deadline, set the WaitFor to the shorter duration + if until := time.Until(ctxDeadline); until < waitFor { + // ignore durations which are smaller than the tolerance + if until < tolerance { + waitFor = 0 + } else { + // always let the test stop with tolerance before the context is in timeout + waitFor = until - tolerance + } + } + } + return waitFor, tick +} diff --git a/internal/notification/handlers/integration_test/telemetry_pusher_test.go b/internal/notification/handlers/integration_test/telemetry_pusher_test.go index c12ab64f35..fdff1180a8 100644 --- a/internal/notification/handlers/integration_test/telemetry_pusher_test.go +++ b/internal/notification/handlers/integration_test/telemetry_pusher_test.go @@ -25,6 +25,8 @@ import ( ) func TestServer_TelemetryPushMilestones(t *testing.T) { + t.Parallel() + sub := sink.Subscribe(CTX, sink.ChannelMilestone) defer sub.Close() From 27ab1a22e73af20fca89cb63289b6086e8da4077 Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Mon, 21 Oct 2024 21:15:02 +0200 Subject: [PATCH 3/4] chore: correct require usage to assert for eventual consistency (#8795) # Which Problems Are Solved Eventual consistency is handled wrongly in the newly improved integration tests. # How the Problems Are Solved Correct the usage of the require package with the assert package where necessary, to remove the panics where the EventuallyWithT functions can rerun. # Additional Changes Modify the timeout values for some EventuallyWithT which can vary when a instance is freshly setup. # Additional Context None (cherry picked from commit fca6b28a97d7cd5cc7cd0de9d5d2e6ff34c92a07) --- .../admin/integration_test/iam_member_test.go | 11 ++-- .../integration_test/iam_settings_test.go | 2 +- .../org/v2/integration_test/query_test.go | 20 ++++---- .../integration_test/execution_target_test.go | 8 ++- .../v3alpha/integration_test/query_test.go | 37 +++++++------- .../v3alpha/integration_test/server_test.go | 6 +-- .../v3alpha/integration_test/query_test.go | 23 +++++---- .../v2/integration_test/settings_test.go | 8 +-- .../v2beta/integration_test/settings_test.go | 6 ++- .../user/v2/integration_test/idp_link_test.go | 7 +-- .../user/v2/integration_test/passkey_test.go | 7 +-- .../user/v2/integration_test/query_test.go | 42 +++++++-------- .../v2beta/integration_test/query_test.go | 51 ++++++++++--------- 13 files changed, 121 insertions(+), 107 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 1b6440923e..ff8d2715d7 100644 --- a/internal/api/grpc/admin/integration_test/iam_member_test.go +++ b/internal/api/grpc/admin/integration_test/iam_member_test.go @@ -92,7 +92,7 @@ func TestServer_ListIAMMembers(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, 20*time.Second) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, time.Minute) assert.EventuallyWithT(t, func(ct *assert.CollectT) { got, err := Client.ListIAMMembers(tt.args.ctx, tt.args.req) if tt.wantErr { @@ -103,10 +103,11 @@ func TestServer_ListIAMMembers(t *testing.T) { wantResult := tt.want.GetResult() gotResult := got.GetResult() - require.Len(ct, gotResult, len(wantResult)) - for i, want := range wantResult { - assert.Equal(ct, want.GetUserId(), gotResult[i].GetUserId()) - assert.ElementsMatch(ct, want.GetRoles(), gotResult[i].GetRoles()) + if assert.Len(ct, gotResult, len(wantResult)) { + for i, want := range wantResult { + assert.Equal(ct, want.GetUserId(), gotResult[i].GetUserId()) + assert.ElementsMatch(ct, want.GetRoles(), gotResult[i].GetRoles()) + } } }, retryDuration, tick) }) diff --git a/internal/api/grpc/admin/integration_test/iam_settings_test.go b/internal/api/grpc/admin/integration_test/iam_settings_test.go index 93da4aed8a..9eca09c06c 100644 --- a/internal/api/grpc/admin/integration_test/iam_settings_test.go +++ b/internal/api/grpc/admin/integration_test/iam_settings_test.go @@ -54,7 +54,7 @@ func TestServer_GetSecurityPolicy(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.ctx, 5*time.Second) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { resp, err := instance.Client.Admin.GetSecurityPolicy(tt.ctx, &admin_pb.GetSecurityPolicyRequest{}) if tt.wantErr { diff --git a/internal/api/grpc/org/v2/integration_test/query_test.go b/internal/api/grpc/org/v2/integration_test/query_test.go index e52ea40018..e476b4e60d 100644 --- a/internal/api/grpc/org/v2/integration_test/query_test.go +++ b/internal/api/grpc/org/v2/integration_test/query_test.go @@ -411,17 +411,17 @@ func TestServer_ListOrganizations(t *testing.T) { // totalResult is unrelated to the tests here so gets carried over, can vary from the count of results due to permissions tt.want.Details.TotalResult = got.Details.TotalResult // always first check length, otherwise its failed anyway - require.Len(ttt, got.Result, len(tt.want.Result)) + if assert.Len(ttt, got.Result, len(tt.want.Result)) { + for i := range tt.want.Result { + // domain from result, as it is generated though the create + tt.want.Result[i].PrimaryDomain = got.Result[i].PrimaryDomain + // sequence from result, as it can be with different sequence from create + tt.want.Result[i].Details.Sequence = got.Result[i].Details.Sequence + } - for i := range tt.want.Result { - // domain from result, as it is generated though the create - tt.want.Result[i].PrimaryDomain = got.Result[i].PrimaryDomain - // sequence from result, as it can be with different sequence from create - tt.want.Result[i].Details.Sequence = got.Result[i].Details.Sequence - } - - for i := range tt.want.Result { - assert.Contains(ttt, got.Result, tt.want.Result[i]) + for i := range tt.want.Result { + assert.Contains(ttt, got.Result, tt.want.Result[i]) + } } integration.AssertListDetails(t, tt.want, got) }, retryDuration, tick, "timeout waiting for expected user result") diff --git a/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go b/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go index 042b7a416e..286048ab51 100644 --- a/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go +++ b/internal/api/grpc/resources/action/v3alpha/integration_test/execution_target_test.go @@ -255,7 +255,7 @@ func TestServer_ExecutionTarget(t *testing.T) { require.NoError(t, err) defer close() } - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 5*time.Second) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := instance.Client.ActionV3Alpha.GetTarget(tt.ctx, tt.req) if tt.wantErr { @@ -278,7 +278,7 @@ func TestServer_ExecutionTarget(t *testing.T) { } func waitForExecutionOnCondition(ctx context.Context, t *testing.T, instance *integration.Instance, condition *action.Condition) { - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, 5*time.Second) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := instance.Client.ActionV3Alpha.SearchExecutions(ctx, &action.SearchExecutionsRequest{ Filters: []*action.ExecutionSearchFilter{ @@ -290,9 +290,7 @@ func waitForExecutionOnCondition(ctx context.Context, t *testing.T, instance *in if !assert.NoError(ttt, err) { return } - if assert.Len(ttt, got.GetResult(), 1) { - return - } + assert.Len(ttt, got.GetResult(), 1) }, retryDuration, tick, "timeout waiting for expected execution result") return } diff --git a/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go b/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go index 3756a144d8..c29900e966 100644 --- a/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go +++ b/internal/api/grpc/resources/action/v3alpha/integration_test/query_test.go @@ -216,14 +216,14 @@ func TestServer_GetTarget(t *testing.T) { err := tt.args.dep(tt.args.ctx, tt.args.req, tt.want) require.NoError(t, err) } - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 5*time.Second) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := instance.Client.ActionV3Alpha.GetTarget(tt.args.ctx, tt.args.req) if tt.wantErr { - require.Error(ttt, err, "Error: "+err.Error()) + assert.Error(ttt, err, "Error: "+err.Error()) return } - require.NoError(ttt, err) + assert.NoError(ttt, err) wantTarget := tt.want.GetTarget() gotTarget := got.GetTarget() @@ -478,7 +478,7 @@ func TestServer_ListTargets(t *testing.T) { require.NoError(t, err) } - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 5*time.Second) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, listErr := instance.Client.ActionV3Alpha.SearchTargets(tt.args.ctx, tt.args.req) if tt.wantErr { @@ -488,11 +488,11 @@ func TestServer_ListTargets(t *testing.T) { require.NoError(ttt, listErr) // always first check length, otherwise its failed anyway - require.Len(ttt, got.Result, len(tt.want.Result)) - - for i := range tt.want.Result { - integration.AssertResourceDetails(ttt, tt.want.Result[i].GetDetails(), got.Result[i].GetDetails()) - assert.EqualExportedValues(ttt, tt.want.Result[i].GetConfig(), got.Result[i].GetConfig()) + if assert.Len(ttt, got.Result, len(tt.want.Result)) { + for i := range tt.want.Result { + integration.AssertResourceDetails(ttt, tt.want.Result[i].GetDetails(), got.Result[i].GetDetails()) + assert.EqualExportedValues(ttt, tt.want.Result[i].GetConfig(), got.Result[i].GetConfig()) + } } integration.AssertResourceListDetails(ttt, tt.want, got) }, retryDuration, tick, "timeout waiting for expected execution result") @@ -863,7 +863,7 @@ func TestServer_SearchExecutions(t *testing.T) { require.NoError(t, err) } - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 5*time.Second) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, listErr := instance.Client.ActionV3Alpha.SearchExecutions(tt.args.ctx, tt.args.req) if tt.wantErr { @@ -872,14 +872,15 @@ func TestServer_SearchExecutions(t *testing.T) { } require.NoError(ttt, listErr) // always first check length, otherwise its failed anyway - require.Len(ttt, got.Result, len(tt.want.Result)) - for i := range tt.want.Result { - // as not sorted, all elements have to be checked - // workaround as oneof elements can only be checked with assert.EqualExportedValues() - if j, found := containExecution(got.Result, tt.want.Result[i]); found { - integration.AssertResourceDetails(ttt, tt.want.Result[i].GetDetails(), got.Result[j].GetDetails()) - got.Result[j].Details = tt.want.Result[i].GetDetails() - assert.EqualExportedValues(ttt, tt.want.Result[i], got.Result[j]) + if assert.Len(ttt, got.Result, len(tt.want.Result)) { + for i := range tt.want.Result { + // as not sorted, all elements have to be checked + // workaround as oneof elements can only be checked with assert.EqualExportedValues() + if j, found := containExecution(got.Result, tt.want.Result[i]); found { + integration.AssertResourceDetails(ttt, tt.want.Result[i].GetDetails(), got.Result[j].GetDetails()) + got.Result[j].Details = tt.want.Result[i].GetDetails() + assert.EqualExportedValues(ttt, tt.want.Result[i], got.Result[j]) + } } } integration.AssertResourceListDetails(ttt, tt.want, got) diff --git a/internal/api/grpc/resources/action/v3alpha/integration_test/server_test.go b/internal/api/grpc/resources/action/v3alpha/integration_test/server_test.go index 3c1c32062d..bc8e43eafc 100644 --- a/internal/api/grpc/resources/action/v3alpha/integration_test/server_test.go +++ b/internal/api/grpc/resources/action/v3alpha/integration_test/server_test.go @@ -44,7 +44,7 @@ func ensureFeatureEnabled(t *testing.T, instance *integration.Instance) { }) require.NoError(t, err) - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, 5*time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { f, err := instance.Client.FeatureV2.GetInstanceFeatures(ctx, &feature.GetInstanceFeaturesRequest{ @@ -54,10 +54,10 @@ func ensureFeatureEnabled(t *testing.T, instance *integration.Instance) { assert.True(ttt, f.Actions.GetEnabled()) }, retryDuration, - time.Second, + tick, "timed out waiting for ensuring instance feature") - retryDuration, tick = integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) + retryDuration, tick = integration.WaitForAndTickWithMaxDuration(ctx, 5*time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { _, err := instance.Client.ActionV3Alpha.ListExecutionMethods(ctx, &action.ListExecutionMethodsRequest{}) diff --git a/internal/api/grpc/resources/userschema/v3alpha/integration_test/query_test.go b/internal/api/grpc/resources/userschema/v3alpha/integration_test/query_test.go index 436af3bc6f..ef7fe02807 100644 --- a/internal/api/grpc/resources/userschema/v3alpha/integration_test/query_test.go +++ b/internal/api/grpc/resources/userschema/v3alpha/integration_test/query_test.go @@ -188,7 +188,7 @@ func TestServer_ListUserSchemas(t *testing.T) { require.NoError(t, err) } - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 20*time.Second) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := instance.Client.UserSchemaV3.SearchUserSchemas(tt.args.ctx, tt.args.req) if tt.wantErr { @@ -197,14 +197,15 @@ func TestServer_ListUserSchemas(t *testing.T) { } require.NoError(ttt, err) // always first check length, otherwise its failed anyway - require.Len(ttt, got.Result, len(tt.want.Result)) - for i := range tt.want.Result { - wantSchema := tt.want.Result[i] - gotSchema := got.Result[i] + if assert.Len(ttt, got.Result, len(tt.want.Result)) { + for i := range tt.want.Result { + wantSchema := tt.want.Result[i] + gotSchema := got.Result[i] - integration.AssertResourceDetails(ttt, wantSchema.GetDetails(), gotSchema.GetDetails()) - wantSchema.Details = gotSchema.GetDetails() - grpc.AllFieldsEqual(ttt, wantSchema.ProtoReflect(), gotSchema.ProtoReflect(), grpc.CustomMappers) + integration.AssertResourceDetails(ttt, wantSchema.GetDetails(), gotSchema.GetDetails()) + wantSchema.Details = gotSchema.GetDetails() + grpc.AllFieldsEqual(ttt, wantSchema.ProtoReflect(), gotSchema.ProtoReflect(), grpc.CustomMappers) + } } integration.AssertListDetails(ttt, tt.want, got) }, retryDuration, tick, "timeout waiting for expected user schema result") @@ -295,14 +296,14 @@ func TestServer_GetUserSchema(t *testing.T) { require.NoError(t, err) } - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, 5*time.Second) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(isolatedIAMOwnerCTX, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := instance.Client.UserSchemaV3.GetUserSchema(tt.args.ctx, tt.args.req) if tt.wantErr { - require.Error(ttt, err, "Error: "+err.Error()) + assert.Error(ttt, err, "Error: "+err.Error()) return } - require.NoError(ttt, err) + assert.NoError(ttt, err) wantSchema := tt.want.GetUserSchema() gotSchema := got.GetUserSchema() diff --git a/internal/api/grpc/settings/v2/integration_test/settings_test.go b/internal/api/grpc/settings/v2/integration_test/settings_test.go index 8ae576d104..16942137c9 100644 --- a/internal/api/grpc/settings/v2/integration_test/settings_test.go +++ b/internal/api/grpc/settings/v2/integration_test/settings_test.go @@ -53,14 +53,16 @@ func TestServer_GetSecuritySettings(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.ctx, 20*time.Second) + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.ctx, time.Minute) assert.EventuallyWithT(t, func(ct *assert.CollectT) { resp, err := Client.GetSecuritySettings(tt.ctx, &settings.GetSecuritySettingsRequest{}) if tt.wantErr { - require.Error(ct, err) + assert.Error(ct, err) + return + } + if !assert.NoError(ct, err) { return } - require.NoError(ct, err) got, want := resp.GetSettings(), tt.want.GetSettings() assert.Equal(ct, want.GetEmbeddedIframe().GetEnabled(), got.GetEmbeddedIframe().GetEnabled(), "enable iframe embedding") assert.Equal(ct, want.GetEmbeddedIframe().GetAllowedOrigins(), got.GetEmbeddedIframe().GetAllowedOrigins(), "allowed origins") diff --git a/internal/api/grpc/settings/v2beta/integration_test/settings_test.go b/internal/api/grpc/settings/v2beta/integration_test/settings_test.go index 9f6968f5e0..d5c1914ba9 100644 --- a/internal/api/grpc/settings/v2beta/integration_test/settings_test.go +++ b/internal/api/grpc/settings/v2beta/integration_test/settings_test.go @@ -57,10 +57,12 @@ func TestServer_GetSecuritySettings(t *testing.T) { assert.EventuallyWithT(t, func(ct *assert.CollectT) { resp, err := Client.GetSecuritySettings(tt.ctx, &settings.GetSecuritySettingsRequest{}) if tt.wantErr { - require.Error(ct, err) + assert.Error(ct, err) + return + } + if !assert.NoError(ct, err) { return } - require.NoError(ct, err) got, want := resp.GetSettings(), tt.want.GetSettings() assert.Equal(ct, want.GetEmbeddedIframe().GetEnabled(), got.GetEmbeddedIframe().GetEnabled(), "enable iframe embedding") assert.Equal(ct, want.GetEmbeddedIframe().GetAllowedOrigins(), got.GetEmbeddedIframe().GetAllowedOrigins(), "allowed origins") 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 e9022f31f8..ab398c7233 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 @@ -245,9 +245,10 @@ func TestServer_ListIDPLinks(t *testing.T) { } require.NoError(ttt, err) // always first check length, otherwise its failed anyway - require.Len(ttt, got.Result, len(tt.want.Result)) - for i := range tt.want.Result { - assert.Contains(ttt, got.Result, tt.want.Result[i]) + if assert.Len(ttt, got.Result, len(tt.want.Result)) { + for i := range tt.want.Result { + assert.Contains(ttt, got.Result, tt.want.Result[i]) + } } integration.AssertListDetails(t, tt.want, got) }, retryDuration, tick, "timeout waiting for expected idplinks result") diff --git a/internal/api/grpc/user/v2/integration_test/passkey_test.go b/internal/api/grpc/user/v2/integration_test/passkey_test.go index 585e5ba413..9d9cb8a047 100644 --- a/internal/api/grpc/user/v2/integration_test/passkey_test.go +++ b/internal/api/grpc/user/v2/integration_test/passkey_test.go @@ -593,9 +593,10 @@ func TestServer_ListPasskeys(t *testing.T) { } require.NoError(ttt, err) // always first check length, otherwise its failed anyway - assert.Len(ttt, got.Result, len(tt.want.Result)) - for i := range tt.want.Result { - assert.Contains(ttt, got.Result, tt.want.Result[i]) + if assert.Len(ttt, got.Result, len(tt.want.Result)) { + for i := range tt.want.Result { + assert.Contains(ttt, got.Result, tt.want.Result[i]) + } } integration.AssertListDetails(ttt, tt.want, got) }, retryDuration, tick, "timeout waiting for expected idplinks result") 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 fc2104d62e..4ee085336c 100644 --- a/internal/api/grpc/user/v2/integration_test/query_test.go +++ b/internal/api/grpc/user/v2/integration_test/query_test.go @@ -162,10 +162,12 @@ func TestServer_GetUserByID(t *testing.T) { require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := Client.GetUserByID(tt.args.ctx, tt.args.req) if tt.wantErr { - require.Error(ttt, err) + assert.Error(ttt, err) + return + } + if !assert.NoError(ttt, err) { return } - require.NoError(ttt, err) tt.want.User.Details = userAttr.Details tt.want.User.UserId = userAttr.UserID @@ -912,27 +914,27 @@ func TestServer_ListUsers(t *testing.T) { // always only give back dependency infos which are required for the response require.Len(ttt, tt.want.Result, len(infos)) // always first check length, otherwise its failed anyway - require.Len(ttt, got.Result, len(tt.want.Result)) + if assert.Len(ttt, got.Result, len(tt.want.Result)) { + // totalResult is unrelated to the tests here so gets carried over, can vary from the count of results due to permissions + tt.want.Details.TotalResult = got.Details.TotalResult - // totalResult is unrelated to the tests here so gets carried over, can vary from the count of results due to permissions - tt.want.Details.TotalResult = got.Details.TotalResult - - // fill in userid and username as it is generated - for i := range infos { - tt.want.Result[i].UserId = infos[i].UserID - tt.want.Result[i].Username = infos[i].Username - tt.want.Result[i].PreferredLoginName = infos[i].Username - tt.want.Result[i].LoginNames = []string{infos[i].Username} - if human := tt.want.Result[i].GetHuman(); human != nil { - human.Email.Email = infos[i].Username - if tt.want.Result[i].GetHuman().GetPasswordChanged() != nil { - human.PasswordChanged = infos[i].Changed + // fill in userid and username as it is generated + for i := range infos { + tt.want.Result[i].UserId = infos[i].UserID + tt.want.Result[i].Username = infos[i].Username + tt.want.Result[i].PreferredLoginName = infos[i].Username + tt.want.Result[i].LoginNames = []string{infos[i].Username} + if human := tt.want.Result[i].GetHuman(); human != nil { + human.Email.Email = infos[i].Username + if tt.want.Result[i].GetHuman().GetPasswordChanged() != nil { + human.PasswordChanged = infos[i].Changed + } } + tt.want.Result[i].Details = infos[i].Details + } + for i := range tt.want.Result { + assert.Contains(ttt, got.Result, tt.want.Result[i]) } - tt.want.Result[i].Details = infos[i].Details - } - for i := range tt.want.Result { - assert.Contains(ttt, got.Result, tt.want.Result[i]) } integration.AssertListDetails(ttt, tt.want, got) }, retryDuration, tick, "timeout waiting for expected user result") 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 4ea911727c..654a84a5d4 100644 --- a/internal/api/grpc/user/v2beta/integration_test/query_test.go +++ b/internal/api/grpc/user/v2beta/integration_test/query_test.go @@ -171,10 +171,12 @@ func TestServer_GetUserByID(t *testing.T) { require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := Client.GetUserByID(tt.args.ctx, tt.args.req) if tt.wantErr { - require.Error(t, err) + assert.Error(ttt, err) + return + } + if !assert.NoError(ttt, err) { return } - require.NoError(t, err) tt.want.User.Details = detailsV2ToV2beta(userAttr.Details) tt.want.User.UserId = userAttr.UserID @@ -188,7 +190,7 @@ func TestServer_GetUserByID(t *testing.T) { } } assert.Equal(ttt, tt.want.User, got.User) - integration.AssertDetails(t, tt.want, got) + integration.AssertDetails(ttt, tt.want, got) }, retryDuration, tick) }) } @@ -314,10 +316,13 @@ func TestServer_GetUserByID_Permission(t *testing.T) { require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, err := Client.GetUserByID(tt.args.ctx, tt.args.req) if tt.wantErr { - require.Error(ttt, err) + assert.Error(ttt, err) return } - require.NoError(ttt, err) + if !assert.NoError(ttt, err) { + return + } + tt.want.User.UserId = tt.args.req.GetUserId() tt.want.User.Username = newOrgOwnerEmail tt.want.User.PreferredLoginName = newOrgOwnerEmail @@ -923,27 +928,27 @@ func TestServer_ListUsers(t *testing.T) { // always only give back dependency infos which are required for the response require.Len(ttt, tt.want.Result, len(infos)) // always first check length, otherwise its failed anyway - require.Len(ttt, got.Result, len(tt.want.Result)) - // fill in userid and username as it is generated + if assert.Len(ttt, got.Result, len(tt.want.Result)) { + // totalResult is unrelated to the tests here so gets carried over, can vary from the count of results due to permissions + tt.want.Details.TotalResult = got.Details.TotalResult - // totalResult is unrelated to the tests here so gets carried over, can vary from the count of results due to permissions - tt.want.Details.TotalResult = got.Details.TotalResult - - for i := range infos { - tt.want.Result[i].UserId = infos[i].UserID - tt.want.Result[i].Username = infos[i].Username - tt.want.Result[i].PreferredLoginName = infos[i].Username - tt.want.Result[i].LoginNames = []string{infos[i].Username} - if human := tt.want.Result[i].GetHuman(); human != nil { - human.Email.Email = infos[i].Username - if tt.want.Result[i].GetHuman().GetPasswordChanged() != nil { - human.PasswordChanged = infos[i].Changed + // fill in userid and username as it is generated + for i := range infos { + tt.want.Result[i].UserId = infos[i].UserID + tt.want.Result[i].Username = infos[i].Username + tt.want.Result[i].PreferredLoginName = infos[i].Username + tt.want.Result[i].LoginNames = []string{infos[i].Username} + if human := tt.want.Result[i].GetHuman(); human != nil { + human.Email.Email = infos[i].Username + if tt.want.Result[i].GetHuman().GetPasswordChanged() != nil { + human.PasswordChanged = infos[i].Changed + } } + tt.want.Result[i].Details = detailsV2ToV2beta(infos[i].Details) + } + for i := range tt.want.Result { + assert.Contains(ttt, got.Result, tt.want.Result[i]) } - tt.want.Result[i].Details = detailsV2ToV2beta(infos[i].Details) - } - for i := range tt.want.Result { - assert.Contains(ttt, got.Result, tt.want.Result[i]) } integration.AssertListDetails(ttt, tt.want, got) }, retryDuration, tick, "timeout waiting for expected user result") From 7508e6c9f36f3e1968c873b7dd7853077e4c8cd8 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Tue, 22 Oct 2024 16:16:44 +0200 Subject: [PATCH 4/4] fix: correctly check denied domains and ips for actions (#8810) # Which Problems Are Solved System administrators can block hosts and IPs for HTTP calls in actions. Using DNS, blocked IPs could be bypassed. # How the Problems Are Solved - Hosts are resolved (DNS lookup) to check whether their corresponding IP is blocked. # Additional Changes - Added complete lookup ip address range and "unspecified" address to the default `DenyList` (cherry picked from commit 79fb4cc1cc6ebba91f9af917807e0e3651516acd) --- cmd/defaults.yaml | 5 ++- cmd/start/config_test.go | 12 ++--- internal/actions/http_module.go | 28 +++++++++--- internal/actions/http_module_config.go | 62 +++++++++++--------------- internal/actions/http_module_test.go | 48 ++++++++++++++++---- 5 files changed, 95 insertions(+), 60 deletions(-) diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index 90c2db1f01..a12fe474ba 100644 --- a/cmd/defaults.yaml +++ b/cmd/defaults.yaml @@ -600,7 +600,10 @@ Actions: # Wildcard sub domains are currently unsupported DenyList: # ZITADEL_ACTIONS_HTTP_DENYLIST (comma separated list) - localhost - - "127.0.0.1" + - "127.0.0.0/8" + - "::1" + - "0.0.0.0" + - "::" LogStore: Access: diff --git a/cmd/start/config_test.go b/cmd/start/config_test.go index 90d4b9d2dc..53c95d35ab 100644 --- a/cmd/start/config_test.go +++ b/cmd/start/config_test.go @@ -47,9 +47,9 @@ Log: `}, want: func(t *testing.T, config *Config) { assert.Equal(t, config.Actions.HTTP.DenyList, []actions.AddressChecker{ - &actions.DomainChecker{Domain: "localhost"}, - &actions.IPChecker{IP: net.ParseIP("127.0.0.1")}, - &actions.DomainChecker{Domain: "foobar"}}) + &actions.HostChecker{Domain: "localhost"}, + &actions.HostChecker{IP: net.ParseIP("127.0.0.1")}, + &actions.HostChecker{Domain: "foobar"}}) }, }, { name: "actions deny list string ok", @@ -63,9 +63,9 @@ Log: `}, want: func(t *testing.T, config *Config) { assert.Equal(t, config.Actions.HTTP.DenyList, []actions.AddressChecker{ - &actions.DomainChecker{Domain: "localhost"}, - &actions.IPChecker{IP: net.ParseIP("127.0.0.1")}, - &actions.DomainChecker{Domain: "foobar"}}) + &actions.HostChecker{Domain: "localhost"}, + &actions.HostChecker{IP: net.ParseIP("127.0.0.1")}, + &actions.HostChecker{Domain: "foobar"}}) }, }, { name: "features ok", diff --git a/internal/actions/http_module.go b/internal/actions/http_module.go index 33cfbc91bc..2f9d09932c 100644 --- a/internal/actions/http_module.go +++ b/internal/actions/http_module.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "io" + "net" "net/http" "net/url" "strings" @@ -19,7 +20,7 @@ import ( func WithHTTP(ctx context.Context) Option { return func(c *runConfig) { c.modules["zitadel/http"] = func(runtime *goja.Runtime, module *goja.Object) { - requireHTTP(ctx, &http.Client{Transport: new(transport)}, runtime, module) + requireHTTP(ctx, &http.Client{Transport: &transport{lookup: net.LookupIP}}, runtime, module) } } } @@ -170,21 +171,34 @@ func parseHeaders(headers *goja.Object) http.Header { return h } -type transport struct{} +type transport struct { + lookup func(string) ([]net.IP, error) +} -func (*transport) RoundTrip(req *http.Request) (*http.Response, error) { +func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) { if httpConfig == nil { return http.DefaultTransport.RoundTrip(req) } - if isHostBlocked(httpConfig.DenyList, req.URL) { + if t.isHostBlocked(httpConfig.DenyList, req.URL) { return nil, zerrors.ThrowInvalidArgument(nil, "ACTIO-N72d0", "host is denied") } return http.DefaultTransport.RoundTrip(req) } -func isHostBlocked(denyList []AddressChecker, address *url.URL) bool { +func (t *transport) isHostBlocked(denyList []AddressChecker, address *url.URL) bool { + host := address.Hostname() + ip := net.ParseIP(host) + ips := []net.IP{ip} + // if the hostname is a domain, we need to check resolve the ip(s), since it might be denied + if ip == nil { + var err error + ips, err = t.lookup(host) + if err != nil { + return true + } + } for _, blocked := range denyList { - if blocked.Matches(address.Hostname()) { + if blocked.Matches(ips, host) { return true } } @@ -192,5 +206,5 @@ func isHostBlocked(denyList []AddressChecker, address *url.URL) bool { } type AddressChecker interface { - Matches(string) bool + Matches([]net.IP, string) bool } diff --git a/internal/actions/http_module_config.go b/internal/actions/http_module_config.go index d10ad39676..d1b965814e 100644 --- a/internal/actions/http_module_config.go +++ b/internal/actions/http_module_config.go @@ -6,8 +6,6 @@ import ( "strings" "github.com/mitchellh/mapstructure" - - "github.com/zitadel/zitadel/internal/zerrors" ) func SetHTTPConfig(config *HTTPConfig) { @@ -48,7 +46,7 @@ func HTTPConfigDecodeHook(from, to reflect.Value) (interface{}, error) { for _, unsplit := range config.DenyList { for _, split := range strings.Split(unsplit, ",") { - parsed, parseErr := parseDenyListEntry(split) + parsed, parseErr := NewHostChecker(split) if parseErr != nil { return nil, parseErr } @@ -61,46 +59,36 @@ func HTTPConfigDecodeHook(from, to reflect.Value) (interface{}, error) { return c, nil } -func parseDenyListEntry(entry string) (AddressChecker, error) { - if checker, err := NewIPChecker(entry); err == nil { - return checker, nil - } - return &DomainChecker{Domain: entry}, nil -} - -func NewIPChecker(i string) (AddressChecker, error) { - _, network, err := net.ParseCIDR(i) +func NewHostChecker(entry string) (AddressChecker, error) { + _, network, err := net.ParseCIDR(entry) if err == nil { - return &IPChecker{Net: network}, nil + return &HostChecker{Net: network}, nil } - if ip := net.ParseIP(i); ip != nil { - return &IPChecker{IP: ip}, nil + if ip := net.ParseIP(entry); ip != nil { + return &HostChecker{IP: ip}, nil } - return nil, zerrors.ThrowInvalidArgument(nil, "ACTIO-ddJ7h", "invalid ip") + return &HostChecker{Domain: entry}, nil } -type IPChecker struct { - Net *net.IPNet - IP net.IP -} - -func (c *IPChecker) Matches(address string) bool { - ip := net.ParseIP(address) - if ip == nil { - return false - } - - if c.IP != nil { - return c.IP.Equal(ip) - } - return c.Net.Contains(ip) -} - -type DomainChecker struct { +type HostChecker struct { + Net *net.IPNet + IP net.IP Domain string } -func (c *DomainChecker) Matches(domain string) bool { - //TODO: allow wild cards - return c.Domain == domain +func (c *HostChecker) Matches(ips []net.IP, address string) bool { + // if the address matches the domain, no additional checks as needed + if c.Domain == address { + return true + } + // otherwise we need to check on ips (incl. the resolved ips of the host) + for _, ip := range ips { + if c.Net != nil && c.Net.Contains(ip) { + return true + } + if c.IP != nil && c.IP.Equal(ip) { + return true + } + } + return false } diff --git a/internal/actions/http_module_test.go b/internal/actions/http_module_test.go index 0d3bdef75e..7a1f8d7816 100644 --- a/internal/actions/http_module_test.go +++ b/internal/actions/http_module_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "io" + "net" "net/http" "net/url" "reflect" @@ -19,17 +20,21 @@ import ( func Test_isHostBlocked(t *testing.T) { SetLogstoreService(logstore.New[*record.ExecutionLog](nil, nil)) var denyList = []AddressChecker{ - mustNewIPChecker(t, "192.168.5.0/24"), - mustNewIPChecker(t, "127.0.0.1"), - &DomainChecker{Domain: "test.com"}, + mustNewHostChecker(t, "192.168.5.0/24"), + mustNewHostChecker(t, "127.0.0.1"), + mustNewHostChecker(t, "test.com"), + } + type fields struct { + lookup func(host string) ([]net.IP, error) } type args struct { address *url.URL } tests := []struct { - name string - args args - want bool + name string + fields fields + args args + want bool }{ { name: "in range", @@ -47,6 +52,11 @@ func Test_isHostBlocked(t *testing.T) { }, { name: "address match", + fields: fields{ + lookup: func(host string) ([]net.IP, error) { + return []net.IP{net.ParseIP("194.264.52.4")}, nil + }, + }, args: args{ address: mustNewURL(t, "https://test.com:42/hodor"), }, @@ -54,24 +64,44 @@ func Test_isHostBlocked(t *testing.T) { }, { name: "address not match", + fields: fields{ + lookup: func(host string) ([]net.IP, error) { + return []net.IP{net.ParseIP("194.264.52.4")}, nil + }, + }, args: args{ address: mustNewURL(t, "https://test2.com/hodor"), }, want: false, }, + { + name: "looked up ip matches", + fields: fields{ + lookup: func(host string) ([]net.IP, error) { + return []net.IP{net.ParseIP("127.0.0.1")}, nil + }, + }, + args: args{ + address: mustNewURL(t, "https://test2.com/hodor"), + }, + want: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := isHostBlocked(denyList, tt.args.address); got != tt.want { + trans := &transport{ + lookup: tt.fields.lookup, + } + if got := trans.isHostBlocked(denyList, tt.args.address); got != tt.want { t.Errorf("isHostBlocked() = %v, want %v", got, tt.want) } }) } } -func mustNewIPChecker(t *testing.T, ip string) AddressChecker { +func mustNewHostChecker(t *testing.T, ip string) AddressChecker { t.Helper() - checker, err := NewIPChecker(ip) + checker, err := NewHostChecker(ip) if err != nil { t.Errorf("unable to parse cidr of %q because: %v", ip, err) t.FailNow()