From bba76c856a426b3045b7557e99712a4390d052d1 Mon Sep 17 00:00:00 2001 From: Stuart Heap Date: Mon, 20 Sep 2021 13:09:57 +0200 Subject: [PATCH] add sso_nonce to database, with checking --- Cargo.lock | 7 - .../mysql/2021-09-16-133000_add-sso/up.sql | 6 + .../2021-09-16-133000_add_sso/up.sql | 6 + .../sqlite/2021-09-16-133000_add_sso/up.sql | 6 + src/api/identity.rs | 161 +++++++++--------- src/db/models/mod.rs | 2 + src/db/schemas/mysql/schema.rs | 9 + src/db/schemas/postgresql/schema.rs | 9 + src/db/schemas/sqlite/schema.rs | 9 + 9 files changed, 132 insertions(+), 83 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 68da3632..790bc0c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3268,12 +3268,6 @@ dependencies = [ "serde", ] -[[package]] -name = "urlencoding" -version = "1.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a1f0175e03a0973cf4afd476bef05c26e228520400eb1fd473ad417b1c00ffb" - [[package]] name = "utf-8" version = "0.7.6" @@ -3340,7 +3334,6 @@ dependencies = [ "tracing", "u2f", "url 2.2.2", - "urlencoding", "uuid", "webauthn-rs", "yubico", diff --git a/migrations/mysql/2021-09-16-133000_add-sso/up.sql b/migrations/mysql/2021-09-16-133000_add-sso/up.sql index fc9465c5..38a8ecbb 100644 --- a/migrations/mysql/2021-09-16-133000_add-sso/up.sql +++ b/migrations/mysql/2021-09-16-133000_add-sso/up.sql @@ -5,3 +5,9 @@ ALTER TABLE organizations ADD COLUMN signed_out_callback_path TEXT NOT NULL; ALTER TABLE organizations ADD COLUMN authority TEXT; ALTER TABLE organizations ADD COLUMN client_id TEXT; ALTER TABLE organizations ADD COLUMN client_secret TEXT; + +CREATE TABLE sso_nonce ( + uuid CHAR(36) NOT NULL PRIMARY KEY, + org_uuid CHAR(36) NOT NULL REFERENCES organizations (uuid), + nonce CHAR(36) NOT NULL +); diff --git a/migrations/postgresql/2021-09-16-133000_add_sso/up.sql b/migrations/postgresql/2021-09-16-133000_add_sso/up.sql index fc9465c5..38a8ecbb 100644 --- a/migrations/postgresql/2021-09-16-133000_add_sso/up.sql +++ b/migrations/postgresql/2021-09-16-133000_add_sso/up.sql @@ -5,3 +5,9 @@ ALTER TABLE organizations ADD COLUMN signed_out_callback_path TEXT NOT NULL; ALTER TABLE organizations ADD COLUMN authority TEXT; ALTER TABLE organizations ADD COLUMN client_id TEXT; ALTER TABLE organizations ADD COLUMN client_secret TEXT; + +CREATE TABLE sso_nonce ( + uuid CHAR(36) NOT NULL PRIMARY KEY, + org_uuid CHAR(36) NOT NULL REFERENCES organizations (uuid), + nonce CHAR(36) NOT NULL +); diff --git a/migrations/sqlite/2021-09-16-133000_add_sso/up.sql b/migrations/sqlite/2021-09-16-133000_add_sso/up.sql index fc9465c5..38a8ecbb 100644 --- a/migrations/sqlite/2021-09-16-133000_add_sso/up.sql +++ b/migrations/sqlite/2021-09-16-133000_add_sso/up.sql @@ -5,3 +5,9 @@ ALTER TABLE organizations ADD COLUMN signed_out_callback_path TEXT NOT NULL; ALTER TABLE organizations ADD COLUMN authority TEXT; ALTER TABLE organizations ADD COLUMN client_id TEXT; ALTER TABLE organizations ADD COLUMN client_secret TEXT; + +CREATE TABLE sso_nonce ( + uuid CHAR(36) NOT NULL PRIMARY KEY, + org_uuid CHAR(36) NOT NULL REFERENCES organizations (uuid), + nonce CHAR(36) NOT NULL +); diff --git a/src/api/identity.rs b/src/api/identity.rs index 1ba3f9e3..3df4defe 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -91,60 +91,78 @@ fn _refresh_login(data: ConnectData, conn: DbConn) -> JsonResult { struct TokenPayload { exp: i64, email: String, + nonce: String, } fn _authorization_login(data: ConnectData, conn: DbConn, ip: &ClientIp) -> JsonResult { let org_identifier = data.org_identifier.as_ref().unwrap(); let code = data.code.as_ref().unwrap(); - let (access_token, refresh_token) = match get_auth_code_access_token(&code, &org_identifier, &conn) { + let organization = Organization::find_by_identifier(org_identifier, &conn).unwrap(); + + let (access_token, refresh_token) = match get_auth_code_access_token(&code, &organization) { Ok((access_token, refresh_token)) => (access_token, refresh_token), Err(err) => err!(err), }; - let token = jsonwebtoken::dangerous_insecure_decode::(access_token.as_str()).unwrap().claims; - let expiry = token.exp; - let user_email = token.email; - let now = Local::now(); - - // COMMON - let user = User::find_by_mail(&user_email, &conn).unwrap(); - - let (mut device, new_device) = get_device(&data, &conn, &user); - - let twofactor_token = twofactor_auth(&user.uuid, &data, &mut device, ip, &conn)?; - - if CONFIG.mail_enabled() && new_device { - if let Err(e) = mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), &now, &device.name) { - error!("Error sending new device email: {:#?}", e); - if CONFIG.require_device_email() { - err!("Could not send login notification email. Please contact your administrator.") + let token = jsonwebtoken::dangerous_insecure_decode::(access_token.as_str()).unwrap().claims; + let nonce = token.nonce; + + match SsoNonce::find_by_org_and_nonce(&organization.uuid, &nonce, &conn) { + Some(sso_nonce) => { + match sso_nonce.delete(&conn) { + Ok(_) => { + let expiry = token.exp; + let user_email = token.email; + let now = Local::now(); + + // COMMON + let user = User::find_by_mail(&user_email, &conn).unwrap(); + + let (mut device, new_device) = get_device(&data, &conn, &user); + + let twofactor_token = twofactor_auth(&user.uuid, &data, &mut device, ip, &conn)?; + + if CONFIG.mail_enabled() && new_device { + if let Err(e) = mail::send_new_device_logged_in(&user.email, &ip.ip.to_string(), &now, &device.name) { + error!("Error sending new device email: {:#?}", e); + + if CONFIG.require_device_email() { + err!("Could not send login notification email. Please contact your administrator.") + } + } + } + + device.refresh_token = refresh_token.clone(); + device.save(&conn)?; + + let mut result = json!({ + "access_token": access_token, + "expires_in": expiry - now.naive_utc().timestamp(), + "token_type": "Bearer", + "refresh_token": refresh_token, + "Key": user.akey, + "PrivateKey": user.private_key, + + "Kdf": user.client_kdf_type, + "KdfIterations": user.client_kdf_iter, + "ResetMasterPassword": false, // TODO: according to official server seems something like: user.password_hash.is_empty(), but would need testing + "scope": "api offline_access", + "unofficialServer": true, + }); + + if let Some(token) = twofactor_token { + result["TwoFactorToken"] = Value::String(token); + } + + Ok(Json(result)) + }, + Err(_) => err!("Failed to delete nonce"), } + }, + None => { + err!("Invalid nonce") } } - - device.refresh_token = refresh_token.clone(); - device.save(&conn)?; - - let mut result = json!({ - "access_token": access_token, - "expires_in": expiry - now.naive_utc().timestamp(), - "token_type": "Bearer", - "refresh_token": refresh_token, - "Key": user.akey, - "PrivateKey": user.private_key, - - "Kdf": user.client_kdf_type, - "KdfIterations": user.client_kdf_iter, - "ResetMasterPassword": false, // TODO: according to official server seems something like: user.password_hash.is_empty(), but would need testing - "scope": "api offline_access", - "unofficialServer": true, - }); - - if let Some(token) = twofactor_token { - result["TwoFactorToken"] = Value::String(token); - } - - Ok(Json(result)) } fn _password_login(data: ConnectData, conn: DbConn, ip: &ClientIp) -> JsonResult { @@ -558,33 +576,24 @@ use openidconnect::{ Scope, OAuth2TokenResponse, }; -fn get_client_from_identifier (identifier: &str, conn: &DbConn) -> Result { - let organization = Organization::find_by_identifier(identifier, conn); - - match organization { - Some(organization) => { - let redirect = organization.callback_path.to_string(); - let client_id = ClientId::new(organization.client_id.unwrap_or_default()); - let client_secret = ClientSecret::new(organization.client_secret.unwrap_or_default()); - let issuer_url = IssuerUrl::new(organization.authority.unwrap_or_default()).expect("invalid issuer URL"); - let provider_metadata = match CoreProviderMetadata::discover(&issuer_url, http_client) { - Ok(metadata) => metadata, - Err(_err) => { - return Err("Failed to discover OpenID provider"); - }, - }; - let client = CoreClient::from_provider_metadata( - provider_metadata, - client_id, - Some(client_secret), - ) - .set_redirect_uri(RedirectUrl::new(redirect).expect("Invalid redirect URL")); - return Ok(client); - }, - None => { - Err("unable to find org") +fn get_client_from_org (organization: &Organization) -> Result { + let redirect = organization.callback_path.to_string(); + let client_id = ClientId::new(organization.client_id.as_ref().unwrap().to_string()); + let client_secret = ClientSecret::new(organization.client_secret.as_ref().unwrap().to_string()); + let issuer_url = IssuerUrl::new(organization.authority.as_ref().unwrap().to_string()).expect("invalid issuer URL"); + let provider_metadata = match CoreProviderMetadata::discover(&issuer_url, http_client) { + Ok(metadata) => metadata, + Err(_err) => { + return Err("Failed to discover OpenID provider"); }, - } + }; + let client = CoreClient::from_provider_metadata( + provider_metadata, + client_id, + Some(client_secret), + ) + .set_redirect_uri(RedirectUrl::new(redirect).expect("Invalid redirect URL")); + return Ok(client); } #[get("/connect/authorize?&")] @@ -593,11 +602,10 @@ fn authorize( state: String, conn: DbConn, ) -> ApiResult { - match get_client_from_identifier(&domain_hint, &conn) { + let organization = Organization::find_by_identifier(&domain_hint, &conn).unwrap(); + match get_client_from_org(&organization) { Ok(client) => { - // TODO store the nonce for validation on authorization token exchange - unclear where to store - // this - let (mut authorize_url, _csrf_state, _nonce) = client + let (mut authorize_url, _csrf_state, nonce) = client .authorize_url( AuthenticationFlow::::AuthorizationCode, CsrfToken::new_random, @@ -607,6 +615,9 @@ fn authorize( .add_scope(Scope::new("profile".to_string())) .url(); + let sso_nonce = SsoNonce::new(organization.uuid, nonce.secret().to_string()); + sso_nonce.save(&conn)?; + // it seems impossible to set the state going in dynamically (requires static lifetime string) // so I change it after the fact let old_pairs = authorize_url.query_pairs().clone(); @@ -628,12 +639,10 @@ fn authorize( fn get_auth_code_access_token ( code: &str, - org_identifier: &str, - conn: &DbConn, + organization: &Organization, ) -> Result<(String, String), &'static str> { let oidc_code = AuthorizationCode::new(String::from(code)); - - match get_client_from_identifier(org_identifier, conn) { + match get_client_from_org(organization) { Ok(client) => { match client.exchange_code(oidc_code).request(http_client) { Ok(token_response) => { diff --git a/src/db/models/mod.rs b/src/db/models/mod.rs index 2179b7f0..601365cb 100644 --- a/src/db/models/mod.rs +++ b/src/db/models/mod.rs @@ -9,6 +9,7 @@ mod organization; mod send; mod two_factor; mod user; +mod sso_nonce; pub use self::attachment::Attachment; pub use self::cipher::Cipher; @@ -21,3 +22,4 @@ pub use self::organization::{Organization, UserOrgStatus, UserOrgType, UserOrgan pub use self::send::{Send, SendType}; pub use self::two_factor::{TwoFactor, TwoFactorType}; pub use self::user::{Invitation, User, UserStampException}; +pub use self::sso_nonce::SsoNonce; diff --git a/src/db/schemas/mysql/schema.rs b/src/db/schemas/mysql/schema.rs index 0ab35d0b..6e0c42ad 100644 --- a/src/db/schemas/mysql/schema.rs +++ b/src/db/schemas/mysql/schema.rs @@ -199,6 +199,14 @@ table! { } } +table! { + sso_nonce (uuid) { + uuid -> Text, + org_uuid -> Text, + nonce -> Text, + } +} + joinable!(attachments -> ciphers (cipher_uuid)); joinable!(ciphers -> organizations (organization_uuid)); joinable!(ciphers -> users (user_uuid)); @@ -217,6 +225,7 @@ joinable!(users_collections -> collections (collection_uuid)); joinable!(users_collections -> users (user_uuid)); joinable!(users_organizations -> organizations (org_uuid)); joinable!(users_organizations -> users (user_uuid)); +joinable!(sso_nonce -> organizations (org_uuid)); allow_tables_to_appear_in_same_query!( attachments, diff --git a/src/db/schemas/postgresql/schema.rs b/src/db/schemas/postgresql/schema.rs index f7fd94f2..71a95ecd 100644 --- a/src/db/schemas/postgresql/schema.rs +++ b/src/db/schemas/postgresql/schema.rs @@ -199,6 +199,14 @@ table! { } } +table! { + sso_nonce (uuid) { + uuid -> Text, + org_uuid -> Text, + nonce -> Text, + } +} + joinable!(attachments -> ciphers (cipher_uuid)); joinable!(ciphers -> organizations (organization_uuid)); joinable!(ciphers -> users (user_uuid)); @@ -217,6 +225,7 @@ joinable!(users_collections -> collections (collection_uuid)); joinable!(users_collections -> users (user_uuid)); joinable!(users_organizations -> organizations (org_uuid)); joinable!(users_organizations -> users (user_uuid)); +joinable!(sso_nonce -> organizations (org_uuid)); allow_tables_to_appear_in_same_query!( attachments, diff --git a/src/db/schemas/sqlite/schema.rs b/src/db/schemas/sqlite/schema.rs index f7fd94f2..71a95ecd 100644 --- a/src/db/schemas/sqlite/schema.rs +++ b/src/db/schemas/sqlite/schema.rs @@ -199,6 +199,14 @@ table! { } } +table! { + sso_nonce (uuid) { + uuid -> Text, + org_uuid -> Text, + nonce -> Text, + } +} + joinable!(attachments -> ciphers (cipher_uuid)); joinable!(ciphers -> organizations (organization_uuid)); joinable!(ciphers -> users (user_uuid)); @@ -217,6 +225,7 @@ joinable!(users_collections -> collections (collection_uuid)); joinable!(users_collections -> users (user_uuid)); joinable!(users_organizations -> organizations (org_uuid)); joinable!(users_organizations -> users (user_uuid)); +joinable!(sso_nonce -> organizations (org_uuid)); allow_tables_to_appear_in_same_query!( attachments,