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 38a83ea37..530d62aa2 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, ( @@ -400,7 +413,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/mod.rs b/crates/core/src/auth/oauth/mod.rs index a88b61f19..5af2a2916 100644 --- a/crates/core/src/auth/oauth/mod.rs +++ b/crates/core/src/auth/oauth/mod.rs @@ -1,3 +1,5 @@ +pub(crate) mod providers; + mod callback; mod list_providers; mod login; @@ -13,7 +15,7 @@ use utoipa_axum::router::OpenApiRouter; use crate::AppState; -pub(crate) use provider::{OAuthClientSettings, OAuthProvider, OAuthUser}; +pub(crate) use providers::interface::{OAuthClientSettings, OAuthProvider, OAuthUser}; pub(crate) use reqwest_client::ReqwestClient; pub fn oauth_router() -> OpenApiRouter { 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/providers/apple.rs b/crates/core/src/auth/oauth/providers/apple.rs index d03598774..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,31 +101,18 @@ 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<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["name", "email"]; } @@ -155,7 +130,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/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 d316c5d20..8640c42ab 100644 --- a/crates/core/src/auth/oauth/providers/discord.rs +++ b/crates/core/src/auth/oauth/providers/discord.rs @@ -1,141 +1,101 @@ -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::providers::social::{ExternalUser, SocialSpec}; +use crate::config::proto::OAuthProviderId; -impl DiscordOAuthProvider { - const NAME: &'static str = "discord"; - const DISPLAY_NAME: &'static str = "Discord"; +pub(crate) struct 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"; - - 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, - }); - } +// 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, - 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)?)) - }), - } - } + // discriminator: Option, + username: Option, + avatar: Option, } -#[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<&'static 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 { - return Err(AuthError::Unauthorized); - } +impl TryFrom for ExternalUser { + type Error = AuthError; + 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()), // (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(Self { + // 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: OAuthProviderId::Discord, - email: user.email, + email: Some(user.email), username: user.username, verified: user.verified, - avatar, }); } } + +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::*; + 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 730e5875f..303801336 100644 --- a/crates/core/src/auth/oauth/providers/facebook.rs +++ b/crates/core/src/auth/oauth/providers/facebook.rs @@ -1,39 +1,40 @@ -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::providers::social::{DataEnvelope, ExternalUser, SocialSpec}; +use crate::config::proto::OAuthProviderId; -#[derive(Default, Deserialize, Debug)] -struct FacebookUserPictureData { - url: String, -} +pub(crate) struct Facebook; #[derive(Default, Deserialize, Debug)] -struct FacebookUserPicture { - data: FacebookUserPictureData, +struct FacebookPicture { + url: String, } #[derive(Default, Deserialize, Debug)] -struct FacebookUser { +pub(crate) struct FacebookUser { id: String, email: String, // name: Option, - picture: Option, + picture: Option>, } -pub(crate) struct FacebookOAuthProvider { - client_id: String, - client_secret: String, +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 FacebookOAuthProvider { +impl SocialSpec for Facebook { + const ID: OAuthProviderId = OAuthProviderId::Facebook; const NAME: &'static str = "facebook"; const DISPLAY_NAME: &'static str = "Facebook"; @@ -42,92 +43,43 @@ 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, - }); - } + const SCOPES: &'static [&'static str] = &["email"]; - 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)?)) - }), - } - } + type User = FacebookUser; } -#[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 - } - - 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(), - }); +#[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") + ); } - fn oauth_scopes(&self) -> Vec<&'static str> { - return vec!["email"]; - } + #[tokio::test] + async fn test_facebook_user_without_picture() { + let user = resolve_user::(serde_json::json!({ + "id": "1234", + "email": "user@example.com", + })) + .await + .unwrap(); - 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()))?; - - return Ok(OAuthUser { - provider_user_id: user.id, - provider_id: OAuthProviderId::Facebook, - email: user.email, - username: None, - verified: true, - avatar: user.picture.map(|p| p.data.url), - }); + 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 6275778ce..4476e1412 100644 --- a/crates/core/src/auth/oauth/providers/github.rs +++ b/crates/core/src/auth/oauth/providers/github.rs @@ -1,166 +1,172 @@ 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::providers::client::UserApi; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; +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 { - const 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"; - // const DEVICE_AUTH_URL: &'static str = "https://github.com/login/device/code"; - const USER_API_URL: &'static str = "https://api.github.com/user"; +impl TryFrom for ExternalUser { + type Error = AuthError; - 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(), - )); + 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 { - client_id, - client_secret, + provider_user_id: user.id.to_string(), + email: Some(email), + username: user.login, + verified: true, + avatar: user.avatar_url, }); } +} - 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)?)) - }), - } - } +#[derive(Default, Deserialize, Debug)] +struct GithubEmail { + email: String, + primary: bool, + verified: bool, + // NOTE: null | "private" | "public" + // 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 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 +impl SocialSpec for Github { + const ID: OAuthProviderId = OAuthProviderId::Github; + const 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"; + // const DEVICE_AUTH_URL: &'static str = "https://github.com/login/device/code"; + const USER_API_URL: &'static str = "https://api.github.com/user"; + + const SCOPES: &'static [&'static str] = &["read:user", "user:email"]; + + type User = GithubUser; + + 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())]; } - 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"); + /// 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(OAuthClientSettings { - auth_url: AUTH_URL.clone(), - token_url: TOKEN_URL.clone(), - client_id: self.client_id.clone(), - client_secret: self.client_secret.clone(), - }); + return user.try_into(); } +} - fn oauth_scopes(&self) -> Vec<&'static str> { - return vec!["read:user", "user:email"]; +#[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")); } - 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(), - )); - } + /// 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(); - // 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, + assert_eq!(user.email.as_deref(), Some("primary@github.com")); } + } - 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()))?; - - // 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 { - #[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 Some(primary) = emails - .into_iter() - .find(|cand| cand.verified && cand.primary) - else { - return Err(AuthError::FailedDependency("missing email".into())); - }; - - primary.email - }; + #[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:?}" + ); + } - return Ok(OAuthUser { - provider_user_id: user.id.to_string(), - provider_id: OAuthProviderId::Github, - email, - username: user.login, - verified: true, - avatar: user.avatar_url, - }); + 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 ad81bc497..bc8ed6047 100644 --- a/crates/core/src/auth/oauth/providers/gitlab.rs +++ b/crates/core/src/auth/oauth/providers/gitlab.rs @@ -1,21 +1,40 @@ -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::providers::social::{ExternalUser, SocialSpec}; +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 TryFrom for ExternalUser { + type Error = AuthError; -pub(crate) struct GitlabOAuthProvider { - client_id: String, - client_secret: String, + 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 GitlabOAuthProvider { +impl SocialSpec for Gitlab { + const ID: OAuthProviderId = OAuthProviderId::Gitlab; const NAME: &'static str = "gitlab"; const DISPLAY_NAME: &'static str = "GitLab"; @@ -23,105 +42,43 @@ 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(), - )); - }; + const SCOPES: &'static [&'static str] = &["read_user"]; - 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)?)) - }), - } - } + type User = GitlabUser; } -#[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 - } - - 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"); - } - - 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<&'static str> { - return vec!["read_user"]; +#[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); } - 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(), - )); - } + #[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; - 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()))?; - let verified = user.state == "active"; - if !verified { - return Err(AuthError::Unauthorized); - } - - return Ok(OAuthUser { - provider_user_id: user.id.to_string(), - provider_id: OAuthProviderId::Gitlab, - email: user.email, - username: user.username, - verified, - avatar: user.avatar_url, - }); + 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 66f59f9c9..4b4b4efac 100644 --- a/crates/core/src/auth/oauth/providers/google.rs +++ b/crates/core/src/auth/oauth/providers/google.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}; +use crate::auth::oauth::providers::social::{ExternalUser, SocialSpec}; +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 TryFrom for ExternalUser { + type Error = AuthError; -pub(crate) struct GoogleOAuthProvider { - client_id: String, - client_secret: String, + 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 GoogleOAuthProvider { +impl SocialSpec for Google { + const ID: OAuthProviderId = OAuthProviderId::Google; const NAME: &'static str = "google"; const DISPLAY_NAME: &'static str = "Google"; @@ -23,105 +38,48 @@ 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(), - )); - }; + const SCOPES: &'static [&'static str] = &[ + "https://www.googleapis.com/auth/userinfo.profile", + "https://www.googleapis.com/auth/userinfo.email", + ]; - 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)?)) - }), - } - } + type User = GoogleUser; } -#[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 - } - - 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"); - } - - 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<&'static str> { - return vec![ - "https://www.googleapis.com/auth/userinfo.profile", - "https://www.googleapis.com/auth/userinfo.email", - ]; +#[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") + ); } - 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(), - )); - } + #[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; - 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()))?; - if !user.verified_email { - return Err(AuthError::Unauthorized); - } - - return Ok(OAuthUser { - provider_user_id: user.id, - provider_id: OAuthProviderId::Google, - email: user.email, - username: None, - verified: user.verified_email, - avatar: user.picture, - }); + assert!(matches!(result, Err(AuthError::Unauthorized)), "{result:?}"); } } diff --git a/crates/core/src/auth/oauth/provider.rs b/crates/core/src/auth/oauth/providers/interface.rs similarity index 77% rename from crates/core/src/auth/oauth/provider.rs rename to crates/core/src/auth/oauth/providers/interface.rs index a8332c5e4..d826050ce 100644 --- a/crates/core/src/auth/oauth/provider.rs +++ b/crates/core/src/auth/oauth/providers/interface.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, @@ -64,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 { @@ -113,7 +114,17 @@ 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>; + + /// 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, @@ -128,25 +139,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/microsoft.rs b/crates/core/src/auth/oauth/providers/microsoft.rs index 8a64d4715..4ba68898e 100644 --- a/crates/core/src/auth/oauth/providers/microsoft.rs +++ b/crates/core/src/auth/oauth/providers/microsoft.rs @@ -1,28 +1,34 @@ -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::providers::social::{ExternalUser, SocialSpec}; +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 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 MicrosoftOAuthProvider { +impl SocialSpec for Microsoft { + const ID: OAuthProviderId = OAuthProviderId::Microsoft; const NAME: &'static str = "microsoft"; const DISPLAY_NAME: &'static str = "Microsoft"; @@ -30,94 +36,28 @@ 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(), - )); - }; + const SCOPES: &'static [&'static str] = &["User.Read"]; - 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)?)) - }), - } - } + type User = MicrosoftUser; } -#[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"); - } - - 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<&'static 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()))?; - - return Ok(OAuthUser { - provider_user_id: user.id, - provider_id: OAuthProviderId::Microsoft, - email: user.mail, - // username: Some(user.displayName), - username: None, - verified: true, - avatar: None, - }); +#[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..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; @@ -16,6 +32,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 +61,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/oidc.rs b/crates/core/src/auth/oauth/providers/oidc.rs index 830285752..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}; @@ -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, @@ -80,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; } @@ -96,28 +103,16 @@ 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 { - 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 new file mode 100644 index 000000000..cf94d7943 --- /dev/null +++ b/crates/core/src/auth/oauth/providers/social.rs @@ -0,0 +1,239 @@ +use async_trait::async_trait; +use oauth2::AuthType; +use serde::Deserialize; +use serde::de::DeserializeOwned; +use std::marker::PhantomData; + +use crate::auth::AuthError; +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}; + +/// 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. + /// + /// 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)> { + 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; + } + + /// Resolves the authenticated user. + /// + /// 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 + Self: Sized, + { + return SocialProvider::::factory(); + } +} + +/// 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 +/// [`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, +} + +pub(crate) struct SocialProvider { + client: ProviderClient, + + // 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, +} + +impl SocialProvider { + fn new(config: &OAuthProviderConfig) -> Result { + return Ok(Self { + client: ProviderClient::new(config, S::DISPLAY_NAME, S::AUTH_URL, S::TOKEN_URL)?, + 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) -> &str { + return S::NAME; + } + fn display_name(&self) -> &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(self.client.settings()); + } + + fn recover_token_response(&self, body: &[u8]) -> Option> { + return S::recover_token_response(body); + } + + async fn get_user(&self, token_response: &TokenResponse) -> Result { + 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?; + + // 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 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, + }); + } +} + +/// 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)] +pub(crate) use testing::{USER_API_TEST_PATH, resolve_user, resolve_user_against}; + +#[cfg(test)] +mod testing { + use axum::Json; + use axum::routing::{Router, get}; + use axum_test::{TestServer, TestServerConfig}; + + use super::*; + 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"; + + /// 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 config = OAuthProviderConfig { + client_id: Some("client_id".to_string()), + client_secret: Some("client_secret".to_string()), + ..Default::default() + }; + + let provider = SocialProvider:: { + 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, + }; + + 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; + } +} diff --git a/crates/core/src/auth/oauth/providers/test.rs b/crates/core/src/auth/oauth/providers/test.rs index 0bfceb400..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 { @@ -68,33 +65,18 @@ impl OAuthProvider for TestOAuthProvider { }); } - fn oauth_scopes(&self) -> Vec<&'static str> { + fn oauth_scopes(&self) -> Vec<&str> { return vec!["identity", "email", "preferences"]; } 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, 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..73057e829 100644 --- a/crates/core/src/auth/oauth/providers/twitch.rs +++ b/crates/core/src/auth/oauth/providers/twitch.rs @@ -1,143 +1,29 @@ -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::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 TwitchOAuthProvider { - client_id: String, - client_secret: String, -} - -impl TwitchOAuthProvider { - const NAME: &'static str = "twitch"; - const DISPLAY_NAME: &'static str = "Twitch"; - - const AUTH_URL: &'static str = "https://id.twitch.tv/oauth2/authorize"; - const TOKEN_URL: &'static str = "https://id.twitch.tv/oauth2/token"; - const USER_API_URL: &'static str = "https://api.twitch.tv/helix/users"; +use crate::auth::oauth::providers::interface::TokenResponse; +use crate::auth::oauth::providers::social::{DataEnvelope, ExternalUser, SocialSpec}; +use crate::config::proto::OAuthProviderId; - 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(), - )); - }; +pub(crate) struct Twitch; - 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)?)) - }), - } - } +// Reference: https://dev.twitch.tv/docs/api/reference#get-users +#[derive(Default, Deserialize, Debug)] +pub(crate) struct TwitchUser { + id: String, + // According to reference above, email is implicitly verified. + email: String, + login: Option, + // display_name: String, + profile_image_url: Option, } -#[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"); - } +impl TryFrom>> for ExternalUser { + type Error = AuthError; - 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 auth_type(&self) -> oauth2::AuthType { - return oauth2::AuthType::RequestBody; - } - - fn oauth_scopes(&self) -> Vec<&'static str> { - return vec!["user:read:email"]; - } - - 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); - } - - 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), + 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(), @@ -150,10 +36,9 @@ impl OAuthProvider for TwitchOAuthProvider { } }; - return Ok(OAuthUser { + return Ok(Self { 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, @@ -161,6 +46,32 @@ impl OAuthProvider for TwitchOAuthProvider { } } +impl SocialSpec for Twitch { + const ID: OAuthProviderId = OAuthProviderId::Twitch; + const NAME: &'static str = "twitch"; + const DISPLAY_NAME: &'static str = "Twitch"; + + const AUTH_URL: &'static str = "https://id.twitch.tv/oauth2/authorize"; + const TOKEN_URL: &'static str = "https://id.twitch.tv/oauth2/token"; + const USER_API_URL: &'static str = "https://api.twitch.tv/helix/users"; + + const SCOPES: &'static [&'static str] = &["user:read:email"]; + + const AUTH_TYPE: oauth2::AuthType = oauth2::AuthType::RequestBody; + + type User = DataEnvelope>; + + fn user_api_headers(client_id: &str) -> Vec<(&'static str, String)> { + return vec![("Client-Id", client_id.to_string())]; + } + + 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)); + } +} + #[derive(Clone, Debug, Deserialize, Serialize)] pub struct TwitchTokenResponse { access_token: String, @@ -191,22 +102,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 +121,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 +135,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 05a724827..ebbef024e 100644 --- a/crates/core/src/auth/oauth/providers/yandex.rs +++ b/crates/core/src/auth/oauth/providers/yandex.rs @@ -1,21 +1,46 @@ -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::providers::social::{ExternalUser, SocialSpec}; +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 TryFrom for ExternalUser { + type Error = AuthError; -pub(crate) struct YandexOAuthProvider { - client_id: String, - client_secret: String, + fn try_from(user: YandexUser) -> Result { + return Ok(Self { + provider_user_id: user.id, + email: Some(user.default_email), + username: user.login, + verified: true, + // 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 + ) + }), + }); + } } -impl YandexOAuthProvider { +impl SocialSpec for Yandex { + const ID: OAuthProviderId = OAuthProviderId::Yandex; const NAME: &'static str = "yandex"; const DISPLAY_NAME: &'static str = "Yandex"; @@ -23,114 +48,48 @@ 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(), - )); - }; + const SCOPES: &'static [&'static str] = &["login:email", "login:avatar", "login:info"]; - 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)?)) - }), - } - } + type User = YandexUser; } -#[async_trait] -impl OAuthProvider for YandexOAuthProvider { - fn name(&self) -> &'static str { - return Self::NAME; - } - - fn provider(&self) -> OAuthProviderId { - return OAuthProviderId::Yandex; +#[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") + ); } - 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<&'static 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()))?; - - 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 { - provider_user_id: user.id, - provider_id: OAuthProviderId::Yandex, - email: user.default_email, - username: user.login, - verified: true, - avatar, - }); + #[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); } } 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(""));