Browse Source

sso_auth improvements

pull/7197/head
Timshel 3 weeks ago
parent
commit
1eddcd6563
  1. 1
      migrations/mysql/2026-05-05-120000_sso_auth_error/down.sql
  2. 1
      migrations/mysql/2026-05-05-120000_sso_auth_error/up.sql
  3. 1
      migrations/postgresql/2026-05-05-120000_sso_auth_error/down.sql
  4. 1
      migrations/postgresql/2026-05-05-120000_sso_auth_error/up.sql
  5. 1
      migrations/sqlite/2026-05-05-120000_sso_auth_error/down.sql
  6. 1
      migrations/sqlite/2026-05-05-120000_sso_auth_error/up.sql
  7. 42
      src/api/identity.rs
  8. 2
      src/db/models/mod.rs
  9. 28
      src/db/models/sso_auth.rs
  10. 1
      src/db/schema.rs
  11. 28
      src/sso.rs

1
migrations/mysql/2026-05-05-120000_sso_auth_error/down.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth DROP COLUMN code_response_error;

1
migrations/mysql/2026-05-05-120000_sso_auth_error/up.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth ADD COLUMN code_response_error TEXT;

1
migrations/postgresql/2026-05-05-120000_sso_auth_error/down.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth DROP COLUMN IF EXISTS code_response_error;

1
migrations/postgresql/2026-05-05-120000_sso_auth_error/up.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth ADD COLUMN IF NOT EXISTS code_response_error TEXT;

1
migrations/sqlite/2026-05-05-120000_sso_auth_error/down.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth DROP COLUMN code_response_error;

1
migrations/sqlite/2026-05-05-120000_sso_auth_error/up.sql

@ -0,0 +1 @@
ALTER TABLE sso_auth ADD COLUMN code_response_error TEXT;

42
src/api/identity.rs

