From 0e9cf512ba91a8bdf070e94b8f48eb6dacb21f6e Mon Sep 17 00:00:00 2001 From: Glenn Griffin Date: Fri, 8 Nov 2019 15:59:29 -0800 Subject: [PATCH] Remove the HTTPRedirectEphemeral variant. In favor of making it the default and removing the option to specify a port to listen on. If needed a variant can be added to specify a port explicitly, but most users should want an ephemeral port chosen so making it the default makes sense while other breaking changes are in flight. --- examples/test-installed/src/main.rs | 2 +- src/installed.rs | 49 +++++++++++------------------ src/lib.rs | 2 +- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/examples/test-installed/src/main.rs b/examples/test-installed/src/main.rs index c333255..54be93f 100644 --- a/examples/test-installed/src/main.rs +++ b/examples/test-installed/src/main.rs @@ -10,7 +10,7 @@ async fn main() { let auth = Authenticator::new(InstalledFlow::new( secret, - yup_oauth2::InstalledFlowReturnMethod::HTTPRedirectEphemeral, + yup_oauth2::InstalledFlowReturnMethod::HTTPRedirect, )) .persist_tokens_to_disk("tokencache.json") .build() diff --git a/src/installed.rs b/src/installed.rs index 8928261..d31ea38 100644 --- a/src/installed.rs +++ b/src/installed.rs @@ -92,11 +92,7 @@ pub enum InstalledFlowReturnMethod { Interactive, /// Involves spinning up a local HTTP server and Google redirecting the browser to /// the server with a URL containing the code (preferred, but not as reliable). - HTTPRedirectEphemeral, - /// Involves spinning up a local HTTP server and Google redirecting the browser to - /// the server with a URL containing the code (preferred, but not as reliable). The - /// parameter is the port to listen on. - HTTPRedirect(u16), + HTTPRedirect, } /// InstalledFlowImpl provides tokens for services that follow the "Installed" OAuth flow. (See @@ -169,12 +165,7 @@ where T: AsRef, { match self.method { - InstalledFlowReturnMethod::HTTPRedirect(port) => { - self.ask_auth_code_via_http(scopes, port).await - } - InstalledFlowReturnMethod::HTTPRedirectEphemeral => { - self.ask_auth_code_via_http(scopes, 0).await - } + InstalledFlowReturnMethod::HTTPRedirect => self.ask_auth_code_via_http(scopes).await, InstalledFlowReturnMethod::Interactive => { self.ask_auth_code_interactively(scopes).await } @@ -211,26 +202,22 @@ where self.exchange_auth_code(&authcode, None).await } - async fn ask_auth_code_via_http( - &self, - scopes: &[T], - desired_port: u16, - ) -> Result + async fn ask_auth_code_via_http(&self, scopes: &[T]) -> Result where T: AsRef, { use std::borrow::Cow; let auth_delegate = &self.fd; let appsecret = &self.appsecret; - let server = InstalledFlowServer::run(desired_port)?; - let bound_port = server.local_addr().port(); + let server = InstalledFlowServer::run()?; + let server_addr = server.local_addr(); // 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(), + None => format!("http://{}", server_addr).into(), }; let url = build_authentication_request_url( &appsecret.auth_uri, @@ -243,17 +230,17 @@ where .await; let auth_code = server.wait_for_auth_code().await; - self.exchange_auth_code(&auth_code, Some(bound_port)).await + self.exchange_auth_code(&auth_code, Some(server_addr)).await } async fn exchange_auth_code( &self, authcode: &str, - port: Option, + server_addr: Option, ) -> Result { let appsec = &self.appsecret; let redirect_uri = self.fd.redirect_uri(); - let request = Self::request_token(appsec, authcode, redirect_uri, port); + let request = Self::request_token(appsec, authcode, redirect_uri, server_addr); let resp = self .client .request(request) @@ -299,12 +286,12 @@ where appsecret: &ApplicationSecret, authcode: &str, custom_redirect_uri: Option<&str>, - port: Option, + server_addr: Option, ) -> hyper::Request { use std::borrow::Cow; - let redirect_uri: Cow = match (custom_redirect_uri, port) { + let redirect_uri: Cow = match (custom_redirect_uri, server_addr) { (Some(uri), _) => uri.into(), - (None, Some(port)) => format!("http://localhost:{}", port).into(), + (None, Some(addr)) => format!("http://{}", addr).into(), (None, None) => OOB_REDIRECT_URI.into(), }; @@ -344,7 +331,7 @@ struct InstalledFlowServer { } impl InstalledFlowServer { - fn run(desired_port: u16) -> Result { + fn run() -> Result { use hyper::service::{make_service_fn, service_fn}; let (auth_code_tx, auth_code_rx) = oneshot::channel::(); let (trigger_shutdown_tx, trigger_shutdown_rx) = oneshot::channel::<()>(); @@ -359,7 +346,7 @@ impl InstalledFlowServer { })) } }); - let addr: std::net::SocketAddr = ([127, 0, 0, 1], desired_port).into(); + let addr: std::net::SocketAddr = ([127, 0, 0, 1], 0).into(); let server = hyper::server::Server::try_bind(&addr)?; let server = server.http1_only(true).serve(service); let addr = server.local_addr(); @@ -572,7 +559,7 @@ mod tests { } // Successful path with HTTP redirect. { - let inf = InstalledFlow::new(app_secret, InstalledFlowReturnMethod::HTTPRedirect(8081)) + let inf = InstalledFlow::new(app_secret, InstalledFlowReturnMethod::HTTPRedirect) .delegate(FD( "authorizationcodefromlocalserver".to_string(), client.clone(), @@ -639,8 +626,8 @@ mod tests { #[tokio::test] async fn test_server_random_local_port() { - let addr1 = InstalledFlowServer::run(0).unwrap().local_addr(); - let addr2 = InstalledFlowServer::run(0).unwrap().local_addr(); + let addr1 = InstalledFlowServer::run().unwrap().local_addr(); + let addr2 = InstalledFlowServer::run().unwrap().local_addr(); assert_ne!(addr1.port(), addr2.port()); } @@ -662,7 +649,7 @@ mod tests { async fn test_server() { let client: hyper::Client = hyper::Client::builder().build_http(); - let server = InstalledFlowServer::run(0).unwrap(); + let server = InstalledFlowServer::run().unwrap(); let response = client .get(format!("http://{}/", server.local_addr()).parse().unwrap()) diff --git a/src/lib.rs b/src/lib.rs index ea9a9b7..209874f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -59,7 +59,7 @@ //! // authenticator takes care of caching tokens to disk and refreshing tokens once //! // they've expired. //! let mut auth = Authenticator::new( -//! InstalledFlow::new(secret, yup_oauth2::InstalledFlowReturnMethod::HTTPRedirect(0)) +//! InstalledFlow::new(secret, yup_oauth2::InstalledFlowReturnMethod::HTTPRedirect) //! ) //! .persist_tokens_to_disk("tokencache.json") //! .build()