From 9542e3a9f18bc33effa6e5d72aefa66b857d17e1 Mon Sep 17 00:00:00 2001 From: Glenn Griffin Date: Fri, 8 Nov 2019 13:40:34 -0800 Subject: [PATCH] Remove instances of cloning ApplicationSecret ApplicationSecret is not a small struct. This removes the instances where it's cloned in favor of passing a shared reference. --- src/authenticator.rs | 4 +-- src/authenticator_delegate.rs | 2 +- src/device.rs | 22 ++++++------ src/installed.rs | 64 +++++++++++++++++------------------ src/refresh.rs | 16 ++++----- src/service_account.rs | 5 +-- src/types.rs | 18 +++++++++- 7 files changed, 73 insertions(+), 58 deletions(-) diff --git a/src/authenticator.rs b/src/authenticator.rs index bbd6781..32f67ca 100644 --- a/src/authenticator.rs +++ b/src/authenticator.rs @@ -216,7 +216,7 @@ where let store = store.clone(); let rr = RefreshFlow::refresh_token( client.clone(), - appsecret.clone(), + appsecret, refresh_token.unwrap(), ) .await?; @@ -291,7 +291,7 @@ impl< self.inner.api_key() } /// Returns the application secret of the inner flow. - fn application_secret(&self) -> ApplicationSecret { + fn application_secret(&self) -> &ApplicationSecret { self.inner.application_secret() } diff --git a/src/authenticator_delegate.rs b/src/authenticator_delegate.rs index 5312c13..4c25969 100644 --- a/src/authenticator_delegate.rs +++ b/src/authenticator_delegate.rs @@ -133,7 +133,7 @@ pub trait FlowDelegate: Clone + Send + Sync { } /// Configure a custom redirect uri if needed. - fn redirect_uri(&self) -> Option { + fn redirect_uri(&self) -> Option<&str> { None } /// The server has returned a `user_code` which must be shown to the user, diff --git a/src/device.rs b/src/device.rs index 018bde7..c8573f2 100644 --- a/src/device.rs +++ b/src/device.rs @@ -114,8 +114,8 @@ where fn api_key(&self) -> Option { None } - fn application_secret(&self) -> ApplicationSecret { - self.application_secret.clone() + fn application_secret(&self) -> &ApplicationSecret { + &self.application_secret } } @@ -135,12 +135,12 @@ where where T: AsRef, { - let application_secret = self.application_secret.clone(); + let application_secret = &self.application_secret; let client = self.client.clone(); let wait = self.wait; let fd = self.fd.clone(); let (pollinf, device_code) = Self::request_code( - application_secret.clone(), + application_secret, client.clone(), self.device_code_url.clone(), scopes, @@ -153,7 +153,7 @@ where let pollinf = pollinf.clone(); tokio::timer::delay_for(pollinf.interval).await; let r = Self::poll_token( - application_secret.clone(), + application_secret, client.clone(), device_code.clone(), pollinf.clone(), @@ -194,7 +194,7 @@ where /// # Examples /// See test-cases in source code for a more complete example. async fn request_code( - application_secret: ApplicationSecret, + application_secret: &ApplicationSecret, client: hyper::Client, device_code_url: String, scopes: &[T], @@ -206,8 +206,8 @@ where // https://github.com/servo/rust-url/issues/81 let req = form_urlencoded::Serializer::new(String::new()) .extend_pairs(&[ - ("client_id", application_secret.client_id.clone()), - ("scope", crate::helper::join(scopes, " ")), + ("client_id", application_secret.client_id.as_str()), + ("scope", crate::helper::join(scopes, " ").as_str()), ]) .finish(); @@ -276,7 +276,7 @@ where /// # Examples /// See test-cases in source code for a more complete example. async fn poll_token<'a>( - application_secret: ApplicationSecret, + application_secret: &ApplicationSecret, client: hyper::Client, device_code: String, pi: PollInformation, @@ -290,8 +290,8 @@ where // We should be ready for a new request let req = form_urlencoded::Serializer::new(String::new()) .extend_pairs(&[ - ("client_id", &application_secret.client_id[..]), - ("client_secret", &application_secret.client_secret), + ("client_id", application_secret.client_id.as_str()), + ("client_secret", application_secret.client_secret.as_str()), ("code", &device_code), ("grant_type", "http://oauth.net/grant_type/device/1.0"), ]) diff --git a/src/installed.rs b/src/installed.rs index 26ee573..0f1db98 100644 --- a/src/installed.rs +++ b/src/installed.rs @@ -24,24 +24,17 @@ const OOB_REDIRECT_URI: &'static str = "urn:ietf:wg:oauth:2.0:oob"; /// Assembles a URL to request an authorization token (with user interaction). /// Note that the redirect_uri here has to be either None or some variation of /// http://localhost:{port}, or the authorization won't work (error "redirect_uri_mismatch") -fn build_authentication_request_url<'a, T, I>( +fn build_authentication_request_url( auth_uri: &str, client_id: &str, - scopes: I, - redirect_uri: Option, + scopes: &[T], + redirect_uri: Option<&str>, ) -> String where - T: AsRef + 'a, - I: IntoIterator, + T: AsRef, { let mut url = String::new(); - let mut scopes_string = scopes.into_iter().fold(String::new(), |mut acc, sc| { - acc.push_str(sc.as_ref()); - acc.push_str(" "); - acc - }); - // Remove last space - scopes_string.pop(); + let scopes_string = crate::helper::join(scopes, " "); url.push_str(auth_uri); vec![ @@ -49,7 +42,7 @@ where format!("&access_type=offline"), format!( "&redirect_uri={}", - redirect_uri.unwrap_or(OOB_REDIRECT_URI.to_string()) + redirect_uri.unwrap_or(OOB_REDIRECT_URI) ), format!("&response_type=code"), format!("&client_id={}", client_id), @@ -78,8 +71,8 @@ where fn api_key(&self) -> Option { None } - fn application_secret(&self) -> ApplicationSecret { - self.appsecret.clone() + fn application_secret(&self) -> &ApplicationSecret { + &self.appsecret } } @@ -232,6 +225,7 @@ where where T: AsRef, { + use std::borrow::Cow; let auth_delegate = &self.fd; let appsecret = &self.appsecret; let server = InstalledFlowServer::run(desired_port)?; @@ -240,13 +234,15 @@ where // Present url to user. // The redirect URI must be this very localhost URL, otherwise authorization is refused // by certain providers. + let redirect_uri: Cow = match auth_delegate.redirect_uri() { + Some(uri) => uri.into(), + None => format!("http://localhost:{}", bound_port).into(), + }; let url = build_authentication_request_url( &appsecret.auth_uri, &appsecret.client_id, scopes, - auth_delegate - .redirect_uri() - .or_else(|| Some(format!("http://localhost:{}", bound_port))), + Some(redirect_uri.as_ref()), ); let _ = auth_delegate .present_user_url(&url, false /* need code */) @@ -262,8 +258,8 @@ where port: Option, ) -> Result { let appsec = &self.appsecret; - let redirect_uri = &self.fd.redirect_uri(); - let request = Self::request_token(appsec.clone(), authcode, redirect_uri.clone(), port); + let redirect_uri = self.fd.redirect_uri(); + let request = Self::request_token(appsec, authcode, redirect_uri, port); let resp = self .client .request(request) @@ -310,27 +306,29 @@ where /// Sends the authorization code to the provider in order to obtain access and refresh tokens. fn request_token<'a>( - appsecret: ApplicationSecret, + appsecret: &ApplicationSecret, authcode: String, - custom_redirect_uri: Option, + custom_redirect_uri: Option<&str>, port: Option, ) -> hyper::Request { - let redirect_uri = custom_redirect_uri.unwrap_or_else(|| match port { - None => OOB_REDIRECT_URI.to_string(), - Some(port) => format!("http://localhost:{}", port), - }); + use std::borrow::Cow; + let redirect_uri: Cow = match (custom_redirect_uri, port) { + (Some(uri), _) => uri.into(), + (None, Some(port)) => format!("http://localhost:{}", port).into(), + (None, None) => OOB_REDIRECT_URI.into(), + }; let body = form_urlencoded::Serializer::new(String::new()) .extend_pairs(vec![ - ("code".to_string(), authcode.to_string()), - ("client_id".to_string(), appsecret.client_id.clone()), - ("client_secret".to_string(), appsecret.client_secret.clone()), - ("redirect_uri".to_string(), redirect_uri), - ("grant_type".to_string(), "authorization_code".to_string()), + ("code", authcode.as_str()), + ("client_id", appsecret.client_id.as_str()), + ("client_secret", appsecret.client_secret.as_str()), + ("redirect_uri", redirect_uri.as_ref()), + ("grant_type", "authorization_code"), ]) .finish(); - let request = hyper::Request::post(appsecret.token_uri) + let request = hyper::Request::post(&appsecret.token_uri) .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") .body(hyper::Body::from(body)) .unwrap(); // TODO: error check @@ -657,7 +655,7 @@ mod tests { "https://accounts.google.com/o/oauth2/auth", "812741506391-h38jh0j4fv0ce1krdkiq0hfvt6n5am\ rf.apps.googleusercontent.com", - vec![&"email".to_string(), &"profile".to_string()], + &["email", "profile"], None ) ); diff --git a/src/refresh.rs b/src/refresh.rs index 717016f..da5bced 100644 --- a/src/refresh.rs +++ b/src/refresh.rs @@ -32,20 +32,20 @@ impl RefreshFlow { /// Please see the crate landing page for an example. pub async fn refresh_token( client: hyper::Client, - client_secret: ApplicationSecret, + client_secret: &ApplicationSecret, refresh_token: String, ) -> Result { // TODO: Does this function ever return RequestError? Maybe have it just return RefreshResult. let req = form_urlencoded::Serializer::new(String::new()) .extend_pairs(&[ - ("client_id", client_secret.client_id.clone()), - ("client_secret", client_secret.client_secret.clone()), - ("refresh_token", refresh_token.to_string()), - ("grant_type", "refresh_token".to_string()), + ("client_id", client_secret.client_id.as_str()), + ("client_secret", client_secret.client_secret.as_str()), + ("refresh_token", refresh_token.as_str()), + ("grant_type", "refresh_token"), ]) .finish(); - let request = hyper::Request::post(client_secret.token_uri.clone()) + let request = hyper::Request::post(&client_secret.token_uri) .header(header::CONTENT_TYPE, "application/x-www-form-urlencoded") .body(hyper::Body::from(req)) .unwrap(); // TODO: error handling @@ -130,7 +130,7 @@ mod tests { let fut = async { let rr = RefreshFlow::refresh_token( client.clone(), - app_secret.clone(), + &app_secret, refresh_token.clone(), ) .await @@ -158,7 +158,7 @@ mod tests { .create(); let fut = async { - let rr = RefreshFlow::refresh_token(client, app_secret, refresh_token) + let rr = RefreshFlow::refresh_token(client, &app_secret, refresh_token) .await .unwrap(); match rr { diff --git a/src/service_account.rs b/src/service_account.rs index 88831c8..0c2298d 100644 --- a/src/service_account.rs +++ b/src/service_account.rs @@ -382,8 +382,9 @@ where /// Returns an empty ApplicationSecret as tokens for service accounts don't need to be /// refreshed (they are simply reissued). - fn application_secret(&self) -> ApplicationSecret { - Default::default() + fn application_secret(&self) -> &ApplicationSecret { + static APP_SECRET: ApplicationSecret = ApplicationSecret::empty(); + &APP_SECRET } fn api_key(&self) -> Option { diff --git a/src/types.rs b/src/types.rs index 5d5b54f..697ddb0 100644 --- a/src/types.rs +++ b/src/types.rs @@ -247,7 +247,7 @@ pub trait GetToken: Send + Sync { /// Return an application secret with at least token_uri, client_secret, and client_id filled /// in. This is used for refreshing tokens without interaction from the flow. - fn application_secret(&self) -> ApplicationSecret; + fn application_secret(&self) -> &ApplicationSecret; } /// Represents a token as returned by OAuth2 servers. @@ -342,6 +342,22 @@ pub struct ApplicationSecret { pub client_x509_cert_url: Option, } +impl ApplicationSecret { + pub const fn empty() -> Self { + ApplicationSecret{ + client_id: String::new(), + client_secret: String::new(), + token_uri: String::new(), + auth_uri: String::new(), + redirect_uris: Vec::new(), + project_id: None, + client_email: None, + auth_provider_x509_cert_url: None, + client_x509_cert_url: None, + } + } +} + /// A type to facilitate reading and writing the json secret file /// as returned by the [google developer console](https://code.google.com/apis/console) #[derive(Deserialize, Serialize, Default)]