From 92d6f5b087c665397da94453e85a835201345f0c Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 7 Dec 2022 14:46:08 +0100 Subject: [PATCH] data-model: simplify the oauth2 clients --- crates/axum-utils/src/client_authorization.rs | 16 +++----- .../src/oauth2/authorization_grant.rs | 4 +- crates/data-model/src/oauth2/client.rs | 38 +++---------------- crates/data-model/src/oauth2/session.rs | 4 +- crates/graphql/src/model/oauth.rs | 4 +- crates/handlers/src/oauth2/token.rs | 6 +-- crates/storage/src/oauth2/access_token.rs | 2 +- .../storage/src/oauth2/authorization_grant.rs | 14 +++---- crates/storage/src/oauth2/client.rs | 18 ++++----- crates/storage/src/oauth2/consent.rs | 16 ++++---- crates/storage/src/oauth2/mod.rs | 2 +- crates/storage/src/oauth2/refresh_token.rs | 2 +- 12 files changed, 46 insertions(+), 80 deletions(-) diff --git a/crates/axum-utils/src/client_authorization.rs b/crates/axum-utils/src/client_authorization.rs index 706f21ee..a7b9d027 100644 --- a/crates/axum-utils/src/client_authorization.rs +++ b/crates/axum-utils/src/client_authorization.rs @@ -26,15 +26,12 @@ use axum::{ }; use headers::{authorization::Basic, Authorization}; use http::{Request, StatusCode}; -use mas_data_model::{Client, JwksOrJwksUri, StorageBackend}; +use mas_data_model::{Client, JwksOrJwksUri}; use mas_http::HttpServiceExt; use mas_iana::oauth::OAuthClientAuthenticationMethod; use mas_jose::{jwk::PublicJsonWebKeySet, jwt::Jwt}; use mas_keystore::Encrypter; -use mas_storage::{ - oauth2::client::{lookup_client_by_client_id, ClientFetchError}, - PostgresqlBackend, -}; +use mas_storage::oauth2::client::{lookup_client_by_client_id, ClientFetchError}; use serde::{de::DeserializeOwned, Deserialize}; use serde_json::Value; use sqlx::PgExecutor; @@ -76,10 +73,7 @@ pub enum Credentials { } impl Credentials { - pub async fn fetch( - &self, - executor: impl PgExecutor<'_>, - ) -> Result, ClientFetchError> { + pub async fn fetch(&self, executor: impl PgExecutor<'_>) -> Result { let client_id = match self { Credentials::None { client_id } | Credentials::ClientSecretBasic { client_id, .. } @@ -91,12 +85,12 @@ impl Credentials { } #[tracing::instrument(skip_all, err)] - pub async fn verify( + pub async fn verify( &self, http_client_factory: &HttpClientFactory, encrypter: &Encrypter, method: &OAuthClientAuthenticationMethod, - client: &Client, + client: &Client, ) -> Result<(), CredentialsVerificationError> { match (self, method) { (Credentials::None { .. }, OAuthClientAuthenticationMethod::None) => {} diff --git a/crates/data-model/src/oauth2/authorization_grant.rs b/crates/data-model/src/oauth2/authorization_grant.rs index a183ac5d..9e3fd485 100644 --- a/crates/data-model/src/oauth2/authorization_grant.rs +++ b/crates/data-model/src/oauth2/authorization_grant.rs @@ -165,7 +165,7 @@ pub struct AuthorizationGrant { #[serde(flatten)] pub stage: AuthorizationGrantStage, pub code: Option, - pub client: Client, + pub client: Client, pub redirect_uri: Url, pub scope: oauth2_types::scope::Scope, pub state: Option, @@ -183,7 +183,7 @@ impl From> for AuthorizationGrant data: (), stage: g.stage.into(), code: g.code, - client: g.client.into(), + client: g.client, redirect_uri: g.redirect_uri, scope: g.scope, state: g.state, diff --git a/crates/data-model/src/oauth2/client.rs b/crates/data-model/src/oauth2/client.rs index c2768eb9..288e7a7a 100644 --- a/crates/data-model/src/oauth2/client.rs +++ b/crates/data-model/src/oauth2/client.rs @@ -20,10 +20,9 @@ use mas_jose::jwk::PublicJsonWebKeySet; use oauth2_types::requests::GrantType; use serde::Serialize; use thiserror::Error; +use ulid::Ulid; use url::Url; -use crate::traits::{StorageBackend, StorageBackendMarker}; - #[derive(Debug, Clone, PartialEq, Eq, Serialize)] #[serde(rename_all = "snake_case")] pub enum JwksOrJwksUri { @@ -34,11 +33,9 @@ pub enum JwksOrJwksUri { JwksUri(Url), } -#[derive(Debug, Clone, PartialEq, Serialize)] -#[serde(bound = "T: StorageBackend")] -pub struct Client { - #[serde(skip_serializing)] - pub data: T::ClientData, +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct Client { + pub id: Ulid, /// Client identifier pub client_id: String, @@ -98,31 +95,6 @@ pub struct Client { pub initiate_login_uri: Option, } -impl From> for Client<()> { - fn from(c: Client) -> Self { - Client { - data: (), - client_id: c.client_id, - encrypted_client_secret: c.encrypted_client_secret, - redirect_uris: c.redirect_uris, - response_types: c.response_types, - grant_types: c.grant_types, - contacts: c.contacts, - client_name: c.client_name, - logo_uri: c.logo_uri, - client_uri: c.client_uri, - policy_uri: c.policy_uri, - tos_uri: c.tos_uri, - jwks: c.jwks, - id_token_signed_response_alg: c.id_token_signed_response_alg, - userinfo_signed_response_alg: c.userinfo_signed_response_alg, - token_endpoint_auth_method: c.token_endpoint_auth_method, - token_endpoint_auth_signing_alg: c.token_endpoint_auth_signing_alg, - initiate_login_uri: c.initiate_login_uri, - } - } -} - #[derive(Debug, Error)] pub enum InvalidRedirectUriError { #[error("redirect_uri is not allowed for this client")] @@ -135,7 +107,7 @@ pub enum InvalidRedirectUriError { NoneRegistered, } -impl Client { +impl Client { pub fn resolve_redirect_uri<'a>( &'a self, redirect_uri: &'a Option, diff --git a/crates/data-model/src/oauth2/session.rs b/crates/data-model/src/oauth2/session.rs index 988b49af..7d6b21a4 100644 --- a/crates/data-model/src/oauth2/session.rs +++ b/crates/data-model/src/oauth2/session.rs @@ -27,7 +27,7 @@ pub struct Session { #[serde(skip_serializing)] pub data: T::SessionData, pub browser_session: BrowserSession, - pub client: Client, + pub client: Client, pub scope: Scope, } @@ -36,7 +36,7 @@ impl From> for Session<()> { Session { data: (), browser_session: s.browser_session, - client: s.client.into(), + client: s.client, scope: s.scope, } } diff --git a/crates/graphql/src/model/oauth.rs b/crates/graphql/src/model/oauth.rs index 2e306ee2..bc5d50df 100644 --- a/crates/graphql/src/model/oauth.rs +++ b/crates/graphql/src/model/oauth.rs @@ -56,13 +56,13 @@ impl OAuth2Session { /// An OAuth 2.0 client #[derive(Description)] -pub struct OAuth2Client(pub mas_data_model::Client); +pub struct OAuth2Client(pub mas_data_model::Client); #[Object(use_type_description)] impl OAuth2Client { /// ID of the object. pub async fn id(&self) -> ID { - NodeType::OAuth2Client.id(self.0.data) + NodeType::OAuth2Client.id(self.0.id) } /// OAuth 2.0 client ID diff --git a/crates/handlers/src/oauth2/token.rs b/crates/handlers/src/oauth2/token.rs index 725e0f04..c58d53b2 100644 --- a/crates/handlers/src/oauth2/token.rs +++ b/crates/handlers/src/oauth2/token.rs @@ -43,7 +43,7 @@ use mas_storage::{ RefreshTokenLookupError, }, }, - DatabaseInconsistencyError, LookupError, PostgresqlBackend, + DatabaseInconsistencyError, LookupError, }; use oauth2_types::{ errors::{ClientError, ClientErrorCode}, @@ -239,7 +239,7 @@ pub(crate) async fn post( #[allow(clippy::too_many_lines)] async fn authorization_code_grant( grant: &AuthorizationCodeGrant, - client: &Client, + client: &Client, key_store: &Keystore, url_builder: &UrlBuilder, mut txn: Transaction<'_, Postgres>, @@ -391,7 +391,7 @@ async fn authorization_code_grant( async fn refresh_token_grant( grant: &RefreshTokenGrant, - client: &Client, + client: &Client, mut txn: Transaction<'_, Postgres>, ) -> Result { let (clock, mut rng) = crate::rng_and_clock()?; diff --git a/crates/storage/src/oauth2/access_token.rs b/crates/storage/src/oauth2/access_token.rs index 21a0f61f..d5462dd0 100644 --- a/crates/storage/src/oauth2/access_token.rs +++ b/crates/storage/src/oauth2/access_token.rs @@ -28,7 +28,7 @@ use crate::{Clock, DatabaseInconsistencyError, LookupError, PostgresqlBackend}; skip_all, fields( session.id = %session.data, - client.id = %session.client.data, + client.id = %session.client.id, user.id = %session.browser_session.user.id, access_token.id, ), diff --git a/crates/storage/src/oauth2/authorization_grant.rs b/crates/storage/src/oauth2/authorization_grant.rs index f378a8cc..b8db3c0b 100644 --- a/crates/storage/src/oauth2/authorization_grant.rs +++ b/crates/storage/src/oauth2/authorization_grant.rs @@ -36,7 +36,7 @@ use crate::{Clock, DatabaseInconsistencyError, PostgresqlBackend}; #[tracing::instrument( skip_all, fields( - client.id = %client.data, + %client.id, grant.id, ), err(Debug), @@ -46,7 +46,7 @@ pub async fn new_authorization_grant( executor: impl PgExecutor<'_>, mut rng: impl Rng + Send, clock: &Clock, - client: Client, + client: Client, redirect_uri: Url, scope: Scope, code: Option, @@ -97,7 +97,7 @@ pub async fn new_authorization_grant( ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15) "#, Uuid::from(id), - Uuid::from(client.data), + Uuid::from(client.id), redirect_uri.to_string(), scope.to_string(), state, @@ -498,7 +498,7 @@ pub async fn lookup_grant_by_code( skip_all, fields( grant.id = %grant.data, - client.id = %grant.client.data, + client.id = %grant.client.id, session.id, user_session.id = %browser_session.id, user.id = %browser_session.user.id, @@ -552,7 +552,7 @@ pub async fn derive_session( skip_all, fields( grant.id = %grant.data, - client.id = %grant.client.data, + client.id = %grant.client.id, session.id = %session.data, user_session.id = %session.browser_session.id, user.id = %session.browser_session.user.id, @@ -592,7 +592,7 @@ pub async fn fulfill_grant( skip_all, fields( grant.id = %grant.data, - client.id = %grant.client.data, + client.id = %grant.client.id, ), err(Debug), )] @@ -622,7 +622,7 @@ pub async fn give_consent_to_grant( skip_all, fields( grant.id = %grant.data, - client.id = %grant.client.data, + client.id = %grant.client.id, ), err(Debug), )] diff --git a/crates/storage/src/oauth2/client.rs b/crates/storage/src/oauth2/client.rs index 7a2ed18b..65f3b704 100644 --- a/crates/storage/src/oauth2/client.rs +++ b/crates/storage/src/oauth2/client.rs @@ -28,7 +28,7 @@ use ulid::Ulid; use url::Url; use uuid::Uuid; -use crate::{Clock, LookupError, PostgresqlBackend}; +use crate::{Clock, LookupError}; // XXX: response_types & contacts #[derive(Debug)] @@ -90,11 +90,11 @@ impl LookupError for ClientFetchError { } } -impl TryInto> for OAuth2ClientLookup { +impl TryInto for OAuth2ClientLookup { type Error = ClientFetchError; #[allow(clippy::too_many_lines)] // TODO: refactor some of the field parsing - fn try_into(self) -> Result, Self::Error> { + fn try_into(self) -> Result { let redirect_uris: Result, _> = self.redirect_uris.iter().map(|s| s.parse()).collect(); let redirect_uris = redirect_uris.map_err(|source| ClientFetchError::ParseUrl { @@ -226,7 +226,7 @@ impl TryInto> for OAuth2ClientLookup { let id = Ulid::from(self.oauth2_client_id); Ok(Client { - data: id, + id, client_id: id.to_string(), encrypted_client_secret: self.encrypted_client_secret, redirect_uris, @@ -253,7 +253,7 @@ impl TryInto> for OAuth2ClientLookup { pub async fn lookup_clients( executor: impl PgExecutor<'_>, ids: impl IntoIterator + Send, -) -> Result>, ClientFetchError> { +) -> Result, ClientFetchError> { let ids: Vec = ids.into_iter().map(Uuid::from).collect(); let res = sqlx::query_as!( OAuth2ClientLookup, @@ -289,9 +289,9 @@ pub async fn lookup_clients( .fetch_all(executor) .await?; - let clients: Result>, _> = res + let clients: Result, _> = res .into_iter() - .map(|r| r.try_into().map(|c: Client| (c.data, c))) + .map(|r| r.try_into().map(|c: Client| (c.id, c))) .collect(); clients @@ -305,7 +305,7 @@ pub async fn lookup_clients( pub async fn lookup_client( executor: impl PgExecutor<'_>, id: Ulid, -) -> Result, ClientFetchError> { +) -> Result { let res = sqlx::query_as!( OAuth2ClientLookup, r#" @@ -353,7 +353,7 @@ pub async fn lookup_client( pub async fn lookup_client_by_client_id( executor: impl PgExecutor<'_>, client_id: &str, -) -> Result, ClientFetchError> { +) -> Result { let id: Ulid = client_id.parse()?; lookup_client(executor, id).await } diff --git a/crates/storage/src/oauth2/consent.rs b/crates/storage/src/oauth2/consent.rs index c3883537..95bd78cb 100644 --- a/crates/storage/src/oauth2/consent.rs +++ b/crates/storage/src/oauth2/consent.rs @@ -21,20 +21,20 @@ use sqlx::PgExecutor; use ulid::Ulid; use uuid::Uuid; -use crate::{Clock, PostgresqlBackend}; +use crate::Clock; #[tracing::instrument( skip_all, fields( %user.id, - client.id = %client.data, + %client.id, ), err(Debug), )] pub async fn fetch_client_consent( executor: impl PgExecutor<'_>, user: &User, - client: &Client, + client: &Client, ) -> Result { let scope_tokens: Vec = sqlx::query_scalar!( r#" @@ -43,7 +43,7 @@ pub async fn fetch_client_consent( WHERE user_id = $1 AND oauth2_client_id = $2 "#, Uuid::from(user.id), - Uuid::from(client.data), + Uuid::from(client.id), ) .fetch_all(executor) .await?; @@ -60,8 +60,8 @@ pub async fn fetch_client_consent( skip_all, fields( %user.id, - client.id = %client.data, - scope = scope.to_string(), + %client.id, + %scope, ), err(Debug), )] @@ -70,7 +70,7 @@ pub async fn insert_client_consent( mut rng: impl Rng + Send, clock: &Clock, user: &User, - client: &Client, + client: &Client, scope: &Scope, ) -> Result<(), anyhow::Error> { let now = clock.now(); @@ -93,7 +93,7 @@ pub async fn insert_client_consent( "#, &ids, Uuid::from(user.id), - Uuid::from(client.data), + Uuid::from(client.id), &tokens, now, ) diff --git a/crates/storage/src/oauth2/mod.rs b/crates/storage/src/oauth2/mod.rs index 0f8e1485..d20aea05 100644 --- a/crates/storage/src/oauth2/mod.rs +++ b/crates/storage/src/oauth2/mod.rs @@ -40,7 +40,7 @@ pub mod refresh_token; session.id = %session.data, user.id = %session.browser_session.user.id, user_session.id = %session.browser_session.id, - client.id = %session.client.data, + client.id = %session.client.id, ), err(Debug), )] diff --git a/crates/storage/src/oauth2/refresh_token.rs b/crates/storage/src/oauth2/refresh_token.rs index 65b7f168..5433c968 100644 --- a/crates/storage/src/oauth2/refresh_token.rs +++ b/crates/storage/src/oauth2/refresh_token.rs @@ -32,7 +32,7 @@ use crate::{Clock, DatabaseInconsistencyError, LookupError, PostgresqlBackend}; session.id = %session.data, user.id = %session.browser_session.user.id, user_session.id = %session.browser_session.id, - client.id = %session.client.data, + client.id = %session.client.id, refresh_token.id, ), err(Debug),