From d9e3286af61b4463201820cb99f79f3148de0d7f Mon Sep 17 00:00:00 2001 From: dgtown Date: Mon, 6 May 2024 13:27:18 -0400 Subject: [PATCH] address comments --- api/server/handlers/user/migrate.go | 44 +++++++++++++++-------- api/server/shared/config/config.go | 2 +- api/server/shared/config/loader/loader.go | 2 +- internal/telemetry/span.go | 2 -- zarf/helm/.serverenv | 1 + 5 files changed, 32 insertions(+), 19 deletions(-) diff --git a/api/server/handlers/user/migrate.go b/api/server/handlers/user/migrate.go index db2a1b030a..9a4f01c10a 100644 --- a/api/server/handlers/user/migrate.go +++ b/api/server/handlers/user/migrate.go @@ -1,6 +1,7 @@ package user import ( + "errors" "fmt" "net/http" "strconv" @@ -43,8 +44,8 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) r = r.Clone(ctx) - user, _ := r.Context().Value(types.UserScope).(*models.User) - if !strings.HasSuffix(user.Email, "@porter.run") { + thisUser, _ := r.Context().Value(types.UserScope).(*models.User) + if !strings.HasSuffix(thisUser.Email, "@porter.run") { err := telemetry.Error(ctx, span, nil, "user is not a porter user") u.HandleAPIError(w, r, apierrors.NewErrForbidden(err)) return @@ -58,7 +59,7 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } var usersMissingAuthMechanism []uint - migrationErrors := map[uint]string{} + migrationErrors := map[string][]uint{} for _, user := range users { // skip users that are already migrated @@ -82,7 +83,8 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } } - if user.Password != "" { + switch { + case user.Password != "": password := user.Password createIdentityBody.Credentials = &ory.IdentityWithCredentials{ Oidc: nil, @@ -93,7 +95,7 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) }, AdditionalProperties: nil, } - } else if user.GithubUserID != 0 { + case user.GithubUserID != 0: createIdentityBody.Credentials = &ory.IdentityWithCredentials{ Oidc: &ory.IdentityWithCredentialsOidc{ Config: &ory.IdentityWithCredentialsOidcConfig{ @@ -107,7 +109,7 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) }, }, } - } else if user.GoogleUserID != "" { + case user.GoogleUserID != "": createIdentityBody.Credentials = &ory.IdentityWithCredentials{ Oidc: &ory.IdentityWithCredentialsOidc{ Config: &ory.IdentityWithCredentialsOidcConfig{ @@ -121,14 +123,19 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) }, }, } - } else { + default: usersMissingAuthMechanism = append(usersMissingAuthMechanism, user.ID) continue } createdIdentity, _, err := u.Config().Ory.IdentityAPI.CreateIdentity(u.Config().OryApiKeyContextWrapper(ctx)).CreateIdentityBody(createIdentityBody).Execute() if err != nil { - migrationErrors[user.ID] = fmt.Sprintf("error creating identity: %s", err.Error()) + errString := fmt.Sprintf("error creating identity: %s", err.Error()) + if len(migrationErrors[err.Error()]) == 0 { + migrationErrors[errString] = []uint{} + } + migrationErrors[errString] = append(migrationErrors[errString], user.ID) + continue } @@ -137,18 +144,25 @@ func (u *MigrateUsersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) _, err = u.Repo().User().UpdateUser(user) if err != nil { - migrationErrors[user.ID] = fmt.Sprintf("error updating user: %s", err.Error()) + errString := fmt.Sprintf("error updating user: %s", err.Error()) + if len(migrationErrors[err.Error()]) == 0 { + migrationErrors[errString] = []uint{} + } + migrationErrors[errString] = append(migrationErrors[errString], user.ID) continue } } - telemetry.WithAttributes(span, - telemetry.AttributeKV{Key: "users-missing-auth-mechanism", Value: usersMissingAuthMechanism}, - telemetry.AttributeKV{Key: "migration-errors", Value: migrationErrors}, - ) + var errs []error + if len(usersMissingAuthMechanism) > 0 { + errs = append(errs, fmt.Errorf("users missing auth mechanism: %v", usersMissingAuthMechanism)) + } + for errString, userIds := range migrationErrors { + errs = append(errs, fmt.Errorf("%s: %v", errString, userIds)) + } - if len(usersMissingAuthMechanism) > 0 || len(migrationErrors) > 0 { - err := telemetry.Error(ctx, span, nil, "error migrating users") + if len(errs) > 0 { + err := telemetry.Error(ctx, span, errors.Join(errs...), "error migrating users") u.HandleAPIError(w, r, apierrors.NewErrPassThroughToClient(err, http.StatusInternalServerError)) return } diff --git a/api/server/shared/config/config.go b/api/server/shared/config/config.go index 87cfadd3c9..03ff5a237d 100644 --- a/api/server/shared/config/config.go +++ b/api/server/shared/config/config.go @@ -124,7 +124,7 @@ type Config struct { TelemetryConfig telemetry.TracerConfig - Ory *ory.APIClient + Ory ory.APIClient OryApiKeyContextWrapper func(ctx context.Context) context.Context } diff --git a/api/server/shared/config/loader/loader.go b/api/server/shared/config/loader/loader.go index a58d05bcab..792ac56a4d 100644 --- a/api/server/shared/config/loader/loader.go +++ b/api/server/shared/config/loader/loader.go @@ -396,7 +396,7 @@ func (e *EnvConfigLoader) LoadConfig() (res *config.Config, err error) { URL: InstanceEnvConf.ServerConf.OryUrl, }} - res.Ory = ory.NewAPIClient(c) + res.Ory = *ory.NewAPIClient(c) res.OryApiKeyContextWrapper = func(ctx context.Context) context.Context { return context.WithValue(ctx, ory.ContextAccessToken, InstanceEnvConf.ServerConf.OryApiKey) } diff --git a/internal/telemetry/span.go b/internal/telemetry/span.go index a270e9747f..04769545e8 100644 --- a/internal/telemetry/span.go +++ b/internal/telemetry/span.go @@ -92,8 +92,6 @@ func WithAttributes(span trace.Span, attrs ...AttributeKV) { zone, offset := val.Zone() span.SetAttributes(attribute.String(prefixSpanKey(fmt.Sprintf("%s-timezone", string(attr.Key))), zone)) span.SetAttributes(attribute.Int(prefixSpanKey(fmt.Sprintf("%s-offset", string(attr.Key))), offset)) - default: - span.SetAttributes(attribute.String(prefixSpanKey(string(attr.Key)), fmt.Sprintf("%v", val))) } } } diff --git a/zarf/helm/.serverenv b/zarf/helm/.serverenv index 1a17dc4a05..08c8455848 100644 --- a/zarf/helm/.serverenv +++ b/zarf/helm/.serverenv @@ -94,6 +94,7 @@ NEON_CLIENT_ID= # NEON_CLIENT_SECRET is used to integrate with Neon NEON_CLIENT_SECRET= +// Note: Ory values can be found in 1Password // ORY_URL is the URL for Ory ORY_URL= // ORY_API_KEY authenticates with Ory