diff --git a/proto/src/attribute.rs b/proto/src/attribute.rs index 5f2946bf1..5ac2611e8 100644 --- a/proto/src/attribute.rs +++ b/proto/src/attribute.rs @@ -1,4 +1,5 @@ use serde::{Deserialize, Serialize}; +use utoipa::ToSchema; use crate::constants::*; use crate::internal::OperationError; @@ -6,7 +7,9 @@ use std::fmt; pub use smartstring::alias::String as AttrString; -#[derive(Serialize, Deserialize, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, Default)] +#[derive( + Serialize, Deserialize, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, Default, ToSchema, +)] #[cfg_attr(test, derive(enum_iterator::Sequence))] #[serde(rename_all = "lowercase", try_from = "&str", into = "AttrString")] pub enum Attribute { diff --git a/server/core/src/https/apidocs/mod.rs b/server/core/src/https/apidocs/mod.rs index ad70ca2a4..ea77d8d09 100644 --- a/server/core/src/https/apidocs/mod.rs +++ b/server/core/src/https/apidocs/mod.rs @@ -208,6 +208,9 @@ impl Modify for SecurityAddon { ), components( schemas( + // kanidm_proto::attribute::Attribute, + + scim_v1::ScimSyncState, scim_v1::ScimSyncRequest, scim_v1::ScimSyncRetentionMode, @@ -215,8 +218,7 @@ impl Modify for SecurityAddon { scim_v1::ScimValue, scim_v1::ScimMeta, scim_v1::ScimAttr, - // TODO: can't add Entry/ProtoEntry to schema as this was only recently supported utoipa v3.5.0 doesn't support it - ref - // v1::Entry, + internal::ApiToken, internal::ApiTokenPurpose, internal::BackupCodesView, @@ -276,8 +278,7 @@ impl Modify for SecurityAddon { internal::IdentifyUserRequest, // terrible workaround for other things response_schema::CreationChallengeResponse, - // terrible workaround for other things - response_schema::ProtoEntry, + // terrible workaround for other things response_schema::PublicKeyCredential, // terrible workaround for other things @@ -290,6 +291,8 @@ impl Modify for SecurityAddon { response_schema::Result, // terrible workaround for other things response_schema::ScimEntry, + // workaround for the fact that BTreeSet can't be represented in JSON + response_schema::ProtoEntry, // terrible workaround for other things response_schema::Jwk, response_schema::ScimComplexAttr, diff --git a/server/core/src/https/apidocs/response_schema.rs b/server/core/src/https/apidocs/response_schema.rs index 06ae6d449..6da434d08 100644 --- a/server/core/src/https/apidocs/response_schema.rs +++ b/server/core/src/https/apidocs/response_schema.rs @@ -60,10 +60,6 @@ impl IntoResponses for ApiResponseWithout200 { } } -/// Placeholder until we can handle a BTree in utipa -#[derive(Debug, Clone, ToSchema)] -pub(crate) struct ProtoEntry {} - #[derive(Debug, Clone, ToSchema)] // TODO: this should be `webauthn_rs_proto::auth::PublicKeyCredential``, but ... I don't know how to make it possible in utoipa pub(crate) struct PublicKeyCredential {} @@ -92,6 +88,13 @@ pub(crate) struct Result {} // TODO: this should be handled elsewhere, but ... I don't know how to make it possible in utoipa pub(crate) struct ScimEntry {} +#[derive(Debug, Clone, ToSchema)] +/// workaround for the fact that BTreeSet can't be represented in JSON +pub(crate) struct ProtoEntry { + #[allow(dead_code, clippy::disallowed_types)] // because it's a schema definition + attrs: BTreeMap>, +} + #[derive(Debug, Clone, ToSchema)] pub(crate) struct Jwk {} diff --git a/server/core/src/https/generic.rs b/server/core/src/https/generic.rs index d33fbdbc4..f6f3f31d3 100644 --- a/server/core/src/https/generic.rs +++ b/server/core/src/https/generic.rs @@ -1,8 +1,7 @@ use axum::extract::State; use axum::http::header::CONTENT_TYPE; use axum::response::IntoResponse; -use axum::routing::get; -use axum::{Extension, Router}; +use axum::Extension; use kanidmd_lib::status::StatusRequestEvent; use super::middleware::KOpId; @@ -51,9 +50,3 @@ pub async fn robots_txt() -> impl IntoResponse { ), ) } - -pub(crate) fn route_setup() -> Router { - Router::new() - .route("/robots.txt", get(robots_txt)) - .route("/status", get(status)) -} diff --git a/server/core/src/https/mod.rs b/server/core/src/https/mod.rs index 4c4f0fc68..4e8e18293 100644 --- a/server/core/src/https/mod.rs +++ b/server/core/src/https/mod.rs @@ -301,10 +301,10 @@ pub async fn create_https_server( ServerRole::WriteReplicaNoUI => Router::new(), }; let app = Router::new() - .merge(generic::route_setup()) .merge(oauth2::route_setup(state.clone())) .merge(v1_scim::route_setup()) - .merge(v1::route_setup(state.clone())); + .merge(v1::route_setup(state.clone())) + .route("/robots.txt", get(generic::robots_txt)); let app = match config.role { ServerRole::WriteReplicaNoUI => app, @@ -362,6 +362,7 @@ pub async fn create_https_server( let app = app.layer(from_fn(middleware::are_we_json_yet)); let app = app + .route("/status", get(generic::status)) // This must be the LAST middleware. // This is because the last middleware here is the first to be entered and the last // to be exited, and this middleware sets up ids' and other bits for for logging diff --git a/server/core/src/https/oauth2.rs b/server/core/src/https/oauth2.rs index 9746ac7c7..699760498 100644 --- a/server/core/src/https/oauth2.rs +++ b/server/core/src/https/oauth2.rs @@ -114,14 +114,10 @@ pub(crate) async fn oauth2_image_get( ) .into_response(), Ok(None) => { - warn!(?rs_name, "No image set for oauth2 client"); + warn!(?rs_name, "No image set for OAuth2 client"); (StatusCode::NOT_FOUND, "").into_response() } - Err(err) => { - error!(?err, "Unable to get image for oauth2 client"); - // TODO: a 404 probably isn't perfect but it's not the worst - (StatusCode::INTERNAL_SERVER_ERROR, "").into_response() - } + Err(err) => WebError::from(err).into_response(), } } @@ -292,12 +288,12 @@ async fn oauth2_authorise( } Err(Oauth2Error::AccessDenied) => { // If scopes are not available for this account. - #[allow(clippy::unwrap_used)] + #[allow(clippy::expect_used)] Response::builder() .status(StatusCode::FORBIDDEN) .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") .body(Body::empty()) - .unwrap() + .expect("Failed to generate a forbidden response") } /* RFC - If the request fails due to a missing, invalid, or mismatching @@ -315,12 +311,12 @@ async fn oauth2_authorise( kopid.eventid, &e.to_string() ); - #[allow(clippy::unwrap_used)] + #[allow(clippy::expect_used)] Response::builder() .status(StatusCode::BAD_REQUEST) .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") .body(Body::empty()) - .unwrap() + .expect("Failed to generate a bad request response") } } } @@ -380,7 +376,7 @@ async fn oauth2_authorise_permit( .clear() .append_pair("state", &state) .append_pair("code", &code); - #[allow(clippy::unwrap_used)] + #[allow(clippy::expect_used)] Response::builder() .status(StatusCode::FOUND) .header(LOCATION, redirect_uri.as_str()) @@ -389,23 +385,30 @@ async fn oauth2_authorise_permit( redirect_uri.origin().ascii_serialization(), ) .body(Body::empty()) - .unwrap() + .expect("Failed to generate response") } - Err(_e) => { - // If an error happens in our consent flow, I think - // that we should NOT redirect to the calling application - // and we need to handle that locally somehow. - // This needs to be better! - // - // Turns out this instinct was correct: - // https://www.proofpoint.com/us/blog/cloud-security/microsoft-and-github-oauth-implementation-vulnerabilities-lead-redirection - // Possible to use this with a malicious client configuration to phish / spam. - #[allow(clippy::unwrap_used)] - Response::builder() - .status(StatusCode::INTERNAL_SERVER_ERROR) - .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") - .body(Body::empty()) - .unwrap() + Err(err) => { + match err { + OperationError::NotAuthenticated => { + WebError::from(err).response_with_access_control_origin_header() + } + _ => { + // If an error happens in our consent flow, I think + // that we should NOT redirect to the calling application + // and we need to handle that locally somehow. + // This needs to be better! + // + // Turns out this instinct was correct: + // https://www.proofpoint.com/us/blog/cloud-security/microsoft-and-github-oauth-implementation-vulnerabilities-lead-redirection + // Possible to use this with a malicious client configuration to phish / spam. + #[allow(clippy::expect_used)] + Response::builder() + .status(StatusCode::INTERNAL_SERVER_ERROR) + .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") + .body(Body::empty()) + .expect("Failed to generate error response") + } + } } } } @@ -464,17 +467,24 @@ async fn oauth2_authorise_reject( .unwrap() // I think the client server needs this } - Err(_e) => { - // If an error happens in our reject flow, I think - // that we should NOT redirect to the calling application - // and we need to handle that locally somehow. - // This needs to be better! - #[allow(clippy::unwrap_used)] - Response::builder() - .status(StatusCode::INTERNAL_SERVER_ERROR) - .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") - .body(Body::empty()) - .unwrap() + Err(err) => { + match err { + OperationError::NotAuthenticated => { + WebError::from(err).response_with_access_control_origin_header() + } + _ => { + // If an error happens in our reject flow, I think + // that we should NOT redirect to the calling application + // and we need to handle that locally somehow. + // This needs to be better! + #[allow(clippy::expect_used)] + Response::builder() + .status(StatusCode::INTERNAL_SERVER_ERROR) + .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") + .body(Body::empty()) + .expect("Failed to generate an error response") + } + } } } } @@ -629,12 +639,12 @@ pub async fn oauth2_token_introspect_post( } Err(Oauth2Error::AuthenticationRequired) => { // This will trigger our ui to auth and retry. - #[allow(clippy::unwrap_used)] + #[allow(clippy::expect_used)] Response::builder() .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") .status(StatusCode::UNAUTHORIZED) .body(Body::empty()) - .unwrap() + .expect("Failed to generate an unauthorized response") } Err(e) => { // https://datatracker.ietf.org/doc/html/rfc6749#section-5.2 @@ -649,12 +659,12 @@ pub async fn oauth2_token_introspect_post( format!("{:?}", e) } }; - #[allow(clippy::unwrap_used)] + #[allow(clippy::expect_used)] Response::builder() .status(StatusCode::BAD_REQUEST) .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") .body(Body::from(body)) - .unwrap() + .expect("Failed to generate an error response") } } }