@ -28,8 +28,9 @@ use crate::{
crypto, crypto,
db::{ db::{
models::{ models::{
AuthRequest, AuthRequestId, Device, DeviceId, EventType, Invitation, OIDCCodeWrapper, OrganizationApiKey, AuthRequest, AuthRequestId, Device, DeviceId, EventType, Invitation, OIDCCodeResponseError,
OrganizationId, SsoAuth, SsoUser, TwoFactor, TwoFactorIncomplete, TwoFactorType, User, UserId, OrganizationApiKey, OrganizationId, SsoAuth, SsoUser, TwoFactor, TwoFactorIncomplete, TwoFactorType, User,
UserId,
}, },
DbConn, DbConn,
}, },
@ -186,7 +187,7 @@ async fn _sso_login(
// Ratelimit the login // Ratelimit the login
crate::ratelimit::check_limit_login(&ip.ip)?; crate::ratelimit::check_limit_login(&ip.ip)?;
let (state, code_verifier) = match (data.code.as_ref(), data.code_verifier.as_ref()) { let (code, code_verifier) = match (data.code.as_ref(), data.code_verifier.as_ref()) {
(None, _) => err!( (None, _) => err!(
"Got no code in OIDC data", "Got no code in OIDC data",
ErrorEvent { ErrorEvent {
@ -202,7 +203,7 @@ async fn _sso_login(
(Some(code), Some(code_verifier)) => (code, code_verifier.clone()), (Some(code), Some(code_verifier)) => (code, code_verifier.clone()),
}; };
let (sso_auth, user_infos) = sso::exchange_code(state, code_verifier, conn).await?; let (sso_auth, user_infos) = sso::exchange_code(code, code_verifier, conn).await?;
let user_with_sso = match SsoUser::find_by_identifier(&user_infos.identifier, 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 => match SsoUser::find_by_mail(&user_infos.email, conn).await {
None => None, None => None,
@ -1138,7 +1139,7 @@ struct ConnectData {
// Needed for authorization code // Needed for authorization code
#[field(name = uncased("code"))] #[field(name = uncased("code"))]
code: Option<OIDCState>, code: Option<OIDCCode>,
#[field(name = uncased("code_verifier"))] #[field(name = uncased("code_verifier"))]
code_verifier: Option<OIDCCodeVerifier>, code_verifier: Option<OIDCCodeVerifier>,
} }
@ -1165,19 +1166,12 @@ const SSO_BINDING_COOKIE: &str = "VW_SSO_BINDING";
#[get("/connect/oidc-signin?<code>&<state>", rank = 1)] #[get("/connect/oidc-signin?<code>&<state>", rank = 1)]
async fn oidcsignin(code: OIDCCode, state: String, cookies: &CookieJar<'_>, mut conn: DbConn) -> ApiResult<Redirect> { async fn oidcsignin(code: OIDCCode, state: String, cookies: &CookieJar<'_>, mut conn: DbConn) -> ApiResult<Redirect> {
_oidcsignin_redirect( _oidcsignin_redirect(state, code, None, cookies, &mut conn).await
state,
OIDCCodeWrapper::Ok {
code,
},
cookies,
&mut conn,
)
.await
} }
// Bitwarden client appear to only care for code and state so we pipe it through // Bitwarden client appear to only care for code and state
// cf: https://github.com/bitwarden/clients/blob/80b74b3300e15b4ae414dc06044cc9b02b6c10a6/libs/auth/src/angular/sso/sso.component.ts#L141 // We save the error in the database and set the encoded state as the code to be able to retrieve them later on
// cf: https://github.com/bitwarden/clients/blob/afd36d290ce18fb0048e0575e7d5a8f78b5dbffc/libs/auth/src/angular/sso/sso.component.ts#L156
#[get("/connect/oidc-signin?<state>&<error>&<error_description>", rank = 2)] #[get("/connect/oidc-signin?<state>&<error>&<error_description>", rank = 2)]
async fn oidcsignin_error( async fn oidcsignin_error(
state: String, state: String,
@ -1187,11 +1181,12 @@ async fn oidcsignin_error(
mut conn: DbConn, mut conn: DbConn,
) -> ApiResult<Redirect> { ) -> ApiResult<Redirect> {
_oidcsignin_redirect( _oidcsignin_redirect(
state, state.clone(),
OIDCCodeWrapper::Error { state.into(),
Some(OIDCCodeResponseError {
error, error,
error_description, error_description,
}, }),
cookies, cookies,
&mut conn, &mut conn,
) )
@ -1200,10 +1195,10 @@ async fn oidcsignin_error(
// The state was encoded using Base64 to ensure no issue with providers. // The state was encoded using Base64 to ensure no issue with providers.
// iss and scope parameters are needed for redirection to work on IOS. // iss and scope parameters are needed for redirection to work on IOS.
// We pass the state as the code to get it back later on.
async fn _oidcsignin_redirect( async fn _oidcsignin_redirect(
base64_state: String, base64_state: String,
code_response: OIDCCodeWrapper, code: OIDCCode,
error: Option<OIDCCodeResponseError>,
cookies: &CookieJar<'_>, cookies: &CookieJar<'_>,
conn: &mut DbConn, conn: &mut DbConn,
) -> ApiResult<Redirect> { ) -> ApiResult<Redirect> {
@ -1225,7 +1220,8 @@ async fn _oidcsignin_redirect(
cookies cookies
.remove(Cookie::build(SSO_BINDING_COOKIE).path(format!("{}/identity/connect/", CONFIG.domain_path())).build()); .remove(Cookie::build(SSO_BINDING_COOKIE).path(format!("{}/identity/connect/", CONFIG.domain_path())).build());
sso_auth.code_response = Some(code_response); sso_auth.code_response = Some(code.clone());
sso_auth.code_response_error = error;
sso_auth.updated_at = Utc::now().naive_utc(); sso_auth.updated_at = Utc::now().naive_utc();
sso_auth.save(conn).await?; sso_auth.save(conn).await?;
@ -1235,7 +1231,7 @@ async fn _oidcsignin_redirect(
}; };
url.query_pairs_mut() url.query_pairs_mut()
.append_pair("code", &state) .append_pair("code", &code)
.append_pair("state", &state) .append_pair("state", &state)
.append_pair("scope", &AuthMethod::Sso.scope()) .append_pair("scope", &AuthMethod::Sso.scope())
.append_pair("iss", &CONFIG.domain()); .append_pair("iss", &CONFIG.domain());

2
src/db/models/mod.rs

@ -38,7 +38,7 @@ pub use self::send::{
id::{SendFileId, SendId}, id::{SendFileId, SendId},
Send, SendType, Send, SendType,
}; };
pub use self::sso_auth::{OIDCAuthenticatedUser, OIDCCodeWrapper, SsoAuth}; pub use self::sso_auth::{OIDCAuthenticatedUser, OIDCCodeResponseError, SsoAuth};
pub use self::two_factor::{TwoFactor, TwoFactorType}; pub use self::two_factor::{TwoFactor, TwoFactorType};
pub use self::two_factor_duo_context::TwoFactorDuoContext; pub use self::two_factor_duo_context::TwoFactorDuoContext;
pub use self::two_factor_incomplete::TwoFactorIncomplete; pub use self::two_factor_incomplete::TwoFactorIncomplete;

28
src/db/models/sso_auth.rs

@ -15,17 +15,12 @@ use diesel::sql_types::Text;
#[derive(AsExpression, Clone, Debug, Serialize, Deserialize, FromSqlRow)] #[derive(AsExpression, Clone, Debug, Serialize, Deserialize, FromSqlRow)]
#[diesel(sql_type = Text)] #[diesel(sql_type = Text)]
pub enum OIDCCodeWrapper { pub struct OIDCCodeResponseError {
Ok { pub error: String,
code: OIDCCode, pub error_description: Option<String>,
},
Error {
error: String,
error_description: Option<String>,
},
} }
impl_FromToSqlText!(OIDCCodeWrapper); impl_FromToSqlText!(OIDCCodeResponseError);
#[derive(AsExpression, Clone, Debug, Serialize, Deserialize, FromSqlRow)] #[derive(AsExpression, Clone, Debug, Serialize, Deserialize, FromSqlRow)]
#[diesel(sql_type = Text)] #[diesel(sql_type = Text)]
@ -50,7 +45,8 @@ pub struct SsoAuth {
pub client_challenge: OIDCCodeChallenge, pub client_challenge: OIDCCodeChallenge,
pub nonce: String, pub nonce: String,
pub redirect_uri: String, pub redirect_uri: String,
pub code_response: Option<OIDCCodeWrapper>, pub code_response: Option<OIDCCode>,
pub code_response_error: Option<OIDCCodeResponseError>,
pub auth_response: Option<OIDCAuthenticatedUser>, pub auth_response: Option<OIDCAuthenticatedUser>,
pub created_at: NaiveDateTime, pub created_at: NaiveDateTime,
pub updated_at: NaiveDateTime, pub updated_at: NaiveDateTime,
@ -76,6 +72,7 @@ impl SsoAuth {
created_at: now, created_at: now,
updated_at: now, updated_at: now,
code_response: None, code_response: None,
code_response_error: None,
auth_response: None, auth_response: None,
binding_hash, binding_hash,
} }
@ -118,6 +115,17 @@ impl SsoAuth {
}} }}
} }
pub async fn find_by_code(code: &OIDCCode, conn: &DbConn) -> Option<Self> {
let oldest = Utc::now().naive_utc() - *SSO_AUTH_EXPIRATION;
db_run! { conn: {
sso_auth::table
.filter(sso_auth::code_response.eq(code))
.filter(sso_auth::created_at.ge(oldest))
.first::<Self>(conn)
.ok()
}}
}
pub async fn delete(self, conn: &DbConn) -> EmptyResult { pub async fn delete(self, conn: &DbConn) -> EmptyResult {
db_run! {conn: { db_run! {conn: {
diesel::delete(sso_auth::table.filter(sso_auth::state.eq(self.state))) diesel::delete(sso_auth::table.filter(sso_auth::state.eq(self.state)))

1
src/db/schema.rs

@ -262,6 +262,7 @@ table! {
nonce -> Text, nonce -> Text,
redirect_uri -> Text, redirect_uri -> Text,
code_response -> Nullable<Text>, code_response -> Nullable<Text>,
code_response_error -> Nullable<Text>,
auth_response -> Nullable<Text>, auth_response -> Nullable<Text>,
created_at -> Timestamp, created_at -> Timestamp,
updated_at -> Timestamp, updated_at -> Timestamp,

28
src/sso.rs

@ -10,7 +10,7 @@ use crate::{
auth, auth,
auth::{AuthMethod, AuthTokens, TokenWrapper, BW_EXPIRATION, DEFAULT_REFRESH_VALIDITY}, auth::{AuthMethod, AuthTokens, TokenWrapper, BW_EXPIRATION, DEFAULT_REFRESH_VALIDITY},
db::{ db::{
models::{Device, OIDCAuthenticatedUser, OIDCCodeWrapper, SsoAuth, SsoUser, User}, models::{Device, OIDCAuthenticatedUser, SsoAuth, SsoUser, User},
DbConn, DbConn,
}, },
sso_client::Client, sso_client::Client,
@ -240,14 +240,14 @@ impl OIDCIdentifier {
// - second time we will rely on `SsoAuth.auth_response` since the `code` has already been exchanged. // - 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. // The `SsoAuth` will ensure that the user is authorized only once.
pub async fn exchange_code( pub async fn exchange_code(
state: &OIDCState, code: &OIDCCode,
client_verifier: OIDCCodeVerifier, client_verifier: OIDCCodeVerifier,
conn: &DbConn, conn: &DbConn,
) -> ApiResult<(SsoAuth, OIDCAuthenticatedUser)> { ) -> ApiResult<(SsoAuth, OIDCAuthenticatedUser)> {
use openidconnect::OAuth2TokenResponse; use openidconnect::OAuth2TokenResponse;
let mut sso_auth = match SsoAuth::find(state, conn).await { let mut sso_auth = match SsoAuth::find_by_code(code, conn).await {
None => err!(format!("Invalid state cannot retrieve sso auth")), None => err!(format!("Invalid code cannot retrieve sso auth")),
Some(sso_auth) => sso_auth, Some(sso_auth) => sso_auth,
}; };
@ -255,18 +255,18 @@ pub async fn exchange_code(
return Ok((sso_auth, authenticated_user)); return Ok((sso_auth, authenticated_user));
} }
let code = match sso_auth.code_response.clone() { let code = match (sso_auth.code_response.clone(), sso_auth.code_response_error.as_ref()) {
Some(OIDCCodeWrapper::Ok { (Some(code), None) => code,
code, (_, Some(re)) => {
}) => code.clone(), let error_msg = format!(
Some(OIDCCodeWrapper::Error { "SSO authorization failed: {}, {}",
error, re.error,
error_description, re.error_description.as_ref().unwrap_or(&String::new())
}) => { );
sso_auth.delete(conn).await?; sso_auth.delete(conn).await?;
err!(format!("SSO authorization failed: {error}, {}", error_description.as_ref().unwrap_or(&String::new()))) err!(error_msg);
} }
None => { (None, _) => {
sso_auth.delete(conn).await?; sso_auth.delete(conn).await?;
err!("Missing authorization provider return"); err!("Missing authorization provider return");
} }

Loading…
Cancel
Save