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 76.59% 76.61% +0.01%
==========================================
Files 832 832
Lines 79524 79524
==========================================
+ Hits 60914 60925 +11
+ Misses 14814 14806 -8
+ Partials 3796 3793 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new PRD documenting browser-based OAuth2 (PKCE) fallback for the Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI/Agent
participant Browser as Browser
participant Auth as OAuth2 Authorization Server
participant Callback as Local Callback Listener
participant STS as AWS STS
CLI->>Browser: open auth URL (PKCE + state)
Browser->>Auth: user authenticates & consents
Auth->>Browser: redirect to localhost callback with code
Browser->>Callback: deliver authorization code
Callback->>CLI: forward code
CLI->>Auth: exchange code + PKCE_verifier for tokens
Auth-->>CLI: return ID token / access token
CLI->>STS: AssumeRoleWithWebIdentity / exchange token for AWS creds
STS-->>CLI: return short-lived AWS credentials
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
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.
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 <noreply@anthropic.com>
bac89ee to
7d8d582
Compare
There was a problem hiding this comment.
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.
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? 🙏 |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
website/src/data/roadmap.js (1)
175-175:⚠️ Potential issue | 🟡 MinorStale PR title in
prs[]doesn't match the milestone.The title
'Add PRD for aws/login provider (native SDK auth)'no longer reflects what this PR delivers. The milestone on Line 165 correctly calls it "Browser-based authentication for aws/user" — the prs entry should match.✏️ Proposed fix
- { number: 1887, title: 'Add PRD for aws/login provider (native SDK auth)' }, + { number: 1887, title: 'Add PRD for browser-based auth for aws/user identity' },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/data/roadmap.js` at line 175, Update the stale PR title in the prs array: find the PR object with number: 1887 and change its title from 'Add PRD for aws/login provider (native SDK auth)' to match the milestone naming "Browser-based authentication for aws/user" (or equivalent phrasing used in the milestone at line ~165) so the prs[] entry reflects the actual deliverable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/prd/aws-browser-auth.md`:
- Around line 111-130: The schema for the aws/user identity is missing the
credentials toggle introduced in Design Decision 2; add the webflow_enabled
boolean under the credentials block for the aws/user identity (e.g.,
credentials.webflow_enabled) with a default of false and document its purpose
and placement alongside region/access_key_id/secret_access_key/mfa_arn in the
"Configuration Schema" section; ensure examples and YAML snippet show
webflow_enabled: false and mention its optionality and default value.
- Around line 76-80: Update the "Credential Lifecycle" section to resolve the
conflict with Design Decision 1 and NFR-1 by clarifying that the
"~/.aws/login/cache" path (or AWS_LOGIN_CACHE_DIRECTORY) is not used by the
native SDK; state explicitly that webflow credentials are stored in the atmos
keychain per Design Decision 1 and that the CLI cache path is only relevant as
an optional, read-only compatibility tier for existing AWS CLI sessions (or
remove the CLI path entirely if stale). Mention "atmos keychain", "Design
Decision 1", and "NFR-1" in the sentence so implementers understand the
authoritative storage location and the optional/legacy nature of the CLI cache.
- Line 313: Update the note about redirect_uri to clarify that for loopback IPs
AWS's arn:aws:signin:::devtools/same-device client follows RFC 8252 and accepts
any ephemeral port at request time (so binding to 127.0.0.1:0 and interpolating
the OS-assigned port into redirect_uri is correct), but retain the guidance to
read back the assigned port and surface a clear, user-friendly error if
ephemeral binding fails (include a hint to check for port conflicts); reference
the terms redirect_uri, 127.0.0.1:0, and arn:aws:signin:::devtools/same-device
in the note so readers know this applies to the described flow.
---
Outside diff comments:
In `@website/src/data/roadmap.js`:
- Line 175: Update the stale PR title in the prs array: find the PR object with
number: 1887 and change its title from 'Add PRD for aws/login provider (native
SDK auth)' to match the milestone naming "Browser-based authentication for
aws/user" (or equivalent phrasing used in the milestone at line ~165) so the
prs[] entry reflects the actual deliverable.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
docs/prd/aws-browser-auth.mdwebsite/src/data/roadmap.js
- Add webflow_enabled field to configuration schema with default true - Clarify credential storage uses atmos keychain (not AWS CLI cache), consistent with Design Decision 1 and NFR-1 - Add RFC 8252 Section 7.3 reference for loopback redirect_uri port acceptance - Update stale PR title in roadmap prs array Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rework Overview and Problem Statement to explain the historical limitation of IAM static credentials, AWS's recent introduction of browser-based auth for IAM/root, and the gap in Atmos's aws/user identity. Link to the AWS announcement blog post. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use XDG data directory for credential storage instead of ~/.aws path - Label example configs as "Existing, Unaffected by Webflow" - Replace --remote flag with existing --interactive=false global flag throughout (FR-2, FR-7, US-4, UX examples, docs, rollout plan) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-for-root-and-iam-identities
…-for-root-and-iam-identities
…-for-root-and-iam-identities
…-for-root-and-iam-identities
|
These changes were released in v1.208.1-test.0. |
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