feat: Dex integration testing #1004

sharma-tapas · 2025-10-14T11:25:16Z

What this PR does / why we need it

Which issue(s) this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged)

fixes #

Special notes for your reviewer

Testing done

please add testing details (logs, screenshots, etc.)

Signed-off-by: Tapas Sharma <tapas@platform9.com>

ui/src/pages/auth/LoginPage.tsx

+      if (isAuthenticated) {
+        // User is already authenticated, redirect to dashboard
+        const returnUrl = new URLSearchParams(window.location.search).get('rd') || '/dashboard/migrations';
+        window.location.href = returnUrl;


The best way to fix this vulnerability is to whitelist return URLs so that only expected, safe paths on your own site can be used as redirect destinations. Specifically, we should check that returnUrl is a pathname within the application's domain (e.g., /dashboard/migrations or any other internal route) and never allow an absolute URL (such as https://evil.com). To do this, we can check if the value of returnUrl starts with a / and does not contain suspicious patterns (e.g., double slashes, schema). If the validation fails, fallback to a safe default route.

You need to adjust both:

The redirect in window.location.href = returnUrl; inside checkAuthStatus

The use of authService.initiateLogin(returnUrl); in handleLogin (if it also relies on a redirect target: ideally, sanitize the param in both places)

Add a utility function such as getSafeRedirectUrl, and update both usages to use this.

All changes are within ui/src/pages/auth/LoginPage.tsx.

ui/src/pages/auth/LoginPage.tsx

+      if (isAuthenticated) {
+        // User is already authenticated, redirect to dashboard
+        const returnUrl = new URLSearchParams(window.location.search).get('rd') || '/dashboard/migrations';
+        window.location.href = returnUrl;


The best approach is to only allow the rd ("return destination") parameter to navigate to URLs within the application's allowed paths. This avoids both open redirects and XSS vectors. Specifically:

Only accept rd values that (a) are relative paths beginning with /, and (b) do not contain protocol specifiers (:), as those could be abused.

If rd is not present, or does not match allowed patterns, default to the safe in-app path (/dashboard/migrations).

You'll need to change both usages of returnUrl (in checkAuthStatus and handleLogin). Any line like window.location.href = returnUrl should only occur with a validated, safe path.

Implementation:

You can introduce a small utility function, e.g., getSafeReturnUrl, within this file, before the component, to encapsulate the logic for extracting and validating the return URL.

That function will parse the query param and only return the path if it is application-local and safe.

No new dependencies are needed, as the required operations are simple.

- Removed unused styled Logo component causing TypeScript build error - Redirect already correctly goes to /dashboard/migrations (not /onboarding) Signed-off-by: Tapas Sharma <tapas@platform9.com>

scripts/update-dex-password.go

+	password := os.Args[1]
+
+	// Generate bcrypt hash using Go's bcrypt library (same as Dex)
+	fmt.Printf("Generating bcrypt hash for password: %s\n", password)


The fix is to ensure that sensitive information (the plaintext password) is not logged. The statement at line 26, fmt.Printf("Generating bcrypt hash for password: %s\n", password), should be changed to avoid printing the password. We can simply change it to print a generic message like "Generating bcrypt hash for password...", without including the actual password value. No further changes are necessary, as the rest of the script does not log the password itself. This fix should be made in the file scripts/update-dex-password.go, replacing just line 26. No new imports or methods are required.

scripts/update-dex-password.go

+	fmt.Printf("  kubectl delete pod -n %s -l app=dex\n", namespace)
+	fmt.Printf("\nNew credentials:\n")
+	fmt.Printf("  Email: admin@vjailbreak.local\n")
+	fmt.Printf("  Password: %s\n", password)


To fix the clear-text logging of the sensitive password, we should remove or obfuscate the logging statement that prints the plaintext password to the console on line 112. The best practice is to omit printing any sensitive information, or at minimum, to mask (obfuscate) it—e.g., "********"—so the value cannot be retrieved from the logs.

Edit scripts/update-dex-password.go: Remove or redact the line that prints the password (fmt.Printf(" Password: %s\n", password)).

Optionally, change the message to indicate that the password has been set, but do not display its value.

No new methods or imports are needed.

Signed-off-by: Tapas Sharma <tapas@platform9.com>

The UI was sending 'Authorization: Bearer null' because it tried to read tokens from localStorage which was never populated. OAuth2 proxy handles authentication via cookies (_oauth2_proxy), so we don't need to manually manage Bearer tokens. Changes: - Simplified createAuthenticatedClient() to only set withCredentials - Removed manual Authorization header that was always null - Backend nginx/oauth2-proxy forwards auth headers automatically Fixes: 502 Bad Gateway errors when creating users from UI Signed-off-by: Tapas Sharma <tapas@platform9.com>

- Added Create, Update, Delete tests for local users - Tests use healthcheck@vjailbreak.local test user - Added Body field to APIEndpoint struct for POST/PUT payloads - Added Skip field to allow disabling individual tests - Updated documentation with CRUD test details Total tests increased from 15 to 18 endpoints Signed-off-by: Tapas Sharma <tapas@platform9.com>

Signed-off-by: Tapas Sharma <tapas@platform9.com>

- Update Dex issuer to https://HOST_IP/dex - Update OAuth2 Proxy to use HTTPS URLs for external endpoints - Enable secure cookies (cookie_secure = true) - Add SSL insecure skip verify for self-signed certs - Configure ingress for API endpoints (routing to K8s API) - Update UI ingress paths to avoid catching /api and /apis - Add priority annotations to ingresses Working: - Dex authentication with HTTPS - OAuth2 Proxy with secure cookies - UI login redirects to /dashboard - SSL termination at ingress Known Issue: - API endpoints (/api, /apis) return 401 through ingress - Documented in KNOWN_ISSUES.md for investigation - Likely requires UI backend to proxy API requests Signed-off-by: Tapas Sharma <tapas@platform9.com>

feat: Dex integration testing

005ca19

Signed-off-by: Tapas Sharma <tapas@platform9.com>

github-advanced-security bot found potential problems Oct 14, 2025

View reviewed changes

fix: Remove unused Logo component in LoginPage

45a79f8

- Removed unused styled Logo component causing TypeScript build error - Redirect already correctly goes to /dashboard/migrations (not /onboarding) Signed-off-by: Tapas Sharma <tapas@platform9.com>

github-advanced-security bot found potential problems Oct 14, 2025

View reviewed changes

sharma-tapas added 13 commits October 14, 2025 18:52

feat: Add user management

4bd3a3e

Signed-off-by: Tapas Sharma <tapas@platform9.com>

fix build issues

01fd1d4

Signed-off-by: Tapas Sharma <tapas@platform9.com>

fix: Local service account master API server access issue

7eda284

Signed-off-by: Tapas Sharma <tapas@platform9.com>

some more changes

979579d

Signed-off-by: Tapas Sharma <tapas@platform9.com>

user management

90b48b2

Signed-off-by: Tapas Sharma <tapas@platform9.com>

ui fixes

f12d796

Signed-off-by: Tapas Sharma <tapas@platform9.com>

some ui fixes for certificates and dev-api

b398929

Signed-off-by: Tapas Sharma <tapas@platform9.com>

Adding some integration tests

987705f

Signed-off-by: Tapas Sharma <tapas@platform9.com>

fix IDP code

ffa2207

Signed-off-by: Tapas Sharma <tapas@platform9.com>

fix: Update OAuth2 Proxy cookie secret to exactly 32 bytes

99d5669

Signed-off-by: Tapas Sharma <tapas@platform9.com>

@@ -28,6 +28,24 @@
               boxShadow: '0 8px 32px rgba(0, 0, 0, 0.1)',
             }));
