diff --git a/.env.template b/.env.template index 457ca803..67f531fc 100644 --- a/.env.template +++ b/.env.template @@ -183,9 +183,9 @@ ## Defaults to every minute. Set blank to disable this job. # DUO_CONTEXT_PURGE_SCHEDULE="30 * * * * *" # -## Cron schedule of the job that cleans sso nonce from incomplete flow +## Cron schedule of the job that cleans sso auth from incomplete flow ## Defaults to daily (20 minutes after midnight). Set blank to disable this job. -# PURGE_INCOMPLETE_SSO_NONCE="0 20 0 * * *" +# PURGE_INCOMPLETE_SSO_AUTH="0 20 0 * * *" ######################## ### General settings ### @@ -348,7 +348,7 @@ ## Default: 2592000 (30 days) # ICON_CACHE_TTL=2592000 ## Cache time-to-live for icons which weren't available, in seconds (0 is "forever") -## Default: 2592000 (3 days) +## Default: 259200 (3 days) # ICON_CACHE_NEGTTL=259200 ## Icon download timeout diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4fd722bc..b2821ab9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -368,22 +368,25 @@ jobs: run: | set +e IFS=',' read -ra IMAGES <<< "${CONTAINER_REGISTRIES}" + IFS=',' read -ra TAGS <<< "${BASE_TAGS}" for img in "${IMAGES[@]}"; do - echo "Creating manifest for $img:${BASE_TAGS}-${BASE_IMAGE}" + for tag in "${TAGS[@]}"; do + echo "Creating manifest for $img:$tag-${BASE_IMAGE}" - OUTPUT=$(docker buildx imagetools create \ - -t "$img:${BASE_TAGS}-${BASE_IMAGE}" \ - $(printf "$img:${BASE_TAGS}-${BASE_IMAGE}@sha256:%s " *) 2>&1) - STATUS=$? + OUTPUT=$(docker buildx imagetools create \ + -t "$img:$tag-${BASE_IMAGE}" \ + $(printf "$img:$tag-${BASE_IMAGE}@sha256:%s " *) 2>&1) + STATUS=$? - if [ $STATUS -ne 0 ]; then - echo "Manifest creation failed for $img" - echo "$OUTPUT" - exit $STATUS - fi + if [ $STATUS -ne 0 ]; then + echo "Manifest creation failed for $img" + echo "$OUTPUT" + exit $STATUS + fi - echo "Manifest created for $img" - echo "$OUTPUT" + echo "Manifest created for $img" + echo "$OUTPUT" + done done set -e diff --git a/migrations/mysql/2025-08-20-120000_sso_nonce_to_auth/down.sql b/migrations/mysql/2025-08-20-120000_sso_nonce_to_auth/down.sql new file mode 100644 index 00000000..3a965886 --- /dev/null +++ b/migrations/mysql/2025-08-20-120000_sso_nonce_to_auth/down.sql @@ -0,0 +1,9 @@ +DROP TABLE IF EXISTS sso_auth; + +CREATE TABLE sso_nonce ( + state VARCHAR(512) NOT NULL PRIMARY KEY, + nonce TEXT NOT NULL, + verifier TEXT, + redirect_uri TEXT NOT NULL, + created_at TIMESTAMP NOT NULL DEFAULT now() +); diff --git a/migrations/mysql/2025-08-20-120000_sso_nonce_to_auth/up.sql b/migrations/mysql/2025-08-20-120000_sso_nonce_to_auth/up.sql new file mode 100644 index 00000000..1a68b715 --- /dev/null +++ b/migrations/mysql/2025-08-20-120000_sso_nonce_to_auth/up.sql @@ -0,0 +1,12 @@ +DROP TABLE IF EXISTS sso_nonce; + +CREATE TABLE sso_auth ( + state VARCHAR(512) NOT NULL PRIMARY KEY, + client_challenge TEXT NOT NULL, + nonce TEXT NOT NULL, + redirect_uri TEXT NOT NULL, + code_response TEXT, + auth_response TEXT, + created_at TIMESTAMP NOT NULL DEFAULT now(), + updated_at TIMESTAMP NOT NULL DEFAULT now() +); diff --git a/migrations/postgresql/2025-08-20-120000_sso_nonce_to_auth/down.sql b/migrations/postgresql/2025-08-20-120000_sso_nonce_to_auth/down.sql new file mode 100644 index 00000000..8cc36353 --- /dev/null +++ b/migrations/postgresql/2025-08-20-120000_sso_nonce_to_auth/down.sql @@ -0,0 +1,9 @@ +DROP TABLE IF EXISTS sso_auth; + +CREATE TABLE sso_nonce ( + state TEXT NOT NULL PRIMARY KEY, + nonce TEXT NOT NULL, + verifier TEXT, + redirect_uri TEXT NOT NULL, + created_at TIMESTAMP NOT NULL DEFAULT now() +); diff --git a/migrations/postgresql/2025-08-20-120000_sso_nonce_to_auth/up.sql b/migrations/postgresql/2025-08-20-120000_sso_nonce_to_auth/up.sql new file mode 100644 index 00000000..0fee1b5a --- /dev/null +++ b/migrations/postgresql/2025-08-20-120000_sso_nonce_to_auth/up.sql @@ -0,0 +1,12 @@ +DROP TABLE IF EXISTS sso_nonce; + +CREATE TABLE sso_auth ( + state TEXT NOT NULL PRIMARY KEY, + client_challenge TEXT NOT NULL, + nonce TEXT NOT NULL, + redirect_uri TEXT NOT NULL, + code_response TEXT, + auth_response TEXT, + created_at TIMESTAMP NOT NULL DEFAULT now(), + updated_at TIMESTAMP NOT NULL DEFAULT now() +); diff --git a/migrations/sqlite/2025-08-20-120000_sso_nonce_to_auth/down.sql b/migrations/sqlite/2025-08-20-120000_sso_nonce_to_auth/down.sql new file mode 100644 index 00000000..453e267b --- /dev/null +++ b/migrations/sqlite/2025-08-20-120000_sso_nonce_to_auth/down.sql @@ -0,0 +1,9 @@ +DROP TABLE IF EXISTS sso_auth; + +CREATE TABLE sso_nonce ( + state TEXT NOT NULL PRIMARY KEY, + nonce TEXT NOT NULL, + verifier TEXT, + redirect_uri TEXT NOT NULL, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP +); diff --git a/migrations/sqlite/2025-08-20-120000_sso_nonce_to_auth/up.sql b/migrations/sqlite/2025-08-20-120000_sso_nonce_to_auth/up.sql new file mode 100644 index 00000000..1cd868b4 --- /dev/null +++ b/migrations/sqlite/2025-08-20-120000_sso_nonce_to_auth/up.sql @@ -0,0 +1,12 @@ +DROP TABLE IF EXISTS sso_nonce; + +CREATE TABLE sso_auth ( + state TEXT NOT NULL PRIMARY KEY, + client_challenge TEXT NOT NULL, + nonce TEXT NOT NULL, + redirect_uri TEXT NOT NULL, + code_response TEXT, + auth_response TEXT, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP +); diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 536564d4..672000b3 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -378,7 +378,7 @@ async fn post_set_password(data: Json, headers: Headers, conn: } if let Some(identifier) = data.org_identifier { - if identifier != crate::sso::FAKE_IDENTIFIER { + if identifier != crate::sso::FAKE_IDENTIFIER && identifier != crate::api::admin::FAKE_ADMIN_UUID { let org = match Organization::find_by_uuid(&identifier.into(), &conn).await { None => err!("Failed to retrieve the associated organization"), Some(org) => org, @@ -405,8 +405,8 @@ async fn post_set_password(data: Json, headers: Headers, conn: user.save(&conn).await?; Ok(Json(json!({ - "Object": "set-password", - "CaptchaBypassToken": "", + "object": "set-password", + "captchaBypassToken": "", }))) } @@ -1409,7 +1409,7 @@ async fn put_device_token(device_id: DeviceId, data: Json, headers: H } device.push_token = Some(token); - if let Err(e) = device.save(&conn).await { + if let Err(e) = device.save(true, &conn).await { err!(format!("An error occurred while trying to save the device push token: {e}")); } diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index e8cca467..8159c9b2 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -370,9 +370,9 @@ async fn get_auto_enroll_status(identifier: &str, headers: Headers, conn: DbConn }; Ok(Json(json!({ - "Id": id, - "Identifier": identifier, - "ResetPasswordEnabled": rp_auto_enroll, + "id": id, + "identifier": identifier, + "resetPasswordEnabled": rp_auto_enroll, }))) } @@ -2057,8 +2057,6 @@ async fn get_policy(org_id: OrganizationId, pol_type: i32, headers: AdminHeaders #[derive(Deserialize)] struct PolicyData { enabled: bool, - #[serde(rename = "type")] - _type: i32, data: Option, } diff --git a/src/api/core/two_factor/email.rs b/src/api/core/two_factor/email.rs index cc6909af..b8724cf1 100644 --- a/src/api/core/two_factor/email.rs +++ b/src/api/core/two_factor/email.rs @@ -10,7 +10,7 @@ use crate::{ auth::Headers, crypto, db::{ - models::{EventType, TwoFactor, TwoFactorType, User, UserId}, + models::{DeviceId, EventType, TwoFactor, TwoFactorType, User, UserId}, DbConn, }, error::{Error, MapResult}, @@ -24,10 +24,12 @@ pub fn routes() -> Vec { #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct SendEmailLoginData { + #[serde(alias = "DeviceIdentifier")] + device_identifier: DeviceId, #[serde(alias = "Email")] - email: String, + email: Option, #[serde(alias = "MasterPasswordHash")] - master_password_hash: String, + master_password_hash: Option, } /// User is trying to login and wants to use email 2FA. @@ -36,25 +38,40 @@ struct SendEmailLoginData { async fn send_email_login(data: Json, conn: DbConn) -> EmptyResult { let data: SendEmailLoginData = data.into_inner(); - use crate::db::models::User; + if !CONFIG._enable_email_2fa() { + err!("Email 2FA is disabled") + } // Get the user - let Some(user) = User::find_by_mail(&data.email, &conn).await else { - err!("Username or password is incorrect. Try again.") + let email = match &data.email { + Some(email) if !email.is_empty() => Some(email), + _ => None, }; + let user = if let Some(email) = email { + let Some(master_password_hash) = &data.master_password_hash else { + err!("No password hash has been submitted.") + }; - if !CONFIG._enable_email_2fa() { - err!("Email 2FA is disabled") - } + let Some(user) = User::find_by_mail(email, &conn).await else { + err!("Username or password is incorrect. Try again.") + }; - // Check password - if !user.check_valid_password(&data.master_password_hash) { - err!("Username or password is incorrect. Try again.") - } + // Check password + if !user.check_valid_password(master_password_hash) { + err!("Username or password is incorrect. Try again.") + } + + user + } else { + // SSO login only sends device id, so we get the user by the most recently used device + let Some(user) = User::find_by_device_for_email2fa(&data.device_identifier, &conn).await else { + err!("Username or password is incorrect. Try again.") + }; - send_token(&user.uuid, &conn).await?; + user + }; - Ok(()) + send_token(&user.uuid, &conn).await } /// Generate the token, save the data for later verification and send email to user diff --git a/src/api/identity.rs b/src/api/identity.rs index 92b6c1e4..52e2c659 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -1,4 +1,4 @@ -use chrono::{NaiveDateTime, Utc}; +use chrono::Utc; use num_traits::FromPrimitive; use rocket::{ form::{Form, FromForm}, @@ -24,14 +24,14 @@ use crate::{ auth::{generate_organization_api_key_login_claims, AuthMethod, ClientHeaders, ClientIp, ClientVersion}, db::{ models::{ - AuthRequest, AuthRequestId, Device, DeviceId, EventType, Invitation, OrganizationApiKey, OrganizationId, - SsoNonce, SsoUser, TwoFactor, TwoFactorIncomplete, TwoFactorType, User, UserId, + AuthRequest, AuthRequestId, Device, DeviceId, EventType, Invitation, OIDCCodeWrapper, OrganizationApiKey, + OrganizationId, SsoAuth, SsoUser, TwoFactor, TwoFactorIncomplete, TwoFactorType, User, UserId, }, DbConn, }, error::MapResult, mail, sso, - sso::{OIDCCode, OIDCState}, + sso::{OIDCCode, OIDCCodeChallenge, OIDCCodeVerifier, OIDCState}, util, CONFIG, }; @@ -92,6 +92,7 @@ async fn login( "authorization_code" if CONFIG.sso_enabled() => { _check_is_some(&data.client_id, "client_id cannot be blank")?; _check_is_some(&data.code, "code cannot be blank")?; + _check_is_some(&data.code_verifier, "code verifier cannot be blank")?; _check_is_some(&data.device_identifier, "device_identifier cannot be blank")?; _check_is_some(&data.device_name, "device_name cannot be blank")?; @@ -147,7 +148,7 @@ async fn _refresh_login(data: ConnectData, conn: &DbConn, ip: &ClientIp) -> Json } Ok((mut device, auth_tokens)) => { // Save to update `device.updated_at` to track usage and toggle new status - device.save(conn).await?; + device.save(true, conn).await?; let result = json!({ "refresh_token": auth_tokens.refresh_token(), @@ -175,17 +176,23 @@ async fn _sso_login( // Ratelimit the login crate::ratelimit::check_limit_login(&ip.ip)?; - let code = match data.code.as_ref() { - None => err!( + let (state, code_verifier) = match (data.code.as_ref(), data.code_verifier.as_ref()) { + (None, _) => err!( "Got no code in OIDC data", ErrorEvent { event: EventType::UserFailedLogIn } ), - Some(code) => code, + (_, None) => err!( + "Got no code verifier in OIDC data", + ErrorEvent { + event: EventType::UserFailedLogIn + } + ), + (Some(code), Some(code_verifier)) => (code, code_verifier.clone()), }; - let user_infos = sso::exchange_code(code, conn).await?; + let (sso_auth, user_infos) = sso::exchange_code(state, code_verifier, conn).await?; let user_with_sso = match SsoUser::find_by_identifier(&user_infos.identifier, conn).await { None => match SsoUser::find_by_mail(&user_infos.email, conn).await { None => None, @@ -248,7 +255,7 @@ async fn _sso_login( _ => (), } - let mut user = User::new(&user_infos.email, user_infos.user_name); + let mut user = User::new(&user_infos.email, user_infos.user_name.clone()); user.verified_at = Some(now); user.save(conn).await?; @@ -267,13 +274,14 @@ async fn _sso_login( } Some((mut user, sso_user)) => { let mut device = get_device(&data, conn, &user).await?; + let twofactor_token = twofactor_auth(&mut user, &data, &mut device, ip, client_version, conn).await?; if user.private_key.is_none() { // User was invited a stub was created user.verified_at = Some(now); - if let Some(user_name) = user_infos.user_name { - user.name = user_name; + if let Some(ref user_name) = user_infos.user_name { + user.name = user_name.clone(); } user.save(conn).await?; @@ -290,30 +298,13 @@ async fn _sso_login( } }; - // We passed 2FA get full user information - let auth_user = sso::redeem(&user_infos.state, conn).await?; - - if sso_user.is_none() { - let user_sso = SsoUser { - user_uuid: user.uuid.clone(), - identifier: user_infos.identifier, - }; - user_sso.save(conn).await?; - } - // Set the user_uuid here to be passed back used for event logging. *user_id = Some(user.uuid.clone()); - let auth_tokens = sso::create_auth_tokens( - &device, - &user, - data.client_id, - auth_user.refresh_token, - auth_user.access_token, - auth_user.expires_in, - )?; + // We passed 2FA get auth tokens + let auth_tokens = sso::redeem(&device, &user, data.client_id, sso_user, sso_auth, user_infos, conn).await?; - authenticated_response(&user, &mut device, auth_tokens, twofactor_token, &now, conn, ip).await + authenticated_response(&user, &mut device, auth_tokens, twofactor_token, conn, ip).await } async fn _password_login( @@ -435,7 +426,7 @@ async fn _password_login( let auth_tokens = auth::AuthTokens::new(&device, &user, AuthMethod::Password, data.client_id); - authenticated_response(&user, &mut device, auth_tokens, twofactor_token, &now, conn, ip).await + authenticated_response(&user, &mut device, auth_tokens, twofactor_token, conn, ip).await } async fn authenticated_response( @@ -443,12 +434,12 @@ async fn authenticated_response( device: &mut Device, auth_tokens: auth::AuthTokens, twofactor_token: Option, - now: &NaiveDateTime, conn: &DbConn, ip: &ClientIp, ) -> JsonResult { if CONFIG.mail_enabled() && device.is_new() { - if let Err(e) = mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), now, device).await { + let now = Utc::now().naive_utc(); + if let Err(e) = mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), &now, device).await { error!("Error sending new device email: {e:#?}"); if CONFIG.require_device_email() { @@ -468,7 +459,7 @@ async fn authenticated_response( } // Save to update `device.updated_at` to track usage and toggle new status - device.save(conn).await?; + device.save(true, conn).await?; let master_password_policy = master_password_policy(user, conn).await; @@ -585,7 +576,7 @@ async fn _user_api_key_login( let access_claims = auth::LoginJwtClaims::default(&device, &user, &AuthMethod::UserApiKey, data.client_id); // Save to update `device.updated_at` to track usage and toggle new status - device.save(conn).await?; + device.save(true, conn).await?; info!("User {} logged in successfully via API key. IP: {}", user.email, ip.ip); @@ -648,7 +639,12 @@ async fn get_device(data: &ConnectData, conn: &DbConn, user: &User) -> ApiResult // Find device or create new match Device::find_by_uuid_and_user(&device_id, &user.uuid, conn).await { Some(device) => Ok(device), - None => Device::new(device_id, user.uuid.clone(), device_name, device_type, conn).await, + None => { + let mut device = Device::new(device_id, user.uuid.clone(), device_name, device_type); + // save device without updating `device.updated_at` + device.save(false, conn).await?; + Ok(device) + } } } @@ -997,9 +993,12 @@ struct ConnectData { two_factor_remember: Option, #[field(name = uncased("authrequest"))] auth_request: Option, + // Needed for authorization code #[field(name = uncased("code"))] - code: Option, + code: Option, + #[field(name = uncased("code_verifier"))] + code_verifier: Option, } fn _check_is_some(value: &Option, msg: &str) -> EmptyResult { if value.is_none() { @@ -1021,14 +1020,13 @@ fn prevalidate() -> JsonResult { } #[get("/connect/oidc-signin?&", rank = 1)] -async fn oidcsignin(code: OIDCCode, state: String, conn: DbConn) -> ApiResult { - oidcsignin_redirect( +async fn oidcsignin(code: OIDCCode, state: String, mut conn: DbConn) -> ApiResult { + _oidcsignin_redirect( state, - |decoded_state| sso::OIDCCodeWrapper::Ok { - state: decoded_state, + OIDCCodeWrapper::Ok { code, }, - &conn, + &mut conn, ) .await } @@ -1040,42 +1038,44 @@ async fn oidcsignin_error( state: String, error: String, error_description: Option, - conn: DbConn, + mut conn: DbConn, ) -> ApiResult { - oidcsignin_redirect( + _oidcsignin_redirect( state, - |decoded_state| sso::OIDCCodeWrapper::Error { - state: decoded_state, + OIDCCodeWrapper::Error { error, error_description, }, - &conn, + &mut conn, ) .await } // The state was encoded using Base64 to ensure no issue with providers. // iss and scope parameters are needed for redirection to work on IOS. -async fn oidcsignin_redirect( +// We pass the state as the code to get it back later on. +async fn _oidcsignin_redirect( base64_state: String, - wrapper: impl FnOnce(OIDCState) -> sso::OIDCCodeWrapper, - conn: &DbConn, + code_response: OIDCCodeWrapper, + conn: &mut DbConn, ) -> ApiResult { let state = sso::decode_state(&base64_state)?; - let code = sso::encode_code_claims(wrapper(state.clone())); - let nonce = match SsoNonce::find(&state, conn).await { - Some(n) => n, - None => err!(format!("Failed to retrieve redirect_uri with {state}")), + let mut sso_auth = match SsoAuth::find(&state, conn).await { + None => err!(format!("Cannot retrieve sso_auth for {state}")), + Some(sso_auth) => sso_auth, }; + sso_auth.code_response = Some(code_response); + sso_auth.updated_at = Utc::now().naive_utc(); + sso_auth.save(conn).await?; - let mut url = match url::Url::parse(&nonce.redirect_uri) { + let mut url = match url::Url::parse(&sso_auth.redirect_uri) { Ok(url) => url, - Err(err) => err!(format!("Failed to parse redirect uri ({}): {err}", nonce.redirect_uri)), + Err(err) => err!(format!("Failed to parse redirect uri ({}): {err}", sso_auth.redirect_uri)), }; url.query_pairs_mut() - .append_pair("code", &code) + .append_pair("code", &state) .append_pair("state", &state) .append_pair("scope", &AuthMethod::Sso.scope()) .append_pair("iss", &CONFIG.domain()); @@ -1098,10 +1098,8 @@ struct AuthorizeData { #[allow(unused)] scope: Option, state: OIDCState, - #[allow(unused)] - code_challenge: Option, - #[allow(unused)] - code_challenge_method: Option, + code_challenge: OIDCCodeChallenge, + code_challenge_method: String, #[allow(unused)] response_mode: Option, #[allow(unused)] @@ -1118,10 +1116,16 @@ async fn authorize(data: AuthorizeData, conn: DbConn) -> ApiResult { client_id, redirect_uri, state, + code_challenge, + code_challenge_method, .. } = data; - let auth_url = sso::authorize_url(state, &client_id, &redirect_uri, conn).await?; + if code_challenge_method != "S256" { + err!("Unsupported code challenge method"); + } + + let auth_url = sso::authorize_url(state, code_challenge, &client_id, &redirect_uri, conn).await?; Ok(Redirect::temporary(String::from(auth_url))) } diff --git a/src/api/push.rs b/src/api/push.rs index 4394e7d2..a7e88455 100644 --- a/src/api/push.rs +++ b/src/api/push.rs @@ -128,7 +128,7 @@ pub async fn register_push_device(device: &mut Device, conn: &DbConn) -> EmptyRe err!(format!("An error occurred while proceeding registration of a device: {e}")); } - if let Err(e) = device.save(conn).await { + if let Err(e) = device.save(true, conn).await { err!(format!("An error occurred while trying to save the (registered) device push uuid: {e}")); } diff --git a/src/auth.rs b/src/auth.rs index e10de615..6360aaf6 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1223,7 +1223,7 @@ pub async fn refresh_tokens( }; // Save to update `updated_at`. - device.save(conn).await?; + device.save(true, conn).await?; let user = match User::find_by_uuid(&device.user_uuid, conn).await { None => err!("Impossible to find user"), diff --git a/src/config.rs b/src/config.rs index 4b8b1dbf..b98f0249 100644 --- a/src/config.rs +++ b/src/config.rs @@ -564,9 +564,9 @@ make_config! { /// Duo Auth context cleanup schedule |> Cron schedule of the job that cleans expired Duo contexts from the database. Does nothing if Duo MFA is disabled or set to use the legacy iframe prompt. /// Defaults to once every minute. Set blank to disable this job. duo_context_purge_schedule: String, false, def, "30 * * * * *".to_string(); - /// Purge incomplete SSO nonce. |> Cron schedule of the job that cleans leftover nonce in db due to incomplete SSO login. + /// Purge incomplete SSO auth. |> Cron schedule of the job that cleans leftover auth in db due to incomplete SSO login. /// Defaults to daily. Set blank to disable this job. - purge_incomplete_sso_nonce: String, false, def, "0 20 0 * * *".to_string(); + purge_incomplete_sso_auth: String, false, def, "0 20 0 * * *".to_string(); }, /// General settings diff --git a/src/db/mod.rs b/src/db/mod.rs index 4fb2da75..ae2b1221 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -337,6 +337,46 @@ macro_rules! db_run { }; } +// Write all ToSql and FromSql given a serializable/deserializable type. +#[macro_export] +macro_rules! impl_FromToSqlText { + ($name:ty) => { + #[cfg(mysql)] + impl ToSql for $name { + fn to_sql<'b>(&'b self, out: &mut Output<'b, '_, diesel::mysql::Mysql>) -> diesel::serialize::Result { + serde_json::to_writer(out, self).map(|_| diesel::serialize::IsNull::No).map_err(Into::into) + } + } + + #[cfg(postgresql)] + impl ToSql for $name { + fn to_sql<'b>(&'b self, out: &mut Output<'b, '_, diesel::pg::Pg>) -> diesel::serialize::Result { + serde_json::to_writer(out, self).map(|_| diesel::serialize::IsNull::No).map_err(Into::into) + } + } + + #[cfg(sqlite)] + impl ToSql for $name { + fn to_sql<'b>(&'b self, out: &mut Output<'b, '_, diesel::sqlite::Sqlite>) -> diesel::serialize::Result { + serde_json::to_string(self).map_err(Into::into).map(|str| { + out.set_value(str); + diesel::serialize::IsNull::No + }) + } + } + + impl FromSql for $name + where + String: FromSql, + { + fn from_sql(bytes: DB::RawValue<'_>) -> diesel::deserialize::Result { + >::from_sql(bytes) + .and_then(|str| serde_json::from_str(&str).map_err(Into::into)) + } + } + }; +} + pub mod schema; // Reexport the models, needs to be after the macros are defined so it can access them diff --git a/src/db/models/device.rs b/src/db/models/device.rs index 0d86870f..4e3d0197 100644 --- a/src/db/models/device.rs +++ b/src/db/models/device.rs @@ -35,6 +35,25 @@ pub struct Device { /// Local methods impl Device { + pub fn new(uuid: DeviceId, user_uuid: UserId, name: String, atype: i32) -> Self { + let now = Utc::now().naive_utc(); + + Self { + uuid, + created_at: now, + updated_at: now, + + user_uuid, + name, + atype, + + push_uuid: Some(PushId(get_uuid())), + push_token: None, + refresh_token: crypto::encode_random_bytes::<64>(&BASE64URL), + twofactor_remember: None, + } + } + pub fn to_json(&self) -> Value { json!({ "id": self.uuid, @@ -110,38 +129,21 @@ impl DeviceWithAuthRequest { } use crate::db::DbConn; -use crate::api::{ApiResult, EmptyResult}; +use crate::api::EmptyResult; use crate::error::MapResult; /// Database methods impl Device { - pub async fn new(uuid: DeviceId, user_uuid: UserId, name: String, atype: i32, conn: &DbConn) -> ApiResult { - let now = Utc::now().naive_utc(); - - let device = Self { - uuid, - created_at: now, - updated_at: now, - - user_uuid, - name, - atype, - - push_uuid: Some(PushId(get_uuid())), - push_token: None, - refresh_token: crypto::encode_random_bytes::<64>(&BASE64URL), - twofactor_remember: None, - }; - - device.inner_save(conn).await.map(|()| device) - } + pub async fn save(&mut self, update_time: bool, conn: &DbConn) -> EmptyResult { + if update_time { + self.updated_at = Utc::now().naive_utc(); + } - async fn inner_save(&self, conn: &DbConn) -> EmptyResult { db_run! { conn: sqlite, mysql { crate::util::retry(|| diesel::replace_into(devices::table) - .values(self) + .values(&*self) .execute(conn), 10, ).map_res("Error saving device") @@ -149,10 +151,10 @@ impl Device { postgresql { crate::util::retry(|| diesel::insert_into(devices::table) - .values(self) + .values(&*self) .on_conflict((devices::uuid, devices::user_uuid)) .do_update() - .set(self) + .set(&*self) .execute(conn), 10, ).map_res("Error saving device") @@ -160,12 +162,6 @@ impl Device { } } - // Should only be called after user has passed authentication - pub async fn save(&mut self, conn: &DbConn) -> EmptyResult { - self.updated_at = Utc::now().naive_utc(); - self.inner_save(conn).await - } - pub async fn delete_all_by_user(user_uuid: &UserId, conn: &DbConn) -> EmptyResult { db_run! { conn: { diesel::delete(devices::table.filter(devices::user_uuid.eq(user_uuid))) diff --git a/src/db/models/mod.rs b/src/db/models/mod.rs index 75c58626..b4fcf658 100644 --- a/src/db/models/mod.rs +++ b/src/db/models/mod.rs @@ -11,7 +11,7 @@ mod group; mod org_policy; mod organization; mod send; -mod sso_nonce; +mod sso_auth; mod two_factor; mod two_factor_duo_context; mod two_factor_incomplete; @@ -36,7 +36,7 @@ pub use self::send::{ id::{SendFileId, SendId}, Send, SendType, }; -pub use self::sso_nonce::SsoNonce; +pub use self::sso_auth::{OIDCAuthenticatedUser, OIDCCodeWrapper, SsoAuth}; pub use self::two_factor::{TwoFactor, TwoFactorType}; pub use self::two_factor_duo_context::TwoFactorDuoContext; pub use self::two_factor_incomplete::TwoFactorIncomplete; diff --git a/src/db/models/sso_auth.rs b/src/db/models/sso_auth.rs new file mode 100644 index 00000000..fec0433a --- /dev/null +++ b/src/db/models/sso_auth.rs @@ -0,0 +1,134 @@ +use chrono::{NaiveDateTime, Utc}; +use std::time::Duration; + +use crate::api::EmptyResult; +use crate::db::schema::sso_auth; +use crate::db::{DbConn, DbPool}; +use crate::error::MapResult; +use crate::sso::{OIDCCode, OIDCCodeChallenge, OIDCIdentifier, OIDCState, SSO_AUTH_EXPIRATION}; + +use diesel::deserialize::FromSql; +use diesel::expression::AsExpression; +use diesel::prelude::*; +use diesel::serialize::{Output, ToSql}; +use diesel::sql_types::Text; + +#[derive(AsExpression, Clone, Debug, Serialize, Deserialize, FromSqlRow)] +#[diesel(sql_type = Text)] +pub enum OIDCCodeWrapper { + Ok { + code: OIDCCode, + }, + Error { + error: String, + error_description: Option, + }, +} + +impl_FromToSqlText!(OIDCCodeWrapper); + +#[derive(AsExpression, Clone, Debug, Serialize, Deserialize, FromSqlRow)] +#[diesel(sql_type = Text)] +pub struct OIDCAuthenticatedUser { + pub refresh_token: Option, + pub access_token: String, + pub expires_in: Option, + pub identifier: OIDCIdentifier, + pub email: String, + pub email_verified: Option, + pub user_name: Option, +} + +impl_FromToSqlText!(OIDCAuthenticatedUser); + +#[derive(Identifiable, Queryable, Insertable, AsChangeset, Selectable)] +#[diesel(table_name = sso_auth)] +#[diesel(treat_none_as_null = true)] +#[diesel(primary_key(state))] +pub struct SsoAuth { + pub state: OIDCState, + pub client_challenge: OIDCCodeChallenge, + pub nonce: String, + pub redirect_uri: String, + pub code_response: Option, + pub auth_response: Option, + pub created_at: NaiveDateTime, + pub updated_at: NaiveDateTime, +} + +/// Local methods +impl SsoAuth { + pub fn new(state: OIDCState, client_challenge: OIDCCodeChallenge, nonce: String, redirect_uri: String) -> Self { + let now = Utc::now().naive_utc(); + + SsoAuth { + state, + client_challenge, + nonce, + redirect_uri, + created_at: now, + updated_at: now, + code_response: None, + auth_response: None, + } + } +} + +/// Database methods +impl SsoAuth { + pub async fn save(&self, conn: &DbConn) -> EmptyResult { + db_run! { conn: + mysql { + diesel::insert_into(sso_auth::table) + .values(self) + .on_conflict(diesel::dsl::DuplicatedKeys) + .do_update() + .set(self) + .execute(conn) + .map_res("Error saving SSO auth") + } + postgresql, sqlite { + diesel::insert_into(sso_auth::table) + .values(self) + .on_conflict(sso_auth::state) + .do_update() + .set(self) + .execute(conn) + .map_res("Error saving SSO auth") + } + } + } + + pub async fn find(state: &OIDCState, conn: &DbConn) -> Option { + let oldest = Utc::now().naive_utc() - *SSO_AUTH_EXPIRATION; + db_run! { conn: { + sso_auth::table + .filter(sso_auth::state.eq(state)) + .filter(sso_auth::created_at.ge(oldest)) + .first::(conn) + .ok() + }} + } + + pub async fn delete(self, conn: &DbConn) -> EmptyResult { + db_run! {conn: { + diesel::delete(sso_auth::table.filter(sso_auth::state.eq(self.state))) + .execute(conn) + .map_res("Error deleting sso_auth") + }} + } + + pub async fn delete_expired(pool: DbPool) -> EmptyResult { + debug!("Purging expired sso_auth"); + if let Ok(conn) = pool.get().await { + let oldest = Utc::now().naive_utc() - *SSO_AUTH_EXPIRATION; + db_run! { conn: { + diesel::delete(sso_auth::table.filter(sso_auth::created_at.lt(oldest))) + .execute(conn) + .map_res("Error deleting expired SSO nonce") + }} + } else { + err!("Failed to get DB connection while purging expired sso_auth") + } + } +} diff --git a/src/db/models/sso_nonce.rs b/src/db/models/sso_nonce.rs deleted file mode 100644 index c0e16076..00000000 --- a/src/db/models/sso_nonce.rs +++ /dev/null @@ -1,87 +0,0 @@ -use chrono::{NaiveDateTime, Utc}; - -use crate::api::EmptyResult; -use crate::db::schema::sso_nonce; -use crate::db::{DbConn, DbPool}; -use crate::error::MapResult; -use crate::sso::{OIDCState, NONCE_EXPIRATION}; -use diesel::prelude::*; - -#[derive(Identifiable, Queryable, Insertable)] -#[diesel(table_name = sso_nonce)] -#[diesel(primary_key(state))] -pub struct SsoNonce { - pub state: OIDCState, - pub nonce: String, - pub verifier: Option, - pub redirect_uri: String, - pub created_at: NaiveDateTime, -} - -/// Local methods -impl SsoNonce { - pub fn new(state: OIDCState, nonce: String, verifier: Option, redirect_uri: String) -> Self { - let now = Utc::now().naive_utc(); - - SsoNonce { - state, - nonce, - verifier, - redirect_uri, - created_at: now, - } - } -} - -/// Database methods -impl SsoNonce { - pub async fn save(&self, conn: &DbConn) -> EmptyResult { - db_run! { conn: - sqlite, mysql { - diesel::replace_into(sso_nonce::table) - .values(self) - .execute(conn) - .map_res("Error saving SSO nonce") - } - postgresql { - diesel::insert_into(sso_nonce::table) - .values(self) - .execute(conn) - .map_res("Error saving SSO nonce") - } - } - } - - pub async fn delete(state: &OIDCState, conn: &DbConn) -> EmptyResult { - db_run! { conn: { - diesel::delete(sso_nonce::table.filter(sso_nonce::state.eq(state))) - .execute(conn) - .map_res("Error deleting SSO nonce") - }} - } - - pub async fn find(state: &OIDCState, conn: &DbConn) -> Option { - let oldest = Utc::now().naive_utc() - *NONCE_EXPIRATION; - db_run! { conn: { - sso_nonce::table - .filter(sso_nonce::state.eq(state)) - .filter(sso_nonce::created_at.ge(oldest)) - .first::(conn) - .ok() - }} - } - - pub async fn delete_expired(pool: DbPool) -> EmptyResult { - debug!("Purging expired sso_nonce"); - if let Ok(conn) = pool.get().await { - let oldest = Utc::now().naive_utc() - *NONCE_EXPIRATION; - db_run! { conn: { - diesel::delete(sso_nonce::table.filter(sso_nonce::created_at.lt(oldest))) - .execute(conn) - .map_res("Error deleting expired SSO nonce") - }} - } else { - err!("Failed to get DB connection while purging expired sso_nonce") - } - } -} diff --git a/src/db/models/user.rs b/src/db/models/user.rs index c7f4e1bc..c96e0fe7 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -1,4 +1,4 @@ -use crate::db::schema::{invitations, sso_users, users}; +use crate::db::schema::{invitations, sso_users, twofactor_incomplete, users}; use chrono::{NaiveDateTime, TimeDelta, Utc}; use derive_more::{AsRef, Deref, Display, From}; use diesel::prelude::*; @@ -10,7 +10,7 @@ use super::{ use crate::{ api::EmptyResult, crypto, - db::DbConn, + db::{models::DeviceId, DbConn}, error::MapResult, sso::OIDCIdentifier, util::{format_date, get_uuid, retry}, @@ -386,6 +386,20 @@ impl User { }} } + pub async fn find_by_device_for_email2fa(device_uuid: &DeviceId, conn: &DbConn) -> Option { + if let Some(user_uuid) = db_run! ( conn: { + twofactor_incomplete::table + .filter(twofactor_incomplete::device_uuid.eq(device_uuid)) + .order_by(twofactor_incomplete::login_time.desc()) + .select(twofactor_incomplete::user_uuid) + .first::(conn) + .ok() + }) { + return Self::find_by_uuid(&user_uuid, conn).await; + } + None + } + pub async fn get_all(conn: &DbConn) -> Vec<(Self, Option)> { db_run! { conn: { users::table diff --git a/src/db/schema.rs b/src/db/schema.rs index a0f31f1e..914b4fe9 100644 --- a/src/db/schema.rs +++ b/src/db/schema.rs @@ -256,12 +256,15 @@ table! { } table! { - sso_nonce (state) { + sso_auth (state) { state -> Text, + client_challenge -> Text, nonce -> Text, - verifier -> Nullable, redirect_uri -> Text, + code_response -> Nullable, + auth_response -> Nullable, created_at -> Timestamp, + updated_at -> Timestamp, } } diff --git a/src/main.rs b/src/main.rs index b431e493..b5ff93ae 100644 --- a/src/main.rs +++ b/src/main.rs @@ -699,10 +699,10 @@ fn schedule_jobs(pool: db::DbPool) { })); } - // Purge sso nonce from incomplete flow (default to daily at 00h20). - if !CONFIG.purge_incomplete_sso_nonce().is_empty() { - sched.add(Job::new(CONFIG.purge_incomplete_sso_nonce().parse().unwrap(), || { - runtime.spawn(db::models::SsoNonce::delete_expired(pool.clone())); + // Purge sso auth from incomplete flow (default to daily at 00h20). + if !CONFIG.purge_incomplete_sso_auth().is_empty() { + sched.add(Job::new(CONFIG.purge_incomplete_sso_auth().parse().unwrap(), || { + runtime.spawn(db::models::SsoAuth::delete_expired(pool.clone())); })); } diff --git a/src/sso.rs b/src/sso.rs index 789f0a3b..ee6d707a 100644 --- a/src/sso.rs +++ b/src/sso.rs @@ -1,8 +1,7 @@ use std::{sync::LazyLock, time::Duration}; use chrono::Utc; -use derive_more::{AsRef, Deref, Display, From}; -use mini_moka::sync::Cache; +use derive_more::{AsRef, Deref, Display, From, Into}; use regex::Regex; use url::Url; @@ -11,7 +10,7 @@ use crate::{ auth, auth::{AuthMethod, AuthTokens, TokenWrapper, BW_EXPIRATION, DEFAULT_REFRESH_VALIDITY}, db::{ - models::{Device, SsoNonce, User}, + models::{Device, OIDCAuthenticatedUser, OIDCCodeWrapper, SsoAuth, SsoUser, User}, DbConn, }, sso_client::Client, @@ -20,12 +19,10 @@ use crate::{ pub static FAKE_IDENTIFIER: &str = "VW_DUMMY_IDENTIFIER_FOR_OIDC"; -static AC_CACHE: LazyLock> = - LazyLock::new(|| Cache::builder().max_capacity(1000).time_to_live(Duration::from_secs(10 * 60)).build()); - static SSO_JWT_ISSUER: LazyLock = LazyLock::new(|| format!("{}|sso", CONFIG.domain_origin())); -pub static NONCE_EXPIRATION: LazyLock = LazyLock::new(|| chrono::TimeDelta::try_minutes(10).unwrap()); +pub static SSO_AUTH_EXPIRATION: LazyLock = + LazyLock::new(|| chrono::TimeDelta::try_minutes(10).unwrap()); #[derive( Clone, @@ -47,6 +44,47 @@ pub static NONCE_EXPIRATION: LazyLock = LazyLock::new(|| chron #[from(forward)] pub struct OIDCCode(String); +#[derive( + Clone, + Debug, + Default, + DieselNewType, + FromForm, + PartialEq, + Eq, + Hash, + Serialize, + Deserialize, + AsRef, + Deref, + Display, + From, + Into, +)] +#[deref(forward)] +#[into(owned)] +pub struct OIDCCodeChallenge(String); + +#[derive( + Clone, + Debug, + Default, + DieselNewType, + FromForm, + PartialEq, + Eq, + Hash, + Serialize, + Deserialize, + AsRef, + Deref, + Display, + Into, +)] +#[deref(forward)] +#[into(owned)] +pub struct OIDCCodeVerifier(String); + #[derive( Clone, Debug, @@ -91,40 +129,6 @@ pub fn encode_ssotoken_claims() -> String { auth::encode_jwt(&claims) } -#[derive(Debug, Serialize, Deserialize)] -pub enum OIDCCodeWrapper { - Ok { - state: OIDCState, - code: OIDCCode, - }, - Error { - state: OIDCState, - error: String, - error_description: Option, - }, -} - -#[derive(Debug, Serialize, Deserialize)] -struct OIDCCodeClaims { - // Expiration time - pub exp: i64, - // Issuer - pub iss: String, - - pub code: OIDCCodeWrapper, -} - -pub fn encode_code_claims(code: OIDCCodeWrapper) -> String { - let time_now = Utc::now(); - let claims = OIDCCodeClaims { - exp: (time_now + chrono::TimeDelta::try_minutes(5).unwrap()).timestamp(), - iss: SSO_JWT_ISSUER.to_string(), - code, - }; - - auth::encode_jwt(&claims) -} - #[derive(Clone, Debug, Serialize, Deserialize)] struct BasicTokenClaims { iat: Option, @@ -178,9 +182,14 @@ pub fn decode_state(base64_state: &str) -> ApiResult { Ok(state) } -// The `nonce` allow to protect against replay attacks // redirect_uri from: https://github.com/bitwarden/server/blob/main/src/Identity/IdentityServer/ApiClient.cs -pub async fn authorize_url(state: OIDCState, client_id: &str, raw_redirect_uri: &str, conn: DbConn) -> ApiResult { +pub async fn authorize_url( + state: OIDCState, + client_challenge: OIDCCodeChallenge, + client_id: &str, + raw_redirect_uri: &str, + conn: DbConn, +) -> ApiResult { let redirect_uri = match client_id { "web" | "browser" => format!("{}/sso-connector.html", CONFIG.domain()), "desktop" | "mobile" => "bitwarden://sso-callback".to_string(), @@ -194,8 +203,8 @@ pub async fn authorize_url(state: OIDCState, client_id: &str, raw_redirect_uri: _ => err!(format!("Unsupported client {client_id}")), }; - let (auth_url, nonce) = Client::authorize_url(state, redirect_uri).await?; - nonce.save(&conn).await?; + let (auth_url, sso_auth) = Client::authorize_url(state, client_challenge, redirect_uri).await?; + sso_auth.save(&conn).await?; Ok(auth_url) } @@ -225,78 +234,45 @@ impl OIDCIdentifier { } } -#[derive(Clone, Debug)] -pub struct AuthenticatedUser { - pub refresh_token: Option, - pub access_token: String, - pub expires_in: Option, - pub identifier: OIDCIdentifier, - pub email: String, - pub email_verified: Option, - pub user_name: Option, -} - -#[derive(Clone, Debug)] -pub struct UserInformation { - pub state: OIDCState, - pub identifier: OIDCIdentifier, - pub email: String, - pub email_verified: Option, - pub user_name: Option, -} - -async fn decode_code_claims(code: &str, conn: &DbConn) -> ApiResult<(OIDCCode, OIDCState)> { - match auth::decode_jwt::(code, SSO_JWT_ISSUER.to_string()) { - Ok(code_claims) => match code_claims.code { - OIDCCodeWrapper::Ok { - state, - code, - } => Ok((code, state)), - OIDCCodeWrapper::Error { - state, - error, - error_description, - } => { - if let Err(err) = SsoNonce::delete(&state, conn).await { - error!("Failed to delete database sso_nonce using {state}: {err}") - } - err!(format!( - "SSO authorization failed: {error}, {}", - error_description.as_ref().unwrap_or(&String::new()) - )) - } - }, - Err(err) => err!(format!("Failed to decode code wrapper: {err}")), - } -} - // During the 2FA flow we will // - retrieve the user information and then only discover he needs 2FA. -// - second time we will rely on the `AC_CACHE` since the `code` has already been exchanged. -// The `nonce` will ensure that the user is authorized only once. -// We return only the `UserInformation` to force calling `redeem` to obtain the `refresh_token`. -pub async fn exchange_code(wrapped_code: &str, conn: &DbConn) -> ApiResult { +// - second time we will rely on `SsoAuth.auth_response` since the `code` has already been exchanged. +// The `SsoAuth` will ensure that the user is authorized only once. +pub async fn exchange_code( + state: &OIDCState, + client_verifier: OIDCCodeVerifier, + conn: &DbConn, +) -> ApiResult<(SsoAuth, OIDCAuthenticatedUser)> { use openidconnect::OAuth2TokenResponse; - let (code, state) = decode_code_claims(wrapped_code, conn).await?; + let mut sso_auth = match SsoAuth::find(state, conn).await { + None => err!(format!("Invalid state cannot retrieve sso auth")), + Some(sso_auth) => sso_auth, + }; - if let Some(authenticated_user) = AC_CACHE.get(&state) { - return Ok(UserInformation { - state, - identifier: authenticated_user.identifier, - email: authenticated_user.email, - email_verified: authenticated_user.email_verified, - user_name: authenticated_user.user_name, - }); + if let Some(authenticated_user) = sso_auth.auth_response.clone() { + return Ok((sso_auth, authenticated_user)); } - let nonce = match SsoNonce::find(&state, conn).await { - None => err!(format!("Invalid state cannot retrieve nonce")), - Some(nonce) => nonce, + let code = match sso_auth.code_response.clone() { + Some(OIDCCodeWrapper::Ok { + code, + }) => code.clone(), + Some(OIDCCodeWrapper::Error { + error, + error_description, + }) => { + sso_auth.delete(conn).await?; + err!(format!("SSO authorization failed: {error}, {}", error_description.as_ref().unwrap_or(&String::new()))) + } + None => { + sso_auth.delete(conn).await?; + err!("Missing authorization provider return"); + } }; let client = Client::cached().await?; - let (token_response, id_claims) = client.exchange_code(code, nonce).await?; + let (token_response, id_claims) = client.exchange_code(code, client_verifier, &sso_auth).await?; let user_info = client.user_info(token_response.access_token().to_owned()).await?; @@ -316,7 +292,7 @@ pub async fn exchange_code(wrapped_code: &str, conn: &DbConn) -> ApiResult ApiResult ApiResult { - if let Err(err) = SsoNonce::delete(state, conn).await { - error!("Failed to delete database sso_nonce using {state}: {err}") +// User has passed 2FA flow we can delete auth info from database +pub async fn redeem( + device: &Device, + user: &User, + client_id: Option, + sso_user: Option, + sso_auth: SsoAuth, + auth_user: OIDCAuthenticatedUser, + conn: &DbConn, +) -> ApiResult { + sso_auth.delete(conn).await?; + + if sso_user.is_none() { + let user_sso = SsoUser { + user_uuid: user.uuid.clone(), + identifier: auth_user.identifier.clone(), + }; + user_sso.save(conn).await?; } - if let Some(au) = AC_CACHE.get(state) { - AC_CACHE.invalidate(state); - Ok(au) + if !CONFIG.sso_auth_only_not_session() { + let now = Utc::now(); + + let (ap_nbf, ap_exp) = + match (decode_token_claims("access_token", &auth_user.access_token), auth_user.expires_in) { + (Ok(ap), _) => (ap.nbf(), ap.exp), + (Err(_), Some(exp)) => (now.timestamp(), (now + exp).timestamp()), + _ => err!("Non jwt access_token and empty expires_in"), + }; + + let access_claims = + auth::LoginJwtClaims::new(device, user, ap_nbf, ap_exp, AuthMethod::Sso.scope_vec(), client_id, now); + + _create_auth_tokens(device, auth_user.refresh_token, access_claims, auth_user.access_token) } else { - err!("Failed to retrieve user info from sso cache") + Ok(AuthTokens::new(device, user, AuthMethod::Sso, client_id)) } } diff --git a/src/sso_client.rs b/src/sso_client.rs index 5dc614e4..0d73d906 100644 --- a/src/sso_client.rs +++ b/src/sso_client.rs @@ -7,8 +7,8 @@ use url::Url; use crate::{ api::{ApiResult, EmptyResult}, - db::models::SsoNonce, - sso::{OIDCCode, OIDCState}, + db::models::SsoAuth, + sso::{OIDCCode, OIDCCodeChallenge, OIDCCodeVerifier, OIDCState}, CONFIG, }; @@ -107,7 +107,11 @@ impl Client { } // The `state` is encoded using base64 to ensure no issue with providers (It contains the Organization identifier). - pub async fn authorize_url(state: OIDCState, redirect_uri: String) -> ApiResult<(Url, SsoNonce)> { + pub async fn authorize_url( + state: OIDCState, + client_challenge: OIDCCodeChallenge, + redirect_uri: String, + ) -> ApiResult<(Url, SsoAuth)> { let scopes = CONFIG.sso_scopes_vec().into_iter().map(Scope::new); let base64_state = data_encoding::BASE64.encode(state.to_string().as_bytes()); @@ -122,22 +126,21 @@ impl Client { .add_scopes(scopes) .add_extra_params(CONFIG.sso_authorize_extra_params_vec()); - let verifier = if CONFIG.sso_pkce() { - let (pkce_challenge, pkce_verifier) = PkceCodeChallenge::new_random_sha256(); - auth_req = auth_req.set_pkce_challenge(pkce_challenge); - Some(pkce_verifier.into_secret()) - } else { - None - }; + if CONFIG.sso_pkce() { + auth_req = auth_req + .add_extra_param::<&str, String>("code_challenge", client_challenge.clone().into()) + .add_extra_param("code_challenge_method", "S256"); + } let (auth_url, _, nonce) = auth_req.url(); - Ok((auth_url, SsoNonce::new(state, nonce.secret().clone(), verifier, redirect_uri))) + Ok((auth_url, SsoAuth::new(state, client_challenge, nonce.secret().clone(), redirect_uri))) } pub async fn exchange_code( &self, code: OIDCCode, - nonce: SsoNonce, + client_verifier: OIDCCodeVerifier, + sso_auth: &SsoAuth, ) -> ApiResult<( StandardTokenResponse< IdTokenFields< @@ -155,17 +158,21 @@ impl Client { let mut exchange = self.core_client.exchange_code(oidc_code); + let verifier = PkceCodeVerifier::new(client_verifier.into()); if CONFIG.sso_pkce() { - match nonce.verifier { - None => err!(format!("Missing verifier in the DB nonce table")), - Some(secret) => exchange = exchange.set_pkce_verifier(PkceCodeVerifier::new(secret)), + exchange = exchange.set_pkce_verifier(verifier); + } else { + let challenge = PkceCodeChallenge::from_code_verifier_sha256(&verifier); + if challenge.as_str() != String::from(sso_auth.client_challenge.clone()) { + err!(format!("PKCE client challenge failed")) + // Might need to notify admin ? how ? } } match exchange.request_async(&self.http_client).await { Err(err) => err!(format!("Failed to contact token endpoint: {:?}", err)), Ok(token_response) => { - let oidc_nonce = Nonce::new(nonce.nonce); + let oidc_nonce = Nonce::new(sso_auth.nonce.clone()); let id_token = match token_response.extra_fields().id_token() { None => err!("Token response did not contain an id_token"),