From aacb97d76c1c40e6b6d25b324502154cd8db415f Mon Sep 17 00:00:00 2001
From: Renar Narubin <r.narubin@gmail.com>
Date: Wed, 18 May 2022 21:05:37 -0700
Subject: [PATCH] Remove file IO panic in ApplicationDefaultCreds flow

The `from_environment` function in
`ApplicationDefaultCredentialsAuthenticator` had an `unwrap` call on an
io::Result after reading the service account key from file. File
operations are inherently fallible, and panicking on such a failure is
generally a bad convention compared to propagating the IO error.

Propagating that error from the `from_environment` function is not
practical however, because the returned Result type does not include IO
errors, and changing the function signature would be semver
incompatible.

This change instead defers reading the key file to a later function
call. Now `from_environment` only reads the value of the
`GOOGLE_APPLICATION_CREDENTIALS` into a PathBuf, and a later call to
`ServiceAccountFlow::new` will actually read the file. That constructor
already returns an io::Result, so folding the read error into it is
possible, and none of the changes impact public items so it's all
semver-compatible.
---
 src/authenticator.rs   | 13 +++++--------
 src/service_account.rs | 33 ++++++++++++++++++++++++---------
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/src/authenticator.rs b/src/authenticator.rs
index 79296a5..0918d44 100644
--- a/src/authenticator.rs
+++ b/src/authenticator.rs
@@ -10,7 +10,7 @@ use crate::installed::{InstalledFlow, InstalledFlowReturnMethod};
 use crate::refresh::RefreshFlow;
 
 #[cfg(feature = "service_account")]
-use crate::service_account::{ServiceAccountFlow, ServiceAccountFlowOpts, ServiceAccountKey};
+use crate::service_account::{self, ServiceAccountFlow, ServiceAccountFlowOpts, ServiceAccountKey};
 use crate::storage::{self, Storage, TokenStorage};
 use crate::types::{AccessToken, ApplicationSecret, TokenInfo};
 use private::AuthFlow;
@@ -278,7 +278,7 @@ impl ServiceAccountAuthenticator {
     ) -> AuthenticatorBuilder<C, ServiceAccountFlowOpts> {
         AuthenticatorBuilder::new(
             ServiceAccountFlowOpts {
-                key: service_account_key,
+                key: service_account::FlowOptsKey::Key(service_account_key),
                 subject: None,
             },
             client,
@@ -312,13 +312,10 @@ impl ApplicationDefaultCredentialsAuthenticator {
     /// Try to build ServiceAccountFlowOpts from the environment
     #[cfg(feature = "service_account")]
     pub async fn from_environment() -> Result<ServiceAccountFlowOpts, std::env::VarError> {
-        let service_account_key =
-            crate::read_service_account_key(std::env::var("GOOGLE_APPLICATION_CREDENTIALS")?)
-                .await
-                .unwrap();
+        let key_path = std::env::var("GOOGLE_APPLICATION_CREDENTIALS")?;
 
         Ok(ServiceAccountFlowOpts {
-            key: service_account_key,
+            key: service_account::FlowOptsKey::Path(key_path.into()),
             subject: None,
         })
     }
@@ -626,7 +623,7 @@ impl<C> AuthenticatorBuilder<C, ServiceAccountFlowOpts> {
     where
         C: HyperClientBuilder,
     {
-        let service_account_auth_flow = ServiceAccountFlow::new(self.auth_flow)?;
+        let service_account_auth_flow = ServiceAccountFlow::new(self.auth_flow).await?;
         Self::common_build(
             self.hyper_client_builder,
             self.storage_type,
diff --git a/src/service_account.rs b/src/service_account.rs
index a659f8d..e846787 100644
--- a/src/service_account.rs
+++ b/src/service_account.rs
@@ -16,7 +16,7 @@
 use crate::error::Error;
 use crate::types::TokenInfo;
 
-use std::io;
+use std::{io, path::PathBuf};
 
 use hyper::header;
 use rustls::{
@@ -158,10 +158,18 @@ impl JWTSigner {
 }
 
 pub struct ServiceAccountFlowOpts {
-    pub(crate) key: ServiceAccountKey,
+    pub(crate) key: FlowOptsKey,
     pub(crate) subject: Option<String>,
 }
 
+/// The source of the key given to ServiceAccountFlowOpts.
+pub(crate) enum FlowOptsKey {
+    /// A path at which the key can be read from disk
+    Path(PathBuf),
+    /// An already initialized key
+    Key(ServiceAccountKey),
+}
+
 /// ServiceAccountFlow can fetch oauth tokens using a service account.
 pub struct ServiceAccountFlow {
     key: ServiceAccountKey,
@@ -170,10 +178,15 @@ pub struct ServiceAccountFlow {
 }
 
 impl ServiceAccountFlow {
-    pub(crate) fn new(opts: ServiceAccountFlowOpts) -> Result<Self, io::Error> {
-        let signer = JWTSigner::new(&opts.key.private_key)?;
+    pub(crate) async fn new(opts: ServiceAccountFlowOpts) -> Result<Self, io::Error> {
+        let key = match opts.key {
+            FlowOptsKey::Path(path) => crate::read_service_account_key(path).await?,
+            FlowOptsKey::Key(key) => key,
+        };
+
+        let signer = JWTSigner::new(&key.private_key)?;
         Ok(ServiceAccountFlow {
-            key: opts.key,
+            key,
             subject: opts.subject,
             signer,
         })
@@ -223,10 +236,12 @@ mod tests {
     //#[tokio::test]
     #[allow(dead_code)]
     async fn test_service_account_e2e() {
-        let key = read_service_account_key(TEST_PRIVATE_KEY_PATH)
-            .await
-            .unwrap();
-        let acc = ServiceAccountFlow::new(ServiceAccountFlowOpts { key, subject: None }).unwrap();
+        let acc = ServiceAccountFlow::new(ServiceAccountFlowOpts {
+            key: FlowOptsKey::Path(TEST_PRIVATE_KEY_PATH.into()),
+            subject: None,
+        })
+        .await
+        .unwrap();
         let client = hyper::Client::builder().build(
             hyper_rustls::HttpsConnectorBuilder::new()
                 .with_native_roots()