-
-
Notifications
You must be signed in to change notification settings - Fork 140
docs: Add PRD for browser-based auth in aws/user identity #1887
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
docs: Add PRD for browser-based auth in aws/user identity #1887
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1887 +/- ##
==========================================
+ Coverage 73.99% 74.03% +0.03%
==========================================
Files 769 769
Lines 69288 69288
==========================================
+ Hits 51273 51297 +24
+ Misses 14604 14581 -23
+ Partials 3411 3410 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughAdds a new PRD documenting a browser-based OAuth2 (PKCE) fallback for the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/prd/aws-browser-auth.md (1)
165-165: Defer fenced code block language tags to documentation cleanup.Static analysis flagged three fenced code blocks missing language specifications (MD040) at lines 165, 189, and 245. These are valid but low-priority style issues. Per established learnings, markdownlint formatting issues should be addressed in a separate documentation cleanup commit to avoid blocking this PR.
Also applies to: 189-189, 245-245
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/prd/aws-browser-auth.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/prd/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
All Product Requirement Documents (PRDs) MUST be placed in docs/prd/ with kebab-case filenames
Files:
docs/prd/aws-browser-auth.md
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.614Z
Learning: Applies to docs/prd/**/*.md : All Product Requirement Documents (PRDs) MUST be placed in docs/prd/ with kebab-case filenames
📚 Learning: 2025-11-10T20:03:56.875Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1775
File: pkg/auth/providers/aws/sso_provisioning.go:40-79
Timestamp: 2025-11-10T20:03:56.875Z
Learning: In the Atmos AWS SSO provider (pkg/auth/providers/aws/sso_provisioning.go), the OAuth access token from the AWS SSO device flow is intentionally stored in the `AccessKeyID` field of `AWSCredentials` during authentication. This token is then extracted and used for ListAccounts and ListAccountRoles API calls during identity provisioning. This design reuses the existing `AWSCredentials` type for token transport rather than creating a separate credential type.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-04-25T20:54:19.701Z
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-12-13T06:07:34.794Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:34.794Z
Learning: For docs in the cloudposse/atmos repository under docs/prd/, markdownlint issues MD040, MD010, and MD034 should be deferred to a separate documentation cleanup commit and must not block the current PR. If needed, address these issues in a follow-up PR dedicated to documentation improvements.
Applied to files:
docs/prd/aws-browser-auth.md
🪛 LanguageTool
docs/prd/aws-browser-auth.md
[grammar] ~255-~255: Ensure spelling is correct
Context: ...ack 6. Exchange code for tokens via AWS signin service 7. Use tokens to obtain tempora...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/aws-browser-auth.md
165-165: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
189-189: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
245-245: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (1)
docs/prd/aws-browser-auth.md (1)
1-337: Strong alignment with past feedback on SDK-native implementation approach.This PRD comprehensively addresses DEV-3829 objectives. I particularly appreciate that lines 236-272 recommend native AWS SDK integration and explicitly reject the CLI wrapper approach—this directly addresses the prior feedback to integrate with SDK support rather than shell out to the AWS CLI.
The structure is solid: user stories cover the full scope (IAM, root, federated, headless, role chaining), technical details are precise, requirements are well-prioritized, configuration examples are practical, and both interactive and headless UX flows are documented. Security considerations and testing strategy round things out nicely.
|
I’m leaning toward modeling this differently. The problem is Instead, I think we should make the split explicit:
This gives us a cleaner and more accurate model:
Example: IAM User via Web Login (Break-Glass User)auth:
methods:
web: # this is just an arbitrary name
kind: aws/login
region: us-east-1
identities:
breakglass-user:
kind: aws/user
principal_arn: arn:aws:iam::123456789012:user/developer
via:
method: webExample: Root Access via Web Login (Break-Glass) auth:
methods:
console:
kind: aws/login
region: us-east-1
identities:
root-breakglass:
kind: aws/root
account_id: "123456789012"
via:
method: consoleExample: Role Assumption from Break-Glass User auth:
identities:
prod-admin:
kind: aws/assume-role
via:
identity: breakglass-user
principal:
role_arn: arn:aws:iam::123456789012:role/AdminRoleThis keeps aws/login focused on what it actually does — obtaining STS credentials via a browser flow — while making identities explicit and giving us a clear place to add root-specific guardrails and UX later. Happy to bikeshed naming, but I think this separation makes the model both safer and easier to reason about long-term. |
Frankly, I agree. I think this is a login type. My only concern is that we're adding too many different major headers, methods, identities, providers. I realize that's not many right now, but I don't see methods being used a whole lot. Perhaps we could embed the method of AWS login to be the default unless hardcoded credentials are used. Meaning the flow would b. First check if there's an environment block, such as AWS Credentials Key and AWS Secret Access Key. Then, if not, check if the credential store has something set for the user. Finally, if nothing is provided, use the AWS login flow. For root users, I think this is just the default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/prd/aws-browser-auth.md (1)
319-319: Minor: Standardize AWS terminology for "sign-in".Line 319 uses "signin service"; AWS conventionally hyphenates this as "sign-in" (e.g., AWS Sign-In service). Update for consistency: "Exchange code for tokens via AWS sign-in service".
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/prd/aws-browser-auth.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/prd/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
All Product Requirement Documents (PRDs) MUST be placed in docs/prd/ with kebab-case filenames
Files:
docs/prd/aws-browser-auth.md
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T18:20:55.630Z
Learning: Applies to docs/prd/**/*.md : All Product Requirement Documents (PRDs) MUST be placed in docs/prd/ with kebab-case filenames
📚 Learning: 2025-11-10T20:03:56.875Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1775
File: pkg/auth/providers/aws/sso_provisioning.go:40-79
Timestamp: 2025-11-10T20:03:56.875Z
Learning: In the Atmos AWS SSO provider (pkg/auth/providers/aws/sso_provisioning.go), the OAuth access token from the AWS SSO device flow is intentionally stored in the `AccessKeyID` field of `AWSCredentials` during authentication. This token is then extracted and used for ListAccounts and ListAccountRoles API calls during identity provisioning. This design reuses the existing `AWSCredentials` type for token transport rather than creating a separate credential type.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-12-13T06:07:34.794Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:34.794Z
Learning: For docs in the cloudposse/atmos repository under docs/prd/, markdownlint issues MD040, MD010, and MD034 should be deferred to a separate documentation cleanup commit and must not block the current PR. If needed, address these issues in a follow-up PR dedicated to documentation improvements.
Applied to files:
docs/prd/aws-browser-auth.md
🪛 LanguageTool
docs/prd/aws-browser-auth.md
[grammar] ~319-~319: Ensure spelling is correct
Context: ...ack 6. Exchange code for tokens via AWS signin service 7. Return temporary AWS credent...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/aws-browser-auth.md
181-181: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
210-210: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
232-232: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
309-309: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (3)
docs/prd/aws-browser-auth.md (3)
97-97: Clarify requirement FR-4: AWS CLI version validation appears misaligned with native SDK approach.FR-4 specifies validating "AWS CLI version >= 2.32.0" as P0, but NFR-1 (line 107) states "Native SDK implementation (no AWS CLI dependency)" as P0, and the implementation approach (line 326) explicitly avoids external AWS CLI dependency. Why validate AWS CLI version if it's not a dependency?
Either remove FR-4 or clarify its purpose. If it exists for environment checks unrelated to the actual OAuth2 flow, document that clearly.
Also applies to: 107-107
1-392: PRD is well-structured and comprehensive.The document clearly articulates the problem, proposes a focused solution (webflow fallback for existing
aws/useridentity), provides concrete user stories, configuration examples, and a phased rollout plan. It appropriately emphasizes native SDK implementation without AWS CLI dependency and aligns with the PR objectives to integrate webflow as a credential resolution fallback rather than introducing a separate provider type.
181-181: Note: Deferred markdownlint MD040 issues for future documentation cleanup.Several fenced code blocks (lines 181, 210, 232, 309) lack language specifications. Per learnings, markdownlint issues MD040 and related formatting concerns should be deferred to a separate documentation cleanup commit and do not block the current PR. These can be addressed in a follow-up.
Also applies to: 210-210, 232-232, 309-309
Add planned milestone for OAuth2 browser authentication as fallback for aws/user identity (PR #1887) to the auth initiative roadmap. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <[email protected]>
bac89ee to
7d8d582
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/prd/aws-browser-auth.md (1)
64-87: Clarify that this describes the OAuth2 flow we'll implement natively.The section title "AWS Login OAuth2 Flow" and line 68 stating "The
aws logincommand uses OAuth 2.0..." could be misinterpreted as planning to shell out to the AWS CLI. Given NFR-1 explicitly requires "Native SDK implementation (no AWS CLI dependency)", consider rewording to clarify we're implementing this OAuth2 flow ourselves using the AWS SDK.🔎 Suggested clarification
-### AWS Login OAuth2 Flow +### OAuth2 Flow (Native SDK Implementation) -The `aws login` command uses OAuth 2.0 Authorization Code flow with PKCE: +AWS browser-based authentication uses OAuth 2.0 Authorization Code flow with PKCE (we'll implement this natively via AWS SDK, not the AWS CLI):
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/prd/aws-browser-auth.mdwebsite/src/data/roadmap.js
🧰 Additional context used
📓 Path-based instructions (3)
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in thewebsite/directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Keep website code in thewebsite/directory, follow the existing website architecture and style, and test website changes locally before committing
Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Files:
website/src/data/roadmap.js
website/src/data/roadmap.js
📄 CodeRabbit inference engine (CLAUDE.md)
PRs labeled minor/major MUST update website/src/data/roadmap.js with shipped milestones, changelog links, PR numbers, and updated initiative progress percentages
Files:
website/src/data/roadmap.js
docs/prd/**
📄 CodeRabbit inference engine (CLAUDE.md)
All Product Requirement Documents (PRDs) MUST be placed in docs/prd/ with kebab-case filenames
Files:
docs/prd/aws-browser-auth.md
🧠 Learnings (4)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
📚 Learning: 2025-12-26T06:31:02.259Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-26T06:31:02.259Z
Learning: Applies to website/src/data/roadmap.js : PRs labeled minor/major MUST update website/src/data/roadmap.js with shipped milestones, changelog links, PR numbers, and updated initiative progress percentages
Applied to files:
website/src/data/roadmap.js
📚 Learning: 2025-11-10T20:03:56.875Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1775
File: pkg/auth/providers/aws/sso_provisioning.go:40-79
Timestamp: 2025-11-10T20:03:56.875Z
Learning: In the Atmos AWS SSO provider (pkg/auth/providers/aws/sso_provisioning.go), the OAuth access token from the AWS SSO device flow is intentionally stored in the `AccessKeyID` field of `AWSCredentials` during authentication. This token is then extracted and used for ListAccounts and ListAccountRoles API calls during identity provisioning. This design reuses the existing `AWSCredentials` type for token transport rather than creating a separate credential type.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-12-13T06:07:34.794Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:34.794Z
Learning: For docs in the cloudposse/atmos repository under docs/prd/, markdownlint issues MD040, MD010, and MD034 should be deferred to a separate documentation cleanup commit and must not block the current PR. If needed, address these issues in a follow-up PR dedicated to documentation improvements.
Applied to files:
docs/prd/aws-browser-auth.md
🪛 LanguageTool
docs/prd/aws-browser-auth.md
[grammar] ~319-~319: Ensure spelling is correct
Context: ...ack 6. Exchange code for tokens via AWS signin service 7. Return temporary AWS credent...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/aws-browser-auth.md
181-181: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
210-210: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
232-232: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
309-309: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (3)
website/src/data/roadmap.js (1)
134-134: LGTM! Milestone entry correctly added.The new planned milestone for browser-based authentication follows the established pattern and accurately reflects the feature described in the PRD. Status, quarter, PR reference, and benefits are all appropriate.
docs/prd/aws-browser-auth.md (2)
276-327: Implementation approach looks solid.The technical implementation correctly emphasizes native SDK integration with no AWS CLI dependency. The webflow.go approach with OAuth2 PKCE flow, local callback server, and credential caching follows established atmos auth patterns. Good reuse of existing authentication infrastructure.
1-391: Comprehensive PRD with clear requirements and rollout plan.The PRD effectively documents the browser-based authentication feature, including user stories, technical details, security considerations, and testing strategy. The three-tier credential resolution (YAML → Keychain → Browser) is clearly explained, and the fallback approach maintains backward compatibility with existing configurations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/prd/aws-browser-auth.md (1)
180-180: Fenced code blocks should specify language (MD040).Lines 180, 209, 231, and 308 have fenced code blocks without a language identifier. Per established learnings, these markdownlint issues (MD040, MD010, MD034) should be addressed in a separate documentation cleanup commit rather than blocking this PR.
Also applies to: 209-209, 231-231, 308-308
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/prd/aws-browser-auth.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/prd/**
📄 CodeRabbit inference engine (CLAUDE.md)
All Product Requirement Documents (PRDs) MUST be placed in docs/prd/ with kebab-case filenames
Files:
docs/prd/aws-browser-auth.md
🧠 Learnings (4)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
📚 Learning: 2025-11-10T20:03:56.875Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1775
File: pkg/auth/providers/aws/sso_provisioning.go:40-79
Timestamp: 2025-11-10T20:03:56.875Z
Learning: In the Atmos AWS SSO provider (pkg/auth/providers/aws/sso_provisioning.go), the OAuth access token from the AWS SSO device flow is intentionally stored in the `AccessKeyID` field of `AWSCredentials` during authentication. This token is then extracted and used for ListAccounts and ListAccountRoles API calls during identity provisioning. This design reuses the existing `AWSCredentials` type for token transport rather than creating a separate credential type.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-04-25T20:54:19.701Z
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-12-13T06:07:34.794Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:34.794Z
Learning: For docs in the cloudposse/atmos repository under docs/prd/, markdownlint issues MD040, MD010, and MD034 should be deferred to a separate documentation cleanup commit and must not block the current PR. If needed, address these issues in a follow-up PR dedicated to documentation improvements.
Applied to files:
docs/prd/aws-browser-auth.md
🪛 LanguageTool
docs/prd/aws-browser-auth.md
[grammar] ~318-~318: Ensure spelling is correct
Context: ...ack 6. Exchange code for tokens via AWS signin service 7. Return temporary AWS credent...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/aws-browser-auth.md
180-180: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
209-209: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
231-231: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
308-308: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (1)
docs/prd/aws-browser-auth.md (1)
1-390: Comprehensive PRD addressing browser-based OAuth2 fallback for aws/user identity.The document is well-structured and covers technical details, security, user experience, and rollout planning thoroughly. It clearly articulates the problem (users without static credentials stuck with configuration friction), the solution (three-tier fallback: YAML → keychain → browser webflow), and implementation path. Configuration examples and UX flows (both interactive and headless) are practical and clear. The reference to native SDK implementation and security considerations around PKCE and short-lived credentials are solid.
|
💥 This pull request now has conflicts. Could you fix it @Benbentwo? 🙏 |
Add product requirements document for implementing browser-based OAuth2 authentication for root and IAM identities using the AWS CLI login command. Covers user stories, technical details, requirements, configuration schema, and implementation approach. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <[email protected]>
Change implementation approach from wrapping `aws login` CLI command to native OAuth2 Authorization Code flow with PKCE using AWS SDK. Removes external AWS CLI dependency requirement. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <[email protected]>
Change approach from new aws/login provider to enhancing existing aws/user identity with browser authentication as a fallback: 1. YAML config credentials (highest priority) 2. Keychain credentials via `atmos auth user configure` 3. Browser webflow authentication (NEW - fallback) This provides zero-config authentication while maintaining backward compatibility with existing aws/user configurations. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <[email protected]>
Add planned milestone for OAuth2 browser authentication as fallback for aws/user identity (PR #1887) to the auth initiative roadmap. 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <[email protected]>
Remove AWS CLI version validation requirement (FR-4) which conflicted with NFR-1 (native SDK implementation, no AWS CLI dependency). Updated functional requirements: - FR-3: Cache credentials in atmos credential store (not AWS CLI cache) - FR-4: Role chaining integration (was FR-5) - Renumbered remaining requirements 🤖 Generated with Claude Code Co-Authored-By: Claude Haiku 4.5 <[email protected]>
Update testing strategy to align with native SDK implementation: - Remove AWS CLI version detection test (contradicts NFR-1) - Add OAuth2 PKCE flow (mocked HTTP server) test - Update terminology from CLI failures to authentication failures
03f8bfd to
08707a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/prd/aws-browser-auth.md (1)
180-180: Markdown linting (MD040) can be deferred.Lines 180, 209, 231, and 308 contain fenced code blocks without language specifiers. Per precedent in the codebase, these markdownlint issues should be addressed in a separate documentation cleanup PR and do not block this change.
Also applies to: 209-209, 231-231, 308-308
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/prd/aws-browser-auth.md
🧰 Additional context used
📓 Path-based instructions (1)
docs/prd/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Place all Product Requirement Documents (PRDs) in
docs/prd/using kebab-case filenames
Files:
docs/prd/aws-browser-auth.md
🧠 Learnings (8)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
📚 Learning: 2025-11-10T20:03:56.875Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1775
File: pkg/auth/providers/aws/sso_provisioning.go:40-79
Timestamp: 2025-11-10T20:03:56.875Z
Learning: In the Atmos AWS SSO provider (pkg/auth/providers/aws/sso_provisioning.go), the OAuth access token from the AWS SSO device flow is intentionally stored in the `AccessKeyID` field of `AWSCredentials` during authentication. This token is then extracted and used for ListAccounts and ListAccountRoles API calls during identity provisioning. This design reuses the existing `AWSCredentials` type for token transport rather than creating a separate credential type.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-01-17T00:18:57.769Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-12-13T06:07:34.794Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:34.794Z
Learning: For docs in the cloudposse/atmos repository under docs/prd/, markdownlint issues MD040, MD010, and MD034 should be deferred to a separate documentation cleanup commit and must not block the current PR. If needed, address these issues in a follow-up PR dedicated to documentation improvements.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-01-25T03:51:57.689Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2024-11-22T12:38:33.132Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_utils.go:496-513
Timestamp: 2024-11-22T12:38:33.132Z
Learning: In the Atmos project, continue to flag path traversal issues in code reviews but acknowledge when they are expected and acceptable in specific cases.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-03-25T12:24:36.177Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: internal/exec/go_getter_utils.go:263-264
Timestamp: 2025-03-25T12:24:36.177Z
Learning: Tests for the default Bitbucket username fallback to "x-token-auth" will be added during a future refactoring phase rather than in this PR.
Applied to files:
docs/prd/aws-browser-auth.md
📚 Learning: 2025-04-25T20:54:19.701Z
Learnt from: mcalhoun
Repo: cloudposse/atmos PR: 963
File: website/docs/core-concepts/projects/configuration/stores.mdx:286-286
Timestamp: 2025-04-25T20:54:19.701Z
Learning: For the AWS SSM Parameter Store implementation in Atmos, support for `read_role_arn` and `write_role_arn` options is essential to enable cross-account access, allowing users to run operations like `terraform plan` in multiple accounts while accessing values across keystores. Azure Key Vault would need similar capabilities for cross-tenant/subscription authentication.
Applied to files:
docs/prd/aws-browser-auth.md
🪛 LanguageTool
docs/prd/aws-browser-auth.md
[grammar] ~318-~318: Ensure spelling is correct
Context: ...ack 6. Exchange code for tokens via AWS signin service 7. Return temporary AWS credent...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/prd/aws-browser-auth.md
180-180: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
209-209: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
231-231: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
308-308: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Summary
🔇 Additional comments (1)
docs/prd/aws-browser-auth.md (1)
1-390: Document is well-structured and comprehensive.The PRD clearly articulates the problem, proposed three-tier credential resolution strategy, and implementation approach. Filename compliance is correct (
docs/prd/with kebab-case). Content aligns with PR objectives and the pragmatic consensus from the discussion thread—embedding browser auth as a fallback withinaws/userrather than introducing a separate auth method type.Previous review feedback (FR-4 removal, test strategy alignment with native SDK) has been properly incorporated. No remaining semantic inconsistencies or contradictions detected.
what
Add Product Requirements Document for enhancing the
aws/useridentity with browser-based OAuth2 authentication as a fallback credential source.aws/useridentity (NOT a new provider type)why
DEV-3829 tracks the implementation of browser-based authentication. This approach:
references
Summary by CodeRabbit
New Features
Documentation
Roadmap
✏️ Tip: You can customize this high-level summary in your review settings.