Skip to content

Commit

Permalink
Merge pull request #336 from jonathanmorley/no-obvious-role
Browse files Browse the repository at this point in the history
No obvious role
  • Loading branch information
jonathanmorley authored May 29, 2024
2 parents 47f794c + af8a629 commit 7daa0f0
Show file tree
Hide file tree
Showing 14 changed files with 173 additions and 22 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ jobs:
cache-all-crates: true
key: ${{ matrix.target }}
- name: Clippy
run: cargo clippy --target=${{ matrix.target }} --all-targets --all-features --workspace --exclude=aws-config -- -D warnings
run: cargo clippy --target=${{ matrix.target }} --all-targets --all-features -- -D warnings
- name: Build
run: cargo build --target=${{ matrix.target }} --all-targets --all-features
- name: Test
Expand Down
1 change: 0 additions & 1 deletion .tool-versions

This file was deleted.

85 changes: 85 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ resolver = "2"
# The preferred cargo-dist version to use in CI (Cargo.toml SemVer syntax)
cargo-dist-version = "0.0.7"
# The preferred Rust toolchain to use in CI (rustup toolchain syntax)
rust-toolchain-version = "1.67.1"
rust-toolchain-version = "1.78.0"
# CI backends to support (see 'cargo dist generate-ci')
ci = ["github"]
# The installers to generate for each app
Expand Down
1 change: 1 addition & 0 deletions aws-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
rustdoc::missing_crate_level_docs,
unreachable_pub
)]
#![allow(clippy::all)]

//! `aws-config` provides implementations of region and credential resolution.
//!
Expand Down
2 changes: 2 additions & 0 deletions oktaws/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ eyre = "0.6.8"
aws-types = "0.55.3"
aws-smithy-types = "0.55.3"
aws-config = { path = "../aws-config", default_features = false }
mockall = "0.12.1"
mockall_double = "0.3.1"

