APIGOV-31447 - Add end-to-end Okta provisioning support for group/policy validation and assignment

[sbolosan]

New variables

Okta group assignment (optional)

AGENTFEATURES_IDP_OKTA_GROUP_1="Marketplace"

Okta policy lookup (optional)

AGENTFEATURES_IDP_OKTA_POLICY_1="shane"

• Introduces a dedicated Okta management client layer and tests for group, app, and policy operations.
• Extends OAuth provider flow to support provider-specific post-registration and post-unregister hooks, including passing/storing provider agent details.
• Implements Okta post-processing to assign apps to configured groups and add apps to configured policies, with explicit errors when configured resources are missing.
• Adds fail-fast validation during provider initialization for configured Okta resources so invalid setup is caught before runtime provisioning.
• Updates provisioning/deprovisioning logic to persist and merge agent details, then reuse them during cleanup/unregister paths.
• Expands external IDP config to include Okta-specific settings (group/policy), with corresponding parsing and unit tests.
• Updates API service revision/provisioning integration paths to carry provider details through the lifecycle.
• Refreshes discovery/provisioning documentation and substantially increases test coverage across provider, Okta provider, client, and config behaviors.
• Logging-related branch changes move operation start/completion and retry-attempt messages to clearer levels (DEBUG/INFO/TRACE) for better verbosity control.

[jcollins-axway]

I know this is in draft, however, here is copilots review.

Prompt: "in the sdk complete a code review of changes in origin/APIGOV-31447 to its base of origin/main"

Code Review: origin/APIGOV-31447 vs origin/main

Summary

• Scope reviewed: 12 changed files (Okta provisioning flow, IDP config, docs, tests).
• Verdict: Changes Requested
• Severity: 1 Blocker, 1 Major

Findings

Blocker: Possible orphaned OAuth clients on post-registration failure

• In pkg/authz/oauth/provider.go, client registration succeeds first, then Okta post-processing runs.
• If post-processing fails, the function returns immediately without compensating deletion of the just-created client.
• Impact: retries can create orphaned/duplicate clients and drift state between Central and IdP.

Major: Unregister path can fail before deleting the client

• In pkg/authz/oauth/provider.go, Okta cleanup runs before OAuth client unregister.
• If cleanup fails, unregister exits early and does not attempt client deletion.
• Impact: transient Okta API issues can leave active credentials unintentionally.

Positive Notes

• Okta config additions are consistently wired through config, tests, and docs:
  • pkg/config/externalidpconfig.go
  • pkg/config/externalidpconfig_test.go
  • docs/discovery/provisioning.md
• New Okta management client status handling has targeted tests:
  • pkg/authz/oauth/clients/okta_test.go

Recommendation

• Add best-effort rollback when post-registration hook fails.
• In unregister flow, continue with client deletion even if pre-cleanup fails, then return aggregated error details.

[sbolosan]

Based on Mr.Bots assessment, registration is now compensating on hook failure, and unregister is now doing cleanup failures while preserving full error context.

[jcollins-axway]

Mr. Bot's latest

Code Review: origin/APIGOV-31447 vs origin/main

Summary

• Scope: 13 files reviewed (11 modified, 2 added), focused on Okta provisioning enhancements (group assignment, policy lookup/assignment), provider hook extensions, and IDP config/docs updates.
• Verdict: request changes
• Severity: medium (2), low (1)

Findings

• Medium – Provider interface breaking change

  • Where: pkg/authz/oauth/provider.go, pkg/authz/oauth/provider.go, pkg/apic/provisioning/idp/provisioner.go
  • Description: Provider.UnregisterClient now requires an additional agentDetails map[string]string argument. This changes a public interface contract.
  • Possible impacts to the codebase: downstream implementations/usages of Provider outside this repo can break at compile time unless updated.

• Medium – Cleanup can silently skip group unassignment

  • Where: pkg/authz/oauth/oktaprovider.go
  • Description: handleGroupUnassignment no-ops when oktaGroupId is absent in agentDetails with no warning/error.
  • Possible impacts to the codebase: orphaned Okta app-group assignments may remain when metadata persistence is incomplete or missing.

• Low – Manual cleanup path may be easy to miss operationally

  • Where: pkg/authz/oauth/provider.go
  • Description: on post-registration hook failure, rollback is attempted and error text indicates manual cleanup may be required, but this path may not be prominent enough for operations.
  • Possible impacts to the codebase: increased MTTR if operators miss partial-provisioning failure context.

Positives

• Good fail-fast validation for configured Okta group/policy at provider initialization.
• Solid rollback attempt on post-registration hook failure to avoid orphaned clients.
• Clear idempotent HTTP handling in Okta client (409 assign and 404 unassign treated as non-fatal).
• Clean provider extensibility via typed IDP hooks (pre/post processing).
• Good test additions around status handling and Okta-specific flows.
• Documentation updated for new Okta config fields and behavior expectations.

Recommendations

• Document the Provider interface signature change explicitly in upgrade notes/changelog for downstream consumers.
• Add trace/warn logging when oktaGroupId is missing during unregister cleanup to aid diagnostics.
• Add/verify a test that covers unregister flow with missing oktaGroupId to lock intended behavior.
• Consider making the manual cleanup error path more operationally visible (structured fields or stronger log signal).
• Keep docs explicit that SDK does not create Okta groups/policies and startup will fail if configured resources don’t exist.

[sbolosan]

Addressed the three review findings in this update based on Mr.Bots second round of findings:

• Reverted the Provider contract back to non-breaking unregister signature (no extra agentDetails arg).

• Removed silent cleanup skip by returning an explicit error when a configured Okta group cannot be resolved during unregister cleanup.

• Improved operational visibility of rollback failure by adding a structured warning signal before returning the manual-cleanup error.