diff --git a/docs/connector.mdx b/docs/connector.mdx index a5278bbd..459bc170 100644 --- a/docs/connector.mdx +++ b/docs/connector.mdx @@ -343,6 +343,7 @@ Navigate to the **Okta API Scopes** tab and grant the scopes required for your u - `okta.users.read` and `okta.groups.read` (required for sync) - `okta.roles.read` and `okta.apps.read` (required for sync) - `okta.users.manage`, `okta.groups.manage`, `okta.roles.manage`, `okta.apps.manage` (required for provisioning) + - `okta.logs.read` (required for the event feed) - `okta.apiTokens.read` (required when **Sync secrets** is enabled) - `okta.devices.read` (required when the **Device** resource type is enabled) diff --git a/docs/docs-info.md b/docs/docs-info.md index 2ab73958..eab4e92a 100644 --- a/docs/docs-info.md +++ b/docs/docs-info.md @@ -50,6 +50,7 @@ Internal technical notes for maintainers. Customer-facing setup lives in [`docs/ * **Does it need specific scopes/permissions?** - Sync (read): `okta.users.read`, `okta.groups.read`, `okta.apps.read`, `okta.roles.read` (roles need elevated admin) + - Event feed: `okta.logs.read` (System Log; required or `list_events` fails every run) - Provision (write): `okta.users.manage`, `okta.groups.manage`, `okta.apps.manage`, `okta.roles.manage` as needed - API tokens inherit the admin role of the creating user (Super Admin / custom role / Read-only+App+Group admin combinations — see the permissions chart in `connector.mdx`) @@ -83,7 +84,7 @@ Consequences for setup docs: The users case is the dangerous one: the sync exits 0 and writes a bundle containing zero users, so a missing admin role presents as a successful empty sync rather than an error. Assigning Super Administrator makes both return data. -**Requested scopes.** On this path the connector requests the four default read scopes plus all four `*.manage` provisioning scopes unconditionally, then adds `okta.apiTokens.read` when `--sync-secrets` is set and `okta.devices.read` when device sync is enabled. Per the note in `connector.go`, a scope the app has not been granted drops from the issued token and only surfaces as a 403 on first use, so a read-only app still authenticates. +**Requested scopes.** On this path the connector requests the four default read scopes, all four `*.manage` provisioning scopes, and `okta.logs.read` unconditionally, then adds `okta.apiTokens.read` when `--sync-secrets` is set and `okta.devices.read` when device sync is enabled. `okta.logs.read` backs the event feed's System Log reads (`GET /api/v1/logs`) and is unconditional because `EventFeeds` is always advertised, so there is no local opt-in to gate it on; without it `list_events` fails every run. Per the note in `connector.go`, a scope the app has not been granted drops from the issued token and only surfaces as a 403 on first use, so a read-only app still authenticates. **Console caveat for whoever writes customer docs.** During CXH-2092 / CXH-2124 validation, the Okta Admin Console's **Okta API Scopes** tab twice failed to persist scope grants with no error shown — the grants did not appear in `GET /api/v1/apps/{id}/grants`. `POST /api/v1/apps/{id}/grants` worked reliably. The customer-facing walkthrough directs readers to that tab, so it carries a note telling them to refresh and visually confirm the granted scopes. diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index d32b6de2..2971f7ed 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -30,6 +30,12 @@ const AccessDeniedErrorCode = "E0000006" const oktaURLScheme = "https" +// Scopes requested only when the matching feature is enabled. +const ( + apiTokensReadScope = "okta.apiTokens.read" //nolint:gosec // not a credential; an OAuth 2.0 scope name + devicesReadScope = "okta.devices.read" +) + // oktaSDKAuthSentinel activates the SDK's Bearer auth path; the oktaauth RoundTripper substitutes the real DPoP/Bearer token per request. const oktaSDKAuthSentinel = "dpop-managed" @@ -140,10 +146,38 @@ var ( "okta.roles.manage", "okta.apps.manage", } + // The event feed reads Okta's System Log (GET /api/v1/logs). Requested + // unconditionally on the OAuth path because EventFeeds is always advertised + // and C1 decides when to call ListEvents -- there is no local opt-in to gate + // on the way device sync has one. + eventFeedScopes = []string{ + "okta.logs.read", + } // TODO (santhosh) Add required scopes for secrets sync ) +// privateKeyScopes is every scope the OAuth path asks for at token exchange. Okta +// issues a token carrying only the requested scopes the app has also granted, so a +// scope the connector never requests is unreachable however the customer configures +// the app -- which is what left the event feed failing on every run without +// okta.logs.read here. Listing one costs nothing: an ungranted scope drops from the +// issued token silently and only surfaces as a 403 on first use. +func privateKeyScopes(cc *cfg.Okta, opts *cli.ConnectorOpts) []string { + scopes := make([]string, 0, len(defaultScopes)+len(provisioningScopes)+len(eventFeedScopes)+2) + scopes = append(scopes, defaultScopes...) + scopes = append(scopes, provisioningScopes...) + scopes = append(scopes, eventFeedScopes...) + + if cc.SyncSecrets { + scopes = append(scopes, apiTokensReadScope) + } + if shouldSyncResourceType(opts, resourceTypeDevice.Id) { + scopes = append(scopes, devicesReadScope) + } + return scopes +} + // nil opts means this is capabilities/metadata generation, not a real sync. func shouldSyncResourceType(opts *cli.ConnectorOpts, resourceTypeID string) bool { if opts == nil { @@ -384,10 +418,7 @@ func parseOktaOrgURL(raw string) (*url.URL, error) { } func New(ctx context.Context, cc *cfg.Okta, opts *cli.ConnectorOpts) (connectorbuilder.ConnectorBuilderV2, []connectorbuilder.Opt, error) { - var ( - oktaClient *okta.Client - scopes = defaultScopes - ) + var oktaClient *okta.Client orgURL, err := parseOktaOrgURL(cc.Domain) if err != nil { @@ -448,16 +479,7 @@ func New(ctx context.Context, cc *cfg.Okta, opts *cli.ConnectorOpts) (connectorb } oktaClientV5 = oktav5.NewAPIClient(config) case cfg.PrivateKeyGroup: - scopes = append(scopes, provisioningScopes...) - - if cc.SyncSecrets { - scopes = append(scopes, "okta.apiTokens.read") - } - - // An ungranted scope silently drops from the token and only surfaces as a 403 on first use. - if shouldSyncResourceType(opts, resourceTypeDevice.Id) { - scopes = append(scopes, "okta.devices.read") - } + scopes := privateKeyScopes(cc, opts) dpopClient, err := oktaauth.NewDPoPHTTPClient(ctx, oktaauth.Config{ Domain: domain, diff --git a/pkg/connector/scopes_test.go b/pkg/connector/scopes_test.go new file mode 100644 index 00000000..3d4d06a6 --- /dev/null +++ b/pkg/connector/scopes_test.go @@ -0,0 +1,110 @@ +package connector + +import ( + "slices" + "testing" + + cfg "github.com/conductorone/baton-okta/pkg/config" + "github.com/conductorone/baton-sdk/pkg/cli" +) + +const logsReadScope = "okta.logs.read" + +// Okta issues a token carrying only the requested scopes the app has also granted, +// so dropping okta.logs.read from this list makes the event feed fail on every run +// no matter what the customer grants in Okta. Verified against a live tenant: with +// the scope granted but not requested, GET /api/v1/logs returns 403. +func TestPrivateKeyScopes_AlwaysRequestsSystemLogRead(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + cc *cfg.Okta + opts *cli.ConnectorOpts + }{ + {name: "defaults", cc: &cfg.Okta{}, opts: nil}, + {name: "secrets sync on", cc: &cfg.Okta{SyncSecrets: true}, opts: nil}, + { + name: "explicit sync filter excluding devices", + cc: &cfg.Okta{}, + opts: &cli.ConnectorOpts{SyncResourceTypeIDs: []string{resourceTypeUser.Id}}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := privateKeyScopes(tc.cc, tc.opts); !slices.Contains(got, logsReadScope) { + t.Errorf("scopes = %v, want %q present", got, logsReadScope) + } + }) + } +} + +func TestPrivateKeyScopes_ConditionalScopes(t *testing.T) { + t.Parallel() + + base := privateKeyScopes(&cfg.Okta{}, nil) + if slices.Contains(base, apiTokensReadScope) { + t.Errorf("%q requested without --sync-secrets", apiTokensReadScope) + } + if !slices.Contains(base, devicesReadScope) { + t.Errorf("%q missing when no sync filter narrows it out", devicesReadScope) + } + + withSecrets := privateKeyScopes(&cfg.Okta{SyncSecrets: true}, nil) + if !slices.Contains(withSecrets, apiTokensReadScope) { + t.Errorf("%q missing with --sync-secrets", apiTokensReadScope) + } + + // A filter that names only users must not pull in the device scope. + narrowed := privateKeyScopes(&cfg.Okta{}, &cli.ConnectorOpts{ + SyncResourceTypeIDs: []string{resourceTypeUser.Id}, + }) + if slices.Contains(narrowed, devicesReadScope) { + t.Errorf("%q requested although device sync is filtered out", devicesReadScope) + } +} + +// Every read and manage scope the sync itself depends on has to stay listed. +func TestPrivateKeyScopes_CoversReadAndManage(t *testing.T) { + t.Parallel() + + got := privateKeyScopes(&cfg.Okta{}, nil) + for _, want := range slices.Concat(defaultScopes, provisioningScopes, eventFeedScopes) { + if !slices.Contains(got, want) { + t.Errorf("scopes = %v, want %q present", got, want) + } + } + // Okta rejects nothing for a duplicate, but a repeat means the assembly is + // appending the same source twice. + seen := map[string]bool{} + for _, s := range got { + if seen[s] { + t.Errorf("scope %q requested twice: %v", s, got) + } + seen[s] = true + } +} + +// The assembly used to start from `scopes = defaultScopes` and append, which only +// avoided mutating the package-level slice because its capacity happened to be +// exhausted. Pin that the sources are untouched. +func TestPrivateKeyScopes_DoesNotMutatePackageSlices(t *testing.T) { + t.Parallel() + + defaultsBefore := slices.Clone(defaultScopes) + provisioningBefore := slices.Clone(provisioningScopes) + eventFeedBefore := slices.Clone(eventFeedScopes) + + _ = privateKeyScopes(&cfg.Okta{SyncSecrets: true}, nil) + _ = privateKeyScopes(&cfg.Okta{}, nil) + + if !slices.Equal(defaultScopes, defaultsBefore) { + t.Errorf("defaultScopes mutated: %v, want %v", defaultScopes, defaultsBefore) + } + if !slices.Equal(provisioningScopes, provisioningBefore) { + t.Errorf("provisioningScopes mutated: %v, want %v", provisioningScopes, provisioningBefore) + } + if !slices.Equal(eventFeedScopes, eventFeedBefore) { + t.Errorf("eventFeedScopes mutated: %v, want %v", eventFeedScopes, eventFeedBefore) + } +}