From 14db59a1b5de0c128613bf590dd6235c36ed37e5 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 19 Feb 2026 12:20:12 +0100 Subject: [PATCH 1/9] fix: wrap groups elevate response in response key per live API Live API exploration revealed the POST /api/access/elevate/groups response IS wrapped in a "response" key (contrary to initial spec). Also documents group session shape from GET /api/access/sessions including the target field with type "groups". --- CLAUDE.md | 3 + docs/entra-groups-api-findings.md | 68 ++++++++++ internal/sca/models/groups.go | 43 +++++++ internal/sca/service.go | 53 ++++++++ internal/sca/service_test.go | 207 ++++++++++++++++++++++++++++++ 5 files changed, 374 insertions(+) create mode 100644 docs/entra-groups-api-findings.md create mode 100644 internal/sca/models/groups.go diff --git a/CLAUDE.md b/CLAUDE.md index 931c7ff..ea796fa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -40,6 +40,8 @@ Custom `SCAAccessService` follows SDK conventions: - `POST /api/access/elevate` — request JIT elevation (AWS responses include `accessCredentials` JSON string) - `GET /api/access/sessions` — list active sessions - `POST /api/access/sessions/revoke` — revoke sessions by ID (request: `sessionIds[]`, response: `SessionRevocationInfo[]`) + - `GET /api/access/{CSP}/eligibility/groups` — list eligible Entra ID groups (response: `groupId`/`groupName`/`directoryId`) + - `POST /api/access/elevate/groups` — request group membership elevation (response wrapped in `response` key, same as cloud elevation) - **Headers:** `Authorization: Bearer {jwt}`, `X-API-Version: 2.0`, `Content-Type: application/json` ## Testing @@ -54,6 +56,7 @@ Custom `SCAAccessService` follows SDK conventions: - `Iilun/survey/v2` for interactive prompts - `grant env` — performs elevation, outputs only `export` statements (no human text); usage: `eval $(grant env --provider aws)` - `grant revoke` — revoke sessions: direct (`grant revoke `), `--all`, or interactive multi-select; `--yes` skips confirmation +- `grant groups` — Entra ID group membership elevation: interactive or direct (`--group "name"`); always targets Azure; uses separate API endpoints (`/eligibility/groups`, `/elevate/groups`) - Multi-CSP: omitting `--provider` fetches eligibility from all supported CSPs and merges results - `fetchEligibility()` and `resolveTargetCSP()` in `cmd/root.go` — shared by root, env, and favorites diff --git a/docs/entra-groups-api-findings.md b/docs/entra-groups-api-findings.md new file mode 100644 index 0000000..ce6a230 --- /dev/null +++ b/docs/entra-groups-api-findings.md @@ -0,0 +1,68 @@ +# Entra ID Groups API Findings + +Captured from live SCA API on 2026-02-19. + +## POST /api/access/elevate/groups — Response + +**Finding:** Response IS wrapped in a `"response"` key (contrary to initial spec assumption). + +```json +{ + "response": { + "directoryId": "29cb7961-e16d-42c7-8ade-1794bbb76782", + "csp": "AZURE", + "results": [ + { + "sessionId": "93b65b90-8c56-4243-92f5-f1a8b7cd38a6", + "groupId": "d554b344-5e88-4299-9b5a-11a2b91f19f7" + } + ] + } +} +``` + +- Fields use **camelCase** (`directoryId`, `sessionId`, `groupId`) +- Response wrapping matches cloud elevation pattern + +## GET /api/access/sessions — Group Sessions + +**Finding:** Group sessions appear in the standard sessions endpoint with a `target` field. + +```json +{ + "response": [ + { + "session_id": "93510f71-c958-489a-941c-5568bc19d468", + "user_id": "tim.schindler@cyberark.cloud.40562", + "csp": "AZURE", + "workspace_id": "29cb7961-e16d-42c7-8ade-1794bbb76782", + "session_duration": 3600, + "target": { + "id": "d554b344-5e88-4299-9b5a-11a2b91f19f7", + "type": "groups" + } + } + ], + "total": 2 +} +``` + +### Key Observations + +| Field | Value | Notes | +|-------|-------|-------| +| `target.type` | `"groups"` | Distinguishes from cloud sessions | +| `target.id` | group UUID | Matches `groupId` from eligibility | +| `workspace_id` | directory UUID | Contains the Entra directory ID | +| `role_id` | **absent** | Not present for group sessions | +| `session_duration` | `3600` | Integer seconds, same as cloud | +| `csp` | `"AZURE"` | Always Azure for Entra groups | + +### Field Casing + +- Session-level fields: **snake_case** (`session_id`, `workspace_id`, `session_duration`) +- Target sub-object fields: **lowercase** (`id`, `type`) — no snake_case or camelCase distinction needed + +### Cloud Sessions (for comparison) + +Cloud sessions do NOT include a `target` field. The absence of `target` (or `target == nil`) indicates a cloud console session. diff --git a/internal/sca/models/groups.go b/internal/sca/models/groups.go new file mode 100644 index 0000000..578e588 --- /dev/null +++ b/internal/sca/models/groups.go @@ -0,0 +1,43 @@ +package models + +// GroupsEligibleTarget represents an Entra ID group the user is eligible to join. +type GroupsEligibleTarget struct { + DirectoryID string `json:"directoryId"` + DirectoryName string `json:"-"` // Set programmatically from cloud eligibility cross-reference + GroupID string `json:"groupId"` + GroupName string `json:"groupName"` +} + +// GroupsEligibilityResponse is the response from GET /api/access/{CSP}/eligibility/groups. +type GroupsEligibilityResponse struct { + Response []GroupsEligibleTarget `json:"response"` + NextToken *string `json:"nextToken"` + Total int `json:"total"` +} + +// GroupsElevateTarget represents a single group target for elevation. +type GroupsElevateTarget struct { + GroupID string `json:"groupId"` +} + +// GroupsElevateRequest is the request body for POST /api/access/elevate/groups. +type GroupsElevateRequest struct { + DirectoryID string `json:"directoryId"` + CSP CSP `json:"csp"` + Targets []GroupsElevateTarget `json:"targets"` +} + +// GroupsElevateTargetResult is the per-target result of a groups elevation request. +type GroupsElevateTargetResult struct { + GroupID string `json:"groupId"` + SessionID string `json:"sessionId"` + ErrorInfo *ErrorInfo `json:"errorInfo"` +} + +// GroupsElevateResponse is the inner response from POST /api/access/elevate/groups. +// Note: The wire format wraps this in a "response" key (same as cloud elevation). +type GroupsElevateResponse struct { + DirectoryID string `json:"directoryId"` + CSP CSP `json:"csp"` + Results []GroupsElevateTargetResult `json:"results"` +} diff --git a/internal/sca/service.go b/internal/sca/service.go index 5e8cafa..9e2c072 100644 --- a/internal/sca/service.go +++ b/internal/sca/service.go @@ -205,3 +205,56 @@ func (s *SCAAccessService) ListSessions(ctx context.Context, csp *models.CSP) (* return &result, nil } + +// ListGroupsEligibility retrieves eligible Entra ID groups for the specified CSP. +// GET /api/access/{CSP}/eligibility/groups +func (s *SCAAccessService) ListGroupsEligibility(ctx context.Context, csp models.CSP) (*models.GroupsEligibilityResponse, error) { + route := fmt.Sprintf("/api/access/%s/eligibility/groups", csp) + + resp, err := s.httpClient.Get(ctx, route, nil) + if err != nil { + return nil, fmt.Errorf("failed to get groups eligibility: %w", err) + } + defer resp.Body.Close() + + if err := checkResponse(resp, "groups eligibility request"); err != nil { + return nil, err + } + + var result models.GroupsEligibilityResponse + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return nil, fmt.Errorf("failed to decode groups eligibility response: %w", err) + } + + return &result, nil +} + +// ElevateGroups requests JIT elevation for the specified Entra ID groups. +// POST /api/access/elevate/groups +func (s *SCAAccessService) ElevateGroups(ctx context.Context, req *models.GroupsElevateRequest) (*models.GroupsElevateResponse, error) { + if req == nil { + return nil, fmt.Errorf("groups elevate request cannot be nil") + } + if len(req.Targets) == 0 { + return nil, fmt.Errorf("groups elevate request must contain at least one target") + } + + resp, err := s.httpClient.Post(ctx, "/api/access/elevate/groups", req) + if err != nil { + return nil, fmt.Errorf("failed to elevate groups: %w", err) + } + defer resp.Body.Close() + + if err := checkResponse(resp, "groups elevate request"); err != nil { + return nil, err + } + + var wrapper struct { + Response models.GroupsElevateResponse `json:"response"` + } + if err := json.NewDecoder(resp.Body).Decode(&wrapper); err != nil { + return nil, fmt.Errorf("failed to decode groups elevate response: %w", err) + } + + return &wrapper.Response, nil +} diff --git a/internal/sca/service_test.go b/internal/sca/service_test.go index 8bf2197..d5d45b5 100644 --- a/internal/sca/service_test.go +++ b/internal/sca/service_test.go @@ -544,3 +544,210 @@ func TestListSessions_WithCSPFilter(t *testing.T) { t.Errorf("expected CSP AZURE, got %s", result.Response[0].CSP) } } + +func TestListGroupsEligibility_Success(t *testing.T) { + resp := models.GroupsEligibilityResponse{ + Response: []models.GroupsEligibleTarget{ + { + DirectoryID: "dir1", + GroupID: "grp1", + GroupName: "Engineering", + }, + }, + Total: 1, + } + + body, _ := json.Marshal(resp) + mock := &mockHTTPClient{ + getResponse: &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(string(body))), + }, + } + + svc := &SCAAccessService{httpClient: mock} + result, err := svc.ListGroupsEligibility(context.Background(), models.CSPAzure) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if result == nil { + t.Fatal("expected non-nil result") + } + if len(result.Response) != 1 { + t.Errorf("expected 1 group, got %d", len(result.Response)) + } + if result.Response[0].GroupName != "Engineering" { + t.Errorf("expected group name Engineering, got %s", result.Response[0].GroupName) + } +} + +func TestListGroupsEligibility_Empty(t *testing.T) { + resp := models.GroupsEligibilityResponse{ + Response: []models.GroupsEligibleTarget{}, + Total: 0, + } + + body, _ := json.Marshal(resp) + mock := &mockHTTPClient{ + getResponse: &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(string(body))), + }, + } + + svc := &SCAAccessService{httpClient: mock} + result, err := svc.ListGroupsEligibility(context.Background(), models.CSPAzure) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if len(result.Response) != 0 { + t.Errorf("expected 0 groups, got %d", len(result.Response)) + } +} + +func TestListGroupsEligibility_HTTPError(t *testing.T) { + mock := &mockHTTPClient{ + getResponse: &http.Response{ + StatusCode: http.StatusUnauthorized, + Body: io.NopCloser(strings.NewReader(`{"error": "unauthorized"}`)), + }, + } + + svc := &SCAAccessService{httpClient: mock} + _, err := svc.ListGroupsEligibility(context.Background(), models.CSPAzure) + + if err == nil { + t.Fatal("expected error for 401 response") + } + if !strings.Contains(err.Error(), "401") { + t.Errorf("expected error to mention status code 401, got: %v", err) + } +} + +func TestElevateGroups_Success(t *testing.T) { + req := &models.GroupsElevateRequest{ + DirectoryID: "dir1", + CSP: models.CSPAzure, + Targets: []models.GroupsElevateTarget{{GroupID: "grp1"}}, + } + + // Groups elevate response IS wrapped in "response" key (confirmed via live API) + resp := struct { + Response models.GroupsElevateResponse `json:"response"` + }{ + Response: models.GroupsElevateResponse{ + DirectoryID: "dir1", + CSP: models.CSPAzure, + Results: []models.GroupsElevateTargetResult{ + { + GroupID: "grp1", + SessionID: "sess1", + }, + }, + }, + } + + body, _ := json.Marshal(resp) + mock := &mockHTTPClient{ + postResponse: &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(string(body))), + }, + } + + svc := &SCAAccessService{httpClient: mock} + result, err := svc.ElevateGroups(context.Background(), req) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if result == nil { + t.Fatal("expected non-nil result") + } + if len(result.Results) != 1 { + t.Errorf("expected 1 result, got %d", len(result.Results)) + } + if result.Results[0].SessionID != "sess1" { + t.Errorf("expected session ID sess1, got %s", result.Results[0].SessionID) + } +} + +func TestElevateGroups_WithError(t *testing.T) { + req := &models.GroupsElevateRequest{ + DirectoryID: "dir1", + CSP: models.CSPAzure, + Targets: []models.GroupsElevateTarget{{GroupID: "grp1"}}, + } + + resp := struct { + Response models.GroupsElevateResponse `json:"response"` + }{ + Response: models.GroupsElevateResponse{ + DirectoryID: "dir1", + CSP: models.CSPAzure, + Results: []models.GroupsElevateTargetResult{ + { + GroupID: "grp1", + SessionID: "", + ErrorInfo: &models.ErrorInfo{ + Code: "ERR_INELIGIBLE", + Message: "Not eligible", + Description: "User not eligible for this group", + }, + }, + }, + }, + } + + body, _ := json.Marshal(resp) + mock := &mockHTTPClient{ + postResponse: &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(string(body))), + }, + } + + svc := &SCAAccessService{httpClient: mock} + result, err := svc.ElevateGroups(context.Background(), req) + + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if result.Results[0].ErrorInfo == nil { + t.Error("expected error info, got nil") + } +} + +func TestElevateGroups_NilRequest(t *testing.T) { + mock := &mockHTTPClient{} + svc := &SCAAccessService{httpClient: mock} + + _, err := svc.ElevateGroups(context.Background(), nil) + if err == nil { + t.Fatal("expected error for nil request") + } + if !strings.Contains(err.Error(), "nil") { + t.Errorf("expected error to mention nil, got: %v", err) + } +} + +func TestElevateGroups_EmptyTargets(t *testing.T) { + req := &models.GroupsElevateRequest{ + DirectoryID: "dir1", + CSP: models.CSPAzure, + Targets: []models.GroupsElevateTarget{}, + } + + mock := &mockHTTPClient{} + svc := &SCAAccessService{httpClient: mock} + + _, err := svc.ElevateGroups(context.Background(), req) + if err == nil { + t.Fatal("expected error for empty targets") + } + if !strings.Contains(err.Error(), "target") { + t.Errorf("expected error about targets, got: %v", err) + } +} From 9f9704aa61d5fbf2b30bdb0e2ed50e4a1765b2d6 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 19 Feb 2026 12:21:46 +0100 Subject: [PATCH 2/9] feat: add SessionTarget and IsGroupSession() to session model Add Target field to SessionInfo for distinguishing group sessions from cloud sessions. Group sessions have target.type "groups" with target.id containing the group UUID. --- internal/sca/models/session.go | 33 +++++-- internal/sca/models/session_test.go | 136 ++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 6 deletions(-) diff --git a/internal/sca/models/session.go b/internal/sca/models/session.go index a6242fd..706d82e 100644 --- a/internal/sca/models/session.go +++ b/internal/sca/models/session.go @@ -1,16 +1,37 @@ package models +const ( + // TargetTypeGroups indicates an Entra ID group membership session. + TargetTypeGroups = "groups" + // TargetTypeCloudConsole indicates a cloud console elevation session. + TargetTypeCloudConsole = "cloud_console" +) + +// SessionTarget identifies what a session is targeting (group or cloud console). +// Present on group sessions; may be absent on older cloud sessions. +type SessionTarget struct { + ID string `json:"id"` + Type string `json:"type"` +} + // SessionInfo represents an active elevated session. // Note: The live SCA API uses snake_case field names, which differs from the // OpenAPI spec's camelCase. The role_id field contains the role display name // (e.g., "User Access Administrator"), not an ARM resource path. +// For group sessions, role_id is absent and Target.Type is "groups". type SessionInfo struct { - SessionID string `json:"session_id"` - UserID string `json:"user_id"` - CSP CSP `json:"csp"` - WorkspaceID string `json:"workspace_id"` - RoleID string `json:"role_id"` - SessionDuration int `json:"session_duration"` + SessionID string `json:"session_id"` + UserID string `json:"user_id"` + CSP CSP `json:"csp"` + WorkspaceID string `json:"workspace_id"` + RoleID string `json:"role_id"` + SessionDuration int `json:"session_duration"` + Target *SessionTarget `json:"target,omitempty"` +} + +// IsGroupSession returns true if this session is for Entra ID group membership. +func (s SessionInfo) IsGroupSession() bool { + return s.Target != nil && s.Target.Type == TargetTypeGroups } // SessionsResponse is the response from GET /api/access/sessions. diff --git a/internal/sca/models/session_test.go b/internal/sca/models/session_test.go index 91cdbfd..f9411fa 100644 --- a/internal/sca/models/session_test.go +++ b/internal/sca/models/session_test.go @@ -141,6 +141,142 @@ func TestSessionsResponse_Empty(t *testing.T) { } } +func TestSessionInfo_WithTarget(t *testing.T) { + t.Parallel() + tests := []struct { + name string + json string + wantTarget bool + wantTargetType string + wantTargetID string + wantIsGroup bool + }{ + { + name: "cloud session without target field", + json: `{ + "session_id": "s1", + "user_id": "user@example.com", + "csp": "AZURE", + "workspace_id": "/subscriptions/sub-1", + "role_id": "Contributor", + "session_duration": 3600 + }`, + wantTarget: false, + wantIsGroup: false, + }, + { + name: "cloud session with cloud_console target", + json: `{ + "session_id": "s2", + "user_id": "user@example.com", + "csp": "AZURE", + "workspace_id": "/subscriptions/sub-1", + "role_id": "Contributor", + "session_duration": 3600, + "target": {"id": "x", "type": "cloud_console"} + }`, + wantTarget: true, + wantTargetType: "cloud_console", + wantTargetID: "x", + wantIsGroup: false, + }, + { + name: "group session with groups target", + json: `{ + "session_id": "s3", + "user_id": "user@example.com", + "csp": "AZURE", + "workspace_id": "29cb7961-e16d-42c7-8ade-1794bbb76782", + "session_duration": 3600, + "target": {"id": "d554b344-5e88-4299-9b5a-11a2b91f19f7", "type": "groups"} + }`, + wantTarget: true, + wantTargetType: "groups", + wantTargetID: "d554b344-5e88-4299-9b5a-11a2b91f19f7", + wantIsGroup: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var session SessionInfo + if err := json.Unmarshal([]byte(tt.json), &session); err != nil { + t.Fatalf("unmarshal error: %v", err) + } + + if tt.wantTarget { + if session.Target == nil { + t.Fatal("expected non-nil Target") + } + if session.Target.Type != tt.wantTargetType { + t.Errorf("Target.Type = %q, want %q", session.Target.Type, tt.wantTargetType) + } + if session.Target.ID != tt.wantTargetID { + t.Errorf("Target.ID = %q, want %q", session.Target.ID, tt.wantTargetID) + } + } else { + if session.Target != nil { + t.Errorf("expected nil Target, got %+v", session.Target) + } + } + + if got := session.IsGroupSession(); got != tt.wantIsGroup { + t.Errorf("IsGroupSession() = %v, want %v", got, tt.wantIsGroup) + } + }) + } +} + +func TestSessionInfo_IsGroupSession_ZeroValue(t *testing.T) { + t.Parallel() + var s SessionInfo + if s.IsGroupSession() { + t.Error("zero-value SessionInfo should not be a group session") + } +} + +func TestSessionInfo_GroupSessionRealPayload(t *testing.T) { + t.Parallel() + // Exact payload captured from live SCA API on 2026-02-19 + jsonInput := `{ + "response": [ + { + "session_id": "93510f71-c958-489a-941c-5568bc19d468", + "user_id": "tim.schindler@cyberark.cloud.40562", + "csp": "AZURE", + "workspace_id": "29cb7961-e16d-42c7-8ade-1794bbb76782", + "session_duration": 3600, + "target": {"id": "d554b344-5e88-4299-9b5a-11a2b91f19f7", "type": "groups"} + } + ], + "total": 1 + }` + + var resp SessionsResponse + if err := json.Unmarshal([]byte(jsonInput), &resp); err != nil { + t.Fatalf("unmarshal error: %v", err) + } + + if len(resp.Response) != 1 { + t.Fatalf("Response length = %d, want 1", len(resp.Response)) + } + + session := resp.Response[0] + if !session.IsGroupSession() { + t.Error("expected IsGroupSession() == true") + } + if session.Target.ID != "d554b344-5e88-4299-9b5a-11a2b91f19f7" { + t.Errorf("Target.ID = %q, want group UUID", session.Target.ID) + } + if session.RoleID != "" { + t.Errorf("RoleID = %q, want empty for group sessions", session.RoleID) + } + if session.WorkspaceID != "29cb7961-e16d-42c7-8ade-1794bbb76782" { + t.Errorf("WorkspaceID = %q, want directory UUID", session.WorkspaceID) + } +} + func TestSessionInfo_RealAPIPayload(t *testing.T) { t.Parallel() // Exact payload captured from live SCA API on 2026-02-10 From 17cf2236ae4ec41ef7c709bcb4f0122e52bc16bb Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 19 Feb 2026 12:23:46 +0100 Subject: [PATCH 3/9] feat: display group sessions separately in grant status Group sessions are now shown under a "Groups sessions:" header instead of being mixed with cloud sessions. FormatSessionOption uses IsGroupSession() to format them as "Group: in ". --- cmd/status.go | 31 ++++++-- cmd/status_test.go | 101 +++++++++++++++++++++++++++ internal/ui/session_selector.go | 10 +++ internal/ui/session_selector_test.go | 36 ++++++++++ 4 files changed, 172 insertions(+), 6 deletions(-) diff --git a/cmd/status.go b/cmd/status.go index f07daa9..cb83a97 100644 --- a/cmd/status.go +++ b/cmd/status.go @@ -81,14 +81,33 @@ func runStatus(cmd *cobra.Command, authLoader authLoader, sessionLister sessionL return nil } - // Group sessions by provider - sessionsByProvider := groupSessionsByProvider(data.sessions.Response) + // Separate cloud sessions from group sessions + var cloudSessions, groupSessions []scamodels.SessionInfo + for _, s := range data.sessions.Response { + if s.IsGroupSession() { + groupSessions = append(groupSessions, s) + } else { + cloudSessions = append(cloudSessions, s) + } + } - // Display grouped sessions fmt.Fprintf(cmd.OutOrStdout(), "\n") - for _, p := range sortedProviders(sessionsByProvider) { - fmt.Fprintf(cmd.OutOrStdout(), "%s sessions:\n", formatProviderName(p)) - for _, session := range sessionsByProvider[p] { + + // Display cloud sessions grouped by provider + if len(cloudSessions) > 0 { + sessionsByProvider := groupSessionsByProvider(cloudSessions) + for _, p := range sortedProviders(sessionsByProvider) { + fmt.Fprintf(cmd.OutOrStdout(), "%s sessions:\n", formatProviderName(p)) + for _, session := range sessionsByProvider[p] { + fmt.Fprintf(cmd.OutOrStdout(), " %s\n", ui.FormatSessionOption(session, data.nameMap)) + } + } + } + + // Display group sessions + if len(groupSessions) > 0 { + fmt.Fprintf(cmd.OutOrStdout(), "Groups sessions:\n") + for _, session := range groupSessions { fmt.Fprintf(cmd.OutOrStdout(), " %s\n", ui.FormatSessionOption(session, data.nameMap)) } } diff --git a/cmd/status_test.go b/cmd/status_test.go index 4457e76..11aff6b 100644 --- a/cmd/status_test.go +++ b/cmd/status_test.go @@ -449,6 +449,107 @@ func TestStatusCommand(t *testing.T) { }, wantErr: false, }, + { + name: "mixed cloud and group sessions", + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{ + Token: "test-jwt", + Username: "tim@iosharp.com", + ExpiresIn: expiresIn, + }, + loadErr: nil, + } + }, + setupSvc: func() *mockSessionLister { + return &mockSessionLister{ + sessions: &scamodels.SessionsResponse{ + Response: []scamodels.SessionInfo{ + { + SessionID: "cloud-session-1", + UserID: "tim@iosharp.com", + CSP: scamodels.CSPAzure, + WorkspaceID: "/subscriptions/sub-1", + RoleID: "Contributor", + SessionDuration: 3600, + }, + { + SessionID: "group-session-1", + UserID: "tim@iosharp.com", + CSP: scamodels.CSPAzure, + WorkspaceID: "29cb7961-dir-uuid", + SessionDuration: 3600, + Target: &scamodels.SessionTarget{ID: "group-uuid-1", Type: scamodels.TargetTypeGroups}, + }, + }, + Total: 2, + }, + listErr: nil, + } + }, + setupEligibility: func() *mockEligibilityLister { + return &mockEligibilityLister{ + response: &scamodels.EligibilityResponse{ + Response: []scamodels.EligibleTarget{ + {WorkspaceID: "/subscriptions/sub-1", WorkspaceName: "My Subscription"}, + }, + }, + } + }, + wantContain: []string{ + "Azure sessions:", + "Contributor on My Subscription (/subscriptions/sub-1)", + "session: cloud-session-1", + "Groups sessions:", + "Group: group-uuid-1 in 29cb7961-dir-uuid", + "session: group-session-1", + }, + wantErr: false, + }, + { + name: "only group sessions", + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{ + Token: "test-jwt", + Username: "tim@iosharp.com", + ExpiresIn: expiresIn, + }, + loadErr: nil, + } + }, + setupSvc: func() *mockSessionLister { + return &mockSessionLister{ + sessions: &scamodels.SessionsResponse{ + Response: []scamodels.SessionInfo{ + { + SessionID: "group-session-1", + UserID: "tim@iosharp.com", + CSP: scamodels.CSPAzure, + WorkspaceID: "dir-uuid-123", + SessionDuration: 1800, + Target: &scamodels.SessionTarget{ID: "grp-uuid", Type: scamodels.TargetTypeGroups}, + }, + }, + Total: 1, + }, + listErr: nil, + } + }, + setupEligibility: func() *mockEligibilityLister { + return &mockEligibilityLister{} + }, + wantContain: []string{ + "Groups sessions:", + "Group: grp-uuid in dir-uuid-123", + "duration: 30m", + }, + wantNotContain: []string{ + "Azure sessions:", + "AWS sessions:", + }, + wantErr: false, + }, { name: "invalid provider flag", setupAuth: func() *mockAuthLoader { diff --git a/internal/ui/session_selector.go b/internal/ui/session_selector.go index 1c0da92..02b9b46 100644 --- a/internal/ui/session_selector.go +++ b/internal/ui/session_selector.go @@ -19,6 +19,16 @@ func FormatSessionOption(session models.SessionInfo, nameMap map[string]string) durationStr = fmt.Sprintf("%dm", durationMin) } + if session.IsGroupSession() { + directory := session.WorkspaceID + if nameMap != nil { + if name, ok := nameMap[session.WorkspaceID]; ok { + directory = name + } + } + return fmt.Sprintf("Group: %s in %s - duration: %s (session: %s)", session.Target.ID, directory, durationStr, session.SessionID) + } + workspace := session.WorkspaceID if nameMap != nil { if name, ok := nameMap[session.WorkspaceID]; ok { diff --git a/internal/ui/session_selector_test.go b/internal/ui/session_selector_test.go index 6f7bb6c..0b97444 100644 --- a/internal/ui/session_selector_test.go +++ b/internal/ui/session_selector_test.go @@ -62,6 +62,42 @@ func TestFormatSessionOption(t *testing.T) { nameMap: map[string]string{}, want: "Owner on /subscriptions/sub-1 - duration: 0m (session: session-4)", }, + { + name: "group session with directory name", + session: models.SessionInfo{ + SessionID: "group-session-1", + CSP: models.CSPAzure, + WorkspaceID: "29cb7961-e16d-42c7-8ade-1794bbb76782", + SessionDuration: 3600, + Target: &models.SessionTarget{ID: "d554b344-group-uuid", Type: models.TargetTypeGroups}, + }, + nameMap: map[string]string{"29cb7961-e16d-42c7-8ade-1794bbb76782": "CyberIAM Tech Labs"}, + want: "Group: d554b344-group-uuid in CyberIAM Tech Labs - duration: 1h 0m (session: group-session-1)", + }, + { + name: "group session without directory name", + session: models.SessionInfo{ + SessionID: "group-session-2", + CSP: models.CSPAzure, + WorkspaceID: "29cb7961-dir-uuid", + SessionDuration: 1800, + Target: &models.SessionTarget{ID: "abcd-group-uuid", Type: models.TargetTypeGroups}, + }, + nameMap: map[string]string{}, + want: "Group: abcd-group-uuid in 29cb7961-dir-uuid - duration: 30m (session: group-session-2)", + }, + { + name: "group session with nil name map", + session: models.SessionInfo{ + SessionID: "group-session-3", + CSP: models.CSPAzure, + WorkspaceID: "dir-uuid-123", + SessionDuration: 7200, + Target: &models.SessionTarget{ID: "group-uuid-456", Type: models.TargetTypeGroups}, + }, + nameMap: nil, + want: "Group: group-uuid-456 in dir-uuid-123 - duration: 2h 0m (session: group-session-3)", + }, } for _, tt := range tests { From a0e062c4a4a66ccf9c692f79e06af76a5abac46e Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 19 Feb 2026 12:23:59 +0100 Subject: [PATCH 4/9] feat: extend Favorite struct with Type, Group, DirectoryID fields Add group favorite support to config with backward compatibility. Legacy configs without type field default to "cloud" via ResolvedType(). New fields use omitempty for clean YAML output. --- internal/config/config.go | 14 ++- internal/config/config_test.go | 153 ++++++++++++++++++++++++++++++ internal/config/favorites.go | 9 ++ internal/config/favorites_test.go | 126 ++++++++++++++++++++++++ 4 files changed, 299 insertions(+), 3 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 5bb80e3..dc62e28 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -9,11 +9,19 @@ import ( "gopkg.in/yaml.v3" ) +const ( + FavoriteTypeCloud = "cloud" + FavoriteTypeGroups = "groups" +) + // Favorite represents a saved elevation target. type Favorite struct { - Provider string `yaml:"provider"` - Target string `yaml:"target"` - Role string `yaml:"role"` + Type string `yaml:"type,omitempty"` // "cloud" or "groups"; empty → "cloud" + Provider string `yaml:"provider"` + Target string `yaml:"target"` + Role string `yaml:"role"` + Group string `yaml:"group,omitempty"` // Group name (groups only) + DirectoryID string `yaml:"directory_id,omitempty"` // Directory ID (groups only) } // Config holds the grant application configuration. diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 8c1f654..8fcf3f5 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -265,3 +265,156 @@ func TestConfigPath_Default(t *testing.T) { t.Error("expected non-empty config path") } } + +func TestSaveConfig_RoundTrip_GroupFavorite(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + + original := &Config{ + Profile: "test-profile", + DefaultProvider: "azure", + Favorites: map[string]Favorite{ + "my-group": { + Type: FavoriteTypeGroups, + Provider: "azure", + Group: "SG-Admin", + DirectoryID: "dir-abc-123", + }, + }, + } + + if err := Save(original, path); err != nil { + t.Fatalf("Save() error = %v", err) + } + + loaded, err := Load(path) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + + fav, ok := loaded.Favorites["my-group"] + if !ok { + t.Fatal("expected favorite 'my-group' to exist") + } + if fav.Type != FavoriteTypeGroups { + t.Errorf("type = %q, want %q", fav.Type, FavoriteTypeGroups) + } + if fav.Provider != "azure" { + t.Errorf("provider = %q, want %q", fav.Provider, "azure") + } + if fav.Group != "SG-Admin" { + t.Errorf("group = %q, want %q", fav.Group, "SG-Admin") + } + if fav.DirectoryID != "dir-abc-123" { + t.Errorf("directory_id = %q, want %q", fav.DirectoryID, "dir-abc-123") + } +} + +func TestLoadConfig_LegacyWithoutType(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + + // Legacy YAML: no type field + content := []byte(`profile: legacy-profile +default_provider: azure +favorites: + old-fav: + provider: azure + target: sub-999 + role: Reader +`) + if err := os.WriteFile(path, content, 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } + + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + + fav, ok := cfg.Favorites["old-fav"] + if !ok { + t.Fatal("expected favorite 'old-fav' to exist") + } + if fav.Type != "" { + t.Errorf("type = %q, want empty string for legacy favorite", fav.Type) + } + if fav.ResolvedType() != FavoriteTypeCloud { + t.Errorf("ResolvedType() = %q, want %q", fav.ResolvedType(), FavoriteTypeCloud) + } + if fav.Provider != "azure" { + t.Errorf("provider = %q, want %q", fav.Provider, "azure") + } + if fav.Target != "sub-999" { + t.Errorf("target = %q, want %q", fav.Target, "sub-999") + } + if fav.Role != "Reader" { + t.Errorf("role = %q, want %q", fav.Role, "Reader") + } +} + +func TestLoadConfig_MixedFavorites(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + + content := []byte(`profile: mixed-profile +default_provider: azure +favorites: + cloud-fav: + type: cloud + provider: aws + target: account-123 + role: Admin + group-fav: + type: groups + provider: azure + group: SG-Dev + directory_id: dir-xyz + legacy-fav: + provider: azure + target: sub-old + role: Reader +`) + if err := os.WriteFile(path, content, 0644); err != nil { + t.Fatalf("failed to write test file: %v", err) + } + + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + + if len(cfg.Favorites) != 3 { + t.Fatalf("favorites length = %d, want 3", len(cfg.Favorites)) + } + + cloud := cfg.Favorites["cloud-fav"] + if cloud.Type != FavoriteTypeCloud { + t.Errorf("cloud-fav type = %q, want %q", cloud.Type, FavoriteTypeCloud) + } + if cloud.Provider != "aws" { + t.Errorf("cloud-fav provider = %q, want %q", cloud.Provider, "aws") + } + if cloud.Target != "account-123" { + t.Errorf("cloud-fav target = %q, want %q", cloud.Target, "account-123") + } + + group := cfg.Favorites["group-fav"] + if group.Type != FavoriteTypeGroups { + t.Errorf("group-fav type = %q, want %q", group.Type, FavoriteTypeGroups) + } + if group.Group != "SG-Dev" { + t.Errorf("group-fav group = %q, want %q", group.Group, "SG-Dev") + } + if group.DirectoryID != "dir-xyz" { + t.Errorf("group-fav directory_id = %q, want %q", group.DirectoryID, "dir-xyz") + } + + legacy := cfg.Favorites["legacy-fav"] + if legacy.Type != "" { + t.Errorf("legacy-fav type = %q, want empty", legacy.Type) + } + if legacy.ResolvedType() != FavoriteTypeCloud { + t.Errorf("legacy-fav ResolvedType() = %q, want %q", legacy.ResolvedType(), FavoriteTypeCloud) + } +} diff --git a/internal/config/favorites.go b/internal/config/favorites.go index 05b454c..e7840ed 100644 --- a/internal/config/favorites.go +++ b/internal/config/favorites.go @@ -45,6 +45,15 @@ func GetFavorite(cfg *Config, name string) (Favorite, error) { return fav, nil } +// ResolvedType returns the effective favorite type. +// Empty Type defaults to "cloud" for backward compatibility. +func (f Favorite) ResolvedType() string { + if f.Type == "" { + return FavoriteTypeCloud + } + return f.Type +} + // ListFavorites returns all favorites sorted alphabetically by name. func ListFavorites(cfg *Config) []FavoriteEntry { entries := make([]FavoriteEntry, 0, len(cfg.Favorites)) diff --git a/internal/config/favorites_test.go b/internal/config/favorites_test.go index ab97de9..5c48d1b 100644 --- a/internal/config/favorites_test.go +++ b/internal/config/favorites_test.go @@ -152,3 +152,129 @@ func TestAddFavorite_DefaultProvider(t *testing.T) { t.Errorf("provider = %q, want %q (should default to 'azure')", got.Provider, "azure") } } + +func TestAddFavorite_GroupFavorite(t *testing.T) { + t.Parallel() + cfg := DefaultConfig() + fav := Favorite{ + Type: FavoriteTypeGroups, + Provider: "azure", + Group: "SG-Admin", + DirectoryID: "dir-abc-123", + } + + err := AddFavorite(cfg, "my-group", fav) + if err != nil { + t.Fatalf("AddFavorite() error = %v", err) + } + + got, ok := cfg.Favorites["my-group"] + if !ok { + t.Fatal("expected favorite 'my-group' to exist") + } + if got.Type != FavoriteTypeGroups { + t.Errorf("type = %q, want %q", got.Type, FavoriteTypeGroups) + } + if got.Provider != "azure" { + t.Errorf("provider = %q, want %q", got.Provider, "azure") + } + if got.Group != "SG-Admin" { + t.Errorf("group = %q, want %q", got.Group, "SG-Admin") + } + if got.DirectoryID != "dir-abc-123" { + t.Errorf("directory_id = %q, want %q", got.DirectoryID, "dir-abc-123") + } +} + +func TestGetFavorite_GroupFavorite(t *testing.T) { + t.Parallel() + cfg := DefaultConfig() + expected := Favorite{ + Type: FavoriteTypeGroups, + Provider: "azure", + Group: "SG-Dev", + DirectoryID: "dir-xyz", + } + cfg.Favorites["my-group"] = expected + + got, err := GetFavorite(cfg, "my-group") + if err != nil { + t.Fatalf("GetFavorite() error = %v", err) + } + + if got.Type != expected.Type { + t.Errorf("type = %q, want %q", got.Type, expected.Type) + } + if got.Provider != expected.Provider { + t.Errorf("provider = %q, want %q", got.Provider, expected.Provider) + } + if got.Group != expected.Group { + t.Errorf("group = %q, want %q", got.Group, expected.Group) + } + if got.DirectoryID != expected.DirectoryID { + t.Errorf("directory_id = %q, want %q", got.DirectoryID, expected.DirectoryID) + } +} + +func TestListFavorites_MixedCloudAndGroups(t *testing.T) { + t.Parallel() + cfg := DefaultConfig() + cfg.Favorites["cloud-fav"] = Favorite{ + Type: FavoriteTypeCloud, + Provider: "aws", + Target: "account-123", + Role: "Admin", + } + cfg.Favorites["group-fav"] = Favorite{ + Type: FavoriteTypeGroups, + Provider: "azure", + Group: "SG-Admin", + DirectoryID: "dir-abc", + } + + entries := ListFavorites(cfg) + if len(entries) != 2 { + t.Fatalf("ListFavorites() length = %d, want 2", len(entries)) + } + + // Sorted alphabetically: cloud-fav, group-fav + if entries[0].Name != "cloud-fav" { + t.Errorf("entries[0].Name = %q, want %q", entries[0].Name, "cloud-fav") + } + if entries[0].Type != FavoriteTypeCloud { + t.Errorf("entries[0].Type = %q, want %q", entries[0].Type, FavoriteTypeCloud) + } + if entries[1].Name != "group-fav" { + t.Errorf("entries[1].Name = %q, want %q", entries[1].Name, "group-fav") + } + if entries[1].Type != FavoriteTypeGroups { + t.Errorf("entries[1].Type = %q, want %q", entries[1].Type, FavoriteTypeGroups) + } + if entries[1].Group != "SG-Admin" { + t.Errorf("entries[1].Group = %q, want %q", entries[1].Group, "SG-Admin") + } +} + +func TestResolvedType(t *testing.T) { + t.Parallel() + tests := []struct { + name string + favType string + wantType string + }{ + {name: "empty defaults to cloud", favType: "", wantType: FavoriteTypeCloud}, + {name: "explicit cloud", favType: FavoriteTypeCloud, wantType: FavoriteTypeCloud}, + {name: "explicit groups", favType: FavoriteTypeGroups, wantType: FavoriteTypeGroups}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + fav := Favorite{Type: tt.favType} + got := fav.ResolvedType() + if got != tt.wantType { + t.Errorf("ResolvedType() = %q, want %q", got, tt.wantType) + } + }) + } +} From dd2092109c157bb9b3b330466f9195a6cdd20c19 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 19 Feb 2026 12:24:19 +0100 Subject: [PATCH 5/9] test: verify grant revoke handles group sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add test cases confirming revoke works with group sessions in direct, --all, and interactive modes. No code changes needed — revoke is session-type agnostic by design. --- CHANGELOG.md | 3 +- cmd/revoke_test.go | 168 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0278b0..50df422 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ All notable changes to this project will be documented in this file. ### Added -- `grant revoke` command for session revocation with three modes: direct (by session ID), `--all`, and interactive (multi-select) +- `grant groups` command for Entra ID group membership elevation with interactive and direct (`--group`) modes +- `grant revoke` command for session revocation with three modes: direct (by session ID), `--all`, and interactive (multi-select); works with both cloud and group sessions - `--yes`/`-y` flag on `grant revoke` to skip confirmation for scripting - `--provider`/`-p` flag on `grant revoke --all` and interactive mode to filter by cloud provider - Session ID displayed in `grant status` output for easy reference with `grant revoke` diff --git a/cmd/revoke_test.go b/cmd/revoke_test.go index a2ea144..2f5ff02 100644 --- a/cmd/revoke_test.go +++ b/cmd/revoke_test.go @@ -404,6 +404,174 @@ func TestRevokeCommand(t *testing.T) { wantContain: []string{"session-1", "SUCCESSFULLY_REVOKED"}, wantErr: false, }, + { + name: "direct mode - group session ID", + args: []string{"group-session-1"}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupLister: func() *mockSessionLister { return &mockSessionLister{} }, + setupElig: func() *mockEligibilityLister { return &mockEligibilityLister{} }, + setupRevoker: func() *mockSessionRevoker { + return &mockSessionRevoker{ + revokeFunc: func(ctx context.Context, req *scamodels.RevokeRequest) (*scamodels.RevokeResponse, error) { + if len(req.SessionIDs) != 1 || req.SessionIDs[0] != "group-session-1" { + t.Errorf("expected [group-session-1], got %v", req.SessionIDs) + } + return &scamodels.RevokeResponse{ + Response: []scamodels.RevocationResult{ + {SessionID: "group-session-1", RevocationStatus: scamodels.RevocationSuccessful}, + }, + }, nil + }, + } + }, + setupSelector: func() *mockSessionSelector { return &mockSessionSelector{} }, + setupConfirm: func() *mockConfirmPrompter { return &mockConfirmPrompter{} }, + wantContain: []string{"group-session-1", "SUCCESSFULLY_REVOKED"}, + wantErr: false, + }, + { + name: "all mode - mixed cloud and group sessions", + args: []string{"--all", "--yes"}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupLister: func() *mockSessionLister { + return &mockSessionLister{ + sessions: &scamodels.SessionsResponse{ + Response: []scamodels.SessionInfo{ + { + SessionID: "cloud-session-1", + UserID: "user@example.com", + CSP: scamodels.CSPAzure, + WorkspaceID: "/subscriptions/sub-1", + RoleID: "Contributor", + SessionDuration: 3600, + }, + { + SessionID: "group-session-1", + UserID: "user@example.com", + CSP: scamodels.CSPAzure, + WorkspaceID: "29cb7961-dir-uuid", + SessionDuration: 3600, + Target: &scamodels.SessionTarget{ID: "group-uuid-1", Type: scamodels.TargetTypeGroups}, + }, + { + SessionID: "group-session-2", + UserID: "user@example.com", + CSP: scamodels.CSPAzure, + WorkspaceID: "29cb7961-dir-uuid", + SessionDuration: 1800, + Target: &scamodels.SessionTarget{ID: "group-uuid-2", Type: scamodels.TargetTypeGroups}, + }, + }, + Total: 3, + }, + } + }, + setupElig: func() *mockEligibilityLister { return &mockEligibilityLister{} }, + setupRevoker: func() *mockSessionRevoker { + return &mockSessionRevoker{ + revokeFunc: func(ctx context.Context, req *scamodels.RevokeRequest) (*scamodels.RevokeResponse, error) { + if len(req.SessionIDs) != 3 { + t.Errorf("expected 3 session IDs, got %d", len(req.SessionIDs)) + } + // Verify all session IDs are collected regardless of type + ids := make(map[string]bool) + for _, id := range req.SessionIDs { + ids[id] = true + } + for _, want := range []string{"cloud-session-1", "group-session-1", "group-session-2"} { + if !ids[want] { + t.Errorf("missing session ID %q in revoke request", want) + } + } + return &scamodels.RevokeResponse{ + Response: []scamodels.RevocationResult{ + {SessionID: "cloud-session-1", RevocationStatus: scamodels.RevocationSuccessful}, + {SessionID: "group-session-1", RevocationStatus: scamodels.RevocationSuccessful}, + {SessionID: "group-session-2", RevocationStatus: scamodels.RevocationSuccessful}, + }, + }, nil + }, + } + }, + setupSelector: func() *mockSessionSelector { return &mockSessionSelector{} }, + setupConfirm: func() *mockConfirmPrompter { return &mockConfirmPrompter{} }, + wantContain: []string{"cloud-session-1", "group-session-1", "group-session-2", "SUCCESSFULLY_REVOKED"}, + wantErr: false, + }, + { + name: "interactive mode - mixed sessions with group session selected", + args: []string{"--yes"}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupLister: func() *mockSessionLister { + return &mockSessionLister{ + sessions: &scamodels.SessionsResponse{ + Response: []scamodels.SessionInfo{ + { + SessionID: "cloud-session-1", + UserID: "user@example.com", + CSP: scamodels.CSPAzure, + WorkspaceID: "/subscriptions/sub-1", + RoleID: "Contributor", + SessionDuration: 3600, + }, + { + SessionID: "group-session-1", + UserID: "user@example.com", + CSP: scamodels.CSPAzure, + WorkspaceID: "29cb7961-dir-uuid", + SessionDuration: 3600, + Target: &scamodels.SessionTarget{ID: "group-uuid", Type: scamodels.TargetTypeGroups}, + }, + }, + Total: 2, + }, + } + }, + setupElig: func() *mockEligibilityLister { return &mockEligibilityLister{} }, + setupRevoker: func() *mockSessionRevoker { + return &mockSessionRevoker{ + revokeFunc: func(ctx context.Context, req *scamodels.RevokeRequest) (*scamodels.RevokeResponse, error) { + if len(req.SessionIDs) != 1 || req.SessionIDs[0] != "group-session-1" { + t.Errorf("expected [group-session-1], got %v", req.SessionIDs) + } + return &scamodels.RevokeResponse{ + Response: []scamodels.RevocationResult{ + {SessionID: "group-session-1", RevocationStatus: scamodels.RevocationSuccessful}, + }, + }, nil + }, + } + }, + setupSelector: func() *mockSessionSelector { + return &mockSessionSelector{ + sessions: []scamodels.SessionInfo{ + { + SessionID: "group-session-1", + UserID: "user@example.com", + CSP: scamodels.CSPAzure, + WorkspaceID: "29cb7961-dir-uuid", + SessionDuration: 3600, + Target: &scamodels.SessionTarget{ID: "group-uuid", Type: scamodels.TargetTypeGroups}, + }, + }, + } + }, + setupConfirm: func() *mockConfirmPrompter { return &mockConfirmPrompter{} }, + wantContain: []string{"group-session-1", "SUCCESSFULLY_REVOKED"}, + wantErr: false, + }, { name: "invalid provider", args: []string{"--all", "--provider", "invalid"}, From 7bfb43e67db267ffa1a7f441e852d6d752839ac7 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 19 Feb 2026 12:28:22 +0100 Subject: [PATCH 6/9] feat: add --type groups support to favorites add and list favorites add now supports --type groups with --group flag for direct mode and interactive group selection. favorites list shows group favorites as "name: groups/group-name". Flag validation prevents mixing cloud and group flags. --- cmd/favorites.go | 138 +++++++++++++++++++------ cmd/favorites_test.go | 230 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 339 insertions(+), 29 deletions(-) diff --git a/cmd/favorites.go b/cmd/favorites.go index 9b6fabf..495461f 100644 --- a/cmd/favorites.go +++ b/cmd/favorites.go @@ -7,6 +7,7 @@ import ( survey "github.com/Iilun/survey/v2" "github.com/aaearon/grant-cli/internal/config" + scamodels "github.com/aaearon/grant-cli/internal/sca/models" "github.com/spf13/cobra" ) @@ -35,6 +36,11 @@ Workflow: // NewFavoritesCommandWithDeps creates the favorites command with injected dependencies for testing func NewFavoritesCommandWithDeps(eligLister eligibilityLister, sel targetSelector, prompter namePrompter) *cobra.Command { + return NewFavoritesCommandWithAllDeps(eligLister, sel, prompter, nil, nil) +} + +// NewFavoritesCommandWithAllDeps creates the favorites command with all injected dependencies including groups +func NewFavoritesCommandWithAllDeps(eligLister eligibilityLister, sel targetSelector, prompter namePrompter, groupsElig groupsEligibilityLister, groupSel groupSelector) *cobra.Command { cmd := &cobra.Command{ Use: "favorites", Short: "Manage saved elevation favorites", @@ -50,7 +56,7 @@ Workflow: } cmd.AddCommand(newFavoritesAddCommandWithRunner(func(c *cobra.Command, args []string) error { - return runFavoritesAddWithDeps(c, args, eligLister, sel, prompter, nil) + return runFavoritesAddWithDeps(c, args, eligLister, sel, prompter, nil, groupsElig, groupSel) })) cmd.AddCommand(newFavoritesListCommand()) cmd.AddCommand(newFavoritesRemoveCommand()) @@ -80,6 +86,8 @@ func newFavoritesAddCommandWithRunner(runFn func(*cobra.Command, []string) error cmd.Flags().StringP("provider", "p", "", "Cloud provider: azure, aws (omit to show all)") cmd.Flags().StringP("target", "t", "", "Target name (subscription, resource group, etc.)") cmd.Flags().StringP("role", "r", "", "Role name") + cmd.Flags().String("type", "", "Favorite type: cloud, groups (default: cloud)") + cmd.Flags().StringP("group", "g", "", "Group name (for --type groups)") return cmd } @@ -112,9 +120,17 @@ func runFavoritesAddProduction(cmd *cobra.Command, args []string) error { return fmt.Errorf("both --target and --role must be provided") } - if target != "" && role != "" { - // Non-interactive: no auth needed - return runFavoritesAddWithDeps(cmd, args, nil, nil, nil, nil) + favType, _ := cmd.Flags().GetString("type") + group, _ := cmd.Flags().GetString("group") + + if favType == config.FavoriteTypeGroups { + if group != "" { + // Non-interactive groups mode: no auth needed + return runFavoritesAddWithDeps(cmd, args, nil, nil, nil, nil, nil, nil) + } + } else if target != "" && role != "" { + // Non-interactive cloud mode: no auth needed + return runFavoritesAddWithDeps(cmd, args, nil, nil, nil, nil, nil, nil) } // Interactive path: load config early for fast-fail duplicate check @@ -135,31 +151,51 @@ func runFavoritesAddProduction(cmd *cobra.Command, args []string) error { return err } - return runFavoritesAddWithDeps(cmd, args, scaService, &uiSelector{}, &surveyNamePrompter{}, cfg) + return runFavoritesAddWithDeps(cmd, args, scaService, &uiSelector{}, &surveyNamePrompter{}, cfg, scaService, &uiGroupSelector{}) } // runFavoritesAddWithDeps contains the core logic for favorites add. // When eligLister and sel are nil, it uses the non-interactive flag path. // If preloadedCfg is non-nil, it is used instead of loading from disk. -func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligibilityLister, sel targetSelector, prompter namePrompter, preloadedCfg *config.Config) error { +func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligibilityLister, sel targetSelector, prompter namePrompter, preloadedCfg *config.Config, groupsElig groupsEligibilityLister, groupSel groupSelector) error { // Read flags provider, _ := cmd.Flags().GetString("provider") target, _ := cmd.Flags().GetString("target") role, _ := cmd.Flags().GetString("role") + favType, _ := cmd.Flags().GetString("type") + group, _ := cmd.Flags().GetString("group") - // Validate: target and role must both be provided or both omitted - if (target != "" && role == "") || (target == "" && role != "") { - return fmt.Errorf("both --target and --role must be provided") + // Validate type flag + if favType != "" && favType != config.FavoriteTypeCloud && favType != config.FavoriteTypeGroups { + return fmt.Errorf("invalid --type %q: must be one of: cloud, groups", favType) } - // Determine name from arg (may be empty for interactive prompt-after-selection) + // Validate flag combinations + if favType == config.FavoriteTypeGroups { + if target != "" || role != "" { + return fmt.Errorf("--target and --role cannot be used with --type groups") + } + } else { + if group != "" { + return fmt.Errorf("--group requires --type groups") + } + if (target != "" && role == "") || (target == "" && role != "") { + return fmt.Errorf("both --target and --role must be provided") + } + } + + // Determine name from arg var name string if len(args) > 0 { name = args[0] } // Non-interactive mode requires name upfront - if target != "" && role != "" && name == "" { + isNonInteractive := (target != "" && role != "") || (favType == config.FavoriteTypeGroups && group != "") + if isNonInteractive && name == "" { + if favType == config.FavoriteTypeGroups { + return fmt.Errorf("name is required when using --group flag\n\nUsage:\n grant favorites add --type groups --group ") + } return fmt.Errorf("name is required when using --target and --role flags\n\nUsage:\n grant favorites add --target --role ") } @@ -184,10 +220,14 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi } } - var fav config.Favorite + // Groups flow + if favType == config.FavoriteTypeGroups { + return addGroupFavorite(cmd, name, group, cfg, cfgPath, groupsElig, groupSel, prompter) + } + // Cloud flow + var fav config.Favorite if target != "" && role != "" { - // Non-interactive mode: use flags directly fav.Target = target fav.Role = role fav.Provider = provider @@ -195,7 +235,6 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi fav.Provider = cfg.DefaultProvider } } else { - // Interactive mode: select from eligible targets (all CSPs when provider is empty) ctx, cancel := context.WithTimeout(context.Background(), apiTimeout) defer cancel() @@ -204,13 +243,10 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi return err } - // Interactive selection selectedTarget, err := sel.SelectTarget(allTargets) if err != nil { return fmt.Errorf("target selection failed: %w", err) } - - // Ensure CSP is set on selected target resolveTargetCSP(selectedTarget, allTargets, provider) if provider != "" { @@ -221,34 +257,78 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi fav.Target = selectedTarget.WorkspaceName fav.Role = selectedTarget.RoleInfo.Name - // Prompt for name after selection if not provided if name == "" { name, err = prompter.PromptName() if err != nil { return fmt.Errorf("failed to read favorite name: %w", err) } - - // Check duplicate for prompted name if _, err := config.GetFavorite(cfg, name); err == nil { return fmt.Errorf("favorite %q already exists", name) } } } - // Add favorite if err := config.AddFavorite(cfg, name, fav); err != nil { return fmt.Errorf("failed to add favorite: %w", err) } - - // Save config if err := config.Save(cfg, cfgPath); err != nil { return fmt.Errorf("failed to save config: %w", err) } - fmt.Fprintf(cmd.OutOrStdout(), "Added favorite %q: %s/%s/%s\n", name, fav.Provider, fav.Target, fav.Role) return nil } +// addGroupFavorite handles the --type groups flow for favorites add. +func addGroupFavorite(cmd *cobra.Command, name, group string, cfg *config.Config, cfgPath string, groupsElig groupsEligibilityLister, groupSel groupSelector, prompter namePrompter) error { + var fav config.Favorite + fav.Type = config.FavoriteTypeGroups + fav.Provider = "azure" + + if group != "" { + // Non-interactive: group specified via flag + fav.Group = group + } else { + // Interactive: select from eligible groups + ctx, cancel := context.WithTimeout(context.Background(), apiTimeout) + defer cancel() + + eligResp, err := groupsElig.ListGroupsEligibility(ctx, scamodels.CSPAzure) + if err != nil { + return fmt.Errorf("failed to fetch eligible groups: %w", err) + } + if len(eligResp.Response) == 0 { + return fmt.Errorf("no eligible groups found") + } + + selected, err := groupSel.SelectGroup(eligResp.Response) + if err != nil { + return fmt.Errorf("group selection failed: %w", err) + } + + fav.Group = selected.GroupName + fav.DirectoryID = selected.DirectoryID + + if name == "" { + name, err = prompter.PromptName() + if err != nil { + return fmt.Errorf("failed to read favorite name: %w", err) + } + if _, err := config.GetFavorite(cfg, name); err == nil { + return fmt.Errorf("favorite %q already exists", name) + } + } + } + + if err := config.AddFavorite(cfg, name, fav); err != nil { + return fmt.Errorf("failed to add favorite: %w", err) + } + if err := config.Save(cfg, cfgPath); err != nil { + return fmt.Errorf("failed to save config: %w", err) + } + fmt.Fprintf(cmd.OutOrStdout(), "Added favorite %q: groups/%s\n", name, fav.Group) + return nil +} + func newFavoritesListCommand() *cobra.Command { return &cobra.Command{ Use: "list", @@ -293,11 +373,11 @@ func runFavoritesList(cmd *cobra.Command, args []string) error { } for _, entry := range favorites { - fmt.Fprintf(cmd.OutOrStdout(), "%s: %s/%s/%s\n", - entry.Name, - entry.Provider, - entry.Target, - entry.Role) + if entry.ResolvedType() == config.FavoriteTypeGroups { + fmt.Fprintf(cmd.OutOrStdout(), "%s: groups/%s\n", entry.Name, entry.Group) + } else { + fmt.Fprintf(cmd.OutOrStdout(), "%s: %s/%s/%s\n", entry.Name, entry.Provider, entry.Target, entry.Role) + } } return nil diff --git a/cmd/favorites_test.go b/cmd/favorites_test.go index 6992880..d49ded0 100644 --- a/cmd/favorites_test.go +++ b/cmd/favorites_test.go @@ -667,3 +667,233 @@ func TestFavoritesAddInteractiveMode(t *testing.T) { }) } } + +func TestFavoritesAddGroupFavorite(t *testing.T) { + tests := []struct { + name string + setupConfig func(string) + groupsElig groupsEligibilityLister + groupSel groupSelector + namePrompter namePrompter + args []string + wantContain []string + wantErr bool + }{ + { + name: "non-interactive - group via flags", + setupConfig: func(path string) { + cfg := config.DefaultConfig() + _ = config.Save(cfg, path) + }, + args: []string{"my-grp", "--type", "groups", "--group", "Engineering"}, + wantContain: []string{"Added favorite", "my-grp", "groups/Engineering"}, + wantErr: false, + }, + { + name: "non-interactive - requires name", + setupConfig: func(path string) { + cfg := config.DefaultConfig() + _ = config.Save(cfg, path) + }, + args: []string{"--type", "groups", "--group", "Engineering"}, + wantContain: []string{"name is required"}, + wantErr: true, + }, + { + name: "invalid type", + setupConfig: func(path string) { + cfg := config.DefaultConfig() + _ = config.Save(cfg, path) + }, + args: []string{"test", "--type", "invalid"}, + wantContain: []string{"invalid --type"}, + wantErr: true, + }, + { + name: "target flag with groups type - error", + setupConfig: func(path string) { + cfg := config.DefaultConfig() + _ = config.Save(cfg, path) + }, + args: []string{"test", "--type", "groups", "--target", "sub-1", "--role", "Reader"}, + wantContain: []string{"--target and --role cannot be used with --type groups"}, + wantErr: true, + }, + { + name: "group flag without type groups - error", + setupConfig: func(path string) { + cfg := config.DefaultConfig() + _ = config.Save(cfg, path) + }, + args: []string{"test", "--group", "Engineering"}, + wantContain: []string{"--group requires --type groups"}, + wantErr: true, + }, + { + name: "interactive - selects from eligible groups", + setupConfig: func(path string) { + cfg := config.DefaultConfig() + _ = config.Save(cfg, path) + }, + groupsElig: &mockGroupsEligibilityLister{ + response: &models.GroupsEligibilityResponse{ + Response: []models.GroupsEligibleTarget{ + {DirectoryID: "dir-1", GroupID: "grp-1", GroupName: "Engineering"}, + }, + Total: 1, + }, + }, + groupSel: &mockGroupSelector{ + group: &models.GroupsEligibleTarget{DirectoryID: "dir-1", GroupID: "grp-1", GroupName: "Engineering"}, + }, + args: []string{"my-grp", "--type", "groups"}, + wantContain: []string{"Added favorite", "my-grp", "groups/Engineering"}, + wantErr: false, + }, + { + name: "non-interactive - persists to disk", + setupConfig: func(path string) { + cfg := config.DefaultConfig() + _ = config.Save(cfg, path) + }, + args: []string{"grp-fav", "--type", "groups", "--group", "DevOps"}, + wantContain: []string{"Added favorite", "grp-fav", "groups/DevOps"}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yaml") + t.Setenv("GRANT_CONFIG", configPath) + + tt.setupConfig(configPath) + + rootCmd := newTestRootCommand() + favCmd := NewFavoritesCommandWithAllDeps(nil, nil, tt.namePrompter, tt.groupsElig, tt.groupSel) + rootCmd.AddCommand(favCmd) + + cmdArgs := append([]string{"favorites", "add"}, tt.args...) + output, err := executeCommand(rootCmd, cmdArgs...) + + if (err != nil) != tt.wantErr { + t.Errorf("error = %v, wantErr %v, output:\n%s", err, tt.wantErr, output) + } + + for _, want := range tt.wantContain { + if !strings.Contains(output, want) { + t.Errorf("output missing %q\ngot:\n%s", want, output) + } + } + }) + } +} + +func TestFavoritesAddGroupPersistence(t *testing.T) { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yaml") + t.Setenv("GRANT_CONFIG", configPath) + + cfg := config.DefaultConfig() + _ = config.Save(cfg, configPath) + + rootCmd := newTestRootCommand() + favCmd := NewFavoritesCommandWithAllDeps(nil, nil, nil, nil, nil) + rootCmd.AddCommand(favCmd) + + _, err := executeCommand(rootCmd, "favorites", "add", "grp-fav", "--type", "groups", "--group", "DevOps") + if err != nil { + t.Fatalf("add group favorite failed: %v", err) + } + + reloaded, err := config.Load(configPath) + if err != nil { + t.Fatalf("reload config failed: %v", err) + } + + fav, err := config.GetFavorite(reloaded, "grp-fav") + if err != nil { + t.Fatalf("favorite not found: %v", err) + } + + if fav.ResolvedType() != config.FavoriteTypeGroups { + t.Errorf("ResolvedType() = %q, want %q", fav.ResolvedType(), config.FavoriteTypeGroups) + } + if fav.Group != "DevOps" { + t.Errorf("Group = %q, want %q", fav.Group, "DevOps") + } + if fav.Provider != "azure" { + t.Errorf("Provider = %q, want %q", fav.Provider, "azure") + } +} + +func TestFavoritesListWithGroupFavorites(t *testing.T) { + tests := []struct { + name string + setupConfig func(string) + wantContain []string + }{ + { + name: "mixed cloud and group favorites", + setupConfig: func(path string) { + cfg := config.DefaultConfig() + _ = config.AddFavorite(cfg, "cloud-fav", config.Favorite{ + Provider: "azure", + Target: "sub-123", + Role: "Contributor", + }) + _ = config.AddFavorite(cfg, "grp-fav", config.Favorite{ + Type: config.FavoriteTypeGroups, + Provider: "azure", + Group: "Engineering", + }) + _ = config.Save(cfg, path) + }, + wantContain: []string{ + "cloud-fav: azure/sub-123/Contributor", + "grp-fav: groups/Engineering", + }, + }, + { + name: "only group favorites", + setupConfig: func(path string) { + cfg := config.DefaultConfig() + _ = config.AddFavorite(cfg, "eng", config.Favorite{ + Type: config.FavoriteTypeGroups, + Provider: "azure", + Group: "Engineering", + }) + _ = config.Save(cfg, path) + }, + wantContain: []string{ + "eng: groups/Engineering", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + configPath := filepath.Join(tmpDir, "config.yaml") + t.Setenv("GRANT_CONFIG", configPath) + + tt.setupConfig(configPath) + + rootCmd := newTestRootCommand() + favCmd := NewFavoritesCommand() + rootCmd.AddCommand(favCmd) + + output, err := executeCommand(rootCmd, "favorites", "list") + if err != nil { + t.Fatalf("list failed: %v", err) + } + + for _, want := range tt.wantContain { + if !strings.Contains(output, want) { + t.Errorf("output missing %q\ngot:\n%s", want, output) + } + } + }) + } +} From 2cc5f563b49ac7a0e4fb5f021ab0b809e3c33de2 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 19 Feb 2026 12:28:58 +0100 Subject: [PATCH 7/9] feat: redirect group favorites to grant groups command When the root command's --favorite resolves to a groups-type favorite, return a helpful error directing to 'grant groups --favorite '. --- CHANGELOG.md | 1 + cmd/root.go | 5 +++++ cmd/root_elevate_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50df422..a65d891 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. ### Added - `grant groups` command for Entra ID group membership elevation with interactive and direct (`--group`) modes +- `grant --favorite ` now detects group-type favorites and redirects users to `grant groups --favorite ` - `grant revoke` command for session revocation with three modes: direct (by session ID), `--all`, and interactive (multi-select); works with both cloud and group sessions - `--yes`/`-y` flag on `grant revoke` to skip confirmation for scripting - `--provider`/`-p` flag on `grant revoke --all` and interactive mode to filter by cloud provider diff --git a/cmd/root.go b/cmd/root.go index 1429201..5f47d2b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -306,6 +306,11 @@ func resolveAndElevate( return nil, fmt.Errorf("favorite %q not found, run 'grant favorites list'", flags.favorite) } + // Group favorites must be used via the groups command + if fav.ResolvedType() == config.FavoriteTypeGroups { + return nil, fmt.Errorf("favorite %q is a group favorite; use 'grant groups --favorite %s' instead", flags.favorite, flags.favorite) + } + // Check provider mismatch if flags.provider != "" && !strings.EqualFold(flags.provider, fav.Provider) { return nil, fmt.Errorf("provider %q does not match favorite provider %q", flags.provider, fav.Provider) diff --git a/cmd/root_elevate_test.go b/cmd/root_elevate_test.go index 99b2ecc..d100261 100644 --- a/cmd/root_elevate_test.go +++ b/cmd/root_elevate_test.go @@ -696,6 +696,30 @@ func TestRootElevate_FavoriteMode(t *testing.T) { }, wantErr: true, }, + { + name: "group favorite redirects to grant groups", + setupMocks: func() (*mockAuthLoader, *mockEligibilityLister, *mockElevateService, *config.Config) { + authLoader := &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "test-jwt"}, + } + + cfg := config.DefaultConfig() + _ = config.AddFavorite(cfg, "my-grp", config.Favorite{ + Type: config.FavoriteTypeGroups, + Provider: "azure", + Group: "Engineering", + DirectoryID: "dir-uuid", + }) + + return authLoader, nil, nil, cfg + }, + args: []string{"--favorite", "my-grp"}, + wantContain: []string{ + "group favorite", + "grant groups --favorite my-grp", + }, + wantErr: true, + }, } for _, tt := range tests { From 70a124cd58baa266ae4f7bea4da26ae8b0147051 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 19 Feb 2026 13:08:33 +0100 Subject: [PATCH 8/9] feat: add grant groups command with favorite mode support Implements Entra ID group membership elevation via three modes: interactive selection, direct (--group), and favorite (--favorite). Includes models, service methods, UI selector, interfaces, mocks, and comprehensive tests for all modes. --- CHANGELOG.md | 2 +- cmd/commands.go | 1 + cmd/groups.go | 218 +++++++++++ cmd/groups_test.go | 557 +++++++++++++++++++++++++++++ cmd/helpers.go | 35 ++ cmd/interfaces.go | 15 + cmd/test_mocks.go | 42 +++ internal/sca/models/groups_test.go | 145 ++++++++ internal/ui/group_selector.go | 65 ++++ internal/ui/group_selector_test.go | 161 +++++++++ 10 files changed, 1240 insertions(+), 1 deletion(-) create mode 100644 cmd/groups.go create mode 100644 cmd/groups_test.go create mode 100644 internal/sca/models/groups_test.go create mode 100644 internal/ui/group_selector.go create mode 100644 internal/ui/group_selector_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a65d891..28107c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this project will be documented in this file. ### Added -- `grant groups` command for Entra ID group membership elevation with interactive and direct (`--group`) modes +- `grant groups` command for Entra ID group membership elevation with interactive, direct (`--group`), and favorite (`--favorite`) modes - `grant --favorite ` now detects group-type favorites and redirects users to `grant groups --favorite ` - `grant revoke` command for session revocation with three modes: direct (by session ID), `--all`, and interactive (multi-select); works with both cloud and group sessions - `--yes`/`-y` flag on `grant revoke` to skip confirmation for scripting diff --git a/cmd/commands.go b/cmd/commands.go index 4b77285..9570f9f 100644 --- a/cmd/commands.go +++ b/cmd/commands.go @@ -10,5 +10,6 @@ func init() { NewFavoritesCommand(), NewEnvCommand(), NewRevokeCommand(), + NewGroupsCommand(), ) } diff --git a/cmd/groups.go b/cmd/groups.go new file mode 100644 index 0000000..90f394f --- /dev/null +++ b/cmd/groups.go @@ -0,0 +1,218 @@ +package cmd + +import ( + "context" + "fmt" + "strings" + + "github.com/aaearon/grant-cli/internal/config" + scamodels "github.com/aaearon/grant-cli/internal/sca/models" + "github.com/aaearon/grant-cli/internal/ui" + sdkmodels "github.com/cyberark/idsec-sdk-golang/pkg/models" + "github.com/spf13/cobra" +) + +// uiGroupSelector wraps ui.SelectGroup to implement groupSelector +type uiGroupSelector struct{} + +func (s *uiGroupSelector) SelectGroup(groups []scamodels.GroupsEligibleTarget) (*scamodels.GroupsEligibleTarget, error) { + return ui.SelectGroup(groups) +} + +// newGroupsCommand creates the groups cobra command with the given RunE function. +func newGroupsCommand(runFn func(*cobra.Command, []string) error) *cobra.Command { + cmd := &cobra.Command{ + Use: "groups", + Short: "Request temporary Entra ID group membership", + Long: `Request temporary Entra ID group membership via CyberArk Secure Cloud Access (SCA). + +Three execution modes: +1. Interactive mode (no flags): Select group interactively +2. Direct mode (--group): Directly specify group name +3. Favorite mode (--favorite): Use a saved favorite + +Examples: + # Interactive selection + grant groups + + # Direct selection + grant groups --group "Cloud Admins" + + # Favorite mode + grant groups --favorite my-group`, + SilenceErrors: true, + SilenceUsage: true, + RunE: runFn, + } + + cmd.Flags().StringP("group", "g", "", "Group name for direct mode") + cmd.Flags().StringP("favorite", "f", "", "Use a saved favorite (see 'grant favorites list')") + + return cmd +} + +// NewGroupsCommand creates the production groups command. +func NewGroupsCommand() *cobra.Command { + return newGroupsCommand(func(cmd *cobra.Command, args []string) error { + ispAuth, svc, profile, err := bootstrapSCAService() + if err != nil { + return err + } + + return runGroups(cmd, ispAuth, svc, svc, svc, &uiGroupSelector{}, profile, nil) + }) +} + +// NewGroupsCommandWithDeps creates a groups command with injected dependencies for testing. +func NewGroupsCommandWithDeps( + auth authLoader, + cloudElig eligibilityLister, + groupsElig groupsEligibilityLister, + elevator groupsElevator, + selector groupSelector, +) *cobra.Command { + return newGroupsCommand(func(cmd *cobra.Command, args []string) error { + return runGroups(cmd, auth, cloudElig, groupsElig, elevator, selector, nil, nil) + }) +} + +// NewGroupsCommandWithDepsAndConfig creates a groups command with injected dependencies and config for testing. +func NewGroupsCommandWithDepsAndConfig( + auth authLoader, + cloudElig eligibilityLister, + groupsElig groupsEligibilityLister, + elevator groupsElevator, + selector groupSelector, + cfg *config.Config, +) *cobra.Command { + return newGroupsCommand(func(cmd *cobra.Command, args []string) error { + return runGroups(cmd, auth, cloudElig, groupsElig, elevator, selector, nil, cfg) + }) +} + +func runGroups( + cmd *cobra.Command, + auth authLoader, + cloudElig eligibilityLister, + groupsElig groupsEligibilityLister, + elevator groupsElevator, + selector groupSelector, + profile *sdkmodels.IdsecProfile, + cfg *config.Config, +) error { + groupFlag, _ := cmd.Flags().GetString("group") + favoriteFlag, _ := cmd.Flags().GetString("favorite") + + if favoriteFlag != "" { + if cfg == nil { + var err error + cfg, _, err = config.LoadDefaultWithPath() + if err != nil { + return err + } + } + + fav, err := config.GetFavorite(cfg, favoriteFlag) + if err != nil { + return fmt.Errorf("favorite %q not found, run 'grant favorites list'", favoriteFlag) + } + + if fav.ResolvedType() != config.FavoriteTypeGroups { + return fmt.Errorf("favorite %q is a cloud favorite; use 'grant --favorite %s' instead", favoriteFlag, favoriteFlag) + } + + groupFlag = fav.Group + } + + // Check authentication + _, err := auth.LoadAuthentication(profile, true) + if err != nil { + return fmt.Errorf("not authenticated, run 'grant login' first: %w", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), apiTimeout) + defer cancel() + + // Fetch groups eligibility (always Azure for Entra ID) + eligResp, err := groupsElig.ListGroupsEligibility(ctx, scamodels.CSPAzure) + if err != nil { + return fmt.Errorf("failed to fetch eligible groups: %w", err) + } + + if len(eligResp.Response) == 0 { + return fmt.Errorf("no eligible groups found, check your SCA policies") + } + + // Resolve directory names from cloud eligibility (best-effort) + dirNameMap := buildDirectoryNameMap(ctx, cloudElig, cmd.ErrOrStderr()) + for i := range eligResp.Response { + if name, ok := dirNameMap[eligResp.Response[i].DirectoryID]; ok { + eligResp.Response[i].DirectoryName = name + } + } + + // Resolve group + var selectedGroup *scamodels.GroupsEligibleTarget + + if groupFlag != "" { + // Direct mode + selectedGroup = findMatchingGroup(eligResp.Response, groupFlag) + if selectedGroup == nil { + return fmt.Errorf("group %q not found, run 'grant groups' to see available options", groupFlag) + } + } else { + // Interactive mode + selectedGroup, err = selector.SelectGroup(eligResp.Response) + if err != nil { + return fmt.Errorf("group selection failed: %w", err) + } + } + + // Build elevation request + req := &scamodels.GroupsElevateRequest{ + DirectoryID: selectedGroup.DirectoryID, + CSP: scamodels.CSPAzure, + Targets: []scamodels.GroupsElevateTarget{ + {GroupID: selectedGroup.GroupID}, + }, + } + + // Execute elevation + elevateResp, err := elevator.ElevateGroups(ctx, req) + if err != nil { + return fmt.Errorf("elevation request failed: %w", err) + } + + // Check results + if len(elevateResp.Results) == 0 { + return fmt.Errorf("elevation failed: no results returned") + } + + result := elevateResp.Results[0] + if result.ErrorInfo != nil { + return fmt.Errorf("elevation failed: %s - %s\n%s", + result.ErrorInfo.Code, + result.ErrorInfo.Message, + result.ErrorInfo.Description) + } + + // Display success + dirContext := "" + if selectedGroup.DirectoryName != "" { + dirContext = fmt.Sprintf(" in %s", selectedGroup.DirectoryName) + } + fmt.Fprintf(cmd.OutOrStdout(), "Elevated to group %s%s\n", selectedGroup.GroupName, dirContext) + fmt.Fprintf(cmd.OutOrStdout(), " Session ID: %s\n", result.SessionID) + + return nil +} + +// findMatchingGroup finds a group by name (case-insensitive) +func findMatchingGroup(groups []scamodels.GroupsEligibleTarget, name string) *scamodels.GroupsEligibleTarget { + for i := range groups { + if strings.EqualFold(groups[i].GroupName, name) { + return &groups[i] + } + } + return nil +} diff --git a/cmd/groups_test.go b/cmd/groups_test.go new file mode 100644 index 0000000..393ed97 --- /dev/null +++ b/cmd/groups_test.go @@ -0,0 +1,557 @@ +// NOTE: Do not use t.Parallel() in cmd/ tests due to package-level state +// (verbose, passedArgValidation) that is mutated during test execution. +package cmd + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/aaearon/grant-cli/internal/config" + scamodels "github.com/aaearon/grant-cli/internal/sca/models" + authmodels "github.com/cyberark/idsec-sdk-golang/pkg/models/auth" + commonmodels "github.com/cyberark/idsec-sdk-golang/pkg/models/common" +) + +func TestGroupsCommand(t *testing.T) { + now := time.Now() + expiresIn := commonmodels.IdsecRFC3339Time(now.Add(1 * time.Hour)) + + // newEligibleGroups returns a fresh copy to avoid mutation bleeding between tests + // (runGroups sets DirectoryName in-place on the response slice). + newEligibleGroups := func() *scamodels.GroupsEligibilityResponse { + return &scamodels.GroupsEligibilityResponse{ + Response: []scamodels.GroupsEligibleTarget{ + {DirectoryID: "dir1", GroupID: "grp1", GroupName: "Engineering"}, + {DirectoryID: "dir1", GroupID: "grp2", GroupName: "DevOps"}, + }, + Total: 2, + } + } + + // Cloud eligibility with DIRECTORY-type entry for name resolution + cloudEligWithDir := &scamodels.EligibilityResponse{ + Response: []scamodels.EligibleTarget{ + { + OrganizationID: "dir1", + WorkspaceID: "dir1", + WorkspaceName: "Contoso", + WorkspaceType: scamodels.WorkspaceTypeDirectory, + }, + }, + Total: 1, + } + + // Empty cloud eligibility (graceful degradation) + emptyCloudElig := &scamodels.EligibilityResponse{Response: []scamodels.EligibleTarget{}, Total: 0} + + tests := []struct { + name string + args []string + setupAuth func() *mockAuthLoader + setupCloudElig func() *mockEligibilityLister + setupElig func() *mockGroupsEligibilityLister + setupElevator func() *mockGroupsElevator + setupSelector func() *mockGroupSelector + wantContain []string + wantErr bool + }{ + { + name: "not authenticated", + args: []string{}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{loadErr: errNotAuthenticated} + }, + setupCloudElig: func() *mockEligibilityLister { return &mockEligibilityLister{} }, + setupElig: func() *mockGroupsEligibilityLister { return &mockGroupsEligibilityLister{} }, + setupElevator: func() *mockGroupsElevator { return &mockGroupsElevator{} }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"not authenticated"}, + wantErr: true, + }, + { + name: "no eligible groups", + args: []string{}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: emptyCloudElig} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{ + response: &scamodels.GroupsEligibilityResponse{Response: []scamodels.GroupsEligibleTarget{}, Total: 0}, + } + }, + setupElevator: func() *mockGroupsElevator { return &mockGroupsElevator{} }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"no eligible groups"}, + wantErr: true, + }, + { + name: "interactive mode - success with directory name", + args: []string{}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: cloudEligWithDir} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{response: newEligibleGroups()} + }, + setupElevator: func() *mockGroupsElevator { + return &mockGroupsElevator{ + response: &scamodels.GroupsElevateResponse{ + DirectoryID: "dir1", + CSP: scamodels.CSPAzure, + Results: []scamodels.GroupsElevateTargetResult{ + {GroupID: "grp1", SessionID: "sess1"}, + }, + }, + } + }, + setupSelector: func() *mockGroupSelector { + return &mockGroupSelector{ + group: &scamodels.GroupsEligibleTarget{DirectoryID: "dir1", DirectoryName: "Contoso", GroupID: "grp1", GroupName: "Engineering"}, + } + }, + wantContain: []string{"Elevated to group Engineering in Contoso", "Session ID: sess1"}, + wantErr: false, + }, + { + name: "success without directory name (graceful degradation)", + args: []string{"--group", "Engineering"}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{listErr: errors.New("cloud eligibility unavailable")} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{response: newEligibleGroups()} + }, + setupElevator: func() *mockGroupsElevator { + return &mockGroupsElevator{ + response: &scamodels.GroupsElevateResponse{ + DirectoryID: "dir1", + CSP: scamodels.CSPAzure, + Results: []scamodels.GroupsElevateTargetResult{ + {GroupID: "grp1", SessionID: "sess1"}, + }, + }, + } + }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"Elevated to group Engineering\n"}, + wantErr: false, + }, + { + name: "direct mode with --group flag", + args: []string{"--group", "Engineering"}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: cloudEligWithDir} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{response: newEligibleGroups()} + }, + setupElevator: func() *mockGroupsElevator { + return &mockGroupsElevator{ + elevateFunc: func(ctx context.Context, req *scamodels.GroupsElevateRequest) (*scamodels.GroupsElevateResponse, error) { + if req.Targets[0].GroupID != "grp1" { + t.Errorf("expected group ID grp1, got %s", req.Targets[0].GroupID) + } + if req.DirectoryID != "dir1" { + t.Errorf("expected directory ID dir1, got %s", req.DirectoryID) + } + return &scamodels.GroupsElevateResponse{ + DirectoryID: "dir1", + CSP: scamodels.CSPAzure, + Results: []scamodels.GroupsElevateTargetResult{ + {GroupID: "grp1", SessionID: "sess1"}, + }, + }, nil + }, + } + }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"Elevated to group Engineering in Contoso", "Session ID: sess1"}, + wantErr: false, + }, + { + name: "direct mode - group not found", + args: []string{"--group", "NonExistent"}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: emptyCloudElig} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{response: newEligibleGroups()} + }, + setupElevator: func() *mockGroupsElevator { return &mockGroupsElevator{} }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"group \"NonExistent\" not found"}, + wantErr: true, + }, + { + name: "direct mode - case insensitive match", + args: []string{"--group", "engineering"}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: emptyCloudElig} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{response: newEligibleGroups()} + }, + setupElevator: func() *mockGroupsElevator { + return &mockGroupsElevator{ + response: &scamodels.GroupsElevateResponse{ + DirectoryID: "dir1", + CSP: scamodels.CSPAzure, + Results: []scamodels.GroupsElevateTargetResult{ + {GroupID: "grp1", SessionID: "sess1"}, + }, + }, + } + }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"Elevated to group Engineering"}, + wantErr: false, + }, + { + name: "elevation error in result", + args: []string{"--group", "Engineering"}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: emptyCloudElig} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{response: newEligibleGroups()} + }, + setupElevator: func() *mockGroupsElevator { + return &mockGroupsElevator{ + response: &scamodels.GroupsElevateResponse{ + DirectoryID: "dir1", + CSP: scamodels.CSPAzure, + Results: []scamodels.GroupsElevateTargetResult{ + { + GroupID: "grp1", + SessionID: "", + ErrorInfo: &scamodels.ErrorInfo{ + Code: "ERR_INELIGIBLE", + Message: "Not eligible", + Description: "User not eligible", + }, + }, + }, + }, + } + }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"elevation failed", "ERR_INELIGIBLE", "Not eligible"}, + wantErr: true, + }, + { + name: "eligibility API error", + args: []string{}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: emptyCloudElig} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{listErr: errors.New("service unavailable")} + }, + setupElevator: func() *mockGroupsElevator { return &mockGroupsElevator{} }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"failed to fetch eligible groups"}, + wantErr: true, + }, + { + name: "elevation API error", + args: []string{"--group", "Engineering"}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: emptyCloudElig} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{response: newEligibleGroups()} + }, + setupElevator: func() *mockGroupsElevator { + return &mockGroupsElevator{elevateErr: errors.New("API error: forbidden")} + }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"elevation request failed"}, + wantErr: true, + }, + { + name: "interactive mode - selection error", + args: []string{}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: emptyCloudElig} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{response: newEligibleGroups()} + }, + setupElevator: func() *mockGroupsElevator { return &mockGroupsElevator{} }, + setupSelector: func() *mockGroupSelector { + return &mockGroupSelector{selectErr: errors.New("prompt interrupted")} + }, + wantContain: []string{"group selection failed"}, + wantErr: true, + }, + { + name: "elevation returns no results", + args: []string{"--group", "Engineering"}, + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: emptyCloudElig} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{response: newEligibleGroups()} + }, + setupElevator: func() *mockGroupsElevator { + return &mockGroupsElevator{ + response: &scamodels.GroupsElevateResponse{ + DirectoryID: "dir1", + CSP: scamodels.CSPAzure, + Results: []scamodels.GroupsElevateTargetResult{}, + }, + } + }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"no results returned"}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + auth := tt.setupAuth() + cloudElig := tt.setupCloudElig() + elig := tt.setupElig() + elevator := tt.setupElevator() + selector := tt.setupSelector() + + cmd := NewGroupsCommandWithDeps(auth, cloudElig, elig, elevator, selector) + output, err := executeCommand(cmd, tt.args...) + + if tt.wantErr && err == nil { + t.Errorf("expected error but got none") + } + if !tt.wantErr && err != nil { + t.Errorf("unexpected error: %v", err) + } + + for _, want := range tt.wantContain { + if !strings.Contains(output, want) { + t.Errorf("output missing %q\ngot:\n%s", want, output) + } + } + }) + } +} + +func TestGroupsCommandFavoriteMode(t *testing.T) { + now := time.Now() + expiresIn := commonmodels.IdsecRFC3339Time(now.Add(1 * time.Hour)) + + tests := []struct { + name string + args []string + cfg *config.Config + setupAuth func() *mockAuthLoader + setupCloudElig func() *mockEligibilityLister + setupElig func() *mockGroupsEligibilityLister + setupElevator func() *mockGroupsElevator + setupSelector func() *mockGroupSelector + wantContain []string + wantErr bool + }{ + { + name: "favorite mode - success", + args: []string{"--favorite", "my-grp"}, + cfg: func() *config.Config { + cfg := config.DefaultConfig() + _ = config.AddFavorite(cfg, "my-grp", config.Favorite{ + Type: config.FavoriteTypeGroups, + Provider: "azure", + Group: "Engineering", + DirectoryID: "dir1", + }) + return cfg + }(), + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: &scamodels.EligibilityResponse{Response: []scamodels.EligibleTarget{}}} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{ + response: &scamodels.GroupsEligibilityResponse{ + Response: []scamodels.GroupsEligibleTarget{ + {DirectoryID: "dir1", GroupID: "grp1", GroupName: "Engineering"}, + }, + Total: 1, + }, + } + }, + setupElevator: func() *mockGroupsElevator { + return &mockGroupsElevator{ + response: &scamodels.GroupsElevateResponse{ + DirectoryID: "dir1", + CSP: scamodels.CSPAzure, + Results: []scamodels.GroupsElevateTargetResult{ + {GroupID: "grp1", SessionID: "sess1"}, + }, + }, + } + }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"Elevated to group Engineering", "Session ID: sess1"}, + wantErr: false, + }, + { + name: "favorite not found", + args: []string{"--favorite", "nonexistent"}, + cfg: config.DefaultConfig(), + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { return &mockEligibilityLister{} }, + setupElig: func() *mockGroupsEligibilityLister { return &mockGroupsEligibilityLister{} }, + setupElevator: func() *mockGroupsElevator { return &mockGroupsElevator{} }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"not found"}, + wantErr: true, + }, + { + name: "cloud favorite is rejected", + args: []string{"--favorite", "cloud-fav"}, + cfg: func() *config.Config { + cfg := config.DefaultConfig() + _ = config.AddFavorite(cfg, "cloud-fav", config.Favorite{ + Provider: "azure", + Target: "sub-1", + Role: "Contributor", + }) + return cfg + }(), + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { return &mockEligibilityLister{} }, + setupElig: func() *mockGroupsEligibilityLister { return &mockGroupsEligibilityLister{} }, + setupElevator: func() *mockGroupsElevator { return &mockGroupsElevator{} }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"cloud favorite", "grant --favorite cloud-fav"}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + auth := tt.setupAuth() + cloudElig := tt.setupCloudElig() + elig := tt.setupElig() + elevator := tt.setupElevator() + selector := tt.setupSelector() + + cmd := NewGroupsCommandWithDeps(auth, cloudElig, elig, elevator, selector) + cmd.SetContext(context.Background()) + + // Inject config via the new cfg parameter + // We need to use the test constructor that accepts config + cmd = NewGroupsCommandWithDepsAndConfig(auth, cloudElig, elig, elevator, selector, tt.cfg) + output, err := executeCommand(cmd, tt.args...) + + if tt.wantErr && err == nil { + t.Errorf("expected error but got none, output:\n%s", output) + } + if !tt.wantErr && err != nil { + t.Errorf("unexpected error: %v", err) + } + + for _, want := range tt.wantContain { + if !strings.Contains(output, want) { + t.Errorf("output missing %q\ngot:\n%s", want, output) + } + } + }) + } +} + +func TestGroupsCommandUsage(t *testing.T) { + cmd := NewGroupsCommand() + + if cmd.Use != "groups" { + t.Errorf("expected Use='groups', got %q", cmd.Use) + } + if cmd.Short == "" { + t.Error("expected non-empty Short description") + } + + groupFlag := cmd.Flags().Lookup("group") + if groupFlag == nil { + t.Fatal("expected --group flag") + } + if groupFlag.Shorthand != "g" { + t.Errorf("expected -g shorthand, got %q", groupFlag.Shorthand) + } + + favoriteFlag := cmd.Flags().Lookup("favorite") + if favoriteFlag == nil { + t.Fatal("expected --favorite flag") + } + if favoriteFlag.Shorthand != "f" { + t.Errorf("expected -f shorthand, got %q", favoriteFlag.Shorthand) + } +} diff --git a/cmd/helpers.go b/cmd/helpers.go index 6b25928..e76923a 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -95,6 +95,41 @@ func fetchStatusData( return &statusData{sessions: sessions, nameMap: nameMap}, nil } +// buildDirectoryNameMap fetches Azure cloud eligibility to resolve directoryId -> name. +// It looks for DIRECTORY-type workspace entries whose workspaceId matches a directory ID. +// Falls back to organizationId -> first workspace name if no DIRECTORY entries exist. +// Errors are silently ignored (graceful degradation — groups display without directory context). +func buildDirectoryNameMap(ctx context.Context, eligLister eligibilityLister, errWriter io.Writer) map[string]string { + nameMap := make(map[string]string) + + resp, err := eligLister.ListEligibility(ctx, scamodels.CSPAzure) + if err != nil { + if verbose { + fmt.Fprintf(errWriter, "Warning: failed to resolve directory names: %v\n", err) + } + return nameMap + } + + // First pass: look for DIRECTORY-type workspaces (most specific) + for _, t := range resp.Response { + if t.WorkspaceType == scamodels.WorkspaceTypeDirectory && t.WorkspaceName != "" { + nameMap[t.WorkspaceID] = t.WorkspaceName + } + } + + // Second pass: fall back to organizationId -> first workspace name + // for orgs that didn't have a DIRECTORY entry + for _, t := range resp.Response { + if t.OrganizationID != "" && t.WorkspaceName != "" { + if _, exists := nameMap[t.OrganizationID]; !exists { + nameMap[t.OrganizationID] = t.WorkspaceName + } + } + } + + return nameMap +} + // buildWorkspaceNameMap fetches eligibility for each unique CSP in sessions // concurrently and builds a workspaceID -> workspaceName map. Errors are // silently ignored (graceful degradation — the raw workspace ID is shown diff --git a/cmd/interfaces.go b/cmd/interfaces.go index b3efe86..d49f51a 100644 --- a/cmd/interfaces.go +++ b/cmd/interfaces.go @@ -67,3 +67,18 @@ type authenticator interface { type profileSaver interface { SaveProfile(profile *sdkmodels.IdsecProfile) error } + +// groupsEligibilityLister interface for listing eligible Entra ID groups +type groupsEligibilityLister interface { + ListGroupsEligibility(ctx context.Context, csp models.CSP) (*models.GroupsEligibilityResponse, error) +} + +// groupsElevator interface for Entra ID group elevation +type groupsElevator interface { + ElevateGroups(ctx context.Context, req *models.GroupsElevateRequest) (*models.GroupsElevateResponse, error) +} + +// groupSelector interface for interactive group selection +type groupSelector interface { + SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTarget, error) +} diff --git a/cmd/test_mocks.go b/cmd/test_mocks.go index d17b406..5738028 100644 --- a/cmd/test_mocks.go +++ b/cmd/test_mocks.go @@ -178,3 +178,45 @@ func (m *mockNamePrompter) PromptName() (string, error) { } return m.name, m.promptErr } + +// mockGroupsEligibilityLister implements groupsEligibilityLister for testing +type mockGroupsEligibilityLister struct { + listFunc func(ctx context.Context, csp models.CSP) (*models.GroupsEligibilityResponse, error) + response *models.GroupsEligibilityResponse + listErr error +} + +func (m *mockGroupsEligibilityLister) ListGroupsEligibility(ctx context.Context, csp models.CSP) (*models.GroupsEligibilityResponse, error) { + if m.listFunc != nil { + return m.listFunc(ctx, csp) + } + return m.response, m.listErr +} + +// mockGroupsElevator implements groupsElevator for testing +type mockGroupsElevator struct { + elevateFunc func(ctx context.Context, req *models.GroupsElevateRequest) (*models.GroupsElevateResponse, error) + response *models.GroupsElevateResponse + elevateErr error +} + +func (m *mockGroupsElevator) ElevateGroups(ctx context.Context, req *models.GroupsElevateRequest) (*models.GroupsElevateResponse, error) { + if m.elevateFunc != nil { + return m.elevateFunc(ctx, req) + } + return m.response, m.elevateErr +} + +// mockGroupSelector implements groupSelector for testing +type mockGroupSelector struct { + selectFunc func(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTarget, error) + group *models.GroupsEligibleTarget + selectErr error +} + +func (m *mockGroupSelector) SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTarget, error) { + if m.selectFunc != nil { + return m.selectFunc(groups) + } + return m.group, m.selectErr +} diff --git a/internal/sca/models/groups_test.go b/internal/sca/models/groups_test.go new file mode 100644 index 0000000..0ffe574 --- /dev/null +++ b/internal/sca/models/groups_test.go @@ -0,0 +1,145 @@ +package models + +import ( + "encoding/json" + "testing" +) + +func TestGroupsEligibleTarget_UnmarshalJSON(t *testing.T) { + t.Parallel() + input := `{"directoryId":"dir1","groupId":"grp1","groupName":"Engineering"}` + + var target GroupsEligibleTarget + if err := json.Unmarshal([]byte(input), &target); err != nil { + t.Fatalf("unmarshal failed: %v", err) + } + if target.DirectoryID != "dir1" { + t.Errorf("DirectoryID = %q, want %q", target.DirectoryID, "dir1") + } + if target.GroupID != "grp1" { + t.Errorf("GroupID = %q, want %q", target.GroupID, "grp1") + } + if target.GroupName != "Engineering" { + t.Errorf("GroupName = %q, want %q", target.GroupName, "Engineering") + } +} + +func TestGroupsEligibilityResponse_UnmarshalJSON(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input string + wantLen int + wantTotal int + wantToken bool + }{ + { + name: "single group", + input: `{"response":[{"directoryId":"dir1","groupId":"grp1","groupName":"Admins"}],"total":1}`, + wantLen: 1, + wantTotal: 1, + }, + { + name: "empty response", + input: `{"response":[],"total":0}`, + wantLen: 0, + wantTotal: 0, + }, + { + name: "with next token", + input: `{"response":[{"directoryId":"d1","groupId":"g1","groupName":"G1"}],"nextToken":"tok123","total":10}`, + wantLen: 1, + wantTotal: 10, + wantToken: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var resp GroupsEligibilityResponse + if err := json.Unmarshal([]byte(tt.input), &resp); err != nil { + t.Fatalf("unmarshal failed: %v", err) + } + if len(resp.Response) != tt.wantLen { + t.Errorf("len(Response) = %d, want %d", len(resp.Response), tt.wantLen) + } + if resp.Total != tt.wantTotal { + t.Errorf("Total = %d, want %d", resp.Total, tt.wantTotal) + } + if tt.wantToken && resp.NextToken == nil { + t.Error("NextToken is nil, want non-nil") + } + }) + } +} + +func TestGroupsElevateRequest_MarshalJSON(t *testing.T) { + t.Parallel() + req := GroupsElevateRequest{ + DirectoryID: "dir1", + CSP: CSPAzure, + Targets: []GroupsElevateTarget{{GroupID: "grp1"}}, + } + + data, err := json.Marshal(req) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + + var m map[string]interface{} + if err := json.Unmarshal(data, &m); err != nil { + t.Fatalf("unmarshal check failed: %v", err) + } + if m["directoryId"] != "dir1" { + t.Errorf("directoryId = %v, want dir1", m["directoryId"]) + } + if m["csp"] != "AZURE" { + t.Errorf("csp = %v, want AZURE", m["csp"]) + } + targets := m["targets"].([]interface{}) + if len(targets) != 1 { + t.Errorf("targets length = %d, want 1", len(targets)) + } +} + +func TestGroupsElevateResponse_UnmarshalJSON(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input string + wantCount int + wantError bool + }{ + { + name: "success", + input: `{"directoryId":"dir1","csp":"AZURE","results":[{"groupId":"grp1","sessionId":"sess1"}]}`, + wantCount: 1, + }, + { + name: "with error", + input: `{"directoryId":"dir1","csp":"AZURE","results":[{"groupId":"grp1","sessionId":"","errorInfo":{"code":"ERR","message":"failed","description":"detail"}}]}`, + wantCount: 1, + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + var resp GroupsElevateResponse + if err := json.Unmarshal([]byte(tt.input), &resp); err != nil { + t.Fatalf("unmarshal failed: %v", err) + } + if len(resp.Results) != tt.wantCount { + t.Errorf("len(Results) = %d, want %d", len(resp.Results), tt.wantCount) + } + if tt.wantError && resp.Results[0].ErrorInfo == nil { + t.Error("ErrorInfo is nil, want non-nil") + } + if !tt.wantError && resp.Results[0].ErrorInfo != nil { + t.Errorf("ErrorInfo = %v, want nil", resp.Results[0].ErrorInfo) + } + }) + } +} diff --git a/internal/ui/group_selector.go b/internal/ui/group_selector.go new file mode 100644 index 0000000..24185ec --- /dev/null +++ b/internal/ui/group_selector.go @@ -0,0 +1,65 @@ +package ui + +import ( + "fmt" + "os" + "sort" + + "github.com/Iilun/survey/v2" + "github.com/aaearon/grant-cli/internal/sca/models" +) + +// FormatGroupOption formats a groups eligible target into a display string. +func FormatGroupOption(group models.GroupsEligibleTarget) string { + if group.DirectoryName != "" { + return fmt.Sprintf("Directory: %s / Group: %s", group.DirectoryName, group.GroupName) + } + return fmt.Sprintf("Group: %s", group.GroupName) +} + +// BuildGroupOptions builds a sorted list of display options from groups eligible targets. +func BuildGroupOptions(groups []models.GroupsEligibleTarget) []string { + if len(groups) == 0 { + return []string{} + } + + options := make([]string, len(groups)) + for i, group := range groups { + options[i] = FormatGroupOption(group) + } + + sort.Strings(options) + return options +} + +// FindGroupByDisplay finds a group by its formatted display string. +func FindGroupByDisplay(groups []models.GroupsEligibleTarget, display string) (*models.GroupsEligibleTarget, error) { + for i := range groups { + if FormatGroupOption(groups[i]) == display { + return &groups[i], nil + } + } + return nil, fmt.Errorf("group not found: %s", display) +} + +// SelectGroup presents an interactive selector for choosing a group. +func SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTarget, error) { + if len(groups) == 0 { + return nil, fmt.Errorf("no eligible groups available") + } + + options := BuildGroupOptions(groups) + + var selected string + prompt := &survey.Select{ + Message: "Select a group:", + Options: options, + Filter: nil, + } + + if err := survey.AskOne(prompt, &selected, survey.WithStdio(os.Stdin, os.Stderr, os.Stderr)); err != nil { + return nil, fmt.Errorf("group selection failed: %w", err) + } + + return FindGroupByDisplay(groups, selected) +} diff --git a/internal/ui/group_selector_test.go b/internal/ui/group_selector_test.go new file mode 100644 index 0000000..11e2d46 --- /dev/null +++ b/internal/ui/group_selector_test.go @@ -0,0 +1,161 @@ +package ui + +import ( + "testing" + + "github.com/aaearon/grant-cli/internal/sca/models" +) + +func TestFormatGroupOption(t *testing.T) { + t.Parallel() + tests := []struct { + name string + group models.GroupsEligibleTarget + want string + }{ + { + name: "simple group without directory name", + group: models.GroupsEligibleTarget{DirectoryID: "dir1", GroupID: "grp1", GroupName: "Engineering"}, + want: "Group: Engineering", + }, + { + name: "group with directory name", + group: models.GroupsEligibleTarget{DirectoryID: "dir1", DirectoryName: "Contoso", GroupID: "grp1", GroupName: "Cloud Admins"}, + want: "Directory: Contoso / Group: Cloud Admins", + }, + { + name: "group with empty directory name", + group: models.GroupsEligibleTarget{DirectoryID: "dir1", DirectoryName: "", GroupID: "grp2", GroupName: "DevOps"}, + want: "Group: DevOps", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := FormatGroupOption(tt.group) + if got != tt.want { + t.Errorf("FormatGroupOption() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestBuildGroupOptions(t *testing.T) { + t.Parallel() + tests := []struct { + name string + groups []models.GroupsEligibleTarget + want []string + }{ + { + name: "empty list", + groups: []models.GroupsEligibleTarget{}, + want: []string{}, + }, + { + name: "single group", + groups: []models.GroupsEligibleTarget{ + {GroupID: "g1", GroupName: "Admins"}, + }, + want: []string{"Group: Admins"}, + }, + { + name: "multiple groups sorted", + groups: []models.GroupsEligibleTarget{ + {GroupID: "g1", GroupName: "Zebra Team"}, + {GroupID: "g2", GroupName: "Alpha Team"}, + {GroupID: "g3", GroupName: "Beta Team"}, + }, + want: []string{ + "Group: Alpha Team", + "Group: Beta Team", + "Group: Zebra Team", + }, + }, + { + name: "groups with directory names sorted", + groups: []models.GroupsEligibleTarget{ + {DirectoryName: "Contoso", GroupID: "g1", GroupName: "Zebra Team"}, + {DirectoryName: "Contoso", GroupID: "g2", GroupName: "Alpha Team"}, + }, + want: []string{ + "Directory: Contoso / Group: Alpha Team", + "Directory: Contoso / Group: Zebra Team", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := BuildGroupOptions(tt.groups) + if len(got) != len(tt.want) { + t.Fatalf("BuildGroupOptions() length = %d, want %d", len(got), len(tt.want)) + } + for i := range got { + if got[i] != tt.want[i] { + t.Errorf("BuildGroupOptions()[%d] = %q, want %q", i, got[i], tt.want[i]) + } + } + }) + } +} + +func TestFindGroupByDisplay(t *testing.T) { + t.Parallel() + groups := []models.GroupsEligibleTarget{ + {DirectoryID: "dir1", DirectoryName: "Contoso", GroupID: "grp1", GroupName: "Engineering"}, + {DirectoryID: "dir1", DirectoryName: "Contoso", GroupID: "grp2", GroupName: "DevOps"}, + } + + tests := []struct { + name string + groups []models.GroupsEligibleTarget + display string + wantID string + wantErr bool + }{ + { + name: "found engineering", + groups: groups, + display: "Directory: Contoso / Group: Engineering", + wantID: "grp1", + }, + { + name: "found devops", + groups: groups, + display: "Directory: Contoso / Group: DevOps", + wantID: "grp2", + }, + { + name: "not found", + groups: groups, + display: "Directory: Contoso / Group: NonExistent", + wantErr: true, + }, + { + name: "empty groups", + groups: []models.GroupsEligibleTarget{}, + display: "Group: Test", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := FindGroupByDisplay(tt.groups, tt.display) + if (err != nil) != tt.wantErr { + t.Errorf("FindGroupByDisplay() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr { + return + } + if got.GroupID != tt.wantID { + t.Errorf("FindGroupByDisplay().GroupID = %q, want %q", got.GroupID, tt.wantID) + } + }) + } +} From 87c1ec928df76b51377cc0e25cb82a159e8dda5a Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 19 Feb 2026 13:51:36 +0100 Subject: [PATCH 9/9] fix: address code review issues in grant groups feature - Verify favorite DirectoryID during group matching to prevent wrong-group elevation across directories - Sort groups copy in SelectGroup so display-collision lookups search the same ordered slice the user sees - Resolve directory names for group sessions in grant status - Remove SilenceErrors/SilenceUsage from groups subcommand - Consolidate test constructors into single NewGroupsCommandWithDeps - Guard buildDirectoryNameMap against nil eligibility response --- CHANGELOG.md | 6 ++ cmd/groups.go | 38 +++++------ cmd/groups_test.go | 100 +++++++++++++++++++++++++++-- cmd/helpers.go | 4 +- cmd/status.go | 8 +++ cmd/status_test.go | 8 ++- internal/ui/group_selector.go | 15 ++++- internal/ui/group_selector_test.go | 37 +++++++++++ 8 files changed, 183 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28107c1..6689e77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,12 @@ All notable changes to this project will be documented in this file. - `grant revoke` now rejects `--provider` in direct mode (session IDs are already explicit) - `grant status` session formatting reuses shared `ui.FormatSessionOption` instead of duplicated logic - `buildWorkspaceNameMap` moved to shared `cmd/helpers.go` to eliminate cross-command dependency +- `grant groups --favorite` now verifies DirectoryID from the favorite, preventing wrong-group elevation when multiple directories have identically-named groups +- `grant groups` interactive selector sorts a local copy of groups, fixing wrong-group selection when display strings collide +- `grant status` now resolves directory names for group sessions via `buildDirectoryNameMap` +- `grant groups` subcommand no longer sets `SilenceErrors`/`SilenceUsage`, matching other subcommand patterns +- Removed dead code in `TestGroupsCommandFavoriteMode` and consolidated `NewGroupsCommandWithDeps`/`NewGroupsCommandWithDepsAndConfig` into a single test constructor +- `buildDirectoryNameMap` now handles nil eligibility response gracefully ## [0.2.1] - 2026-02-18 diff --git a/cmd/groups.go b/cmd/groups.go index 90f394f..8114b83 100644 --- a/cmd/groups.go +++ b/cmd/groups.go @@ -40,9 +40,7 @@ Examples: # Favorite mode grant groups --favorite my-group`, - SilenceErrors: true, - SilenceUsage: true, - RunE: runFn, + RunE: runFn, } cmd.Flags().StringP("group", "g", "", "Group name for direct mode") @@ -65,19 +63,7 @@ func NewGroupsCommand() *cobra.Command { // NewGroupsCommandWithDeps creates a groups command with injected dependencies for testing. func NewGroupsCommandWithDeps( - auth authLoader, - cloudElig eligibilityLister, - groupsElig groupsEligibilityLister, - elevator groupsElevator, - selector groupSelector, -) *cobra.Command { - return newGroupsCommand(func(cmd *cobra.Command, args []string) error { - return runGroups(cmd, auth, cloudElig, groupsElig, elevator, selector, nil, nil) - }) -} - -// NewGroupsCommandWithDepsAndConfig creates a groups command with injected dependencies and config for testing. -func NewGroupsCommandWithDepsAndConfig( + profile *sdkmodels.IdsecProfile, auth authLoader, cloudElig eligibilityLister, groupsElig groupsEligibilityLister, @@ -86,7 +72,7 @@ func NewGroupsCommandWithDepsAndConfig( cfg *config.Config, ) *cobra.Command { return newGroupsCommand(func(cmd *cobra.Command, args []string) error { - return runGroups(cmd, auth, cloudElig, groupsElig, elevator, selector, nil, cfg) + return runGroups(cmd, auth, cloudElig, groupsElig, elevator, selector, profile, cfg) }) } @@ -103,6 +89,8 @@ func runGroups( groupFlag, _ := cmd.Flags().GetString("group") favoriteFlag, _ := cmd.Flags().GetString("favorite") + var favDirectoryID string + if favoriteFlag != "" { if cfg == nil { var err error @@ -122,6 +110,7 @@ func runGroups( } groupFlag = fav.Group + favDirectoryID = fav.DirectoryID } // Check authentication @@ -155,9 +144,12 @@ func runGroups( var selectedGroup *scamodels.GroupsEligibleTarget if groupFlag != "" { - // Direct mode - selectedGroup = findMatchingGroup(eligResp.Response, groupFlag) + // Direct mode (or favorite-resolved) + selectedGroup = findMatchingGroup(eligResp.Response, groupFlag, favDirectoryID) if selectedGroup == nil { + if favDirectoryID != "" { + return fmt.Errorf("group %q not found in directory %q, run 'grant groups' to see available options", groupFlag, favDirectoryID) + } return fmt.Errorf("group %q not found, run 'grant groups' to see available options", groupFlag) } } else { @@ -207,10 +199,14 @@ func runGroups( return nil } -// findMatchingGroup finds a group by name (case-insensitive) -func findMatchingGroup(groups []scamodels.GroupsEligibleTarget, name string) *scamodels.GroupsEligibleTarget { +// findMatchingGroup finds a group by name (case-insensitive). +// If directoryID is non-empty, only matches groups in that directory. +func findMatchingGroup(groups []scamodels.GroupsEligibleTarget, name string, directoryID string) *scamodels.GroupsEligibleTarget { for i := range groups { if strings.EqualFold(groups[i].GroupName, name) { + if directoryID != "" && groups[i].DirectoryID != directoryID { + continue + } return &groups[i] } } diff --git a/cmd/groups_test.go b/cmd/groups_test.go index 393ed97..17dd81d 100644 --- a/cmd/groups_test.go +++ b/cmd/groups_test.go @@ -374,7 +374,7 @@ func TestGroupsCommand(t *testing.T) { elevator := tt.setupElevator() selector := tt.setupSelector() - cmd := NewGroupsCommandWithDeps(auth, cloudElig, elig, elevator, selector) + cmd := NewGroupsCommandWithDeps(nil, auth, cloudElig, elig, elevator, selector, nil) output, err := executeCommand(cmd, tt.args...) if tt.wantErr && err == nil { @@ -471,6 +471,97 @@ func TestGroupsCommandFavoriteMode(t *testing.T) { wantContain: []string{"not found"}, wantErr: true, }, + { + name: "favorite mode - directory ID filters correct group", + args: []string{"--favorite", "my-grp"}, + cfg: func() *config.Config { + cfg := config.DefaultConfig() + _ = config.AddFavorite(cfg, "my-grp", config.Favorite{ + Type: config.FavoriteTypeGroups, + Provider: "azure", + Group: "Engineering", + DirectoryID: "dir1", + }) + return cfg + }(), + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: &scamodels.EligibilityResponse{Response: []scamodels.EligibleTarget{}}} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{ + response: &scamodels.GroupsEligibilityResponse{ + Response: []scamodels.GroupsEligibleTarget{ + {DirectoryID: "dir2", GroupID: "grp-wrong", GroupName: "Engineering"}, + {DirectoryID: "dir1", GroupID: "grp-right", GroupName: "Engineering"}, + }, + Total: 2, + }, + } + }, + setupElevator: func() *mockGroupsElevator { + return &mockGroupsElevator{ + elevateFunc: func(ctx context.Context, req *scamodels.GroupsElevateRequest) (*scamodels.GroupsElevateResponse, error) { + if req.Targets[0].GroupID != "grp-right" { + t.Errorf("expected group ID grp-right, got %s", req.Targets[0].GroupID) + } + if req.DirectoryID != "dir1" { + t.Errorf("expected directory ID dir1, got %s", req.DirectoryID) + } + return &scamodels.GroupsElevateResponse{ + DirectoryID: "dir1", + CSP: scamodels.CSPAzure, + Results: []scamodels.GroupsElevateTargetResult{ + {GroupID: "grp-right", SessionID: "sess1"}, + }, + }, nil + }, + } + }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"Elevated to group Engineering", "Session ID: sess1"}, + wantErr: false, + }, + { + name: "favorite mode - directory ID mismatch returns error", + args: []string{"--favorite", "my-grp"}, + cfg: func() *config.Config { + cfg := config.DefaultConfig() + _ = config.AddFavorite(cfg, "my-grp", config.Favorite{ + Type: config.FavoriteTypeGroups, + Provider: "azure", + Group: "Engineering", + DirectoryID: "dir-nonexistent", + }) + return cfg + }(), + setupAuth: func() *mockAuthLoader { + return &mockAuthLoader{ + token: &authmodels.IdsecToken{Token: "jwt", Username: "user@example.com", ExpiresIn: expiresIn}, + } + }, + setupCloudElig: func() *mockEligibilityLister { + return &mockEligibilityLister{response: &scamodels.EligibilityResponse{Response: []scamodels.EligibleTarget{}}} + }, + setupElig: func() *mockGroupsEligibilityLister { + return &mockGroupsEligibilityLister{ + response: &scamodels.GroupsEligibilityResponse{ + Response: []scamodels.GroupsEligibleTarget{ + {DirectoryID: "dir1", GroupID: "grp1", GroupName: "Engineering"}, + }, + Total: 1, + }, + } + }, + setupElevator: func() *mockGroupsElevator { return &mockGroupsElevator{} }, + setupSelector: func() *mockGroupSelector { return &mockGroupSelector{} }, + wantContain: []string{"not found in directory"}, + wantErr: true, + }, { name: "cloud favorite is rejected", args: []string{"--favorite", "cloud-fav"}, @@ -505,12 +596,7 @@ func TestGroupsCommandFavoriteMode(t *testing.T) { elevator := tt.setupElevator() selector := tt.setupSelector() - cmd := NewGroupsCommandWithDeps(auth, cloudElig, elig, elevator, selector) - cmd.SetContext(context.Background()) - - // Inject config via the new cfg parameter - // We need to use the test constructor that accepts config - cmd = NewGroupsCommandWithDepsAndConfig(auth, cloudElig, elig, elevator, selector, tt.cfg) + cmd := NewGroupsCommandWithDeps(nil, auth, cloudElig, elig, elevator, selector, tt.cfg) output, err := executeCommand(cmd, tt.args...) if tt.wantErr && err == nil { diff --git a/cmd/helpers.go b/cmd/helpers.go index e76923a..857322f 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -103,8 +103,8 @@ func buildDirectoryNameMap(ctx context.Context, eligLister eligibilityLister, er nameMap := make(map[string]string) resp, err := eligLister.ListEligibility(ctx, scamodels.CSPAzure) - if err != nil { - if verbose { + if err != nil || resp == nil { + if verbose && err != nil { fmt.Fprintf(errWriter, "Warning: failed to resolve directory names: %v\n", err) } return nameMap diff --git a/cmd/status.go b/cmd/status.go index cb83a97..6aa2305 100644 --- a/cmd/status.go +++ b/cmd/status.go @@ -75,6 +75,14 @@ func runStatus(cmd *cobra.Command, authLoader authLoader, sessionLister sessionL return err } + // Resolve directory names for group sessions (best-effort) + dirNameMap := buildDirectoryNameMap(ctx, eligLister, cmd.ErrOrStderr()) + for k, v := range dirNameMap { + if _, exists := data.nameMap[k]; !exists { + data.nameMap[k] = v + } + } + // Display sessions if len(data.sessions.Response) == 0 { fmt.Fprintf(cmd.OutOrStdout(), "\nNo active sessions.\n") diff --git a/cmd/status_test.go b/cmd/status_test.go index 11aff6b..8d6db13 100644 --- a/cmd/status_test.go +++ b/cmd/status_test.go @@ -492,6 +492,12 @@ func TestStatusCommand(t *testing.T) { response: &scamodels.EligibilityResponse{ Response: []scamodels.EligibleTarget{ {WorkspaceID: "/subscriptions/sub-1", WorkspaceName: "My Subscription"}, + { + OrganizationID: "29cb7961-dir-uuid", + WorkspaceID: "29cb7961-dir-uuid", + WorkspaceName: "Contoso Directory", + WorkspaceType: scamodels.WorkspaceTypeDirectory, + }, }, }, } @@ -501,7 +507,7 @@ func TestStatusCommand(t *testing.T) { "Contributor on My Subscription (/subscriptions/sub-1)", "session: cloud-session-1", "Groups sessions:", - "Group: group-uuid-1 in 29cb7961-dir-uuid", + "Group: group-uuid-1 in Contoso Directory", "session: group-session-1", }, wantErr: false, diff --git a/internal/ui/group_selector.go b/internal/ui/group_selector.go index 24185ec..b20d830 100644 --- a/internal/ui/group_selector.go +++ b/internal/ui/group_selector.go @@ -43,12 +43,23 @@ func FindGroupByDisplay(groups []models.GroupsEligibleTarget, display string) (* } // SelectGroup presents an interactive selector for choosing a group. +// It sorts a copy of the groups so that FindGroupByDisplay searches the same +// ordered slice the user saw, avoiding wrong-group selection on display collisions. func SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTarget, error) { if len(groups) == 0 { return nil, fmt.Errorf("no eligible groups available") } - options := BuildGroupOptions(groups) + sorted := make([]models.GroupsEligibleTarget, len(groups)) + copy(sorted, groups) + sort.Slice(sorted, func(i, j int) bool { + return FormatGroupOption(sorted[i]) < FormatGroupOption(sorted[j]) + }) + + options := make([]string, len(sorted)) + for i := range sorted { + options[i] = FormatGroupOption(sorted[i]) + } var selected string prompt := &survey.Select{ @@ -61,5 +72,5 @@ func SelectGroup(groups []models.GroupsEligibleTarget) (*models.GroupsEligibleTa return nil, fmt.Errorf("group selection failed: %w", err) } - return FindGroupByDisplay(groups, selected) + return FindGroupByDisplay(sorted, selected) } diff --git a/internal/ui/group_selector_test.go b/internal/ui/group_selector_test.go index 11e2d46..5597e80 100644 --- a/internal/ui/group_selector_test.go +++ b/internal/ui/group_selector_test.go @@ -102,6 +102,43 @@ func TestBuildGroupOptions(t *testing.T) { } } +func TestBuildGroupOptions_DuplicateDisplayStrings(t *testing.T) { + t.Parallel() + // Two groups with same name and no DirectoryName produce identical display strings + groups := []models.GroupsEligibleTarget{ + {DirectoryID: "dir1", GroupID: "grp1", GroupName: "Engineering"}, + {DirectoryID: "dir2", GroupID: "grp2", GroupName: "Engineering"}, + } + options := BuildGroupOptions(groups) + if len(options) != 2 { + t.Fatalf("expected 2 options, got %d", len(options)) + } + // Both should be "Group: Engineering" (duplicate is expected) + for _, opt := range options { + if opt != "Group: Engineering" { + t.Errorf("unexpected option %q", opt) + } + } +} + +func TestFindGroupByDisplay_DuplicateDisplayStrings(t *testing.T) { + t.Parallel() + // When display strings collide, FindGroupByDisplay returns the first match + // in the slice it's given. SelectGroup sorts a copy, so the caller controls order. + groups := []models.GroupsEligibleTarget{ + {DirectoryID: "dir2", GroupID: "grp2", GroupName: "Engineering"}, + {DirectoryID: "dir1", GroupID: "grp1", GroupName: "Engineering"}, + } + got, err := FindGroupByDisplay(groups, "Group: Engineering") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Should return first in slice (grp2 since dir2 is first) + if got.GroupID != "grp2" { + t.Errorf("expected grp2 (first in slice), got %q", got.GroupID) + } +} + func TestFindGroupByDisplay(t *testing.T) { t.Parallel() groups := []models.GroupsEligibleTarget{