feat: add OAuth credential provider creation during deploy

[aidandaly24]

Description

Implement OAuth credential provider creation during the deploy pipeline, running before CDK synth so credential ARNs are available in deployed-state.json for CDK to reference.

• OAuth2 credential operations (oauth2-credential-provider.ts): Create/Get/Update operations for OAuth2 credential providers via AgentCore Identity APIs, with defensive ARN field extraction handling API response inconsistencies (credentialProviderArn vs oAuth2CredentialProviderArn)
• Deploy pipeline integration: setupOAuth2Providers() reads client credentials from .env.local (AGENTCORE_CREDENTIAL_{NAME}_CLIENT_ID/CLIENT_SECRET), creates/updates providers, and handles "already exists" conflicts by falling back to GET
• State persistence: Credential ARNs and callbackUrl written to deployed-state.json via buildDeployedState()
• Deployed state schema: Added CredentialDeployedStateSchema with credentialProviderArn, clientSecretArn, and callbackUrl fields

Note: TUI deploy flow (useDeployFlow.ts) does not yet include OAuth credential setup — tracked as a follow-up task.

Related Issue

Closes #

Documentation PR

N/A

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Other (please describe):

Testing

How have you tested the change?

• I ran npm run test:unit and npm run test:integ
• I ran npm run typecheck
• I ran npm run lint
• If I modified src/assets/, I ran npm run test:update-snapshots and committed the updated snapshots

Checklist

• I have read the CONTRIBUTING document
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[notgitika]

/strands review

[github-actions]

Code Review Summary

I've reviewed PR #407 which adds OAuth credential provider creation during the deploy pipeline. The implementation follows good patterns from the existing API key credential provider code, but there are some blocking issues that need to be addressed before this can be merged.

---

⛔ Blocking Issues

1. Missing Unit Tests for oauth2-credential-provider.ts

Location: src/cli/operations/identity/oauth2-credential-provider.ts (160 lines, no tests)

Issue: The new OAuth2 credential provider module has no unit tests, while the similar api-key-credential-provider.ts has comprehensive test coverage at src/cli/operations/identity/__tests__/api-key-credential-provider.test.ts.

Required tests:

• oAuth2ProviderExists() - should handle exists, not found, and error cases
• createOAuth2Provider() - should test success, conflict fallback to GET, and error cases
• getOAuth2Provider() - should test success and error cases
• updateOAuth2Provider() - should test success and error cases
• extractArn() - should handle both credentialProviderArn and oAuth2CredentialProviderArn field names
• extractResult() - should handle response parsing and missing ARN case

Reference: See api-key-credential-provider.test.ts for the testing pattern to follow.

---

2. Missing Tests for setupOAuth2Providers

Location: src/cli/operations/deploy/pre-deploy-identity.ts lines 276-372

Issue: The setupOAuth2Providers() function and hasOwnedIdentityOAuthProviders() helper have no test coverage. The file src/cli/operations/deploy/__tests__/pre-deploy-identity.test.ts exists but doesn't cover the new OAuth functionality.

Required tests:

• setupOAuth2Providers() - success case with valid credentials
• setupOAuth2Providers() - skipped case with missing env vars
• setupOAuth2Providers() - error handling for API failures
• setupOAuth2Providers() - merge of runtime and env credentials
• hasOwnedIdentityOAuthProviders() - true/false cases
• setupSingleOAuth2Provider() - create vs update logic

---

3. Type Safety Issue in extractResult

Location: src/cli/operations/identity/oauth2-credential-provider.ts line 49

const clientSecretArn = response.clientSecretArn as { secretArn?: string } | undefined;
return {
  credentialProviderArn,
  clientSecretArn: clientSecretArn?.secretArn,  // ⚠️ Unsafe access
  callbackUrl: response.callbackUrl as string | undefined,
};

Issue: The type cast assumes clientSecretArn is an object with a secretArn property, but if the API returns it as a string or other type, this will cause a runtime error or silently return undefined.

Fix: Add a type guard:

