Skip to content

Commit

Permalink
feat: better error when not enough scopes for SSO login (#9948)
Browse files Browse the repository at this point in the history
### Description

When you already have a token but don't have SSO scopes, we were
throwing an error that didn't have much information. The error should be
more clear now when you are in this state, informing you to use
`--force`.

Specifically you could get into this state by doing:
```
turbo login
turbo login --sso-team=my-team
```

#### Note
I happy-pathed (error-pathed?) this for the specific case I wanted to
solve for. I'm not sure if this is accidentally changing the error for
other problematic states you can be in.

### Testing Instructions

I'm struggling to write a unit test. Help would be appreciate if you
think one would be good for this (I do).

Additionally, here's a before and after:
Before:
```
▲ 👟 turbo on shew/6b0e1
  turbo login
turbo 2.4.0

>>> Opening browser to https://vercel.com/turborepo/token?redirect_uri=redacted

>>> Success! Turborepo CLI authorized for [email protected]
To connect to your Remote Cache, run the following in any turborepo:
  npx turbo link


▲ 👟 turbo on shew/6b0e1 took 6s
  turbo login --sso-team=my-team
turbo 2.4.0

  × Error making HTTP request: HTTP status client error (403 Forbidden) for url (https://vercel.com/api/v2/teams/my-team)
  ╰─▶ HTTP status client error (403 Forbidden) for url (https://vercel.com/api/v2/teams/my-team)
```

After:
```
▲ 👟 turbo on shew/6b0e1
  dt login
turbo 2.4.2-canary.0

>>> Opening browser to https://vercel.com/turborepo/token?redirect_uri=redacted

>>> Success! Turborepo CLI authorized for [email protected]
To connect to your Remote Cache, run the following in any turborepo:
  npx turbo link


▲ 👟 turbo on shew/6b0e1 took 2s
  dt login --sso-team=my-team
turbo 2.4.2-canary.0

  × [HTTP 403] request to https://vercel.com/api/v2/teams/my-team returned "HTTP status client error (403 Forbidden) for url (https://vercel.com/api/v2/teams/my-team)"
  │ Try logging in again, or force a refresh of your token (turbo login --sso-team=your-team --force).
  ```

---------

Co-authored-by: Chris Olszewski <[email protected]>
  • Loading branch information
anthonyshew and chris-olszewski authored Feb 14, 2025
1 parent 8e8c6b7 commit 80a6f65
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 1 deletion.
2 changes: 2 additions & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions crates/turborepo-auth/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,6 @@ url = { workspace = true }
webbrowser = { workspace = true }

[dev-dependencies]
http = "1.1.0"
httpmock = { workspace = true }
port_scanner = { workspace = true }
154 changes: 153 additions & 1 deletion crates/turborepo-auth/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,33 @@ impl Token {
Ok(true)
}

async fn handle_sso_token_error<T: TokenClient>(
&self,
client: &T,
error: reqwest::Error,
) -> Result<bool, Error> {
if error.status() == Some(reqwest::StatusCode::FORBIDDEN) {
let metadata = self.fetch_metadata(client).await?;
if !metadata.token_type.is_empty() {
return Err(Error::APIError(turborepo_api_client::Error::InvalidToken {
status: error
.status()
.unwrap_or(reqwest::StatusCode::FORBIDDEN)
.as_u16(),
url: error
.url()
.map(|u| u.to_string())
.unwrap_or("Unknown url".to_string()),
message: error.to_string(),
}));
}
}

Err(Error::APIError(turborepo_api_client::Error::ReqwestError(
error,
)))
}

/// This is the same as `is_valid`, but also checks if the token is valid
/// for SSO.
///
Expand Down Expand Up @@ -158,7 +185,12 @@ impl Token {

Ok(true)
}
(Err(e), _) | (_, Err(e)) => Err(Error::APIError(e)),
(Err(e), _) | (_, Err(e)) => match e {
turborepo_api_client::Error::ReqwestError(e) => {
self.handle_sso_token_error(client, e).await
}
e => Err(Error::APIError(e)),
},
}
}

Expand Down Expand Up @@ -675,4 +707,124 @@ mod tests {
let result = Token::from_file(&file_path);
assert!(matches!(result, Err(Error::TokenNotFound)));
}

struct MockSSOTokenClient {
metadata_response: Option<ResponseTokenMetadata>,
}

impl TokenClient for MockSSOTokenClient {
async fn get_metadata(
&self,
_token: &str,
) -> turborepo_api_client::Result<ResponseTokenMetadata> {
if let Some(metadata) = &self.metadata_response {
Ok(metadata.clone())
} else {
Ok(ResponseTokenMetadata {
id: "test".to_string(),
name: "test".to_string(),
token_type: "".to_string(),
origin: "test".to_string(),
scopes: vec![],
active_at: current_unix_time() - 100,
created_at: 0,
})
}
}

async fn delete_token(&self, _token: &str) -> turborepo_api_client::Result<()> {
Ok(())
}
}

#[tokio::test]
async fn test_handle_sso_token_error_forbidden_with_invalid_token_error() {
let token = Token::new("test-token".to_string());
let client = MockSSOTokenClient {
metadata_response: Some(ResponseTokenMetadata {
id: "test".to_string(),
name: "test".to_string(),
token_type: "sso".to_string(),
origin: "test".to_string(),
scopes: vec![],
active_at: current_unix_time() - 100,
created_at: 0,
}),
};

let errorful_response = reqwest::Response::from(
http::Response::builder()
.status(reqwest::StatusCode::FORBIDDEN)
.body("")
.unwrap(),
);

let result = token
.handle_sso_token_error(&client, errorful_response.error_for_status().unwrap_err())
.await;
assert!(matches!(
result,
Err(Error::APIError(
turborepo_api_client::Error::InvalidToken { .. }
))
));
}

#[tokio::test]
async fn test_handle_sso_token_error_forbidden_without_token_type() {
let token = Token::new("test-token".to_string());
let client = MockSSOTokenClient {
metadata_response: Some(ResponseTokenMetadata {
id: "test".to_string(),
name: "test".to_string(),
token_type: "".to_string(),
origin: "test".to_string(),
scopes: vec![],
active_at: current_unix_time() - 100,
created_at: 0,
}),
};

let errorful_response = reqwest::Response::from(
http::Response::builder()
.status(reqwest::StatusCode::FORBIDDEN)
.body("")
.unwrap(),
);

let result = token
.handle_sso_token_error(&client, errorful_response.error_for_status().unwrap_err())
.await;
assert!(matches!(
result,
Err(Error::APIError(turborepo_api_client::Error::ReqwestError(
_
)))
));
}

#[tokio::test]
async fn test_handle_sso_token_error_non_forbidden() {
let token = Token::new("test-token".to_string());
let client = MockSSOTokenClient {
metadata_response: None,
};

let errorful_response = reqwest::Response::from(
http::Response::builder()
.status(reqwest::StatusCode::INTERNAL_SERVER_ERROR)
.body("")
.unwrap(),
);

let result = token
.handle_sso_token_error(&client, errorful_response.error_for_status().unwrap_err())
.await;
assert!(matches!(
result,
Err(Error::APIError(turborepo_api_client::Error::ReqwestError(
_
)))
));
}
}

0 comments on commit 80a6f65

Please sign in to comment.