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.

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<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + 'static>
 
         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<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + 'static>
                         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<dyn futures::Future<Item = (String, String), Error = Error> + Send>
-    {
+    ) -> Box<dyn futures::Future<Item = String, Error = Error> + 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<dyn futures::Future<Item = (String, String), Error = Error> + Send>
-    {
+    ) -> Box<dyn futures::Future<Item = String, Error = Error> + 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<dyn futures::Future<Item = (), Error = Error> + 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<Self>,
         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<dyn futures::Future<Item = (), Error = Error> + 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<u8> for MessageType {
@@ -268,9 +269,10 @@ impl std::convert::TryFrom<u8> 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<Packet> 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<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + 'static>
                     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<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + 'static>
                             // 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<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + 'static>
                     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<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + 'static>
         Ok(())
     }
 
-    fn handle_message_oauth_response_code(
+    fn handle_message_oauth_cli_response(
         &mut self,
         conn: &mut Connection<S>,
         code: &str,
@@ -612,14 +613,14 @@ impl<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + 'static>
     > {
         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<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + 'static>
         Ok(Some(Box::new(fut)))
     }
 
-    fn handle_message_oauth_response_token(
-        &mut self,
-        conn: &mut Connection<S>,
-        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<S>,
@@ -720,17 +676,9 @@ impl<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Send + 'static>
         >,
     > {
         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 }),
         }
     }