feat: add OAuth2 provider support to identity - #1856
Conversation
github-actions
Bot
added
the
agentcore-harness-reviewing
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #1856 +/- ##
============================================
- Coverage 95.26% 94.86% -0.41%
============================================
Files 176 182 +6
Lines 8202 8585 +383
============================================
+ Hits 7814 8144 +330
- Misses 388 441 +53 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
github-actions
Bot
removed
the
agentcore-harness-reviewing
| flag( | ||
| "provider-configuration", | ||
| "complete OAuth2 provider configuration JSON (alternative to guided flags)", | ||
| z.string().optional(), |
There was a problem hiding this comment.
I think --provider-configuration also needs to be marked sensitive in both create and update. This flag accepts the complete SDK config, which can include clientSecret. I tested that form and the plaintext value was written to ~/.agentcore/logs while --client-secret was redacted. Marking both flag definitions sensitive should cover it.
| try { | ||
| parsed = JSON.parse(raw); | ||
| } catch (error) { | ||
| throw new TypeError( |
There was a problem hiding this comment.
This shared parser previously threw InputValidationError, but the move changes it to TypeError. The new OAuth handlers do the same. I reproduced a conflicting-secret invocation and the log classified it as source: "internal". These user-input failures should remain InputValidationError.
| vendorConfig.clientSecretConfig = clientSecretConfig; | ||
| vendorConfig.clientSecretSource = clientSecretSource; | ||
| oauth2ProviderConfigInput = config; | ||
| } else { |
There was a problem hiding this comment.
The two configuration modes are not fully enforced here. Without --provider-configuration, this always builds customOauth2ProviderConfig, even for --vendor GithubOauth2, and guided Custom OAuth also allows the required discovery configuration to be omitted. I reproduced the resulting service validation error. We should require provider configuration for non-Custom vendors and one discovery form for the guided Custom path, including the equivalent update validation.
| ), | ||
| flag( | ||
| "client-secret-reference", | ||
| 'external secret reference JSON: {"secretId":"<arn>","jsonKey":"<key>"}', |
There was a problem hiding this comment.
does this need to be sensitive or is it strictly pointers to secrets?
| ], | ||
| handle: async (ctx, flags) => { | ||
| if (!flags.name) { | ||
| throw new TypeError("required option '--name <name>' not specified"); |
There was a problem hiding this comment.
should we throw modeled input validation errors here? (same with below)
| "--provider-configuration and guided flags (--client-id, --discovery-url, --authorization-server-metadata) are mutually exclusive", | ||
| ); | ||
| } | ||
| if (flags["discovery-url"] && flags["authorization-server-metadata"]) { |
There was a problem hiding this comment.
same question for errors here.
| throw new TypeError(`--${flagName} must be a JSON object with "secretId" and "jsonKey"`); | ||
| } | ||
|
|
||
| const obj = parsed as Record<string, unknown>; |
There was a problem hiding this comment.
could we simplify this with a strict zod schema? Something like:
const secretReferenceSchema = z
.object({
secretId: z.string().min(1),
jsonKey: z.string().min(1),
})
.strict();
4 participants
Description
Adds command-line CRUDL for Identity OAuth2 credential providers:
identity oauth2-credential-provider createidentity oauth2-credential-provider getidentity oauth2-credential-provider listidentity oauth2-credential-provider updateidentity oauth2-credential-provider deleteSupports two configuration paths:
--client-id,--discovery-url, and--authorization-server-metadatabuild the provider config from scalar flags. Only valid with--vendor CustomOauth2--provider-configuration <json>accepts the complete SDK config union for any of the supported vendors. The CLI merges the resolved secret into the vendor config before sendingSecret handling follows the same pattern established with api-key-credential-provider:
--client-secretis source-aware (inline, file://path, or - for stdin) and marked sensitive for log redaction--client-secret-reference '{"secretId":"...","jsonKey":"..."}'for external Secrets Manager secretsThis PR also generalizes
parseSecretReferenceto a shared identity utility (src/handlers/identity/parser.tsx) parameterized by flag name.Related Issue
Closes #
Documentation PR
Type of Change
Testing
Manual tested cli by invoking all commands, both happy-path and negative validation. Expected results across the board. Additionally ran the full test suite, against latest HEAD after rebasing refactor branch:
How have you tested the change?
npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.