From 9da86f005fae6e3aadb06de74b8c73959f3b1558 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 | 47 +++++++++++++++++++++++++++++++++++++++++++-- src/graphql/mod.rs | 44 +++++++++--------------------------------- 2 files changed, 54 insertions(+), 37 deletions(-) diff --git a/src/graphql/auth.rs b/src/graphql/auth.rs index 54ff884..9b6b286 100644 --- a/src/graphql/auth.rs +++ b/src/graphql/auth.rs @@ -14,12 +14,12 @@ use std::str::FromStr; -use async_graphql::{Error, ErrorExtensions}; +use async_graphql::{Context, Error, ErrorExtensions, 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; @@ -180,6 +180,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(|e| e.extend()) + } 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 707d42b..88cb651 100644 --- a/src/graphql/mod.rs +++ b/src/graphql/mod.rs @@ -14,18 +14,17 @@ use std::any; use std::borrow::Cow; -use std::future::Future; use std::io::Write; use std::path::{Component, PathBuf}; use async_graphql::extensions::Tracing; use async_graphql::http::GraphiQLSource; use async_graphql::{ - Context, Description, EmptySubscription, ErrorExtensions, InputObject, InputValueError, - InputValueResult, Object, Scalar, ScalarType, Schema, SimpleObject, TypeName, Value, + Context, Description, EmptySubscription, InputObject, InputValueError, InputValueResult, + 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}; @@ -357,16 +356,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:?}"); @@ -376,13 +372,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 { @@ -405,6 +401,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, @@ -413,10 +412,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 @@ -445,6 +440,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, @@ -452,10 +448,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:?}"); @@ -468,24 +460,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(|e| e.extend()) - } else { - trace!("No authorization configured"); - Ok(()) - } -} - /// Changes that should be made to an instrument's configuration #[derive(Debug, InputObject)] struct ConfigurationUpdates {