fix: More robust token validation #7407

Zertsov · 2024-02-16T20:51:16Z

Description

Some more modularization and better checking of validity for tokens.

Testing Instructions

Try logins as normal, then login while logged in. If you can expire a token manually, try logging in again and it should do the normal login route again.

Closes TURBO-2391

vercel · 2024-02-16T20:51:17Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
examples-basic-web	🔄 Building (Inspect)	Visit Preview	💬 Add feedback	Feb 21, 2024 0:39am
rust-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 21, 2024 0:39am

9 Ignored Deployments

Name	Status	Preview	Updated (UTC)
examples-designsystem-docs	⬜️ Ignored (Inspect)	Visit Preview	Feb 21, 2024 0:39am
examples-gatsby-web	⬜️ Ignored (Inspect)	Visit Preview	Feb 21, 2024 0:39am
examples-kitchensink-blog	⬜️ Ignored (Inspect)	Visit Preview	Feb 21, 2024 0:39am
examples-native-web	⬜️ Ignored (Inspect)	Visit Preview	Feb 21, 2024 0:39am
examples-nonmonorepo	⬜️ Ignored (Inspect)	Visit Preview	Feb 21, 2024 0:39am
examples-svelte-web	⬜️ Ignored (Inspect)	Visit Preview	Feb 21, 2024 0:39am
examples-tailwind-web	⬜️ Ignored (Inspect)	Visit Preview	Feb 21, 2024 0:39am
examples-vite-web	⬜️ Ignored (Inspect)	Visit Preview	Feb 21, 2024 0:39am
turbo-site	⬜️ Ignored (Inspect)	Visit Preview	Feb 21, 2024 0:39am

Zertsov · 2024-02-16T20:51:30Z

feature: Add invalidate flag to logout #7444
chore: move logout functionality into auth crate #7443
fix: More robust token validation #7407 👈
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Zertsov and the rest of your teammates on Graphite

github-actions · 2024-02-16T20:51:55Z

🟢 Turbopack Benchmark CI successful 🟢

Thanks

github-actions · 2024-02-16T21:51:57Z

🟢 CI successful 🟢

Thanks

arlyon · 2024-02-20T13:18:08Z

crates/turborepo-auth/src/auth/login.rs

@@ -267,6 +293,60 @@ mod tests {
        }
    }

+    #[async_trait]


Perhaps out of scope for this PR but as of rust 1.75 we don't need async_trait any more. If you need to specify things like Send (the default for async_trait traits) you can use return position impl instead.

async fn get_artifact(...) -> Result<Option<Response>, turborepo_api_client::Error> // or fn get_artifact(...) -> impl Future<Output=Result<Option<Response>, turborepo_api_client::Error>> + Send

The side effect being no more Pin<Box<dyn Future>> :)

See here: https://github.com/vercel/turbo/blob/main/crates/turborepo-repository/src/discovery.rs#L43-L57

I quickly tried to swap out [async_trait] but I had a bunch of errors around the Output not being a Future, so I'll migrate these in another PR

chris-olszewski

Nothing blocking, just a few thoughts.

chris-olszewski · 2024-02-21T17:42:16Z

crates/turborepo-auth/src/auth/sso.rs

+    let valid_token_callback = |message: &str, ui: &UI| {
+        let message = message.to_string();
+        let ui = *ui;
+        move |user_email: &str| {
+            println!("{}", ui.apply(BOLD.apply_to(message)));
+            ui::print_cli_authorized(user_email, &ui);
+        }
+    };


I think this might be too clever as opposed to having is_valid return a Result<Option<String>> which contains the email if the token is valid and None if it isn't valid.

I plan on doing some refactoring later to make it so is_valid can still just return a bool, since I think it's quicker and easier to grok than is_valid returning anything other than a bool

chris-olszewski · 2024-02-21T17:57:54Z

crates/turborepo-auth/src/lib.rs

