From c6b4a2b141a399f410cd94c0e590ee25f4389d89 Mon Sep 17 00:00:00 2001 From: ConnorYoh <40631091+ConnorYoh@users.noreply.github.com> Date: Thu, 4 Dec 2025 17:48:19 +0000 Subject: [PATCH 1/6] Desktop to match normal login screens (#5122)1 Also fixed issue with csrf Also fixed issue with rust keychain --------- Co-authored-by: James Brunton --- .../common/model/ApplicationProperties.java | 1 - .../common/service/PostHogService.java | 5 +- .../software/SPDF/config/InitialSetup.java | 16 +- .../controller/api/SettingsController.java | 7 - .../web/ReactRoutingController.java | 10 +- .../src/main/resources/settings.yml.template | 1 - .../configuration/SecurityConfiguration.java | 50 +--- .../public/locales/en-GB/translation.toml | 9 + frontend/src-tauri/Cargo.lock | 19 +- frontend/src-tauri/Cargo.toml | 2 +- frontend/src-tauri/src/commands/auth.rs | 67 ++--- .../SetupWizard/DesktopAuthLayout.tsx | 72 ++++++ .../SetupWizard/DesktopOAuthButtons.tsx | 110 +++++++++ .../components/SetupWizard/LoginForm.tsx | 225 ----------------- .../components/SetupWizard/ModeSelection.tsx | 72 ------ .../SetupWizard/SaaSLoginScreen.tsx | 95 ++++++++ .../components/SetupWizard/SelfHostedLink.tsx | 25 ++ .../SetupWizard/SelfHostedLoginScreen.tsx | 105 ++++++++ .../SetupWizard/ServerSelection.tsx | 91 ++++++- .../SetupWizard/ServerSelectionScreen.tsx | 34 +++ .../components/SetupWizard/SetupWizard.css | 23 -- .../desktop/components/SetupWizard/index.tsx | 230 ++++++++---------- frontend/src/desktop/services/apiClient.ts | 2 +- .../src/desktop/services/apiClientSetup.ts | 14 +- frontend/src/desktop/services/authService.ts | 74 ++++-- .../desktop/services/connectionModeService.ts | 1 + .../src/desktop/services/tauriHttpClient.ts | 7 +- .../configSections/AdminSecuritySection.tsx | 19 -- 28 files changed, 779 insertions(+), 607 deletions(-) create mode 100644 frontend/src/desktop/components/SetupWizard/DesktopAuthLayout.tsx create mode 100644 frontend/src/desktop/components/SetupWizard/DesktopOAuthButtons.tsx delete mode 100644 frontend/src/desktop/components/SetupWizard/LoginForm.tsx delete mode 100644 frontend/src/desktop/components/SetupWizard/ModeSelection.tsx create mode 100644 frontend/src/desktop/components/SetupWizard/SaaSLoginScreen.tsx create mode 100644 frontend/src/desktop/components/SetupWizard/SelfHostedLink.tsx create mode 100644 frontend/src/desktop/components/SetupWizard/SelfHostedLoginScreen.tsx create mode 100644 frontend/src/desktop/components/SetupWizard/ServerSelectionScreen.tsx delete mode 100644 frontend/src/desktop/components/SetupWizard/SetupWizard.css diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index 6a6ee8453b..f6afa62ea4 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -112,7 +112,6 @@ public class ApplicationProperties { @Data public static class Security { private Boolean enableLogin; - private Boolean csrfDisabled; private InitialLogin initialLogin = new InitialLogin(); private OAUTH2 oauth2 = new OAUTH2(); private SAML2 saml2 = new SAML2(); diff --git a/app/common/src/main/java/stirling/software/common/service/PostHogService.java b/app/common/src/main/java/stirling/software/common/service/PostHogService.java index 310fc43ab2..786c04a437 100644 --- a/app/common/src/main/java/stirling/software/common/service/PostHogService.java +++ b/app/common/src/main/java/stirling/software/common/service/PostHogService.java @@ -254,10 +254,7 @@ public class PostHogService { properties, "security_enableLogin", applicationProperties.getSecurity().getEnableLogin()); - addIfNotEmpty( - properties, - "security_csrfDisabled", - applicationProperties.getSecurity().getCsrfDisabled()); + addIfNotEmpty(properties, "security_csrfDisabled", true); addIfNotEmpty( properties, "security_loginAttemptCount", diff --git a/app/core/src/main/java/stirling/software/SPDF/config/InitialSetup.java b/app/core/src/main/java/stirling/software/SPDF/config/InitialSetup.java index 0a63a6f486..ef592cb550 100644 --- a/app/core/src/main/java/stirling/software/SPDF/config/InitialSetup.java +++ b/app/core/src/main/java/stirling/software/SPDF/config/InitialSetup.java @@ -34,7 +34,6 @@ public class InitialSetup { public void init() throws IOException { initUUIDKey(); initSecretKey(); - initEnableCSRFSecurity(); initLegalUrls(); initSetAppVersion(); GeneralUtils.extractPipeline(); @@ -59,19 +58,6 @@ public class InitialSetup { applicationProperties.getAutomaticallyGenerated().setKey(secretKey); } } - - public void initEnableCSRFSecurity() throws IOException { - if (GeneralUtils.isVersionHigher( - "0.46.0", applicationProperties.getAutomaticallyGenerated().getAppVersion())) { - Boolean csrf = applicationProperties.getSecurity().getCsrfDisabled(); - if (!csrf) { - GeneralUtils.saveKeyToSettings("security.csrfDisabled", false); - GeneralUtils.saveKeyToSettings("system.enableAnalytics", true); - applicationProperties.getSecurity().setCsrfDisabled(false); - } - } - } - public void initLegalUrls() throws IOException { // Initialize Terms and Conditions String termsUrl = applicationProperties.getLegal().getTermsAndConditions(); @@ -95,7 +81,7 @@ public class InitialSetup { isNewServer = existingVersion == null || existingVersion.isEmpty() - || existingVersion.equals("0.0.0"); + || "0.0.0".equals(existingVersion); String appVersion = "0.0.0"; Resource resource = new ClassPathResource("version.properties"); diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/SettingsController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/SettingsController.java index 9657d8f150..1d9f63818b 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/SettingsController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/SettingsController.java @@ -124,7 +124,6 @@ public class SettingsController { ApplicationProperties.Security security = applicationProperties.getSecurity(); settings.put("enableLogin", security.getEnableLogin()); - settings.put("csrfDisabled", security.getCsrfDisabled()); settings.put("loginMethod", security.getLoginMethod()); settings.put("loginAttemptCount", security.getLoginAttemptCount()); settings.put("loginResetTimeMinutes", security.getLoginResetTimeMinutes()); @@ -159,12 +158,6 @@ public class SettingsController { .getSecurity() .setEnableLogin((Boolean) settings.get("enableLogin")); } - if (settings.containsKey("csrfDisabled")) { - GeneralUtils.saveKeyToSettings("security.csrfDisabled", settings.get("csrfDisabled")); - applicationProperties - .getSecurity() - .setCsrfDisabled((Boolean) settings.get("csrfDisabled")); - } if (settings.containsKey("loginMethod")) { GeneralUtils.saveKeyToSettings("security.loginMethod", settings.get("loginMethod")); applicationProperties diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/web/ReactRoutingController.java b/app/core/src/main/java/stirling/software/SPDF/controller/web/ReactRoutingController.java index 7741220f24..6373e07520 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/web/ReactRoutingController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/web/ReactRoutingController.java @@ -4,8 +4,6 @@ import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; -import jakarta.annotation.PostConstruct; - import org.springframework.beans.factory.annotation.Value; import org.springframework.core.io.ClassPathResource; import org.springframework.http.MediaType; @@ -13,6 +11,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.GetMapping; +import jakarta.annotation.PostConstruct; import jakarta.servlet.http.HttpServletRequest; @Controller @@ -63,9 +62,10 @@ public class ReactRoutingController { } } - @GetMapping(value = {"/", "/index.html"}, produces = MediaType.TEXT_HTML_VALUE) - public ResponseEntity serveIndexHtml(HttpServletRequest request) - throws IOException { + @GetMapping( + value = {"/", "/index.html"}, + produces = MediaType.TEXT_HTML_VALUE) + public ResponseEntity serveIndexHtml(HttpServletRequest request) throws IOException { if (indexHtmlExists && cachedIndexHtml != null) { return ResponseEntity.ok().contentType(MediaType.TEXT_HTML).body(cachedIndexHtml); } diff --git a/app/core/src/main/resources/settings.yml.template b/app/core/src/main/resources/settings.yml.template index 5ea28f71e8..4c2ec003e3 100644 --- a/app/core/src/main/resources/settings.yml.template +++ b/app/core/src/main/resources/settings.yml.template @@ -12,7 +12,6 @@ security: enableLogin: true # set to 'true' to enable login - csrfDisabled: false # set to 'true' to disable CSRF protection (not recommended for production) loginAttemptCount: 5 # lock user account after 5 tries; when using e.g. Fail2Ban you can deactivate the function with -1 loginResetTimeMinutes: 120 # lock account for 2 hours after x attempts loginMethod: all # Accepts values like 'all' and 'normal'(only Login with Username/Password), 'oauth2'(only Login with OAuth2) or 'saml2'(only Login with SAML2) diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java index ab1e4934d8..2a0cd57348 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java @@ -1,7 +1,6 @@ package stirling.software.proprietary.security.configuration; import java.util.List; -import java.util.Optional; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; @@ -25,8 +24,6 @@ import org.springframework.security.saml2.provider.service.web.authentication.Op import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.authentication.rememberme.PersistentTokenRepository; -import org.springframework.security.web.csrf.CookieCsrfTokenRepository; -import org.springframework.security.web.csrf.CsrfTokenRequestAttributeHandler; import org.springframework.security.web.savedrequest.NullRequestCache; import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import org.springframework.web.cors.CorsConfiguration; @@ -47,7 +44,6 @@ import stirling.software.proprietary.security.database.repository.PersistentLogi import stirling.software.proprietary.security.filter.IPRateLimitingFilter; import stirling.software.proprietary.security.filter.JwtAuthenticationFilter; import stirling.software.proprietary.security.filter.UserAuthenticationFilter; -import stirling.software.proprietary.security.model.User; import stirling.software.proprietary.security.oauth2.CustomOAuth2AuthenticationFailureHandler; import stirling.software.proprietary.security.oauth2.CustomOAuth2AuthenticationSuccessHandler; import stirling.software.proprietary.security.saml2.CustomSaml2AuthenticationFailureHandler; @@ -198,9 +194,7 @@ public class SecurityConfiguration { http.cors(cors -> cors.disable()); } - if (securityProperties.getCsrfDisabled() || !loginEnabledValue) { - http.csrf(CsrfConfigurer::disable); - } + http.csrf(CsrfConfigurer::disable); if (loginEnabledValue) { boolean v2Enabled = appConfig.v2Enabled(); @@ -210,48 +204,6 @@ public class SecurityConfiguration { .addFilterBefore(rateLimitingFilter, UsernamePasswordAuthenticationFilter.class) .addFilterBefore(jwtAuthenticationFilter, UserAuthenticationFilter.class); - if (!securityProperties.getCsrfDisabled()) { - CookieCsrfTokenRepository cookieRepo = - CookieCsrfTokenRepository.withHttpOnlyFalse(); - CsrfTokenRequestAttributeHandler requestHandler = - new CsrfTokenRequestAttributeHandler(); - requestHandler.setCsrfRequestAttributeName(null); - http.csrf( - csrf -> - csrf.ignoringRequestMatchers( - request -> { - String uri = request.getRequestURI(); - - // Ignore CSRF for auth endpoints - if (uri.startsWith("/api/v1/auth/")) { - return true; - } - - String apiKey = request.getHeader("X-API-KEY"); - // If there's no API key, don't ignore CSRF - // (return false) - if (apiKey == null || apiKey.trim().isEmpty()) { - return false; - } - // Validate API key using existing UserService - try { - Optional user = - userService.getUserByApiKey(apiKey); - // If API key is valid, ignore CSRF (return - // true) - // If API key is invalid, don't ignore CSRF - // (return false) - return user.isPresent(); - } catch (Exception e) { - // If there's any error validating the API - // key, don't ignore CSRF - return false; - } - }) - .csrfTokenRepository(cookieRepo) - .csrfTokenRequestHandler(requestHandler)); - } - http.sessionManagement( sessionManagement -> { if (v2Enabled) { diff --git a/frontend/public/locales/en-GB/translation.toml b/frontend/public/locales/en-GB/translation.toml index b68a380d64..c60df3a850 100644 --- a/frontend/public/locales/en-GB/translation.toml +++ b/frontend/public/locales/en-GB/translation.toml @@ -5901,6 +5901,7 @@ subtitle = "Sign in with your Stirling account" [setup.selfhosted] title = "Sign in to Server" subtitle = "Enter your server credentials" +link = "or connect to a self-hosted account" [setup.server] title = "Connect to Server" @@ -5919,6 +5920,14 @@ description = "Enter the full URL of your self-hosted Stirling PDF server" emptyUrl = "Please enter a server URL" unreachable = "Could not connect to server" testFailed = "Connection test failed" +configFetch = "Failed to fetch server configuration. Please check the URL and try again." + +[setup.server.error.securityDisabled] +title = "Login Not Enabled" +body = "This server does not have login enabled. To connect to this server, you must enable authentication:" +step1 = "Set DOCKER_ENABLE_SECURITY=true in your environment" +step2 = "Or set security.enableLogin=true in settings.yml" +step3 = "Restart the server" [setup.login] title = "Sign In" diff --git a/frontend/src-tauri/Cargo.lock b/frontend/src-tauri/Cargo.lock index 9d2395e2de..9719752dc8 100644 --- a/frontend/src-tauri/Cargo.lock +++ b/frontend/src-tauri/Cargo.lock @@ -2152,7 +2152,11 @@ version = "3.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eebcc3aff044e5944a8fbaf69eb277d11986064cba30c468730e8b9909fb551c" dependencies = [ + "byteorder", "log", + "security-framework 2.11.1", + "security-framework 3.5.1", + "windows-sys 0.60.2", "zeroize", ] @@ -2378,7 +2382,7 @@ dependencies = [ "openssl-probe", "openssl-sys", "schannel", - "security-framework", + "security-framework 2.11.1", "security-framework-sys", "tempfile", ] @@ -3841,6 +3845,19 @@ dependencies = [ "security-framework-sys", ] +[[package]] +name = "security-framework" +version = "3.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b3297343eaf830f66ede390ea39da1d462b6b0c1b000f420d0a83f898bbbe6ef" +dependencies = [ + "bitflags 2.10.0", + "core-foundation 0.10.1", + "core-foundation-sys", + "libc", + "security-framework-sys", +] + [[package]] name = "security-framework-sys" version = "2.15.0" diff --git a/frontend/src-tauri/Cargo.toml b/frontend/src-tauri/Cargo.toml index 6884bd1788..dc84ad8a23 100644 --- a/frontend/src-tauri/Cargo.toml +++ b/frontend/src-tauri/Cargo.toml @@ -32,7 +32,7 @@ tauri-plugin-http = "2.4.4" tauri-plugin-single-instance = "2.0.1" tauri-plugin-store = "2.1.0" tauri-plugin-opener = "2.0.0" -keyring = "3.6.1" +keyring = { version = "3.6.1", features = ["apple-native", "windows-native"] } tokio = { version = "1.0", features = ["time", "sync"] } reqwest = { version = "0.11", features = ["json"] } tiny_http = "0.12" diff --git a/frontend/src-tauri/src/commands/auth.rs b/frontend/src-tauri/src/commands/auth.rs index 30ec0d6c40..3e75b452ec 100644 --- a/frontend/src-tauri/src/commands/auth.rs +++ b/frontend/src-tauri/src/commands/auth.rs @@ -1,4 +1,4 @@ -use keyring::Entry; +use keyring::{Entry}; use serde::{Deserialize, Serialize}; use std::sync::{Arc, Mutex}; use tauri::AppHandle; @@ -21,53 +21,70 @@ pub struct UserInfo { } fn get_keyring_entry() -> Result { - Entry::new(KEYRING_SERVICE, KEYRING_TOKEN_KEY) - .map_err(|e| format!("Failed to access keyring: {}", e)) + log::debug!("Creating keyring entry with service='{}' username='{}'", KEYRING_SERVICE, KEYRING_TOKEN_KEY); + let entry = Entry::new(KEYRING_SERVICE, KEYRING_TOKEN_KEY) + .map_err(|e| { + log::error!("Failed to create keyring entry: {}", e); + format!("Failed to access keyring: {}", e) + })?; + log::debug!("Keyring entry created successfully"); + Ok(entry) } #[tauri::command] pub async fn save_auth_token(_app_handle: AppHandle, token: String) -> Result<(), String> { - log::info!("Saving auth token to keyring"); + if token.is_empty() { + log::warn!("Attempted to save empty auth token"); + return Err("Token cannot be empty".to_string()); + } let entry = get_keyring_entry()?; entry .set_password(&token) - .map_err(|e| format!("Failed to save token to keyring: {}", e))?; + .map_err(|e| { + log::error!("Failed to set password in keyring: {}", e); + format!("Failed to save token to keyring: {}", e) + })?; + + // Verify the save worked + match entry.get_password() { + Ok(retrieved_token) => { + if retrieved_token != token { + log::error!("Token verification failed: Retrieved token doesn't match"); + return Err("Token verification failed after save".to_string()); + } + } + Err(e) => { + log::error!("Token verification failed: {}", e); + return Err(format!("Token verification failed: {}", e)); + } + } - log::info!("Auth token saved successfully"); Ok(()) } #[tauri::command] pub async fn get_auth_token(_app_handle: AppHandle) -> Result, String> { - log::debug!("Retrieving auth token from keyring"); - let entry = get_keyring_entry()?; match entry.get_password() { Ok(token) => Ok(Some(token)), Err(keyring::Error::NoEntry) => Ok(None), - Err(e) => Err(format!("Failed to retrieve token: {}", e)), + Err(e) => { + log::error!("Failed to retrieve token from keyring: {}", e); + Err(format!("Failed to retrieve token: {}", e)) + }, } } #[tauri::command] pub async fn clear_auth_token(_app_handle: AppHandle) -> Result<(), String> { - log::info!("Clearing auth token from keyring"); - let entry = get_keyring_entry()?; // Delete the token - ignore error if it doesn't exist match entry.delete_credential() { - Ok(_) => { - log::info!("Auth token cleared successfully"); - Ok(()) - } - Err(keyring::Error::NoEntry) => { - log::info!("Auth token was already cleared"); - Ok(()) - } + Ok(_) | Err(keyring::Error::NoEntry) => Ok(()), Err(e) => Err(format!("Failed to clear token: {}", e)), } } @@ -78,8 +95,6 @@ pub async fn save_user_info( username: String, email: Option, ) -> Result<(), String> { - log::info!("Saving user info for: {}", username); - let user_info = UserInfo { username, email }; let store = app_handle @@ -96,7 +111,6 @@ pub async fn save_user_info( .save() .map_err(|e| format!("Failed to save store: {}", e))?; - log::info!("User info saved successfully"); Ok(()) } @@ -117,8 +131,6 @@ pub async fn get_user_info(app_handle: AppHandle) -> Result, St #[tauri::command] pub async fn clear_user_info(app_handle: AppHandle) -> Result<(), String> { - log::info!("Clearing user info"); - let store = app_handle .store(STORE_FILE) .map_err(|e| format!("Failed to access store: {}", e))?; @@ -129,7 +141,6 @@ pub async fn clear_user_info(app_handle: AppHandle) -> Result<(), String> { .save() .map_err(|e| format!("Failed to save store: {}", e))?; - log::info!("User info cleared successfully"); Ok(()) } @@ -186,12 +197,8 @@ pub async fn login( supabase_key: String, saas_server_url: String, ) -> Result { - log::info!("Login attempt for user: {} to server: {}", username, server_url); - // Detect if this is Supabase (SaaS) or Spring Boot (self-hosted) - // Compare against the configured SaaS server URL let is_supabase = server_url.trim_end_matches('/') == saas_server_url.trim_end_matches('/'); - log::info!("Authentication type: {}", if is_supabase { "Supabase (SaaS)" } else { "Spring Boot (Self-hosted)" }); // Create HTTP client let client = reqwest::Client::new(); @@ -248,8 +255,6 @@ pub async fn login( .or_else(|| email.clone()) .unwrap_or_else(|| username); - log::info!("Supabase login successful for user: {}", username); - Ok(LoginResponse { token: login_response.access_token, username, diff --git a/frontend/src/desktop/components/SetupWizard/DesktopAuthLayout.tsx b/frontend/src/desktop/components/SetupWizard/DesktopAuthLayout.tsx new file mode 100644 index 0000000000..92558857b4 --- /dev/null +++ b/frontend/src/desktop/components/SetupWizard/DesktopAuthLayout.tsx @@ -0,0 +1,72 @@ +import React, { useEffect, useMemo, useRef, useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import LoginRightCarousel from '@app/components/shared/LoginRightCarousel'; +import buildLoginSlides from '@app/components/shared/loginSlides'; +import styles from '@app/routes/authShared/AuthLayout.module.css'; +import { useLogoVariant } from '@app/hooks/useLogoVariant'; + +interface DesktopAuthLayoutProps { + children: React.ReactNode; +} + +export const DesktopAuthLayout: React.FC = ({ children }) => { + const { t } = useTranslation(); + const cardRef = useRef(null); + const [hideRightPanel, setHideRightPanel] = useState(false); + const logoVariant = useLogoVariant(); + const imageSlides = useMemo(() => buildLoginSlides(logoVariant, t), [logoVariant, t]); + + // Force light mode on auth pages + useEffect(() => { + const htmlElement = document.documentElement; + const previousColorScheme = htmlElement.getAttribute('data-mantine-color-scheme'); + + // Set light mode + htmlElement.setAttribute('data-mantine-color-scheme', 'light'); + + // Cleanup: restore previous theme when leaving auth pages + return () => { + if (previousColorScheme) { + htmlElement.setAttribute('data-mantine-color-scheme', previousColorScheme); + } + }; + }, []); + + useEffect(() => { + const update = () => { + // Use viewport to avoid hysteresis when the card is already in single-column mode + const viewportWidth = window.innerWidth; + const viewportHeight = window.innerHeight; + const cardWidthIfTwoCols = Math.min(1180, viewportWidth * 0.96); // matches min(73.75rem, 96vw) + const columnWidth = cardWidthIfTwoCols / 2; + const tooNarrow = columnWidth < 470; + const tooShort = viewportHeight < 740; + setHideRightPanel(tooNarrow || tooShort); + }; + update(); + window.addEventListener('resize', update); + window.addEventListener('orientationchange', update); + return () => { + window.removeEventListener('resize', update); + window.removeEventListener('orientationchange', update); + }; + }, []); + + return ( +
+
+
+
+ {children} +
+
+ {!hideRightPanel && ( + + )} +
+
+ ); +}; diff --git a/frontend/src/desktop/components/SetupWizard/DesktopOAuthButtons.tsx b/frontend/src/desktop/components/SetupWizard/DesktopOAuthButtons.tsx new file mode 100644 index 0000000000..49b55a3869 --- /dev/null +++ b/frontend/src/desktop/components/SetupWizard/DesktopOAuthButtons.tsx @@ -0,0 +1,110 @@ +import React, { useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { authService, UserInfo } from '@app/services/authService'; +import { buildOAuthCallbackHtml } from '@app/utils/oauthCallbackHtml'; +import { BASE_PATH } from '@app/constants/app'; +import '@app/routes/authShared/auth.css'; + +export type OAuthProvider = 'google' | 'github' | 'keycloak' | 'azure' | 'apple' | 'oidc'; + +interface DesktopOAuthButtonsProps { + onOAuthSuccess: (userInfo: UserInfo) => Promise; + onError: (error: string) => void; + isDisabled: boolean; + serverUrl: string; + providers: OAuthProvider[]; +} + +export const DesktopOAuthButtons: React.FC = ({ + onOAuthSuccess, + onError, + isDisabled, + serverUrl, + providers, +}) => { + const { t } = useTranslation(); + const [oauthLoading, setOauthLoading] = useState(false); + + const handleOAuthLogin = async (provider: OAuthProvider) => { + // Prevent concurrent OAuth attempts + if (oauthLoading || isDisabled) { + return; + } + + try { + setOauthLoading(true); + + // Build callback page HTML with translations and dark mode support + const successHtml = buildOAuthCallbackHtml({ + title: t('oauth.success.title', 'Authentication Successful'), + message: t('oauth.success.message', 'You can close this window and return to Stirling PDF.'), + isError: false, + }); + + const errorHtml = buildOAuthCallbackHtml({ + title: t('oauth.error.title', 'Authentication Failed'), + message: t('oauth.error.message', 'Authentication was not successful. You can close this window and try again.'), + isError: true, + errorPlaceholder: true, // {error} will be replaced by Rust + }); + + const userInfo = await authService.loginWithOAuth(provider, serverUrl, successHtml, errorHtml); + + // Call the onOAuthSuccess callback to complete setup + await onOAuthSuccess(userInfo); + } catch (error) { + console.error('OAuth login failed:', error); + + const errorMessage = error instanceof Error + ? error.message + : t('setup.login.error.oauthFailed', 'OAuth login failed. Please try again.'); + + onError(errorMessage); + setOauthLoading(false); + } + }; + + const providerConfig: Record = { + google: { label: 'Google', file: 'google.svg' }, + github: { label: 'GitHub', file: 'github.svg' }, + keycloak: { label: 'Keycloak', file: 'keycloak.svg' }, + azure: { label: 'Microsoft', file: 'microsoft.svg' }, + apple: { label: 'Apple', file: 'apple.svg' }, + oidc: { label: 'OpenID', file: 'oidc.svg' }, + }; + + if (providers.length === 0) { + return null; + } + + return ( +
+ {providers + .filter((providerId) => providerId in providerConfig) + .map((providerId) => { + const provider = providerConfig[providerId]; + return ( + + ); + })} + {oauthLoading && ( +

+ {t('setup.login.oauthPending', 'Opening browser for authentication...')} +

+ )} +
+ ); +}; diff --git a/frontend/src/desktop/components/SetupWizard/LoginForm.tsx b/frontend/src/desktop/components/SetupWizard/LoginForm.tsx deleted file mode 100644 index 4ad388822f..0000000000 --- a/frontend/src/desktop/components/SetupWizard/LoginForm.tsx +++ /dev/null @@ -1,225 +0,0 @@ -import React, { useState } from 'react'; -import { Stack, TextInput, PasswordInput, Button, Text, Divider, Group, Collapse, Anchor, Box } from '@mantine/core'; -import { useTranslation } from 'react-i18next'; -import { authService } from '@app/services/authService'; -import { STIRLING_SAAS_URL } from '@app/constants/connection'; -import { buildOAuthCallbackHtml } from '@app/utils/oauthCallbackHtml'; -import { BASE_PATH } from '@app/constants/app'; - -interface LoginFormProps { - serverUrl: string; - isSaaS?: boolean; - onLogin: (username: string, password: string) => Promise; - loading: boolean; -} - -export const LoginForm: React.FC = ({ serverUrl, isSaaS = false, onLogin, loading }) => { - const { t } = useTranslation(); - const [username, setUsername] = useState(''); - const [password, setPassword] = useState(''); - const [validationError, setValidationError] = useState(null); - const [oauthLoading, setOauthLoading] = useState(false); - const [showInstructions, setShowInstructions] = useState(false); - - const handleSubmit = async (e: React.FormEvent) => { - e.preventDefault(); - - // Validation - if (!username.trim()) { - setValidationError(isSaaS - ? t('setup.login.error.emptyEmail', 'Please enter your email') - : t('setup.login.error.emptyUsername', 'Please enter your username')); - return; - } - - if (!password) { - setValidationError(t('setup.login.error.emptyPassword', 'Please enter your password')); - return; - } - - setValidationError(null); - await onLogin(username.trim(), password); - }; - - const handleOAuthLogin = async (provider: 'google' | 'github') => { - // Prevent concurrent OAuth attempts - if (oauthLoading || loading) { - return; - } - - try { - setOauthLoading(true); - setValidationError(null); - - // For SaaS, use configured SaaS URL; for self-hosted, derive from serverUrl - const authServerUrl = isSaaS - ? STIRLING_SAAS_URL - : serverUrl; // Self-hosted might have its own auth - - // Build callback page HTML with translations and dark mode support - const successHtml = buildOAuthCallbackHtml({ - title: t('oauth.success.title', 'Authentication Successful'), - message: t('oauth.success.message', 'You can close this window and return to Stirling PDF.'), - isError: false, - }); - - const errorHtml = buildOAuthCallbackHtml({ - title: t('oauth.error.title', 'Authentication Failed'), - message: t('oauth.error.message', 'Authentication was not successful. You can close this window and try again.'), - isError: true, - errorPlaceholder: true, // {error} will be replaced by Rust - }); - - const userInfo = await authService.loginWithOAuth(provider, authServerUrl, successHtml, errorHtml); - - // Call the onLogin callback to complete setup (username/password not needed for OAuth) - await onLogin(userInfo.username, ''); - } catch (error) { - console.error('OAuth login failed:', error); - - const errorMessage = error instanceof Error - ? error.message - : t('setup.login.error.oauthFailed', 'OAuth login failed. Please try again.'); - - setValidationError(errorMessage); - setOauthLoading(false); - } - }; - - return ( -
- - - {t('setup.login.connectingTo', 'Connecting to:')} {isSaaS ? 'stirling.com' : serverUrl} - - - {/* Login requirement note for self-hosted servers */} - {!isSaaS && ( - - - {t('setup.login.serverRequirement', 'Note: The server must have login enabled.')}{' '} - setShowInstructions(!showInstructions)} - style={{ cursor: 'pointer' }} - > - {showInstructions - ? t('setup.login.hideInstructions', 'Hide instructions') - : t('setup.login.showInstructions', 'How to enable?')} - - - - - - - {t('setup.login.instructions', 'To enable login on your Stirling PDF server:')} - - - {t('setup.login.instructionsEnvVar', 'Set the environment variable:')} - - - SECURITY_ENABLELOGIN=true - - - {t('setup.login.instructionsOrYml', 'Or in settings.yml:')} - - - security.enableLogin: true - - - {t('setup.login.instructionsRestart', 'Then restart your server for the changes to take effect.')} - - - - - )} - - {/* OAuth Login Buttons - Only show for SaaS */} - {isSaaS && ( - <> - - - - - - - - {oauthLoading && ( - - {t('setup.login.oauthPending', 'Opening browser for authentication...')} - - )} - - - - - )} - - { - setUsername(e.target.value); - setValidationError(null); - }} - disabled={loading} - required - /> - - { - setPassword(e.target.value); - setValidationError(null); - }} - disabled={loading} - required - /> - - {validationError && ( - - {validationError} - - )} - - - -
- ); -}; diff --git a/frontend/src/desktop/components/SetupWizard/ModeSelection.tsx b/frontend/src/desktop/components/SetupWizard/ModeSelection.tsx deleted file mode 100644 index 8242fa5ed0..0000000000 --- a/frontend/src/desktop/components/SetupWizard/ModeSelection.tsx +++ /dev/null @@ -1,72 +0,0 @@ -import React from 'react'; -import { Stack, Button, Text } from '@mantine/core'; -import { useTranslation } from 'react-i18next'; -import CloudIcon from '@mui/icons-material/Cloud'; -import ComputerIcon from '@mui/icons-material/Computer'; - -interface ModeSelectionProps { - onSelect: (mode: 'saas' | 'selfhosted') => void; - loading: boolean; -} - -export const ModeSelection: React.FC = ({ onSelect, loading }) => { - const { t } = useTranslation(); - - return ( - - - - - - ); -}; diff --git a/frontend/src/desktop/components/SetupWizard/SaaSLoginScreen.tsx b/frontend/src/desktop/components/SetupWizard/SaaSLoginScreen.tsx new file mode 100644 index 0000000000..ab82bde1ed --- /dev/null +++ b/frontend/src/desktop/components/SetupWizard/SaaSLoginScreen.tsx @@ -0,0 +1,95 @@ +import React, { useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import LoginHeader from '@app/routes/login/LoginHeader'; +import ErrorMessage from '@app/routes/login/ErrorMessage'; +import EmailPasswordForm from '@app/routes/login/EmailPasswordForm'; +import DividerWithText from '@app/components/shared/DividerWithText'; +import { DesktopOAuthButtons } from '@app/components/SetupWizard/DesktopOAuthButtons'; +import { SelfHostedLink } from '@app/components/SetupWizard/SelfHostedLink'; +import { UserInfo } from '@app/services/authService'; +import '@app/routes/authShared/auth.css'; + +interface SaaSLoginScreenProps { + serverUrl: string; + onLogin: (username: string, password: string) => Promise; + onOAuthSuccess: (userInfo: UserInfo) => Promise; + onSelfHostedClick: () => void; + loading: boolean; + error: string | null; +} + +export const SaaSLoginScreen: React.FC = ({ + serverUrl, + onLogin, + onOAuthSuccess, + onSelfHostedClick, + loading, + error, +}) => { + const { t } = useTranslation(); + const [email, setEmail] = useState(''); + const [password, setPassword] = useState(''); + const [validationError, setValidationError] = useState(null); + + const handleEmailPasswordSubmit = async () => { + // Validation + if (!email.trim()) { + setValidationError(t('setup.login.error.emptyEmail', 'Please enter your email')); + return; + } + + if (!password) { + setValidationError(t('setup.login.error.emptyPassword', 'Please enter your password')); + return; + } + + setValidationError(null); + await onLogin(email.trim(), password); + }; + + const handleOAuthError = (errorMessage: string) => { + setValidationError(errorMessage); + }; + + const displayError = error || validationError; + + return ( + <> + + + + + + + + + { + setEmail(value); + setValidationError(null); + }} + setPassword={(value) => { + setPassword(value); + setValidationError(null); + }} + onSubmit={handleEmailPasswordSubmit} + isSubmitting={loading} + submitButtonText={t('setup.login.submit', 'Login')} + /> + + + + ); +}; diff --git a/frontend/src/desktop/components/SetupWizard/SelfHostedLink.tsx b/frontend/src/desktop/components/SetupWizard/SelfHostedLink.tsx new file mode 100644 index 0000000000..6a184cc584 --- /dev/null +++ b/frontend/src/desktop/components/SetupWizard/SelfHostedLink.tsx @@ -0,0 +1,25 @@ +import React from 'react'; +import { useTranslation } from 'react-i18next'; +import '@app/routes/authShared/auth.css'; + +interface SelfHostedLinkProps { + onClick: () => void; + disabled?: boolean; +} + +export const SelfHostedLink: React.FC = ({ onClick, disabled = false }) => { + const { t } = useTranslation(); + + return ( +
+ +
+ ); +}; diff --git a/frontend/src/desktop/components/SetupWizard/SelfHostedLoginScreen.tsx b/frontend/src/desktop/components/SetupWizard/SelfHostedLoginScreen.tsx new file mode 100644 index 0000000000..9a68b91561 --- /dev/null +++ b/frontend/src/desktop/components/SetupWizard/SelfHostedLoginScreen.tsx @@ -0,0 +1,105 @@ +import React, { useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { Text } from '@mantine/core'; +import LoginHeader from '@app/routes/login/LoginHeader'; +import ErrorMessage from '@app/routes/login/ErrorMessage'; +import EmailPasswordForm from '@app/routes/login/EmailPasswordForm'; +import DividerWithText from '@app/components/shared/DividerWithText'; +import { DesktopOAuthButtons, OAuthProvider } from '@app/components/SetupWizard/DesktopOAuthButtons'; +import { UserInfo } from '@app/services/authService'; +import '@app/routes/authShared/auth.css'; + +interface SelfHostedLoginScreenProps { + serverUrl: string; + enabledOAuthProviders?: string[]; + onLogin: (username: string, password: string) => Promise; + onOAuthSuccess: (userInfo: UserInfo) => Promise; + loading: boolean; + error: string | null; +} + +export const SelfHostedLoginScreen: React.FC = ({ + serverUrl, + enabledOAuthProviders, + onLogin, + onOAuthSuccess, + loading, + error, +}) => { + const { t } = useTranslation(); + const [username, setUsername] = useState(''); + const [password, setPassword] = useState(''); + const [validationError, setValidationError] = useState(null); + + const handleSubmit = async () => { + // Validation + if (!username.trim()) { + setValidationError(t('setup.login.error.emptyUsername', 'Please enter your username')); + return; + } + + if (!password) { + setValidationError(t('setup.login.error.emptyPassword', 'Please enter your password')); + return; + } + + setValidationError(null); + await onLogin(username.trim(), password); + }; + + const handleOAuthError = (errorMessage: string) => { + setValidationError(errorMessage); + }; + + const displayError = error || validationError; + + return ( + <> + + + + + + {t('setup.login.connectingTo', 'Connecting to:')} {serverUrl} + + + {/* Show OAuth buttons if providers are available */} + {enabledOAuthProviders && enabledOAuthProviders.length > 0 && ( + <> + + + + + )} + + { + setUsername(value); + setValidationError(null); + }} + setPassword={(value) => { + setPassword(value); + setValidationError(null); + }} + onSubmit={handleSubmit} + isSubmitting={loading} + submitButtonText={t('setup.login.submit', 'Login')} + /> + + ); +}; diff --git a/frontend/src/desktop/components/SetupWizard/ServerSelection.tsx b/frontend/src/desktop/components/SetupWizard/ServerSelection.tsx index 3ca5ea65b2..f8a0d4f9e1 100644 --- a/frontend/src/desktop/components/SetupWizard/ServerSelection.tsx +++ b/frontend/src/desktop/components/SetupWizard/ServerSelection.tsx @@ -1,8 +1,9 @@ import React, { useState } from 'react'; -import { Stack, Button, TextInput } from '@mantine/core'; +import { Stack, Button, TextInput, Alert, Text } from '@mantine/core'; import { useTranslation } from 'react-i18next'; import { ServerConfig } from '@app/services/connectionModeService'; import { connectionModeService } from '@app/services/connectionModeService'; +import LocalIcon from '@app/components/shared/LocalIcon'; interface ServerSelectionProps { onSelect: (config: ServerConfig) => void; @@ -14,11 +15,13 @@ export const ServerSelection: React.FC = ({ onSelect, load const [customUrl, setCustomUrl] = useState(''); const [testing, setTesting] = useState(false); const [testError, setTestError] = useState(null); + const [securityDisabled, setSecurityDisabled] = useState(false); const handleSubmit = async (e: React.FormEvent) => { e.preventDefault(); - const url = customUrl.trim(); + // Normalize URL: trim and remove trailing slashes + const url = customUrl.trim().replace(/\/+$/, ''); if (!url) { setTestError(t('setup.server.error.emptyUrl', 'Please enter a server URL')); @@ -28,6 +31,7 @@ export const ServerSelection: React.FC = ({ onSelect, load // Test connection before proceeding setTesting(true); setTestError(null); + setSecurityDisabled(false); try { const isReachable = await connectionModeService.testConnection(url); @@ -38,9 +42,67 @@ export const ServerSelection: React.FC = ({ onSelect, load return; } - // Connection successful + // Fetch OAuth providers and check if login is enabled + let enabledProviders: string[] = []; + try { + const response = await fetch(`${url}/api/v1/proprietary/ui-data/login`); + + // Check if security is disabled (status 403 or error response) + if (!response.ok) { + if (response.status === 403 || response.status === 401) { + setSecurityDisabled(true); + setTesting(false); + return; + } + // Other error statuses - show generic error + setTestError( + t('setup.server.error.configFetch', 'Failed to fetch server configuration (status {{status}})', { + status: response.status + }) + ); + setTesting(false); + return; + } + + const data = await response.json(); + console.log('Login UI data:', data); + + // Check if the response indicates security is disabled + if (data.enableLogin === false || data.securityEnabled === false) { + setSecurityDisabled(true); + setTesting(false); + return; + } + + // Extract provider IDs from authorization URLs + // Example: "/oauth2/authorization/google" → "google" + enabledProviders = Object.keys(data.providerList || {}) + .map(key => key.split('/').pop()) + .filter((id): id is string => id !== undefined); + + console.log('[ServerSelection] Detected OAuth providers:', enabledProviders); + } catch (err) { + console.error('[ServerSelection] Failed to fetch login configuration', err); + + // Check if it's a security disabled error + if (err instanceof Error && (err.message.includes('403') || err.message.includes('401'))) { + setSecurityDisabled(true); + setTesting(false); + return; + } + + // For any other error (network, CORS, invalid JSON, etc.), show error and don't proceed + setTestError( + t('setup.server.error.configFetch', 'Failed to fetch server configuration. Please check the URL and try again.') + ); + setTesting(false); + return; + } + + // Connection successful - pass URL and OAuth providers onSelect({ url, + enabledOAuthProviders: enabledProviders.length > 0 ? enabledProviders : undefined, }); } catch (error) { console.error('Connection test failed:', error); @@ -64,6 +126,7 @@ export const ServerSelection: React.FC = ({ onSelect, load onChange={(e) => { setCustomUrl(e.target.value); setTestError(null); + setSecurityDisabled(false); }} disabled={loading || testing} error={testError} @@ -73,6 +136,28 @@ export const ServerSelection: React.FC = ({ onSelect, load )} /> + {securityDisabled && ( + } + title={t('setup.server.error.securityDisabled.title', 'Login Not Enabled')} + > + + + {t('setup.server.error.securityDisabled.body', 'This server does not have login enabled. To connect to this server, you must enable authentication:')} + + +
    +
  1. {t('setup.server.error.securityDisabled.step1', 'Set DOCKER_ENABLE_SECURITY=true in your environment')}
    2. +
  2. {t('setup.server.error.securityDisabled.step2', 'Or set security.enableLogin=true in settings.yml')}
    3. +
  3. {t('setup.server.error.securityDisabled.step3', 'Restart the server')}
    4. +
+ + + + )} + - )} - - - - + {/* Back Button */} + {activeStep > SetupStep.SaaSLogin && !loading && ( +
+ +
+ )} + ); }; diff --git a/frontend/src/desktop/services/apiClient.ts b/frontend/src/desktop/services/apiClient.ts index 8773afc5e4..257099c801 100644 --- a/frontend/src/desktop/services/apiClient.ts +++ b/frontend/src/desktop/services/apiClient.ts @@ -14,7 +14,7 @@ import { getApiBaseUrl } from '@app/services/apiClientConfig'; const apiClient = create({ baseURL: getApiBaseUrl(), responseType: 'json', - withCredentials: true, + withCredentials: false, // Desktop doesn't need credentials }); // Setup interceptors (desktop-specific auth and backend ready checks) diff --git a/frontend/src/desktop/services/apiClientSetup.ts b/frontend/src/desktop/services/apiClientSetup.ts index d01c0d9973..ee9cbcf55f 100644 --- a/frontend/src/desktop/services/apiClientSetup.ts +++ b/frontend/src/desktop/services/apiClientSetup.ts @@ -48,13 +48,21 @@ export function setupApiInterceptors(client: AxiosInstance): void { // Debug logging console.debug(`[apiClientSetup] Request to: ${extendedConfig.url}`); - // Add auth token for remote requests + // Add auth token for remote requests and enable credentials const isRemote = await operationRouter.isSelfHostedMode(); if (isRemote) { + // Self-hosted mode: enable credentials for session management + extendedConfig.withCredentials = true; + const token = await authService.getAuthToken(); if (token) { extendedConfig.headers.Authorization = `Bearer ${token}`; + } else { + console.warn('[apiClientSetup] Self-hosted mode but no auth token available'); } + } else { + // SaaS mode: disable credentials (security disabled on local backend) + extendedConfig.withCredentials = false; } // Backend readiness check (for local backend) @@ -85,7 +93,9 @@ export function setupApiInterceptors(client: AxiosInstance): void { // Response interceptor: Handle auth errors client.interceptors.response.use( - (response) => response, + (response) => { + return response; + }, async (error) => { const originalRequest = error.config as ExtendedRequestConfig; diff --git a/frontend/src/desktop/services/authService.ts b/frontend/src/desktop/services/authService.ts index 76f8aa1577..ed8be911ff 100644 --- a/frontend/src/desktop/services/authService.ts +++ b/frontend/src/desktop/services/authService.ts @@ -25,6 +25,7 @@ export class AuthService { private static instance: AuthService; private authStatus: AuthStatus = 'unauthenticated'; private userInfo: UserInfo | null = null; + private cachedToken: string | null = null; private authListeners = new Set<(status: AuthStatus, userInfo: UserInfo | null) => void>(); static getInstance(): AuthService { @@ -38,13 +39,32 @@ export class AuthService { * Save token to all storage locations and notify listeners */ private async saveTokenEverywhere(token: string): Promise { - // Save to Tauri store - await invoke('save_auth_token', { token }); - console.log('[Desktop AuthService] Token saved to Tauri store'); + // Validate token before caching + if (!token || token.trim().length === 0) { + console.warn('[Desktop AuthService] Attempted to save invalid/empty token'); + throw new Error('Invalid token'); + } - // Sync to localStorage for web layer - localStorage.setItem('stirling_jwt', token); - console.log('[Desktop AuthService] Token saved to localStorage'); + try { + // Save to Tauri store + await invoke('save_auth_token', { token }); + console.log('[Desktop AuthService] ✅ Token saved to Tauri store'); + } catch (error) { + console.error('[Desktop AuthService] ❌ Failed to save token to Tauri store:', error); + // Don't throw - we can still use localStorage + } + + try { + // Sync to localStorage for web layer + localStorage.setItem('stirling_jwt', token); + console.log('[Desktop AuthService] ✅ Token saved to localStorage'); + } catch (error) { + console.error('[Desktop AuthService] ❌ Failed to save token to localStorage:', error); + } + + // Cache the valid token in memory + this.cachedToken = token; + console.log('[Desktop AuthService] ✅ Token cached in memory'); // Notify other parts of the system window.dispatchEvent(new CustomEvent('jwt-available')); @@ -56,20 +76,25 @@ export class AuthService { */ private async getTokenFromAnySource(): Promise { // Try Tauri store first - console.log('[Desktop AuthService] Retrieving token from Tauri store...'); - const token = await invoke('get_auth_token'); + try { + const token = await invoke('get_auth_token'); - if (token) { - console.log(`[Desktop AuthService] Token found in Tauri store (length: ${token.length})`); - return token; + if (token) { + console.log(`[Desktop AuthService] ✅ Token found in Tauri store (length: ${token.length})`); + return token; + } + + console.log('[Desktop AuthService] ℹ️ No token in Tauri store, checking localStorage...'); + } catch (error) { + console.error('[Desktop AuthService] ❌ Failed to read from Tauri store:', error); } - console.log('[Desktop AuthService] No token in Tauri store'); - // Fallback to localStorage const localStorageToken = localStorage.getItem('stirling_jwt'); if (localStorageToken) { - console.log('[Desktop AuthService] Token found in localStorage (length:', localStorageToken.length, ')'); + console.log(`[Desktop AuthService] ✅ Token found in localStorage (length: ${localStorageToken.length})`); + } else { + console.log('[Desktop AuthService] ❌ No token found in any storage'); } return localStorageToken; @@ -79,6 +104,10 @@ export class AuthService { * Clear token from all storage locations */ private async clearTokenEverywhere(): Promise { + // Invalidate cache + this.cachedToken = null; + console.log('[Desktop AuthService] Cache invalidated'); + await invoke('clear_auth_token'); localStorage.removeItem('stirling_jwt'); } @@ -183,7 +212,22 @@ export class AuthService { async getAuthToken(): Promise { try { - return await this.getTokenFromAnySource(); + // Return cached token if available + if (this.cachedToken) { + console.debug('[Desktop AuthService] ✅ Returning cached token'); + return this.cachedToken; + } + + console.debug('[Desktop AuthService] Cache miss, fetching from storage...'); + const token = await this.getTokenFromAnySource(); + + // Cache the token if valid + if (token && token.trim().length > 0) { + this.cachedToken = token; + console.log('[Desktop AuthService] ✅ Token cached in memory after retrieval'); + } + + return token; } catch (error) { console.error('[Desktop AuthService] Failed to get auth token:', error); return null; diff --git a/frontend/src/desktop/services/connectionModeService.ts b/frontend/src/desktop/services/connectionModeService.ts index f01dbc40a6..cf454a50b3 100644 --- a/frontend/src/desktop/services/connectionModeService.ts +++ b/frontend/src/desktop/services/connectionModeService.ts @@ -5,6 +5,7 @@ export type ConnectionMode = 'saas' | 'selfhosted'; export interface ServerConfig { url: string; + enabledOAuthProviders?: string[]; } export interface ConnectionConfig { diff --git a/frontend/src/desktop/services/tauriHttpClient.ts b/frontend/src/desktop/services/tauriHttpClient.ts index 89c87bfbb1..b5bbceb930 100644 --- a/frontend/src/desktop/services/tauriHttpClient.ts +++ b/frontend/src/desktop/services/tauriHttpClient.ts @@ -61,7 +61,7 @@ class TauriHttpClient { headers: {}, timeout: 120000, responseType: 'json', - withCredentials: true, + withCredentials: false, // Desktop doesn't need credentials (backend has allowCredentials=false) }; public interceptors: Interceptors = { @@ -173,14 +173,15 @@ class TauriHttpClient { } try { - // Debug logging - console.debug(`[tauriHttpClient] Fetch request:`, { url, method }); + // Convert withCredentials to fetch API's credentials option + const credentials: RequestCredentials = finalConfig.withCredentials ? 'include' : 'omit'; // Make the request using Tauri's native HTTP client (standard Fetch API) const response = await fetch(url, { method, headers, body, + credentials, }); // Parse response based on responseType diff --git a/frontend/src/proprietary/components/shared/config/configSections/AdminSecuritySection.tsx b/frontend/src/proprietary/components/shared/config/configSections/AdminSecuritySection.tsx index 8934f4c457..19721b7600 100644 --- a/frontend/src/proprietary/components/shared/config/configSections/AdminSecuritySection.tsx +++ b/frontend/src/proprietary/components/shared/config/configSections/AdminSecuritySection.tsx @@ -13,7 +13,6 @@ import LoginRequiredBanner from '@app/components/shared/config/LoginRequiredBann interface SecuritySettingsData { enableLogin?: boolean; - csrfDisabled?: boolean; loginMethod?: string; loginAttemptCount?: number; loginResetTimeMinutes?: number; @@ -123,7 +122,6 @@ export default function AdminSecuritySection() { const deltaSettings: Record = { // Security settings 'security.enableLogin': securitySettings.enableLogin, - 'security.csrfDisabled': securitySettings.csrfDisabled, 'security.loginMethod': securitySettings.loginMethod, 'security.loginAttemptCount': securitySettings.loginAttemptCount, 'security.loginResetTimeMinutes': securitySettings.loginResetTimeMinutes, @@ -282,23 +280,6 @@ export default function AdminSecuritySection() { disabled={!loginEnabled} /> - -
-
- {t('admin.settings.security.csrfDisabled.label', 'Disable CSRF Protection')} - - {t('admin.settings.security.csrfDisabled.description', 'Disable Cross-Site Request Forgery protection (not recommended)')} - -
- - setSettings({ ...settings, csrfDisabled: e.target.checked })} - disabled={!loginEnabled} - /> - - -
From e7db714091af8ac62bf47e99047bca7b2247994f Mon Sep 17 00:00:00 2001 From: James Brunton Date: Thu, 4 Dec 2025 17:53:08 +0000 Subject: [PATCH 2/6] More fixes for automate (#5168) # Description of Changes Fix file missed in #5127 to use `apiClient` instead of `axios` directly Co-authored-by: ConnorYoh <40631091+ConnorYoh@users.noreply.github.com> --- frontend/src/core/utils/automationFileProcessor.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/src/core/utils/automationFileProcessor.ts b/frontend/src/core/utils/automationFileProcessor.ts index b07fe961be..311848bfd6 100644 --- a/frontend/src/core/utils/automationFileProcessor.ts +++ b/frontend/src/core/utils/automationFileProcessor.ts @@ -2,7 +2,7 @@ * File processing utilities specifically for automation workflows */ -import axios from 'axios'; +import apiClient from '@app/services/apiClient'; import { zipFileService } from '@app/services/zipFileService'; import { ResourceManager } from '@app/utils/resourceManager'; import { AUTOMATION_CONSTANTS } from '@app/constants/automation'; @@ -97,7 +97,7 @@ export class AutomationFileProcessor { options: AutomationProcessingOptions = {} ): Promise { try { - const response = await axios.post(endpoint, formData, { + const response = await apiClient.post(endpoint, formData, { responseType: options.responseType || 'blob', timeout: options.timeout || AUTOMATION_CONSTANTS.OPERATION_TIMEOUT }); @@ -139,7 +139,7 @@ export class AutomationFileProcessor { options: AutomationProcessingOptions = {} ): Promise { try { - const response = await axios.post(endpoint, formData, { + const response = await apiClient.post(endpoint, formData, { responseType: options.responseType || 'blob', timeout: options.timeout || AUTOMATION_CONSTANTS.OPERATION_TIMEOUT }); From 3a2370ea1f2feb9887e17afb2fced4acc198102c Mon Sep 17 00:00:00 2001 From: Keon Chen <66115421+keonchennl@users.noreply.github.com> Date: Thu, 4 Dec 2025 22:35:11 +0100 Subject: [PATCH 3/6] Update OCR setup guide link in LanguagePicker (#5162) # Description of Changes --- ## Checklist ### General - [ x] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [x ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md) (if applicable) - [ x] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ x] I have performed a self-review of my own code - [ x] My changes generate no new warnings ### Documentation - [x ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [x ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md#6-testing) for more details. --- frontend/src/core/components/tools/ocr/LanguagePicker.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/core/components/tools/ocr/LanguagePicker.tsx b/frontend/src/core/components/tools/ocr/LanguagePicker.tsx index 784d22da5c..7e023e9ccd 100644 --- a/frontend/src/core/components/tools/ocr/LanguagePicker.tsx +++ b/frontend/src/core/components/tools/ocr/LanguagePicker.tsx @@ -134,7 +134,7 @@ const LanguagePicker: React.FC = ({ textDecoration: 'underline', textAlign: 'center' }} - onClick={() => window.open('https://docs.stirlingpdf.com/Advanced%20Configuration/OCR', '_blank')} + onClick={() => window.open('https://docs.stirlingpdf.com/Configuration/OCR', '_blank')} > {t('ocr.languagePicker.viewSetupGuide', 'View setup guide →')} @@ -158,4 +158,4 @@ const LanguagePicker: React.FC = ({ ); }; -export default LanguagePicker; \ No newline at end of file +export default LanguagePicker; From 9fd8fd89ed26de1147e4ed21a585390658b0cfe4 Mon Sep 17 00:00:00 2001 From: EthanHealy01 <80844253+EthanHealy01@users.noreply.github.com> Date: Fri, 5 Dec 2025 13:18:23 +0000 Subject: [PATCH 4/6] add enum SERVER to list of valid licenses (#5172) --- frontend/src/core/hooks/useServerExperience.ts | 2 +- frontend/src/proprietary/contexts/ServerExperienceContext.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/core/hooks/useServerExperience.ts b/frontend/src/core/hooks/useServerExperience.ts index 8dd60069a7..28f62c1c57 100644 --- a/frontend/src/core/hooks/useServerExperience.ts +++ b/frontend/src/core/hooks/useServerExperience.ts @@ -65,7 +65,7 @@ export function useServerExperience(): ServerExperienceValue { const loginEnabled = config?.enableLogin !== false; const configIsAdmin = Boolean(config?.isAdmin); const effectiveIsAdmin = configIsAdmin || (!loginEnabled && selfReportedAdmin); - const hasPaidLicense = config?.license === 'PRO' || config?.license === 'ENTERPRISE'; + const hasPaidLicense = config?.license === 'SERVER' || config?.license === 'PRO' || config?.license === 'ENTERPRISE'; const setSelfReportedAdmin = useCallback((value: boolean) => { setSelfReportedAdminState(value); diff --git a/frontend/src/proprietary/contexts/ServerExperienceContext.tsx b/frontend/src/proprietary/contexts/ServerExperienceContext.tsx index 17ac572fc3..92ce8148a4 100644 --- a/frontend/src/proprietary/contexts/ServerExperienceContext.tsx +++ b/frontend/src/proprietary/contexts/ServerExperienceContext.tsx @@ -249,7 +249,7 @@ export function ServerExperienceProvider({ children }: { children: ReactNode }) }, [fetchUserCounts]); const hasPaidLicense = useMemo(() => { - return config?.license === 'PRO' || config?.license === 'ENTERPRISE'; + return config?.license === 'SERVER' || config?.license === 'PRO' || config?.license === 'ENTERPRISE'; }, [config?.license]); const licenseKeyValid = useMemo(() => { From 82dbcfbb9b78a01dee5a2754391a5444172b8875 Mon Sep 17 00:00:00 2001 From: Dario Ghunney Ware Date: Fri, 5 Dec 2025 23:19:41 +0000 Subject: [PATCH 5/6] SSO login fix (#5167) Fixes bug where SSO login with custom providers caused an `InvalidClientRegistrationIdException: Invalid Client Registration with Id: oidc` errors. Root Cause: - Backend: Redirect URI was hardcoded to `/login/oauth2/code/oidc` regardless of provider registration ID - Frontend: Unknown providers were mapped back to 'oidc' instead of using actual provider ID Closes #5141 --------- Co-authored-by: Anthony Stirling <77850077+frooodle@users.noreply.github.com> Co-authored-by: Keon Chen <66115421+keonchennl@users.noreply.github.com> --- ...tomOAuth2AuthenticationSuccessHandler.java | 8 + .../security/oauth2/OAuth2Configuration.java | 24 +- ...stomSaml2AuthenticationSuccessHandler.java | 6 + .../service/UserLicenseSettingsService.java | 71 ++++- .../oauth2/OAuth2ConfigurationTest.java | 162 ++++++++++ .../UserLicenseSettingsServiceTest.java | 218 +++++++++++++ frontend/src/proprietary/auth/oauthTypes.ts | 24 ++ .../src/proprietary/auth/springAuthClient.ts | 8 +- .../src/proprietary/routes/Login.test.tsx | 178 +++++++++-- frontend/src/proprietary/routes/Login.tsx | 21 +- .../routes/login/OAuthButtons.test.tsx | 291 ++++++++++++++++++ .../proprietary/routes/login/OAuthButtons.tsx | 5 +- 12 files changed, 961 insertions(+), 55 deletions(-) create mode 100644 app/proprietary/src/test/java/stirling/software/proprietary/security/oauth2/OAuth2ConfigurationTest.java create mode 100644 frontend/src/proprietary/auth/oauthTypes.ts create mode 100644 frontend/src/proprietary/routes/login/OAuthButtons.test.tsx diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java index e1e6703945..793c6b62fa 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java @@ -27,6 +27,7 @@ import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpSession; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; import stirling.software.common.model.ApplicationProperties; import stirling.software.common.model.exception.UnsupportedProviderException; @@ -39,6 +40,7 @@ import stirling.software.proprietary.security.service.JwtServiceInterface; import stirling.software.proprietary.security.service.LoginAttemptService; import stirling.software.proprietary.security.service.UserService; +@Slf4j @RequiredArgsConstructor public class CustomOAuth2AuthenticationSuccessHandler extends SavedRequestAwareAuthenticationSuccessHandler { @@ -77,12 +79,18 @@ public class CustomOAuth2AuthenticationSuccessHandler if (user != null && !licenseSettingsService.isOAuthEligible(user)) { // User is not grandfathered and no paid license - block OAuth login + log.warn( + "OAuth login blocked for existing user '{}' - not eligible (not grandfathered and no paid license)", + username); response.sendRedirect( request.getContextPath() + "/logout?oAuth2RequiresLicense=true"); return; } } else if (!licenseSettingsService.isOAuthEligible(null)) { // No existing user and no paid license -> block auto creation + log.warn( + "OAuth login blocked for new user '{}' - not eligible (no paid license for auto-creation)", + username); response.sendRedirect(request.getContextPath() + "/logout?oAuth2RequiresLicense=true"); return; } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/OAuth2Configuration.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/OAuth2Configuration.java index a053c1ead2..2d5f94620a 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/OAuth2Configuration.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/OAuth2Configuration.java @@ -67,10 +67,15 @@ public class OAuth2Configuration { keycloakClientRegistration().ifPresent(registrations::add); if (registrations.isEmpty()) { - log.error("No OAuth2 provider registered"); + log.error("No OAuth2 provider registered - check your OAuth2 configuration"); throw new NoProviderFoundException("At least one OAuth2 provider must be configured."); } + log.info( + "OAuth2 ClientRegistrationRepository created with {} provider(s): {}", + registrations.size(), + registrations.stream().map(ClientRegistration::getRegistrationId).toList()); + return new InMemoryClientRegistrationRepository(registrations); } @@ -165,7 +170,6 @@ public class OAuth2Configuration { githubClient.getUseAsUsername()); boolean isValid = validateProvider(github); - log.info("Initialised GitHub OAuth2 provider"); return isValid ? Optional.of( @@ -208,7 +212,19 @@ public class OAuth2Configuration { null, null); - return !isStringEmpty(oidcProvider.getIssuer()) || validateProvider(oidcProvider) + boolean isValid = + !isStringEmpty(oidcProvider.getIssuer()) || validateProvider(oidcProvider); + if (isValid) { + log.info( + "Initialised OIDC OAuth2 provider: registrationId='{}', issuer='{}', redirectUri='{}'", + name, + oauth.getIssuer(), + REDIRECT_URI_PATH + name); + } else { + log.warn("OIDC OAuth2 provider validation failed - provider will not be registered"); + } + + return isValid ? Optional.of( ClientRegistrations.fromIssuerLocation(oauth.getIssuer()) .registrationId(name) @@ -217,7 +233,7 @@ public class OAuth2Configuration { .scope(oidcProvider.getScopes()) .userNameAttributeName(oidcProvider.getUseAsUsername().getName()) .clientName(clientName) - .redirectUri(REDIRECT_URI_PATH + "oidc") + .redirectUri(REDIRECT_URI_PATH + name) .authorizationGrantType(AUTHORIZATION_CODE) .build()) : Optional.empty(); diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2AuthenticationSuccessHandler.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2AuthenticationSuccessHandler.java index b342fdcb46..e8bce579a0 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2AuthenticationSuccessHandler.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2AuthenticationSuccessHandler.java @@ -74,12 +74,18 @@ public class CustomSaml2AuthenticationSuccessHandler if (user != null && !licenseSettingsService.isSamlEligible(user)) { // User is not grandfathered and no ENTERPRISE license - block SAML login + log.warn( + "SAML2 login blocked for existing user '{}' - not eligible (not grandfathered and no ENTERPRISE license)", + username); response.sendRedirect( request.getContextPath() + "/logout?saml2RequiresLicense=true"); return; } } else if (!licenseSettingsService.isSamlEligible(null)) { // No existing user and no ENTERPRISE license -> block auto creation + log.warn( + "SAML2 login blocked for new user '{}' - not eligible (no ENTERPRISE license for auto-creation)", + username); response.sendRedirect( request.getContextPath() + "/logout?saml2RequiresLicense=true"); return; diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/service/UserLicenseSettingsService.java b/app/proprietary/src/main/java/stirling/software/proprietary/service/UserLicenseSettingsService.java index d3bade89c0..aa794e6997 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/service/UserLicenseSettingsService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/service/UserLicenseSettingsService.java @@ -21,6 +21,7 @@ import stirling.software.common.model.ApplicationProperties; import stirling.software.proprietary.model.UserLicenseSettings; import stirling.software.proprietary.security.configuration.ee.KeygenLicenseVerifier.License; import stirling.software.proprietary.security.configuration.ee.LicenseKeyChecker; +import stirling.software.proprietary.security.model.User; import stirling.software.proprietary.security.repository.UserLicenseSettingsRepository; import stirling.software.proprietary.security.service.UserService; @@ -331,28 +332,45 @@ public class UserLicenseSettingsService { } /** - * Checks if a user is eligible to use OAuth authentication. + * Checks if a user is eligible to use OAuth/SAML authentication. * *

A user is eligible if: * *

    *
  • They are grandfathered for OAuth (existing user before policy change), OR - *
  • The system has a paid license (SERVER or ENTERPRISE) + *
  • The system has an ENTERPRISE license (SSO is enterprise-only) *
* * @param user The user to check - * @return true if the user can use OAuth + * @return true if the user can use OAuth/SAML */ - public boolean isOAuthEligible(stirling.software.proprietary.security.model.User user) { + public boolean isOAuthEligible(User user) { + String username = (user != null) ? user.getUsername() : ""; + log.info("OAuth eligibility check for user: {}", username); + // Grandfathered users always have OAuth access if (user != null && user.isOauthGrandfathered()) { log.debug("User {} is grandfathered for OAuth", user.getUsername()); return true; } + // todo: remove + if (user != null) { + log.info( + "User {} is NOT grandfathered (isOauthGrandfathered={})", + username, + user.isOauthGrandfathered()); + } else { + log.info("New user attempting OAuth login - checking license requirement"); + } + // Users can use OAuth with SERVER or ENTERPRISE license boolean hasPaid = hasPaidLicense(); - log.debug("OAuth eligibility check: hasPaidLicense={}", hasPaid); + log.info( + "OAuth eligibility result: hasPaidLicense={}, user={}, eligible={}", + hasPaid, + username, + hasPaid); return hasPaid; } @@ -369,16 +387,32 @@ public class UserLicenseSettingsService { * @param user The user to check * @return true if the user can use SAML */ - public boolean isSamlEligible(stirling.software.proprietary.security.model.User user) { + public boolean isSamlEligible(User user) { + String username = (user != null) ? user.getUsername() : ""; + log.info("SAML2 eligibility check for user: {}", username); + // Grandfathered users always have SAML access if (user != null && user.isOauthGrandfathered()) { - log.debug("User {} is grandfathered for SAML", user.getUsername()); + log.info("User {} is grandfathered for SAML2 - ELIGIBLE", username); return true; } + if (user != null) { + log.info( + "User {} is NOT grandfathered (isOauthGrandfathered={})", + username, + user.isOauthGrandfathered()); + } else { + log.info("New user attempting SAML2 login - checking license requirement"); + } + // Users can use SAML only with ENTERPRISE license boolean hasEnterprise = hasEnterpriseLicense(); - log.debug("SAML eligibility check: hasEnterpriseLicense={}", hasEnterprise); + log.info( + "SAML2 eligibility result: hasEnterpriseLicense={}, user={}, eligible={}", + hasEnterprise, + username, + hasEnterprise); return hasEnterprise; } @@ -521,12 +555,17 @@ public class UserLicenseSettingsService { if (checker == null) { return false; } + License license = checker.getPremiumLicenseEnabledResult(); - return license == License.SERVER || license == License.ENTERPRISE; + boolean hasPaid = (license == License.SERVER || license == License.ENTERPRISE); + log.info("License check result: type={}, requiresPaid=true, hasPaid={}", license, hasPaid); + + return hasPaid; } /** - * Checks if the system has an ENTERPRISE license. Used for enterprise-only features like SAML. + * Checks if the system has an ENTERPRISE license. Used for enterprise-only features like SSO + * (OAuth/SAML). * * @return true if ENTERPRISE license is active */ @@ -535,7 +574,19 @@ public class UserLicenseSettingsService { if (checker == null) { return false; } + License license = checker.getPremiumLicenseEnabledResult(); + log.info( + "License check result: type={}, requiresEnterprise=true, hasEnterprise={}", + license, + (license == License.ENTERPRISE)); + + if (license != License.ENTERPRISE) { + log.warn( + "SAML2 requires ENTERPRISE license but found: {}. SAML2 login will be blocked.", + license); + } + return license == License.ENTERPRISE; } } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/oauth2/OAuth2ConfigurationTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/oauth2/OAuth2ConfigurationTest.java new file mode 100644 index 0000000000..750696b770 --- /dev/null +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/oauth2/OAuth2ConfigurationTest.java @@ -0,0 +1,162 @@ +package stirling.software.proprietary.security.oauth2; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import org.junit.jupiter.api.Test; + +/** + * Unit tests for OAuth2Configuration redirect URI logic. + * + *

These tests validate the critical fix for GitHub issue #5141: The redirect URI path segment + * MUST match the registration ID. Previously, the redirect URI was hardcoded to 'oidc', causing + * InvalidClientRegistrationIdException when custom provider names were used. + * + *

Note: These are conceptual tests documenting the expected behavior. Full integration testing + * with actual OIDC discovery would require: 1. Mock HTTP server for OIDC discovery endpoints 2. + * Valid OIDC configuration responses 3. Network mocking infrastructure + */ +class OAuth2ConfigurationTest { + + /** + * Tests the redirect URI pattern for OIDC provider configurations. + * + *

Critical behavior (GitHub issue #5141 fix): The redirect URI path segment MUST match the + * registration ID. For example: - Provider name: "authentik" → Redirect URI: + * "/login/oauth2/code/authentik" - Provider name: "mycompany" → Redirect URI: + * "/login/oauth2/code/mycompany" - Provider name: "oidc" → Redirect URI: + * "/login/oauth2/code/oidc" + * + *

Previously, the redirect URI was hardcoded to 'oidc', causing Spring Security to look for + * a registration with ID 'oidc' when the provider redirected back. This caused + * InvalidClientRegistrationIdException when custom provider names were used. + */ + @Test + void testRedirectUriPattern_usesProviderNameNotHardcodedOidc() { + // Verify the redirect URI pattern constant + String redirectUriBase = "{baseUrl}/login/oauth2/code/"; + + // Test cases: provider name → expected redirect URI + String[][] testCases = { + {"authentik", redirectUriBase + "authentik"}, + {"mycompany", redirectUriBase + "mycompany"}, + {"oidc", redirectUriBase + "oidc"}, + {"okta", redirectUriBase + "okta"}, + {"auth0", redirectUriBase + "auth0"} + }; + + for (String[] testCase : testCases) { + String providerName = testCase[0]; + String expectedRedirectUri = testCase[1]; + + // The fix ensures: .redirectUri(REDIRECT_URI_PATH + name) + // instead of: .redirectUri(REDIRECT_URI_PATH + "oidc") + String actualRedirectUri = redirectUriBase + providerName; + + assertEquals( + expectedRedirectUri, + actualRedirectUri, + String.format( + "Redirect URI for provider '%s' must use provider name, not hardcoded 'oidc'", + providerName)); + } + } + + /** + * Documents the critical fix for OAuth2 redirect URI mismatch. + * + *

This test validates the logic that was changed in OAuth2Configuration.java line 220: + * + * 

+     * // BEFORE (bug):
+     * .redirectUri(REDIRECT_URI_PATH + "oidc")  // Always "oidc"
+     *
+     * // AFTER (fix):
+     * .redirectUri(REDIRECT_URI_PATH + name)  // Dynamic provider name
+     *
+ */ + @Test + void testCriticalFix_redirectUriMatchesRegistrationId() { + // The redirect URI path segment extraction by Spring Security + String callbackUrl = "http://localhost:8080/login/oauth2/code/authentik?code=abc123"; + + // Spring extracts the path segment between "code/" and "?" + String extractedRegistrationId = extractRegistrationIdFromCallback(callbackUrl); + + // The extracted ID MUST match an actual registration ID + assertEquals("authentik", extractedRegistrationId); + + // If we had used hardcoded "oidc", the callback would be: + String buggyCallbackUrl = "http://localhost:8080/login/oauth2/code/oidc?code=abc123"; + String buggyExtractedId = extractRegistrationIdFromCallback(buggyCallbackUrl); + + // This would look for registration with ID "oidc" but we registered "authentik" + assertEquals("oidc", buggyExtractedId); + + // The mismatch: registrationId="authentik", but Spring looks for "oidc" + // Result: InvalidClientRegistrationIdException + assertNotNull(buggyExtractedId, "This demonstrates the bug that was fixed"); + } + + /** Helper method simulating Spring's extraction of registration ID from callback URL */ + private String extractRegistrationIdFromCallback(String callbackUrl) { + // Simplified version of what Spring Security does + // Actual: OAuth2AuthorizationRequestRedirectFilter extracts from path + String path = callbackUrl.split("\\?")[0]; + String[] parts = path.split("/"); + return parts[parts.length - 1]; // Last path segment + } + + /** + * Validates the frontend-backend flow for custom provider names. + * + *

Complete flow: 1. Backend: Provider configured as "authentik" in settings.yml 2. Backend: + * ClientRegistration created with registrationId="authentik" 3. Backend: Redirect URI set to + * "{baseUrl}/login/oauth2/code/authentik" 4. Backend: Login endpoint returns providerList with + * "/oauth2/authorization/authentik" 5. Frontend: Extracts "authentik" from path and uses it for + * OAuth login 6. Frontend: Redirects to "/oauth2/authorization/authentik" 7. Backend: Spring + * Security redirects to provider with redirect_uri containing "authentik" 8. Provider: + * Redirects back to "/login/oauth2/code/authentik?code=..." 9. Backend: Spring Security + * extracts "authentik" from callback URL 10. Backend: Looks up ClientRegistration with ID + * "authentik" ✅ SUCCESS + * + *

If redirect URI was hardcoded to "oidc" (the bug): Step 7: Provider redirects to + * "/login/oauth2/code/oidc?code=..." Step 9: Spring Security looks for registration ID "oidc" + * Step 10: FAIL - No registration found with ID "oidc" (we registered "authentik") Result: + * InvalidClientRegistrationIdException + */ + @Test + void testEndToEndFlow_registrationIdConsistency() { + String providerName = "authentik"; + + // Step 2: Registration ID + String registrationId = providerName; + assertEquals("authentik", registrationId); + + // Step 3: Redirect URI (MUST use same name) + String redirectUri = "{baseUrl}/login/oauth2/code/" + providerName; + assertEquals("{baseUrl}/login/oauth2/code/authentik", redirectUri); + + // Step 4: Provider list endpoint + String authorizationPath = "/oauth2/authorization/" + providerName; + assertEquals("/oauth2/authorization/authentik", authorizationPath); + + // Step 5: Frontend extracts provider ID + String frontendProviderId = + authorizationPath.substring(authorizationPath.lastIndexOf('/') + 1); + assertEquals("authentik", frontendProviderId); + + // Step 6-8: OAuth flow (external) + + // Step 9: Callback URL from provider + String callbackUrl = + "http://localhost:8080/login/oauth2/code/" + providerName + "?code=abc123"; + String extractedId = extractRegistrationIdFromCallback(callbackUrl); + + // Step 10: Registration lookup + assertEquals( + registrationId, + extractedId, + "Registration ID from callback MUST match original registration ID"); + } +} diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/service/UserLicenseSettingsServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/service/UserLicenseSettingsServiceTest.java index 139146d707..7f9445ad7c 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/service/UserLicenseSettingsServiceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/service/UserLicenseSettingsServiceTest.java @@ -267,4 +267,222 @@ class UserLicenseSettingsServiceTest { verify(userService, times(1)).grandfatherAllOAuthUsers(); verify(userService, times(1)).grandfatherPendingSsoUsersWithoutSession(); } + + // ===== OAuth Eligibility Tests ===== + + @Test + void isOAuthEligible_grandfatheredUser_returnsTrue() { + // Grandfathered user should be eligible regardless of license + stirling.software.proprietary.security.model.User user = + new stirling.software.proprietary.security.model.User(); + user.setUsername("grandfathered-user"); + user.setOauthGrandfathered(true); + + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.NORMAL); + + boolean result = service.isOAuthEligible(user); + + assertEquals(true, result, "Grandfathered user should be eligible for OAuth"); + } + + @Test + void isOAuthEligible_nonGrandfatheredUserWithServerLicense_returnsTrue() { + // Non-grandfathered user with SERVER license should be eligible + stirling.software.proprietary.security.model.User user = + new stirling.software.proprietary.security.model.User(); + user.setUsername("test-user"); + user.setOauthGrandfathered(false); + + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.SERVER); + + boolean result = service.isOAuthEligible(user); + + assertEquals(true, result, "Non-grandfathered user with SERVER license should be eligible"); + } + + @Test + void isOAuthEligible_nonGrandfatheredUserWithEnterpriseLicense_returnsTrue() { + // Non-grandfathered user with ENTERPRISE license should be eligible + stirling.software.proprietary.security.model.User user = + new stirling.software.proprietary.security.model.User(); + user.setUsername("test-user"); + user.setOauthGrandfathered(false); + + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.ENTERPRISE); + + boolean result = service.isOAuthEligible(user); + + assertEquals( + true, result, "Non-grandfathered user with ENTERPRISE license should be eligible"); + } + + @Test + void isOAuthEligible_nonGrandfatheredUserWithNoLicense_returnsFalse() { + // Non-grandfathered user without license should NOT be eligible + stirling.software.proprietary.security.model.User user = + new stirling.software.proprietary.security.model.User(); + user.setUsername("test-user"); + user.setOauthGrandfathered(false); + + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.NORMAL); + + boolean result = service.isOAuthEligible(user); + + assertEquals( + false, + result, + "Non-grandfathered user without paid license should NOT be eligible"); + } + + @Test + void isOAuthEligible_newUserWithServerLicense_returnsTrue() { + // New user (null) with SERVER license should be eligible for auto-creation + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.SERVER); + + boolean result = service.isOAuthEligible(null); + + assertEquals( + true, result, "New user with SERVER license should be eligible for auto-creation"); + } + + @Test + void isOAuthEligible_newUserWithNoLicense_returnsFalse() { + // New user (null) without license should NOT be eligible + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.NORMAL); + + boolean result = service.isOAuthEligible(null); + + assertEquals( + false, + result, + "New user without paid license should NOT be eligible for auto-creation"); + } + + @Test + void isOAuthEligible_licenseCheckerUnavailable_returnsFalse() { + // If LicenseKeyChecker is unavailable, OAuth should be blocked + when(licenseKeyCheckerProvider.getIfAvailable()).thenReturn(null); + + stirling.software.proprietary.security.model.User user = + new stirling.software.proprietary.security.model.User(); + user.setUsername("test-user"); + user.setOauthGrandfathered(false); + + boolean result = service.isOAuthEligible(user); + + assertEquals( + false, result, "OAuth should be blocked when LicenseKeyChecker is unavailable"); + } + + // ===== SAML Eligibility Tests ===== + + @Test + void isSamlEligible_grandfatheredUser_returnsTrue() { + // Grandfathered user should be eligible for SAML regardless of license + stirling.software.proprietary.security.model.User user = + new stirling.software.proprietary.security.model.User(); + user.setUsername("grandfathered-user"); + user.setOauthGrandfathered(true); + + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.NORMAL); + + boolean result = service.isSamlEligible(user); + + assertEquals(true, result, "Grandfathered user should be eligible for SAML"); + } + + @Test + void isSamlEligible_nonGrandfatheredUserWithEnterpriseLicense_returnsTrue() { + // Non-grandfathered user with ENTERPRISE license should be eligible + stirling.software.proprietary.security.model.User user = + new stirling.software.proprietary.security.model.User(); + user.setUsername("test-user"); + user.setOauthGrandfathered(false); + + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.ENTERPRISE); + + boolean result = service.isSamlEligible(user); + + assertEquals( + true, + result, + "Non-grandfathered user with ENTERPRISE license should be eligible for SAML"); + } + + @Test + void isSamlEligible_nonGrandfatheredUserWithServerLicense_returnsFalse() { + // Non-grandfathered user with SERVER license should NOT be eligible for SAML + stirling.software.proprietary.security.model.User user = + new stirling.software.proprietary.security.model.User(); + user.setUsername("test-user"); + user.setOauthGrandfathered(false); + + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.SERVER); + + boolean result = service.isSamlEligible(user); + + assertEquals( + false, + result, + "Non-grandfathered user with SERVER license should NOT be eligible for SAML"); + } + + @Test + void isSamlEligible_nonGrandfatheredUserWithNoLicense_returnsFalse() { + // Non-grandfathered user without license should NOT be eligible + stirling.software.proprietary.security.model.User user = + new stirling.software.proprietary.security.model.User(); + user.setUsername("test-user"); + user.setOauthGrandfathered(false); + + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.NORMAL); + + boolean result = service.isSamlEligible(user); + + assertEquals( + false, + result, + "Non-grandfathered user without ENTERPRISE license should NOT be eligible for SAML"); + } + + @Test + void isSamlEligible_newUserWithEnterpriseLicense_returnsTrue() { + // New user (null) with ENTERPRISE license should be eligible for auto-creation + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.ENTERPRISE); + + boolean result = service.isSamlEligible(null); + + assertEquals( + true, + result, + "New user with ENTERPRISE license should be eligible for SAML auto-creation"); + } + + @Test + void isSamlEligible_newUserWithServerLicense_returnsFalse() { + // New user (null) with SERVER license should NOT be eligible for SAML + when(licenseKeyChecker.getPremiumLicenseEnabledResult()).thenReturn(License.SERVER); + + boolean result = service.isSamlEligible(null); + + assertEquals( + false, + result, + "New user with SERVER license should NOT be eligible for SAML (requires ENTERPRISE)"); + } + + @Test + void isSamlEligible_licenseCheckerUnavailable_returnsFalse() { + // If LicenseKeyChecker is unavailable, SAML should be blocked + when(licenseKeyCheckerProvider.getIfAvailable()).thenReturn(null); + + stirling.software.proprietary.security.model.User user = + new stirling.software.proprietary.security.model.User(); + user.setUsername("test-user"); + user.setOauthGrandfathered(false); + + boolean result = service.isSamlEligible(user); + + assertEquals(false, result, "SAML should be blocked when LicenseKeyChecker is unavailable"); + } } diff --git a/frontend/src/proprietary/auth/oauthTypes.ts b/frontend/src/proprietary/auth/oauthTypes.ts new file mode 100644 index 0000000000..2d38f1b3e5 --- /dev/null +++ b/frontend/src/proprietary/auth/oauthTypes.ts @@ -0,0 +1,24 @@ +/** + * Known OAuth providers with dedicated UI support. + * Custom providers are also supported - the backend determines availability. + */ +export const KNOWN_OAUTH_PROVIDERS = [ + 'github', + 'google', + 'apple', + 'azure', + 'keycloak', + 'cloudron', + 'authentik', + 'oidc', +] as const; + +export type KnownOAuthProvider = typeof KNOWN_OAUTH_PROVIDERS[number]; + +/** + * OAuth provider ID - can be any known provider or custom string. + * The backend configuration determines which providers are available. + * + * @example 'github' | 'google' | 'mycompany' | 'authentik' + */ +export type OAuthProvider = KnownOAuthProvider | (string & {}); diff --git a/frontend/src/proprietary/auth/springAuthClient.ts b/frontend/src/proprietary/auth/springAuthClient.ts index 2f1aa36cb5..646b711823 100644 --- a/frontend/src/proprietary/auth/springAuthClient.ts +++ b/frontend/src/proprietary/auth/springAuthClient.ts @@ -10,6 +10,7 @@ import apiClient from '@app/services/apiClient'; import { AxiosError } from 'axios'; import { BASE_PATH } from '@app/constants/app'; +import { type OAuthProvider } from '@app/auth/oauthTypes'; // Helper to extract error message from axios error function getErrorMessage(error: unknown, fallback: string): string { @@ -248,11 +249,14 @@ class SpringAuthClient { } /** - * Sign in with OAuth provider (GitHub, Google, etc.) + * Sign in with OAuth provider (GitHub, Google, Authentik, etc.) * This redirects to the Spring OAuth2 authorization endpoint + * + * @param params.provider - OAuth provider ID (e.g., 'github', 'google', 'authentik', 'mycompany') + * Can be any known provider or custom string - the backend determines available providers */ async signInWithOAuth(params: { - provider: 'github' | 'google' | 'apple' | 'azure' | 'keycloak' | 'oidc'; + provider: OAuthProvider; options?: { redirectTo?: string; queryParams?: Record }; }): Promise<{ error: AuthError | null }> { try { diff --git a/frontend/src/proprietary/routes/Login.test.tsx b/frontend/src/proprietary/routes/Login.test.tsx index 62679f22ac..996176c01c 100644 --- a/frontend/src/proprietary/routes/Login.test.tsx +++ b/frontend/src/proprietary/routes/Login.test.tsx @@ -7,6 +7,7 @@ import Login from '@app/routes/Login'; import { useAuth } from '@app/auth/UseSession'; import { springAuth } from '@app/auth/springAuthClient'; import { PreferencesProvider } from '@app/contexts/PreferencesContext'; +import apiClient from '@app/services/apiClient'; // Mock i18n to return fallback text vi.mock('react-i18next', () => ({ @@ -36,8 +37,13 @@ vi.mock('@app/hooks/useDocumentMeta', () => ({ useDocumentMeta: vi.fn(), })); -// Mock fetch for provider list -global.fetch = vi.fn(); +// Mock apiClient for provider list +vi.mock('@app/services/apiClient', () => ({ + default: { + get: vi.fn(), + post: vi.fn(), + }, +})); const mockNavigate = vi.fn(); const mockBackendProbeState = { @@ -89,14 +95,13 @@ describe('Login', () => { refreshSession: vi.fn(), }); - // Mock fetch for login UI data - vi.mocked(fetch).mockResolvedValue({ - ok: true, - json: async () => ({ + // Mock apiClient for login UI data + vi.mocked(apiClient.get).mockResolvedValue({ + data: { enableLogin: true, providerList: {}, - }), - } as Response); + }, + }); }); it('should render login form', async () => { @@ -239,6 +244,136 @@ describe('Login', () => { }); }); + it('should use actual provider ID for OAuth login (authentik)', async () => { + const user = userEvent.setup(); + + // Mock provider list with authentik + vi.mocked(apiClient.get).mockResolvedValue({ + data: { + enableLogin: true, + providerList: { + '/oauth2/authorization/authentik': 'Authentik', + }, + }, + }); + + vi.mocked(springAuth.signInWithOAuth).mockResolvedValueOnce({ + error: null, + }); + + render( + + + + + + ); + + // Wait for OAuth button to appear + await waitFor(() => { + const button = screen.queryByText('Authentik'); + expect(button).toBeTruthy(); + }, { timeout: 3000 }); + + const oauthButton = screen.getByText('Authentik'); + await user.click(oauthButton); + + await waitFor(() => { + // Should use 'authentik' directly, NOT map to 'oidc' + expect(springAuth.signInWithOAuth).toHaveBeenCalledWith({ + provider: 'authentik', + options: { redirectTo: '/auth/callback' } + }); + }); + }); + + it('should use actual provider ID for OAuth login (custom provider)', async () => { + const user = userEvent.setup(); + + // Mock provider list with custom provider 'mycompany' + vi.mocked(apiClient.get).mockResolvedValue({ + data: { + enableLogin: true, + providerList: { + '/oauth2/authorization/mycompany': 'My Company SSO', + }, + }, + }); + + vi.mocked(springAuth.signInWithOAuth).mockResolvedValueOnce({ + error: null, + }); + + render( + + + + + + ); + + // Wait for OAuth button to appear (will show 'Mycompany' as label) + await waitFor(() => { + const button = screen.queryByText('Mycompany'); + expect(button).toBeTruthy(); + }, { timeout: 3000 }); + + const oauthButton = screen.getByText('Mycompany'); + await user.click(oauthButton); + + await waitFor(() => { + // Should use 'mycompany' directly - this is the critical fix + // Previously it would map unknown providers to 'oidc' + expect(springAuth.signInWithOAuth).toHaveBeenCalledWith({ + provider: 'mycompany', + options: { redirectTo: '/auth/callback' } + }); + }); + }); + + it('should use oidc provider ID when explicitly configured', async () => { + const user = userEvent.setup(); + + // Mock provider list with 'oidc' + vi.mocked(apiClient.get).mockResolvedValue({ + data: { + enableLogin: true, + providerList: { + '/oauth2/authorization/oidc': 'OIDC', + }, + }, + }); + + vi.mocked(springAuth.signInWithOAuth).mockResolvedValueOnce({ + error: null, + }); + + render( + + + + + + ); + + // Wait for OAuth button to appear + await waitFor(() => { + const button = screen.queryByText('OIDC'); + expect(button).toBeTruthy(); + }, { timeout: 3000 }); + + const oauthButton = screen.getByText('OIDC'); + await user.click(oauthButton); + + await waitFor(() => { + // Should use 'oidc' when explicitly configured + expect(springAuth.signInWithOAuth).toHaveBeenCalledWith({ + provider: 'oidc', + options: { redirectTo: '/auth/callback' } + }); + }); + }); + it('should show error on failed login', async () => { const user = userEvent.setup(); const errorMessage = 'Invalid credentials'; @@ -359,13 +494,12 @@ describe('Login', () => { it('should redirect to home when login disabled', async () => { mockBackendProbeState.loginDisabled = true; mockProbe.mockResolvedValueOnce({ status: 'up', loginDisabled: true, loading: false }); - vi.mocked(fetch).mockResolvedValueOnce({ - ok: true, - json: async () => ({ + vi.mocked(apiClient.get).mockResolvedValueOnce({ + data: { enableLogin: false, providerList: {}, - }), - } as Response); + }, + }); render( @@ -381,15 +515,14 @@ describe('Login', () => { }); it('should handle OAuth provider click', async () => { - vi.mocked(fetch).mockResolvedValueOnce({ - ok: true, - json: async () => ({ + vi.mocked(apiClient.get).mockResolvedValueOnce({ + data: { enableLogin: true, providerList: { '/oauth2/authorization/github': 'GitHub', }, - }), - } as Response); + }, + }); vi.mocked(springAuth.signInWithOAuth).mockResolvedValueOnce({ error: null, @@ -416,13 +549,12 @@ describe('Login', () => { }); it('should show email form by default when no SSO providers', async () => { - vi.mocked(fetch).mockResolvedValueOnce({ - ok: true, - json: async () => ({ + vi.mocked(apiClient.get).mockResolvedValueOnce({ + data: { enableLogin: true, providerList: {}, // No providers - }), - } as Response); + }, + }); render( diff --git a/frontend/src/proprietary/routes/Login.tsx b/frontend/src/proprietary/routes/Login.tsx index 80a12e1d09..cf6004e505 100644 --- a/frontend/src/proprietary/routes/Login.tsx +++ b/frontend/src/proprietary/routes/Login.tsx @@ -10,6 +10,7 @@ import AuthLayout from '@app/routes/authShared/AuthLayout'; import { useBackendProbe } from '@app/hooks/useBackendProbe'; import apiClient from '@app/services/apiClient'; import { BASE_PATH } from '@app/constants/app'; +import { type OAuthProvider } from '@app/auth/oauthTypes'; // Import login components import LoginHeader from '@app/routes/login/LoginHeader'; @@ -31,7 +32,7 @@ export default function Login() { const [showEmailForm, setShowEmailForm] = useState(false); const [email, setEmail] = useState(() => searchParams.get('email') ?? ''); const [password, setPassword] = useState(''); - const [enabledProviders, setEnabledProviders] = useState([]); + const [enabledProviders, setEnabledProviders] = useState([]); const [hasSSOProviders, setHasSSOProviders] = useState(false); const [_enableLogin, setEnableLogin] = useState(null); const backendProbe = useBackendProbe(); @@ -226,25 +227,17 @@ export default function Login() { ); } - // Known OAuth providers that have dedicated backend support - const KNOWN_OAUTH_PROVIDERS = ['github', 'google', 'apple', 'azure', 'keycloak', 'oidc'] as const; - type KnownOAuthProvider = typeof KNOWN_OAUTH_PROVIDERS[number]; - - const signInWithProvider = async (provider: string) => { + const signInWithProvider = async (provider: OAuthProvider) => { try { setIsSigningIn(true); setError(null); - // Map unknown providers to 'oidc' for the backend redirect - const backendProvider: KnownOAuthProvider = KNOWN_OAUTH_PROVIDERS.includes(provider as KnownOAuthProvider) - ? (provider as KnownOAuthProvider) - : 'oidc'; + console.log(`[Login] Signing in with provider: ${provider}`); - console.log(`[Login] Signing in with ${provider} (backend: ${backendProvider})`); - - // Redirect to Spring OAuth2 endpoint + // Redirect to Spring OAuth2 endpoint using the actual provider ID from backend + // The backend returns the correct registration ID (e.g., 'authentik', 'oidc', 'keycloak') const { error } = await springAuth.signInWithOAuth({ - provider: backendProvider, + provider: provider, options: { redirectTo: `${BASE_PATH}/auth/callback` } }); diff --git a/frontend/src/proprietary/routes/login/OAuthButtons.test.tsx b/frontend/src/proprietary/routes/login/OAuthButtons.test.tsx new file mode 100644 index 0000000000..62f121d68c --- /dev/null +++ b/frontend/src/proprietary/routes/login/OAuthButtons.test.tsx @@ -0,0 +1,291 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MantineProvider } from '@mantine/core'; +import OAuthButtons from '@app/routes/login/OAuthButtons'; + +// Mock i18n +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (key: string, fallback?: string) => fallback || key, + }), +})); + +const TestWrapper = ({ children }: { children: React.ReactNode }) => ( + {children} +); + +describe('OAuthButtons', () => { + const mockOnProviderClick = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should render known providers with correct labels', () => { + const enabledProviders = ['google', 'github', 'authentik']; + + render( + + + + ); + + // Check that known providers are rendered with their labels + expect(screen.getByText('Google')).toBeTruthy(); + expect(screen.getByText('GitHub')).toBeTruthy(); + expect(screen.getByText('Authentik')).toBeTruthy(); + }); + + it('should render unknown provider with capitalized label and generic icon', () => { + const enabledProviders = ['mycompany']; + + render( + + + + ); + + // Unknown provider should be capitalized + expect(screen.getByText('Mycompany')).toBeTruthy(); + + // Check that button has generic OIDC icon + const button = screen.getByText('Mycompany').closest('button'); + expect(button).toBeTruthy(); + const img = button?.querySelector('img'); + expect(img?.src).toContain('oidc.svg'); + }); + + it('should call onProviderClick with actual provider ID (not "oidc")', async () => { + const user = userEvent.setup(); + const enabledProviders = ['mycompany']; + + render( + + + + ); + + const button = screen.getByText('Mycompany'); + await user.click(button); + + // Should use actual provider ID 'mycompany', NOT 'oidc' + expect(mockOnProviderClick).toHaveBeenCalledWith('mycompany'); + }); + + it('should call onProviderClick with "authentik" when authentik is clicked', async () => { + const user = userEvent.setup(); + const enabledProviders = ['authentik']; + + render( + + + + ); + + const button = screen.getByText('Authentik'); + await user.click(button); + + expect(mockOnProviderClick).toHaveBeenCalledWith('authentik'); + }); + + it('should call onProviderClick with "oidc" when OIDC is explicitly configured', async () => { + const user = userEvent.setup(); + const enabledProviders = ['oidc']; + + render( + + + + ); + + const button = screen.getByText('OIDC'); + await user.click(button); + + expect(mockOnProviderClick).toHaveBeenCalledWith('oidc'); + }); + + it('should disable buttons when isSubmitting is true', () => { + const enabledProviders = ['google', 'github']; + + render( + + + + ); + + const googleButton = screen.getByText('Google').closest('button') as HTMLButtonElement; + const githubButton = screen.getByText('GitHub').closest('button') as HTMLButtonElement; + + expect(googleButton.disabled).toBe(true); + expect(githubButton.disabled).toBe(true); + }); + + it('should render nothing when no providers are enabled', () => { + const { container } = render( + + + + ); + + // Should render null/nothing (excluding Mantine's style tags) + const hasContent = Array.from(container.children).some( + child => child.tagName.toLowerCase() !== 'style' + ); + expect(hasContent).toBe(false); + }); + + it('should render multiple unknown providers with correct IDs', async () => { + const user = userEvent.setup(); + const enabledProviders = ['company1', 'company2', 'company3']; + + render( + + + + ); + + // All should be capitalized + expect(screen.getByText('Company1')).toBeTruthy(); + expect(screen.getByText('Company2')).toBeTruthy(); + expect(screen.getByText('Company3')).toBeTruthy(); + + // Click each and verify correct ID is passed + await user.click(screen.getByText('Company1')); + expect(mockOnProviderClick).toHaveBeenCalledWith('company1'); + + await user.click(screen.getByText('Company2')); + expect(mockOnProviderClick).toHaveBeenCalledWith('company2'); + + await user.click(screen.getByText('Company3')); + expect(mockOnProviderClick).toHaveBeenCalledWith('company3'); + }); + + it('should use correct icon for known providers', () => { + const enabledProviders = ['google', 'github', 'authentik', 'keycloak']; + + render( + + + + ); + + // Check that each known provider has its specific icon + const googleButton = screen.getByText('Google').closest('button'); + expect(googleButton?.querySelector('img')?.src).toContain('google.svg'); + + const githubButton = screen.getByText('GitHub').closest('button'); + expect(githubButton?.querySelector('img')?.src).toContain('github.svg'); + + const authentikButton = screen.getByText('Authentik').closest('button'); + expect(authentikButton?.querySelector('img')?.src).toContain('authentik.svg'); + + const keycloakButton = screen.getByText('Keycloak').closest('button'); + expect(keycloakButton?.querySelector('img')?.src).toContain('keycloak.svg'); + }); + + it('should handle mixed known and unknown providers', async () => { + const user = userEvent.setup(); + const enabledProviders = ['google', 'mycompany', 'authentik', 'custom']; + + render( + + + + ); + + // Known providers with correct labels + expect(screen.getByText('Google')).toBeTruthy(); + expect(screen.getByText('Authentik')).toBeTruthy(); + + // Unknown providers with capitalized labels + expect(screen.getByText('Mycompany')).toBeTruthy(); + expect(screen.getByText('Custom')).toBeTruthy(); + + // Click each and verify IDs are preserved + await user.click(screen.getByText('Google')); + expect(mockOnProviderClick).toHaveBeenCalledWith('google'); + + await user.click(screen.getByText('Mycompany')); + expect(mockOnProviderClick).toHaveBeenCalledWith('mycompany'); + + await user.click(screen.getByText('Authentik')); + expect(mockOnProviderClick).toHaveBeenCalledWith('authentik'); + + await user.click(screen.getByText('Custom')); + expect(mockOnProviderClick).toHaveBeenCalledWith('custom'); + }); + + it('should maintain provider ID consistency - critical for OAuth redirect', async () => { + const user = userEvent.setup(); + + // This test ensures the fix for GitHub issue #5141 + // The provider ID used in the button click MUST match the backend registration ID + // Previously, unknown providers were mapped to 'oidc', breaking the OAuth flow + + const enabledProviders = ['authentik', 'okta', 'auth0']; + + render( + + + + ); + + // Each provider should use its actual ID, not 'oidc' + await user.click(screen.getByText('Authentik')); + expect(mockOnProviderClick).toHaveBeenLastCalledWith('authentik'); + + await user.click(screen.getByText('Okta')); + expect(mockOnProviderClick).toHaveBeenLastCalledWith('okta'); + + await user.click(screen.getByText('Auth0')); + expect(mockOnProviderClick).toHaveBeenLastCalledWith('auth0'); + + // Verify none were called with 'oidc' instead of their actual ID + expect(mockOnProviderClick).not.toHaveBeenCalledWith('oidc'); + }); +}); diff --git a/frontend/src/proprietary/routes/login/OAuthButtons.tsx b/frontend/src/proprietary/routes/login/OAuthButtons.tsx index aaa280519f..d62edfdc15 100644 --- a/frontend/src/proprietary/routes/login/OAuthButtons.tsx +++ b/frontend/src/proprietary/routes/login/OAuthButtons.tsx @@ -1,5 +1,6 @@ import { useTranslation } from 'react-i18next'; import { BASE_PATH } from '@app/constants/app'; +import { type OAuthProvider } from '@app/auth/oauthTypes'; // Debug flag to show all providers for UI testing // Set to true to see all SSO options regardless of backend configuration @@ -22,10 +23,10 @@ export const oauthProviderConfig: Record void + onProviderClick: (provider: OAuthProvider) => void isSubmitting: boolean layout?: 'vertical' | 'grid' | 'icons' - enabledProviders?: string[] // List of enabled provider IDs from backend + enabledProviders?: OAuthProvider[] // List of enabled provider IDs from backend } export default function OAuthButtons({ onProviderClick, isSubmitting, layout = 'vertical', enabledProviders = [] }: OAuthButtonsProps) { From bb201ef9c10609398576fb4cc1d2033a9d4034e4 Mon Sep 17 00:00:00 2001 From: EthanHealy01 <80844253+EthanHealy01@users.noreply.github.com> Date: Fri, 5 Dec 2025 23:22:32 +0000 Subject: [PATCH 6/6] Chore/bump gradle version number (#5176) bump version number --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 2ae1355944..85f6a480de 100644 --- a/build.gradle +++ b/build.gradle @@ -57,7 +57,7 @@ repositories { allprojects { group = 'stirling.software' - version = '2.1.0' + version = '2.1.1' configurations.configureEach { exclude group: 'commons-logging', module: 'commons-logging'