Skip to content

Commit

Permalink
Merge pull request #183 from sebadob/fix-password-reset-with-old-passkey
Browse files Browse the repository at this point in the history
Fix password reset with old passkey
  • Loading branch information
sebadob committed Nov 24, 2023
2 parents 4f451de + e90c0fc commit 24af03c
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 31 deletions.
12 changes: 6 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ members = [
exclude = ["rauthy-client"]

[workspace.package]
version = "0.19.1-20231123"
version = "0.19.2-20231123"
edition = "2021"
authors = ["Sebastian Dobe <sebastiandobe@mailbox.org>"]
license = "Apache-2.0"
Expand Down
1 change: 0 additions & 1 deletion rauthy-handlers/src/middleware/principal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ where
principal.session = Some(s);
}

// req.extensions_mut().insert(session);
req.extensions_mut().insert(principal);

service.call(req).await
Expand Down
34 changes: 15 additions & 19 deletions rauthy-handlers/src/users.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,18 +557,25 @@ pub async fn get_user_webauthn_passkeys(
pub async fn post_webauthn_auth_start(
data: web::Data<AppState>,
id: web::Path<String>,
// The principal here must be optional to make cases like user password reset in a
// fully new / different browser which does not have any lefter data or cookies
principal: ReqPrincipal,
req: HttpRequest,
req_data: Json<WebauthnAuthStartRequest>,
) -> Result<HttpResponse, ErrorResponse> {
principal.validate_session_auth_or_init()?;

let purpose = req_data.into_inner().purpose;
let id = match purpose {
// only for a Login purpose, this can be accessed without authentication (yet)
MfaPurpose::Login(_) => id.into_inner(),
MfaPurpose::Login(_) => {
// During Login, the session is allowed to be in init only state
principal.validate_session_auth_or_init()?;
id.into_inner()
}

MfaPurpose::PasswordReset => {
// A password reset webauthn req can be opened without any session at all.
// This is mandatory to make password reset flows fully work, even with an old
// account with linked Passkeys.
match req.cookie(PWD_RESET_COOKIE) {
None => {
return Err(ErrorResponse::new(
Expand All @@ -591,6 +598,7 @@ pub async fn post_webauthn_auth_start(
}

_ => {
// for all other purposes, we need an authenticated session
principal.validate_session_auth()?;

// make sure the principal is this very user
Expand Down Expand Up @@ -624,27 +632,15 @@ pub async fn post_webauthn_auth_start(
pub async fn post_webauthn_auth_finish(
data: web::Data<AppState>,
id: web::Path<String>,
principal: ReqPrincipal,
req_data: Json<WebauthnAuthFinishRequest>,
) -> Result<HttpResponse, ErrorResponse> {
let id = id.into_inner();
let res = if principal.validate_session_init().is_ok() {
// The Session is only in init state in a very tiny window, when the /oidc/authorize page has
// been received and until the credentials have been validated.
// As a double check, we have the 'code' from the /start endpoint.
webauthn::auth_finish(&data, id, req_data.into_inner()).await?
} else if principal.validate_session_auth().is_ok() {
// For any authenticated request, validate that Principal matches the user.
principal.is_user(&id)?;

webauthn::auth_finish(&data, id, req_data.into_inner()).await?
} else {
return Err(ErrorResponse::new(
ErrorResponseType::Unauthorized,
"No valid session".to_string(),
));
};
// We do not need to further validate the principal here.
// All of this is done at the /start endpoint.
// This here will simply fail, if the secret code from the /start does not exist.

let res = webauthn::auth_finish(&data, id, req_data.into_inner()).await?;
Ok(res.into_response())
}

Expand Down
1 change: 0 additions & 1 deletion rauthy-models/src/entity/users.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,6 @@ impl User {
id: String,
upd_user: UpdateUserRequest,
user: Option<User>,
// OK((User, is_new_rauthy_admin))
) -> Result<(User, bool), ErrorResponse> {
let mut user = match user {
None => User::find(data, id).await?,
Expand Down
6 changes: 3 additions & 3 deletions rauthy-models/src/entity/webauthn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -728,12 +728,12 @@ pub async fn auth_finish(
user_id: String,
req: WebauthnAuthFinishRequest,
) -> Result<WebauthnAdditionalData, ErrorResponse> {
let mut user = User::find(data, user_id).await?;
let force_uv = user.account_type() == AccountType::Passkey || *WEBAUTHN_FORCE_UV;

let auth_data = WebauthnData::find(data, req.code).await?;
let auth_state = serde_json::from_str(&auth_data.auth_state_json).unwrap();

let mut user = User::find(data, user_id).await?;
let force_uv = user.account_type() == AccountType::Passkey || *WEBAUTHN_FORCE_UV;

let pks = PasskeyEntity::find_for_user(data, &user.id).await?;

match data
Expand Down
4 changes: 4 additions & 0 deletions rauthy-service/src/password_reset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rauthy_models::app_state::AppState;
use rauthy_models::entity::colors::ColorEntity;
use rauthy_models::entity::magic_links::{MagicLink, MagicLinkUsage};
use rauthy_models::entity::password::PasswordPolicy;
use rauthy_models::entity::sessions::Session;
use rauthy_models::entity::users::User;
use rauthy_models::entity::webauthn;
use rauthy_models::entity::webauthn::WebauthnServiceReq;
Expand Down Expand Up @@ -218,6 +219,9 @@ pub async fn handle_put_user_password_reset<'a>(
.await
.unwrap();

// delete all existing user sessions to have a clean flow
Session::invalidate_for_user(data, &user.id).await?;

// delete the cookie
let cookie = cookie::Cookie::build(PWD_RESET_COOKIE, "")
.secure(true)
Expand Down

0 comments on commit 24af03c

Please sign in to comment.