[dev-dependencies]
aws-smithy-client = { version = "0.55", features = ["test-util"] }
Expand Down
2 changes: 1 addition & 1 deletion oktaws/src/aws/role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ mod tests {
let connector : TestConnection<_>= TestConnection::new(vec![(
http::Request::builder()
.uri(http::Uri::from_static("https://sts.us-east-1.amazonaws.com/"))
.body(SdkBody::from(r#"Action=AssumeRoleWithSAML&Version=2011-06-15&RoleArn=arn%3Aaws%3Aiam%3A%3A123456789012%3Arole%2Fmock-role&PrincipalArn=arn%3Aaws%3Aiam%3A%3A123456789012%3Asaml-provider%2Fokta-idp&SAMLAssertion=SAML_ASSERTION"#)).unwrap(),
.body(SdkBody::from(r"Action=AssumeRoleWithSAML&Version=2011-06-15&RoleArn=arn%3Aaws%3Aiam%3A%3A123456789012%3Arole%2Fmock-role&PrincipalArn=arn%3Aaws%3Aiam%3A%3A123456789012%3Asaml-provider%2Fokta-idp&SAMLAssertion=SAML_ASSERTION")).unwrap(),
http::Response::builder()
.status(http::StatusCode::from_u16(403).unwrap())
.body(saml_xml).unwrap())
Expand Down
4 changes: 2 additions & 2 deletions oktaws/src/aws/saml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ pub fn extract_saml_account_name(text: &str) -> Result<String> {
let doc = kuchiki::parse_html().one(text);
let account_str = doc
.select("div.saml-account-name")
.map_err(|_| eyre!("SAML account name Not found"))?
.map_err(|()| eyre!("SAML account name Not found"))?
.next()
.ok_or_else(|| eyre!("SAML account name Not found"))?
.text_contents();
Expand All @@ -143,7 +143,7 @@ pub fn extract_dashboard_account_name(text: &str) -> Result<String> {
let doc = kuchiki::parse_html().one(text);
let account_str = doc
.select("span[data-testid='awsc-nav-account-menu-button']")
.map_err(|_| eyre!("Dashboard Account selector not valid"))?
.map_err(|()| eyre!("Dashboard Account selector not valid"))?
.next()
.ok_or_else(|| eyre!("Dashboard Account name not found in {}", text))?
.text_contents();
Expand Down
52 changes: 47 additions & 5 deletions oktaws/src/config/organization.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use mockall_double::double;

use crate::config::oktaws_home;
use crate::config::profile::{self, Profile};
#[double]
use crate::okta::client::Client as OktaClient;
use crate::select_opt;

Expand Down Expand Up @@ -59,11 +62,16 @@ impl Config {
.filter(|&(i, _)| i > 1)
.map(|(_, x)| x)
.collect::<Vec<_>>();
let default_role = select_opt(
default_role_names,
"Choose Default Role [None]",
ToOwned::to_owned,
)?;

let default_role = if default_role_names.is_empty() {
None
} else {
select_opt(
default_role_names,
"Choose Default Role [None]",
ToOwned::to_owned,
)?
};

let profile_futures = selected_links
.into_iter()
Expand Down Expand Up @@ -213,6 +221,8 @@ impl Pattern {

#[cfg(test)]
mod tests {
use crate::aws::role::SamlRole;

use super::*;

use std::env;
Expand Down Expand Up @@ -413,4 +423,36 @@ foo = "foo"

assert_eq!(organizations.len(), 2);
}

#[tokio::test]
async fn init_without_obvious_default_role() {
let mut client = OktaClient::new();
client.expect_app_links().returning(|_| Ok(Vec::new()));

// With two (different) roles
client.expect_all_roles().returning(|_| {
Ok(vec![
SamlRole {
provider: "arn:aws:iam::123456789012:saml-provider/okta-idp"
.parse()
.unwrap(),
role: "arn:aws:iam::123456789012:role/mock-role".parse().unwrap(),
},
SamlRole {
provider: "arn:aws:iam::123456789012:saml-provider/okta-idp"
.parse()
.unwrap(),
role: "arn:aws:iam::123456789012:role/mock-role-2"
.parse()
.unwrap(),
},
])
});

let config = Config::from_organization(&client, String::from("test_user"))
.await
.unwrap();

assert_eq!(config.role, None);
}
}
11 changes: 7 additions & 4 deletions oktaws/src/config/profile.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use mockall_double::double;

#[double]
use crate::okta::client::Client as OktaClient;
use crate::{
aws::saml::extract_account_name,
aws::sso::Client as SsoClient,
aws::{get_account_alias, sts_client},
okta::applications::AppLink,
okta::client::Client as OktaClient,
select,
};

Expand All @@ -27,7 +30,7 @@ pub enum Config {
}

impl Config {
#[instrument(skip(client, link, default_role), fields(organization=%client.base_url, application=%link.label))]
#[instrument(skip(client, link, default_role), fields(organization=%client.base_url(), application=%link.label))]
pub async fn from_app_link(
client: &OktaClient,
link: AppLink,
Expand All @@ -47,7 +50,7 @@ impl Config {

let saml_role = match roles.len() {
0 => Err(eyre!("No role found")),
1 => Ok(roles.get(0).unwrap()),
1 => Ok(roles.first().unwrap()),
_ => {
if let Some(default_role) = default_role.clone() {
match roles
Expand Down Expand Up @@ -148,7 +151,7 @@ impl Profile {
})
}

#[instrument(skip(self, client), fields(organization=%client.base_url, profile=%self.name))]
#[instrument(skip(self, client), fields(organization=%client.base_url(), profile=%self.name))]
pub async fn into_credentials(self, client: &OktaClient) -> Result<Credentials> {
let saml_app_link = client.app_links(None).await?.into_iter().find(|app_link| {
app_link.app_name == "amazon_aws" && app_link.label == self.application_name
Expand Down
9 changes: 6 additions & 3 deletions oktaws/src/okta/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ impl Client {
"Credentials"
};

debug!("Attempting to login to {} with {login_type}", self.base_url);
debug!(
"Attempting to login to {} with {login_type}",
self.base_url()
);

self.post("api/v1/authn", req).await
}
Expand Down Expand Up @@ -196,7 +199,7 @@ impl Client {

let extra_verification = if let Ok(head) = doc.select_first("head") {
if let Ok(title) = head.as_node().select_first("title") {
let re = Regex::new(r#".* - Extra Verification$"#)?;
let re = Regex::new(r".* - Extra Verification$")?;
re.is_match(&title.text_contents())
} else {
false
Expand All @@ -206,7 +209,7 @@ impl Client {
};

if extra_verification {
Regex::new(r#"var stateToken = '(.+)';"#)?
Regex::new(r"var stateToken = '(.+)';")?
.captures(text)
.map_or_else(
|| Err(eyre!("No state token found")),
Expand Down
19 changes: 17 additions & 2 deletions oktaws/src/okta/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use url::Url;
#[derive(Debug)]
pub struct Client {
client: HttpClient,
pub base_url: Url,
base_url: Url,
pub cookies: Arc<Jar>,
}

Expand Down Expand Up @@ -68,7 +68,7 @@ impl Client {
let mut base_url = Url::parse(&format!("https://{organization}.okta.com/"))?;
base_url
.set_username(&username)
.map_err(|_| eyre!("Cannot set username for URL"))?;
.map_err(|()| eyre!("Cannot set username for URL"))?;

let cookies = Arc::from(Jar::default());

Expand Down Expand Up @@ -124,6 +124,11 @@ impl Client {
Ok(client)
}

#[must_use]
pub const fn base_url(&self) -> &Url {
&self.base_url
}

pub fn set_session_id(&mut self, session_id: &str) {
self.cookies
.add_cookie_str(&format!("sid={session_id}"), &self.base_url);
Expand Down Expand Up @@ -276,3 +281,13 @@ impl Client {
}
}
}

#[cfg(test)]
mockall::mock! {
pub Client {
pub fn base_url(&self) -> &Url;
pub async fn app_links(&self, user_id: Option<()>) -> Result<Vec<crate::okta::applications::AppLink>>;
pub async fn all_roles(&self, links: Vec<crate::okta::applications::AppLink>) -> Result<Vec<crate::aws::role::SamlRole>>;
pub async fn get_saml_response(&self, url: Url) -> Result<crate::aws::saml::Response>;
}
}
Loading

0 comments on commit 7daa0f0

Please sign in to comment.