+        let is_active = self.is_active(client).await?;
+        let has_cache_access = self.has_cache_access(client, None).await?;
+        if !is_active || !has_cache_access {


I think we can just do these at the same time

Suggested change

let is_active = self.is_active(client).await?;

let has_cache_access = self.has_cache_access(client, None).await?;

if !is_active || !has_cache_access {

let (is_active, has_cache_access) = tokio::try_join!(self.is_active(client), self.has_cache_access(client, None))?;

if !is_active || !has_cache_access {

chris-olszewski · 2024-02-21T17:59:37Z

crates/turborepo-auth/src/lib.rs

+    /// * `client` - The client to use for API calls.
+    /// * `valid_message_fn` - An optional callback that gets called if the
+    ///   token is valid. It will be passed the user's email.
+    // TODO(voz): This should do a `get_user` or `get_teams` instead of the caller


Does this TODO still apply now that we're calling self.user?

Nope, missed this before merging, so I'll clean it up somewhere in this stack.

Zertsov requested a review from a team as a code owner February 16, 2024 20:51

Zertsov requested review from gsoltis and tknickman February 16, 2024 20:51

turbo-orchestrator bot added created-by: turborepo owned-by: turborepo labels Feb 16, 2024

vercel bot deployed to Preview – rust-docs February 16, 2024 20:58 View deployment

Zertsov force-pushed the voz/more-validation branch from 1a8e694 to a39417b Compare February 16, 2024 21:00

vercel bot deployed to Preview – rust-docs February 16, 2024 21:10 View deployment

Zertsov force-pushed the voz/more-validation branch 2 times, most recently from 27d1cf5 to c148244 Compare February 16, 2024 21:17

vercel bot deployed to Preview – rust-docs February 16, 2024 21:28 View deployment

arlyon reviewed Feb 20, 2024

View reviewed changes

Zertsov force-pushed the voz/more-validation branch from c148244 to aa00b63 Compare February 20, 2024 17:04

vercel bot deployed to Preview – rust-docs February 20, 2024 17:12 View deployment

fix: More robust token validation

b25a81b

Zertsov force-pushed the voz/more-validation branch from aa00b63 to b25a81b Compare February 21, 2024 00:31

This was referenced Feb 21, 2024

chore: move logout functionality into auth crate #7443

Merged

feature: Add invalidate flag to logout #7444

Merged

vercel bot deployed to Preview – rust-docs February 21, 2024 00:39 View deployment

chris-olszewski approved these changes Feb 21, 2024

View reviewed changes

Zertsov merged commit 1745f9c into main Feb 21, 2024
60 checks passed

Zertsov deleted the voz/more-validation branch February 21, 2024 19:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: More robust token validation #7407

fix: More robust token validation #7407

Zertsov commented Feb 16, 2024 •

edited by padmaia

vercel bot commented Feb 16, 2024 •

edited

Zertsov commented Feb 16, 2024 •

edited

github-actions bot commented Feb 16, 2024 •

edited

github-actions bot commented Feb 16, 2024 •

edited

arlyon Feb 20, 2024

Zertsov Feb 20, 2024

chris-olszewski left a comment

chris-olszewski Feb 21, 2024

Zertsov Feb 21, 2024

chris-olszewski Feb 21, 2024

chris-olszewski Feb 21, 2024

Zertsov Feb 21, 2024

fix: More robust token validation #7407

fix: More robust token validation #7407

Conversation

Zertsov commented Feb 16, 2024 • edited by padmaia

Description

Testing Instructions

vercel bot commented Feb 16, 2024 • edited

Zertsov commented Feb 16, 2024 • edited

github-actions bot commented Feb 16, 2024 • edited

🟢 Turbopack Benchmark CI successful 🟢

github-actions bot commented Feb 16, 2024 • edited

🟢 CI successful 🟢

arlyon Feb 20, 2024

Choose a reason for hiding this comment

Zertsov Feb 20, 2024

Choose a reason for hiding this comment

chris-olszewski left a comment

Choose a reason for hiding this comment

chris-olszewski Feb 21, 2024

Choose a reason for hiding this comment

Zertsov Feb 21, 2024

Choose a reason for hiding this comment

chris-olszewski Feb 21, 2024

Choose a reason for hiding this comment

chris-olszewski Feb 21, 2024

Choose a reason for hiding this comment

Zertsov Feb 21, 2024

Choose a reason for hiding this comment

Zertsov commented Feb 16, 2024 •

edited by padmaia

vercel bot commented Feb 16, 2024 •

edited

Zertsov commented Feb 16, 2024 •

edited

github-actions bot commented Feb 16, 2024 •

edited

github-actions bot commented Feb 16, 2024 •

edited