chore: write credentials to a file #1320

bergjaak · 2025-04-23T01:21:59Z

Description of changes:

piggy back off of the token refresh and each time write it to a file
This file is then read by another process (specifically, MCP servers) on the same machine
requests from this process is then authenticated

The purpose of this is to allow MCP servers to use credentials to make requests to AWS services, such as CodeWhisperer, which will be needed to support future features.

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

Add functionality to write user credentials to ~/.aws/amazonq/creds.json when a user logs in or when their credentials are refreshed. This allows other tools to access the authentication information. 🤖 Assisted by [Amazon Q Developer](https://aws.amazon.com/q/developer)

Add unit tests to verify that credentials are correctly written to ~/.aws/amazonq/creds.json for both BuilderId and IamIdentityCenter token types. Tests ensure that the file contains the correct token information. 🤖 Assisted by [Amazon Q Developer](https://aws.amazon.com/q/developer)

codecov-commenter · 2025-04-23T01:23:42Z

Codecov Report

Attention: Patch coverage is 0% with 63 lines in your changes missing coverage. Please review.

Project coverage is 14.34%. Comparing base (da59663) to head (e59192b).

Files with missing lines	Patch %	Lines
crates/fig_auth/src/builder_id.rs	0.00%	51 Missing ⚠️
crates/q_cli/src/cli/user.rs	0.00%	8 Missing ⚠️
crates/fig_util/src/directories.rs	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1320      +/-   ##
==========================================
- Coverage   14.36%   14.34%   -0.02%     
==========================================
  Files        2368     2368              
  Lines      206348   206405      +57     
  Branches   186712   186769      +57     
==========================================
- Hits        29633    29605      -28     
- Misses     175257   175343      +86     
+ Partials     1458     1457       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dingfeli · 2025-04-23T17:31:39Z

Not entirely sure why the lock file was changed here since it does not seem like you had pulled in new dependencies.
Our CI is run with --lock flag, which menas the lock file would need to be up to date in the commit, otherwise it would fail since the --lock flag prevents it from updating.

When it becomes out of sync for whatever reason, checkout the version of the lock file from main with git checkout main -- Cargo.lock as a baseline and run one of cargo check, cargo build or any commit that would trigger a build to update the lock file. This way all of the existing versions are preserved and you won't run into compatibility warnings like you did there.

dingfeli · 2025-04-23T17:36:29Z

crates/fig_auth/src/builder_id.rs

@@ -537,14 +537,73 @@ pub async fn poll_create_token(
    }
 }

+/// Write credentials to ~/.aws/amazonq/creds.json
+pub fn write_credentials_to_file(token: &BuilderIdToken) -> Result<()> {


Since the callee of this function already has an async scope we should probably subscribe to the same IO behavior here and make this async here as well. Otherwise we would be blocking the executing thread for as long as the file write takes.

dingfeli · 2025-04-23T17:42:13Z

crates/fig_auth/src/builder_id.rs

+    });
+
+    // Ensure ~/.aws/amazonq directory exists
+    let mut path = PathBuf::from(std::env::var("HOME").unwrap_or_else(|_| "~".to_string()));


I don't think tilde expansion works out of the box like this.
See https://github.com/aws/amazon-q-developer-cli/blob/main/crates/fig_desktop_api/src/util.rs#L10-L17

…into feature/write-creds

chaynabors

I think we might be missing handling for auth through the desktop app. I'll check and get back to you.

chaynabors · 2025-04-24T17:28:37Z

crates/q_cli/src/cli/user.rs

@@ -244,6 +247,13 @@ pub async fn login_interactive(args: LoginArgs) -> Result<()> {
                        ]);
                        registration.finish(&client, Some(&secret_store)).await?;
                        fig_telemetry::send_user_logged_in().await;