function extractResult(response: Record<string, unknown>): OAuth2ProviderResult | undefined {
  const credentialProviderArn = extractArn(response);
  if (!credentialProviderArn) return undefined;

  const clientSecretArnRaw = response.clientSecretArn;
  const clientSecretArn = 
    clientSecretArnRaw && typeof clientSecretArnRaw === 'object' && 'secretArn' in clientSecretArnRaw
      ? (clientSecretArnRaw as { secretArn?: string }).secretArn
      : undefined;
      
  return {
    credentialProviderArn,
    clientSecretArn,
    callbackUrl: response.callbackUrl as string | undefined,
  };
}

---

💡 Suggestions

4. Code Duplication Between OAuth and API Key Setup

Location: src/cli/operations/deploy/pre-deploy-identity.ts

Observation: setupOAuth2Providers() (lines 276-298) and setupApiKeyProviders() (lines 45-89) share a very similar structure:

1. Read env file
2. Merge with runtime credentials
3. Create client
4. Loop through credentials
5. Return results with error flag

Suggestion: Consider extracting common logic into a shared helper:

async function setupCredentialProviders<TResult>(
  options: {
    projectSpec: AgentCoreProjectSpec;
    configBaseDir: string;
    region: string;
    runtimeCredentials?: SecureCredentials;
  },
  filter: (c: Credential) => boolean,
  setupFn: (client: BedrockAgentCoreControlClient, credential: Credential, creds: SecureCredentials) => Promise<TResult>
): Promise<{ results: TResult[]; hasErrors: boolean }>

This would make it easier to add future credential types and reduce maintenance burden.

---

5. Inconsistent Error Handling Between Create Functions

Location: oauth2-credential-provider.ts vs api-key-credential-provider.ts

Observation:

• createOAuth2Provider() on conflict: Falls back to getOAuth2Provider() to retrieve provider data
• createApiKeyProvider() on conflict: Returns { success: true } immediately (no data)

Question: Why does OAuth need to fetch the ARN back on conflict while API key doesn't? Is this because the ARN is needed in deployed-state.json?

Suggestion: Add a comment explaining this difference, or unify the behavior if possible. The current difference might confuse future maintainers.

---

6. Missing Validation for OAuth Credential Fields

Location: src/cli/operations/deploy/pre-deploy-identity.ts lines 331-337

const params = {
  name: credential.name,
  vendor: credential.vendor,          // ⚠️ Not validated
  discoveryUrl: credential.discoveryUrl,  // ⚠️ Not validated
  clientId,
  clientSecret,
};

Issue: While the schema validates these fields when the credential is added, there's no runtime validation before passing to the AWS SDK.

Suggestion: Add validation or at least a type assertion:

const params: OAuth2ProviderParams = {
  name: credential.name,
  vendor: credential.vendor,  // Type system ensures this is string from schema
  discoveryUrl: credential.discoveryUrl,
  clientId,
  clientSecret,
};

Consider validating that vendor is a valid CredentialProviderVendorType at runtime if the AWS SDK provides an enum.

---

❓ Questions

7. TUI Deploy Flow Not Updated

From PR Description:

Note: TUI deploy flow (useDeployFlow.ts) does not yet include OAuth credential setup — tracked as a follow-up task.

Question: Is it acceptable to merge this without TUI support? This means:

• agentcore deploy (CLI) will support OAuth credential setup ✅
• agentcore (TUI) → Deploy will NOT support OAuth credential setup ❌

Impact: Users who rely on the TUI will not be able to deploy projects with OAuth credentials until the follow-up is completed.

Recommendation: Either:

1. Update TUI in this PR to maintain feature parity, OR
2. Add a clear warning in the TUI when OAuth credentials are detected, OR
3. Document this limitation in the PR and ensure the follow-up task is tracked

---

8. Missing Related Issue Number

Location: PR description

Issue: The "Related Issue" section shows "Closes #" with no issue number.

Per CONTRIBUTING.md line 25:

Every PR must be linked to an issue. Open an issue first (or find an existing one) and reference it in your PR using Closes #issue-number.

