From cf0254dc3e03aea5b0ce86725d30feda2a9602c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Mon, 24 Mar 2025 16:32:05 +0200 Subject: [PATCH] fix(authz): simplify system permission mapping (#9627) # Which Problems Are Solved Remove the need of maintaining a channel for System user permission mapping, while still ensuring the mapping is built only once per request. # How the Problems Are Solved Build the slice of `SystemUserPermissionsDBQuery` once, when the context is being created in the context middleware. This slice will remain valid during the request lifetime as part of the context. This removes the need of "caching" and additional synchronization. # Additional Changes ```sh gci write . --skip-generated -s standard -s default -s 'prefix(github.com/zitadel/zitadel)' --custom-order ``` # Additional Context - Follow up on discussion https://github.com/zitadel/zitadel/pull/9460#discussion_r2006769497 --- internal/api/authz/authorization.go | 64 ++++++++++------------------- internal/api/authz/context.go | 10 ++--- 2 files changed, 27 insertions(+), 47 deletions(-) diff --git a/internal/api/authz/authorization.go b/internal/api/authz/authorization.go index ac68c7ddb7..ea20a2438f 100644 --- a/internal/api/authz/authorization.go +++ b/internal/api/authz/authorization.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/zitadel/logging" + "github.com/zitadel/zitadel/internal/telemetry/tracing" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -18,7 +19,7 @@ const ( // CheckUserAuthorization verifies that: // - the token is active, -// - the organisation (**either** provided by ID or verified domain) exists +// - the organization (**either** provided by ID or verified domain) exists // - the user is permitted to call the requested endpoint (permission option in proto) // it will pass the [CtxData] and permission of the user into the ctx [context.Context] func CheckUserAuthorization(ctx context.Context, req interface{}, token, orgID, orgDomain string, verifier APITokenVerifier, systemRolePermissionMapping []RoleMapping, rolePermissionMapping []RoleMapping, requiredAuthOption Option, method string) (ctxSetter func(context.Context) context.Context, err error) { @@ -32,7 +33,7 @@ func CheckUserAuthorization(ctx context.Context, req interface{}, token, orgID, if requiredAuthOption.Permission == authenticated { return func(parent context.Context) context.Context { - parent = addGetSystemUserRolesFuncToCtx(parent, systemRolePermissionMapping, ctxData) + parent = addGetSystemUserRolesToCtx(parent, systemRolePermissionMapping, ctxData) return context.WithValue(parent, dataKey, ctxData) }, nil } @@ -53,7 +54,7 @@ func CheckUserAuthorization(ctx context.Context, req interface{}, token, orgID, parent = context.WithValue(parent, dataKey, ctxData) parent = context.WithValue(parent, allPermissionsKey, allPermissions) parent = context.WithValue(parent, requestPermissionsKey, requestedPermissions) - parent = addGetSystemUserRolesFuncToCtx(parent, systemRolePermissionMapping, ctxData) + parent = addGetSystemUserRolesToCtx(parent, systemRolePermissionMapping, ctxData) return parent }, nil } @@ -137,56 +138,35 @@ type SystemUserPermissionsDBQuery struct { Permissions []string `json:"permissions"` } -func addGetSystemUserRolesFuncToCtx(ctx context.Context, systemUserRoleMap []RoleMapping, ctxData CtxData) context.Context { +func addGetSystemUserRolesToCtx(ctx context.Context, systemUserRoleMap []RoleMapping, ctxData CtxData) context.Context { if len(ctxData.SystemMemberships) == 0 { return ctx - } else { - ctx = context.WithValue(ctx, systemUserRolesFuncKey, func() func(ctx context.Context) []SystemUserPermissionsDBQuery { - var systemUserPermissionsDbJsonQueryStruct []SystemUserPermissionsDBQuery - chann := make(chan struct{}, 1) - return func(ctx context.Context) []SystemUserPermissionsDBQuery { - if systemUserPermissionsDbJsonQueryStruct != nil { - return systemUserPermissionsDbJsonQueryStruct - } - - chann <- struct{}{} - defer func() { - <-chann - }() - if systemUserPermissionsDbJsonQueryStruct != nil { - return systemUserPermissionsDbJsonQueryStruct - } - - systemUserPermissionsDbJsonQueryStruct = make([]SystemUserPermissionsDBQuery, len(ctxData.SystemMemberships)) - - for i, systemPerm := range ctxData.SystemMemberships { - permissions := []string{} - for _, role := range systemPerm.Roles { - permissions = append(permissions, getPermissionsFromRole(systemUserRoleMap, role)...) - } - slices.Sort(permissions) - permissions = slices.Compact(permissions) - - systemUserPermissionsDbJsonQueryStruct[i].MemberType = systemPerm.MemberType.String() - systemUserPermissionsDbJsonQueryStruct[i].AggregateID = systemPerm.AggregateID - systemUserPermissionsDbJsonQueryStruct[i].Permissions = permissions - } - return systemUserPermissionsDbJsonQueryStruct - } - }()) } - return ctx + systemUserPermissions := make([]SystemUserPermissionsDBQuery, len(ctxData.SystemMemberships)) + for i, systemPerm := range ctxData.SystemMemberships { + permissions := make([]string, 0, len(systemPerm.Roles)) + for _, role := range systemPerm.Roles { + permissions = append(permissions, getPermissionsFromRole(systemUserRoleMap, role)...) + } + slices.Sort(permissions) + permissions = slices.Compact(permissions) + + systemUserPermissions[i].MemberType = systemPerm.MemberType.String() + systemUserPermissions[i].AggregateID = systemPerm.AggregateID + systemUserPermissions[i].Permissions = permissions + } + return context.WithValue(ctx, systemUserRolesKey, systemUserPermissions) } func GetSystemUserPermissions(ctx context.Context) []SystemUserPermissionsDBQuery { - getSystemUserRolesFuncValue := ctx.Value(systemUserRolesFuncKey) + getSystemUserRolesFuncValue := ctx.Value(systemUserRolesKey) if getSystemUserRolesFuncValue == nil { return nil } - getSystemUserRolesFunc, ok := getSystemUserRolesFuncValue.(func(context.Context) []SystemUserPermissionsDBQuery) + systemUserRoles, ok := getSystemUserRolesFuncValue.([]SystemUserPermissionsDBQuery) if !ok { logging.WithFields("Authz").Error("unable to cast []SystemUserPermissionsDBQuery") return nil } - return getSystemUserRolesFunc(ctx) + return systemUserRoles } diff --git a/internal/api/authz/context.go b/internal/api/authz/context.go index 779675ba40..d6528cd017 100644 --- a/internal/api/authz/context.go +++ b/internal/api/authz/context.go @@ -18,11 +18,11 @@ import ( type key int const ( - requestPermissionsKey key = 1 - dataKey key = 2 - allPermissionsKey key = 3 - instanceKey key = 4 - systemUserRolesFuncKey key = 5 + requestPermissionsKey key = 1 + dataKey key = 2 + allPermissionsKey key = 3 + instanceKey key = 4 + systemUserRolesKey key = 5 ) type CtxData struct {