+
+                        // Write credentials to file after successful login
+                        if let Ok(Some(_)) = fig_auth::builder_id_token().await {


We aren't logging the failure case

chaynabors · 2025-04-24T17:30:08Z

crates/q_cli/src/cli/user.rs

@@ -306,6 +316,12 @@ async fn try_device_authorization(
            PollCreateToken::Complete(_) => {
                fig_telemetry::send_user_logged_in().await;
                spinner.stop_with_message("Logged in successfully".into());
+
+                // Write credentials to file after successful device authorization
+                if let Ok(Some(_)) = fig_auth::builder_id_token().await {


Should probably log this if it fails, a match instead of an if let condition

I can't remember if error level messages are output to the terminal by default, can you test to see if it's user facing and let me know?

crates/fig_auth/src/builder_id.rs

chaynabors · 2025-04-24T17:46:16Z

crates/fig_auth/src/builder_id.rs

@@ -715,4 +785,153 @@ mod tests {
        let token = refresh_token().await.unwrap().unwrap();
        println!("{:?}", token);
    }
+
+    fn cleanup_test_file() {


Tests are ran in parallel. I don't think you can do this because you'll encounter race conditions between tests.

crates/fig_auth/src/error.rs

crates/fig_auth/src/builder_id.rs

…into feature/write-creds

bergjaak added 5 commits April 22, 2025 23:49

feat: tests:

819f066

feat: tests:

a8080c7

feat: tests:

aea1aee

bergjaak requested a review from a team as a code owner April 23, 2025 01:22

dingfeli reviewed Apr 23, 2025

View reviewed changes

bergjaak added 2 commits April 23, 2025 18:06

Merge branch 'main' of https://github.com/aws/amazon-q-developer-cli …

746b6e1

…into feature/write-creds

chore: make func async

a0d1e99

dingfeli requested review from chaynabors and brandonskiser April 23, 2025 19:17

bergjaak added 7 commits April 23, 2025 19:24

Merge branch 'main' of https://github.com/aws/amazon-q-developer-cli …

20fe6e3

…into feature/write-creds

chore: use env.home() instead of tilde

9338492

feat: make select_command async

f29abd4

feat: make select_command async

9792b93

feat: make select_command async

755cb01

init

06cdb8b

feat: make select_command async

1a8ded2

chaynabors requested changes Apr 24, 2025

View reviewed changes

mikewrighton reviewed Apr 24, 2025

View reviewed changes

crates/fig_auth/src/builder_id.rs Outdated Show resolved Hide resolved

bergjaak added 7 commits April 24, 2025 20:10

Merge branch 'main' of https://github.com/aws/amazon-q-developer-cli …

d8b78ef

…into feature/write-creds

chore: pr feedback

b6612f9

chore: pr feedback

84ca499

chore: pr feedback

f580869

chore: pr feedback

b1cf3c3

chore: pr feedback

a8c5782

feat: make select_command async

e59192b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: write credentials to a file #1320

chore: write credentials to a file #1320

bergjaak commented Apr 23, 2025 •

edited

Loading

codecov-commenter commented Apr 23, 2025 •

edited

Loading

dingfeli commented Apr 23, 2025

dingfeli Apr 23, 2025

dingfeli Apr 23, 2025

chaynabors left a comment

chaynabors Apr 24, 2025

chaynabors Apr 24, 2025

chaynabors Apr 24, 2025

chaynabors Apr 24, 2025

chaynabors Apr 24, 2025

chore: write credentials to a file #1320

Are you sure you want to change the base?

chore: write credentials to a file #1320

Conversation

bergjaak commented Apr 23, 2025 • edited Loading

codecov-commenter commented Apr 23, 2025 • edited Loading

Codecov Report

dingfeli commented Apr 23, 2025

dingfeli Apr 23, 2025

Choose a reason for hiding this comment

dingfeli Apr 23, 2025

Choose a reason for hiding this comment

chaynabors left a comment

Choose a reason for hiding this comment

chaynabors Apr 24, 2025

Choose a reason for hiding this comment

chaynabors Apr 24, 2025

Choose a reason for hiding this comment

chaynabors Apr 24, 2025

Choose a reason for hiding this comment

chaynabors Apr 24, 2025

Choose a reason for hiding this comment

chaynabors Apr 24, 2025

Choose a reason for hiding this comment

bergjaak commented Apr 23, 2025 •

edited

Loading

codecov-commenter commented Apr 23, 2025 •

edited

Loading