From 44320e92631bca332c66003e9413b741a6c649bf Mon Sep 17 00:00:00 2001 From: Marek Fajkus Date: Sat, 13 Jan 2024 15:45:31 +0100 Subject: [PATCH] Finish integration with keycloak for accepting new members (#161) Finishes implementation of keycloak integration. * improves error handling * small fixes/improvements in api (tracking date) * add error handling to front-end --- melon-head/src/App/MemberDetail.res | 36 +++++++++++++++++------ orca/src/api/members/query.rs | 1 + orca/src/api/mod.rs | 3 ++ orca/src/api/session/mod.rs | 2 +- orca/src/server/oid.rs | 5 ++-- orca/src/server/oid/keycloak.rs | 45 ++++++++++++++++------------- 6 files changed, 60 insertions(+), 32 deletions(-) diff --git a/melon-head/src/App/MemberDetail.res b/melon-head/src/App/MemberDetail.res index 5b5ad47..70403e3 100644 --- a/melon-head/src/App/MemberDetail.res +++ b/melon-head/src/App/MemberDetail.res @@ -96,18 +96,34 @@ module Actions = { module Accept = { @react.component - let make = (~modal, ~api, ~id) => { + let make = (~modal, ~api, ~id, ~setDetail) => { + let (error, setError) = React.useState(() => None) + let doAccept = (_: JsxEvent.Mouse.t) => { let req = api->Api.patchJson( ~path="/members/" ++ Uuid.toString(id) ++ "/accept", - ~decoder=Json.Decode.string, + ~decoder=MemberData.Decode.detail, ~body=Js.Json.null, ) + + req->Future.get(res => { + switch res { + | Ok(data) => { + setDetail(_ => RemoteData.Success(data)) + Modal.Interface.closeModal(modal) + } + | Error(e) => setError(_ => Some(e)) + } + }) } -

{React.string("Approve user and give allow them to access internal resources")}

+

{React.string("Accept member and allow them to access internal resources.")}

+ {switch error { + | None => React.null + | Some(err) => {React.string(err->Api.showError)} + }} @@ -153,7 +171,7 @@ let viewOccupation = (occupation: MemberData.occupation) => { @react.component let make = (~api, ~id, ~modal) => { - let (detail: Api.webData, _setDetail, _) = + let (detail: Api.webData, setDetail, _) = api->Hook.getData(~path="/members/" ++ Uuid.toString(id), ~decoder=MemberData.Decode.detail) let status = RemoteData.map(detail, MemberData.getStatus) @@ -264,7 +282,7 @@ let make = (~api, ~id, ~modal) => { /> {switch status { - | Success(s) => + | Success(s) => | _ => React.null }} diff --git a/orca/src/api/members/query.rs b/orca/src/api/members/query.rs index 8ece63f..c44b6d3 100644 --- a/orca/src/api/members/query.rs +++ b/orca/src/api/members/query.rs @@ -250,6 +250,7 @@ pub fn assing_member_oid_sub<'a>(id: Id, uuid: Uuid) -> QueryAs<'a, Deta " UPDATE members SET sub = $2 +, onboarding_finished_at = NOW() WHERE id = $1 RETURNING id , member_number diff --git a/orca/src/api/mod.rs b/orca/src/api/mod.rs index 39c9397..055c9ab 100644 --- a/orca/src/api/mod.rs +++ b/orca/src/api/mod.rs @@ -85,6 +85,9 @@ impl<'r> Responder<'r, 'static> for oid::Error { BadToken(_) => Err(Status::Unauthorized), Http(_) => Err(Status::BadGateway), Parsing(_) => Err(Status::InternalServerError), + Proxy(status_code) => Err(rocket::http::Status { + code: status_code.as_u16(), + }), } } } diff --git a/orca/src/api/session/mod.rs b/orca/src/api/session/mod.rs index 4f074f5..3ad0ad3 100644 --- a/orca/src/api/session/mod.rs +++ b/orca/src/api/session/mod.rs @@ -39,7 +39,7 @@ async fn pair_by_email<'r>( oid_provider: &State, token: JwtToken<'r>, ) -> Response> { - // TODO: shouw we require some role for this action? + // TODO: should we require some role for this action? // in a way we're already trusting token so maybe we can also just // let any member assing themselves. let token_data = oid_provider.inner().decode_jwt(&token)?; diff --git a/orca/src/server/oid.rs b/orca/src/server/oid.rs index 3dbe6cf..20f9c44 100644 --- a/orca/src/server/oid.rs +++ b/orca/src/server/oid.rs @@ -16,9 +16,9 @@ use self::keycloak::KeycloakProvider; #[derive(Debug, sqlx::FromRow)] pub struct User { - first_name: String, - last_name: String, email: String, + first_name: Option, + last_name: Option, } #[derive(Debug, Clone, FromForm)] @@ -182,6 +182,7 @@ pub enum Error { Disabled, Http(reqwest::Error), Parsing(String), + Proxy(reqwest::StatusCode), } impl From for Error { diff --git a/orca/src/server/oid/keycloak.rs b/orca/src/server/oid/keycloak.rs index 62cbd19..df7e18a 100644 --- a/orca/src/server/oid/keycloak.rs +++ b/orca/src/server/oid/keycloak.rs @@ -132,29 +132,34 @@ impl OidProvider for KeycloakProvider { .send() .await?; - debug!("Keycloak response status: {}", response.status()); + let status = response.status(); + debug!("Keycloak response status: {}", status); debug!("Keycloak response: {:?}", response); - // Keycloak responds with empty body but it includes `Location` with full - // api path to new user resource. - // Last segment of this path is uuid identifying new user so we can parse it out of this header. - match response.headers().get("Location") { - Some(header) => { - let string = header - .to_str() - .map_err(|_| Error::Parsing(format!("Bad header {:?}", header)))?; - let url = Url::parse(string) - .map_err(|_| Error::Parsing(format!("Expected URL got {}", string)))?; - let uuid = url - .path_segments() - .ok_or(Error::Parsing(format!("Bad url {url}")))? - .last() - .ok_or(Error::Parsing(format!("Bad url {url}")))?; - - Uuid::parse_str(uuid) - .map_err(|_| Error::Parsing(format!("Canot parse UUID from {}", uuid))) + if status.is_success() { + // Keycloak responds with empty body but it includes `Location` with full + // api path to new user resource. + // Last segment of this path is uuid identifying new user so we can parse it out of this header. + match response.headers().get("Location") { + Some(header) => { + let string = header + .to_str() + .map_err(|_| Error::Parsing(format!("Bad header {:?}", header)))?; + let url = Url::parse(string) + .map_err(|_| Error::Parsing(format!("Expected URL got {}", string)))?; + let uuid = url + .path_segments() + .ok_or(Error::Parsing(format!("Bad url {url}")))? + .last() + .ok_or(Error::Parsing(format!("Bad url {url}")))?; + + Uuid::parse_str(uuid) + .map_err(|_| Error::Parsing(format!("Canot parse UUID from {}", uuid))) + } + None => Err(Error::Parsing("Missing Location header".to_string())), } - None => Err(Error::Parsing("Missing Location header".to_string())), + } else { + Err(Error::Proxy(status)) } } }