Question: What issue does this PR close? Please update the PR description.

---

🎨 Nits

9. Naming Inconsistency

Observation:

• Function names: oAuth2ProviderExists, setupOAuth2Providers (mixed case)
• AWS SDK: CreateOauth2CredentialProviderCommand (camelCase)
• Type names: OAuth2ProviderResult, OAuth2ProviderParams (PascalCase)

Suggestion: For consistency:

• Types and exports: OAuth2 (PascalCase with uppercase OAuth)
• Internal functions: oauth2 (camelCase)
• This aligns with TypeScript naming conventions

---

✅ Positive Observations

• Defensive programming: The extractArn() function handles API response field inconsistencies well
• Error handling: Good use of isNoCredentialsError() for clear error messages
• Documentation: JSDoc comments clearly explain why this code exists (Identity not yet modeled as CDK constructs)
• Consistency: Follows patterns from existing api-key-credential-provider.ts
• Schema design: CredentialDeployedStateSchema is clean and extensible
• Export organization: Proper updates to index files for clean API surface

---

Recommendation

Do not merge until:

1. ✅ Unit tests added for oauth2-credential-provider.ts
2. ✅ Tests added for setupOAuth2Providers and related functions
3. ✅ Type safety issue fixed in extractResult
4. ✅ Related issue number added to PR description

Once blocking issues are resolved, the suggestions and questions can be addressed in follow-up PRs if preferred.

[github-actions]

Additional Finding: OAuth Credentials Not Read from process.env

🐛 Bug: Runtime OAuth Credentials Not Supported

Location: src/cli/commands/deploy/actions.ts lines 172-182

Issue: The code reads runtime credentials from process.env for non-interactive deploy (with -y flag), but it only collects API key credentials:

// Read runtime credentials from process.env (enables non-interactive deploy with -y)
const neededCredentials = getAllCredentials(context.projectSpec);  // ⚠️ Only returns API key creds
const envCredentials: Record<string, string> = {};
for (const cred of neededCredentials) {
  const value = process.env[cred.envVarName];
  if (value) {
    envCredentials[cred.envVarName] = value;
  }
}
const runtimeCredentials =
  Object.keys(envCredentials).length > 0 ? new SecureCredentials(envCredentials) : undefined;

Root cause: getAllCredentials() in pre-deploy-identity.ts line 232 only checks for ApiKeyCredentialProvider:

export function getAllCredentials(projectSpec: AgentCoreProjectSpec): MissingCredential[] {
  const credentials: MissingCredential[] = [];

  for (const credential of projectSpec.credentials) {
    if (credential.type === 'ApiKeyCredentialProvider') {  // ⚠️ Doesn't handle OAuth
      credentials.push({
        providerName: credential.name,
        envVarName: computeDefaultCredentialEnvVarName(credential.name),
      });
    }
  }

  return credentials;
}

Impact:

• OAuth credentials from process.env are NOT passed to setupOAuth2Providers as runtimeCredentials
• Non-interactive deploy with -y flag will fail for OAuth credentials if they're only in environment variables
• Works if credentials are in .env.local, but not for CI/CD scenarios where credentials come from environment

Fix: Update getAllCredentials to also collect OAuth credentials:

export function getAllCredentials(projectSpec: AgentCoreProjectSpec): MissingCredential[] {
  const credentials: MissingCredential[] = [];

  for (const credential of projectSpec.credentials) {
    if (credential.type === 'ApiKeyCredentialProvider') {
      credentials.push({
        providerName: credential.name,
        envVarName: computeDefaultCredentialEnvVarName(credential.name),
      });
    } else if (credential.type === 'OAuthCredentialProvider') {
      const nameKey = credential.name.toUpperCase().replace(/-/g, '_');
      credentials.push(
        {
          providerName: credential.name,
          envVarName: `AGENTCORE_CREDENTIAL_${nameKey}_CLIENT_ID`,
        },
        {
          providerName: credential.name,
          envVarName: `AGENTCORE_CREDENTIAL_${nameKey}_CLIENT_SECRET`,
        }
      );
    }
  }

  return credentials;
}

