SGA-12959- sync satori credentials and config using satori as source … (#146)

* SGA-12959- sync satori credentials and config using satori as source of truth and updating the user aws account

* SGA-12959- fix old errors found uisng clippy

* SGA-12959- add audit package

* SGA-12959- add audit package ,fix typo

* SGA-12959- add tests

Author: asafyat

.github/workflows/rust.yml

@@ -2,9 +2,9 @@ name: Rust
 
 on:
   push:
-    branches: [ "main" ]
+    branches: ["main"]
   pull_request:
-    branches: [ "main" ]
+    branches: ["main"]
 
 env:
   CARGO_TERM_COLOR: always
@@ -14,68 +14,67 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-    - uses: actions/checkout@v4
-    - name: Build
-      run: cargo build --verbose
-    - name: install next test
-      run: curl -LsSf https://get.nexte.st/latest/linux | tar zxf - -C ${CARGO_HOME:-~/.cargo}/bin 
-    - name: Run tests
-      run: cargo nextest run --verbose
-    - name: Upload Artifact
-      uses: actions/upload-artifact@v4
-      with:
-        if-no-files-found: error
-        name: satori_linux
-        path: ./target/debug/satori
-  
+      - uses: actions/checkout@v4
+      - name: Build
+        run: cargo build --verbose
+      - name: install next test
+        run: curl -LsSf https://get.nexte.st/latest/linux | tar zxf - -C ${CARGO_HOME:-~/.cargo}/bin
+      - name: Run tests
+        run: cargo nextest run --verbose
+      - name: Upload Artifact
+        uses: actions/upload-artifact@v4
+        with:
+          if-no-files-found: error
+          name: satori_linux
+          path: ./target/debug/satori
+
   build-windows:
     runs-on: windows-latest
 
     steps:
-    - uses: actions/checkout@v4
-    - name: Build
-      run: cargo build --verbose
-    - name: Upload Artifact
-      uses: actions/upload-artifact@v4
-      with:
-        if-no-files-found: error
-        name: satori.exe
-        path: ./target/debug/satori.exe
-  
+      - uses: actions/checkout@v4
+      - name: Build
+        run: cargo build --verbose
+      - name: Upload Artifact
+        uses: actions/upload-artifact@v4
+        with:
+          if-no-files-found: error
+          name: satori.exe
+          path: ./target/debug/satori.exe
+
   build-mac:
     runs-on: macos-latest
 
     steps:
-    - uses: actions/checkout@v4
-    - name: Build
-      run: cargo build --verbose    
-    - name: Upload Artifact
-      uses: actions/upload-artifact@v4
-      with:
-        if-no-files-found: error
-        name: satori_mac
-        path: ./target/debug/satori
-  
+      - uses: actions/checkout@v4
+      - name: Build
+        run: cargo build --verbose
+      - name: Upload Artifact
+        uses: actions/upload-artifact@v4
+        with:
+          if-no-files-found: error
+          name: satori_mac
+          path: ./target/debug/satori
+
   clippy:
     runs-on: ubuntu-latest
-    steps:  
-    - uses: actions/checkout@v4
-    - name: Build
-      run: cargo build --verbose    
-    - name: clippy
-      run: cargo clippy -- -D warnings
+    steps:
+      - uses: actions/checkout@v4
+      - name: Build
+        run: cargo build --verbose
+      - name: clippy
+        run: cargo clippy -- -D warnings
 
   audit:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
       - name: Audit
-        run: cargo audit
-  
+        run: cargo install cargo-audit && cargo audit
+
   fmt:
     runs-on: ubuntu-latest
-    steps:  
-    - uses: actions/checkout@v4
-    - name: fmt
-      run: cargo fmt -- --check
-      
+    steps:
+      - uses: actions/checkout@v4
+      - name: fmt
+        run: cargo fmt -- --check

src/cli/parsers/run/dbt.rs

@@ -38,7 +38,7 @@ pub fn build(args: &ArgMatches) -> Result<Flow, CliError> {
 /// 2. `DBT_PROFILES_DIR` environment variable is set
 /// 3. profiles.yml file is found in the current directory
 /// 4. default to ~/.dbt directory
-/// The file is always named profiles.yml
+///    The file is always named profiles.yml
 fn get_profiles_path(args: &ArgMatches) -> PathBuf {
     match args.get_one::<PathBuf>("profile-dir") {
         Some(profile_dir) => Path::new(&profile_dir).to_path_buf(),

src/cli/parsers/tools/aws.rs

@@ -42,8 +42,7 @@ fn get_from_env_or_default(env_var: &str, default: &str) -> Result<PathBuf, cli:
         Err(err) => {
             log::debug!("failed to read path from env var: {}", err);
 
-            let home_dir =
-                homedir::get_my_home()?.ok_or_else(|| cli::errors::CliError::HomeDirNotFound)?;
+            let home_dir = homedir::get_my_home()?.ok_or(cli::errors::CliError::HomeDirNotFound)?;
             Ok(home_dir.join(default))
         }
     }

src/cli/parsers/tools/pgpass.rs

@@ -32,7 +32,7 @@ pub fn build(args: &ArgMatches) -> Result<PgPass, CliError> {
 #[cfg(target_family = "unix")]
 fn get_pgpass_file_path() -> Result<PathBuf, CliError> {
     Ok(homedir::get_my_home()?
-        .ok_or_else(|| CliError::HomeDirNotFound)?
+        .ok_or(CliError::HomeDirNotFound)?
         .join(Path::new(PGPASS_FILE_NAME)))
 }
 

src/tools/aws/flow.rs

@@ -1,5 +1,5 @@
 use std::{
-    collections::hash_map::DefaultHasher,
+    collections::{hash_map::DefaultHasher, HashSet},
     hash::{Hash, Hasher},
     path::Path,
 };
@@ -28,6 +28,8 @@ where
     let (credentials, datastores_info) =
         login::run_with_file(&params.login, user_input_stream).await?;
 
+    let mut expected_satori_profiles = HashSet::new();
+
     let mut is_first = true;
     for (datastore_name, datastore_info) in get_aws_datastores(&datastores_info) {
         let datastore_type = format!("{:?}", &datastore_info.r#type);
@@ -38,6 +40,8 @@ where
         );
         let endpoint_url = &datastore_info.get_datastore_name()?;
 
+        expected_satori_profiles.insert(profile_name.clone());
+
         config_content
             .with_section(Some(format!("profile {profile_name}")))
             .set("endpoint_url", format!("https://{endpoint_url}"));
@@ -53,6 +57,9 @@ where
         log::info!("    {datastore_name}: {profile_name}");
     }
 
+    remove_stale_satori_profiles(&mut credentials_content, &expected_satori_profiles);
+    remove_stale_satori_profiles_from_config(&mut config_content, &expected_satori_profiles);
+
     credentials_content
         .write_to_file(params.credentials_path.clone())
         .map_err(|err| errors::ToolsError::FailedToWriteToFile(err, params.credentials_path))?;
@@ -98,6 +105,55 @@ fn get_hash_for_datastore(datastore_info: &DatastoreInfo, num_digits: u32) -> u6
     hash_value % divisor
 }
 
+fn remove_stale_satori_profiles(ini: &mut Ini, expected_profiles: &HashSet<String>) {
+    let sections_to_remove: Vec<String> = ini
+        .sections()
+        .filter_map(|section_name| {
+            section_name.and_then(|name| {
+                if name.starts_with(PROFILE_NAME_PREFIX) && !expected_profiles.contains(name) {
+                    Some(name.to_string())
+                } else {
+                    None
+                }
+            })
+        })
+        .collect();
+
+    for section in &sections_to_remove {
+        log::info!("Removing stale profile: {}", section);
+        ini.delete(Some(section));
+    }
+}
+
+fn remove_stale_satori_profiles_from_config(ini: &mut Ini, expected_profiles: &HashSet<String>) {
+    let prefix = format!("profile {}", PROFILE_NAME_PREFIX);
+    let sections_to_remove: Vec<String> = ini
+        .sections()
+        .filter_map(|section_name| {
+            section_name.and_then(|name| {
+                if name.starts_with(&prefix) {
+                    // Extract profile name without "profile " prefix
+                    let profile_name = name.strip_prefix("profile ").unwrap_or(name);
+                    if profile_name.starts_with(PROFILE_NAME_PREFIX)
+                        && !expected_profiles.contains(profile_name)
+                    {
+                        Some(name.to_string())
+                    } else {
+                        None
+                    }
+                } else {
+                    None
+                }
+            })
+        })
+        .collect();
+
+    for section in &sections_to_remove {
+        log::info!("Removing stale config section: {}", section);
+        ini.delete(Some(section));
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::collections::HashMap;

tests/aws_files/stale_config_with_s3

@@ -0,0 +1,11 @@
+[default]
+region=us-east-1
+
+[profile satori_athena_190337]
+endpoint_url=https://athena.example.com
+
+[profile satori_s3_575495]
+endpoint_url=https://s3.example.com
+
+[profile personal-account]
+region=us-west-2

tests/aws_files/stale_credentials_with_s3

@@ -0,0 +1,15 @@
+[default]
+aws_access_key_id=USER_PERSONAL
+aws_secret_access_key=PERSONAL_PASSWORD
+
+[satori_athena_190337]
+aws_access_key_id=USER
+aws_secret_access_key=OLD_PASSWORD
+
+[satori_s3_575495]
+aws_access_key_id=USER
+aws_secret_access_key=OLD_PASSWORD
+
+[personal-account]
+aws_access_key_id=PERSONAL_KEY
+aws_secret_access_key=PERSONAL_SECRET

tests/datastores_files/athena_only_datastores.json

@@ -0,0 +1,12 @@
+{
+    "account_id": "account_id",
+    "datastores": {
+        "athena1": {
+            "satori_host": "athena.example.com",
+            "databases": [],
+            "port": null,
+            "type": "ATHENA",
+            "deployment_type": null
+        }
+    }
+}

tests/test_aws.rs

@@ -141,6 +141,87 @@ async fn test_aws_expired_credentials() {
     validates_aws_files(&temp_dir, &credentials, &datastores_info);
 }
 
+/// Test that stale Satori profiles are removed when user loses access to datastores
+/// User had S3 and Athena, now only has Athena. S3 profile should be removed.
+#[tokio::test]
+async fn test_aws_stale_profiles_removed() {
+    let temp_dir = temp_dir::generate();
+    let credentials = get_old_credentials_expire_two_hours();
+    let datastores_info = get_mock_datastores("athena_only_datastores.json");
+    let datastores_entries_response_path = get_access_details_db_empty_response_path();
+
+    // Start with files that have both S3 and Athena profiles
+    let old_config = read_ini_file(AWS_CREDENTIALS_DIR, "stale_config_with_s3");
+    let old_credentials = read_ini_file(AWS_CREDENTIALS_DIR, "stale_credentials_with_s3");
+
+    write_credentials_temp_dir(&credentials, &temp_dir);
+    write_datastores_temp_dir(&datastores_info, &temp_dir);
+    write_aws_temp_dir(&temp_dir, old_config, old_credentials);
+
+    run_aws_with_server_assert_no_calls_to_server(
+        &temp_dir,
+        &datastores_entries_response_path,
+        AwsBuilder::default(),
+    )
+    .await;
+
+    // Read the resulting files
+    let actual_credentials = read_actual_aws_file(&temp_dir, "credentials");
+    let actual_config = read_actual_aws_file(&temp_dir, "config");
+
+    // Verify S3 profile was removed from credentials
+    assert!(
+        actual_credentials.section(Some(S3_PROFILE)).is_none(),
+        "S3 profile should be removed from credentials"
+    );
+
+    // Verify S3 profile was removed from config
+    let s3_config_section = format!("profile {}", S3_PROFILE);
+    assert!(
+        actual_config.section(Some(&s3_config_section)).is_none(),
+        "S3 profile should be removed from config"
+    );
+
+    // Verify Athena profile still exists in credentials
+    assert!(
+        actual_credentials.section(Some(ATHENA_PROFILE)).is_some(),
+        "Athena profile should still exist in credentials"
+    );
+
+    // Verify Athena profile still exists in config
+    let athena_config_section = format!("profile {}", ATHENA_PROFILE);
+    assert!(
+        actual_config
+            .section(Some(&athena_config_section))
+            .is_some(),
+        "Athena profile should still exist in config"
+    );
+
+    // Verify non-Satori profiles are preserved in credentials
+    assert!(
+        actual_credentials.section(Some("default")).is_some(),
+        "Default profile should be preserved"
+    );
+    assert!(
+        actual_credentials
+            .section(Some("personal-account"))
+            .is_some(),
+        "Personal account profile should be preserved"
+    );
+
+    // Verify non-Satori profiles are preserved in config
+    assert!(
+        actual_config.section(Some("default")).is_some(),
+        "Default config should be preserved"
+    );
+    assert!(
+        actual_config
+            .section(Some("profile personal-account"))
+            .is_some(),
+        "Personal account config should be preserved"
+    );
+}
+
 async fn run_aws_with_server_assert_no_calls_to_server(
     temp_dir: &TempDir,
     datastores_info_file_path: &Path,