From d63396a7409feeebb68da828fb0cec71dd5df1bf Mon Sep 17 00:00:00 2001 From: Glenn Griffin Date: Thu, 21 Nov 2019 09:29:43 -0800 Subject: [PATCH] Split FlowDelegate into DeviceFlowDelegate and InstalledFlowDelegate. Each flow invokes a non-overlapping set of methods. There doesn't appear to be any benefit in having both flows use a common trait. The benefit of splitting the traits is that it makes it clear which methods need to be updated for each flow type where previously comments were required to communicate that information. --- src/authenticator.rs | 16 +++++++------- src/authenticator_delegate.rs | 39 ++++++++++++++++++++--------------- src/device.rs | 14 +++++++------ src/installed.rs | 12 +++++------ 4 files changed, 44 insertions(+), 37 deletions(-) diff --git a/src/authenticator.rs b/src/authenticator.rs index 5546739..af43e14 100644 --- a/src/authenticator.rs +++ b/src/authenticator.rs @@ -1,6 +1,6 @@ //! Module contianing the core functionality for OAuth2 Authentication. use crate::authenticator_delegate::{ - AuthenticatorDelegate, DefaultAuthenticatorDelegate, FlowDelegate, + AuthenticatorDelegate, DefaultAuthenticatorDelegate, DeviceFlowDelegate, InstalledFlowDelegate, }; use crate::device::DeviceFlow; use crate::error::Error; @@ -87,7 +87,7 @@ pub struct AuthenticatorBuilder { /// ``` /// # async fn foo() { /// # use yup_oauth2::InstalledFlowReturnMethod; -/// # let custom_flow_delegate = yup_oauth2::authenticator_delegate::DefaultFlowDelegate; +/// # let custom_flow_delegate = yup_oauth2::authenticator_delegate::DefaultInstalledFlowDelegate; /// # let app_secret = yup_oauth2::read_application_secret("/tmp/foo").await.unwrap(); /// let authenticator = yup_oauth2::InstalledFlowAuthenticator::builder( /// app_secret, @@ -240,7 +240,7 @@ impl AuthenticatorBuilder { /// ## Methods available when building a device flow Authenticator. /// ``` /// # async fn foo() { -/// # let custom_flow_delegate = yup_oauth2::authenticator_delegate::DefaultFlowDelegate; +/// # let custom_flow_delegate = yup_oauth2::authenticator_delegate::DefaultDeviceFlowDelegate; /// # let app_secret = yup_oauth2::read_application_secret("/tmp/foo").await.unwrap(); /// let authenticator = yup_oauth2::DeviceFlowAuthenticator::builder(app_secret) /// .device_code_url("foo") @@ -264,8 +264,8 @@ impl AuthenticatorBuilder { } } - /// Use the provided FlowDelegate. - pub fn flow_delegate(self, flow_delegate: Box) -> Self { + /// Use the provided DeviceFlowDelegate. + pub fn flow_delegate(self, flow_delegate: Box) -> Self { AuthenticatorBuilder { auth_flow: DeviceFlow { flow_delegate, @@ -316,7 +316,7 @@ impl AuthenticatorBuilder { /// ``` /// # async fn foo() { /// # use yup_oauth2::InstalledFlowReturnMethod; -/// # let custom_flow_delegate = yup_oauth2::authenticator_delegate::DefaultFlowDelegate; +/// # let custom_flow_delegate = yup_oauth2::authenticator_delegate::DefaultInstalledFlowDelegate; /// # let app_secret = yup_oauth2::read_application_secret("/tmp/foo").await.unwrap(); /// let authenticator = yup_oauth2::InstalledFlowAuthenticator::builder( /// app_secret, @@ -329,8 +329,8 @@ impl AuthenticatorBuilder { /// # } /// ``` impl AuthenticatorBuilder { - /// Use the provided FlowDelegate. - pub fn flow_delegate(self, flow_delegate: Box) -> Self { + /// Use the provided InstalledFlowDelegate. + pub fn flow_delegate(self, flow_delegate: Box) -> Self { AuthenticatorBuilder { auth_flow: InstalledFlow { flow_delegate, diff --git a/src/authenticator_delegate.rs b/src/authenticator_delegate.rs index 3ebb225..51650c4 100644 --- a/src/authenticator_delegate.rs +++ b/src/authenticator_delegate.rs @@ -75,9 +75,9 @@ pub trait AuthenticatorDelegate: Send + Sync { fn token_refresh_failed(&self, _: &RefreshError) {} } -/// FlowDelegate methods are called when an OAuth flow needs to ask the application what to do in -/// certain cases. -pub trait FlowDelegate: Send + Sync { +/// DeviceFlowDelegate methods are called when a device flow needs to ask the +/// application what to do in certain cases. +pub trait DeviceFlowDelegate: Send + Sync { /// Called if the request code is expired. You will have to start over in this case. /// This will be the last call the delegate receives. /// Given `DateTime` is the expiration date @@ -91,22 +91,14 @@ pub trait FlowDelegate: Send + Sync { /// Can be used to print progress information, or decide to time-out. /// /// If the returned `Retry` variant is a duration. - /// # Notes - /// * Only used in `DeviceFlow`. Return value will only be used if it - /// is larger than the interval desired by the server. fn pending(&self, _: &PollInformation) -> Retry { Retry::After(Duration::from_secs(5)) } - /// Configure a custom redirect uri if needed. - fn redirect_uri(&self) -> Option<&str> { - None - } /// The server has returned a `user_code` which must be shown to the user, /// along with the `verification_url`. /// # Notes /// * Will be called exactly once, provided we didn't abort during `request_code` phase. - /// * Will only be called if the Authenticator's flow_type is `DeviceFlow`. fn present_user_code(&self, pi: &PollInformation) { println!( "Please enter {} at {} and grant access to this application", @@ -118,8 +110,16 @@ pub trait FlowDelegate: Send + Sync { pi.expires_at.with_timezone(&Local) ); } +} + +/// InstalledFlowDelegate methods are called when an installed flow needs to ask +/// the application what to do in certain cases. +pub trait InstalledFlowDelegate: Send + Sync { + /// Configure a custom redirect uri if needed. + fn redirect_uri(&self) -> Option<&str> { + None + } - /// This method is used by the InstalledFlow. /// We need the user to navigate to a URL using their browser and potentially paste back a code /// (or maybe not). Whether they have to enter a code depends on the InstalledFlowReturnMethod /// used. @@ -167,11 +167,16 @@ async fn present_user_url( /// Uses all default implementations by AuthenticatorDelegate, and makes the trait's /// implementation usable in the first place. -#[derive(Clone)] +#[derive(Copy, Clone)] pub struct DefaultAuthenticatorDelegate; impl AuthenticatorDelegate for DefaultAuthenticatorDelegate {} -/// Uses all default implementations in the FlowDelegate trait. -#[derive(Clone)] -pub struct DefaultFlowDelegate; -impl FlowDelegate for DefaultFlowDelegate {} +/// Uses all default implementations in the DeviceFlowDelegate trait. +#[derive(Copy, Clone)] +pub struct DefaultDeviceFlowDelegate; +impl DeviceFlowDelegate for DefaultDeviceFlowDelegate {} + +/// Uses all default implementations in the DeviceFlowDelegate trait. +#[derive(Copy, Clone)] +pub struct DefaultInstalledFlowDelegate; +impl InstalledFlowDelegate for DefaultInstalledFlowDelegate {} diff --git a/src/device.rs b/src/device.rs index d2758a6..f61e092 100644 --- a/src/device.rs +++ b/src/device.rs @@ -1,4 +1,6 @@ -use crate::authenticator_delegate::{DefaultFlowDelegate, FlowDelegate, PollInformation, Retry}; +use crate::authenticator_delegate::{ + DefaultDeviceFlowDelegate, DeviceFlowDelegate, PollInformation, Retry, +}; use crate::error::{Error, JsonErrorOr, PollError}; use crate::types::{ApplicationSecret, Token}; @@ -24,7 +26,7 @@ pub const GOOGLE_GRANT_TYPE: &str = "http://oauth.net/grant_type/device/1.0"; pub struct DeviceFlow { pub(crate) app_secret: ApplicationSecret, pub(crate) device_code_url: Cow<'static, str>, - pub(crate) flow_delegate: Box, + pub(crate) flow_delegate: Box, pub(crate) wait_duration: Duration, pub(crate) grant_type: Cow<'static, str>, } @@ -36,7 +38,7 @@ impl DeviceFlow { DeviceFlow { app_secret, device_code_url: GOOGLE_DEVICE_CODE_URL.into(), - flow_delegate: Box::new(DefaultFlowDelegate), + flow_delegate: Box::new(DefaultDeviceFlowDelegate), wait_duration: Duration::from_secs(120), grant_type: GOOGLE_GRANT_TYPE.into(), } @@ -93,7 +95,7 @@ impl DeviceFlow { device_code, grant_type, pollinf.expires_at, - &*self.flow_delegate as &dyn FlowDelegate, + &*self.flow_delegate as &dyn DeviceFlowDelegate, ) .await { @@ -203,7 +205,7 @@ impl DeviceFlow { device_code: &str, grant_type: &str, expires_at: DateTime, - flow_delegate: &dyn FlowDelegate, + flow_delegate: &dyn DeviceFlowDelegate, ) -> Result, PollError> where C: hyper::client::connect::Connect + 'static, @@ -278,7 +280,7 @@ mod tests { async fn test_device_end2end() { #[derive(Clone)] struct FD; - impl FlowDelegate for FD { + impl DeviceFlowDelegate for FD { fn present_user_code(&self, pi: &PollInformation) { assert_eq!("https://example.com/verify", pi.verification_url); } diff --git a/src/installed.rs b/src/installed.rs index 81528f9..25f6549 100644 --- a/src/installed.rs +++ b/src/installed.rs @@ -2,7 +2,7 @@ // // Refer to the project root for licensing information. // -use crate::authenticator_delegate::{DefaultFlowDelegate, FlowDelegate}; +use crate::authenticator_delegate::{DefaultInstalledFlowDelegate, InstalledFlowDelegate}; use crate::error::{Error, JsonErrorOr}; use crate::types::{ApplicationSecret, Token}; @@ -68,7 +68,7 @@ pub enum InstalledFlowReturnMethod { pub struct InstalledFlow { pub(crate) app_secret: ApplicationSecret, pub(crate) method: InstalledFlowReturnMethod, - pub(crate) flow_delegate: Box, + pub(crate) flow_delegate: Box, } impl InstalledFlow { @@ -80,7 +80,7 @@ impl InstalledFlow { InstalledFlow { app_secret, method, - flow_delegate: Box::new(DefaultFlowDelegate), + flow_delegate: Box::new(DefaultInstalledFlowDelegate), } } @@ -89,7 +89,7 @@ impl InstalledFlow { /// . Obtain a token and refresh token using that code. /// . Return that token /// - /// It's recommended not to use the DefaultFlowDelegate, but a specialized one. + /// It's recommended not to use the DefaultInstalledFlowDelegate, but a specialized one. pub(crate) async fn token( &self, hyper_client: &hyper::Client, @@ -404,7 +404,7 @@ mod tests { use mockito::mock; use super::*; - use crate::authenticator_delegate::FlowDelegate; + use crate::authenticator_delegate::InstalledFlowDelegate; #[tokio::test] async fn test_end2end() { @@ -413,7 +413,7 @@ mod tests { String, hyper::Client, hyper::Body>, ); - impl FlowDelegate for FD { + impl InstalledFlowDelegate for FD { /// Depending on need_code, return the pre-set code or send the code to the server at /// the redirect_uri given in the url. fn present_user_url<'a>(