From 92bf64ce74050545c96694c8d34202b1546ab390 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Wed, 27 Nov 2019 13:48:55 -0500 Subject: rethink how oauth logins need to work i can't exchange refresh tokens for access tokens without knowing the webface oauth configuration either, so this strategy also won't work. the server actually needs to only receive access tokens, and request the web server to refresh them as needed. --- teleterm/src/client.rs | 4 +- teleterm/src/oauth.rs | 50 +++++++--------------- teleterm/src/protocol.rs | 105 +++++++++++++++++++++++++---------------------- teleterm/src/server.rs | 72 +++++--------------------------- 4 files changed, 81 insertions(+), 150 deletions(-) diff --git a/teleterm/src/client.rs b/teleterm/src/client.rs index 473cb9b..0672811 100644 --- a/teleterm/src/client.rs +++ b/teleterm/src/client.rs @@ -259,7 +259,7 @@ impl if !self.raw { match msg { - crate::protocol::Message::OauthRequest { url, id } => { + crate::protocol::Message::OauthCliRequest { url, id } => { let mut state = None; let parsed_url = url::Url::parse(&url).unwrap(); for (k, v) in parsed_url.query_pairs() { @@ -373,7 +373,7 @@ impl return Err(Error::ParseHttpRequestMissingCode); }; Ok(( - crate::protocol::Message::oauth_response_code(&code), + crate::protocol::Message::oauth_cli_response(&code), lines.into_inner().into_inner(), )) }) diff --git a/teleterm/src/oauth.rs b/teleterm/src/oauth.rs index 0d55a9f..e692a1e 100644 --- a/teleterm/src/oauth.rs +++ b/teleterm/src/oauth.rs @@ -29,11 +29,10 @@ pub trait Oauth { auth_url.to_string() } - fn get_tokens_from_auth_code( + fn get_access_token_from_auth_code( &self, code: &str, - ) -> Box + Send> - { + ) -> Box + Send> { let token_cache_file = self.server_token_file(false).unwrap(); let fut = self .client() @@ -44,24 +43,16 @@ pub trait Oauth { Error::ExchangeCode { msg } }) .and_then(|token| { - let access_token = token.access_token().secret().to_string(); - let refresh_token = - token.refresh_token().unwrap().secret().to_string(); - cache_refresh_token( - token_cache_file, - &access_token, - &refresh_token, - ) - .map(move |_| (access_token, refresh_token)) + cache_refresh_token(token_cache_file, &token) + .map(move |_| token.access_token().secret().to_string()) }); Box::new(fut) } - fn get_tokens_from_refresh_token( + fn get_access_token_from_refresh_token( &self, token: &str, - ) -> Box + Send> - { + ) -> Box + Send> { let token_cache_file = self.server_token_file(false).unwrap(); let fut = self .client() @@ -74,28 +65,12 @@ pub trait Oauth { Error::ExchangeRefreshToken { msg } }) .and_then(|token| { - let access_token = token.access_token().secret().to_string(); - let refresh_token = - token.refresh_token().unwrap().secret().to_string(); - cache_refresh_token( - token_cache_file, - &access_token, - &refresh_token, - ) - .map(move |_| (access_token, refresh_token)) + cache_refresh_token(token_cache_file, &token) + .map(move |_| token.access_token().secret().to_string()) }); Box::new(fut) } - fn save_tokens( - &self, - access_token: &str, - refresh_token: &str, - ) -> Box + Send> { - let token_cache_file = self.server_token_file(false).unwrap(); - cache_refresh_token(token_cache_file, access_token, refresh_token) - } - fn get_username_from_access_token( self: Box, token: &str, @@ -141,10 +116,13 @@ fn client_id_file( fn cache_refresh_token( token_cache_file: std::path::PathBuf, - access_token: &str, - refresh_token: &str, + token: &oauth2::basic::BasicTokenResponse, ) -> Box + Send> { - let token_data = format!("{}\n{}\n", refresh_token, access_token); + let token_data = format!( + "{}\n{}\n", + token.refresh_token().unwrap().secret(), + token.access_token().secret(), + ); let fut = tokio::fs::File::create(token_cache_file.clone()) .with_context(move || crate::error::CreateFile { filename: token_cache_file.to_string_lossy().to_string(), diff --git a/teleterm/src/protocol.rs b/teleterm/src/protocol.rs index cb95947..7da5e92 100644 --- a/teleterm/src/protocol.rs +++ b/teleterm/src/protocol.rs @@ -247,9 +247,10 @@ pub enum MessageType { Error, Resize, LoggedIn, - OauthRequest, - OauthResponseCode, - OauthResponseToken, + OauthCliRequest, + OauthCliResponse, + OauthWebRequest, + OauthWebResponse, } impl std::convert::TryFrom for MessageType { @@ -268,9 +269,10 @@ impl std::convert::TryFrom for MessageType { 8 => Self::Error, 9 => Self::Resize, 10 => Self::LoggedIn, - 11 => Self::OauthRequest, - 12 => Self::OauthResponseCode, - 13 => Self::OauthResponseToken, + 11 => Self::OauthCliRequest, + 12 => Self::OauthCliResponse, + 13 => Self::OauthWebRequest, + 14 => Self::OauthWebResponse, _ => return Err(Error::InvalidMessageType { ty: n }), }) } @@ -308,16 +310,18 @@ pub enum Message { LoggedIn { username: String, }, - OauthRequest { + OauthCliRequest { url: String, id: String, }, - OauthResponseCode { + OauthCliResponse { code: String, }, - OauthResponseToken { + OauthWebRequest { + id: String, + }, + OauthWebResponse { access_token: String, - refresh_token: String, }, } @@ -383,26 +387,26 @@ impl Message { } } - pub fn oauth_request(url: &str, id: &str) -> Self { - Self::OauthRequest { + pub fn oauth_cli_request(url: &str, id: &str) -> Self { + Self::OauthCliRequest { url: url.to_string(), id: id.to_string(), } } - pub fn oauth_response_code(code: &str) -> Self { - Self::OauthResponseCode { + pub fn oauth_cli_response(code: &str) -> Self { + Self::OauthCliResponse { code: code.to_string(), } } - pub fn oauth_response_token( - access_token: &str, - refresh_token: &str, - ) -> Self { - Self::OauthResponseToken { + pub fn oauth_web_request(id: &str) -> Self { + Self::OauthWebRequest { id: id.to_string() } + } + + pub fn oauth_web_response(access_token: &str) -> Self { + Self::OauthWebResponse { access_token: access_token.to_string(), - refresh_token: refresh_token.to_string(), } } @@ -419,11 +423,10 @@ impl Message { Self::Error { .. } => MessageType::Error, Self::Resize { .. } => MessageType::Resize, Self::LoggedIn { .. } => MessageType::LoggedIn, - Self::OauthRequest { .. } => MessageType::OauthRequest, - Self::OauthResponseCode { .. } => MessageType::OauthResponseCode, - Self::OauthResponseToken { .. } => { - MessageType::OauthResponseToken - } + Self::OauthCliRequest { .. } => MessageType::OauthCliRequest, + Self::OauthCliResponse { .. } => MessageType::OauthCliResponse, + Self::OauthWebRequest { .. } => MessageType::OauthWebRequest, + Self::OauthWebResponse { .. } => MessageType::OauthWebResponse, } } @@ -463,12 +466,17 @@ impl Message { } // these are security-sensitive, keep them out of logs - Self::OauthRequest { .. } => "OauthRequest {{ .. }}".to_string(), - Self::OauthResponseCode { .. } => { - "OauthResponseCode {{ .. }}".to_string() + Self::OauthCliRequest { .. } => { + "OauthCliRequest {{ .. }}".to_string() + } + Self::OauthCliResponse { .. } => { + "OauthCliResponse {{ .. }}".to_string() } - Self::OauthResponseToken { .. } => { - "OauthResponseToken {{ .. }}".to_string() + Self::OauthWebRequest { .. } => { + "OauthWebRequest {{ .. }}".to_string() + } + Self::OauthWebResponse { .. } => { + "OauthWebResponse {{ .. }}".to_string() } _ => format!("{:?}", self), @@ -649,19 +657,18 @@ impl From<&Message> for Packet { Message::LoggedIn { username } => { write_str(username, &mut data); } - Message::OauthRequest { url, id } => { + Message::OauthCliRequest { url, id } => { write_str(url, &mut data); write_str(id, &mut data); } - Message::OauthResponseCode { code } => { + Message::OauthCliResponse { code } => { write_str(code, &mut data); } - Message::OauthResponseToken { - access_token, - refresh_token, - } => { + Message::OauthWebRequest { id } => { + write_str(id, &mut data); + } + Message::OauthWebResponse { access_token } => { write_str(access_token, &mut data); - write_str(refresh_token, &mut data); } } @@ -843,28 +850,26 @@ impl std::convert::TryFrom for Message { (Self::LoggedIn { username }, data) } - MessageType::OauthRequest => { + MessageType::OauthCliRequest => { let (url, data) = read_str(data)?; let (id, data) = read_str(data)?; - (Self::OauthRequest { url, id }, data) + (Self::OauthCliRequest { url, id }, data) } - MessageType::OauthResponseCode => { + MessageType::OauthCliResponse => { let (code, data) = read_str(data)?; - (Self::OauthResponseCode { code }, data) + (Self::OauthCliResponse { code }, data) + } + MessageType::OauthWebRequest => { + let (id, data) = read_str(data)?; + + (Self::OauthWebRequest { id }, data) } - MessageType::OauthResponseToken => { + MessageType::OauthWebResponse => { let (access_token, data) = read_str(data)?; - let (refresh_token, data) = read_str(data)?; - ( - Self::OauthResponseToken { - access_token, - refresh_token, - }, - data, - ) + (Self::OauthWebResponse { access_token }, data) } }; diff --git a/teleterm/src/server.rs b/teleterm/src/server.rs index 2215319..71a8ac4 100644 --- a/teleterm/src/server.rs +++ b/teleterm/src/server.rs @@ -383,6 +383,7 @@ impl crate::error::AuthTypeMissingOauthConfig { ty }, )?; let (refresh, client) = match oauth { + // XXX handle this differently based on auth_client too crate::protocol::Auth::RecurseCenter { id, .. } => ( id.is_some(), ty.oauth_client( @@ -426,10 +427,10 @@ impl // XXX unwrap here isn't super safe let refresh_token = refresh_token.unwrap(); client - .get_tokens_from_refresh_token( + .get_access_token_from_refresh_token( refresh_token.trim(), ) - .and_then(|(access_token, _)| { + .and_then(|access_token| { client.get_username_from_access_token( &access_token, ) @@ -455,7 +456,7 @@ impl let authorize_url = client.generate_authorize_url(); let user_id = client.user_id().to_string(); conn.send_message( - crate::protocol::Message::oauth_request( + crate::protocol::Message::oauth_cli_request( &authorize_url, &user_id, ), @@ -596,7 +597,7 @@ impl Ok(()) } - fn handle_message_oauth_response_code( + fn handle_message_oauth_cli_response( &mut self, conn: &mut Connection, code: &str, @@ -612,14 +613,14 @@ impl > { let client = conn.oauth_client.take().ok_or_else(|| { Error::UnexpectedMessage { - message: crate::protocol::Message::oauth_response_code(code), + message: crate::protocol::Message::oauth_cli_response(code), } })?; let term_info = conn.state.term_info().unwrap().clone(); let fut = client - .get_tokens_from_auth_code(code) - .and_then(|(access_token, _)| { + .get_access_token_from_auth_code(code) + .and_then(|access_token| { client.get_username_from_access_token(&access_token) }) .map(|username| { @@ -635,51 +636,6 @@ impl Ok(Some(Box::new(fut))) } - fn handle_message_oauth_response_token( - &mut self, - conn: &mut Connection, - access_token: &str, - refresh_token: &str, - ) -> Result< - Option< - Box< - dyn futures::Future< - Item = (ConnectionState, crate::protocol::Message), - Error = Error, - > + Send, - >, - >, - > { - let client = conn.oauth_client.take().ok_or_else(|| { - Error::UnexpectedMessage { - message: crate::protocol::Message::oauth_response_token( - access_token, - refresh_token, - ), - } - })?; - - let term_info = conn.state.term_info().unwrap().clone(); - let access_token = access_token.to_string(); - let fut = client.save_tokens(&access_token, refresh_token).and_then( - move |_| { - client.get_username_from_access_token(&access_token).map( - |username| { - ( - ConnectionState::LoggedIn { - term_info, - username: username.clone(), - }, - crate::protocol::Message::logged_in(&username), - ) - }, - ) - }, - ); - - Ok(Some(Box::new(fut))) - } - fn handle_accepted_message( &mut self, conn: &mut Connection, @@ -720,17 +676,9 @@ impl >, > { match message { - crate::protocol::Message::OauthResponseCode { code } => { - self.handle_message_oauth_response_code(conn, &code) + crate::protocol::Message::OauthCliResponse { code } => { + self.handle_message_oauth_cli_response(conn, &code) } - crate::protocol::Message::OauthResponseToken { - access_token, - refresh_token, - } => self.handle_message_oauth_response_token( - conn, - &access_token, - &refresh_token, - ), m => Err(Error::UnauthenticatedMessage { message: m }), } } -- cgit v1.2.3-54-g00ecf