From 141e7352f2aa4fd7cc8d0ce92de9f0c1d884e4b4 Mon Sep 17 00:00:00 2001 From: Shubham Mali Date: Thu, 10 Sep 2026 16:20:54 +0530 Subject: [PATCH 1/3] keystone-connector: support system and domain scoped role-groups. --- connector/keystone/keystone.go | 141 ++++------------ connector/keystone/keystone_unit_test.go | 199 +++++++++++++++++++++++ connector/keystone/types.go | 42 +++-- 3 files changed, 261 insertions(+), 121 deletions(-) create mode 100644 connector/keystone/keystone_unit_test.go diff --git a/connector/keystone/keystone.go b/connector/keystone/keystone.go index 794bea44f4..cb3e121e5a 100644 --- a/connector/keystone/keystone.go +++ b/connector/keystone/keystone.go @@ -342,9 +342,9 @@ func getRoleAssignments(ctx context.Context, client *http.Client, baseURL, token return nil, err } if len(opts.userID) > 0 { - endpoint = fmt.Sprintf("%s?effective&user.id=%s", endpoint, opts.userID) + endpoint = fmt.Sprintf("%s?effective&include_names&user.id=%s", endpoint, opts.userID) } else if len(opts.groupID) > 0 { - endpoint = fmt.Sprintf("%s?group.id=%s", endpoint, opts.groupID) + endpoint = fmt.Sprintf("%s?include_names&group.id=%s", endpoint, opts.groupID) } // https://docs.openstack.org/api-ref/identity/v3/?expanded=validate-and-show-information-for-token-detail,list-role-assignments-detail#list-role-assignments @@ -378,80 +378,6 @@ func getRoleAssignments(ctx context.Context, client *http.Client, baseURL, token return roleAssignmentResp.RoleAssignments, nil } -// getRoles returns all roles in keystone -func getRoles(ctx context.Context, client *http.Client, baseURL, token string, logger *slog.Logger) ([]role, error) { - // https://docs.openstack.org/api-ref/identity/v3/?expanded=validate-and-show-information-for-token-detail,list-role-assignments-detail,list-roles-detail#list-roles - rolesURL, err := url.JoinPath(baseURL, "v3", "roles") - if err != nil { - return nil, err - } - req, err := http.NewRequest(http.MethodGet, rolesURL, nil) - if err != nil { - return nil, err - } - req.Header.Set("X-Auth-Token", token) - req = req.WithContext(ctx) - resp, err := client.Do(req) - if err != nil { - logger.Error("failed to fetch keystone roles", "error", err) - return nil, err - } - - data, err := io.ReadAll(resp.Body) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - rolesResp := struct { - Roles []role `json:"roles"` - }{} - - err = json.Unmarshal(data, &rolesResp) - if err != nil { - return nil, err - } - - return rolesResp.Roles, nil -} - -// getProjects returns all projects in keystone -func getProjects(ctx context.Context, client *http.Client, baseURL, token string, logger *slog.Logger) ([]project, error) { - // https://docs.openstack.org/api-ref/identity/v3/?expanded=validate-and-show-information-for-token-detail,list-role-assignments-detail,list-roles-detail#list-roles - projectsURL, err := url.JoinPath(baseURL, "v3", "projects") - if err != nil { - return nil, err - } - req, err := http.NewRequest(http.MethodGet, projectsURL, nil) - if err != nil { - return nil, err - } - req.Header.Set("X-Auth-Token", token) - req = req.WithContext(ctx) - resp, err := client.Do(req) - if err != nil { - logger.Error("failed to fetch keystone projects", "error", err) - return nil, err - } - - data, err := io.ReadAll(resp.Body) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - projectsResp := struct { - Projects []project `json:"projects"` - }{} - - err = json.Unmarshal(data, &projectsResp) - if err != nil { - return nil, err - } - - return projectsResp.Projects, nil -} - func getUser(ctx context.Context, client *http.Client, baseURL, userID, token string) (*userResponse, error) { // https://developer.openstack.org/api-ref/identity/v3/#show-user-details userURL, err := url.JoinPath(baseURL, "v3", "users", userID) @@ -566,24 +492,6 @@ func getAllGroupsForUser(ctx context.Context, client *http.Client, baseURL, toke return userGroups, nil } - roles, err := getRoles(ctx, client, baseURL, token, logger) - if err != nil { - return userGroups, err - } - roleMap := map[string]role{} - for _, role := range roles { - roleMap[role.ID] = role - } - - projects, err := getProjects(ctx, client, baseURL, token, logger) - if err != nil { - return userGroups, err - } - projectMap := map[string]project{} - for _, project := range projects { - projectMap[project.ID] = project - } - // 3. Now create groups based on the role assignments roleGroups := make([]string, 0, len(roleAssignments)) @@ -595,19 +503,19 @@ func getAllGroupsForUser(ctx context.Context, client *http.Client, baseURL, toke return userGroups, err } } - for _, roleAssignment := range roleAssignments { - role, ok := roleMap[roleAssignment.Role.ID] - if !ok { - // Ignore role assignments to non-existent roles (shouldn't happen) + for _, ra := range roleAssignments { + if ra.Role.Name == "" { + // Ignore role assignments Keystone couldn't resolve a name for continue } - project, ok := projectMap[roleAssignment.Scope.Project.ID] - if !ok { - // Ignore role assignments to non-existent projects (shouldn't happen) - continue + switch { + case ra.Scope.Project != nil: + roleGroups = append(roleGroups, generateGroupName(*ra.Scope.Project, ra.Role, customerName)) + case ra.Scope.Domain != nil: + roleGroups = append(roleGroups, generateDomainGroupName(*ra.Scope.Domain, ra.Role, customerName)) + case ra.Scope.System != nil: + roleGroups = append(roleGroups, generateSystemGroupName(ra.Role, customerName)) } - groupName := generateGroupName(project, role, customerName, domainID) - roleGroups = append(roleGroups, groupName) } // combine local groups + sso groups + role groups @@ -665,17 +573,36 @@ func pruneDuplicates(ss []string) []string { return ns } -// generateGroupName generates a group name based on project, role, customer name, and domain ID -func generateGroupName(project project, role role, customerName, domainID string) string { +// generateGroupName generates a group name based on project scope and role +func generateGroupName(project projectScope, role namedIdentifier, customerName string) string { roleName := role.Name if roleName == "_member_" { roleName = "member" } - domainName := strings.ToLower(strings.ReplaceAll(domainID, "_", "-")) + domainName := strings.ToLower(strings.ReplaceAll(project.Domain.Name, "_", "-")) projectName := strings.ToLower(strings.ReplaceAll(project.Name, "_", "-")) return customerName + "-" + domainName + "-" + projectName + "-" + roleName } +// generateDomainGroupName generates a group name for a domain-scoped role assignment +func generateDomainGroupName(domain namedIdentifier, role namedIdentifier, customerName string) string { + roleName := role.Name + if roleName == "_member_" { + roleName = "member" + } + domainName := strings.ToLower(strings.ReplaceAll(domain.Name, "_", "-")) + return customerName + "-" + domainName + "-" + roleName +} + +// generateSystemGroupName generates a group name for a system-scoped role assignment +func generateSystemGroupName(role namedIdentifier, customerName string) string { + roleName := role.Name + if roleName == "_member_" { + roleName = "member" + } + return customerName + "-" + roleName +} + func findGroupByID(groups []keystoneGroup, groupID string) (group keystoneGroup, ok bool) { for _, group := range groups { if group.ID == groupID { diff --git a/connector/keystone/keystone_unit_test.go b/connector/keystone/keystone_unit_test.go new file mode 100644 index 0000000000..544c01c7bf --- /dev/null +++ b/connector/keystone/keystone_unit_test.go @@ -0,0 +1,199 @@ +package keystone + +import ( + "encoding/json" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" +) + +func TestGetRoleAssignments_IncludeNames(t *testing.T) { + var gotUserQuery, gotGroupQuery string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.RawQuery, "user.id=") { + gotUserQuery = r.URL.RawQuery + } + if strings.Contains(r.URL.RawQuery, "group.id=") { + gotGroupQuery = r.URL.RawQuery + } + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(struct { + RoleAssignments []roleAssignment `json:"role_assignments"` + }{}) + })) + defer ts.Close() + + logger := slog.New(slog.NewTextHandler(testDiscard{}, nil)) + + if _, err := getRoleAssignments(t.Context(), ts.Client(), ts.URL, "tok", getRoleAssignmentsOptions{userID: "u1"}, logger); err != nil { + t.Fatalf("getRoleAssignments (userID) error: %v", err) + } + if _, err := getRoleAssignments(t.Context(), ts.Client(), ts.URL, "tok", getRoleAssignmentsOptions{groupID: "g1"}, logger); err != nil { + t.Fatalf("getRoleAssignments (groupID) error: %v", err) + } + + unescapedUserQuery, err := url.QueryUnescape(gotUserQuery) + if err != nil { + t.Fatalf("failed to unescape user query: %v", err) + } + if !strings.Contains(unescapedUserQuery, "include_names") { + t.Fatalf("expected include_names in user.id request, got query: %q", gotUserQuery) + } + unescapedGroupQuery, err := url.QueryUnescape(gotGroupQuery) + if err != nil { + t.Fatalf("failed to unescape group query: %v", err) + } + if !strings.Contains(unescapedGroupQuery, "include_names") { + t.Fatalf("expected include_names in group.id request, got query: %q", gotGroupQuery) + } +} + +// multiScopeHandler serves the minimal set of Keystone endpoints +// getAllGroupsForUser needs, returning one project-scoped, one +// domain-scoped, and one system-scoped role assignment for the same user. +func multiScopeHandler(t *testing.T, projectDomainName string) http.HandlerFunc { + t.Helper() + return func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/v3/groups"): + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(groupsResponse{}) + return + case strings.Contains(r.URL.Path, "/v3/users/") && strings.HasSuffix(r.URL.Path, "/groups"): + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(groupsResponse{}) + return + case strings.HasSuffix(r.URL.Path, "/v3/role_assignments"): + body := `{ + "role_assignments": [ + { + "scope": {"project": {"id": "proj-1", "name": "My_Project", "domain": {"id": "dom-1", "name": "` + projectDomainName + `"}}}, + "user": {"id": "u1"}, + "role": {"id": "role-admin", "name": "admin"} + }, + { + "scope": {"domain": {"id": "dom-2", "name": "Customer_Domain"}, "OS-INHERIT:inherited_to": "projects"}, + "user": {"id": "u1"}, + "role": {"id": "role-cda", "name": "customer_domain_admin"} + }, + { + "scope": {"system": {"all": true}}, + "user": {"id": "u1"}, + "role": {"id": "role-pa", "name": "platform_admin"} + } + ] + }` + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(body)) + return + default: + w.WriteHeader(http.StatusNotFound) + } + } +} + +func TestGetAllGroupsForUser_MultiScopeDispatch(t *testing.T) { + ts := httptest.NewServer(multiScopeHandler(t, "Cust_Domain")) + defer ts.Close() + + logger := slog.New(slog.NewTextHandler(testDiscard{}, nil)) + info := &tokenInfo{User: userKeystone{ID: "u1", Name: "user1"}} + + groups, err := getAllGroupsForUser(t.Context(), ts.Client(), ts.URL, "tok", "cust", "login-domain", info, logger) + if err != nil { + t.Fatalf("getAllGroupsForUser error: %v", err) + } + + want := map[string]bool{ + "cust-cust-domain-my-project-admin": true, // 4-part project group + "cust-customer-domain-customer_domain_admin": true, // 3-part domain group + "cust-platform_admin": true, // 2-part system group + } + if len(groups) != len(want) { + t.Fatalf("unexpected groups: got %v, want keys %v", groups, want) + } + for _, g := range groups { + if !want[g] { + t.Errorf("unexpected group %q in result %v", g, groups) + } + } +} + +func TestGetAllGroupsForUser_ProjectOnlyUsesRowDomainNotConfig(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/v3/groups"): + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(groupsResponse{}) + return + case strings.Contains(r.URL.Path, "/v3/users/") && strings.HasSuffix(r.URL.Path, "/groups"): + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(groupsResponse{}) + return + case strings.HasSuffix(r.URL.Path, "/v3/role_assignments"): + body := `{ + "role_assignments": [ + { + "scope": {"project": {"id": "proj-1", "name": "myproject", "domain": {"id": "dom-1", "name": "RowDomain"}}}, + "user": {"id": "u1"}, + "role": {"id": "role-admin", "name": "admin"} + } + ] + }` + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(body)) + return + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer ts.Close() + + logger := slog.New(slog.NewTextHandler(testDiscard{}, nil)) + info := &tokenInfo{User: userKeystone{ID: "u1", Name: "user1"}} + + // Pass a DIFFERENT domainID (the connector's configured login domain) + // than the row's own scope.project.domain.name, to prove the row's + // data wins, not the config value. + groups, err := getAllGroupsForUser(t.Context(), ts.Client(), ts.URL, "tok", "cust", "login-domain", info, logger) + if err != nil { + t.Fatalf("getAllGroupsForUser error: %v", err) + } + + want := "cust-rowdomain-myproject-admin" + if len(groups) != 1 || groups[0] != want { + t.Fatalf("unexpected groups: got %v, want [%q]", groups, want) + } +} + +func TestGenerateGroupName(t *testing.T) { + p := projectScope{Name: "My_Project", Domain: namedIdentifier{Name: "My_Domain"}} + role := namedIdentifier{Name: "_member_"} + got := generateGroupName(p, role, "cust") + want := "cust-my-domain-my-project-member" + if got != want { + t.Fatalf("got %q, want %q", got, want) + } +} + +func TestGenerateDomainGroupName(t *testing.T) { + domain := namedIdentifier{Name: "Customer_Domain"} + role := namedIdentifier{Name: "customer_domain_admin"} + got := generateDomainGroupName(domain, role, "cust") + want := "cust-customer-domain-customer_domain_admin" + if got != want { + t.Fatalf("got %q, want %q", got, want) + } +} + +func TestGenerateSystemGroupName(t *testing.T) { + role := namedIdentifier{Name: "_member_"} + got := generateSystemGroupName(role, "cust") + want := "cust-member" + if got != want { + t.Fatalf("got %q, want %q", got, want) + } +} diff --git a/connector/keystone/types.go b/connector/keystone/types.go index 30855415d9..4ed8b2f717 100644 --- a/connector/keystone/types.go +++ b/connector/keystone/types.go @@ -121,29 +121,43 @@ type role struct { Description string `json:"description"` } -// project represents a Keystone project -type project struct { - ID string `json:"id"` - Name string `json:"name"` - DomainID string `json:"domain_id"` - Description string `json:"description"` -} - // identifierContainer represents an object with an ID type identifierContainer struct { ID string `json:"id"` } -// projectScope represents a project scope for authorization +// namedIdentifier represents an object with an ID and a Name, as returned +// by Keystone when a request includes include_names=true. +type namedIdentifier struct { + ID string `json:"id"` + Name string `json:"name"` +} + +// projectScope represents a project scope for authorization, including its +// owning domain (only populated when the role_assignments request includes +// include_names=true). type projectScope struct { - Project identifierContainer `json:"project"` + ID string `json:"id"` + Name string `json:"name"` + Domain namedIdentifier `json:"domain"` +} + +// systemScope represents a system-wide scope for authorization. +type systemScope struct { + All bool `json:"all"` } -// roleAssignment represents a role assignment +// roleAssignment represents a role assignment. Scope is exactly one of +// Project, Domain, or System, mirroring Keystone's own mutually-exclusive +// scope model. type roleAssignment struct { - Scope projectScope `json:"scope"` - User identifierContainer `json:"user"` - Role identifierContainer `json:"role"` + Scope struct { + Project *projectScope `json:"project,omitempty"` + Domain *namedIdentifier `json:"domain,omitempty"` + System *systemScope `json:"system,omitempty"` + } `json:"scope"` + User identifierContainer `json:"user"` + Role namedIdentifier `json:"role"` } // connectorData represents data stored with the connector From 7833d97055acafe9af229e686599f23595630ceb Mon Sep 17 00:00:00 2001 From: Shubham Mali Date: Thu, 10 Sep 2026 17:57:59 +0530 Subject: [PATCH 2/3] add fix for system-scoped role-groups --- connector/keystone/keystone.go | 22 ++++++- connector/keystone/keystone_unit_test.go | 79 ++++++++++++++++++++++++ connector/keystone/types.go | 5 +- 3 files changed, 102 insertions(+), 4 deletions(-) diff --git a/connector/keystone/keystone.go b/connector/keystone/keystone.go index cb3e121e5a..aee6f69f99 100644 --- a/connector/keystone/keystone.go +++ b/connector/keystone/keystone.go @@ -342,7 +342,10 @@ func getRoleAssignments(ctx context.Context, client *http.Client, baseURL, token return nil, err } if len(opts.userID) > 0 { - endpoint = fmt.Sprintf("%s?effective&include_names&user.id=%s", endpoint, opts.userID) + endpoint = fmt.Sprintf("%s?include_names&user.id=%s", endpoint, opts.userID) + if opts.effective { + endpoint += "&effective" + } } else if len(opts.groupID) > 0 { endpoint = fmt.Sprintf("%s?include_names&group.id=%s", endpoint, opts.groupID) } @@ -464,7 +467,12 @@ func getAllGroupsForUser(ctx context.Context, client *http.Client, baseURL, toke userGroupIDs = append(userGroupIDs, localGroup.ID) } - // Get user-related role assignments + // Get user-related role assignments. + // + // Two queries are needed: "effective" expands role implications (e.g. admin implies + // member/reader) and OS-Inherit domain roles down to their concrete projects, but Keystone + // drops system-scoped assignments (e.g. system.all) entirely when "effective" is set. The + // non-effective query is the only one that returns those, so both are fetched and merged. roleAssignments := []roleAssignment{} localUserRoleAssignments, err := getRoleAssignments(ctx, client, baseURL, token, getRoleAssignmentsOptions{ userID: tokenInfo.User.ID, @@ -475,6 +483,16 @@ func getAllGroupsForUser(ctx context.Context, client *http.Client, baseURL, toke } roleAssignments = append(roleAssignments, localUserRoleAssignments...) + effectiveUserRoleAssignments, err := getRoleAssignments(ctx, client, baseURL, token, getRoleAssignmentsOptions{ + userID: tokenInfo.User.ID, + effective: true, + }, logger) + if err != nil { + logger.Error("failed to fetch effective role assignments for user", "userID", tokenInfo.User.ID, "error", err) + return userGroups, err + } + roleAssignments = append(roleAssignments, effectiveUserRoleAssignments...) + // Get group-related role assignments for _, groupID := range userGroupIDs { groupRoleAssignments, err := getRoleAssignments(ctx, client, baseURL, token, getRoleAssignmentsOptions{ diff --git a/connector/keystone/keystone_unit_test.go b/connector/keystone/keystone_unit_test.go index 544c01c7bf..054b30ad58 100644 --- a/connector/keystone/keystone_unit_test.go +++ b/connector/keystone/keystone_unit_test.go @@ -169,6 +169,85 @@ func TestGetAllGroupsForUser_ProjectOnlyUsesRowDomainNotConfig(t *testing.T) { } } +// effectiveDropsSystemScopeHandler mimics real Keystone behavior observed against +// a live deployment: a GET /v3/role_assignments?user.id=... request without +// "effective" returns a system-scoped assignment, but the same request with +// "effective" set silently omits it (system-scoped assignments have no +// project/domain to expand into). The connector must merge both queries so +// system-scoped roles aren't lost. +func effectiveDropsSystemScopeHandler(t *testing.T) http.HandlerFunc { + t.Helper() + return func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/v3/groups"): + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(groupsResponse{}) + return + case strings.Contains(r.URL.Path, "/v3/users/") && strings.HasSuffix(r.URL.Path, "/groups"): + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(groupsResponse{}) + return + case strings.HasSuffix(r.URL.Path, "/v3/role_assignments"): + w.WriteHeader(http.StatusOK) + if strings.Contains(r.URL.RawQuery, "effective") { + _, _ = w.Write([]byte(`{ + "role_assignments": [ + { + "scope": {"project": {"id": "proj-1", "name": "My_Project", "domain": {"id": "dom-1", "name": "Cust_Domain"}}}, + "user": {"id": "u1"}, + "role": {"id": "role-admin", "name": "admin"} + } + ] + }`)) + return + } + _, _ = w.Write([]byte(`{ + "role_assignments": [ + { + "scope": {"project": {"id": "proj-1", "name": "My_Project", "domain": {"id": "dom-1", "name": "Cust_Domain"}}}, + "user": {"id": "u1"}, + "role": {"id": "role-admin", "name": "admin"} + }, + { + "scope": {"system": {"all": true}}, + "user": {"id": "u1"}, + "role": {"id": "role-pa", "name": "platform_admin"} + } + ] + }`)) + return + default: + w.WriteHeader(http.StatusNotFound) + } + } +} + +func TestGetAllGroupsForUser_EffectiveDoesNotDropSystemScope(t *testing.T) { + ts := httptest.NewServer(effectiveDropsSystemScopeHandler(t)) + defer ts.Close() + + logger := slog.New(slog.NewTextHandler(testDiscard{}, nil)) + info := &tokenInfo{User: userKeystone{ID: "u1", Name: "user1"}} + + groups, err := getAllGroupsForUser(t.Context(), ts.Client(), ts.URL, "tok", "cust", "login-domain", info, logger) + if err != nil { + t.Fatalf("getAllGroupsForUser error: %v", err) + } + + want := map[string]bool{ + "cust-cust-domain-my-project-admin": true, + "cust-platform_admin": true, + } + if len(groups) != len(want) { + t.Fatalf("unexpected groups: got %v, want keys %v", groups, want) + } + for _, g := range groups { + if !want[g] { + t.Errorf("unexpected group %q in result %v", g, groups) + } + } +} + func TestGenerateGroupName(t *testing.T) { p := projectScope{Name: "My_Project", Domain: namedIdentifier{Name: "My_Domain"}} role := namedIdentifier{Name: "_member_"} diff --git a/connector/keystone/types.go b/connector/keystone/types.go index 4ed8b2f717..b34cb897c6 100644 --- a/connector/keystone/types.go +++ b/connector/keystone/types.go @@ -167,6 +167,7 @@ type connectorData struct { // getRoleAssignmentsOptions represents options for getting role assignments type getRoleAssignmentsOptions struct { - userID string - groupID string + userID string + groupID string + effective bool } From 76260e321cb77c8efd5ee765777f75c80bed3865 Mon Sep 17 00:00:00 2001 From: Shubham Mali Date: Thu, 10 Sep 2026 18:24:06 +0530 Subject: [PATCH 3/3] remove effective param --- connector/keystone/keystone.go | 18 ------ connector/keystone/keystone_unit_test.go | 79 ------------------------ connector/keystone/types.go | 5 +- 3 files changed, 2 insertions(+), 100 deletions(-) diff --git a/connector/keystone/keystone.go b/connector/keystone/keystone.go index aee6f69f99..77c78f8b3b 100644 --- a/connector/keystone/keystone.go +++ b/connector/keystone/keystone.go @@ -343,9 +343,6 @@ func getRoleAssignments(ctx context.Context, client *http.Client, baseURL, token } if len(opts.userID) > 0 { endpoint = fmt.Sprintf("%s?include_names&user.id=%s", endpoint, opts.userID) - if opts.effective { - endpoint += "&effective" - } } else if len(opts.groupID) > 0 { endpoint = fmt.Sprintf("%s?include_names&group.id=%s", endpoint, opts.groupID) } @@ -468,11 +465,6 @@ func getAllGroupsForUser(ctx context.Context, client *http.Client, baseURL, toke } // Get user-related role assignments. - // - // Two queries are needed: "effective" expands role implications (e.g. admin implies - // member/reader) and OS-Inherit domain roles down to their concrete projects, but Keystone - // drops system-scoped assignments (e.g. system.all) entirely when "effective" is set. The - // non-effective query is the only one that returns those, so both are fetched and merged. roleAssignments := []roleAssignment{} localUserRoleAssignments, err := getRoleAssignments(ctx, client, baseURL, token, getRoleAssignmentsOptions{ userID: tokenInfo.User.ID, @@ -483,16 +475,6 @@ func getAllGroupsForUser(ctx context.Context, client *http.Client, baseURL, toke } roleAssignments = append(roleAssignments, localUserRoleAssignments...) - effectiveUserRoleAssignments, err := getRoleAssignments(ctx, client, baseURL, token, getRoleAssignmentsOptions{ - userID: tokenInfo.User.ID, - effective: true, - }, logger) - if err != nil { - logger.Error("failed to fetch effective role assignments for user", "userID", tokenInfo.User.ID, "error", err) - return userGroups, err - } - roleAssignments = append(roleAssignments, effectiveUserRoleAssignments...) - // Get group-related role assignments for _, groupID := range userGroupIDs { groupRoleAssignments, err := getRoleAssignments(ctx, client, baseURL, token, getRoleAssignmentsOptions{ diff --git a/connector/keystone/keystone_unit_test.go b/connector/keystone/keystone_unit_test.go index 054b30ad58..544c01c7bf 100644 --- a/connector/keystone/keystone_unit_test.go +++ b/connector/keystone/keystone_unit_test.go @@ -169,85 +169,6 @@ func TestGetAllGroupsForUser_ProjectOnlyUsesRowDomainNotConfig(t *testing.T) { } } -// effectiveDropsSystemScopeHandler mimics real Keystone behavior observed against -// a live deployment: a GET /v3/role_assignments?user.id=... request without -// "effective" returns a system-scoped assignment, but the same request with -// "effective" set silently omits it (system-scoped assignments have no -// project/domain to expand into). The connector must merge both queries so -// system-scoped roles aren't lost. -func effectiveDropsSystemScopeHandler(t *testing.T) http.HandlerFunc { - t.Helper() - return func(w http.ResponseWriter, r *http.Request) { - switch { - case strings.HasSuffix(r.URL.Path, "/v3/groups"): - w.WriteHeader(http.StatusOK) - _ = json.NewEncoder(w).Encode(groupsResponse{}) - return - case strings.Contains(r.URL.Path, "/v3/users/") && strings.HasSuffix(r.URL.Path, "/groups"): - w.WriteHeader(http.StatusOK) - _ = json.NewEncoder(w).Encode(groupsResponse{}) - return - case strings.HasSuffix(r.URL.Path, "/v3/role_assignments"): - w.WriteHeader(http.StatusOK) - if strings.Contains(r.URL.RawQuery, "effective") { - _, _ = w.Write([]byte(`{ - "role_assignments": [ - { - "scope": {"project": {"id": "proj-1", "name": "My_Project", "domain": {"id": "dom-1", "name": "Cust_Domain"}}}, - "user": {"id": "u1"}, - "role": {"id": "role-admin", "name": "admin"} - } - ] - }`)) - return - } - _, _ = w.Write([]byte(`{ - "role_assignments": [ - { - "scope": {"project": {"id": "proj-1", "name": "My_Project", "domain": {"id": "dom-1", "name": "Cust_Domain"}}}, - "user": {"id": "u1"}, - "role": {"id": "role-admin", "name": "admin"} - }, - { - "scope": {"system": {"all": true}}, - "user": {"id": "u1"}, - "role": {"id": "role-pa", "name": "platform_admin"} - } - ] - }`)) - return - default: - w.WriteHeader(http.StatusNotFound) - } - } -} - -func TestGetAllGroupsForUser_EffectiveDoesNotDropSystemScope(t *testing.T) { - ts := httptest.NewServer(effectiveDropsSystemScopeHandler(t)) - defer ts.Close() - - logger := slog.New(slog.NewTextHandler(testDiscard{}, nil)) - info := &tokenInfo{User: userKeystone{ID: "u1", Name: "user1"}} - - groups, err := getAllGroupsForUser(t.Context(), ts.Client(), ts.URL, "tok", "cust", "login-domain", info, logger) - if err != nil { - t.Fatalf("getAllGroupsForUser error: %v", err) - } - - want := map[string]bool{ - "cust-cust-domain-my-project-admin": true, - "cust-platform_admin": true, - } - if len(groups) != len(want) { - t.Fatalf("unexpected groups: got %v, want keys %v", groups, want) - } - for _, g := range groups { - if !want[g] { - t.Errorf("unexpected group %q in result %v", g, groups) - } - } -} - func TestGenerateGroupName(t *testing.T) { p := projectScope{Name: "My_Project", Domain: namedIdentifier{Name: "My_Domain"}} role := namedIdentifier{Name: "_member_"} diff --git a/connector/keystone/types.go b/connector/keystone/types.go index b34cb897c6..4ed8b2f717 100644 --- a/connector/keystone/types.go +++ b/connector/keystone/types.go @@ -167,7 +167,6 @@ type connectorData struct { // getRoleAssignmentsOptions represents options for getting role assignments type getRoleAssignmentsOptions struct { - userID string - groupID string - effective bool + userID string + groupID string }