Skip to content

Commit

Permalink
Base64 encode state before sending it to providers
Browse files Browse the repository at this point in the history
  • Loading branch information
Timshel committed Nov 28, 2024
1 parent 982a6a1 commit 96fd189
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 13 deletions.
28 changes: 20 additions & 8 deletions src/api/identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -937,10 +937,10 @@ fn prevalidate() -> JsonResult {
#[get("/connect/oidc-signin?<code>&<state>", rank = 1)]
async fn oidcsignin(code: String, state: String, conn: DbConn) -> ApiResult<Redirect> {
oidcsignin_redirect(
state.clone(),
sso::OIDCCodeWrapper::Ok {
state,
|decoded_state| sso::OIDCCodeWrapper::Ok {
state: decoded_state,
code,
state,
},
&conn,
)
Expand All @@ -957,9 +957,9 @@ async fn oidcsignin_error(
conn: DbConn,
) -> ApiResult<Redirect> {
oidcsignin_redirect(
state.clone(),
sso::OIDCCodeWrapper::Error {
state,
state,
|decoded_state| sso::OIDCCodeWrapper::Error {
state: decoded_state,
error,
error_description,
},
Expand All @@ -968,9 +968,21 @@ async fn oidcsignin_error(
.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(state: String, wrapper: sso::OIDCCodeWrapper, conn: &DbConn) -> ApiResult<Redirect> {
let code = sso::encode_code_claims(wrapper);
async fn oidcsignin_redirect(
base64_state: String,
wrapper: impl FnOnce(String) -> sso::OIDCCodeWrapper,
conn: &DbConn,
) -> ApiResult<Redirect> {
let state = match data_encoding::BASE64.decode(base64_state.as_bytes()) {
Ok(vec) => match String::from_utf8(vec) {
Ok(valid) => valid,
Err(_) => err!(format!("Invalid utf8 chars in {base64_state} after base64 decoding")),
},
Err(_) => err!(format!("Failed to decode {base64_state} using base64")),
};
let code = sso::encode_code_claims(wrapper(state.clone()));

let nonce = match SsoNonce::find(&state, conn).await {
Some(n) => n,
Expand Down
12 changes: 7 additions & 5 deletions src/sso.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ pub fn encode_ssotoken_claims() -> String {
#[derive(Debug, Serialize, Deserialize)]
pub enum OIDCCodeWrapper {
Ok {
code: String,
state: String,
code: String,
},
Error {
state: String,
Expand Down Expand Up @@ -209,9 +209,11 @@ impl CoreClientExt for CoreClient {
}

// The `nonce` allow to protect against replay attacks
// The `state` is encoded using base64 to ensure no issue with providers (It contains the Organization identifier).
// redirect_uri from: https://github.com/bitwarden/server/blob/main/src/Identity/IdentityServer/ApiClient.cs
pub async fn authorize_url(state: String, client_id: &str, raw_redirect_uri: &str, mut conn: DbConn) -> ApiResult<Url> {
let scopes = CONFIG.sso_scopes_vec().into_iter().map(Scope::new);
let base64_state = data_encoding::BASE64.encode(state.as_bytes());

let redirect_uri = match client_id {
"web" | "browser" => format!("{}/sso-connector.html", CONFIG.domain()),
Expand All @@ -230,7 +232,7 @@ pub async fn authorize_url(state: String, client_id: &str, raw_redirect_uri: &st
let mut auth_req = client
.authorize_url(
AuthenticationFlow::<CoreResponseType>::AuthorizationCode,
|| CsrfToken::new(state),
|| CsrfToken::new(base64_state),
Nonce::new_random,
)
.add_scopes(scopes)
Expand All @@ -244,9 +246,9 @@ pub async fn authorize_url(state: String, client_id: &str, raw_redirect_uri: &st
None
};

let (auth_url, csrf_state, nonce) = auth_req.url();
let (auth_url, _, nonce) = auth_req.url();

let sso_nonce = SsoNonce::new(csrf_state.secret().to_string(), nonce.secret().to_string(), verifier, redirect_uri);
let sso_nonce = SsoNonce::new(state, nonce.secret().to_string(), verifier, redirect_uri);
sso_nonce.save(&mut conn).await?;

Ok(auth_url)
Expand Down Expand Up @@ -276,8 +278,8 @@ async fn decode_code_claims(code: &str, conn: &mut DbConn) -> ApiResult<(String,
match auth::decode_jwt::<OIDCCodeClaims>(code, SSO_JWT_ISSUER.to_string()) {
Ok(code_claims) => match code_claims.code {
OIDCCodeWrapper::Ok {
code,
state,
code,
} => Ok((code, state)),
OIDCCodeWrapper::Error {
state,
Expand Down

0 comments on commit 96fd189

Please sign in to comment.