From 19b6618dcef423a0a8dc2f4f34c6ca547e54de88 Mon Sep 17 00:00:00 2001 From: agustin-conductor Date: Thu, 27 Aug 2026 15:29:16 -0300 Subject: [PATCH] fix: request okta.logs.read so event feeds work on OAuth auth (CXP-1021) Event feeds never worked on the OAuth 2.0 / private-key path. The feed reads Okta's System Log (GET /api/v1/logs), which requires okta.logs.read, and that scope was requested nowhere in the connector -- it appeared in no scope list, neither unconditionally nor behind a flag. Every list_events task failed with "listing events failed: the API returned an unknown error". Only the private-key path is affected, and there it is a total failure of the capability rather than a degradation. API-token auth sends no scope list at all: the token inherits the creating admin's role, so the feed works there whenever that admin can read the System Log. Okta issues a token carrying the intersection of the requested and granted scopes, so a scope the connector does not request is unreachable however the customer configures the app -- granting okta.logs.read in Okta did nothing on its own. Verified against a developer org with the scope granted, reading the scp claim out of the access token: requested without okta.logs.read -> scp lacks it -> GET /api/v1/logs 403 requested with okta.logs.read -> scp has it -> GET /api/v1/logs 200 Adding a scope cannot break a tenant that has not granted it. Ungranted scopes are dropped from the issued token silently, leaving the granted set and every sync call untouched; the exchange fails only when nothing requested is granted, which cannot happen here because the eight core read and manage scopes are what make the connector work at all. Confirmed by requesting three ungranted scopes alongside: scp unchanged, /api/v1/logs and /api/v1/users both 200. Also extracts the scope assembly into privateKeyScopes so it is testable, which removes a latent aliasing hazard: the old code seeded `scopes = defaultScopes` and appended, and only avoided mutating the package-level slice because its capacity happened to be exhausted. Validated end to end with an on-demand event feed run: real usage events stream where the same binary previously failed on every attempt. Co-Authored-By: Claude Opus 5 (1M context) --- docs/connector.mdx | 1 + docs/docs-info.md | 3 +- pkg/connector/connector.go | 50 +++++++++++----- pkg/connector/scopes_test.go | 110 +++++++++++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 15 deletions(-) create mode 100644 pkg/connector/scopes_test.go diff --git a/docs/connector.mdx b/docs/connector.mdx index a5278bbd4..459bc1705 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 2ab739584..eab4e92aa 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 d32b6de2c..2971f7edf 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 000000000..3d4d06a6b --- /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) + } +}