From 301302f1d0cf016b1b35080819edc199fb387810 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Sun, 15 Jun 2025 14:41:09 +0100 Subject: [PATCH] Support access control based on user roles --- ...16d8a3548e0a90adaf27ba526cb3894b56a6b.json | 22 +++++++ nixos_module.nix | 4 +- sample/config.toml | 2 +- src/config.rs | 14 +++-- src/store.rs | 18 ++++++ src/web/oauth_openid/authorisation.rs | 61 +++++++++++++++---- templates/pages/access_wrong_role.hnb | 13 ++++ translations/en/oidc_auth.ftl | 4 ++ 8 files changed, 117 insertions(+), 21 deletions(-) create mode 100644 .sqlx/query-3a0c05d42c2f5cb868649a5c2a216d8a3548e0a90adaf27ba526cb3894b56a6b.json create mode 100644 templates/pages/access_wrong_role.hnb create mode 100644 translations/en/oidc_auth.ftl diff --git a/.sqlx/query-3a0c05d42c2f5cb868649a5c2a216d8a3548e0a90adaf27ba526cb3894b56a6b.json b/.sqlx/query-3a0c05d42c2f5cb868649a5c2a216d8a3548e0a90adaf27ba526cb3894b56a6b.json new file mode 100644 index 0000000..2081770 --- /dev/null +++ b/.sqlx/query-3a0c05d42c2f5cb868649a5c2a216d8a3548e0a90adaf27ba526cb3894b56a6b.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT role_id\n FROM users_roles\n WHERE user_id = $1\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "role_id", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Uuid" + ] + }, + "nullable": [ + false + ] + }, + "hash": "3a0c05d42c2f5cb868649a5c2a216d8a3548e0a90adaf27ba526cb3894b56a6b" +} diff --git a/nixos_module.nix b/nixos_module.nix index 4d9dc74..6ad4b0e 100644 --- a/nixos_module.nix +++ b/nixos_module.nix @@ -26,10 +26,10 @@ let Consult the documentation for the other service if you aren't sure. ''; }; - allow_user_classes = mkOption { + allow_user_roles = mkOption { type = types.listOf types.str; description = '' - List of user classes which are authorised (allowed) to use this client (access this service). + List of user roles which are authorised (allowed) to use this client (access this service). As of idCoop v0.0.1, this setting is unimplemented and has no effect. ''; }; diff --git a/sample/config.toml b/sample/config.toml index 72a7067..7d97722 100644 --- a/sample/config.toml +++ b/sample/config.toml @@ -10,7 +10,7 @@ rsa_keypair = "keypair.pem" [oidc.clients.x] name = "some service" redirect_uris = ["http://localhost:9876/callback"] -allow_user_classes = ["user"] +allow_user_roles = ["*"] [postgres] connect = "postgres:" diff --git a/src/config.rs b/src/config.rs index 6d36929..5dd1b17 100644 --- a/src/config.rs +++ b/src/config.rs @@ -109,12 +109,14 @@ pub struct OidcClientConfiguration { /// Friendly name for the service. Will be shown in the user interface. pub name: String, - /// TODO User classes to allow - /// Must be explicit because it is security sensitive and we don't want a typo to fail open. - /// User classes are defined by the admin but at the very least includes 'active' and 'not active' (implied if no 'active' class set). - /// (Design subject to change) - /// TODO not sure this design is current - pub allow_user_classes: Vec, + /// User roles to allow to access this application. + /// + /// The `*` 'role' can be used to allow all users to access this application. + /// + /// Must always be explicitly specified. + /// + /// Warning: This setting does not currently apply retrospectively / to existing sessions. + pub allow_user_roles: Vec, /// The shared secret for the client. /// Must be populated (this is checked on startup). diff --git a/src/store.rs b/src/store.rs index c0db3fb..7af9818 100644 --- a/src/store.rs +++ b/src/store.rs @@ -2,6 +2,8 @@ //! //! This file contains PostgreSQL queries. +use std::collections::BTreeSet; + use chrono::DateTime; use chrono::NaiveDateTime; use chrono::Utc; @@ -412,4 +414,20 @@ impl IdCoopStoreTxn<'_, '_> { user_id: row.user_id, })) } + + /// Fetches all role_ids of roles that the given user has been granted. + pub async fn get_user_role_ids(&mut self, user_id: Uuid) -> eyre::Result> { + sqlx::query_scalar!( + " + SELECT role_id + FROM users_roles + WHERE user_id = $1 + ", + user_id + ) + .fetch_all(&mut **self.txn) + .await + .context("failed to fetch roleset of user") + .map(|v| v.into_iter().collect()) + } } diff --git a/src/web/oauth_openid/authorisation.rs b/src/web/oauth_openid/authorisation.rs index 4e0c241..068b758 100644 --- a/src/web/oauth_openid/authorisation.rs +++ b/src/web/oauth_openid/authorisation.rs @@ -17,18 +17,21 @@ use tracing::{error, warn}; use crate::{ config::{Configuration, OidcClientConfiguration}, + store::IdCoopStore, utils::{Clock, RandGen}, web::{ ambient::Ambient, login::LoginSession, make_login_redirect, oauth_openid::ext_codes::{AuthCode, AuthCodeBinding}, - DesiredLocale, Rendered, TEMPLATING, + DesiredLocale, Rendered, WebResult, TEMPLATING, }, }; use super::ext_codes::VolatileCodeStore; +pub const EVERYONE_ROLE: &str = "*"; + /// Query string parameters for the OIDC authorisation request. #[derive(Deserialize, Debug)] pub struct AuthorisationQuery { @@ -75,29 +78,30 @@ pub(crate) async fn oidc_authorisation( Extension(config): Extension>, Extension(code_store): Extension, Extension(clock): Extension, + Extension(store): Extension>, Extension(mut randgen): Extension, DesiredLocale(locale): DesiredLocale, OriginalUri(uri): OriginalUri, -) -> Response { +) -> WebResult { let Query(query) = match query { Ok(query) => query, Err(err) => { // TODO(ui) this should be a pretty page - return ( + return Ok(( StatusCode::BAD_REQUEST, format!("TODO bad authorisation request: {err:?}"), ) - .into_response(); + .into_response()); } }; if let Some(nonce) = &query.nonce { if nonce.len() > MAX_NONCE_LENGTH { - return ( + return Ok(( StatusCode::BAD_REQUEST, format!("Bad authorisation request: Nonce too long (> {MAX_NONCE_LENGTH})"), ) - .into_response(); + .into_response()); } } @@ -107,17 +111,50 @@ pub(crate) async fn oidc_authorisation( let (client_id, client_config) = match validate_authorisation_basics(&query, &config) { Ok(x) => x, - Err(resp) => return resp, + Err(resp) => return Ok(resp), }; // If the user isn't logged in, we need to get them to do that first and then come back here. let Some(login_session) = login_session else { - return make_login_redirect(uri); + return Ok(make_login_redirect(uri)); }; + // Check if the user has the correct role to access this application + if !client_config + .allow_user_roles + .iter() + .any(|s| s == EVERYONE_ROLE) + { + // The application isn't available to the EVERYONE (*) role, so we need to + // consider the user's specific roles and match them against the list. + let user_roles = store + .txn(|mut txn| { + Box::pin(async move { txn.get_user_role_ids(login_session.user_id).await }) + }) + .await?; + + if !client_config + .allow_user_roles + .iter() + .any(|role| user_roles.contains(role)) + { + // User doesn't have the right role + return Ok(( + StatusCode::FORBIDDEN, + Rendered(render_template_string!( + TEMPLATING, + access_wrong_role, + locale, + { ambient, client_name: client_config.name.clone() } + )), + ) + .into_response()); + } + } + // If the application requires consent, then we should ask for that. if !client_config.skip_consent { - return show_consent_page( + return Ok(show_consent_page( ambient, login_session, client_config, @@ -126,12 +163,12 @@ pub(crate) async fn oidc_authorisation( &config, &query.redirect_uri, ) - .await; + .await); } // No consent needed: process the authorisation. - process_authorisation( + Ok(process_authorisation( query, login_session, client_id, @@ -141,7 +178,7 @@ pub(crate) async fn oidc_authorisation( &clock, &code_store, ) - .await + .await) } /// Body parameters for OIDC authorisation consent. diff --git a/templates/pages/access_wrong_role.hnb b/templates/pages/access_wrong_role.hnb new file mode 100644 index 0000000..b84d4b4 --- /dev/null +++ b/templates/pages/access_wrong_role.hnb @@ -0,0 +1,13 @@ +CentredPage {$ambient} + :title + @access_wrong_role_title + + :main + h1 + @access_wrong_role_title + + article + @access_wrong_role_main1 + strong + "${$client_name}" + @access_wrong_role_main2 diff --git a/translations/en/oidc_auth.ftl b/translations/en/oidc_auth.ftl new file mode 100644 index 0000000..89666e1 --- /dev/null +++ b/translations/en/oidc_auth.ftl @@ -0,0 +1,4 @@ +access_wrong_role_title = Access denied + +access_wrong_role_main1 = You don't have the right role to access{" "} +access_wrong_role_main2 = . Contact your administrator for assistance if this is unexpected.