sso_auth improvements

This commit is contained in:
Timshel 2026-05-05 18:25:17 +02:00
parent f21a3adae2
commit 9c758349a4
11 changed files with 68 additions and 48 deletions

View File

@ -0,0 +1,3 @@
ALTER TABLE sso_auth DROP COLUMN code_response_error;
DROP INDEX code_response_index;
ALTER TABLE sso_auth MODIFY COLUMN code_response TEXT;

View File

@ -0,0 +1,4 @@
DELETE FROM sso_auth;
ALTER TABLE sso_auth ADD COLUMN code_response_error TEXT;
ALTER TABLE sso_auth MODIFY COLUMN code_response VARCHAR(768);
CREATE INDEX code_response_index ON sso_auth(code_response);

View File

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

View File

@ -0,0 +1,2 @@
ALTER TABLE sso_auth ADD COLUMN IF NOT EXISTS code_response_error TEXT;
CREATE INDEX IF NOT EXISTS code_response_index ON sso_auth(code_response);

View File

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

View File

@ -0,0 +1,2 @@
ALTER TABLE sso_auth ADD COLUMN code_response_error TEXT;
CREATE INDEX code_response_index ON sso_auth(code_response);

View File

@ -28,8 +28,9 @@ use crate::{
crypto,
db::{
models::{
AuthRequest, AuthRequestId, Device, DeviceId, EventType, Invitation, OIDCCodeWrapper, OrganizationApiKey,
OrganizationId, SsoAuth, SsoUser, TwoFactor, TwoFactorIncomplete, TwoFactorType, User, UserId,
AuthRequest, AuthRequestId, Device, DeviceId, EventType, Invitation, OIDCCodeResponseError,
OrganizationApiKey, OrganizationId, SsoAuth, SsoUser, TwoFactor, TwoFactorIncomplete, TwoFactorType, User,
UserId,
},
DbConn,
},
@ -186,7 +187,7 @@ async fn _sso_login(
// Ratelimit the login
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!(
"Got no code in OIDC data",
ErrorEvent {
@ -202,7 +203,7 @@ async fn _sso_login(
(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 {
None => match SsoUser::find_by_mail(&user_infos.email, conn).await {
None => None,
@ -1138,7 +1139,7 @@ struct ConnectData {
// Needed for authorization code
#[field(name = uncased("code"))]
code: Option<OIDCState>,
code: Option<OIDCCode>,
#[field(name = uncased("code_verifier"))]
code_verifier: Option<OIDCCodeVerifier>,
}
@ -1165,19 +1166,12 @@ const SSO_BINDING_COOKIE: &str = "VW_SSO_BINDING";
#[get("/connect/oidc-signin?<code>&<state>", rank = 1)]
async fn oidcsignin(code: OIDCCode, state: String, cookies: &CookieJar<'_>, mut conn: DbConn) -> ApiResult<Redirect> {
_oidcsignin_redirect(
state,
OIDCCodeWrapper::Ok {
code,
},
cookies,
&mut conn,
)
.await
_oidcsignin_redirect(state, code, None, cookies, &mut conn).await
}
// Bitwarden client appear to only care for code and state so we pipe it through
// cf: https://github.com/bitwarden/clients/blob/80b74b3300e15b4ae414dc06044cc9b02b6c10a6/libs/auth/src/angular/sso/sso.component.ts#L141
// Bitwarden client appear to only care for code and state
// 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)]
async fn oidcsignin_error(
state: String,
@ -1187,11 +1181,12 @@ async fn oidcsignin_error(
mut conn: DbConn,
) -> ApiResult<Redirect> {
_oidcsignin_redirect(
state,
OIDCCodeWrapper::Error {
state.clone(),
state.into(),
Some(OIDCCodeResponseError {
error,
error_description,
},
}),
cookies,
&mut conn,
)
@ -1200,10 +1195,10 @@ async fn oidcsignin_error(
// The state was encoded using Base64 to ensure no issue with providers.
// 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(
base64_state: String,
code_response: OIDCCodeWrapper,
code: OIDCCode,
error: Option<OIDCCodeResponseError>,
cookies: &CookieJar<'_>,
conn: &mut DbConn,
) -> ApiResult<Redirect> {
@ -1224,7 +1219,8 @@ async fn _oidcsignin_redirect(
}
cookies.remove(Cookie::build(SSO_BINDING_COOKIE).path("/identity/connect/").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.save(conn).await?;
@ -1234,7 +1230,7 @@ async fn _oidcsignin_redirect(
};
url.query_pairs_mut()
.append_pair("code", &state)
.append_pair("code", &code)
.append_pair("state", &state)
.append_pair("scope", &AuthMethod::Sso.scope())
.append_pair("iss", &CONFIG.domain());

View File

@ -38,7 +38,7 @@ pub use self::send::{
id::{SendFileId, SendId},
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_duo_context::TwoFactorDuoContext;
pub use self::two_factor_incomplete::TwoFactorIncomplete;

View File

@ -15,17 +15,12 @@ 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<String>,
},
pub struct OIDCCodeResponseError {
pub error: String,
pub error_description: Option<String>,
}
impl_FromToSqlText!(OIDCCodeWrapper);
impl_FromToSqlText!(OIDCCodeResponseError);
#[derive(AsExpression, Clone, Debug, Serialize, Deserialize, FromSqlRow)]
#[diesel(sql_type = Text)]
@ -50,7 +45,8 @@ pub struct SsoAuth {
pub client_challenge: OIDCCodeChallenge,
pub nonce: 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 created_at: NaiveDateTime,
pub updated_at: NaiveDateTime,
@ -76,6 +72,7 @@ impl SsoAuth {
created_at: now,
updated_at: now,
code_response: None,
code_response_error: None,
auth_response: None,
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 {
db_run! {conn: {
diesel::delete(sso_auth::table.filter(sso_auth::state.eq(self.state)))

View File

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

View File

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