From c205f15a29202fd99f2bfc2003ca23ac12e7a6ce Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 17 Jan 2025 17:17:59 +0800 Subject: [PATCH] fix: create roles and rolesmapping based on project IDs and names only Previously lagoon-opensearch-sync was creating project roles and rolesmapping by iterating over groups, including for project groups. This logic was based on the incorrect assumption that projects and project groups (AKA project default groups) are a 1:1 mapping. In reality, project groups can have multiple project "members". So the new logic ignores the project groups and just uses project IDs and names for roles and rolesmapping. This matches the logic used in the custom Keycloak mapper Lagoon uses to grant roles to Opensearch users. --- internal/sync/roles.go | 64 ++++++++++------------------------- internal/sync/rolesmapping.go | 38 ++++++++++----------- internal/sync/sync.go | 2 +- 3 files changed, 37 insertions(+), 67 deletions(-) diff --git a/internal/sync/roles.go b/internal/sync/roles.go index 36957db..33a2eb3 100644 --- a/internal/sync/roles.go +++ b/internal/sync/roles.go @@ -3,7 +3,6 @@ package sync import ( "context" "fmt" - "strings" "github.com/uselagoon/lagoon-opensearch-sync/internal/keycloak" "github.com/uselagoon/lagoon-opensearch-sync/internal/opensearch" @@ -69,40 +68,13 @@ func isLagoonGroup( return ok } -// projectGroupRoleName generates the name of a project group role from the -// ID of the group's project. -func projectGroupRoleName( - group keycloak.Group, - groupProjectsMap map[string][]int, -) (string, error) { - projectIDs, ok := groupProjectsMap[group.ID] - if !ok { - return "", fmt.Errorf("missing project group ID %s in groupProjectsMap", - group.ID) - } - if len(projectIDs) != 1 { - return "", fmt.Errorf("too many projects in group ID %s: %d", group.ID, - len(projectIDs)) - } - if projectIDs[0] < 0 { - return "", fmt.Errorf("invalid project ID in group ID %s: %d", group.ID, - projectIDs[0]) - } - return fmt.Sprintf("p%d", projectIDs[0]), nil -} - -// generateProjectGroupRole constructs an opensearch.Role from the given -// keycloak group corresponding to a Lagoon project group. -func generateProjectGroupRole( - group keycloak.Group, - groupProjectsMap map[string][]int, -) (string, *opensearch.Role, error) { - name, err := projectGroupRoleName(group, groupProjectsMap) - if err != nil { - return "", nil, - fmt.Errorf("couldn't generate project group role name: %v", err) - } - return name, &opensearch.Role{ +// generateProjectRole constructs an opensearch.Role from the given +// project ID and project name. +func generateProjectRole( + id int, + name string, +) (string, *opensearch.Role) { + return fmt.Sprintf("p%d", id), &opensearch.Role{ RolePermissions: opensearch.RolePermissions{ // use an empty slice instead of omitting this entirely because the // Opensearch API errors if this field is omitted. @@ -115,8 +87,7 @@ func generateProjectGroupRole( }, IndexPatterns: []string{ fmt.Sprintf( - `/^(application|container|lagoon|router)-logs-%s-_-.+/`, - strings.TrimPrefix(group.Name, "project-")), + `/^(application|container|lagoon|router)-logs-%s-_-.+/`, name), }, }, }, @@ -127,7 +98,7 @@ func generateProjectGroupRole( }, }, }, - }, nil + } } // generateRegularGroupRole constructs an opensearch.Role from the given @@ -184,10 +155,12 @@ func generateRegularGroupRole( } // generateRoles returns a slice of roles generated from the given slice of -// keycloak Groups. +// keycloak Groups, and the projectNames map. // // Any groups which are not recognized as either project groups or regular // Lagoon groups are ignored. +// +// All projectNames map entries generate a single role. func generateRoles( log *zap.Logger, groups []keycloak.Group, @@ -199,14 +172,7 @@ func generateRoles( var role *opensearch.Role var err error for _, group := range groups { - if isProjectGroup(log, group) { - name, role, err = generateProjectGroupRole(group, groupProjectsMap) - if err != nil { - log.Warn("couldn't generate role for project group", - zap.String("group name", group.Name), zap.Error(err)) - continue - } - } else if isLagoonGroup(group, groupProjectsMap) { + if isLagoonGroup(group, groupProjectsMap) && !isProjectGroup(log, group) { name, role, err = generateRegularGroupRole(log, group, projectNames, groupProjectsMap) if err != nil { @@ -219,6 +185,10 @@ func generateRoles( roles[name] = *role } } + for pid, pname := range projectNames { + name, role = generateProjectRole(pid, pname) + roles[name] = *role + } return roles } diff --git a/internal/sync/rolesmapping.go b/internal/sync/rolesmapping.go index cebcf6f..1ed9bfc 100644 --- a/internal/sync/rolesmapping.go +++ b/internal/sync/rolesmapping.go @@ -2,6 +2,7 @@ package sync import ( "context" + "fmt" "github.com/uselagoon/lagoon-opensearch-sync/internal/keycloak" "github.com/uselagoon/lagoon-opensearch-sync/internal/opensearch" @@ -55,34 +56,21 @@ func calculateRoleMappingDiff( } // generateRolesMapping returns a slice of rolesmapping generated from the -// given slice of keycloak Groups. +// given slice of keycloak Groups, and the projectNames map. // // Any groups which are not recognized as either project groups or regular // Lagoon groups are ignored. +// +// All projectNames map entries generate a single rolesmapping. func generateRolesMapping( log *zap.Logger, groups []keycloak.Group, + projectNames map[int]string, groupProjectsMap map[string][]int, ) map[string]opensearch.RoleMapping { rolesmapping := map[string]opensearch.RoleMapping{} for _, group := range groups { - // figure out if this is a regular group or project group - if isProjectGroup(log, group) { - name, err := projectGroupRoleName(group, groupProjectsMap) - if err != nil { - log.Warn("couldn't generate project group role name", zap.Error(err), - zap.String("group name", group.Name)) - continue - } - rolesmapping[name] = opensearch.RoleMapping{ - RoleMappingPermissions: opensearch.RoleMappingPermissions{ - BackendRoles: []string{name}, - AndBackendRoles: []string{}, - Hosts: []string{}, - Users: []string{}, - }, - } - } else if isLagoonGroup(group, groupProjectsMap) { + if isLagoonGroup(group, groupProjectsMap) && !isProjectGroup(log, group) { rolesmapping[group.Name] = opensearch.RoleMapping{ RoleMappingPermissions: opensearch.RoleMappingPermissions{ BackendRoles: []string{group.Name}, @@ -93,6 +81,17 @@ func generateRolesMapping( } } } + for pid := range projectNames { + roleName := fmt.Sprintf("p%d", pid) + rolesmapping[roleName] = opensearch.RoleMapping{ + RoleMappingPermissions: opensearch.RoleMappingPermissions{ + BackendRoles: []string{roleName}, + AndBackendRoles: []string{}, + Hosts: []string{}, + Users: []string{}, + }, + } + } return rolesmapping } @@ -123,6 +122,7 @@ func syncRolesMapping( ctx context.Context, log *zap.Logger, groups []keycloak.Group, + projectNames map[int]string, roles map[string]opensearch.Role, groupProjectsMap map[string][]int, o OpensearchService, @@ -137,7 +137,7 @@ func syncRolesMapping( // ignore non-lagoon rolesmapping existing = filterRolesMapping(existing, roles) // generate the rolesmapping required by Lagoon - required := generateRolesMapping(log, groups, groupProjectsMap) + required := generateRolesMapping(log, groups, projectNames, groupProjectsMap) // calculate rolesmapping to add/remove toCreate, toDelete := calculateRoleMappingDiff(existing, required) for _, name := range toDelete { diff --git a/internal/sync/sync.go b/internal/sync/sync.go index 667ef92..a2469f3 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -112,7 +112,7 @@ func Sync(ctx context.Context, log *zap.Logger, l LagoonDBService, case "roles": syncRoles(ctx, log, groups, projectNames, roles, groupProjectsMap, o, dryRun) case "rolesmapping": - syncRolesMapping(ctx, log, groups, roles, groupProjectsMap, o, dryRun) + syncRolesMapping(ctx, log, groups, projectNames, roles, groupProjectsMap, o, dryRun) case "indexpatterns": syncIndexPatterns(ctx, log, groupsSansGlobal, projectNames, groupProjectsMap, o, d, dryRun, legacyIndexPatternDelimiter)