From 1d139a26f1832794078cfeca60f2d5fc9503ce35 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 21 Aug 2025 17:09:41 +0100 Subject: [PATCH] Use graphql Guard to remove auth checks from resolvers Using the attributes allows the auth logic to be separated from the logic in the resolver endpoints and makes it easier to re-use the checks for other future endpoints if they're added. --- src/graphql/auth.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++- src/graphql/mod.rs | 40 +++++++-------------------------------- 2 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/graphql/auth.rs b/src/graphql/auth.rs index 074eb32..dc0cbe7 100644 --- a/src/graphql/auth.rs +++ b/src/graphql/auth.rs @@ -14,11 +14,12 @@ use std::str::FromStr; +use async_graphql::{Context, Guard}; use axum_extra::headers::authorization::Bearer; use axum_extra::headers::Authorization; use derive_more::{Display, Error, From}; use serde::{Deserialize, Serialize}; -use tracing::info; +use tracing::{info, trace}; use crate::cli::PolicyOptions; @@ -176,6 +177,49 @@ impl PolicyCheck { } } +#[derive(Debug)] +pub(crate) enum AuthGuard<'a> { + Access { + instrument: &'a str, + instrument_session: &'a str, + }, + InstrumentAdmin { + instrument: &'a str, + }, + Admin, +} + +impl<'a> Guard for AuthGuard<'a> { + async fn check(&self, ctx: &Context<'_>) -> async_graphql::Result<()> { + if let Some(policy) = ctx.data::>()? { + trace!("Auth enabled: checking token"); + let token = ctx.data::>>()?; + let check = match self { + AuthGuard::Access { + instrument, + instrument_session, + } => { + policy + .check_access(token.as_ref(), instrument, instrument_session) + .await + } + AuthGuard::InstrumentAdmin { instrument } => { + policy + .check_instrument_admin(token.as_ref(), instrument) + .await + } + AuthGuard::Admin => policy.check_admin(token.as_ref()).await, + }; + check + .inspect_err(|e| info!("Authorization failed: {e:?}")) + .map_err(async_graphql::Error::from) + } else { + trace!("No authorization configured"); + Ok(()) + } + } +} + #[derive(Debug, Display, Error, From)] pub enum AuthError { #[display("Invalid authorization configuration")] diff --git a/src/graphql/mod.rs b/src/graphql/mod.rs index 988e938..9598357 100644 --- a/src/graphql/mod.rs +++ b/src/graphql/mod.rs @@ -14,7 +14,6 @@ use std::any; use std::borrow::Cow; -use std::future::Future; use std::io::Write; use std::path::{Component, PathBuf}; @@ -25,7 +24,7 @@ use async_graphql::{ Object, Scalar, ScalarType, Schema, SimpleObject, TypeName, Value, }; use async_graphql_axum::{GraphQLRequest, GraphQLResponse}; -use auth::{AuthError, PolicyCheck}; +use auth::{AuthGuard, PolicyCheck}; use axum::http::StatusCode; use axum::response::{Html, IntoResponse}; use axum::routing::{get, post}; @@ -342,16 +341,13 @@ impl Query { } /// Get the current configuration for the given instrument + #[graphql(guard = "AuthGuard::InstrumentAdmin{instrument: &instrument}")] #[instrument(skip(self, ctx))] async fn configuration( &self, ctx: &Context<'_>, instrument: String, ) -> async_graphql::Result { - check_auth(ctx, |policy, token| { - policy.check_instrument_admin(token, &instrument) - }) - .await?; let db = ctx.data::()?; let nt = ctx.data::()?; trace!("Getting config for {instrument:?}"); @@ -361,13 +357,13 @@ impl Query { /// Get the configurations for all available instruments /// Can be filtered to provide one or more specific instruments + #[graphql(guard = "AuthGuard::Admin")] #[instrument(skip(self, ctx))] async fn configurations( &self, ctx: &Context<'_>, instrument_filters: Option>, ) -> async_graphql::Result> { - check_auth(ctx, |policy, token| policy.check_admin(token)).await?; let db = ctx.data::()?; let nt = ctx.data::()?; let configurations = match instrument_filters { @@ -390,6 +386,9 @@ impl Query { /// Queries that modify the state of the numtracker configuration in some way impl Mutation { /// Generate scan file locations for the next scan + #[graphql( + guard = "AuthGuard::Access{instrument: &instrument, instrument_session: &instrument_session}" + )] #[instrument(skip(self, ctx))] async fn scan( &self, @@ -398,10 +397,6 @@ impl Mutation { instrument_session: String, sub: Option, ) -> async_graphql::Result { - check_auth(ctx, |policy, token| { - policy.check_access(token, &instrument, &instrument_session) - }) - .await?; let db = ctx.data::()?; let nt = ctx.data::()?; // There is a race condition here if a process increments the file @@ -430,6 +425,7 @@ impl Mutation { } /// Add or modify the stored configuration for an instrument + #[graphql(guard = "AuthGuard::InstrumentAdmin{instrument: &instrument}")] #[instrument(skip(self, ctx))] async fn configure( &self, @@ -437,10 +433,6 @@ impl Mutation { instrument: String, config: ConfigurationUpdates, ) -> async_graphql::Result { - check_auth(ctx, |pc, token| { - pc.check_instrument_admin(token, &instrument) - }) - .await?; let db = ctx.data::()?; let nt = ctx.data::()?; trace!("Configuring: {instrument}: {config:?}"); @@ -453,24 +445,6 @@ impl Mutation { } } -async fn check_auth<'ctx, Check, R>(ctx: &Context<'ctx>, check: Check) -> async_graphql::Result<()> -where - Check: Fn(&'ctx PolicyCheck, Option<&'ctx Authorization>) -> R, - R: Future>, -{ - if let Some(policy) = ctx.data::>()? { - trace!("Auth enabled: checking token"); - let token = ctx.data::>>()?; - check(policy, token.as_ref()) - .await - .inspect_err(|e| info!("Authorization failed: {e:?}")) - .map_err(async_graphql::Error::from) - } else { - trace!("No authorization configured"); - Ok(()) - } -} - /// Changes that should be made to an instrument's configuration #[derive(Debug, InputObject)] struct ConfigurationUpdates {