Fix the password reset form and possible resolver issue (#3398)

While testing for everything open I noticed two possible
issues. This PR fixes both.

The first is a possible recursion in the resolver. I think
I need to fix up it's transactions a bit in another PR.

The second was that the submit button on the reset form
doesn't work. This fixes that as well as post reset redirecting
to the correct location.
This commit is contained in:
Firstyear 2025-02-05 14:18:09 +10:00 committed by GitHub
parent 4938c6796b
commit 41b2eac1f4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 45 additions and 16 deletions

View file

@ -24,6 +24,7 @@ use kanidm_proto::internal::{
CredentialDetail, OperationError, PasskeyDetail, PasswordFeedback, TotpAlgo, UserAuthToken, CredentialDetail, OperationError, PasskeyDetail, PasswordFeedback, TotpAlgo, UserAuthToken,
COOKIE_CU_SESSION_TOKEN, COOKIE_CU_SESSION_TOKEN,
}; };
use kanidmd_lib::prelude::ClientAuthInfo;
use super::constants::Urls; use super::constants::Urls;
use super::navbar::NavbarCtx; use super::navbar::NavbarCtx;
@ -204,11 +205,41 @@ impl Display for PasskeyClass {
} }
} }
/// When the credential update session is ended through a commit or discard of the changes
/// we need to redirect the user to a relevant location. This location depends on the sessions
/// current authentication state. If they are authenticated, they are sent to their profile. If
/// they are not authenticated, they are sent to the login screen.
async fn end_session_response(
state: ServerState,
kopid: KOpId,
client_auth_info: ClientAuthInfo,
jar: CookieJar,
) -> axum::response::Result<Response> {
let is_logged_in = state
.qe_r_ref
.handle_auth_valid(client_auth_info, kopid.eventid)
.await
.is_ok();
let redirect_location = if is_logged_in {
Urls::Profile.as_ref()
} else {
Urls::Login.as_ref()
};
Ok((
jar,
HxLocation::from(Uri::from_static(redirect_location)),
"",
)
.into_response())
}
pub(crate) async fn commit( pub(crate) async fn commit(
State(state): State<ServerState>, State(state): State<ServerState>,
Extension(kopid): Extension<KOpId>, Extension(kopid): Extension<KOpId>,
HxRequest(_hx_request): HxRequest, HxRequest(_hx_request): HxRequest,
VerifiedClientInformation(_client_auth_info): VerifiedClientInformation, VerifiedClientInformation(client_auth_info): VerifiedClientInformation,
DomainInfo(domain_info): DomainInfo, DomainInfo(domain_info): DomainInfo,
jar: CookieJar, jar: CookieJar,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
@ -223,14 +254,14 @@ pub(crate) async fn commit(
// No longer need the cookie jar. // No longer need the cookie jar.
let jar = cookies::destroy(jar, COOKIE_CU_SESSION_TOKEN, &state); let jar = cookies::destroy(jar, COOKIE_CU_SESSION_TOKEN, &state);
Ok((jar, HxLocation::from(Uri::from_static("/ui")), "").into_response()) end_session_response(state, kopid, client_auth_info, jar).await
} }
pub(crate) async fn cancel_cred_update( pub(crate) async fn cancel_cred_update(
State(state): State<ServerState>, State(state): State<ServerState>,
Extension(kopid): Extension<KOpId>, Extension(kopid): Extension<KOpId>,
HxRequest(_hx_request): HxRequest, HxRequest(_hx_request): HxRequest,
VerifiedClientInformation(_client_auth_info): VerifiedClientInformation, VerifiedClientInformation(client_auth_info): VerifiedClientInformation,
DomainInfo(domain_info): DomainInfo, DomainInfo(domain_info): DomainInfo,
jar: CookieJar, jar: CookieJar,
) -> axum::response::Result<Response> { ) -> axum::response::Result<Response> {
@ -245,12 +276,7 @@ pub(crate) async fn cancel_cred_update(
// No longer need the cookie jar. // No longer need the cookie jar.
let jar = cookies::destroy(jar, COOKIE_CU_SESSION_TOKEN, &state); let jar = cookies::destroy(jar, COOKIE_CU_SESSION_TOKEN, &state);
Ok(( end_session_response(state, kopid, client_auth_info, jar).await
jar,
HxLocation::from(Uri::from_static(Urls::Profile.as_ref())),
"",
)
.into_response())
} }
pub(crate) async fn cancel_mfareg( pub(crate) async fn cancel_mfareg(
@ -782,7 +808,7 @@ pub(crate) async fn view_reset_get(
State(state): State<ServerState>, State(state): State<ServerState>,
Extension(kopid): Extension<KOpId>, Extension(kopid): Extension<KOpId>,
HxRequest(_hx_request): HxRequest, HxRequest(_hx_request): HxRequest,
VerifiedClientInformation(_client_auth_info): VerifiedClientInformation, VerifiedClientInformation(client_auth_info): VerifiedClientInformation,
DomainInfo(domain_info): DomainInfo, DomainInfo(domain_info): DomainInfo,
Query(params): Query<ResetTokenParam>, Query(params): Query<ResetTokenParam>,
mut jar: CookieJar, mut jar: CookieJar,
@ -791,7 +817,7 @@ pub(crate) async fn view_reset_get(
let cookie = jar.get(COOKIE_CU_SESSION_TOKEN); let cookie = jar.get(COOKIE_CU_SESSION_TOKEN);
let is_logged_in = state let is_logged_in = state
.qe_r_ref .qe_r_ref
.handle_auth_valid(_client_auth_info.clone(), kopid.eventid) .handle_auth_valid(client_auth_info.clone(), kopid.eventid)
.await .await
.is_ok(); .is_ok();

View file

@ -56,10 +56,9 @@
Return to the home page Return to the home page
</button> </button>
<button class="btn btn-primary" <button class="btn btn-primary"
hx-get=(Urls::CredReset) hx-get=""
hx-include="#token" hx-include="form"
hx-target="#cred-reset-form" hx-select="#cred-reset-form" hx-target="body"
hx-swap="outerHTML"
type="submit"> type="submit">
Submit Submit
</button> </button>

View file

@ -42,7 +42,7 @@
</div> </div>
</div> </div>
<div> <div>
<button type="button" class="btn btn-outline-danger" hx-post="/ui/api/delete_alt_creds" hx-confirm="Delete your Password and any associated MFA?\nNote: this will not remove Passkeys."> <button type="button" class="btn btn-outline-danger" hx-post="/ui/api/delete_alt_creds" hx-confirm="Delete your Password and any associated alternative authentication methods? NOTE: this will not remove Passkeys.">
Delete Alternative Credentials Delete Alternative Credentials
</button> </button>
</div> </div>

View file

@ -228,6 +228,8 @@ impl Resolver {
debug!("get_cached_usertoken {:?}", err); debug!("get_cached_usertoken {:?}", err);
})?; })?;
drop(dbtxn);
match r { match r {
Some((ut, ex)) => { Some((ut, ex)) => {
// Are we expired? // Are we expired?
@ -276,6 +278,8 @@ impl Resolver {
let mut dbtxn = self.db.write().await; let mut dbtxn = self.db.write().await;
let r = dbtxn.get_group(grp_id).map_err(|_| ())?; let r = dbtxn.get_group(grp_id).map_err(|_| ())?;
drop(dbtxn);
match r { match r {
Some((ut, ex)) => { Some((ut, ex)) => {
// Are we expired? // Are we expired?