Alternatively, you might want to create a separate function getAllOAuthCredentials() and call it separately if the logic becomes too complex.

---

This should be considered blocking as it breaks non-interactive OAuth credential deployments.

[github-actions]

Review Complete ✅

I've finished reviewing PR #407. Here's the summary:

📊 Review Stats

• Files changed: 7
• Lines added: ~290
• Blocking issues: 4
• Suggestions: 3
• Questions: 2
• Nits: 1

🚫 Blocking Issues Summary

1. Missing unit tests for oauth2-credential-provider.ts (160 lines, 0 tests)
2. Missing tests for setupOAuth2Providers() and related functions
3. Type safety issue in extractResult() - unsafe object property access
4. OAuth credentials not read from process.env - breaks non-interactive deploy with -y flag

⏭️ Next Steps

Before this PR can be merged:

1. Add unit tests for all new functions in oauth2-credential-provider.ts
2. Add tests for OAuth setup functions in pre-deploy-identity.test.ts
3. Fix type safety issue in extractResult() with proper type guard
4. Update getAllCredentials() to include OAuth CLIENT_ID and CLIENT_SECRET env vars
5. Add related issue number to PR description (required by CONTRIBUTING.md)

The suggestions and questions can be addressed in follow-up PRs if needed, but the blocking issues should be resolved before merge.

💚 What's Good

The implementation shows solid engineering:

• Follows existing patterns consistently
• Good defensive programming (ARN field extraction)
• Clear documentation explaining architectural decisions
• Proper error handling throughout
• Clean schema design

Once the blocking issues are addressed, this will be a solid addition to the codebase! 🚀

[notgitika]

is ! needed? we just checked for it above.

[aidandaly24]

The ! is required — resources is typed as optional in TargetDeployedState even though we initialize it in the literal above. TypeScript can't narrow it.

[notgitika]

Client is created per-call to setupOAuth2Providers but not shared with setupApiKeyProviders.

Both functions create their own BedrockAgentCoreControlClient. If credentials or region config diverge between the two calls, you'd get inconsistent behavior.

This adds another duplicated client instance. We have an existing issue: #342

Maybe this can be addressed in a diff PR

[aidandaly24]

Agreed, the duplicate client is a known issue tracked in #342. Happy to address here if you'd prefer, but keeping it consistent with the existing setupApiKeyProviders pattern for now.

[notgitika]

The vendor field is cast to CredentialProviderVendorType, which suggests there are known vendor presets. But the config always builds a customOauth2ProviderConfig regardless of the vendor value. If a vendor like "Google" or "Microsoft" has a dedicated config path in the API, this would send it through the wrong config shape. Is this intentional?

[aidandaly24]

Good catch. The API does have vendor-specific config paths (googleOauth2ProviderConfig, githubOauth2ProviderConfig, etc.) with simpler shapes (just clientId/clientSecret, no discovery URL). Our schema defaults vendor to 'CustomOauth2' and the CLI doesn't expose vendor selection yet, so this works for Phase 1. When we add vendor selection in a future task, we'll need to route to the correct config path based on vendor. Added a comment noting this.

[notgitika]

When createOAuth2Provider catches a ConflictException, it falls back to getOAuth2Provider returning the existing provider without updating it.

But the caller in setupSingleOAuth2Provider (pre-deploy-identity.ts:340) already checks existence and routes to update. So this fallback only triggers in a race condition where another process created the provider between the exists-check and the create call. In that case, the user's new clientId/clientSecret would be silently ignored. Should this fall back to updateOAuth2Provider instead?

[notgitika]

Casting to Record<string, unknown> everywhere bypasses the SDK types — if the field name changes again we won't get a compile error, it'll just silently return undefined. Can we at least add a comment noting which SDK version has the credentialProviderArn vs oAuth2CredentialProviderArn inconsistency? That way someone can clean this up once the API stabilizes instead of it living forever.

[notgitika]

LGTM thank you!