feat: add access request workflow commands#39
Conversation
Implement grant request subcommands: submit, list, get, cancel, approve, reject. Includes workflows service with ISP auth, interactive submit prompts, list filter validation, timezone validation via time.LoadLocation, and injectable DI for testing.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “access requests” workflow to the CLI by adding a Workflows API client (internal/workflows) and a new grant request command group for submitting, listing, viewing, canceling, and finalizing access requests (with JSON output support).
Changes:
- Add
internal/workflowsservice + models to call the Access Requests (Workflows) API, including offset/limit pagination and logging. - Add
grant requestcommand group with subcommands (submit,list,get,cancel,approve,reject) and JSON output types. - Add/extend tests and documentation/changelog entries for the new functionality.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/workflows/service.go | Implements Access Requests API client (forms, list, get, submit, cancel, finalize) + pagination + response checking. |
| internal/workflows/service_config.go | Registers service configuration (service name + required isp authenticator). |
| internal/workflows/logging_client.go | Adds request/response logging wrapper for the workflows HTTP client. |
| internal/workflows/service_test.go | Adds unit tests for workflows service behavior, pagination, and error handling. |
| internal/workflows/models/request.go | Introduces Access Request domain model types (AccessRequest, RequestState, RequestResult, etc.). |
| internal/workflows/models/request_test.go | Adds JSON unmarshal + helper method tests for request models. |
| internal/workflows/models/form.go | Adds request form model types and IsRequired helper. |
| internal/workflows/models/form_test.go | Adds tests for form parsing and IsRequired. |
| internal/workflows/models/submit.go | Adds submit request payload model. |
| internal/workflows/models/cancel.go | Adds cancel request payload model. |
| internal/workflows/models/finalize.go | Adds finalize request payload model. |
| cmd/commands.go | Registers NewRequestCommand() on the root command. |
| cmd/interfaces.go | Adds accessRequestService interface for DI in command code/tests. |
| cmd/test_mocks.go | Adds mockAccessRequestService for command tests. |
| cmd/request.go | Adds grant request parent command, bootstrapping, formatting, and output mapping. |
| cmd/request_submit.go | Implements grant request submit (interactive + non-interactive flags, validation, target resolution). |
| cmd/request_list.go | Implements grant request list with validated filters/sort and text/JSON output. |
| cmd/request_get.go | Implements grant request get (text + JSON). |
| cmd/request_cancel.go | Implements grant request cancel (text + JSON). |
| cmd/request_finalize.go | Implements grant request approve/reject (text + JSON). |
| cmd/output_types.go | Adds JSON output structs for access requests. |
| cmd/request_test.go | Adds comprehensive command tests (list/get/submit/cancel/finalize, JSON output, validation). |
| CLAUDE.md | Documents the new Workflows API package, endpoints, and CLI commands. |
| CHANGELOG.md | Adds changelog entries for the new grant request commands and workflows client. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for range maxPages { | ||
| qp := make(map[string]string) | ||
| qp["limit"] = strconv.Itoa(limit) | ||
| qp["offset"] = strconv.Itoa(offset) | ||
| if params.Filter != "" { |
There was a problem hiding this comment.
ListRequests stops after maxPages iterations but then returns success even if more pages remain. If totalCount > maxPages*limit, this will silently return a truncated item list while still reporting the larger totalCount. Consider tracking the page index and returning an explicit error when the loop exhausts maxPages without hitting the break condition (similar to internal/sca paginate).
| // formatTimestamp truncates a timestamp to just the date+time portion (no microseconds). | ||
| func formatTimestamp(ts string) string { | ||
| if len(ts) > 19 { | ||
| return ts[:19] | ||
| } | ||
| return ts |
There was a problem hiding this comment.
formatTimestamp truncates the input to 19 characters, which will also strip timezone/offset information if the API ever returns RFC3339 timestamps with a trailing "Z" or "+/-HH:MM". If the goal is only to drop fractional seconds, consider parsing RFC3339/RFC3339Nano and re-formatting without subseconds while preserving the timezone, or only trimming the fractional part when present.
- Resolve real IANA timezone instead of "Local" default - Validate provider flag via parseProvider() before API calls - Add context.WithTimeout to prevent indefinite hangs - Add confirmation prompt with --yes flag to skip - Add inline survey validators for date/time/timezone formats - Handle partial --target/--role by filtering selector - Prompt only for missing fields (pass existing to promptFn) - Add IsInteractive() guard in defaultSubmitPrompt() - Reorder prompts: timezone before date for correct defaults - Enforce presence of date/timezone/from/to in validation
|
@copilot review the latest commit |
…imestamp - ListRequests: return errPaginationLimit sentinel error when maxPages exhausted instead of silently returning truncated data (matches SCA paginate() pattern) - formatTimestamp: parse RFC3339Nano and reformat without fractional seconds while preserving timezone offset; fall back to trimming at '.' for non-RFC3339 timestamps - Extract pagination limit error to package-level var errPaginationLimit - Test: use errors.Is(err, errPaginationLimit) for precise assertion - Test: add RFC3339 timezone cases to TestFormatTimestamp Agent-Logs-Url: https://github.com/aaearon/grant-cli/sessions/a09d2658-41d5-414b-9336-32ddff12742f Co-authored-by: aaearon <812640+aaearon@users.noreply.github.com>
Addressed both reviewer comments in commit
|
The interactive selector now shows deduplicated workspaces (not role/workspace combos) since on-demand requests are for roles the user doesn't already have a JIT policy for. Users select a workspace they have access to, then provide the role ID they want to request. - Replace role-based target selector with workspace selector - Add --role-id flag (required) for the role being requested - --role flag becomes optional display name (defaults to role ID) - Deduplicate eligibility targets to unique workspaces by ID - Add workspace type labels in selector (Account, Subscription, etc.)
API returns mixed-case workspace types (e.g. "directory", "management_group") while Go constants are uppercase. Use case-insensitive comparison for display labels.
Replaces the manual Role ID prompt with a fuzzy-filterable list fetched from the SCA on-demand role discovery endpoints. Supported workspace types: DIRECTORY (azure_ad), ACCOUNT (aws), MANAGEMENT_GROUP (azure_resource). Other Azure-resource scopes reject with a clear error pointing to --role-id. Roles are cached in ~/.grant/cache/ondemand_roles_<platform>_<sha256>.json (shared 4h Store). Interactive prompt order is now: workspace → role → reason → priority → timezone → date → start time → end time.
When <requestId> is omitted in a terminal, each command now shows a fuzzy-filterable picker scoped to actionable requests: - cancel: STARTING/RUNNING/PENDING requests you created (role=CREATOR) - approve/reject: PENDING requests assigned to you (role=APPROVER) - get: all requests (no filter) Positional arg path is unchanged. Non-TTY invocation (pipe, CI) returns ErrNotInteractive with a hint to pass the ID or run `grant request list`. Adds internal/ui/request_selector.go (Format/Build/Select trio) and cmd/request_picker.go (pickerScope + resolveRequestIDFn injectable var).
Guard against auth errors in non-interactive contexts by checking ErrNotInteractive before bootstrapping the workflows service. Fix timezone-offset timestamp sort by parsing to time.Time instead of relying on lexicographic string comparison.
Summary
grant requestparent command with subcommands:submit,list,get,cancel,approve,rejectinternal/workflows/package withAccessRequestService(ISP auth, offset/limit pagination)time.LoadLocation(accepts UTC, CET, IANA identifiers)resolveSubmitTargetFn,submitPromptFn) for testabilityTest plan
make test— all tests pass (new + existing)make lint— zero violationsmake build— binary buildsgrant request list --state INVALIDreturns validation errorgrant request list --state runningworks (case-insensitive)grant request submit --helpshows all flagsgrant request submitwith--timezone UTCaccepted