bind Duo oauth flow to device id, drop redundant device type handling

src/api/core/two_factor/duo_oidc.rs

@@ -1,12 +1,14 @@
 use chrono::Utc;
+use data_encoding::HEXLOWER;
 use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header, Validation};
 use reqwest::{header, StatusCode};
+use ring::digest::{digest, Digest, SHA512_256};
 use serde::Serialize;
 use std::collections::HashMap;
+
 use url::Url;
 use crate::{
     api::{core::two_factor::duo::get_duo_keys_email, EmptyResult},
-    auth::ClientType,
     crypto,
     db::{models::{
             EventType,
@@ -423,10 +425,13 @@ fn make_callback_url(client_name: &str) -> Result<String, Error> {
 
 // Pre-redirect first stage of the Duo WebSDKv4 authentication flow.
 // Returns the "AuthUrl" that should be returned to clients for MFA.
-pub async fn get_duo_auth_url(email: &str, client_type: &ClientType, conn: &mut DbConn) -> Result<String, Error> {
+pub async fn get_duo_auth_url(email: &str,
+                              client_id: &String,
+                              device_identifier: &String,
+                              conn: &mut DbConn) -> Result<String, Error> {
     let (ik, sk, _, host) = get_duo_keys_email(email, conn).await?;
 
-    let callback_url = match make_callback_url(client_type.as_str()) {
+    let callback_url = match make_callback_url(client_id.as_str()) {
         Ok(url) => url,
         Err(e) => err!(format!("{}", e)),
     };
@@ -439,11 +444,16 @@ pub async fn get_duo_auth_url(email: &str, client_type: &ClientType, conn: &mut
     };
 
     // Generate random OAuth2 state and OIDC Nonce
-    let state = generate_state();
-    let nonce = generate_state();
+    let state: String = generate_state();
+    let nonce: String = generate_state();
+
+    // Bind the nonce to the device that's currently authing by hashing the nonce and device id
+    // and sending that as the OIDC nonce.
+    let d: Digest = digest(&SHA512_256, format!("{nonce}{device_identifier}").as_bytes());
+    let hash: String = HEXLOWER.encode(d.as_ref());
 
     match TwoFactorDuoContext::save(state.as_str(), email, nonce.as_str(), CTX_VALIDITY_SECS, conn).await {
-        Ok(()) => client.make_authz_req_url(email, state, nonce),
+        Ok(()) => client.make_authz_req_url(email, state, hash),
         Err(e) => err!(format!("Error storing Duo authentication context: {}", e))
     }
 }
@@ -453,12 +463,10 @@ pub async fn get_duo_auth_url(email: &str, client_type: &ClientType, conn: &mut
 pub async fn validate_duo_login(
     email: &str,
     two_factor_token: &str,
-    client_type: &ClientType,
+    client_id: &String,
+    device_identifier: &String,
     conn: &mut DbConn,
 ) -> EmptyResult {
-    // TODO: The OIDC nonce should somehow be bound to a specific authentication attempt.
-    // e.g. hashed and in an httponly cookie.
-    // This may not be possible given the way that BW clients redirect users for final auth.
     let email = &email.to_lowercase();
 
     let split: Vec<&str> = two_factor_token.split('|').collect();
@@ -506,7 +514,7 @@ pub async fn validate_duo_login(
         )
     }
 
-    let callback_url = match make_callback_url(client_type.as_str()) {
+    let callback_url = match make_callback_url(client_id.as_str()) {
         Ok(url) => url,
         Err(e) => err!(format!("{}", e)),
     };
@@ -518,7 +526,10 @@ pub async fn validate_duo_login(
         Err(e) => err!(format!("{}", e)),
     };
 
-    match client.exchange_authz_code_for_result(code, email, ctx.nonce.as_str()).await {
+    let d: Digest = digest(&SHA512_256, format!("{}{}", ctx.nonce, device_identifier).as_bytes());
+    let hash: String = HEXLOWER.encode(d.as_ref());
+
+    match client.exchange_authz_code_for_result(code, email, hash.as_str()).await {
         Ok(_) => Ok(()),
         Err(_) => {
             err!(

src/api/identity.rs

@@ -17,7 +17,7 @@ use crate::{
         push::register_push_device,
         ApiResult, EmptyResult, JsonResult,
     },
-    auth::{generate_organization_api_key_login_claims, ClientHeaders, ClientIp, ClientType},
+    auth::{generate_organization_api_key_login_claims, ClientHeaders, ClientIp},
     db::{models::*, DbConn},
     error::MapResult,
     mail, util, CONFIG,
@@ -48,7 +48,7 @@ async fn login(data: Form<ConnectData>, client_header: ClientHeaders, mut conn:
             _check_is_some(&data.device_name, "device_name cannot be blank")?;
             _check_is_some(&data.device_type, "device_type cannot be blank")?;
 
-            _password_login(data, &mut user_uuid, &mut conn, &client_header.ip, client_header.client_type).await
+            _password_login(data, &mut user_uuid, &mut conn, &client_header.ip).await
         }
         "client_credentials" => {
             _check_is_some(&data.client_id, "client_id cannot be blank")?;
@@ -140,7 +140,6 @@ async fn _password_login(
     user_uuid: &mut Option<String>,
     conn: &mut DbConn,
     ip: &ClientIp,
-    client_type: ClientType,
 ) -> JsonResult {
     // Validate scope
     let scope = data.scope.as_ref().unwrap();
@@ -251,7 +250,7 @@ async fn _password_login(
 
     let (mut device, new_device) = get_device(&data, conn, &user).await;
 
-    let twofactor_token = twofactor_auth(&user, &data, &mut device, ip, &client_type, conn).await?;
+    let twofactor_token = twofactor_auth(&user, &data, &mut device, ip, conn).await?;
 
     if CONFIG.mail_enabled() && new_device {
         if let Err(e) = mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), &now, &device.name).await {
@@ -486,7 +485,6 @@ async fn twofactor_auth(
     data: &ConnectData,
     device: &mut Device,
     ip: &ClientIp,
-    client_type: &ClientType,
     conn: &mut DbConn,
 ) -> ApiResult<Option<String>> {
     let twofactors = TwoFactor::find_by_user(&user.uuid, conn).await;
@@ -504,7 +502,7 @@ async fn twofactor_auth(
 
     let twofactor_code = match data.two_factor_token {
         Some(ref code) => code,
-        None => err_json!(_json_err_twofactor(&twofactor_ids, &user.uuid, client_type, conn).await?, "2FA token not provided"),
+        None => err_json!(_json_err_twofactor(&twofactor_ids, &user.uuid, &data, conn).await?, "2FA token not provided"),
     };
 
     let selected_twofactor = twofactors.into_iter().find(|tf| tf.atype == selected_id && tf.enabled);
@@ -528,7 +526,11 @@ async fn twofactor_auth(
                 }
                 false => {
                     // OIDC based flow
-                    duo_oidc::validate_duo_login(data.username.as_ref().unwrap().trim(), twofactor_code, client_type, conn).await?
+                    duo_oidc::validate_duo_login(data.username.as_ref().unwrap().trim(),
+                                                 twofactor_code,
+                                                 data.client_id.as_ref().unwrap(),
+                                                 data.device_identifier.as_ref().unwrap(),
+                                                 conn).await?
                 }
             }
         }
@@ -543,7 +545,7 @@ async fn twofactor_auth(
                 }
                 _ => {
                     err_json!(
-                        _json_err_twofactor(&twofactor_ids, &user.uuid, client_type, conn).await?,
+                        _json_err_twofactor(&twofactor_ids, &user.uuid, &data, conn).await?,
                         "2FA Remember token not provided"
                     )
                 }
@@ -571,7 +573,7 @@ fn _selected_data(tf: Option<TwoFactor>) -> ApiResult<String> {
     tf.map(|t| t.data).map_res("Two factor doesn't exist")
 }
 
-async fn _json_err_twofactor(providers: &[i32], user_uuid: &str, client_type: &ClientType, conn: &mut DbConn) -> ApiResult<Value> {
+async fn _json_err_twofactor(providers: &[i32], user_uuid: &str, data: &ConnectData, conn: &mut DbConn) -> ApiResult<Value> {
     let mut result = json!({
         "error" : "invalid_grant",
         "error_description" : "Two factor required.",
@@ -610,7 +612,10 @@ async fn _json_err_twofactor(providers: &[i32], user_uuid: &str, client_type: &C
                     }
                     false => {
                         // OIDC based flow
-                        let auth_url = duo_oidc::get_duo_auth_url(&email, client_type, conn).await?;
+                        let auth_url = duo_oidc::get_duo_auth_url(&email,
+                                                                  data.client_id.as_ref().unwrap(),
+                                                                  data.device_identifier.as_ref().unwrap(),
+                                                                  conn).await?;
 
                         result["TwoFactorProviders2"][provider.to_string()] = json!({
                         "AuthUrl": auth_url,

src/auth.rs

@@ -416,47 +416,9 @@ impl<'r> FromRequest<'r> for Host {
     }
 }
 
-pub enum ClientType {
-    Unspecified = 0,
-    Web = 1,
-    Browser = 2,
-    Desktop = 3,
-    Mobile = 4,
-    Cli = 5,
-    DirectoryConnector = 6,
-}
-
-impl ClientType {
-    pub fn as_str(&self) -> &'static str {
-        match self {
-            ClientType::Unspecified => "",
-            ClientType::Web => "web",
-            ClientType::Browser => "browser",
-            ClientType::Desktop => "desktop",
-            ClientType::Mobile => "mobile",
-            ClientType::Cli => "cli",
-            ClientType::DirectoryConnector => "connector",
-        }
-    }
-
-    pub fn from_str(client_name: &str) -> ClientType {
-        match client_name {
-            "web" => ClientType::Web,
-            "browser" => ClientType::Browser,
-            "desktop" => ClientType::Desktop,
-            "mobile" => ClientType::Mobile,
-            "cli" => ClientType::Cli,
-            "connector" => ClientType::DirectoryConnector,
-            _ => ClientType::Unspecified,
-        }
-    }
-}
-
-
 pub struct ClientHeaders {
     pub device_type: i32,
     pub ip: ClientIp,
-    pub client_type: ClientType,
 }
 
 #[rocket::async_trait]
@@ -472,13 +434,9 @@ impl<'r> FromRequest<'r> for ClientHeaders {
         let device_type: i32 =
             request.headers().get_one("device-type").map(|d| d.parse().unwrap_or(14)).unwrap_or_else(|| 14);
 
-        let client_name = request.headers().get_one("Bitwarden-Client-Name").unwrap_or_else(|| "");
-        let client_type: ClientType = ClientType::from_str(client_name);
-
         Outcome::Success(ClientHeaders {
             device_type,
             ip,
-            client_type,
         })
     }
 }