mart-w/kanidm
1
0
Fork 0
mirror of https://github.com/kanidm/kanidm.git synced 2025-02-23 12:37:00 +01:00
Code Issues Actions 16 Packages Projects Releases Wiki Activity

Resolve UI Auth Loop with OAuth2 (#3226)

Browse source 
If an OAuth2 auth request resume cookie was present, and at the same
time the kani instance was restarted, the cookie would now fail
to validate on the instance. This caused the user to experience an auth
loop where after every authentication they would see an error *despite*
logging in correctly, and then a refresh would show the correct
apps page.

This removes the auth_req cookie correctly even if it fails to
deserialise.
This commit is contained in:
Firstyear 2024-11-21 19:29:35 +10:00 committed by GitHub
parent ce0ad8f854
commit 809cacdb85
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 39 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

56
server/core/src/https/views/oauth2.rs
View file

@ -54,7 +54,15 @@ pub async fn view_index_get(
jar: CookieJar, jar: CookieJar,
Query(auth_req): Query<AuthorisationRequest>, Query(auth_req): Query<AuthorisationRequest>,
) -> Response { ) -> Response {
oauth2_auth_req(state, kopid, client_auth_info, domain_info, jar, auth_req).await oauth2_auth_req(
state,
kopid,
client_auth_info,
domain_info,
jar,
Some(auth_req),
)
.await
} }
pub async fn view_resume_get( pub async fn view_resume_get(
@ -63,19 +71,19 @@ pub async fn view_resume_get(
VerifiedClientInformation(client_auth_info): VerifiedClientInformation, VerifiedClientInformation(client_auth_info): VerifiedClientInformation,
DomainInfo(domain_info): DomainInfo, DomainInfo(domain_info): DomainInfo,
jar: CookieJar, jar: CookieJar,
) -> Result<Response, UnrecoverableErrorView> { ) -> Response {
let maybe_auth_req = let maybe_auth_req =
cookies::get_signed::<AuthorisationRequest>(&state, &jar, COOKIE_OAUTH2_REQ); cookies::get_signed::<AuthorisationRequest>(&state, &jar, COOKIE_OAUTH2_REQ);
if let Some(auth_req) = maybe_auth_req { oauth2_auth_req(
Ok(oauth2_auth_req(state, kopid, client_auth_info, domain_info, jar, auth_req).await) state,
} else { kopid,
error!("unable to resume session, no auth_req was found in the cookie"); client_auth_info,
Err(UnrecoverableErrorView { domain_info,
err_code: OperationError::InvalidState, jar,
operation_id: kopid.eventid, maybe_auth_req,
}) )
} .await
} }
async fn oauth2_auth_req( async fn oauth2_auth_req(
@ -84,13 +92,8 @@ async fn oauth2_auth_req(
client_auth_info: ClientAuthInfo, client_auth_info: ClientAuthInfo,
domain_info: DomainInfoRead, domain_info: DomainInfoRead,
jar: CookieJar, jar: CookieJar,
auth_req: AuthorisationRequest, maybe_auth_req: Option<AuthorisationRequest>,
) -> Response { ) -> Response {
let res: Result<AuthoriseResponse, Oauth2Error> = state
.qe_r_ref
.handle_oauth2_authorise(client_auth_info, auth_req.clone(), kopid.eventid)
.await;
// No matter what, we always clear the stored oauth2 cookie to prevent // No matter what, we always clear the stored oauth2 cookie to prevent
// ui loops // ui loops
let jar = if let Some(authreq_cookie) = jar.get(COOKIE_OAUTH2_REQ) { let jar = if let Some(authreq_cookie) = jar.get(COOKIE_OAUTH2_REQ) {
@ -102,6 +105,25 @@ async fn oauth2_auth_req(
jar jar
}; };
// If the auth_req was cross-signed, old, or just bad, error. But we have *cleared* it
// from the cookie which means we won't see it again.
let Some(auth_req) = maybe_auth_req else {
error!("unable to resume session, no valid auth_req was found in the cookie. This cookie has been removed.");
return (
jar,
UnrecoverableErrorView {
err_code: OperationError::InvalidState,
operation_id: kopid.eventid,
},
)
.into_response();
};
let res: Result<AuthoriseResponse, Oauth2Error> = state
.qe_r_ref
.handle_oauth2_authorise(client_auth_info, auth_req.clone(), kopid.eventid)
.await;
match res { match res {
Ok(AuthoriseResponse::Permitted(AuthorisePermitSuccess { Ok(AuthoriseResponse::Permitted(AuthorisePermitSuccess {
mut redirect_uri, mut redirect_uri,