From 9ae71553478ba84ab716be0597e15fc19267fcfb Mon Sep 17 00:00:00 2001 From: Benjamin Sobel Date: Mon, 3 Aug 2026 01:21:51 -0700 Subject: [PATCH 1/7] fix promote anonymous crashing --- crates/core/src/auth/auth_test.rs | 71 +++++++++++++++++++++++++++++++ crates/core/src/auth/tokens.rs | 10 +++-- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/crates/core/src/auth/auth_test.rs b/crates/core/src/auth/auth_test.rs index feb172651..337875680 100644 --- a/crates/core/src/auth/auth_test.rs +++ b/crates/core/src/auth/auth_test.rs @@ -1422,6 +1422,77 @@ async fn test_auth_annonymous_signin() { .unwrap(); } +#[tokio::test] +async fn test_auth_refresh_after_anonymous_promotion() { + let mailer = TestAsyncSmtpTransport::new(); + let state = test_state(Some(TestStateOptions { + mailer: Some(Mailer::Smtp(Arc::new(mailer.clone()))), + config: Some({ + let mut config = build_test_config_with_trivial_tokens(); + config.auth.user_identifier = Some(UserIdentifier::RequireEmail.into()); + config.auth.enable_anonymous_signin = Some(true); + config + }), + ..Default::default() + })) + .await + .unwrap(); + + let response = login_anonymous_user_handler( + State(state.clone()), + Query(Default::default()), + Cookies::default(), + Either::Json(LoginAnonymousRequest { + params: Default::default(), + }), + ) + .await + .unwrap(); + + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let login_response: LoginResponse = serde_json::from_slice(&body).unwrap(); + let user = User::from_auth_token(&state, &login_response.auth_token).unwrap(); + + // Anonymous users are unverified but have no email, so refreshing has to keep working for them. + let Json(_refreshed_tokens) = refresh_handler( + State(state.clone()), + Json(RefreshRequest { + refresh_token: login_response.refresh_token.clone(), + }), + ) + .await + .unwrap(); + + promote_anonymous_user_handler( + State(state.clone()), + Query(Default::default()), + user.clone(), + Either::Json(PromoteAnonymousRequest { + new_password: "secret123".to_string(), + new_email: Some("user@test.org".to_string()), + ..Default::default() + }), + ) + .await + .unwrap(); + + // Promotion attaches a not-yet-verified email to the pre-existing session. Since tokens must + // never be minted for a user with an unverified email, refreshing that session has to fail + // rather than hand out fresh tokens. + assert!(matches!( + refresh_handler( + State(state.clone()), + Json(RefreshRequest { + refresh_token: login_response.refresh_token, + }), + ) + .await, + Err(AuthError::Unauthorized) + )); +} + async fn session_exists(state: &AppState, user_id: Uuid) -> bool { return state .session_conn() diff --git a/crates/core/src/auth/tokens.rs b/crates/core/src/auth/tokens.rs index 8562fba98..ffbc9cb81 100644 --- a/crates/core/src/auth/tokens.rs +++ b/crates/core/src/auth/tokens.rs @@ -231,20 +231,24 @@ pub(crate) async fn reauth_with_refresh_token( return Err(AuthError::Unauthorized); }; - const USER_QUERY: &str = formatcp!(r#"SELECT * FROM "{USER_TABLE}" WHERE id = $1"#); + // NOTE: The `verified` condition mirrors `mint_new_tokens`: a user with an email must have it + // verified before we hand out tokens. Anonymous users are exempt, since they have no email. + const USER_QUERY: &str = + formatcp!(r#"SELECT * FROM "{USER_TABLE}" WHERE id = $1 AND (email IS NULL OR verified)"#); let Some(db_user) = state .user_conn() .read_query_value::(USER_QUERY, params!(user_id)) .await? else { - // Row not found case, typically expected in one of 4 cases: + // Row not found case, typically expected in one of 5 cases: // 1. Above where clause doesn't match, e.g. refresh token expired. // 2. Token was actively deleted and thus revoked. // 3. User explicitly logged out, which will delete **all** sessions for that user. // 4. Database was overwritten, e.g. by tests or periodic reset for the demo. + // 5. User's email is not verified (yet), e.g. right after promoting an anonymous user. #[cfg(debug_assertions)] - log::debug!("User not found"); + log::debug!("User not found or unverified"); return Err(AuthError::Unauthorized); }; From 3a3ba93d3d4c2fcbd5bb74ce620634c7730c63a5 Mon Sep 17 00:00:00 2001 From: Benjamin Sobel Date: Mon, 3 Aug 2026 01:45:31 -0700 Subject: [PATCH 2/7] Improve the test --- crates/core/src/auth/auth_test.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/core/src/auth/auth_test.rs b/crates/core/src/auth/auth_test.rs index 337875680..9d2c7740c 100644 --- a/crates/core/src/auth/auth_test.rs +++ b/crates/core/src/auth/auth_test.rs @@ -1485,12 +1485,30 @@ async fn test_auth_refresh_after_anonymous_promotion() { refresh_handler( State(state.clone()), Json(RefreshRequest { - refresh_token: login_response.refresh_token, + refresh_token: login_response.refresh_token.clone(), }), ) .await, Err(AuthError::Unauthorized) )); + + state + .user_conn() + .execute_batch(format!( + "UPDATE {USER_TABLE} SET verified = TRUE WHERE email = 'user@test.org';" + )) + .await + .unwrap(); + + // Verifying doesn't revoke the session, it merely paused it. Refreshing works again. + let Json(_refreshed_tokens) = refresh_handler( + State(state.clone()), + Json(RefreshRequest { + refresh_token: login_response.refresh_token, + }), + ) + .await + .unwrap(); } async fn session_exists(state: &AppState, user_id: Uuid) -> bool { From 41fc33aa04be1d77de4fa2d68b35fc5f1eb13af4 Mon Sep 17 00:00:00 2001 From: Benjamin Sobel Date: Mon, 3 Aug 2026 02:55:03 -0700 Subject: [PATCH 3/7] Configurable scopes --- crates/assets/js/admin/proto/config.ts | 33 ++- .../js/admin/src/components/FormFields.tsx | 34 +++ .../src/components/settings/AuthSettings.tsx | 24 +- crates/core/proto/config.proto | 8 + crates/core/src/auth/oauth/callback.rs | 15 +- crates/core/src/auth/oauth/oauth_test.rs | 216 ++++++++++++++++-- crates/core/src/auth/oauth/provider.rs | 10 +- crates/core/src/auth/oauth/providers/apple.rs | 4 +- .../core/src/auth/oauth/providers/discord.rs | 4 +- .../core/src/auth/oauth/providers/facebook.rs | 4 +- .../core/src/auth/oauth/providers/github.rs | 4 +- .../core/src/auth/oauth/providers/gitlab.rs | 4 +- .../core/src/auth/oauth/providers/google.rs | 4 +- .../src/auth/oauth/providers/microsoft.rs | 4 +- crates/core/src/auth/oauth/providers/oidc.rs | 19 +- crates/core/src/auth/oauth/providers/test.rs | 4 +- .../core/src/auth/oauth/providers/twitch.rs | 4 +- .../core/src/auth/oauth/providers/yandex.rs | 4 +- crates/core/src/config.rs | 82 +++++++ 19 files changed, 434 insertions(+), 47 deletions(-) diff --git a/crates/assets/js/admin/proto/config.ts b/crates/assets/js/admin/proto/config.ts index 55362f75f..8da38e93c 100644 --- a/crates/assets/js/admin/proto/config.ts +++ b/crates/assets/js/admin/proto/config.ts @@ -440,7 +440,18 @@ export interface OAuthProviderConfig { displayName?: string | undefined; authUrl?: string | undefined; tokenUrl?: string | undefined; - userApiUrl?: string | undefined; + userApiUrl?: + | string + | undefined; + /** + * Replaces the provider's default scopes, when set. Needed for providers that + * don't offer the defaults, e.g. an OIDC provider w/o `email` or `profile`. + * + * NOTE: Claims not covered by the requested scopes won't be returned by the + * provider's user-info endpoint, i.e. dropping `email` requires a + * username-based `UserIdentifier`. + */ + scopes: string[]; } export interface AuthConfig { @@ -1076,7 +1087,7 @@ export const EmailConfig: MessageFns = { }; function createBaseOAuthProviderConfig(): OAuthProviderConfig { - return {}; + return { scopes: [] }; } export const OAuthProviderConfig: MessageFns = { @@ -1102,6 +1113,9 @@ export const OAuthProviderConfig: MessageFns = { if (message.userApiUrl !== undefined && message.userApiUrl !== "") { writer.uint32(114).string(message.userApiUrl); } + for (const v of message.scopes) { + writer.uint32(122).string(v!); + } return writer; }, @@ -1168,6 +1182,14 @@ export const OAuthProviderConfig: MessageFns = { message.userApiUrl = reader.string(); continue; } + case 15: { + if (tag !== 122) { + break; + } + + message.scopes.push(reader.string()); + continue; + } } if ((tag & 7) === 4 || tag === 0) { break; @@ -1214,6 +1236,9 @@ export const OAuthProviderConfig: MessageFns = { : isSet(object.user_api_url) ? globalThis.String(object.user_api_url) : undefined, + scopes: globalThis.Array.isArray(object?.scopes) + ? object.scopes.map((e: any) => globalThis.String(e)) + : [], }; }, @@ -1240,6 +1265,9 @@ export const OAuthProviderConfig: MessageFns = { if (message.userApiUrl !== undefined && message.userApiUrl !== "") { obj.userApiUrl = message.userApiUrl; } + if (message.scopes?.length) { + obj.scopes = message.scopes; + } return obj; }, @@ -1255,6 +1283,7 @@ export const OAuthProviderConfig: MessageFns = { message.authUrl = object.authUrl ?? ""; message.tokenUrl = object.tokenUrl ?? ""; message.userApiUrl = object.userApiUrl ?? ""; + message.scopes = object.scopes?.map((e) => e) || []; return message; }, }; diff --git a/crates/assets/js/admin/src/components/FormFields.tsx b/crates/assets/js/admin/src/components/FormFields.tsx index eec8b655a..79ef66228 100644 --- a/crates/assets/js/admin/src/components/FormFields.tsx +++ b/crates/assets/js/admin/src/components/FormFields.tsx @@ -133,6 +133,40 @@ export function buildOptionalTextFormField(opts: TextFieldOptions) { }; } +/// Used for repeated proto string fields, entered as a whitespace-separated list. +export function buildStringListFormField(opts: Omit) { + return function builder(field: () => FieldApiT) { + return ( + +
+ {opts.label()} + + { + const value = (e.target as HTMLInputElement).value; + field().handleChange(value.split(/\s+/).filter((s) => s !== "")); + }} + onInput={opts.onInput} + data-testid="input" + /> + + + +
+ + ); + }; +} + export function buildSecretFormField(opts: Omit) { const [type, setType] = createSignal("password"); diff --git a/crates/assets/js/admin/src/components/settings/AuthSettings.tsx b/crates/assets/js/admin/src/components/settings/AuthSettings.tsx index d69e3bb27..be687011b 100644 --- a/crates/assets/js/admin/src/components/settings/AuthSettings.tsx +++ b/crates/assets/js/admin/src/components/settings/AuthSettings.tsx @@ -16,6 +16,7 @@ import { buildOptionalBoolFormField, buildOptionalSecretFormField, buildOptionalTextFormField, + buildStringListFormField, } from "@/components/FormFields"; import { Accordion, @@ -137,11 +138,11 @@ function proxyToConfig(proxy: AuthConfigProxy): AuthConfig { const clientSecret = entry.state?.clientSecret?.trim(); if (clientId && clientSecret) { - config.oauthProviders[p.name] = { + config.oauthProviders[p.name] = OAuthProviderConfig.fromPartial({ providerId: p.id, ...entry.state, - }; + }); } else { console.debug("Skipping incomplete: ", entry); } @@ -177,7 +178,7 @@ function ProviderSettingsSubForm(props: { } const s = state.values.namedOAuthProviders[props.index].state; - setOnce({ ...s }); + setOnce(s && OAuthProviderConfig.fromPartial(s)); return s; })(), ); @@ -258,6 +259,23 @@ function ProviderSettingsSubForm(props: { > {buildOptionalTextFormField({ label: () => User API URL })} + + + {buildStringListFormField({ + label: () => Scopes, + placeholder: "openid email profile", + info: ( +

+ Space-separated scopes to request. Empty means the defaults: + "openid email profile". Only claims covered by the requested + scopes are returned, so dropping "email" requires a + username-based user identifier above. +

+ ), + })} + diff --git a/crates/core/proto/config.proto b/crates/core/proto/config.proto index db81f7aad..c506c7306 100644 --- a/crates/core/proto/config.proto +++ b/crates/core/proto/config.proto @@ -64,6 +64,14 @@ message OAuthProviderConfig { optional string auth_url = 12; optional string token_url = 13; optional string user_api_url = 14; + + // Replaces the provider's default scopes, when set. Needed for providers that + // don't offer the defaults, e.g. an OIDC provider w/o `email` or `profile`. + // + // NOTE: Claims not covered by the requested scopes won't be returned by the + // provider's user-info endpoint, i.e. dropping `email` requires a + // username-based `UserIdentifier`. + repeated string scopes = 15; } // What user identifier to use for new user registrations as well as diff --git a/crates/core/src/auth/oauth/callback.rs b/crates/core/src/auth/oauth/callback.rs index 898e77cf1..8c55c5984 100644 --- a/crates/core/src/auth/oauth/callback.rs +++ b/crates/core/src/auth/oauth/callback.rs @@ -300,6 +300,19 @@ async fn create_user_for_external_provider( return Err(AuthError::Unauthorized); } + // Providers only return claims covered by the scopes we requested, so the email may be missing, + // e.g. for an OIDC provider configured w/o the `email` scope. Only username-based identifiers + // can do without one. + let email: Option = match (email, user_identifier) { + (Some(email), _) => Some(email), + (None, UserIdentifier::OnlyUsername | UserIdentifier::RequireUsername) => None, + (None, _) => { + return Err(AuthError::BadRequest( + "OAuth provider returned no email address. Requires a username-based `user_identifier`", + )); + } + }; + let mut username: Option = match (user_identifier, username) { (UserIdentifier::OnlyEmail | UserIdentifier::Undefined, _) => None, ( @@ -401,7 +414,7 @@ mod tests { return OAuthUser { provider_user_id: rand.clone(), provider_id: OAuthProviderId::Test, - email: format!("email_{rand}@test.org"), + email: Some(format!("email_{rand}@test.org")), username, verified: true, avatar: None, diff --git a/crates/core/src/auth/oauth/oauth_test.rs b/crates/core/src/auth/oauth/oauth_test.rs index c0ac78c45..68de215d5 100644 --- a/crates/core/src/auth/oauth/oauth_test.rs +++ b/crates/core/src/auth/oauth/oauth_test.rs @@ -1,5 +1,5 @@ use axum::extract::{Form, Json, Path, Query, State}; -use axum::response::{IntoResponse, Redirect}; +use axum::response::{IntoResponse, Redirect, Response}; use axum::routing::{Router, get, post}; use axum_test::{TestServer, TestServerConfig}; use base64::prelude::*; @@ -12,6 +12,7 @@ use uuid::Uuid; use crate::api::AuthTokenClaims; use crate::app_state::{AppState, TestStateOptions, test_state}; +use crate::auth::AuthError; use crate::auth::api::token::{ AuthCodeToTokenRequest, TokenResponse as TokenHandlerResponse, auth_code_to_token_handler, }; @@ -21,7 +22,7 @@ use crate::auth::oauth::state::OAuthStateClaims; use crate::auth::oauth::{callback, list_providers, login}; use crate::auth::user::DbUser; use crate::auth::util::derive_pkce_code_challenge; -use crate::config::proto::{Config, OAuthProviderConfig, OAuthProviderId}; +use crate::config::proto::{Config, OAuthProviderConfig, OAuthProviderId, UserIdentifier}; use crate::constants::{ AUTH_API_PATH, COOKIE_AUTH_TOKEN, COOKIE_OAUTH_STATE, COOKIE_REFRESH_TOKEN, SESSION_TABLE, USER_TABLE, @@ -56,11 +57,54 @@ struct TokenResponse { const EXTERNAL_USER_ID: &str = "ExternalUserId"; const EXTERNAL_USER_EMAIL: &str = "foo@bar.com"; +/// The name the OIDC factory registers itself under, i.e. the required config key. +const OIDC_PROVIDER_NAME: &str = "oidc0"; + +struct FakeProviderOptions { + provider_id: OAuthProviderId, + provider_name: String, + /// Overrides the provider's default scopes when non-empty. + scopes: Vec, + user_identifier: Option, + /// Payload served by the fake user-info endpoint. + user_info: serde_json::Value, +} + async fn setup_fake_oauth_server(site_url: &str) -> (TestServer, AppState) { + return setup_fake_provider( + site_url, + FakeProviderOptions { + provider_id: OAuthProviderId::Test, + provider_name: TestOAuthProvider::NAME.to_string(), + scopes: vec![], + user_identifier: None, + user_info: serde_json::to_value(TestUser { + id: EXTERNAL_USER_ID.to_string(), + email: EXTERNAL_USER_EMAIL.to_string(), + verified: true, + }) + .unwrap(), + }, + ) + .await; +} + +async fn setup_fake_provider( + site_url: &str, + options: FakeProviderOptions, +) -> (TestServer, AppState) { const AUTH_PATH: &str = "/auth"; const TOKEN_PATH: &str = "/token"; const USER_INFO_PATH: &str = "/user"; + let FakeProviderOptions { + provider_id, + provider_name, + scopes, + user_identifier, + user_info, + } = options; + let app = Router::new() // AUTH endpoint takes: app info, desired auth flow (e.g. PKCE) and provides a redirect to the // provider's login form. Called by TB's /oauth//login handler. @@ -85,16 +129,7 @@ async fn setup_fake_oauth_server(site_url: &str) -> (TestServer, AppState) { ) // USER_INFO endpoint provides user information given an autorized get request, e.g. tokens in // the cookies. Called by TB's /oauth//callback. - .route( - USER_INFO_PATH, - get(|| async { - Json(TestUser { - id: EXTERNAL_USER_ID.to_string(), - email: EXTERNAL_USER_EMAIL.to_string(), - verified: true, - }) - }), - ); + .route(USER_INFO_PATH, get(|| async move { Json(user_info) })); let server = TestServer::new_with_config( app, @@ -108,16 +143,18 @@ async fn setup_fake_oauth_server(site_url: &str) -> (TestServer, AppState) { config: Some({ let mut config = Config::new_with_custom_defaults(); config.server.site_url = Some(site_url.to_string()); + config.auth.user_identifier = user_identifier.map(|ui| ui as i32); config.auth.oauth_providers = [( - TestOAuthProvider::NAME.to_string(), + provider_name.clone(), OAuthProviderConfig { client_id: Some("test_client_id".to_string()), client_secret: Some("test_client_secret".to_string()), - provider_id: Some(OAuthProviderId::Test as i32), + provider_id: Some(provider_id as i32), // OIDC paths auth_url: Some(server.server_url(AUTH_PATH).unwrap().to_string()), token_url: Some(server.server_url(TOKEN_PATH).unwrap().to_string()), user_api_url: Some(server.server_url(USER_INFO_PATH).unwrap().to_string()), + scopes, ..Default::default() }, )] @@ -133,13 +170,13 @@ async fn setup_fake_oauth_server(site_url: &str) -> (TestServer, AppState) { let auth_options = state.auth_options(); let providers = auth_options.list_oauth_providers(); assert_eq!(providers.len(), 1); - assert_eq!(providers[0].name, TestOAuthProvider::NAME); + assert_eq!(providers[0].name, provider_name); let Json(response) = list_providers::list_configured_providers_handler(State(state.clone())) .await .unwrap(); assert_eq!(response.providers.len(), 1); - assert_eq!(response.providers[0].0, TestOAuthProvider::NAME); + assert_eq!(response.providers[0].0, provider_name); return (server, state); } @@ -378,6 +415,153 @@ async fn test_oauth_login_flow_with_pkce() { ); } +#[tokio::test] +async fn test_oidc_requests_default_scopes() { + let site_url = "https://bar.org"; + let (_server, state) = setup_fake_provider( + site_url, + FakeProviderOptions { + provider_id: OAuthProviderId::Oidc0, + provider_name: OIDC_PROVIDER_NAME.to_string(), + scopes: vec![], + user_identifier: None, + user_info: serde_json::json!({ + "sub": EXTERNAL_USER_ID, + "email": EXTERNAL_USER_EMAIL, + "email_verified": true, + "preferred_username": "external_user", + }), + }, + ) + .await; + + let (auth_query, result) = run_login_flow(&state, OIDC_PROVIDER_NAME, site_url).await; + result.unwrap(); + + assert_eq!(auth_query.scope, "openid email profile"); + + let db_user = user_by_provider_user_id(&state, EXTERNAL_USER_ID) + .await + .unwrap(); + assert_eq!(EXTERNAL_USER_EMAIL, db_user.email.as_deref().unwrap()); +} + +/// Providers that only grant `openid` are the motivation for configurable scopes: without an +/// `email` scope there's no email claim, so the user has to be identified by username instead. +#[tokio::test] +async fn test_oidc_with_configured_scopes_and_no_email_claim() { + let site_url = "https://bar.org"; + let (_server, state) = setup_fake_provider( + site_url, + FakeProviderOptions { + provider_id: OAuthProviderId::Oidc0, + provider_name: OIDC_PROVIDER_NAME.to_string(), + scopes: vec!["openid".to_string()], + user_identifier: Some(UserIdentifier::RequireUsername), + user_info: serde_json::json!({ "sub": EXTERNAL_USER_ID }), + }, + ) + .await; + + let (auth_query, result) = run_login_flow(&state, OIDC_PROVIDER_NAME, site_url).await; + result.unwrap(); + + assert_eq!(auth_query.scope, "openid"); + + let db_user = user_by_provider_user_id(&state, EXTERNAL_USER_ID) + .await + .unwrap(); + assert_eq!(db_user.email, None); + // No `preferred_username` claim either, so one gets made up. + assert!(db_user.username.is_some()); + assert!(session_exists(&state, db_user.uuid()).await); +} + +/// The flip-side of the above: an email-based `UserIdentifier` cannot represent an emailless user, +/// so login must fail rather than create an account that no flow can address. +#[tokio::test] +async fn test_oidc_no_email_claim_is_rejected_for_email_identifier() { + let site_url = "https://bar.org"; + let (_server, state) = setup_fake_provider( + site_url, + FakeProviderOptions { + provider_id: OAuthProviderId::Oidc0, + provider_name: OIDC_PROVIDER_NAME.to_string(), + scopes: vec!["openid".to_string()], + user_identifier: Some(UserIdentifier::RequireEmail), + user_info: serde_json::json!({ "sub": EXTERNAL_USER_ID }), + }, + ) + .await; + + let (_auth_query, result) = run_login_flow(&state, OIDC_PROVIDER_NAME, site_url).await; + assert!( + matches!(result, Err(AuthError::BadRequest(_))), + "{:?}", + result.err() + ); + + assert!( + user_by_provider_user_id(&state, EXTERNAL_USER_ID) + .await + .is_none() + ); +} + +/// Drives the cookie-based login flow end-to-end, returning what the provider's auth endpoint saw +/// alongside the outcome of TrailBase's callback handler. +async fn run_login_flow( + state: &AppState, + provider_name: &str, + site_url: &str, +) -> (AuthQuery, Result) { + let cookies = Cookies::default(); + let external_redirect = login::login_with_external_auth_provider( + State(state.clone()), + Path(provider_name.to_string()), + Query(LoginInputParams { + redirect_uri: Some(format!("{site_url}/login-success-welcome")), + mfa_redirect_uri: None, + response_type: None, + pkce_code_challenge: None, + }), + cookies.clone(), + ) + .await + .unwrap(); + + let auth_query: AuthQuery = reqwest::get(&get_redirect_location(external_redirect).unwrap()) + .await + .unwrap() + .json() + .await + .unwrap(); + + let result = callback::callback_from_external_auth_provider( + State(state.clone()), + Path(provider_name.to_string()), + Query(callback::AuthQuery { + state: auth_query.state.clone(), + code: auth_query.code_challenge.clone(), + }), + cookies, + ) + .await; + + return (auth_query, result); +} + +async fn user_by_provider_user_id(state: &AppState, provider_user_id: &str) -> Option { + return state + .user_conn() + .read_query_value::( + format!("SELECT * FROM {USER_TABLE} WHERE provider_user_id = $1"), + (provider_user_id.to_string(),), + ) + .await + .unwrap(); +} + fn get_redirect_location(response: T) -> Option { return response .into_response() diff --git a/crates/core/src/auth/oauth/provider.rs b/crates/core/src/auth/oauth/provider.rs index a8332c5e4..9d5ce058c 100644 --- a/crates/core/src/auth/oauth/provider.rs +++ b/crates/core/src/auth/oauth/provider.rs @@ -46,7 +46,9 @@ pub struct OAuthUser { pub provider_user_id: String, pub provider_id: OAuthProviderId, - pub email: String, + /// Absent when the provider wasn't asked for, or doesn't expose, an email address. Requires a + /// username-based `UserIdentifier`, see `create_user_for_external_provider`. + pub email: Option, pub username: Option, pub verified: bool, @@ -113,7 +115,11 @@ pub trait OAuthProvider { return Ok(client); } - fn oauth_scopes(&self) -> Vec<&'static str>; + /// Scopes to request from the provider. + /// + /// NOTE: Tied to `&self`'s lifetime rather than `'static`, so providers can return scopes + /// that were read from the config. + fn oauth_scopes(&self) -> Vec<&str>; async fn get_token( &self, diff --git a/crates/core/src/auth/oauth/providers/apple.rs b/crates/core/src/auth/oauth/providers/apple.rs index d03598774..ef8698dbf 100644 --- a/crates/core/src/auth/oauth/providers/apple.rs +++ b/crates/core/src/auth/oauth/providers/apple.rs @@ -137,7 +137,7 @@ impl OAuthProvider for AppleOAuthProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["name", "email"]; } @@ -155,7 +155,7 @@ impl OAuthProvider for AppleOAuthProvider { return Ok(OAuthUser { provider_user_id: apple_id_token.sub, provider_id: OAuthProviderId::Apple, - email, + email: Some(email), username: None, verified: apple_id_token.email_verified.is_some_and(|v| v == "true"), avatar: None, diff --git a/crates/core/src/auth/oauth/providers/discord.rs b/crates/core/src/auth/oauth/providers/discord.rs index d316c5d20..b1d074c30 100644 --- a/crates/core/src/auth/oauth/providers/discord.rs +++ b/crates/core/src/auth/oauth/providers/discord.rs @@ -77,7 +77,7 @@ impl OAuthProvider for DiscordOAuthProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["identify", "email"]; } @@ -132,7 +132,7 @@ impl OAuthProvider for DiscordOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id, provider_id: OAuthProviderId::Discord, - email: user.email, + email: Some(user.email), username: user.username, verified: user.verified, avatar, diff --git a/crates/core/src/auth/oauth/providers/facebook.rs b/crates/core/src/auth/oauth/providers/facebook.rs index 730e5875f..09fc69b44 100644 --- a/crates/core/src/auth/oauth/providers/facebook.rs +++ b/crates/core/src/auth/oauth/providers/facebook.rs @@ -98,7 +98,7 @@ impl OAuthProvider for FacebookOAuthProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["email"]; } @@ -124,7 +124,7 @@ impl OAuthProvider for FacebookOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id, provider_id: OAuthProviderId::Facebook, - email: user.email, + email: Some(user.email), username: None, verified: true, avatar: user.picture.map(|p| p.data.url), diff --git a/crates/core/src/auth/oauth/providers/github.rs b/crates/core/src/auth/oauth/providers/github.rs index 6275778ce..6f5a4dcfa 100644 --- a/crates/core/src/auth/oauth/providers/github.rs +++ b/crates/core/src/auth/oauth/providers/github.rs @@ -78,7 +78,7 @@ impl OAuthProvider for GithubOAuthProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["read:user", "user:email"]; } @@ -157,7 +157,7 @@ impl OAuthProvider for GithubOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id.to_string(), provider_id: OAuthProviderId::Github, - email, + email: Some(email), username: user.login, verified: true, avatar: user.avatar_url, diff --git a/crates/core/src/auth/oauth/providers/gitlab.rs b/crates/core/src/auth/oauth/providers/gitlab.rs index ad81bc497..143ef279d 100644 --- a/crates/core/src/auth/oauth/providers/gitlab.rs +++ b/crates/core/src/auth/oauth/providers/gitlab.rs @@ -77,7 +77,7 @@ impl OAuthProvider for GitlabOAuthProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["read_user"]; } @@ -118,7 +118,7 @@ impl OAuthProvider for GitlabOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id.to_string(), provider_id: OAuthProviderId::Gitlab, - email: user.email, + email: Some(user.email), username: user.username, verified, avatar: user.avatar_url, diff --git a/crates/core/src/auth/oauth/providers/google.rs b/crates/core/src/auth/oauth/providers/google.rs index 66f59f9c9..fa313258c 100644 --- a/crates/core/src/auth/oauth/providers/google.rs +++ b/crates/core/src/auth/oauth/providers/google.rs @@ -77,7 +77,7 @@ impl OAuthProvider for GoogleOAuthProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec![ "https://www.googleapis.com/auth/userinfo.profile", "https://www.googleapis.com/auth/userinfo.email", @@ -118,7 +118,7 @@ impl OAuthProvider for GoogleOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id, provider_id: OAuthProviderId::Google, - email: user.email, + email: Some(user.email), username: None, verified: user.verified_email, avatar: user.picture, diff --git a/crates/core/src/auth/oauth/providers/microsoft.rs b/crates/core/src/auth/oauth/providers/microsoft.rs index 8a64d4715..c5b3b3078 100644 --- a/crates/core/src/auth/oauth/providers/microsoft.rs +++ b/crates/core/src/auth/oauth/providers/microsoft.rs @@ -87,7 +87,7 @@ impl OAuthProvider for MicrosoftOAuthProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["User.Read"]; } @@ -113,7 +113,7 @@ impl OAuthProvider for MicrosoftOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id, provider_id: OAuthProviderId::Microsoft, - email: user.mail, + email: Some(user.mail), // username: Some(user.displayName), username: None, verified: true, diff --git a/crates/core/src/auth/oauth/providers/oidc.rs b/crates/core/src/auth/oauth/providers/oidc.rs index 830285752..deba5be5d 100644 --- a/crates/core/src/auth/oauth/providers/oidc.rs +++ b/crates/core/src/auth/oauth/providers/oidc.rs @@ -19,9 +19,16 @@ pub struct OidcProvider { auth_url: String, token_url: String, user_api_url: String, + scopes: Vec, } impl OidcProvider { + /// Scopes requested when the config doesn't override them. + /// + /// NOTE: `openid` is mandated by the spec, `email` and `profile` merely back the claims we map + /// onto our user model below. Providers that don't offer the latter need `scopes` configured. + const DEFAULT_SCOPES: [&'static str; 3] = ["openid", "email", "profile"]; + pub fn factory(index: u64) -> OAuthProviderFactory { let (id, factory_name, factory_display_name) = match index { 0 => (OAuthProviderId::Oidc0, "oidc0", "OpenID Connect"), @@ -57,6 +64,7 @@ impl OidcProvider { auth_url, token_url, user_api_url, + scopes: config.scopes.clone(), })) }), } @@ -67,9 +75,11 @@ impl OidcProvider { #[derive(Default, Debug, Deserialize, Serialize)] pub struct OidcUser { pub sub: String, - pub email: String, + /// Requires the `email` scope. Absent for providers that don't offer it. + pub email: Option, pub email_verified: Option, + /// Requires the `profile` scope. pub preferred_username: Option, // pub name: Option, pub picture: Option, @@ -96,8 +106,11 @@ impl OAuthProvider for OidcProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { - return vec!["openid", "email", "profile"]; + fn oauth_scopes(&self) -> Vec<&str> { + if self.scopes.is_empty() { + return Self::DEFAULT_SCOPES.to_vec(); + } + return self.scopes.iter().map(String::as_str).collect(); } async fn get_user(&self, token_response: &TokenResponse) -> Result { diff --git a/crates/core/src/auth/oauth/providers/test.rs b/crates/core/src/auth/oauth/providers/test.rs index 0bfceb400..eb8a0ff76 100644 --- a/crates/core/src/auth/oauth/providers/test.rs +++ b/crates/core/src/auth/oauth/providers/test.rs @@ -68,7 +68,7 @@ impl OAuthProvider for TestOAuthProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["identity", "email", "preferences"]; } @@ -94,7 +94,7 @@ impl OAuthProvider for TestOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id, provider_id: OAuthProviderId::Test, - email: user.email, + email: Some(user.email), username: None, verified: user.verified, avatar: None, diff --git a/crates/core/src/auth/oauth/providers/twitch.rs b/crates/core/src/auth/oauth/providers/twitch.rs index 59fca9cb8..5c809d028 100644 --- a/crates/core/src/auth/oauth/providers/twitch.rs +++ b/crates/core/src/auth/oauth/providers/twitch.rs @@ -83,7 +83,7 @@ impl OAuthProvider for TwitchOAuthProvider { return oauth2::AuthType::RequestBody; } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["user:read:email"]; } @@ -153,7 +153,7 @@ impl OAuthProvider for TwitchOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id, provider_id: OAuthProviderId::Twitch, - email: user.email, + email: Some(user.email), username: user.login, verified: true, avatar: user.profile_image_url, diff --git a/crates/core/src/auth/oauth/providers/yandex.rs b/crates/core/src/auth/oauth/providers/yandex.rs index 05a724827..fe09431b6 100644 --- a/crates/core/src/auth/oauth/providers/yandex.rs +++ b/crates/core/src/auth/oauth/providers/yandex.rs @@ -79,7 +79,7 @@ impl OAuthProvider for YandexOAuthProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["login:email", "login:avatar", "login:info"]; } @@ -127,7 +127,7 @@ impl OAuthProvider for YandexOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id, provider_id: OAuthProviderId::Yandex, - email: user.default_email, + email: Some(user.default_email), username: user.login, verified: true, avatar, diff --git a/crates/core/src/config.rs b/crates/core/src/config.rs index 2b10955dd..1dae1969e 100644 --- a/crates/core/src/config.rs +++ b/crates/core/src/config.rs @@ -630,6 +630,22 @@ pub async fn validate_config( { return ierr(format!("Invalid user api url for '{name}")); } + + // Scopes are space-delimited on the wire, so an embedded space would silently turn into two + // scopes. + for scope in &provider.scopes { + if scope.is_empty() || scope.split_whitespace().count() != 1 { + return ierr(format!( + "OAuth provider {name}'s scopes must be non-empty and whitespace-free: '{scope}'" + )); + } + } + } else if !provider.scopes.is_empty() { + // The built-in providers parse a fixed set of claims out of their user-info response, so + // narrowing their scopes would only break them at login time. + return ierr(format!( + "Custom scopes are only supported for OIDC, not: {name}" + )); } } @@ -1011,6 +1027,72 @@ mod test { assert_eq!(config, merged); } + #[tokio::test] + async fn test_oauth_provider_scope_validation() { + let state = test_state(None).await.unwrap(); + + let config_with_scopes = |name: &str, provider_id: OAuthProviderId, scopes: &[&str]| { + let mut config = Config::new_with_custom_defaults(); + config.auth.oauth_providers = HashMap::from([( + name.to_string(), + OAuthProviderConfig { + client_id: Some("client_id".to_string()), + client_secret: Some("client_secret".to_string()), + provider_id: Some(provider_id as i32), + auth_url: Some("https://example.com/auth".to_string()), + token_url: Some("https://example.com/token".to_string()), + user_api_url: Some("https://example.com/user".to_string()), + scopes: scopes.iter().map(|s| s.to_string()).collect(), + ..Default::default() + }, + )]); + config + }; + + let validate = async |config: Config| { + validate_config(&state.connection_manager(), &config) + .await + .map(|_| ()) + }; + + const OIDC: OAuthProviderId = OAuthProviderId::Oidc0; + assert!( + validate(config_with_scopes("oidc0", OIDC, &[])) + .await + .is_ok() + ); + assert!( + validate(config_with_scopes("oidc0", OIDC, &["openid", "email"])) + .await + .is_ok() + ); + + // A scope containing whitespace would silently expand into multiple scopes on the wire. + assert!( + validate(config_with_scopes("oidc0", OIDC, &["openid email"])) + .await + .is_err() + ); + assert!( + validate(config_with_scopes("oidc0", OIDC, &[""])) + .await + .is_err() + ); + + // Built-in providers parse a fixed set of claims, so their scopes are not negotiable. + const GOOGLE: OAuthProviderId = OAuthProviderId::Google; + assert!( + validate(config_with_scopes("google", GOOGLE, &[])) + .await + .is_ok() + ); + assert!( + validate(config_with_scopes("google", GOOGLE, &["email"])) + .await + .is_err() + ); + } + #[test] fn test_is_valid_hostname_or_ip() { assert_eq!(false, is_valid_hostname_or_ip("")); From 1cc3f64794dbcdaffb188e7ea5cdca0412595273 Mon Sep 17 00:00:00 2001 From: Benjamin Sobel Date: Mon, 3 Aug 2026 03:14:25 -0700 Subject: [PATCH 4/7] cleanup --- crates/core/src/auth/oauth/provider.rs | 26 +- .../core/src/auth/oauth/providers/discord.rs | 166 +++++------ .../core/src/auth/oauth/providers/facebook.rs | 141 ++++------ .../core/src/auth/oauth/providers/github.rs | 233 ++++++++-------- .../core/src/auth/oauth/providers/gitlab.rs | 156 ++++------- .../core/src/auth/oauth/providers/google.rs | 160 ++++------- .../src/auth/oauth/providers/microsoft.rs | 127 +++------ crates/core/src/auth/oauth/providers/mod.rs | 21 +- .../core/src/auth/oauth/providers/social.rs | 260 ++++++++++++++++++ .../core/src/auth/oauth/providers/twitch.rs | 210 +++++--------- .../core/src/auth/oauth/providers/yandex.rs | 168 +++++------ 11 files changed, 810 insertions(+), 858 deletions(-) create mode 100644 crates/core/src/auth/oauth/providers/social.rs diff --git a/crates/core/src/auth/oauth/provider.rs b/crates/core/src/auth/oauth/provider.rs index 9d5ce058c..c5679a853 100644 --- a/crates/core/src/auth/oauth/provider.rs +++ b/crates/core/src/auth/oauth/provider.rs @@ -121,6 +121,12 @@ pub trait OAuthProvider { /// that were read from the config. fn oauth_scopes(&self) -> Vec<&str>; + /// Salvages a token response that failed to parse because the provider doesn't comply with + /// RFC-6749. Returning `None` propagates the original parse error. + fn recover_token_response(&self, _body: &[u8]) -> Option> { + return None; + } + async fn get_token( &self, state: &AppState, @@ -134,25 +140,29 @@ pub trait OAuthProvider { .map_err(|err| AuthError::Internal(err.into()))?; let client = self.oauth_client(state)?; - let token_response: TokenResponse = client + return client .exchange_code(AuthorizationCode::new(auth_code)) .set_pkce_verifier(PkceCodeVerifier::new(server_pkce_code_verifier)) .request_async(&ReqwestClient(http_client)) .await - .map_err(|err| { + .or_else(|err| { + if let oauth2::RequestTokenError::Parse(ref _path, ref body) = err + && let Some(recovered) = self.recover_token_response(body) + { + return recovered; + } + #[cfg(debug_assertions)] - return match err { + return Err(match err { oauth2::RequestTokenError::Parse(_path, resp) => { AuthError::Internal(String::from_utf8_lossy(&resp).into()) } err => AuthError::FailedDependency(format!("{err:?}").into()), - }; + }); #[cfg(not(debug_assertions))] - return AuthError::FailedDependency(err.into()); - })?; - - return Ok(token_response); + return Err(AuthError::FailedDependency(err.into())); + }); } async fn get_user(&self, token_response: &TokenResponse) -> Result; diff --git a/crates/core/src/auth/oauth/providers/discord.rs b/crates/core/src/auth/oauth/providers/discord.rs index b1d074c30..405763d0f 100644 --- a/crates/core/src/auth/oauth/providers/discord.rs +++ b/crates/core/src/auth/oauth/providers/discord.rs @@ -1,21 +1,28 @@ use async_trait::async_trait; -use lazy_static::lazy_static; -use oauth2::TokenResponse as _; use serde::Deserialize; -use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; -use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; -use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; -use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; - -pub(crate) struct DiscordOAuthProvider { - client_id: String, - client_secret: String, +use crate::auth::oauth::OAuthUser; +use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::config::proto::OAuthProviderId; + +pub(crate) struct Discord; + +// Checkout available fields on: https://discord.com/developers/docs/resources/user +#[derive(Default, Deserialize, Debug)] +pub(crate) struct DiscordUser { + id: String, + email: String, + verified: bool, + + // discriminator: Option, + username: Option, + avatar: Option, } -impl DiscordOAuthProvider { +#[async_trait] +impl SocialSpec for Discord { + const ID: OAuthProviderId = OAuthProviderId::Discord; const NAME: &'static str = "discord"; const DISPLAY_NAME: &'static str = "Discord"; @@ -23,96 +30,12 @@ impl DiscordOAuthProvider { const TOKEN_URL: &'static str = "https://discord.com/api/oauth2/token"; const USER_API_URL: &'static str = "https://discord.com/api/users/@me"; - fn new(config: &OAuthProviderConfig) -> Result { - let Some(client_id) = config.client_id.clone() else { - return Err(OAuthProviderError::Missing("Discord client id".to_string())); - }; - let Some(client_secret) = config.client_secret.clone() else { - return Err(OAuthProviderError::Missing( - "Discord client secret".to_string(), - )); - }; - - return Ok(Self { - client_id, - client_secret, - }); - } + const SCOPES: &'static [&'static str] = &["identify", "email"]; - pub fn factory() -> OAuthProviderFactory { - OAuthProviderFactory { - id: OAuthProviderId::Discord, - factory_name: Self::NAME, - factory_display_name: Self::DISPLAY_NAME, - factory: Box::new(|_name: &str, config: &OAuthProviderConfig| { - Ok(Box::new(Self::new(config)?)) - }), - } - } -} + type User = DiscordUser; -#[async_trait] -impl OAuthProvider for DiscordOAuthProvider { - fn name(&self) -> &'static str { - Self::NAME - } - fn provider(&self) -> OAuthProviderId { - OAuthProviderId::Discord - } - fn display_name(&self) -> &'static str { - Self::DISPLAY_NAME - } - - fn settings(&self) -> Result { - lazy_static! { - static ref AUTH_URL: Url = Url::parse(DiscordOAuthProvider::AUTH_URL).expect("infallible"); - static ref TOKEN_URL: Url = Url::parse(DiscordOAuthProvider::TOKEN_URL).expect("infallible"); - } - - return Ok(OAuthClientSettings { - auth_url: AUTH_URL.clone(), - token_url: TOKEN_URL.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); - } - - fn oauth_scopes(&self) -> Vec<&str> { - return vec!["identify", "email"]; - } - - async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - let response = reqwest::Client::new() - .get(Self::USER_API_URL) - .bearer_auth(token_response.access_token().secret()) - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - // Checkout available fields on: https://discord.com/developers/docs/resources/user - #[derive(Default, Deserialize, Debug)] - struct DiscordUser { - id: String, - email: String, - verified: bool, - - // discriminator: Option, - username: Option, - avatar: Option, - } - - let user = response - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - let verified = user.verified; - if !verified { + async fn map_user(_api: &UserApi<'_>, user: DiscordUser) -> Result { + if !user.verified { return Err(AuthError::Unauthorized); } @@ -131,7 +54,7 @@ impl OAuthProvider for DiscordOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id, - provider_id: OAuthProviderId::Discord, + provider_id: Self::ID, email: Some(user.email), username: user.username, verified: user.verified, @@ -139,3 +62,44 @@ impl OAuthProvider for DiscordOAuthProvider { }); } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::auth::oauth::providers::social::resolve_user; + + #[tokio::test] + async fn test_discord_user_mapping() { + let user = resolve_user::(serde_json::json!({ + "id": "80351110224678912", + "email": "user@example.com", + "verified": true, + "username": "nelly", + "avatar": "8342729096ea3675442027381ff50dfe", + })) + .await + .unwrap(); + + assert_eq!(user.email.as_deref(), Some("user@example.com")); + assert_eq!(user.username.as_deref(), Some("nelly")); + // Discord only hands out the avatar's hash, the CDN URL is ours to build. + assert_eq!( + user.avatar.as_deref(), + Some( + "https://cdn.discordapp.com/avatars/80351110224678912/8342729096ea3675442027381ff50dfe.png" + ) + ); + } + + #[tokio::test] + async fn test_discord_rejects_unverified_user() { + let result = resolve_user::(serde_json::json!({ + "id": "80351110224678912", + "email": "user@example.com", + "verified": false, + })) + .await; + + assert!(matches!(result, Err(AuthError::Unauthorized)), "{result:?}"); + } +} diff --git a/crates/core/src/auth/oauth/providers/facebook.rs b/crates/core/src/auth/oauth/providers/facebook.rs index 09fc69b44..51bcf25ca 100644 --- a/crates/core/src/auth/oauth/providers/facebook.rs +++ b/crates/core/src/auth/oauth/providers/facebook.rs @@ -1,14 +1,12 @@ use async_trait::async_trait; -use lazy_static::lazy_static; -use oauth2::TokenResponse as _; use serde::Deserialize; -use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; -use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; -use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; -use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; +use crate::auth::oauth::OAuthUser; +use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::config::proto::OAuthProviderId; + +pub(crate) struct Facebook; #[derive(Default, Deserialize, Debug)] struct FacebookUserPictureData { @@ -21,19 +19,16 @@ struct FacebookUserPicture { } #[derive(Default, Deserialize, Debug)] -struct FacebookUser { +pub(crate) struct FacebookUser { id: String, email: String, // name: Option, picture: Option, } -pub(crate) struct FacebookOAuthProvider { - client_id: String, - client_secret: String, -} - -impl FacebookOAuthProvider { +#[async_trait] +impl SocialSpec for Facebook { + const ID: OAuthProviderId = OAuthProviderId::Facebook; const NAME: &'static str = "facebook"; const DISPLAY_NAME: &'static str = "Facebook"; @@ -42,88 +37,14 @@ impl FacebookOAuthProvider { const USER_API_URL: &'static str = "https://graph.facebook.com/me?fields=name,email,picture.type(large)"; - fn new(config: &OAuthProviderConfig) -> Result { - let Some(client_id) = config.client_id.clone() else { - return Err(OAuthProviderError::Missing( - "Facebook client id".to_string(), - )); - }; - let Some(client_secret) = config.client_secret.clone() else { - return Err(OAuthProviderError::Missing( - "Facebook client secret".to_string(), - )); - }; - - return Ok(Self { - client_id, - client_secret, - }); - } - - pub fn factory() -> OAuthProviderFactory { - OAuthProviderFactory { - id: OAuthProviderId::Facebook, - factory_name: Self::NAME, - factory_display_name: Self::DISPLAY_NAME, - factory: Box::new(|_name: &str, config: &OAuthProviderConfig| { - Ok(Box::new(Self::new(config)?)) - }), - } - } -} - -#[async_trait] -impl OAuthProvider for FacebookOAuthProvider { - fn name(&self) -> &'static str { - Self::NAME - } - fn provider(&self) -> OAuthProviderId { - OAuthProviderId::Facebook - } - fn display_name(&self) -> &'static str { - Self::DISPLAY_NAME - } + const SCOPES: &'static [&'static str] = &["email"]; - fn settings(&self) -> Result { - lazy_static! { - static ref AUTH_URL: Url = Url::parse(FacebookOAuthProvider::AUTH_URL).expect("infallible"); - static ref TOKEN_URL: Url = Url::parse(FacebookOAuthProvider::TOKEN_URL).expect("infallible"); - } - - return Ok(OAuthClientSettings { - auth_url: AUTH_URL.clone(), - token_url: TOKEN_URL.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); - } - - fn oauth_scopes(&self) -> Vec<&str> { - return vec!["email"]; - } - - async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - let response = reqwest::Client::new() - .get(Self::USER_API_URL) - .bearer_auth(token_response.access_token().secret()) - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - let user = response - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; + type User = FacebookUser; + async fn map_user(_api: &UserApi<'_>, user: FacebookUser) -> Result { return Ok(OAuthUser { provider_user_id: user.id, - provider_id: OAuthProviderId::Facebook, + provider_id: Self::ID, email: Some(user.email), username: None, verified: true, @@ -131,3 +52,39 @@ impl OAuthProvider for FacebookOAuthProvider { }); } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::auth::oauth::providers::social::resolve_user; + + #[tokio::test] + async fn test_facebook_user_mapping() { + let user = resolve_user::(serde_json::json!({ + "id": "1234", + "email": "user@example.com", + "picture": { "data": { "url": "https://scontent.xx.fbcdn.net/avatar" } }, + })) + .await + .unwrap(); + + assert_eq!(user.email.as_deref(), Some("user@example.com")); + // The avatar is nested two levels deep in Facebook's response. + assert_eq!( + user.avatar.as_deref(), + Some("https://scontent.xx.fbcdn.net/avatar") + ); + } + + #[tokio::test] + async fn test_facebook_user_without_picture() { + let user = resolve_user::(serde_json::json!({ + "id": "1234", + "email": "user@example.com", + })) + .await + .unwrap(); + + assert_eq!(user.avatar, None); + } +} diff --git a/crates/core/src/auth/oauth/providers/github.rs b/crates/core/src/auth/oauth/providers/github.rs index 6f5a4dcfa..da9508eeb 100644 --- a/crates/core/src/auth/oauth/providers/github.rs +++ b/crates/core/src/auth/oauth/providers/github.rs @@ -1,21 +1,36 @@ use async_trait::async_trait; -use lazy_static::lazy_static; -use oauth2::TokenResponse as _; use serde::Deserialize; -use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; -use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; -use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; -use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; - -pub(crate) struct GithubOAuthProvider { - client_id: String, - client_secret: String, +use crate::auth::oauth::OAuthUser; +use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::config::proto::OAuthProviderId; + +pub(crate) struct Github; + +// Checkout available fields on: https://docs.github.com/en/rest/users/users?apiVersion=2026-03-10 +#[derive(Default, Deserialize, Debug)] +pub(crate) struct GithubUser { + id: i64, + login: Option, + // name: String, + email: Option, + // verified: bool, + avatar_url: Option, } -impl GithubOAuthProvider { +#[derive(Default, Deserialize, Debug)] +struct GithubEmail { + email: String, + primary: bool, + verified: bool, + // NOTE: null | "private" | "public" + // visibility: Option, +} + +#[async_trait] +impl SocialSpec for Github { + const ID: OAuthProviderId = OAuthProviderId::Github; const NAME: &'static str = "github"; const DISPLAY_NAME: &'static str = "Github"; @@ -24,96 +39,16 @@ impl GithubOAuthProvider { // const DEVICE_AUTH_URL: &'static str = "https://github.com/login/device/code"; const USER_API_URL: &'static str = "https://api.github.com/user"; - fn new(config: &OAuthProviderConfig) -> Result { - let Some(client_id) = config.client_id.clone() else { - return Err(OAuthProviderError::Missing("Github client id".to_string())); - }; - let Some(client_secret) = config.client_secret.clone() else { - return Err(OAuthProviderError::Missing( - "Github client secret".to_string(), - )); - }; + const SCOPES: &'static [&'static str] = &["read:user", "user:email"]; - return Ok(Self { - client_id, - client_secret, - }); - } - - pub fn factory() -> OAuthProviderFactory { - OAuthProviderFactory { - id: OAuthProviderId::Github, - factory_name: Self::NAME, - factory_display_name: Self::DISPLAY_NAME, - factory: Box::new(|_name: &str, config: &OAuthProviderConfig| { - Ok(Box::new(Self::new(config)?)) - }), - } - } -} - -#[async_trait] -impl OAuthProvider for GithubOAuthProvider { - fn name(&self) -> &'static str { - Self::NAME - } - fn provider(&self) -> OAuthProviderId { - OAuthProviderId::Github - } - fn display_name(&self) -> &'static str { - Self::DISPLAY_NAME - } - - fn settings(&self) -> Result { - lazy_static! { - static ref AUTH_URL: Url = Url::parse(GithubOAuthProvider::AUTH_URL).expect("infallible"); - static ref TOKEN_URL: Url = Url::parse(GithubOAuthProvider::TOKEN_URL).expect("infallible"); - } - - return Ok(OAuthClientSettings { - auth_url: AUTH_URL.clone(), - token_url: TOKEN_URL.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); - } + type User = GithubUser; - fn oauth_scopes(&self) -> Vec<&str> { - return vec!["read:user", "user:email"]; + fn user_api_headers(_client_id: &str) -> Vec<(&'static str, String)> { + // Github rejects requests without a user agent. + return vec![("User-Agent", "TrailBase".to_string())]; } - async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - // Checkout available fields on: https://docs.github.com/en/rest/users/users?apiVersion=2026-03-10 - #[derive(Default, Deserialize, Debug)] - struct GithubUser { - id: i64, - login: Option, - // name: String, - email: Option, - // verified: bool, - avatar_url: Option, - } - - let client = reqwest::Client::new(); - let response = client - .get(Self::USER_API_URL) - .bearer_auth(token_response.access_token().secret()) - .header(axum::http::header::USER_AGENT, "TrailBase") - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - let user = response - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - + async fn map_user(api: &UserApi<'_>, user: GithubUser) -> Result { // Users can set the "Keep my email private" option, in which case the user api will return an // empty email and we'll have to call the dedicated `/emails` endpoint. let email = if let Some(email) = user.email @@ -121,28 +56,9 @@ impl OAuthProvider for GithubOAuthProvider { { email } else { - #[allow(non_snake_case)] - #[derive(Default, Deserialize, Debug)] - struct GithubEmail { - email: String, - primary: bool, - verified: bool, - // NOTE: null | "private" | "public" - // visibility: Option, - } - - let email_response = client - .get(format!("{}/emails", Self::USER_API_URL)) - .bearer_auth(token_response.access_token().secret()) - .header(axum::http::header::USER_AGENT, "TrailBase") - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - let emails: Vec = email_response - .json() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; + let emails: Vec = api + .get_json(&format!("{}/emails", api.user_api_url())) + .await?; let Some(primary) = emails .into_iter() @@ -156,7 +72,7 @@ impl OAuthProvider for GithubOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id.to_string(), - provider_id: OAuthProviderId::Github, + provider_id: Self::ID, email: Some(email), username: user.login, verified: true, @@ -164,3 +80,78 @@ impl OAuthProvider for GithubOAuthProvider { }); } } + +#[cfg(test)] +mod tests { + use axum::Json; + use axum::routing::{Router, get}; + + use super::*; + use crate::auth::oauth::providers::social::{ + USER_API_TEST_PATH, resolve_user, resolve_user_against, + }; + + #[tokio::test] + async fn test_github_user_mapping() { + let user = resolve_user::(serde_json::json!({ + "id": 1234, + "login": "octocat", + "email": "octocat@github.com", + "avatar_url": "https://github.com/images/octocat.gif", + })) + .await + .unwrap(); + + // Github ids are numeric, ours are strings. + assert_eq!(user.provider_user_id, "1234"); + assert_eq!(user.email.as_deref(), Some("octocat@github.com")); + assert_eq!(user.username.as_deref(), Some("octocat")); + } + + /// Users with "Keep my email private" force a second call to `/emails`, out of which only the + /// verified primary address may be used. + #[tokio::test] + async fn test_github_falls_back_to_email_endpoint() { + for private_email in [serde_json::Value::Null, "".into()] { + let user = resolve_user_against::(github_routes( + serde_json::json!({ + "id": 1234, + "login": "octocat", + "email": private_email, + }), + serde_json::json!([ + { "email": "unverified@github.com", "primary": true, "verified": false }, + { "email": "secondary@github.com", "primary": false, "verified": true }, + { "email": "primary@github.com", "primary": true, "verified": true }, + ]), + )) + .await + .unwrap(); + + assert_eq!(user.email.as_deref(), Some("primary@github.com")); + } + } + + #[tokio::test] + async fn test_github_without_any_usable_email() { + let result = resolve_user_against::(github_routes( + serde_json::json!({ "id": 1234, "login": "octocat" }), + serde_json::json!([{ "email": "a@github.com", "primary": true, "verified": false }]), + )) + .await; + + assert!( + matches!(result, Err(AuthError::FailedDependency(_))), + "{result:?}" + ); + } + + fn github_routes(user: serde_json::Value, emails: serde_json::Value) -> Router { + return Router::new() + .route(USER_API_TEST_PATH, get(|| async move { Json(user) })) + .route( + &format!("{USER_API_TEST_PATH}/emails"), + get(|| async move { Json(emails) }), + ); + } +} diff --git a/crates/core/src/auth/oauth/providers/gitlab.rs b/crates/core/src/auth/oauth/providers/gitlab.rs index 143ef279d..e81b6c490 100644 --- a/crates/core/src/auth/oauth/providers/gitlab.rs +++ b/crates/core/src/auth/oauth/providers/gitlab.rs @@ -1,21 +1,27 @@ use async_trait::async_trait; -use lazy_static::lazy_static; -use oauth2::TokenResponse as _; use serde::Deserialize; -use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; -use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; -use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; -use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; - -pub(crate) struct GitlabOAuthProvider { - client_id: String, - client_secret: String, +use crate::auth::oauth::OAuthUser; +use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::config::proto::OAuthProviderId; + +pub(crate) struct Gitlab; + +// https://docs.gitlab.com/ee/api/users.html#for-user +#[derive(Default, Deserialize, Debug)] +pub(crate) struct GitlabUser { + id: i64, + // name: String, + username: Option, + email: String, + avatar_url: Option, + state: String, } -impl GitlabOAuthProvider { +#[async_trait] +impl SocialSpec for Gitlab { + const ID: OAuthProviderId = OAuthProviderId::Gitlab; const NAME: &'static str = "gitlab"; const DISPLAY_NAME: &'static str = "GitLab"; @@ -23,93 +29,11 @@ impl GitlabOAuthProvider { const TOKEN_URL: &'static str = "https://gitlab.com/oauth/token"; const USER_API_URL: &'static str = "https://gitlab.com/api/v4/user"; - fn new(config: &OAuthProviderConfig) -> Result { - let Some(client_id) = config.client_id.clone() else { - return Err(OAuthProviderError::Missing("GitLab client id".to_string())); - }; - let Some(client_secret) = config.client_secret.clone() else { - return Err(OAuthProviderError::Missing( - "GitLab client secret".to_string(), - )); - }; - - return Ok(Self { - client_id, - client_secret, - }); - } - - pub fn factory() -> OAuthProviderFactory { - OAuthProviderFactory { - id: OAuthProviderId::Gitlab, - factory_name: Self::NAME, - factory_display_name: Self::DISPLAY_NAME, - factory: Box::new(|_name: &str, config: &OAuthProviderConfig| { - Ok(Box::new(Self::new(config)?)) - }), - } - } -} - -#[async_trait] -impl OAuthProvider for GitlabOAuthProvider { - fn name(&self) -> &'static str { - Self::NAME - } - fn provider(&self) -> OAuthProviderId { - OAuthProviderId::Gitlab - } - fn display_name(&self) -> &'static str { - Self::DISPLAY_NAME - } + const SCOPES: &'static [&'static str] = &["read_user"]; - fn settings(&self) -> Result { - lazy_static! { - static ref AUTH_URL: Url = Url::parse(GitlabOAuthProvider::AUTH_URL).expect("infallible"); - static ref TOKEN_URL: Url = Url::parse(GitlabOAuthProvider::TOKEN_URL).expect("infallible"); - } + type User = GitlabUser; - return Ok(OAuthClientSettings { - auth_url: AUTH_URL.clone(), - token_url: TOKEN_URL.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); - } - - fn oauth_scopes(&self) -> Vec<&str> { - return vec!["read_user"]; - } - - async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - let response = reqwest::Client::new() - .get(Self::USER_API_URL) - .bearer_auth(token_response.access_token().secret()) - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - // https://docs.gitlab.com/ee/api/users.html#for-user - #[derive(Default, Deserialize, Debug)] - struct GitlabUser { - id: i64, - // name: String, - username: Option, - email: String, - avatar_url: Option, - state: String, - } - - let user = response - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; + async fn map_user(_api: &UserApi<'_>, user: GitlabUser) -> Result { let verified = user.state == "active"; if !verified { return Err(AuthError::Unauthorized); @@ -117,7 +41,7 @@ impl OAuthProvider for GitlabOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id.to_string(), - provider_id: OAuthProviderId::Gitlab, + provider_id: Self::ID, email: Some(user.email), username: user.username, verified, @@ -125,3 +49,39 @@ impl OAuthProvider for GitlabOAuthProvider { }); } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::auth::oauth::providers::social::resolve_user; + + #[tokio::test] + async fn test_gitlab_user_mapping() { + let user = resolve_user::(serde_json::json!({ + "id": 42, + "username": "john_smith", + "email": "john@example.com", + "avatar_url": "https://gitlab.com/uploads/user/avatar/42/index.jpg", + "state": "active", + })) + .await + .unwrap(); + + // GitLab ids are numeric, ours are strings. + assert_eq!(user.provider_user_id, "42"); + assert_eq!(user.username.as_deref(), Some("john_smith")); + assert!(user.verified); + } + + #[tokio::test] + async fn test_gitlab_rejects_inactive_user() { + let result = resolve_user::(serde_json::json!({ + "id": 42, + "email": "john@example.com", + "state": "blocked", + })) + .await; + + assert!(matches!(result, Err(AuthError::Unauthorized)), "{result:?}"); + } +} diff --git a/crates/core/src/auth/oauth/providers/google.rs b/crates/core/src/auth/oauth/providers/google.rs index fa313258c..40b1fb145 100644 --- a/crates/core/src/auth/oauth/providers/google.rs +++ b/crates/core/src/auth/oauth/providers/google.rs @@ -1,21 +1,25 @@ use async_trait::async_trait; -use lazy_static::lazy_static; -use oauth2::TokenResponse as _; use serde::Deserialize; -use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; -use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; -use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; -use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; - -pub(crate) struct GoogleOAuthProvider { - client_id: String, - client_secret: String, +use crate::auth::oauth::OAuthUser; +use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::config::proto::OAuthProviderId; + +pub(crate) struct Google; + +#[derive(Default, Deserialize, Debug)] +pub(crate) struct GoogleUser { + id: String, + // name: Option, + email: String, + verified_email: bool, + picture: Option, } -impl GoogleOAuthProvider { +#[async_trait] +impl SocialSpec for Google { + const ID: OAuthProviderId = OAuthProviderId::Google; const NAME: &'static str = "google"; const DISPLAY_NAME: &'static str = "Google"; @@ -23,101 +27,21 @@ impl GoogleOAuthProvider { const TOKEN_URL: &'static str = "https://accounts.google.com/o/oauth2/token"; const USER_API_URL: &'static str = "https://www.googleapis.com/oauth2/v1/userinfo"; - fn new(config: &OAuthProviderConfig) -> Result { - let Some(client_id) = config.client_id.clone() else { - return Err(OAuthProviderError::Missing("Google client id".to_string())); - }; - let Some(client_secret) = config.client_secret.clone() else { - return Err(OAuthProviderError::Missing( - "Google client secret".to_string(), - )); - }; - - return Ok(Self { - client_id, - client_secret, - }); - } - - pub fn factory() -> OAuthProviderFactory { - OAuthProviderFactory { - id: OAuthProviderId::Google, - factory_name: Self::NAME, - factory_display_name: Self::DISPLAY_NAME, - factory: Box::new(|_name: &str, config: &OAuthProviderConfig| { - Ok(Box::new(Self::new(config)?)) - }), - } - } -} - -#[async_trait] -impl OAuthProvider for GoogleOAuthProvider { - fn name(&self) -> &'static str { - Self::NAME - } - fn provider(&self) -> OAuthProviderId { - OAuthProviderId::Google - } - fn display_name(&self) -> &'static str { - Self::DISPLAY_NAME - } + const SCOPES: &'static [&'static str] = &[ + "https://www.googleapis.com/auth/userinfo.profile", + "https://www.googleapis.com/auth/userinfo.email", + ]; - fn settings(&self) -> Result { - lazy_static! { - static ref AUTH_URL: Url = Url::parse(GoogleOAuthProvider::AUTH_URL).expect("infallible"); - static ref TOKEN_URL: Url = Url::parse(GoogleOAuthProvider::TOKEN_URL).expect("infallible"); - } + type User = GoogleUser; - return Ok(OAuthClientSettings { - auth_url: AUTH_URL.clone(), - token_url: TOKEN_URL.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); - } - - fn oauth_scopes(&self) -> Vec<&str> { - return vec![ - "https://www.googleapis.com/auth/userinfo.profile", - "https://www.googleapis.com/auth/userinfo.email", - ]; - } - - async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - let response = reqwest::Client::new() - .get(Self::USER_API_URL) - .bearer_auth(token_response.access_token().secret()) - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - #[derive(Default, Deserialize, Debug)] - struct GoogleUser { - id: String, - // name: Option, - email: String, - verified_email: bool, - picture: Option, - } - - let user = response - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; + async fn map_user(_api: &UserApi<'_>, user: GoogleUser) -> Result { if !user.verified_email { return Err(AuthError::Unauthorized); } return Ok(OAuthUser { provider_user_id: user.id, - provider_id: OAuthProviderId::Google, + provider_id: Self::ID, email: Some(user.email), username: None, verified: user.verified_email, @@ -125,3 +49,41 @@ impl OAuthProvider for GoogleOAuthProvider { }); } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::auth::oauth::providers::social::resolve_user; + + #[tokio::test] + async fn test_google_user_mapping() { + let user = resolve_user::(serde_json::json!({ + "id": "1234", + "email": "user@gmail.com", + "verified_email": true, + "picture": "https://lh3.googleusercontent.com/a/avatar", + })) + .await + .unwrap(); + + assert_eq!(user.provider_user_id, "1234"); + assert_eq!(user.email.as_deref(), Some("user@gmail.com")); + assert!(user.verified); + assert_eq!( + user.avatar.as_deref(), + Some("https://lh3.googleusercontent.com/a/avatar") + ); + } + + #[tokio::test] + async fn test_google_rejects_unverified_email() { + let result = resolve_user::(serde_json::json!({ + "id": "1234", + "email": "user@gmail.com", + "verified_email": false, + })) + .await; + + assert!(matches!(result, Err(AuthError::Unauthorized)), "{result:?}"); + } +} diff --git a/crates/core/src/auth/oauth/providers/microsoft.rs b/crates/core/src/auth/oauth/providers/microsoft.rs index c5b3b3078..c330e2057 100644 --- a/crates/core/src/auth/oauth/providers/microsoft.rs +++ b/crates/core/src/auth/oauth/providers/microsoft.rs @@ -1,28 +1,23 @@ use async_trait::async_trait; -use lazy_static::lazy_static; -use oauth2::TokenResponse as _; use serde::Deserialize; -use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; -use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; -use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; -use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; +use crate::auth::oauth::OAuthUser; +use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::config::proto::OAuthProviderId; + +pub(crate) struct Microsoft; #[derive(Default, Deserialize, Debug)] -struct MicrosoftUser { +pub(crate) struct MicrosoftUser { id: String, mail: String, // displayName: String, } -pub(crate) struct MicrosoftOAuthProvider { - client_id: String, - client_secret: String, -} - -impl MicrosoftOAuthProvider { +#[async_trait] +impl SocialSpec for Microsoft { + const ID: OAuthProviderId = OAuthProviderId::Microsoft; const NAME: &'static str = "microsoft"; const DISPLAY_NAME: &'static str = "Microsoft"; @@ -30,89 +25,14 @@ impl MicrosoftOAuthProvider { const TOKEN_URL: &'static str = "https://login.microsoftonline.com/common/oauth2/v2.0/token"; const USER_API_URL: &'static str = "https://graph.microsoft.com/v1.0/me"; - fn new(config: &OAuthProviderConfig) -> Result { - let Some(client_id) = config.client_id.clone() else { - return Err(OAuthProviderError::Missing( - "Microsoft client id".to_string(), - )); - }; - let Some(client_secret) = config.client_secret.clone() else { - return Err(OAuthProviderError::Missing( - "Microsoft client secret".to_string(), - )); - }; - - return Ok(Self { - client_id, - client_secret, - }); - } - - pub fn factory() -> OAuthProviderFactory { - OAuthProviderFactory { - id: OAuthProviderId::Microsoft, - factory_name: Self::NAME, - factory_display_name: Self::DISPLAY_NAME, - factory: Box::new(|_name: &str, config: &OAuthProviderConfig| { - Ok(Box::new(Self::new(config)?)) - }), - } - } -} - -#[async_trait] -impl OAuthProvider for MicrosoftOAuthProvider { - fn name(&self) -> &'static str { - Self::NAME - } - fn provider(&self) -> OAuthProviderId { - OAuthProviderId::Microsoft - } - fn display_name(&self) -> &'static str { - Self::DISPLAY_NAME - } - - fn settings(&self) -> Result { - lazy_static! { - static ref AUTH_URL: Url = Url::parse(MicrosoftOAuthProvider::AUTH_URL).expect("infallible"); - static ref TOKEN_URL: Url = - Url::parse(MicrosoftOAuthProvider::TOKEN_URL).expect("infallible"); - } + const SCOPES: &'static [&'static str] = &["User.Read"]; - return Ok(OAuthClientSettings { - auth_url: AUTH_URL.clone(), - token_url: TOKEN_URL.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); - } - - fn oauth_scopes(&self) -> Vec<&str> { - return vec!["User.Read"]; - } - - async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - let response = reqwest::Client::new() - .get(Self::USER_API_URL) - .bearer_auth(token_response.access_token().secret()) - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - let user = response - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; + type User = MicrosoftUser; + async fn map_user(_api: &UserApi<'_>, user: MicrosoftUser) -> Result { return Ok(OAuthUser { provider_user_id: user.id, - provider_id: OAuthProviderId::Microsoft, + provider_id: Self::ID, email: Some(user.mail), // username: Some(user.displayName), username: None, @@ -121,3 +41,24 @@ impl OAuthProvider for MicrosoftOAuthProvider { }); } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::auth::oauth::providers::social::resolve_user; + + #[tokio::test] + async fn test_microsoft_user_mapping() { + let user = resolve_user::(serde_json::json!({ + "id": "1234", + "mail": "user@contoso.com", + "displayName": "User", + })) + .await + .unwrap(); + + assert_eq!(user.provider_user_id, "1234"); + assert_eq!(user.email.as_deref(), Some("user@contoso.com")); + assert!(user.verified); + } +} diff --git a/crates/core/src/auth/oauth/providers/mod.rs b/crates/core/src/auth/oauth/providers/mod.rs index ff8454db6..2cb7007ec 100644 --- a/crates/core/src/auth/oauth/providers/mod.rs +++ b/crates/core/src/auth/oauth/providers/mod.rs @@ -6,6 +6,7 @@ mod gitlab; mod google; mod microsoft; mod oidc; +mod social; mod twitch; mod yandex; @@ -16,6 +17,7 @@ use std::sync::LazyLock; use thiserror::Error; use crate::auth::oauth::OAuthProvider; +use crate::auth::oauth::providers::social::SocialSpec as _; use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; #[derive(Debug, Error)] @@ -44,15 +46,18 @@ pub(crate) fn oauth_providers_static_registry() -> &'static [OAuthProviderFactor // NOTE: In the future we might want to have more than one OIDC factory. oidc::OidcProvider::factory(0), // "Social" OAuth providers. + // + // NOTE: All but Apple, which reads its claims off a JWT rather than a user API, are + // declared as a `social::SocialSpec`. apple::AppleOAuthProvider::factory(), - discord::DiscordOAuthProvider::factory(), - gitlab::GitlabOAuthProvider::factory(), - github::GithubOAuthProvider::factory(), - google::GoogleOAuthProvider::factory(), - facebook::FacebookOAuthProvider::factory(), - microsoft::MicrosoftOAuthProvider::factory(), - twitch::TwitchOAuthProvider::factory(), - yandex::YandexOAuthProvider::factory(), + discord::Discord::factory(), + gitlab::Gitlab::factory(), + github::Github::factory(), + google::Google::factory(), + facebook::Facebook::factory(), + microsoft::Microsoft::factory(), + twitch::Twitch::factory(), + yandex::Yandex::factory(), ] }); diff --git a/crates/core/src/auth/oauth/providers/social.rs b/crates/core/src/auth/oauth/providers/social.rs new file mode 100644 index 000000000..53790771e --- /dev/null +++ b/crates/core/src/auth/oauth/providers/social.rs @@ -0,0 +1,260 @@ +use async_trait::async_trait; +use oauth2::{AuthType, TokenResponse as _}; +use serde::de::DeserializeOwned; +use std::marker::PhantomData; +use url::Url; + +use crate::auth::AuthError; +use crate::auth::oauth::provider::TokenResponse; +use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; +use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; +use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; + +/// Declarative description of an OAuth provider that identifies users by calling a REST user-info +/// endpoint with the access token. +/// +/// The plumbing shared by all of them - reading client credentials off the config, parsing the +/// endpoint URLs, exchanging the auth code, issuing the authenticated user-info request - lives in +/// [`SocialProvider`]. Implementors are left with the provider's metadata, the shape of its +/// user-info response and how that maps onto our user model. +/// +/// Providers that don't fit, i.e. Apple (claims come from a JWT rather than an API) and OIDC (URLs +/// and scopes come from the config), implement [`OAuthProvider`] directly. +#[async_trait] +pub(crate) trait SocialSpec: Send + Sync + 'static { + const ID: OAuthProviderId; + /// Config key and URL path segment, therefore also the name users authenticate against. + const NAME: &'static str; + const DISPLAY_NAME: &'static str; + + const AUTH_URL: &'static str; + const TOKEN_URL: &'static str; + const USER_API_URL: &'static str; + + const SCOPES: &'static [&'static str]; + + /// How client credentials are passed to the token endpoint. + const AUTH_TYPE: AuthType = AuthType::BasicAuth; + + /// The provider's user-info response. + type User: DeserializeOwned + Send; + + /// Headers the user-info request needs on top of the bearer token. + fn user_api_headers(_client_id: &str) -> Vec<(&'static str, String)> { + return vec![]; + } + + /// Salvages a token response that doesn't comply with RFC-6749. `None` propagates the original + /// parse error. + fn recover_token_response(_body: &[u8]) -> Option> { + return None; + } + + /// Maps the provider's user onto our user model. + /// + /// `api` is only needed by the few providers that have to make follow-up calls, e.g. Github's + /// separate email endpoint. + async fn map_user(api: &UserApi<'_>, user: Self::User) -> Result; + + fn factory() -> OAuthProviderFactory + where + Self: Sized, + { + return SocialProvider::::factory(); + } +} + +/// Authenticated client for a provider's user-info API. +pub(crate) struct UserApi<'a> { + client: reqwest::Client, + access_token: &'a str, + user_api_url: &'a str, + headers: Vec<(&'static str, String)>, +} + +impl UserApi<'_> { + /// The provider's user-info endpoint, i.e. [`SocialSpec::USER_API_URL`] outside of tests. + /// + /// Providers making follow-up calls must derive them from this rather than the constant, so + /// tests can redirect them too. + pub(crate) fn user_api_url(&self) -> &str { + return self.user_api_url; + } + + pub(crate) async fn get_json(&self, url: &str) -> Result { + let mut request = self.client.get(url).bearer_auth(self.access_token); + for (name, value) in &self.headers { + request = request.header(*name, value); + } + + return request + .send() + .await + .map_err(|err| AuthError::FailedDependency(err.into()))? + .json::() + .await + .map_err(|err| AuthError::FailedDependency(err.into())); + } +} + +pub(crate) struct SocialProvider { + client_id: String, + client_secret: String, + + // NOTE: Held rather than derived from `S` on every call, both to parse the URLs only once and so + // tests can point a provider at a fake server. + auth_url: Url, + token_url: Url, + user_api_url: String, + + spec: PhantomData, +} + +impl SocialProvider { + fn new(config: &OAuthProviderConfig) -> Result { + let Some(client_id) = config.client_id.clone() else { + return Err(OAuthProviderError::Missing(format!( + "{} client id", + S::DISPLAY_NAME + ))); + }; + let Some(client_secret) = config.client_secret.clone() else { + return Err(OAuthProviderError::Missing(format!( + "{} client secret", + S::DISPLAY_NAME + ))); + }; + + return Ok(Self { + client_id, + client_secret, + // NOTE: Infallible, the URLs are compile-time constants. + auth_url: Url::parse(S::AUTH_URL).expect("infallible"), + token_url: Url::parse(S::TOKEN_URL).expect("infallible"), + user_api_url: S::USER_API_URL.to_string(), + spec: PhantomData, + }); + } + + pub fn factory() -> OAuthProviderFactory { + return OAuthProviderFactory { + id: S::ID, + factory_name: S::NAME, + factory_display_name: S::DISPLAY_NAME, + factory: Box::new(|_name: &str, config: &OAuthProviderConfig| { + Ok(Box::new(Self::new(config)?)) + }), + }; + } +} + +#[async_trait] +impl OAuthProvider for SocialProvider { + fn name(&self) -> &'static str { + return S::NAME; + } + fn provider(&self) -> OAuthProviderId { + return S::ID; + } + fn display_name(&self) -> &'static str { + return S::DISPLAY_NAME; + } + fn auth_type(&self) -> AuthType { + return S::AUTH_TYPE; + } + fn oauth_scopes(&self) -> Vec<&str> { + return S::SCOPES.to_vec(); + } + + fn settings(&self) -> Result { + return Ok(OAuthClientSettings { + auth_url: self.auth_url.clone(), + token_url: self.token_url.clone(), + client_id: self.client_id.clone(), + client_secret: self.client_secret.clone(), + }); + } + + fn recover_token_response(&self, body: &[u8]) -> Option> { + return S::recover_token_response(body); + } + + async fn get_user(&self, token_response: &TokenResponse) -> Result { + if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { + return Err(AuthError::Internal( + format!("Unexpected token type: {:?}", token_response.token_type()).into(), + )); + } + + let api = UserApi { + client: reqwest::Client::new(), + access_token: token_response.access_token().secret(), + user_api_url: &self.user_api_url, + headers: S::user_api_headers(&self.client_id), + }; + + let user = api.get_json::(api.user_api_url()).await?; + + return S::map_user(&api, user).await; + } +} + +/// Test helpers letting each spec be exercised against a fake provider. +/// +/// This is what the indirection buys beyond deduplication: because the user-info endpoint is a +/// field rather than a constant, a spec's request, response parsing and user mapping can be tested +/// without reaching out to the real provider. +#[cfg(test)] +mod testing { + use axum::Json; + use axum::routing::{Router, get}; + use axum_test::{TestServer, TestServerConfig}; + + use super::*; + use crate::auth::oauth::provider::ExtraTokenFields; + + /// Path the fake user-info endpoint is served under. + pub(crate) const USER_API_TEST_PATH: &str = "/user"; + + /// Runs everything `get_user` does, with `routes` standing in for the provider. + pub(crate) async fn resolve_user_against( + routes: Router, + ) -> Result { + let server = TestServer::new_with_config( + routes, + TestServerConfig { + transport: Some(axum_test::Transport::HttpRandomPort), + ..Default::default() + }, + ); + + let provider = SocialProvider:: { + client_id: "client_id".to_string(), + client_secret: "client_secret".to_string(), + auth_url: Url::parse(S::AUTH_URL).expect("infallible"), + token_url: Url::parse(S::TOKEN_URL).expect("infallible"), + user_api_url: server.server_url(USER_API_TEST_PATH).unwrap().to_string(), + spec: PhantomData, + }; + + return provider + .get_user(&TokenResponse::new( + oauth2::AccessToken::new("access_token".to_string()), + oauth2::basic::BasicTokenType::Bearer, + ExtraTokenFields { id_token: None }, + )) + .await; + } + + /// [`resolve_user_against`] for the common case of a single, static user-info payload. + pub(crate) async fn resolve_user( + user_info: serde_json::Value, + ) -> Result { + let routes = Router::new().route(USER_API_TEST_PATH, get(|| async move { Json(user_info) })); + + return resolve_user_against::(routes).await; + } +} + +#[cfg(test)] +pub(crate) use testing::{USER_API_TEST_PATH, resolve_user, resolve_user_against}; diff --git a/crates/core/src/auth/oauth/providers/twitch.rs b/crates/core/src/auth/oauth/providers/twitch.rs index 5c809d028..a5e983097 100644 --- a/crates/core/src/auth/oauth/providers/twitch.rs +++ b/crates/core/src/auth/oauth/providers/twitch.rs @@ -1,23 +1,33 @@ use async_trait::async_trait; -use lazy_static::lazy_static; -use oauth2::{AuthorizationCode, PkceCodeVerifier, TokenResponse as _}; use serde::{Deserialize, Serialize}; -use url::Url; -use crate::AppState; use crate::auth::AuthError; -use crate::auth::oauth::ReqwestClient; +use crate::auth::oauth::OAuthUser; use crate::auth::oauth::provider::TokenResponse; -use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; -use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; -use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; +use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::config::proto::OAuthProviderId; -pub(crate) struct TwitchOAuthProvider { - client_id: String, - client_secret: String, +pub(crate) struct Twitch; + +// Reference: https://dev.twitch.tv/docs/api/reference#get-users +#[derive(Default, Deserialize, Debug)] +struct TwitchUser { + id: String, + // According to reference above, email is implicitly verified. + email: String, + login: Option, + // display_name: String, + profile_image_url: Option, +} + +#[derive(Deserialize, Debug)] +pub(crate) struct TwitchUsersResponse { + data: Vec, } -impl TwitchOAuthProvider { +#[async_trait] +impl SocialSpec for Twitch { + const ID: OAuthProviderId = OAuthProviderId::Twitch; const NAME: &'static str = "twitch"; const DISPLAY_NAME: &'static str = "Twitch"; @@ -25,119 +35,28 @@ impl TwitchOAuthProvider { const TOKEN_URL: &'static str = "https://id.twitch.tv/oauth2/token"; const USER_API_URL: &'static str = "https://api.twitch.tv/helix/users"; - fn new(config: &OAuthProviderConfig) -> Result { - let Some(client_id) = config.client_id.clone() else { - return Err(OAuthProviderError::Missing("Twitch client id".to_string())); - }; - let Some(client_secret) = config.client_secret.clone() else { - return Err(OAuthProviderError::Missing( - "Twitch client secret".to_string(), - )); - }; - - return Ok(Self { - client_id, - client_secret, - }); - } - - pub fn factory() -> OAuthProviderFactory { - OAuthProviderFactory { - id: OAuthProviderId::Twitch, - factory_name: Self::NAME, - factory_display_name: Self::DISPLAY_NAME, - factory: Box::new(|_name: &str, config: &OAuthProviderConfig| { - Ok(Box::new(Self::new(config)?)) - }), - } - } -} - -#[async_trait] -impl OAuthProvider for TwitchOAuthProvider { - fn name(&self) -> &'static str { - Self::NAME - } - fn provider(&self) -> OAuthProviderId { - OAuthProviderId::Twitch - } - fn display_name(&self) -> &'static str { - Self::DISPLAY_NAME - } - - fn settings(&self) -> Result { - lazy_static! { - static ref AUTH_URL: Url = Url::parse(TwitchOAuthProvider::AUTH_URL).expect("infallible"); - static ref TOKEN_URL: Url = Url::parse(TwitchOAuthProvider::TOKEN_URL).expect("infallible"); - } + const SCOPES: &'static [&'static str] = &["user:read:email"]; - return Ok(OAuthClientSettings { - auth_url: AUTH_URL.clone(), - token_url: TOKEN_URL.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); - } + const AUTH_TYPE: oauth2::AuthType = oauth2::AuthType::RequestBody; - fn auth_type(&self) -> oauth2::AuthType { - return oauth2::AuthType::RequestBody; - } + type User = TwitchUsersResponse; - fn oauth_scopes(&self) -> Vec<&str> { - return vec!["user:read:email"]; + fn user_api_headers(client_id: &str) -> Vec<(&'static str, String)> { + return vec![("Client-Id", client_id.to_string())]; } - async fn get_token( - &self, - state: &AppState, - auth_code: String, - server_pkce_code_verifier: String, - ) -> Result { - let http_client = reqwest::ClientBuilder::new() - // Following redirects might set us up for server-side request forgery (SSRF). - .redirect(reqwest::redirect::Policy::none()) - .build() - .map_err(|err| AuthError::Internal(err.into()))?; - - let client = self.oauth_client(state)?; - let token_response: TokenResponse = client - .exchange_code(AuthorizationCode::new(auth_code)) - .set_pkce_verifier(PkceCodeVerifier::new(server_pkce_code_verifier)) - .request_async(&ReqwestClient(http_client)) - .await - .or_else(|err| match err { - // Twitch returns non-RFC-6749 compliant body: scopes are an array rather than space - // delimited list. - oauth2::RequestTokenError::Parse(_path, resp) => parse_twitch_token_response(&resp), - err => Err(AuthError::FailedDependency(err.into())), - })?; - - return Ok(token_response); + fn recover_token_response(body: &[u8]) -> Option> { + // Twitch returns non-RFC-6749 compliant body: scopes are an array rather than space delimited + // list. + return Some(parse_twitch_token_response(body)); } - async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - let response = reqwest::Client::new() - .get(Self::USER_API_URL) - .header("Client-Id", &self.client_id) - .bearer_auth(token_response.access_token().secret()) - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - let mut users = response - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))? - .data; - - let user = match users.len() { - 1 => users.swap_remove(0), + async fn map_user( + _api: &UserApi<'_>, + mut response: TwitchUsersResponse, + ) -> Result { + let user = match response.data.len() { + 1 => response.data.swap_remove(0), 0 => { return Err(AuthError::FailedDependency( "Twitch user response had empty data".into(), @@ -152,7 +71,7 @@ impl OAuthProvider for TwitchOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id, - provider_id: OAuthProviderId::Twitch, + provider_id: Self::ID, email: Some(user.email), username: user.login, verified: true, @@ -191,22 +110,6 @@ fn parse_twitch_token_response(body: &[u8]) -> Result .map_err(|_err| AuthError::Internal("Failed to deserialize".into())); } -// Reference: https://dev.twitch.tv/docs/api/reference#get-users -#[derive(Default, Deserialize, Debug)] -struct TwitchUser { - id: String, - // According to reference above, email is implicitly verified. - email: String, - login: Option, - // display_name: String, - profile_image_url: Option, -} - -#[derive(Deserialize, Debug)] -struct TwitchUsersResponse { - data: Vec, -} - pub fn serialize_space_delimited_vec( vec_opt: &Option>, serializer: S, @@ -226,6 +129,7 @@ where #[cfg(test)] mod tests { use super::*; + use crate::auth::oauth::providers::social::resolve_user; #[test] fn parse_twitch_token_response_test() { @@ -239,4 +143,40 @@ mod tests { parse_twitch_token_response(response.as_bytes()).unwrap(); } + + /// Twitch wraps the user in a `data` array rather than returning it directly. + #[tokio::test] + async fn test_twitch_user_mapping() { + let user = resolve_user::(serde_json::json!({ + "data": [{ + "id": "141981764", + "email": "twitchdev@example.com", + "login": "twitchdev", + "profile_image_url": "https://static-cdn.jtvnw.net/avatar.png", + }], + })) + .await + .unwrap(); + + assert_eq!(user.provider_user_id, "141981764"); + assert_eq!(user.email.as_deref(), Some("twitchdev@example.com")); + assert_eq!(user.username.as_deref(), Some("twitchdev")); + } + + #[tokio::test] + async fn test_twitch_rejects_ambiguous_user_response() { + for data in [ + serde_json::json!([]), + serde_json::json!([ + { "id": "1", "email": "a@example.com" }, + { "id": "2", "email": "b@example.com" }, + ]), + ] { + let result = resolve_user::(serde_json::json!({ "data": data })).await; + assert!( + matches!(result, Err(AuthError::FailedDependency(_))), + "{result:?}" + ); + } + } } diff --git a/crates/core/src/auth/oauth/providers/yandex.rs b/crates/core/src/auth/oauth/providers/yandex.rs index fe09431b6..bc6b1be3b 100644 --- a/crates/core/src/auth/oauth/providers/yandex.rs +++ b/crates/core/src/auth/oauth/providers/yandex.rs @@ -1,21 +1,29 @@ use async_trait::async_trait; -use lazy_static::lazy_static; -use oauth2::TokenResponse as _; use serde::Deserialize; -use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; -use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; -use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; -use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; - -pub(crate) struct YandexOAuthProvider { - client_id: String, - client_secret: String, +use crate::auth::oauth::OAuthUser; +use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::config::proto::OAuthProviderId; + +pub(crate) struct Yandex; + +// Checkout available fields on: +// * https://yandex.com/dev/id/doc/en/user-information +// * https://authjs.dev/reference/core/providers/yandex. +#[derive(Default, Deserialize, Debug)] +pub(crate) struct YandexUser { + id: String, + // real_name: String, + login: Option, + default_email: String, + is_avatar_empty: bool, + default_avatar_id: String, } -impl YandexOAuthProvider { +#[async_trait] +impl SocialSpec for Yandex { + const ID: OAuthProviderId = OAuthProviderId::Yandex; const NAME: &'static str = "yandex"; const DISPLAY_NAME: &'static str = "Yandex"; @@ -23,98 +31,11 @@ impl YandexOAuthProvider { const TOKEN_URL: &'static str = "https://oauth.yandex.com/token"; const USER_API_URL: &'static str = "https://login.yandex.ru/info"; - fn new(config: &OAuthProviderConfig) -> Result { - let Some(client_id) = config.client_id.clone() else { - return Err(OAuthProviderError::Missing("Yandex client id".to_string())); - }; - let Some(client_secret) = config.client_secret.clone() else { - return Err(OAuthProviderError::Missing( - "Yandex client secret".to_string(), - )); - }; - - return Ok(Self { - client_id, - client_secret, - }); - } - - pub fn factory() -> OAuthProviderFactory { - OAuthProviderFactory { - id: OAuthProviderId::Yandex, - factory_name: Self::NAME, - factory_display_name: Self::DISPLAY_NAME, - factory: Box::new(|_name: &str, config: &OAuthProviderConfig| { - Ok(Box::new(Self::new(config)?)) - }), - } - } -} - -#[async_trait] -impl OAuthProvider for YandexOAuthProvider { - fn name(&self) -> &'static str { - return Self::NAME; - } - - fn provider(&self) -> OAuthProviderId { - return OAuthProviderId::Yandex; - } + const SCOPES: &'static [&'static str] = &["login:email", "login:avatar", "login:info"]; - fn display_name(&self) -> &'static str { - return Self::DISPLAY_NAME; - } - - fn settings(&self) -> Result { - lazy_static! { - static ref AUTH_URL: Url = Url::parse(YandexOAuthProvider::AUTH_URL).expect("infallible"); - static ref TOKEN_URL: Url = Url::parse(YandexOAuthProvider::TOKEN_URL).expect("infallible"); - } - - return Ok(OAuthClientSettings { - auth_url: AUTH_URL.clone(), - token_url: TOKEN_URL.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); - } - - fn oauth_scopes(&self) -> Vec<&str> { - return vec!["login:email", "login:avatar", "login:info"]; - } - - async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - let response = reqwest::Client::new() - .get(Self::USER_API_URL) - .bearer_auth(token_response.access_token().secret()) - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - // Checkout available fields on: - // * https://yandex.com/dev/id/doc/en/user-information - // * https://authjs.dev/reference/core/providers/yandex. - #[derive(Default, Deserialize, Debug)] - struct YandexUser { - id: String, - // real_name: String, - login: Option, - default_email: String, - is_avatar_empty: bool, - default_avatar_id: String, - } - - let user = response - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; + type User = YandexUser; + async fn map_user(_api: &UserApi<'_>, user: YandexUser) -> Result { let avatar = if !user.is_avatar_empty { Some(format!( "https://avatars.yandex.net/get-yapic/{}/islands-200", @@ -126,7 +47,7 @@ impl OAuthProvider for YandexOAuthProvider { return Ok(OAuthUser { provider_user_id: user.id, - provider_id: OAuthProviderId::Yandex, + provider_id: Self::ID, email: Some(user.default_email), username: user.login, verified: true, @@ -134,3 +55,44 @@ impl OAuthProvider for YandexOAuthProvider { }); } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::auth::oauth::providers::social::resolve_user; + + #[tokio::test] + async fn test_yandex_user_mapping() { + let user = resolve_user::(serde_json::json!({ + "id": "1234", + "login": "ivan", + "default_email": "ivan@yandex.ru", + "is_avatar_empty": false, + "default_avatar_id": "abcdef", + })) + .await + .unwrap(); + + assert_eq!(user.email.as_deref(), Some("ivan@yandex.ru")); + assert_eq!(user.username.as_deref(), Some("ivan")); + assert_eq!( + user.avatar.as_deref(), + Some("https://avatars.yandex.net/get-yapic/abcdef/islands-200") + ); + } + + #[tokio::test] + async fn test_yandex_user_without_avatar() { + let user = resolve_user::(serde_json::json!({ + "id": "1234", + "default_email": "ivan@yandex.ru", + // Yandex sends a placeholder id alongside this flag, which must not become a URL. + "is_avatar_empty": true, + "default_avatar_id": "0/0-0", + })) + .await + .unwrap(); + + assert_eq!(user.avatar, None); + } +} From 3a74d6a3b1c6ff677bbd5122ccb1d6195a014d2c Mon Sep 17 00:00:00 2001 From: Benjamin Sobel Date: Mon, 3 Aug 2026 04:30:22 -0700 Subject: [PATCH 5/7] external user cleanup --- .../core/src/auth/oauth/providers/discord.rs | 26 ++++------ .../core/src/auth/oauth/providers/facebook.rs | 19 +++---- .../core/src/auth/oauth/providers/github.rs | 8 ++- .../core/src/auth/oauth/providers/gitlab.rs | 17 +++---- .../core/src/auth/oauth/providers/google.rs | 14 ++---- .../src/auth/oauth/providers/microsoft.rs | 11 ++-- .../core/src/auth/oauth/providers/social.rs | 50 ++++++++++++++++--- .../core/src/auth/oauth/providers/twitch.rs | 19 +++---- .../core/src/auth/oauth/providers/yandex.rs | 25 ++++------ 9 files changed, 92 insertions(+), 97 deletions(-) diff --git a/crates/core/src/auth/oauth/providers/discord.rs b/crates/core/src/auth/oauth/providers/discord.rs index 405763d0f..030c35d04 100644 --- a/crates/core/src/auth/oauth/providers/discord.rs +++ b/crates/core/src/auth/oauth/providers/discord.rs @@ -2,8 +2,7 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::OAuthUser; -use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; use crate::config::proto::OAuthProviderId; pub(crate) struct Discord; @@ -34,31 +33,26 @@ impl SocialSpec for Discord { type User = DiscordUser; - async fn map_user(_api: &UserApi<'_>, user: DiscordUser) -> Result { - if !user.verified { - return Err(AuthError::Unauthorized); - } - + async fn map_user(_api: &UserApi<'_>, user: DiscordUser) -> Result { // let username = match (user.discriminator, user.username) { // (Some(discriminator), Some(username)) => Some(format!("{username}#{discriminator}")), // (None, Some(username)) => Some(username.to_string()), // (Some(discriminator), None) => Some(discriminator.to_string()), // (None, None) => None, // }; - let avatar = user.avatar.map(|avatar| { - format!( - "https://cdn.discordapp.com/avatars/{id}/{avatar}.png", - id = user.id - ) - }); - return Ok(OAuthUser { + return Ok(ExternalUser { + // Discord only hands out the avatar's hash, the CDN URL is ours to build. + avatar: user.avatar.map(|avatar| { + format!( + "https://cdn.discordapp.com/avatars/{id}/{avatar}.png", + id = user.id + ) + }), provider_user_id: user.id, - provider_id: Self::ID, email: Some(user.email), username: user.username, verified: user.verified, - avatar, }); } } diff --git a/crates/core/src/auth/oauth/providers/facebook.rs b/crates/core/src/auth/oauth/providers/facebook.rs index 51bcf25ca..25014a47a 100644 --- a/crates/core/src/auth/oauth/providers/facebook.rs +++ b/crates/core/src/auth/oauth/providers/facebook.rs @@ -2,28 +2,22 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::OAuthUser; -use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::auth::oauth::providers::social::{DataEnvelope, ExternalUser, SocialSpec, UserApi}; use crate::config::proto::OAuthProviderId; pub(crate) struct Facebook; #[derive(Default, Deserialize, Debug)] -struct FacebookUserPictureData { +struct FacebookPicture { url: String, } -#[derive(Default, Deserialize, Debug)] -struct FacebookUserPicture { - data: FacebookUserPictureData, -} - #[derive(Default, Deserialize, Debug)] pub(crate) struct FacebookUser { id: String, email: String, // name: Option, - picture: Option, + picture: Option>, } #[async_trait] @@ -41,14 +35,13 @@ impl SocialSpec for Facebook { type User = FacebookUser; - async fn map_user(_api: &UserApi<'_>, user: FacebookUser) -> Result { - return Ok(OAuthUser { + async fn map_user(_api: &UserApi<'_>, user: FacebookUser) -> Result { + return Ok(ExternalUser { provider_user_id: user.id, - provider_id: Self::ID, email: Some(user.email), - username: None, verified: true, avatar: user.picture.map(|p| p.data.url), + ..Default::default() }); } } diff --git a/crates/core/src/auth/oauth/providers/github.rs b/crates/core/src/auth/oauth/providers/github.rs index da9508eeb..ddb0edf55 100644 --- a/crates/core/src/auth/oauth/providers/github.rs +++ b/crates/core/src/auth/oauth/providers/github.rs @@ -2,8 +2,7 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::OAuthUser; -use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; use crate::config::proto::OAuthProviderId; pub(crate) struct Github; @@ -48,7 +47,7 @@ impl SocialSpec for Github { return vec![("User-Agent", "TrailBase".to_string())]; } - async fn map_user(api: &UserApi<'_>, user: GithubUser) -> Result { + async fn map_user(api: &UserApi<'_>, user: GithubUser) -> Result { // Users can set the "Keep my email private" option, in which case the user api will return an // empty email and we'll have to call the dedicated `/emails` endpoint. let email = if let Some(email) = user.email @@ -70,9 +69,8 @@ impl SocialSpec for Github { primary.email }; - return Ok(OAuthUser { + return Ok(ExternalUser { provider_user_id: user.id.to_string(), - provider_id: Self::ID, email: Some(email), username: user.login, verified: true, diff --git a/crates/core/src/auth/oauth/providers/gitlab.rs b/crates/core/src/auth/oauth/providers/gitlab.rs index e81b6c490..132c77be9 100644 --- a/crates/core/src/auth/oauth/providers/gitlab.rs +++ b/crates/core/src/auth/oauth/providers/gitlab.rs @@ -2,8 +2,7 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::OAuthUser; -use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; use crate::config::proto::OAuthProviderId; pub(crate) struct Gitlab; @@ -33,18 +32,14 @@ impl SocialSpec for Gitlab { type User = GitlabUser; - async fn map_user(_api: &UserApi<'_>, user: GitlabUser) -> Result { - let verified = user.state == "active"; - if !verified { - return Err(AuthError::Unauthorized); - } - - return Ok(OAuthUser { + async fn map_user(_api: &UserApi<'_>, user: GitlabUser) -> Result { + return Ok(ExternalUser { provider_user_id: user.id.to_string(), - provider_id: Self::ID, email: Some(user.email), username: user.username, - verified, + // GitLab has no email-confirmation flag, but blocked and deactivated accounts must not + // be able to log in. + verified: user.state == "active", avatar: user.avatar_url, }); } diff --git a/crates/core/src/auth/oauth/providers/google.rs b/crates/core/src/auth/oauth/providers/google.rs index 40b1fb145..bc93887a2 100644 --- a/crates/core/src/auth/oauth/providers/google.rs +++ b/crates/core/src/auth/oauth/providers/google.rs @@ -2,8 +2,7 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::OAuthUser; -use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; use crate::config::proto::OAuthProviderId; pub(crate) struct Google; @@ -34,18 +33,13 @@ impl SocialSpec for Google { type User = GoogleUser; - async fn map_user(_api: &UserApi<'_>, user: GoogleUser) -> Result { - if !user.verified_email { - return Err(AuthError::Unauthorized); - } - - return Ok(OAuthUser { + async fn map_user(_api: &UserApi<'_>, user: GoogleUser) -> Result { + return Ok(ExternalUser { provider_user_id: user.id, - provider_id: Self::ID, email: Some(user.email), - username: None, verified: user.verified_email, avatar: user.picture, + ..Default::default() }); } } diff --git a/crates/core/src/auth/oauth/providers/microsoft.rs b/crates/core/src/auth/oauth/providers/microsoft.rs index c330e2057..8eccf04f4 100644 --- a/crates/core/src/auth/oauth/providers/microsoft.rs +++ b/crates/core/src/auth/oauth/providers/microsoft.rs @@ -2,8 +2,7 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::OAuthUser; -use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; use crate::config::proto::OAuthProviderId; pub(crate) struct Microsoft; @@ -29,15 +28,13 @@ impl SocialSpec for Microsoft { type User = MicrosoftUser; - async fn map_user(_api: &UserApi<'_>, user: MicrosoftUser) -> Result { - return Ok(OAuthUser { + async fn map_user(_api: &UserApi<'_>, user: MicrosoftUser) -> Result { + return Ok(ExternalUser { provider_user_id: user.id, - provider_id: Self::ID, email: Some(user.mail), // username: Some(user.displayName), - username: None, verified: true, - avatar: None, + ..Default::default() }); } } diff --git a/crates/core/src/auth/oauth/providers/social.rs b/crates/core/src/auth/oauth/providers/social.rs index 53790771e..0dc7b0f4e 100644 --- a/crates/core/src/auth/oauth/providers/social.rs +++ b/crates/core/src/auth/oauth/providers/social.rs @@ -1,5 +1,6 @@ use async_trait::async_trait; use oauth2::{AuthType, TokenResponse as _}; +use serde::Deserialize; use serde::de::DeserializeOwned; use std::marker::PhantomData; use url::Url; @@ -50,11 +51,11 @@ pub(crate) trait SocialSpec: Send + Sync + 'static { return None; } - /// Maps the provider's user onto our user model. + /// Maps the provider's user onto [`ExternalUser`]. /// /// `api` is only needed by the few providers that have to make follow-up calls, e.g. Github's /// separate email endpoint. - async fn map_user(api: &UserApi<'_>, user: Self::User) -> Result; + async fn map_user(api: &UserApi<'_>, user: Self::User) -> Result; fn factory() -> OAuthProviderFactory where @@ -64,6 +65,28 @@ pub(crate) trait SocialSpec: Send + Sync + 'static { } } +/// What a [`SocialSpec`] pulls out of its provider's user-info response. +/// +/// Narrower than [`OAuthUser`] on purpose: [`SocialProvider`] fills in the provider id, so a spec +/// can't accidentally claim to be a different provider, and it turns an unverified user into +/// [`AuthError::Unauthorized`], so no spec has to remember that check. Anything a provider doesn't +/// expose is left at its default. +#[derive(Default, Debug)] +pub(crate) struct ExternalUser { + pub provider_user_id: String, + pub email: Option, + pub username: Option, + /// Whether the provider vouches for the account, e.g. confirmed the email address. + pub verified: bool, + pub avatar: Option, +} + +/// Payload wrapper for the providers that nest their responses under a `data` key. +#[derive(Debug, Deserialize)] +pub(crate) struct DataEnvelope { + pub data: T, +} + /// Authenticated client for a provider's user-info API. pub(crate) struct UserApi<'a> { client: reqwest::Client, @@ -193,9 +216,22 @@ impl OAuthProvider for SocialProvider { headers: S::user_api_headers(&self.client_id), }; - let user = api.get_json::(api.user_api_url()).await?; + let user = S::map_user(&api, api.get_json::(api.user_api_url()).await?).await?; + + // Central so that no spec can forget it: whatever signal a provider offers, `map_user` folds + // it into `verified` and an account the provider won't vouch for never becomes a local user. + if !user.verified { + return Err(AuthError::Unauthorized); + } - return S::map_user(&api, user).await; + return Ok(OAuthUser { + provider_user_id: user.provider_user_id, + provider_id: S::ID, + email: user.email, + username: user.username, + verified: user.verified, + avatar: user.avatar, + }); } } @@ -204,6 +240,9 @@ impl OAuthProvider for SocialProvider { /// This is what the indirection buys beyond deduplication: because the user-info endpoint is a /// field rather than a constant, a spec's request, response parsing and user mapping can be tested /// without reaching out to the real provider. +#[cfg(test)] +pub(crate) use testing::{USER_API_TEST_PATH, resolve_user, resolve_user_against}; + #[cfg(test)] mod testing { use axum::Json; @@ -255,6 +294,3 @@ mod testing { return resolve_user_against::(routes).await; } } - -#[cfg(test)] -pub(crate) use testing::{USER_API_TEST_PATH, resolve_user, resolve_user_against}; diff --git a/crates/core/src/auth/oauth/providers/twitch.rs b/crates/core/src/auth/oauth/providers/twitch.rs index a5e983097..f4394a6ef 100644 --- a/crates/core/src/auth/oauth/providers/twitch.rs +++ b/crates/core/src/auth/oauth/providers/twitch.rs @@ -2,16 +2,15 @@ use async_trait::async_trait; use serde::{Deserialize, Serialize}; use crate::auth::AuthError; -use crate::auth::oauth::OAuthUser; use crate::auth::oauth::provider::TokenResponse; -use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::auth::oauth::providers::social::{DataEnvelope, ExternalUser, SocialSpec, UserApi}; use crate::config::proto::OAuthProviderId; pub(crate) struct Twitch; // Reference: https://dev.twitch.tv/docs/api/reference#get-users #[derive(Default, Deserialize, Debug)] -struct TwitchUser { +pub(crate) struct TwitchUser { id: String, // According to reference above, email is implicitly verified. email: String, @@ -20,11 +19,6 @@ struct TwitchUser { profile_image_url: Option, } -#[derive(Deserialize, Debug)] -pub(crate) struct TwitchUsersResponse { - data: Vec, -} - #[async_trait] impl SocialSpec for Twitch { const ID: OAuthProviderId = OAuthProviderId::Twitch; @@ -39,7 +33,7 @@ impl SocialSpec for Twitch { const AUTH_TYPE: oauth2::AuthType = oauth2::AuthType::RequestBody; - type User = TwitchUsersResponse; + type User = DataEnvelope>; fn user_api_headers(client_id: &str) -> Vec<(&'static str, String)> { return vec![("Client-Id", client_id.to_string())]; @@ -53,8 +47,8 @@ impl SocialSpec for Twitch { async fn map_user( _api: &UserApi<'_>, - mut response: TwitchUsersResponse, - ) -> Result { + mut response: DataEnvelope>, + ) -> Result { let user = match response.data.len() { 1 => response.data.swap_remove(0), 0 => { @@ -69,9 +63,8 @@ impl SocialSpec for Twitch { } }; - return Ok(OAuthUser { + return Ok(ExternalUser { provider_user_id: user.id, - provider_id: Self::ID, email: Some(user.email), username: user.login, verified: true, diff --git a/crates/core/src/auth/oauth/providers/yandex.rs b/crates/core/src/auth/oauth/providers/yandex.rs index bc6b1be3b..4834da53e 100644 --- a/crates/core/src/auth/oauth/providers/yandex.rs +++ b/crates/core/src/auth/oauth/providers/yandex.rs @@ -2,8 +2,7 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::OAuthUser; -use crate::auth::oauth::providers::social::{SocialSpec, UserApi}; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; use crate::config::proto::OAuthProviderId; pub(crate) struct Yandex; @@ -35,23 +34,19 @@ impl SocialSpec for Yandex { type User = YandexUser; - async fn map_user(_api: &UserApi<'_>, user: YandexUser) -> Result { - let avatar = if !user.is_avatar_empty { - Some(format!( - "https://avatars.yandex.net/get-yapic/{}/islands-200", - user.default_avatar_id - )) - } else { - None - }; - - return Ok(OAuthUser { + async fn map_user(_api: &UserApi<'_>, user: YandexUser) -> Result { + return Ok(ExternalUser { provider_user_id: user.id, - provider_id: Self::ID, email: Some(user.default_email), username: user.login, verified: true, - avatar, + // NOTE: Yandex sends a placeholder id alongside the flag, so the flag decides. + avatar: (!user.is_avatar_empty).then(|| { + format!( + "https://avatars.yandex.net/get-yapic/{}/islands-200", + user.default_avatar_id + ) + }), }); } } From 741dad0740ea3a765633131168bedc523c4e9e2f Mon Sep 17 00:00:00 2001 From: Benjamin Sobel Date: Mon, 3 Aug 2026 07:10:41 -0700 Subject: [PATCH 6/7] Further refactor --- crates/core/src/auth/oauth/mod.rs | 3 +- crates/core/src/auth/oauth/providers/apple.rs | 45 ++----- .../core/src/auth/oauth/providers/client.rs | 124 ++++++++++++++++++ .../core/src/auth/oauth/providers/discord.rs | 3 +- .../core/src/auth/oauth/providers/facebook.rs | 3 +- .../core/src/auth/oauth/providers/github.rs | 5 +- .../core/src/auth/oauth/providers/gitlab.rs | 3 +- .../core/src/auth/oauth/providers/google.rs | 3 +- .../{provider.rs => providers/interface.rs} | 5 +- .../src/auth/oauth/providers/microsoft.rs | 3 +- crates/core/src/auth/oauth/providers/mod.rs | 17 ++- crates/core/src/auth/oauth/providers/oidc.rs | 26 +--- .../core/src/auth/oauth/providers/social.rs | 111 ++++------------ crates/core/src/auth/oauth/providers/test.rs | 34 ++--- .../core/src/auth/oauth/providers/twitch.rs | 5 +- .../core/src/auth/oauth/providers/yandex.rs | 3 +- 16 files changed, 206 insertions(+), 187 deletions(-) create mode 100644 crates/core/src/auth/oauth/providers/client.rs rename crates/core/src/auth/oauth/{provider.rs => providers/interface.rs} (97%) diff --git a/crates/core/src/auth/oauth/mod.rs b/crates/core/src/auth/oauth/mod.rs index 8a70baa6c..00a5a36ea 100644 --- a/crates/core/src/auth/oauth/mod.rs +++ b/crates/core/src/auth/oauth/mod.rs @@ -1,4 +1,3 @@ -pub(crate) mod provider; pub(crate) mod providers; mod callback; @@ -14,7 +13,7 @@ use axum::Router; use axum::routing::get; use utoipa::OpenApi; -pub(crate) use provider::{OAuthClientSettings, OAuthProvider, OAuthUser}; +pub(crate) use providers::interface::{OAuthClientSettings, OAuthProvider, OAuthUser}; pub(crate) use reqwest_client::ReqwestClient; use crate::AppState; diff --git a/crates/core/src/auth/oauth/providers/apple.rs b/crates/core/src/auth/oauth/providers/apple.rs index ef8698dbf..a32b31b27 100644 --- a/crates/core/src/auth/oauth/providers/apple.rs +++ b/crates/core/src/auth/oauth/providers/apple.rs @@ -1,17 +1,15 @@ use async_trait::async_trait; -use lazy_static::lazy_static; use serde::Deserialize; -use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; +use crate::auth::oauth::providers::client::ProviderClient; +use crate::auth::oauth::providers::interface::TokenResponse; use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; pub(crate) struct AppleOAuthProvider { - client_id: String, - client_secret: String, + client: ProviderClient, } #[allow(unused)] @@ -54,18 +52,8 @@ impl AppleOAuthProvider { const TOKEN_URL: &str = "https://appleid.apple.com/auth/token"; fn new(config: &OAuthProviderConfig) -> Result { - let Some(client_id) = config.client_id.clone() else { - return Err(OAuthProviderError::Missing("Apple client id".to_string())); - }; - let Some(client_secret) = config.client_secret.clone() else { - return Err(OAuthProviderError::Missing( - "Apple client secret".to_string(), - )); - }; - return Ok(Self { - client_id, - client_secret, + client: ProviderClient::new(config, Self::DISPLAY_NAME, Self::AUTH_URL, Self::TOKEN_URL)?, }); } @@ -101,7 +89,7 @@ impl AppleOAuthProvider { .map_err(|err| AuthError::FailedDependency(err.into()))?; let mut validation = jsonwebtoken::Validation::new(jsonwebtoken::Algorithm::RS256); - validation.set_audience(&[&self.client_id]); + validation.set_audience(&[self.client.client_id()]); validation.set_issuer(&["https://appleid.apple.com"]); let token_data = jsonwebtoken::decode::(id_token, &decoding_key, &validation) @@ -113,28 +101,15 @@ impl AppleOAuthProvider { #[async_trait] impl OAuthProvider for AppleOAuthProvider { - fn name(&self) -> &'static str { - Self::NAME + fn name(&self) -> &str { + return Self::NAME; } - fn provider(&self) -> OAuthProviderId { - OAuthProviderId::Apple - } - fn display_name(&self) -> &'static str { - Self::DISPLAY_NAME + fn display_name(&self) -> &str { + return Self::DISPLAY_NAME; } fn settings(&self) -> Result { - lazy_static! { - static ref AUTH_URL: Url = Url::parse(AppleOAuthProvider::AUTH_URL).expect("infallible"); - static ref TOKEN_URL: Url = Url::parse(AppleOAuthProvider::TOKEN_URL).expect("infallible"); - } - - return Ok(OAuthClientSettings { - auth_url: AUTH_URL.clone(), - token_url: TOKEN_URL.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); + return Ok(self.client.settings()); } fn oauth_scopes(&self) -> Vec<&str> { diff --git a/crates/core/src/auth/oauth/providers/client.rs b/crates/core/src/auth/oauth/providers/client.rs new file mode 100644 index 000000000..2ecf2c48e --- /dev/null +++ b/crates/core/src/auth/oauth/providers/client.rs @@ -0,0 +1,124 @@ +//! The client side of talking to an external provider: what we authenticate *with* +//! ([`ProviderClient`]) and how we then ask *who the user is* ([`UserApi`]). +//! +//! Both are shared across the provider implementations in this directory, including the ones that +//! are too irregular to be a [`super::social::SocialSpec`]. + +use oauth2::TokenResponse as _; +use serde::de::DeserializeOwned; +use url::Url; + +use crate::auth::AuthError; +use crate::auth::oauth::providers::OAuthProviderError; +use crate::auth::oauth::providers::interface::{OAuthClientSettings, TokenResponse}; +use crate::config::proto::OAuthProviderConfig; + +/// Credentials and endpoints of a provider whose URLs are known at compile time. +/// +/// Shared by [`super::social::SocialProvider`] and Apple, which otherwise have nothing in common: +/// Apple reads its claims off a JWT rather than a user API, but it still has to pick the same two +/// secrets out of the config and hand back the same settings. +pub struct ProviderClient { + client_id: String, + client_secret: String, + auth_url: Url, + token_url: Url, +} + +impl ProviderClient { + /// `display_name` only names the provider in the error. The URLs must be parseable. + pub fn new( + config: &OAuthProviderConfig, + display_name: &str, + auth_url: &str, + token_url: &str, + ) -> Result { + let Some(client_id) = config.client_id.clone() else { + return Err(OAuthProviderError::Missing(format!( + "{display_name} client id" + ))); + }; + let Some(client_secret) = config.client_secret.clone() else { + return Err(OAuthProviderError::Missing(format!( + "{display_name} client secret" + ))); + }; + + return Ok(Self { + client_id, + client_secret, + // NOTE: Infallible, callers pass compile-time constants. + auth_url: Url::parse(auth_url).expect("infallible"), + token_url: Url::parse(token_url).expect("infallible"), + }); + } + + pub fn client_id(&self) -> &str { + return &self.client_id; + } + + pub fn settings(&self) -> OAuthClientSettings { + return OAuthClientSettings { + auth_url: self.auth_url.clone(), + token_url: self.token_url.clone(), + client_id: self.client_id.clone(), + client_secret: self.client_secret.clone(), + }; + } +} + +/// Bearer-authenticated client for a provider's user-info API. +pub struct UserApi<'a> { + client: reqwest::Client, + access_token: &'a str, + user_api_url: &'a str, + headers: Vec<(&'static str, String)>, +} + +impl<'a> UserApi<'a> { + /// Constructing through the token response is what keeps the bearer check from being forgotten + /// by any one provider. + /// + /// `headers` are sent on top of the bearer token, for the providers that demand more. + pub fn new( + token_response: &'a TokenResponse, + user_api_url: &'a str, + headers: Vec<(&'static str, String)>, + ) -> Result { + if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { + return Err(AuthError::Internal( + format!("Unexpected token type: {:?}", token_response.token_type()).into(), + )); + } + + return Ok(Self { + client: reqwest::Client::new(), + access_token: token_response.access_token().secret(), + user_api_url, + headers, + }); + } + + /// The provider's user-info endpoint. + /// + /// Providers making follow-up calls must derive them from this rather than from a constant, so + /// tests can redirect those too. + pub fn user_api_url(&self) -> &str { + return self.user_api_url; + } + + pub async fn get_json(&self, url: &str) -> Result { + let mut request = self.client.get(url).bearer_auth(self.access_token); + for (name, value) in &self.headers { + request = request.header(*name, value); + } + + return request + .send() + .await + .map_err(|err| AuthError::FailedDependency(err.into()))? + .json::() + .await + .map_err(|err| AuthError::FailedDependency(err.into())); + } +} diff --git a/crates/core/src/auth/oauth/providers/discord.rs b/crates/core/src/auth/oauth/providers/discord.rs index 030c35d04..361085b62 100644 --- a/crates/core/src/auth/oauth/providers/discord.rs +++ b/crates/core/src/auth/oauth/providers/discord.rs @@ -2,7 +2,8 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; +use crate::auth::oauth::providers::client::UserApi; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; pub(crate) struct Discord; diff --git a/crates/core/src/auth/oauth/providers/facebook.rs b/crates/core/src/auth/oauth/providers/facebook.rs index 25014a47a..1a8f5bf8b 100644 --- a/crates/core/src/auth/oauth/providers/facebook.rs +++ b/crates/core/src/auth/oauth/providers/facebook.rs @@ -2,7 +2,8 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::social::{DataEnvelope, ExternalUser, SocialSpec, UserApi}; +use crate::auth::oauth::providers::client::UserApi; +use crate::auth::oauth::providers::social::{DataEnvelope, ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; pub(crate) struct Facebook; diff --git a/crates/core/src/auth/oauth/providers/github.rs b/crates/core/src/auth/oauth/providers/github.rs index ddb0edf55..4266ac359 100644 --- a/crates/core/src/auth/oauth/providers/github.rs +++ b/crates/core/src/auth/oauth/providers/github.rs @@ -2,7 +2,8 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; +use crate::auth::oauth::providers::client::UserApi; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; pub(crate) struct Github; @@ -31,7 +32,7 @@ struct GithubEmail { impl SocialSpec for Github { const ID: OAuthProviderId = OAuthProviderId::Github; const NAME: &'static str = "github"; - const DISPLAY_NAME: &'static str = "Github"; + const DISPLAY_NAME: &'static str = "GitHub"; const AUTH_URL: &'static str = "https://github.com/login/oauth/authorize"; const TOKEN_URL: &'static str = "https://github.com/login/oauth/access_token"; diff --git a/crates/core/src/auth/oauth/providers/gitlab.rs b/crates/core/src/auth/oauth/providers/gitlab.rs index 132c77be9..aa9588335 100644 --- a/crates/core/src/auth/oauth/providers/gitlab.rs +++ b/crates/core/src/auth/oauth/providers/gitlab.rs @@ -2,7 +2,8 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; +use crate::auth::oauth::providers::client::UserApi; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; pub(crate) struct Gitlab; diff --git a/crates/core/src/auth/oauth/providers/google.rs b/crates/core/src/auth/oauth/providers/google.rs index bc93887a2..9cab38cf8 100644 --- a/crates/core/src/auth/oauth/providers/google.rs +++ b/crates/core/src/auth/oauth/providers/google.rs @@ -2,7 +2,8 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; +use crate::auth::oauth::providers::client::UserApi; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; pub(crate) struct Google; diff --git a/crates/core/src/auth/oauth/provider.rs b/crates/core/src/auth/oauth/providers/interface.rs similarity index 97% rename from crates/core/src/auth/oauth/provider.rs rename to crates/core/src/auth/oauth/providers/interface.rs index c5679a853..d826050ce 100644 --- a/crates/core/src/auth/oauth/provider.rs +++ b/crates/core/src/auth/oauth/providers/interface.rs @@ -66,11 +66,10 @@ pub struct OAuthClientSettings { /// Common trait for OAuth providers like Discord, etc. #[async_trait] pub trait OAuthProvider { - #[allow(unused)] - fn provider(&self) -> OAuthProviderId; - + /// Config key and URL path segment, i.e. what users authenticate against. fn name(&self) -> &str; + /// Human-readable name, shown in the admin UI and returned by the providers API. fn display_name(&self) -> &str; fn auth_type(&self) -> AuthType { diff --git a/crates/core/src/auth/oauth/providers/microsoft.rs b/crates/core/src/auth/oauth/providers/microsoft.rs index 8eccf04f4..fcd820243 100644 --- a/crates/core/src/auth/oauth/providers/microsoft.rs +++ b/crates/core/src/auth/oauth/providers/microsoft.rs @@ -2,7 +2,8 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; +use crate::auth::oauth::providers::client::UserApi; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; pub(crate) struct Microsoft; diff --git a/crates/core/src/auth/oauth/providers/mod.rs b/crates/core/src/auth/oauth/providers/mod.rs index 2cb7007ec..c08f86c7c 100644 --- a/crates/core/src/auth/oauth/providers/mod.rs +++ b/crates/core/src/auth/oauth/providers/mod.rs @@ -1,3 +1,19 @@ +//! External OAuth providers: which ones exist, and the pieces they're built from. +//! +//! - [`interface`]: what a provider must implement, i.e. the [`OAuthProvider`] trait. +//! - [`client`]: how to talk to one, i.e. credentials, endpoints and the user-info request. +//! - [`social`]: the declarative shortcut all but three providers take. +//! - One module per provider, plus the registry at the bottom of this file. + +/// What a provider must implement. +pub(crate) mod interface; + +/// Credentials, endpoints and the authenticated user-info request. +pub(crate) mod client; + +/// Declarative provider descriptions, used by all but Apple, OIDC and the test provider. +mod social; + mod apple; mod discord; mod facebook; @@ -6,7 +22,6 @@ mod gitlab; mod google; mod microsoft; mod oidc; -mod social; mod twitch; mod yandex; diff --git a/crates/core/src/auth/oauth/providers/oidc.rs b/crates/core/src/auth/oauth/providers/oidc.rs index deba5be5d..6617e2776 100644 --- a/crates/core/src/auth/oauth/providers/oidc.rs +++ b/crates/core/src/auth/oauth/providers/oidc.rs @@ -1,10 +1,10 @@ use async_trait::async_trait; -use oauth2::TokenResponse as _; use serde::{Deserialize, Serialize}; use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; +use crate::auth::oauth::providers::client::UserApi; +use crate::auth::oauth::providers::interface::TokenResponse; use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; @@ -90,9 +90,6 @@ impl OAuthProvider for OidcProvider { fn name(&self) -> &str { return &self.name; } - fn provider(&self) -> OAuthProviderId { - OAuthProviderId::Oidc0 - } fn display_name(&self) -> &str { return &self.display_name; } @@ -114,23 +111,8 @@ impl OAuthProvider for OidcProvider { } async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - let response = reqwest::Client::new() - .get(&self.user_api_url) - .bearer_auth(token_response.access_token().secret()) - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - let user = response - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; + let api = UserApi::new(token_response, &self.user_api_url, vec![])?; + let user = api.get_json::(api.user_api_url()).await?; return Ok(OAuthUser { provider_user_id: user.sub, diff --git a/crates/core/src/auth/oauth/providers/social.rs b/crates/core/src/auth/oauth/providers/social.rs index 0dc7b0f4e..4f09513bc 100644 --- a/crates/core/src/auth/oauth/providers/social.rs +++ b/crates/core/src/auth/oauth/providers/social.rs @@ -1,12 +1,12 @@ use async_trait::async_trait; -use oauth2::{AuthType, TokenResponse as _}; +use oauth2::AuthType; use serde::Deserialize; use serde::de::DeserializeOwned; use std::marker::PhantomData; -use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; +use crate::auth::oauth::providers::client::{ProviderClient, UserApi}; +use crate::auth::oauth::providers::interface::TokenResponse; use crate::auth::oauth::providers::{OAuthProviderError, OAuthProviderFactory}; use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; @@ -87,47 +87,10 @@ pub(crate) struct DataEnvelope { pub data: T, } -/// Authenticated client for a provider's user-info API. -pub(crate) struct UserApi<'a> { - client: reqwest::Client, - access_token: &'a str, - user_api_url: &'a str, - headers: Vec<(&'static str, String)>, -} - -impl UserApi<'_> { - /// The provider's user-info endpoint, i.e. [`SocialSpec::USER_API_URL`] outside of tests. - /// - /// Providers making follow-up calls must derive them from this rather than the constant, so - /// tests can redirect them too. - pub(crate) fn user_api_url(&self) -> &str { - return self.user_api_url; - } - - pub(crate) async fn get_json(&self, url: &str) -> Result { - let mut request = self.client.get(url).bearer_auth(self.access_token); - for (name, value) in &self.headers { - request = request.header(*name, value); - } - - return request - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))? - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into())); - } -} - pub(crate) struct SocialProvider { - client_id: String, - client_secret: String, + client: ProviderClient, - // NOTE: Held rather than derived from `S` on every call, both to parse the URLs only once and so - // tests can point a provider at a fake server. - auth_url: Url, - token_url: Url, + // NOTE: A field rather than `S::USER_API_URL` so tests can point a provider at a fake server. user_api_url: String, spec: PhantomData, @@ -135,25 +98,8 @@ pub(crate) struct SocialProvider { impl SocialProvider { fn new(config: &OAuthProviderConfig) -> Result { - let Some(client_id) = config.client_id.clone() else { - return Err(OAuthProviderError::Missing(format!( - "{} client id", - S::DISPLAY_NAME - ))); - }; - let Some(client_secret) = config.client_secret.clone() else { - return Err(OAuthProviderError::Missing(format!( - "{} client secret", - S::DISPLAY_NAME - ))); - }; - return Ok(Self { - client_id, - client_secret, - // NOTE: Infallible, the URLs are compile-time constants. - auth_url: Url::parse(S::AUTH_URL).expect("infallible"), - token_url: Url::parse(S::TOKEN_URL).expect("infallible"), + client: ProviderClient::new(config, S::DISPLAY_NAME, S::AUTH_URL, S::TOKEN_URL)?, user_api_url: S::USER_API_URL.to_string(), spec: PhantomData, }); @@ -173,13 +119,10 @@ impl SocialProvider { #[async_trait] impl OAuthProvider for SocialProvider { - fn name(&self) -> &'static str { + fn name(&self) -> &str { return S::NAME; } - fn provider(&self) -> OAuthProviderId { - return S::ID; - } - fn display_name(&self) -> &'static str { + fn display_name(&self) -> &str { return S::DISPLAY_NAME; } fn auth_type(&self) -> AuthType { @@ -190,12 +133,7 @@ impl OAuthProvider for SocialProvider { } fn settings(&self) -> Result { - return Ok(OAuthClientSettings { - auth_url: self.auth_url.clone(), - token_url: self.token_url.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); + return Ok(self.client.settings()); } fn recover_token_response(&self, body: &[u8]) -> Option> { @@ -203,18 +141,11 @@ impl OAuthProvider for SocialProvider { } async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - let api = UserApi { - client: reqwest::Client::new(), - access_token: token_response.access_token().secret(), - user_api_url: &self.user_api_url, - headers: S::user_api_headers(&self.client_id), - }; + let api = UserApi::new( + token_response, + &self.user_api_url, + S::user_api_headers(self.client.client_id()), + )?; let user = S::map_user(&api, api.get_json::(api.user_api_url()).await?).await?; @@ -250,7 +181,7 @@ mod testing { use axum_test::{TestServer, TestServerConfig}; use super::*; - use crate::auth::oauth::provider::ExtraTokenFields; + use crate::auth::oauth::providers::interface::ExtraTokenFields; /// Path the fake user-info endpoint is served under. pub(crate) const USER_API_TEST_PATH: &str = "/user"; @@ -267,11 +198,15 @@ mod testing { }, ); + let config = OAuthProviderConfig { + client_id: Some("client_id".to_string()), + client_secret: Some("client_secret".to_string()), + ..Default::default() + }; + let provider = SocialProvider:: { - client_id: "client_id".to_string(), - client_secret: "client_secret".to_string(), - auth_url: Url::parse(S::AUTH_URL).expect("infallible"), - token_url: Url::parse(S::TOKEN_URL).expect("infallible"), + client: ProviderClient::new(&config, S::DISPLAY_NAME, S::AUTH_URL, S::TOKEN_URL) + .expect("infallible"), user_api_url: server.server_url(USER_API_TEST_PATH).unwrap().to_string(), spec: PhantomData, }; diff --git a/crates/core/src/auth/oauth/providers/test.rs b/crates/core/src/auth/oauth/providers/test.rs index eb8a0ff76..9bc38c9bd 100644 --- a/crates/core/src/auth/oauth/providers/test.rs +++ b/crates/core/src/auth/oauth/providers/test.rs @@ -1,11 +1,11 @@ use async_trait::async_trait; -use oauth2::TokenResponse as _; use serde::{Deserialize, Serialize}; use url::Url; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; use crate::auth::oauth::providers::OAuthProviderFactory; +use crate::auth::oauth::providers::client::UserApi; +use crate::auth::oauth::providers::interface::TokenResponse; use crate::auth::oauth::{OAuthClientSettings, OAuthProvider, OAuthUser}; use crate::config::proto::{OAuthProviderConfig, OAuthProviderId}; @@ -49,14 +49,11 @@ pub struct TestUser { #[async_trait] impl OAuthProvider for TestOAuthProvider { - fn name(&self) -> &'static str { - Self::NAME + fn name(&self) -> &str { + return Self::NAME; } - fn provider(&self) -> OAuthProviderId { - OAuthProviderId::Test - } - fn display_name(&self) -> &'static str { - Self::DISPLAY_NAME + fn display_name(&self) -> &str { + return Self::DISPLAY_NAME; } fn settings(&self) -> Result { @@ -73,23 +70,8 @@ impl OAuthProvider for TestOAuthProvider { } async fn get_user(&self, token_response: &TokenResponse) -> Result { - if *token_response.token_type() != oauth2::basic::BasicTokenType::Bearer { - return Err(AuthError::Internal( - format!("Unexpected token type: {:?}", token_response.token_type()).into(), - )); - } - - let response = reqwest::Client::new() - .get(&self.user_api_url) - .bearer_auth(token_response.access_token().secret()) - .send() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; - - let user = response - .json::() - .await - .map_err(|err| AuthError::FailedDependency(err.into()))?; + let api = UserApi::new(token_response, &self.user_api_url, vec![])?; + let user = api.get_json::(api.user_api_url()).await?; return Ok(OAuthUser { provider_user_id: user.id, diff --git a/crates/core/src/auth/oauth/providers/twitch.rs b/crates/core/src/auth/oauth/providers/twitch.rs index f4394a6ef..77f435eae 100644 --- a/crates/core/src/auth/oauth/providers/twitch.rs +++ b/crates/core/src/auth/oauth/providers/twitch.rs @@ -2,8 +2,9 @@ use async_trait::async_trait; use serde::{Deserialize, Serialize}; use crate::auth::AuthError; -use crate::auth::oauth::provider::TokenResponse; -use crate::auth::oauth::providers::social::{DataEnvelope, ExternalUser, SocialSpec, UserApi}; +use crate::auth::oauth::providers::client::UserApi; +use crate::auth::oauth::providers::interface::TokenResponse; +use crate::auth::oauth::providers::social::{DataEnvelope, ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; pub(crate) struct Twitch; diff --git a/crates/core/src/auth/oauth/providers/yandex.rs b/crates/core/src/auth/oauth/providers/yandex.rs index 4834da53e..97d1e82f0 100644 --- a/crates/core/src/auth/oauth/providers/yandex.rs +++ b/crates/core/src/auth/oauth/providers/yandex.rs @@ -2,7 +2,8 @@ use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec, UserApi}; +use crate::auth::oauth::providers::client::UserApi; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; pub(crate) struct Yandex; From 4ca1bbe776a31a46af683c223114b25947bc2d4c Mon Sep 17 00:00:00 2001 From: Benjamin Sobel Date: Tue, 4 Aug 2026 03:20:55 -0700 Subject: [PATCH 7/7] tryfrom --- .../core/src/auth/oauth/providers/discord.rs | 35 ++++----- .../core/src/auth/oauth/providers/facebook.rs | 27 +++---- .../core/src/auth/oauth/providers/github.rs | 72 +++++++++++-------- .../core/src/auth/oauth/providers/gitlab.rs | 31 ++++---- .../core/src/auth/oauth/providers/google.rs | 27 +++---- .../src/auth/oauth/providers/microsoft.rs | 27 +++---- .../core/src/auth/oauth/providers/social.rs | 20 ++++-- .../core/src/auth/oauth/providers/twitch.rs | 58 ++++++++------- .../core/src/auth/oauth/providers/yandex.rs | 35 ++++----- 9 files changed, 180 insertions(+), 152 deletions(-) diff --git a/crates/core/src/auth/oauth/providers/discord.rs b/crates/core/src/auth/oauth/providers/discord.rs index 361085b62..8640c42ab 100644 --- a/crates/core/src/auth/oauth/providers/discord.rs +++ b/crates/core/src/auth/oauth/providers/discord.rs @@ -1,8 +1,6 @@ -use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::client::UserApi; use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; @@ -20,21 +18,10 @@ pub(crate) struct DiscordUser { avatar: Option, } -#[async_trait] -impl SocialSpec for Discord { - const ID: OAuthProviderId = OAuthProviderId::Discord; - const NAME: &'static str = "discord"; - const DISPLAY_NAME: &'static str = "Discord"; +impl TryFrom for ExternalUser { + type Error = AuthError; - const AUTH_URL: &'static str = "https://discord.com/oauth2/authorize"; - const TOKEN_URL: &'static str = "https://discord.com/api/oauth2/token"; - const USER_API_URL: &'static str = "https://discord.com/api/users/@me"; - - const SCOPES: &'static [&'static str] = &["identify", "email"]; - - type User = DiscordUser; - - async fn map_user(_api: &UserApi<'_>, user: DiscordUser) -> Result { + fn try_from(user: DiscordUser) -> Result { // let username = match (user.discriminator, user.username) { // (Some(discriminator), Some(username)) => Some(format!("{username}#{discriminator}")), // (None, Some(username)) => Some(username.to_string()), @@ -42,7 +29,7 @@ impl SocialSpec for Discord { // (None, None) => None, // }; - return Ok(ExternalUser { + return Ok(Self { // Discord only hands out the avatar's hash, the CDN URL is ours to build. avatar: user.avatar.map(|avatar| { format!( @@ -58,6 +45,20 @@ impl SocialSpec for Discord { } } +impl SocialSpec for Discord { + const ID: OAuthProviderId = OAuthProviderId::Discord; + const NAME: &'static str = "discord"; + const DISPLAY_NAME: &'static str = "Discord"; + + const AUTH_URL: &'static str = "https://discord.com/oauth2/authorize"; + const TOKEN_URL: &'static str = "https://discord.com/api/oauth2/token"; + const USER_API_URL: &'static str = "https://discord.com/api/users/@me"; + + const SCOPES: &'static [&'static str] = &["identify", "email"]; + + type User = DiscordUser; +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/core/src/auth/oauth/providers/facebook.rs b/crates/core/src/auth/oauth/providers/facebook.rs index 1a8f5bf8b..303801336 100644 --- a/crates/core/src/auth/oauth/providers/facebook.rs +++ b/crates/core/src/auth/oauth/providers/facebook.rs @@ -1,8 +1,6 @@ -use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::client::UserApi; use crate::auth::oauth::providers::social::{DataEnvelope, ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; @@ -21,7 +19,20 @@ pub(crate) struct FacebookUser { picture: Option>, } -#[async_trait] +impl TryFrom for ExternalUser { + type Error = AuthError; + + fn try_from(user: FacebookUser) -> Result { + return Ok(Self { + provider_user_id: user.id, + email: Some(user.email), + verified: true, + avatar: user.picture.map(|p| p.data.url), + ..Default::default() + }); + } +} + impl SocialSpec for Facebook { const ID: OAuthProviderId = OAuthProviderId::Facebook; const NAME: &'static str = "facebook"; @@ -35,16 +46,6 @@ impl SocialSpec for Facebook { const SCOPES: &'static [&'static str] = &["email"]; type User = FacebookUser; - - async fn map_user(_api: &UserApi<'_>, user: FacebookUser) -> Result { - return Ok(ExternalUser { - provider_user_id: user.id, - email: Some(user.email), - verified: true, - avatar: user.picture.map(|p| p.data.url), - ..Default::default() - }); - } } #[cfg(test)] diff --git a/crates/core/src/auth/oauth/providers/github.rs b/crates/core/src/auth/oauth/providers/github.rs index 4266ac359..4476e1412 100644 --- a/crates/core/src/auth/oauth/providers/github.rs +++ b/crates/core/src/auth/oauth/providers/github.rs @@ -19,6 +19,26 @@ pub(crate) struct GithubUser { avatar_url: Option, } +impl TryFrom for ExternalUser { + type Error = AuthError; + + fn try_from(user: GithubUser) -> Result { + // NOTE: Github blanks the email for users who keep it private, which `Github::map_user` fills + // in beforehand. Reaching here without one means neither endpoint had anything usable. + let Some(email) = user.email.filter(|email| !email.is_empty()) else { + return Err(AuthError::FailedDependency("missing email".into())); + }; + + return Ok(Self { + provider_user_id: user.id.to_string(), + email: Some(email), + username: user.login, + verified: true, + avatar: user.avatar_url, + }); + } +} + #[derive(Default, Deserialize, Debug)] struct GithubEmail { email: String, @@ -28,6 +48,23 @@ struct GithubEmail { // visibility: Option, } +/// Github's dedicated email endpoint, needed for users who opted out of exposing their address on +/// the user endpoint. +async fn primary_email(api: &UserApi<'_>) -> Result { + let emails: Vec = api + .get_json(&format!("{}/emails", api.user_api_url())) + .await?; + + let Some(primary) = emails + .into_iter() + .find(|cand| cand.verified && cand.primary) + else { + return Err(AuthError::FailedDependency("missing email".into())); + }; + + return Ok(primary.email); +} + #[async_trait] impl SocialSpec for Github { const ID: OAuthProviderId = OAuthProviderId::Github; @@ -48,35 +85,14 @@ impl SocialSpec for Github { return vec![("User-Agent", "TrailBase".to_string())]; } - async fn map_user(api: &UserApi<'_>, user: GithubUser) -> Result { - // Users can set the "Keep my email private" option, in which case the user api will return an - // empty email and we'll have to call the dedicated `/emails` endpoint. - let email = if let Some(email) = user.email - && !email.is_empty() - { - email - } else { - let emails: Vec = api - .get_json(&format!("{}/emails", api.user_api_url())) - .await?; - - let Some(primary) = emails - .into_iter() - .find(|cand| cand.verified && cand.primary) - else { - return Err(AuthError::FailedDependency("missing email".into())); - }; - - primary.email - }; + /// Overridden because users can set the "Keep my email private" option, in which case the user + /// api returns an empty email and we have to ask the dedicated `/emails` endpoint instead. + async fn map_user(api: &UserApi<'_>, mut user: GithubUser) -> Result { + if user.email.as_deref().unwrap_or_default().is_empty() { + user.email = Some(primary_email(api).await?); + } - return Ok(ExternalUser { - provider_user_id: user.id.to_string(), - email: Some(email), - username: user.login, - verified: true, - avatar: user.avatar_url, - }); + return user.try_into(); } } diff --git a/crates/core/src/auth/oauth/providers/gitlab.rs b/crates/core/src/auth/oauth/providers/gitlab.rs index aa9588335..bc8ed6047 100644 --- a/crates/core/src/auth/oauth/providers/gitlab.rs +++ b/crates/core/src/auth/oauth/providers/gitlab.rs @@ -1,8 +1,6 @@ -use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::client::UserApi; use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; @@ -19,7 +17,22 @@ pub(crate) struct GitlabUser { state: String, } -#[async_trait] +impl TryFrom for ExternalUser { + type Error = AuthError; + + fn try_from(user: GitlabUser) -> Result { + return Ok(Self { + provider_user_id: user.id.to_string(), + email: Some(user.email), + username: user.username, + // GitLab has no email-confirmation flag, but blocked and deactivated accounts must not + // be able to log in. + verified: user.state == "active", + avatar: user.avatar_url, + }); + } +} + impl SocialSpec for Gitlab { const ID: OAuthProviderId = OAuthProviderId::Gitlab; const NAME: &'static str = "gitlab"; @@ -32,18 +45,6 @@ impl SocialSpec for Gitlab { const SCOPES: &'static [&'static str] = &["read_user"]; type User = GitlabUser; - - async fn map_user(_api: &UserApi<'_>, user: GitlabUser) -> Result { - return Ok(ExternalUser { - provider_user_id: user.id.to_string(), - email: Some(user.email), - username: user.username, - // GitLab has no email-confirmation flag, but blocked and deactivated accounts must not - // be able to log in. - verified: user.state == "active", - avatar: user.avatar_url, - }); - } } #[cfg(test)] diff --git a/crates/core/src/auth/oauth/providers/google.rs b/crates/core/src/auth/oauth/providers/google.rs index 9cab38cf8..4b4b4efac 100644 --- a/crates/core/src/auth/oauth/providers/google.rs +++ b/crates/core/src/auth/oauth/providers/google.rs @@ -1,8 +1,6 @@ -use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::client::UserApi; use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; @@ -17,7 +15,20 @@ pub(crate) struct GoogleUser { picture: Option, } -#[async_trait] +impl TryFrom for ExternalUser { + type Error = AuthError; + + fn try_from(user: GoogleUser) -> Result { + return Ok(Self { + provider_user_id: user.id, + email: Some(user.email), + verified: user.verified_email, + avatar: user.picture, + ..Default::default() + }); + } +} + impl SocialSpec for Google { const ID: OAuthProviderId = OAuthProviderId::Google; const NAME: &'static str = "google"; @@ -33,16 +44,6 @@ impl SocialSpec for Google { ]; type User = GoogleUser; - - async fn map_user(_api: &UserApi<'_>, user: GoogleUser) -> Result { - return Ok(ExternalUser { - provider_user_id: user.id, - email: Some(user.email), - verified: user.verified_email, - avatar: user.picture, - ..Default::default() - }); - } } #[cfg(test)] diff --git a/crates/core/src/auth/oauth/providers/microsoft.rs b/crates/core/src/auth/oauth/providers/microsoft.rs index fcd820243..4ba68898e 100644 --- a/crates/core/src/auth/oauth/providers/microsoft.rs +++ b/crates/core/src/auth/oauth/providers/microsoft.rs @@ -1,8 +1,6 @@ -use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::client::UserApi; use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; @@ -15,7 +13,20 @@ pub(crate) struct MicrosoftUser { // displayName: String, } -#[async_trait] +impl TryFrom for ExternalUser { + type Error = AuthError; + + fn try_from(user: MicrosoftUser) -> Result { + return Ok(Self { + provider_user_id: user.id, + email: Some(user.mail), + // username: Some(user.displayName), + verified: true, + ..Default::default() + }); + } +} + impl SocialSpec for Microsoft { const ID: OAuthProviderId = OAuthProviderId::Microsoft; const NAME: &'static str = "microsoft"; @@ -28,16 +39,6 @@ impl SocialSpec for Microsoft { const SCOPES: &'static [&'static str] = &["User.Read"]; type User = MicrosoftUser; - - async fn map_user(_api: &UserApi<'_>, user: MicrosoftUser) -> Result { - return Ok(ExternalUser { - provider_user_id: user.id, - email: Some(user.mail), - // username: Some(user.displayName), - verified: true, - ..Default::default() - }); - } } #[cfg(test)] diff --git a/crates/core/src/auth/oauth/providers/social.rs b/crates/core/src/auth/oauth/providers/social.rs index 4f09513bc..cf94d7943 100644 --- a/crates/core/src/auth/oauth/providers/social.rs +++ b/crates/core/src/auth/oauth/providers/social.rs @@ -38,7 +38,10 @@ pub(crate) trait SocialSpec: Send + Sync + 'static { const AUTH_TYPE: AuthType = AuthType::BasicAuth; /// The provider's user-info response. - type User: DeserializeOwned + Send; + /// + /// Its `TryFrom for ExternalUser` impl is where the mapping onto our user model + /// lives, next to the struct describing the wire format rather than out here in the metadata. + type User: DeserializeOwned + Send + TryInto; /// Headers the user-info request needs on top of the bearer token. fn user_api_headers(_client_id: &str) -> Vec<(&'static str, String)> { @@ -51,11 +54,13 @@ pub(crate) trait SocialSpec: Send + Sync + 'static { return None; } - /// Maps the provider's user onto [`ExternalUser`]. + /// Resolves the authenticated user. /// - /// `api` is only needed by the few providers that have to make follow-up calls, e.g. Github's - /// separate email endpoint. - async fn map_user(api: &UserApi<'_>, user: Self::User) -> Result; + /// Defaults to [`Self::User`]'s own conversion. Override only when the mapping needs a further + /// request, as Github's does for users who keep their address private. + async fn map_user(_api: &UserApi<'_>, user: Self::User) -> Result { + return user.try_into(); + } fn factory() -> OAuthProviderFactory where @@ -65,7 +70,10 @@ pub(crate) trait SocialSpec: Send + Sync + 'static { } } -/// What a [`SocialSpec`] pulls out of its provider's user-info response. +/// Our user model, as far as an external provider can describe it. +/// +/// Each provider's user-info struct converts into this via `TryFrom`, which is where you'll find +/// the per-provider mapping. /// /// Narrower than [`OAuthUser`] on purpose: [`SocialProvider`] fills in the provider id, so a spec /// can't accidentally claim to be a different provider, and it turns an unverified user into diff --git a/crates/core/src/auth/oauth/providers/twitch.rs b/crates/core/src/auth/oauth/providers/twitch.rs index 77f435eae..73057e829 100644 --- a/crates/core/src/auth/oauth/providers/twitch.rs +++ b/crates/core/src/auth/oauth/providers/twitch.rs @@ -1,8 +1,6 @@ -use async_trait::async_trait; use serde::{Deserialize, Serialize}; use crate::auth::AuthError; -use crate::auth::oauth::providers::client::UserApi; use crate::auth::oauth::providers::interface::TokenResponse; use crate::auth::oauth::providers::social::{DataEnvelope, ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; @@ -20,7 +18,34 @@ pub(crate) struct TwitchUser { profile_image_url: Option, } -#[async_trait] +impl TryFrom>> for ExternalUser { + type Error = AuthError; + + fn try_from(mut response: DataEnvelope>) -> Result { + let user = match response.data.len() { + 1 => response.data.swap_remove(0), + 0 => { + return Err(AuthError::FailedDependency( + "Twitch user response had empty data".into(), + )); + } + n => { + return Err(AuthError::FailedDependency( + format!("Twitch user response contains {n} users").into(), + )); + } + }; + + return Ok(Self { + provider_user_id: user.id, + email: Some(user.email), + username: user.login, + verified: true, + avatar: user.profile_image_url, + }); + } +} + impl SocialSpec for Twitch { const ID: OAuthProviderId = OAuthProviderId::Twitch; const NAME: &'static str = "twitch"; @@ -45,33 +70,6 @@ impl SocialSpec for Twitch { // list. return Some(parse_twitch_token_response(body)); } - - async fn map_user( - _api: &UserApi<'_>, - mut response: DataEnvelope>, - ) -> Result { - let user = match response.data.len() { - 1 => response.data.swap_remove(0), - 0 => { - return Err(AuthError::FailedDependency( - "Twitch user response had empty data".into(), - )); - } - n => { - return Err(AuthError::FailedDependency( - format!("Twitch user response contains {n} users").into(), - )); - } - }; - - return Ok(ExternalUser { - provider_user_id: user.id, - email: Some(user.email), - username: user.login, - verified: true, - avatar: user.profile_image_url, - }); - } } #[derive(Clone, Debug, Deserialize, Serialize)] diff --git a/crates/core/src/auth/oauth/providers/yandex.rs b/crates/core/src/auth/oauth/providers/yandex.rs index 97d1e82f0..ebbef024e 100644 --- a/crates/core/src/auth/oauth/providers/yandex.rs +++ b/crates/core/src/auth/oauth/providers/yandex.rs @@ -1,8 +1,6 @@ -use async_trait::async_trait; use serde::Deserialize; use crate::auth::AuthError; -use crate::auth::oauth::providers::client::UserApi; use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; use crate::config::proto::OAuthProviderId; @@ -21,22 +19,11 @@ pub(crate) struct YandexUser { default_avatar_id: String, } -#[async_trait] -impl SocialSpec for Yandex { - const ID: OAuthProviderId = OAuthProviderId::Yandex; - const NAME: &'static str = "yandex"; - const DISPLAY_NAME: &'static str = "Yandex"; +impl TryFrom for ExternalUser { + type Error = AuthError; - const AUTH_URL: &'static str = "https://oauth.yandex.com/authorize"; - const TOKEN_URL: &'static str = "https://oauth.yandex.com/token"; - const USER_API_URL: &'static str = "https://login.yandex.ru/info"; - - const SCOPES: &'static [&'static str] = &["login:email", "login:avatar", "login:info"]; - - type User = YandexUser; - - async fn map_user(_api: &UserApi<'_>, user: YandexUser) -> Result { - return Ok(ExternalUser { + fn try_from(user: YandexUser) -> Result { + return Ok(Self { provider_user_id: user.id, email: Some(user.default_email), username: user.login, @@ -52,6 +39,20 @@ impl SocialSpec for Yandex { } } +impl SocialSpec for Yandex { + const ID: OAuthProviderId = OAuthProviderId::Yandex; + const NAME: &'static str = "yandex"; + const DISPLAY_NAME: &'static str = "Yandex"; + + const AUTH_URL: &'static str = "https://oauth.yandex.com/authorize"; + const TOKEN_URL: &'static str = "https://oauth.yandex.com/token"; + const USER_API_URL: &'static str = "https://login.yandex.ru/info"; + + const SCOPES: &'static [&'static str] = &["login:email", "login:avatar", "login:info"]; + + type User = YandexUser; +} + #[cfg(test)] mod tests { use super::*;