+            // Whitelist internal routes for safe redirection
+            function getSafeRedirectUrl(redirectUrl: string | null): string {
+              // Only allow relative paths starting with /
+              if (
+                redirectUrl &&
+                redirectUrl.startsWith('/') &&
+                // Prevent // (protocol-like) and suspicious substrings
+                !redirectUrl.startsWith('//') &&
+                !redirectUrl.includes('://') &&
+                !redirectUrl.includes('\r') &&
+                !redirectUrl.includes('\n')
+              ) {
+                return redirectUrl;
+              }
+              // Default safe route
+              return '/dashboard/migrations';
+            }
             const LoginPage = () => {
               const [isChecking, setIsChecking] = useState(true);
               const [error, setError] = useState<string | null>(null);
@@ -41,7 +59,8 @@
                   const isAuthenticated = await authService.checkAuth();
                   if (isAuthenticated) {
                     // User is already authenticated, redirect to dashboard
-                    const returnUrl = new URLSearchParams(window.location.search).get('rd') || '/dashboard/migrations';
+                    const unsafeReturnUrl = new URLSearchParams(window.location.search).get('rd');
+                    const returnUrl = getSafeRedirectUrl(unsafeReturnUrl);
                     window.location.href = returnUrl;
                   } else {
                     setIsChecking(false);
@@ -54,7 +73,8 @@
               const handleLogin = () => {
                 try {
-                  const returnUrl = new URLSearchParams(window.location.search).get('rd') || '/dashboard/migrations';
+                  const unsafeReturnUrl = new URLSearchParams(window.location.search).get('rd');
+                  const returnUrl = getSafeRedirectUrl(unsafeReturnUrl);
                   authService.initiateLogin(returnUrl);
                 } catch (err: any) {
                   setError(err.message || 'Failed to initiate login');

@@ -12,7 +12,6 @@
             import { styled } from '@mui/material/styles';
             import { authService } from '../../api/auth/authService';
-            const LoginContainer = styled(Container)(({ theme }) => ({
               minHeight: '100vh',
               display: 'flex',
               alignItems: 'center',
@@ -41,7 +40,7 @@
                   const isAuthenticated = await authService.checkAuth();
                   if (isAuthenticated) {
                     // User is already authenticated, redirect to dashboard
-                    const returnUrl = new URLSearchParams(window.location.search).get('rd') || '/dashboard/migrations';
+                    const returnUrl = getSafeReturnUrl();
                     window.location.href = returnUrl;
                   } else {
                     setIsChecking(false);
@@ -54,7 +53,7 @@
               const handleLogin = () => {
                 try {
-                  const returnUrl = new URLSearchParams(window.location.search).get('rd') || '/dashboard/migrations';
+                  const returnUrl = getSafeReturnUrl();
                   authService.initiateLogin(returnUrl);
                 } catch (err: any) {
                   setError(err.message || 'Failed to initiate login');

@@ -23,7 +23,7 @@
             	password := os.Args[1]
             	// Generate bcrypt hash using Go's bcrypt library (same as Dex)
-            	fmt.Printf("Generating bcrypt hash for password: %s\n", password)
+            	fmt.Println("Generating bcrypt hash for password...")
             	hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
             	if err != nil {
             		log.Fatalf("Failed to generate bcrypt hash: %v", err)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Dex integration testing #1004

feat: Dex integration testing #1004

Uh oh!

sharma-tapas commented Oct 14, 2025 •

edited

Loading

Uh oh!

Check warning

Copilot Autofix

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Check failure

Copilot Autofix

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

feat: Dex integration testing #1004

Are you sure you want to change the base?

feat: Dex integration testing #1004

Uh oh!

Conversation

sharma-tapas commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it

Which issue(s) this PR fixes

Special notes for your reviewer

Testing done

Uh oh!

Check warning

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Check failure

Uh oh!

Copilot Autofix

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sharma-tapas commented Oct 14, 2025 •

edited

Loading