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.
This commit is contained in:
Renar Narubin
2022-05-18 21:05:37 -07:00
parent 29a72447f6
commit aacb97d76c
2 changed files with 29 additions and 17 deletions

View File

@@ -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,

View File

@@ -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()