mirror of
https://github.com/dani-garcia/vaultwarden.git
synced 2026-05-26 22:10:19 +03:00
sso_auth improvements (#7197)
Co-authored-by: Timshel <timshel@users.noreply.github.com>
This commit is contained in:
@@ -0,0 +1 @@
|
|||||||
|
ALTER TABLE sso_auth DROP COLUMN code_response_error;
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
ALTER TABLE sso_auth ADD COLUMN code_response_error TEXT;
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
ALTER TABLE sso_auth DROP COLUMN IF EXISTS code_response_error;
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
ALTER TABLE sso_auth ADD COLUMN IF NOT EXISTS code_response_error TEXT;
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
ALTER TABLE sso_auth DROP COLUMN code_response_error;
|
||||||
@@ -0,0 +1 @@
|
|||||||
|
ALTER TABLE sso_auth ADD COLUMN code_response_error TEXT;
|
||||||
+19
-23
@@ -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());
|
||||||
|
|||||||
@@ -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;
|
||||||
|
|||||||
+18
-10
@@ -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)))
|
||||||
|
|||||||
@@ -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,
|
||||||
|
|||||||
+14
-14
@@ -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");
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user