Merge pull request #1999 from GitoxideLabs/credential-helper-protocol-fix

fix #1998

Author: Byron

Cargo.lock

@@ -25,6 +25,7 @@ gix-path = { version = "^0.10.18", path = "../gix-path" }
 gix-command = { version = "^0.6.0", path = "../gix-command" }
 gix-config-value = { version = "^0.15.0", path = "../gix-config-value" }
 gix-prompt = { version = "^0.11.0", path = "../gix-prompt" }
+gix-date = { version = "^0.10.1", path = "../gix-date" }
 gix-trace = { version = "^0.1.12", path = "../gix-trace" }
 
 thiserror = "2.0.0"

gix-credentials/Cargo.toml

@@ -88,30 +88,51 @@ impl Cascade {
             match helper::invoke::raw(program, &action) {
                 Ok(None) => {}
                 Ok(Some(stdout)) => {
-                    let ctx = Context::from_bytes(&stdout)?;
+                    let Context {
+                        protocol,
+                        host,
+                        path,
+                        username,
+                        password,
+                        oauth_refresh_token,
+                        password_expiry_utc,
+                        url: ctx_url,
+                        quit,
+                    } = Context::from_bytes(&stdout)?;
                     if let Some(dst_ctx) = action.context_mut() {
-                        if let Some(src) = ctx.path {
+                        if let Some(src) = path {
                             dst_ctx.path = Some(src);
                         }
+                        if let Some(src) = password_expiry_utc {
+                            dst_ctx.password_expiry_utc = Some(src);
+                        }
                         for (src, dst) in [
-                            (ctx.protocol, &mut dst_ctx.protocol),
-                            (ctx.host, &mut dst_ctx.host),
-                            (ctx.username, &mut dst_ctx.username),
-                            (ctx.password, &mut dst_ctx.password),
+                            (protocol, &mut dst_ctx.protocol),
+                            (host, &mut dst_ctx.host),
+                            (username, &mut dst_ctx.username),
+                            (password, &mut dst_ctx.password),
+                            (oauth_refresh_token, &mut dst_ctx.oauth_refresh_token),
                         ] {
                             if let Some(src) = src {
                                 *dst = Some(src);
                             }
                         }
-                        if let Some(src) = ctx.url {
+                        if let Some(src) = ctx_url {
                             dst_ctx.url = Some(src);
                             url = dst_ctx.destructure_url_in_place(self.use_http_path)?.url.take();
                         }
+                        if dst_ctx
+                            .password_expiry_utc
+                            .is_some_and(|expiry_date| expiry_date < gix_date::Time::now_utc().seconds)
+                        {
+                            dst_ctx.password_expiry_utc = None;
+                            dst_ctx.clear_secrets();
+                        }
                         if dst_ctx.username.is_some() && dst_ctx.password.is_some() {
                             break;
                         }
-                        if ctx.quit.unwrap_or_default() {
-                            dst_ctx.quit = ctx.quit;
+                        if quit.unwrap_or_default() {
+                            dst_ctx.quit = quit;
                             break;
                         }
                     }
@@ -152,6 +173,7 @@ impl Cascade {
             action.context().map(|ctx| helper::Outcome {
                 username: ctx.username.clone(),
                 password: ctx.password.clone(),
+                oauth_refresh_token: ctx.oauth_refresh_token.clone(),
                 quit: ctx.quit.unwrap_or(false),
                 next: ctx.to_owned().into(),
             }),

gix-credentials/src/helper/cascade.rs

@@ -30,6 +30,7 @@ pub fn invoke(helper: &mut crate::Program, action: &Action) -> Result {
             Ok(Some(Outcome {
                 username: ctx.username,
                 password: ctx.password,
+                oauth_refresh_token: ctx.oauth_refresh_token,
                 quit: ctx.quit.unwrap_or(false),
                 next: NextAction {
                     previous_output: stdout.into(),

gix-credentials/src/helper/invoke.rs

@@ -25,6 +25,8 @@ pub struct Outcome {
     pub username: Option<String>,
     /// The password to use in the identity, if set.
     pub password: Option<String>,
+    /// An OAuth refresh token that may accompany a password. It is to be treated confidentially, just like the password.
+    pub oauth_refresh_token: Option<String>,
     /// If set, the helper asked to stop the entire process, whether the identity is complete or not.
     pub quit: bool,
     /// A handle to the action to perform next in another call to [`helper::invoke()`][crate::helper::invoke()].
@@ -42,7 +44,11 @@ impl Outcome {
         self.username
             .take()
             .zip(self.password.take())
-            .map(|(username, password)| gix_sec::identity::Account { username, password })
+            .map(|(username, password)| gix_sec::identity::Account {
+                username,
+                password,
+                oauth_refresh_token: self.oauth_refresh_token.take(),
+            })
     }
 }
 
@@ -131,7 +137,7 @@ impl Action {
     }
 }
 
-/// A handle to [store][NextAction::store()] or [erase][NextAction::erase()] the outcome of the initial action.
+/// A handle to [store](NextAction::store()) or [erase](NextAction::erase()) the outcome of the initial action.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct NextAction {
     previous_output: BString,

gix-credentials/src/helper/mod.rs

@@ -14,6 +14,42 @@ mod access {
     use crate::protocol::Context;
 
     impl Context {
+        /// Clear all fields that are considered secret.
+        pub fn clear_secrets(&mut self) {
+            let Context {
+                protocol: _,
+                host: _,
+                path: _,
+                username: _,
+                password,
+                oauth_refresh_token,
+                password_expiry_utc: _,
+                url: _,
+                quit: _,
+            } = self;
+
+            *password = None;
+            *oauth_refresh_token = None;
+        }
+        /// Replace existing secrets with the word `<redacted>`.
+        pub fn redacted(mut self) -> Self {
+            let Context {
+                protocol: _,
+                host: _,
+                path: _,
+                username: _,
+                password,
+                oauth_refresh_token,
+                password_expiry_utc: _,
+                url: _,
+                quit: _,
+            } = &mut self;
+            for secret in [password, oauth_refresh_token].into_iter().flatten() {
+                *secret = "<redacted>".into();
+            }
+            self
+        }
+
         /// Convert all relevant fields into a URL for consumption.
         pub fn to_url(&self) -> Option<BString> {
             use bstr::{ByteSlice, ByteVec};

gix-credentials/src/protocol/context/mod.rs

@@ -17,25 +17,46 @@ mod write {
                 out.write_all(value)?;
                 out.write_all(b"\n")
             }
-            for (key, value) in [("url", &self.url), ("path", &self.path)] {
+            let Context {
+                protocol,
+                host,
+                path,
+                username,
+                password,
+                oauth_refresh_token,
+                password_expiry_utc,
+                url,
+                // We only decode quit and interpret it, but won't get to pass it on as it means to stop the
+                // credential helper invocation chain.
+                quit: _,
+            } = self;
+            for (key, value) in [("url", url), ("path", path)] {
                 if let Some(value) = value {
                     validate(key, value.as_slice().into())
                         .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
                     write_key(&mut out, key, value.as_ref()).ok();
                 }
             }
             for (key, value) in [
-                ("protocol", &self.protocol),
-                ("host", &self.host),
-                ("username", &self.username),
-                ("password", &self.password),
+                ("protocol", protocol),
+                ("host", host),
+                ("username", username),
+                ("password", password),
+                ("oauth_refresh_token", oauth_refresh_token),
             ] {
                 if let Some(value) = value {
                     validate(key, value.as_str().into())
                         .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
                     write_key(&mut out, key, value.as_bytes().as_bstr()).ok();
                 }
             }
+            if let Some(value) = password_expiry_utc {
+                let key = "password_expiry_utc";
+                let value = value.to_string();
+                validate(key, value.as_str().into())
+                    .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
+                write_key(&mut out, key, value.as_bytes().as_bstr()).ok();
+            }
             Ok(())
         }
 
@@ -70,6 +91,17 @@ pub mod decode {
         /// Decode ourselves from `input` which is the format written by [`write_to()`][Self::write_to()].
         pub fn from_bytes(input: &[u8]) -> Result<Self, Error> {
             let mut ctx = Context::default();
+            let Context {
+                protocol,
+                host,
+                path,
+                username,
+                password,
+                oauth_refresh_token,
+                password_expiry_utc,
+                url,
+                quit,
+            } = &mut ctx;
             for res in input.lines().take_while(|line| !line.is_empty()).map(|line| {
                 let mut it = line.splitn(2, |b| *b == b'=');
                 match (
@@ -84,23 +116,27 @@ pub mod decode {
             }) {
                 let (key, value) = res?;
                 match key {
-                    "protocol" | "host" | "username" | "password" => {
+                    "protocol" | "host" | "username" | "password" | "oauth_refresh_token" => {
                         if !value.is_utf8() {
                             return Err(Error::IllformedUtf8InValue { key: key.into(), value });
                         }
                         let value = value.to_string();
                         *match key {
-                            "protocol" => &mut ctx.protocol,
-                            "host" => &mut ctx.host,
-                            "username" => &mut ctx.username,
-                            "password" => &mut ctx.password,
+                            "protocol" => &mut *protocol,
+                            "host" => host,
+                            "username" => username,
+                            "password" => password,
+                            "oauth_refresh_token" => oauth_refresh_token,
                             _ => unreachable!("checked field names in match above"),
                         } = Some(value);
                     }
-                    "url" => ctx.url = Some(value),
-                    "path" => ctx.path = Some(value),
+                    "password_expiry_utc" => {
+                        *password_expiry_utc = value.to_str().ok().and_then(|value| value.parse().ok());
+                    }
+                    "url" => *url = Some(value),
+                    "path" => *path = Some(value),
                     "quit" => {
-                        ctx.quit = gix_config_value::Boolean::try_from(value.as_ref()).ok().map(Into::into);
+                        *quit = gix_config_value::Boolean::try_from(value.as_ref()).ok().map(Into::into);
                     }
                     _ => {}
                 }

gix-credentials/src/protocol/context/serde.rs

@@ -48,6 +48,10 @@ pub struct Context {
     pub username: Option<String>,
     /// The credential’s password, if we are asking it to be stored.
     pub password: Option<String>,
+    /// An OAuth refresh token that may accompany a password. It is to be treated confidentially, just like the password.
+    pub oauth_refresh_token: Option<String>,
+    /// The expiry date of OAuth tokens as seconds from Unix epoch.
+    pub password_expiry_utc: Option<gix_date::SecondsSinceUnixEpoch>,
     /// When this special attribute is read by git credential, the value is parsed as a URL and treated as if its constituent
     /// parts were read (e.g., url=<https://example.com> would behave as if
     /// protocol=https and host=example.com had been provided). This can help callers avoid parsing URLs themselves.
@@ -59,14 +63,10 @@ pub struct Context {
 /// Convert the outcome of a helper invocation to a helper result, assuring that the identity is complete in the process.
 #[allow(clippy::result_large_err)]
 pub fn helper_outcome_to_result(outcome: Option<helper::Outcome>, action: helper::Action) -> Result {
-    fn redact(mut ctx: Context) -> Context {
-        if let Some(pw) = ctx.password.as_mut() {
-            *pw = "<redacted>".into();
-        }
-        ctx
-    }
     match (action, outcome) {
-        (helper::Action::Get(ctx), None) => Err(Error::IdentityMissing { context: redact(ctx) }),
+        (helper::Action::Get(ctx), None) => Err(Error::IdentityMissing {
+            context: ctx.redacted(),
+        }),
         (helper::Action::Get(ctx), Some(mut outcome)) => match outcome.consume_identity() {
             Some(identity) => Ok(Some(Outcome {
                 identity,
@@ -75,7 +75,9 @@ pub fn helper_outcome_to_result(outcome: Option<helper::Outcome>, action: helper
             None => Err(if outcome.quit {
                 Error::Quit
             } else {
-                Error::IdentityMissing { context: redact(ctx) }
+                Error::IdentityMissing {
+                    context: ctx.redacted(),
+                }
             }),
         },
         (helper::Action::Store(_) | helper::Action::Erase(_), _ignore) => Ok(None),

gix-credentials/src/protocol/mod.rs

@@ -0,0 +1,7 @@
+#!/usr/bin/env bash
+set -eu
+
+test "$1" = get && \
+echo username=user-expired && \
+echo password=pass-expired && \
+echo password_expiry_utc=1

gix-credentials/tests/fixtures/expired.sh

@@ -0,0 +1,5 @@
+#!/usr/bin/env bash
+set -eu
+
+test "$1" = get && \
+echo oauth_refresh_token=oauth-token

gix-credentials/tests/fixtures/oauth-token.sh

@@ -119,6 +119,25 @@ mod invoke {
         assert_eq!(actual.identity, identity("user", "pass"));
     }
 
+    #[test]
+    fn expired_credentials_are_not_returned() {
+        let actual = invoke_cascade(
+            ["expired", "oauth-token", "custom-helper"],
+            Action::get_for_url("http://github.com"),
+        )
+        .unwrap()
+        .expect("credentials");
+
+        assert_eq!(
+            actual.identity,
+            Account {
+                oauth_refresh_token: Some("oauth-token".into()),
+                ..identity("user-script", "pass-script")
+            },
+            "it ignored the expired password, which otherwise would have come first"
+        );
+    }
+
     #[test]
     fn bogus_password_overrides_any_helper_and_helper_overrides_username_in_url() {
         let actual = Cascade::default()
@@ -144,6 +163,7 @@ mod invoke {
         Account {
             username: user.into(),
             password: pass.into(),
+            oauth_refresh_token: None,
         }
     }