refactor: allow testing of mocked providers via AuthRegistry creation, avoid creation of responses in auth

Cargo.lock

@@ -3108,6 +3108,7 @@ dependencies = [
  "bon",
  "bytes 1.10.1",
  "chrono",
+ "cookie",
  "dashmap",
  "dotenvy",
  "figment",

pacman-server/Cargo.toml

@@ -64,6 +64,7 @@ axum-test = "18.1.0"
 pretty_assertions = "1.4.1"
 testcontainers = "0.25.0"
 bon = "3.7.2"
+cookie = "0.18.1"
 
 [lints.rust]
 unexpected_cfgs = { level = "warn", check-cfg = [

pacman-server/src/auth/discord.rs

@@ -1,5 +1,3 @@
-use axum::{response::IntoResponse, response::Redirect};
-use axum_cookie::CookieManager;
 use jsonwebtoken::EncodingKey;
 use oauth2::{AuthorizationCode, CsrfToken, PkceCodeVerifier, Scope, TokenResponse};
 use serde::{Deserialize, Serialize};
@@ -7,7 +5,7 @@ use serde::{Deserialize, Serialize};
 use std::sync::Arc;
 use tracing::{trace, warn};
 
-use crate::auth::provider::{AuthUser, OAuthProvider};
+use crate::auth::provider::{AuthUser, AuthorizeInfo, OAuthProvider};
 use crate::errors::ErrorResponse;
 use crate::session;
 
@@ -65,7 +63,7 @@ impl OAuthProvider for DiscordProvider {
         "Discord"
     }
 
-    async fn authorize(&self, cookie: &CookieManager, encoding_key: &EncodingKey) -> axum::response::Response {
+    async fn authorize(&self, encoding_key: &EncodingKey) -> Result<AuthorizeInfo, ErrorResponse> {
         let (pkce_challenge, pkce_verifier) = oauth2::PkceCodeChallenge::new_random_sha256();
         let (authorize_url, csrf_state) = self
             .client
@@ -77,10 +75,12 @@ impl OAuthProvider for DiscordProvider {
 
         // Store PKCE verifier and CSRF state in session
         let session_token = session::create_pkce_session(pkce_verifier.secret(), csrf_state.secret(), encoding_key);
-        session::set_session_cookie(cookie, &session_token);
 
         trace!(state = %csrf_state.secret(), "Generated OAuth authorization URL");
-        Redirect::to(authorize_url.as_str()).into_response()
+        Ok(AuthorizeInfo {
+            authorize_url,
+            session_token,
+        })
     }
 
     async fn exchange_code_for_token(&self, code: &str, verifier: &str) -> Result<String, ErrorResponse> {

pacman-server/src/auth/github.rs

@@ -1,5 +1,3 @@
-use axum::{response::IntoResponse, response::Redirect};
-use axum_cookie::CookieManager;
 use jsonwebtoken::EncodingKey;
 use oauth2::{AuthorizationCode, CsrfToken, PkceCodeVerifier, Scope, TokenResponse};
 use serde::{Deserialize, Serialize};
@@ -8,7 +6,7 @@ use std::sync::Arc;
 use tracing::{trace, warn};
 
 use crate::{
-    auth::provider::{AuthUser, OAuthProvider},
+    auth::provider::{AuthUser, AuthorizeInfo, OAuthProvider},
     errors::ErrorResponse,
     session,
 };
@@ -73,7 +71,7 @@ impl OAuthProvider for GitHubProvider {
         "GitHub"
     }
 
-    async fn authorize(&self, cookie: &CookieManager, encoding_key: &EncodingKey) -> axum::response::Response {
+    async fn authorize(&self, encoding_key: &EncodingKey) -> Result<AuthorizeInfo, ErrorResponse> {
         let (pkce_challenge, pkce_verifier) = oauth2::PkceCodeChallenge::new_random_sha256();
         let (authorize_url, csrf_state) = self
             .client
@@ -85,10 +83,12 @@ impl OAuthProvider for GitHubProvider {
 
         // Store PKCE verifier and CSRF state in session
         let session_token = session::create_pkce_session(pkce_verifier.secret(), csrf_state.secret(), encoding_key);
-        session::set_session_cookie(cookie, &session_token);
 
         trace!(state = %csrf_state.secret(), "Generated OAuth authorization URL");
-        Redirect::to(authorize_url.as_str()).into_response()
+        Ok(AuthorizeInfo {
+            authorize_url,
+            session_token,
+        })
     }
 
     async fn exchange_code_for_token(&self, code: &str, verifier: &str) -> Result<String, ErrorResponse> {

pacman-server/src/auth/mod.rs

@@ -17,7 +17,7 @@ type OAuthClient =
     BasicClient<oauth2::EndpointSet, oauth2::EndpointNotSet, oauth2::EndpointNotSet, oauth2::EndpointNotSet, oauth2::EndpointSet>;
 
 pub struct AuthRegistry {
-    providers: HashMap<&'static str, Arc<dyn provider::OAuthProvider>>,
+    pub providers: HashMap<&'static str, Arc<dyn provider::OAuthProvider>>,
 }
 
 impl AuthRegistry {

pacman-server/src/auth/provider.rs

@@ -24,13 +24,22 @@ pub struct AuthUser {
     pub avatar_url: Option<String>,
 }
 
+// Information required to begin an OAuth authorization flow.
+#[derive(Debug)]
+pub struct AuthorizeInfo {
+    // The URL to redirect the user to for authorization.
+    pub authorize_url: oauth2::url::Url,
+    // A session token to be stored in the user's session cookie.
+    pub session_token: String,
+}
+
 #[automock]
 #[async_trait]
 pub trait OAuthProvider: Send + Sync {
-    // Builds a server response to redirect the user to the provider's authorization page.
+    // Builds the necessary information to redirect the user to the provider's authorization page.
     // This generally also includes beginning a PKCE flow (proof key for code exchange).
-    // The cookie manager is used to store the PKCE verifier in the session.
-    async fn authorize(&self, cookie: &CookieManager, encoding_key: &EncodingKey) -> axum::response::Response;
+    // The returned session token should be stored in the user's session cookie.
+    async fn authorize(&self, encoding_key: &EncodingKey) -> Result<AuthorizeInfo, ErrorResponse>;
 
     // Handles the callback from the provider after the user has authorized the app.
     // This generally also includes completing the PKCE flow (proof key for code exchange).

pacman-server/src/routes.rs

@@ -46,9 +46,14 @@ pub async fn oauth_authorize_handler(
                 .build(),
         );
     }
-    let resp = prov.authorize(&cookie, &app_state.jwt_encoding_key).await;
+    let auth_info = match prov.authorize(&app_state.jwt_encoding_key).await {
+        Ok(info) => info,
+        Err(e) => return e.into_response(),
+    };
+
+    session::set_session_cookie(&cookie, &auth_info.session_token);
     trace!("Redirecting to provider authorization page");
-    resp
+    Redirect::to(auth_info.authorize_url.as_str()).into_response()
 }
 
 pub async fn oauth_callback_handler(

pacman-server/tests/common/mod.rs

@@ -26,7 +26,7 @@ pub struct TestContext {
 }
 
 #[builder]
-pub async fn test_context(use_database: bool) -> TestContext {
+pub async fn test_context(use_database: bool, auth_registry: Option<AuthRegistry>) -> TestContext {
     CRYPTO_INIT.call_once(|| {
         rustls::crypto::ring::default_provider()
             .install_default()
@@ -74,7 +74,7 @@ pub async fn test_context(use_database: bool) -> TestContext {
     };
 
     // Create auth registry
-    let auth = AuthRegistry::new(&config).expect("Failed to create auth registry");
+    let auth = auth_registry.unwrap_or_else(|| AuthRegistry::new(&config).expect("Failed to create auth registry"));
 
     // Create app state
     let notify = Arc::new(Notify::new());

pacman-server/tests/oauth.rs

@@ -1,3 +1,9 @@
+use std::{collections::HashMap, sync::Arc};
+
+use pacman_server::auth::{
+    provider::{MockOAuthProvider, OAuthProvider},
+    AuthRegistry,
+};
 use pretty_assertions::assert_eq;
 
 mod common;
@@ -34,17 +40,51 @@ async fn test_oauth_callback_handling() {
 /// Test OAuth authorization flow
 #[tokio::test]
 async fn test_oauth_authorization_flow() {
-    let TestContext { server, .. } = test_context().use_database(false).call().await;
+    let mut mock = MockOAuthProvider::new();
+    mock.expect_authorize().returning(|_| {
+        Ok(pacman_server::auth::provider::AuthorizeInfo {
+            authorize_url: "https://example.com".parse().unwrap(),
+            session_token: "a_token".to_string(),
+        })
+    });
 
-    // TODO: Test that the OAuth authorize handler redirects to the provider's authorization page for valid providers
-    // TODO: Test that the OAuth authorize handler returns an error for unknown providers
-    // TODO: Test that the OAuth authorize handler sets a link cookie when the link parameter is true
+    let provider: Arc<dyn OAuthProvider> = Arc::new(mock);
+    let mock_registry = AuthRegistry {
+        providers: HashMap::from([("mock", provider)]),
+    };
+
+    let TestContext { server, .. } = test_context().use_database(false).auth_registry(mock_registry).call().await;
+
+    // Test that valid handlers redirect
+    let response = server.get("/auth/mock").await;
+    assert_eq!(response.status_code(), 303); // Redirect to GitHub OAuth
+
+    // Test that unknown handlers return an error
+    let response = server.get("/auth/unknown").await;
+    assert_eq!(response.status_code(), 400); // Bad request for unknown provider
+
+    // Test that session cookie is set
+    let response = server.get("/auth/mock").await;
+    assert_eq!(response.status_code(), 303);
+    let cookies = {
+        let cookies = response.cookies();
+        cookies.iter().cloned().collect::<Vec<_>>()
+    };
+    assert_eq!(cookies.len(), 1);
+    assert_eq!(cookies[0].name(), "session");
+    assert_eq!(cookies[0].value(), "a_token");
+
+    // Test that link parameter redirects and sets a link cookie
+    let response = server.get("/auth/mock?link=true").await;
+    assert_eq!(response.status_code(), 303);
+    assert_eq!(response.maybe_cookie("link").is_some(), true);
+    assert_eq!(response.maybe_cookie("link").unwrap().value(), "1");
 }
 
 /// Test OAuth callback validation
 #[tokio::test]
 async fn test_oauth_callback_validation() {
-    let TestContext { server, .. } = test_context().use_database(false).call().await;
+    let TestContext { server: _server, .. } = test_context().use_database(false).call().await;
 
     // TODO: Test that the OAuth callback handler validates the provider exists before processing
     // TODO: Test that the OAuth callback handler returns an error when the provider returns an OAuth error
@@ -55,7 +95,7 @@ async fn test_oauth_callback_validation() {
 /// Test OAuth callback processing
 #[tokio::test]
 async fn test_oauth_callback_processing() {
-    let TestContext { server, .. } = test_context().use_database(false).call().await;
+    let TestContext { server: _server, .. } = test_context().use_database(false).call().await;
 
     // TODO: Test that the OAuth callback handler exchanges the authorization code for user information successfully
     // TODO: Test that the OAuth callback handler handles provider callback errors gracefully
@@ -67,7 +107,7 @@ async fn test_oauth_callback_processing() {
 /// Test account linking flow
 #[tokio::test]
 async fn test_account_linking_flow() {
-    let TestContext { server, .. } = test_context().use_database(false).call().await;
+    let TestContext { server: _server, .. } = test_context().use_database(false).call().await;
 
     // TODO: Test that the OAuth callback handler links a new provider to an existing user when link intent is present and session is valid
     // TODO: Test that the OAuth callback handler redirects to profile after successful account linking
@@ -77,7 +117,7 @@ async fn test_account_linking_flow() {
 /// Test new user registration
 #[tokio::test]
 async fn test_new_user_registration() {
-    let TestContext { server, .. } = test_context().use_database(false).call().await;
+    let TestContext { server: _server, .. } = test_context().use_database(false).call().await;
 
     // TODO: Test that the OAuth callback handler creates a new user account when no existing user is found
     // TODO: Test that the OAuth callback handler requires an email address for all sign-ins
@@ -87,7 +127,7 @@ async fn test_new_user_registration() {
 /// Test existing user sign-in
 #[tokio::test]
 async fn test_existing_user_sign_in() {
-    let TestContext { server, .. } = test_context().use_database(false).call().await;
+    let TestContext { server: _server, .. } = test_context().use_database(false).call().await;
 
     // TODO: Test that the OAuth callback handler allows sign-in when the provider is already linked to an existing user
     // TODO: Test that the OAuth callback handler requires explicit linking when a user with the same email exists and has other providers linked
@@ -97,7 +137,7 @@ async fn test_existing_user_sign_in() {
 /// Test avatar processing
 #[tokio::test]
 async fn test_avatar_processing() {
-    let TestContext { server, .. } = test_context().use_database(false).call().await;
+    let TestContext { server: _server, .. } = test_context().use_database(false).call().await;
 
     // TODO: Test that the OAuth callback handler processes user avatars asynchronously without blocking the response
     // TODO: Test that the OAuth callback handler handles avatar processing errors gracefully
@@ -106,7 +146,7 @@ async fn test_avatar_processing() {
 /// Test profile access
 #[tokio::test]
 async fn test_profile_access() {
-    let TestContext { server, .. } = test_context().use_database(false).call().await;
+    let TestContext { server: _server, .. } = test_context().use_database(false).call().await;
 
     // TODO: Test that the profile handler returns user information when a valid session exists
     // TODO: Test that the profile handler returns an error when no session cookie is present
@@ -118,7 +158,7 @@ async fn test_profile_access() {
 /// Test logout functionality
 #[tokio::test]
 async fn test_logout_functionality() {
-    let TestContext { server, .. } = test_context().use_database(false).call().await;
+    let TestContext { server: _server, .. } = test_context().use_database(false).call().await;
 
     // TODO: Test that the logout handler clears the session if a session was there
     // TODO: Test that the logout handler removes the session from memory storage
@@ -129,7 +169,7 @@ async fn test_logout_functionality() {
 /// Test provider configuration
 #[tokio::test]
 async fn test_provider_configuration() {
-    let TestContext { server, .. } = test_context().use_database(false).call().await;
+    let TestContext { server: _server, .. } = test_context().use_database(false).call().await;
 
     // TODO: Test that the providers list handler returns all configured OAuth providers
     // TODO: Test that the providers list handler includes provider status (active/inactive)