More openapi tweaks (#3038)

This commit is contained in:
James Hodgkinson 2024-09-17 13:01:54 +10:00 committed by GitHub
parent a2cdb810a2
commit 4cbec48307
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 74 additions and 61 deletions

View file

@ -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 {

View file

@ -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 <https://github.com/juhaku/utoipa/pull/756/files>
// 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,

View file

@ -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<String, Vec<String>>,
}
#[derive(Debug, Clone, ToSchema)]
pub(crate) struct Jwk {}

View file

@ -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<ServerState> {
Router::new()
.route("/robots.txt", get(robots_txt))
.route("/status", get(status))
}

View file

@ -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

View file

@ -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,9 +385,14 @@ async fn oauth2_authorise_permit(
redirect_uri.origin().ascii_serialization(),
)
.body(Body::empty())
.unwrap()
.expect("Failed to generate response")
}
Err(_e) => {
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.
@ -400,12 +401,14 @@ async fn oauth2_authorise_permit(
// 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)]
#[allow(clippy::expect_used)]
Response::builder()
.status(StatusCode::INTERNAL_SERVER_ERROR)
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
.body(Body::empty())
.unwrap()
.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) => {
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::unwrap_used)]
#[allow(clippy::expect_used)]
Response::builder()
.status(StatusCode::INTERNAL_SERVER_ERROR)
.header(ACCESS_CONTROL_ALLOW_ORIGIN, "*")
.body(Body::empty())
.unwrap()
.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")
}
}
}