diff --git a/CHANGELOG.md b/CHANGELOG.md index f5aba00..0b5fdec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ All notable changes to this project will be documented in this file. - `--output json` is now a pure serialisation flag; it no longer forces non-interactive mode. Interactive pickers and prompts (e.g. `grant request get -o json` with no ID, `grant request submit -o json` without `--target`/`--role`) work in a TTY, writing prompts to stderr and JSON to stdout. +### Fixed + +- `grant request submit` no longer performs 2–3 back-to-back ISP authentication cycles. Profile load + `Authenticate` is now memoized per-invocation and shared by `bootstrapSCAService` / `bootstrapWorkflowsService`. `Authenticate` is also called with `refreshAuth=false` so cached, unexpired keyring tokens short-circuit the network round-trip. Keyring ops drop from ~18× to ~6× per invocation. + ### Added - `grant request` command group for managing access requests through the approval workflow diff --git a/cmd/request.go b/cmd/request.go index 59068ac..79645c2 100644 --- a/cmd/request.go +++ b/cmd/request.go @@ -8,9 +8,6 @@ import ( "github.com/aaearon/grant-cli/internal/workflows" "github.com/aaearon/grant-cli/internal/workflows/models" - "github.com/cyberark/idsec-sdk-golang/pkg/auth" - authmodels "github.com/cyberark/idsec-sdk-golang/pkg/models/auth" - "github.com/cyberark/idsec-sdk-golang/pkg/profiles" "github.com/spf13/cobra" ) @@ -54,18 +51,12 @@ func NewRequestCommandWithDeps(reqSvc accessRequestService) *cobra.Command { } // bootstrapWorkflowsService creates an authenticated AccessRequestService. +// Reuses the memoized ISP auth from bootstrapISPAuth so repeated bootstraps in +// a single invocation (e.g. `grant request submit`) share one auth cycle. func bootstrapWorkflowsService() (*workflows.AccessRequestService, error) { - loader := profiles.DefaultProfilesLoader() - profile, err := (*loader).LoadProfile("grant") + ispAuth, _, err := bootstrapISPAuth() if err != nil { - return nil, fmt.Errorf("failed to load profile: %w", err) - } - - ispAuth := auth.NewIdsecISPAuth(true) - - _, err = ispAuth.Authenticate(profile, nil, &authmodels.IdsecSecret{Secret: ""}, false, true) - if err != nil { - return nil, fmt.Errorf("authentication failed: %w", err) + return nil, err } svc, err := workflows.NewAccessRequestService(ispAuth) diff --git a/cmd/root.go b/cmd/root.go index 1a80e7c..a448fc3 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -129,19 +129,55 @@ Examples: var rootCmd = newRootCommand(runElevateProduction) -// bootstrapSCAService loads the profile, authenticates, and creates the SCA service. -func bootstrapSCAService() (auth.IdsecAuth, *sca.SCAAccessService, *sdkmodels.IdsecProfile, error) { +// bootstrap memoization state — shared auth/profile across all service +// bootstraps within a single process invocation so we authenticate exactly once. +var ( + bootstrapOnce = new(sync.Once) + bootstrapISPAuthVal auth.IdsecAuth + bootstrapProfileVal *sdkmodels.IdsecProfile + errBootstrap error +) + +// bootstrapImpl is the function that performs the profile load + authentication. +// Overridable for tests. refreshAuth=false lets the SDK reuse cached keyring +// tokens while still valid; a full re-auth only happens on token expiry. +var bootstrapImpl = func() (auth.IdsecAuth, *sdkmodels.IdsecProfile, error) { loader := profiles.DefaultProfilesLoader() profile, err := (*loader).LoadProfile("grant") if err != nil { - return nil, nil, nil, fmt.Errorf("failed to load profile: %w", err) + return nil, nil, fmt.Errorf("failed to load profile: %w", err) } - ispAuth := auth.NewIdsecISPAuth(true) + if _, err := ispAuth.Authenticate(profile, nil, &authmodels.IdsecSecret{Secret: ""}, false, false); err != nil { + return nil, nil, fmt.Errorf("authentication failed: %w", err) + } + return ispAuth, profile, nil +} - _, err = ispAuth.Authenticate(profile, nil, &authmodels.IdsecSecret{Secret: ""}, false, true) +// bootstrapISPAuth returns a process-wide memoized auth.IdsecAuth and profile so +// repeated service bootstraps in one invocation share a single auth cycle. +func bootstrapISPAuth() (auth.IdsecAuth, *sdkmodels.IdsecProfile, error) { + bootstrapOnce.Do(func() { + bootstrapISPAuthVal, bootstrapProfileVal, errBootstrap = bootstrapImpl() + }) + return bootstrapISPAuthVal, bootstrapProfileVal, errBootstrap +} + +// resetBootstrapCache clears the memoized auth state. Intended for tests. +// Uses pointer reassignment to avoid copying the sync.Once value. +func resetBootstrapCache() { + bootstrapOnce = new(sync.Once) + bootstrapISPAuthVal = nil + bootstrapProfileVal = nil + errBootstrap = nil +} + +// bootstrapSCAService loads the profile, authenticates, and creates the SCA service. +// The underlying auth is memoized across calls within a single invocation. +func bootstrapSCAService() (auth.IdsecAuth, *sca.SCAAccessService, *sdkmodels.IdsecProfile, error) { + ispAuth, profile, err := bootstrapISPAuth() if err != nil { - return nil, nil, nil, fmt.Errorf("authentication failed: %w", err) + return nil, nil, nil, err } svc, err := sca.NewSCAAccessService(ispAuth) diff --git a/cmd/root_test.go b/cmd/root_test.go index 41c3c04..2618e93 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -8,10 +8,57 @@ import ( "github.com/aaearon/grant-cli/internal/sca/models" "github.com/aaearon/grant-cli/internal/ui" + sdkauth "github.com/cyberark/idsec-sdk-golang/pkg/auth" "github.com/cyberark/idsec-sdk-golang/pkg/config" + sdkmodels "github.com/cyberark/idsec-sdk-golang/pkg/models" "github.com/spf13/cobra" ) +func TestBootstrapISPAuth_MemoizesRepeatCalls(t *testing.T) { + resetBootstrapCache() + t.Cleanup(resetBootstrapCache) + + origImpl := bootstrapImpl + t.Cleanup(func() { bootstrapImpl = origImpl }) + + calls := 0 + stubAuth := sdkauth.NewIdsecISPAuth(false) // non-caching stub; Authenticate never called + stubProfile := &sdkmodels.IdsecProfile{} + bootstrapImpl = func() (sdkauth.IdsecAuth, *sdkmodels.IdsecProfile, error) { + calls++ + return stubAuth, stubProfile, nil + } + + ispAuth1, profile1, err := bootstrapISPAuth() + if err != nil { + t.Fatalf("bootstrapISPAuth #1: %v", err) + } + ispAuth2, profile2, err := bootstrapISPAuth() + if err != nil { + t.Fatalf("bootstrapISPAuth #2: %v", err) + } + ispAuth3, profile3, err := bootstrapISPAuth() + if err != nil { + t.Fatalf("bootstrapISPAuth #3: %v", err) + } + + if calls != 1 { + t.Errorf("expected bootstrapImpl to be invoked exactly once across 3 calls, got %d", calls) + } + if ispAuth1 != ispAuth2 || ispAuth2 != ispAuth3 { + t.Error("expected ispAuth to be the same instance across all bootstrap calls") + } + if profile1 != profile2 || profile2 != profile3 { + t.Error("expected profile to be the same instance across all bootstrap calls") + } + if ispAuth1 != stubAuth { + t.Error("expected returned auth to match stub") + } + if profile1 != stubProfile { + t.Error("expected returned profile to match stub") + } +} + func TestNewRootCommand_SilenceFlags(t *testing.T) { cmd := newRootCommand(nil)