mirror of
https://github.com/arsvendg/Stirling-PDF.git
synced 2026-07-14 10:34:06 +02:00
fix file sharing bug (#6161)
# Description of Changes Fixes share-link navigation for SSO users. Reported on v2.9.2 with `SSOAutoLogin: true`: clicking a `/share/<token>` link in an email redirected the user to the home page after SSO instead of the shared file. ## Root cause Three compounding issues had to be fixed together; the first was the initial symptom but the other two only surfaced during live verification. 1. **Spring Security blocked `/share/<token>` for unauthenticated users.** The route wasn't in `RequestUriUtils.isPublicAuthEndpoint`, so the server 302'd straight to `/login` before React could load `ShareLinkPage`. The share URL was lost because `NullRequestCache` is configured and never persisted the original destination. 2. **`httpErrorHandler` full-page-redirected to `/login?from=<path>` on any unhandled 401** (fired by `LicenseContext`, `AppConfig`, etc. during normal ShareLinkPage mount). That *did* preserve the return path — but **Spring Security strips query strings from `/login`** (302 to bare `/login`), so `?from=` never reached React. Confirmed via `curl -i http://localhost:8080/login?from=xyz` → `Location: /login`. 3. **`AuthCallback.tsx` unconditionally `navigate("/")`** after the SAML/OAuth round-trip, discarding any intended destination. ## Fix **Backend** — make `/share/<token>` a public SPA bootstrap, data APIs stay protected: - `RequestUriUtils.isPublicAuthEndpoint` — permits `^/share/[^/]+/?$` (tight regex, single token segment only; `/share/<token>/anything` stays protected). - `ReactRoutingController` — dedicated `@GetMapping("/share/{token}")` mirroring `/auth/callback`. - `/api/v1/storage/share-links/**` remains behind Spring Security with its existing `canAccessShareLink` check. **Frontend** — persist the return path across full-page redirects via `sessionStorage` (same-origin, survives the SSO round-trip): - `httpErrorHandler.ts` — stashes current pathname to `stirling_post_login_path` before the 401 → `/login` redirect. - `springAuthClient.ts` — new `isSafePostLoginRedirect` / `setPostLoginRedirectPath` / `consumePostLoginRedirectPath` helpers (rejects protocol-relative URLs and auth-plumbing paths to guard against open-redirect abuse). - `Login.tsx` — on explicit user sign-in, read path from `location.state` or `?from=` query and stash it; don't clobber an already-stashed value. - `AuthCallback.tsx` — consume the stashed path (single-use) and `navigate(target)` instead of always `/`. --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [ ] 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) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] 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) - [ ] 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) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### 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 run `task check` to verify linters, typechecks, and tests pass - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md#7-testing) for more details. --------- Co-authored-by: EthanHealy01 <[email protected]>
This commit is contained in:
Anthony Stirling
co-authored by
EthanHealy01
EthanHealy01
parent
3e94157137
commit
c294e9b2cb
+35
-1
@@ -1,6 +1,8 @@
|
||||
# Version control
|
||||
.git/
|
||||
.gitignore
|
||||
.git-blame-ignore-revs
|
||||
.gitattributes
|
||||
|
||||
# Build outputs
|
||||
build/
|
||||
@@ -15,22 +17,34 @@ version_builds/
|
||||
# Gradle caches (local, not what's in the container)
|
||||
.gradle/
|
||||
**/.gradle/
|
||||
.gradle-home/
|
||||
|
||||
# Task (go-task) cache
|
||||
.task/
|
||||
|
||||
# Node / frontend
|
||||
node_modules/
|
||||
**/node_modules/
|
||||
frontend/node_modules/
|
||||
frontend/dist/
|
||||
frontend/playwright-report/
|
||||
.npm/
|
||||
.yarn/
|
||||
|
||||
# Tauri/desktop builds
|
||||
src-tauri/target/
|
||||
src-tauri/dist/
|
||||
frontend/src-tauri/target/
|
||||
frontend/src-tauri/dist/
|
||||
|
||||
# IDE and editor
|
||||
.idea/
|
||||
.vscode/
|
||||
.settings/
|
||||
.settings.zip
|
||||
.classpath
|
||||
.project
|
||||
.devcontainer/
|
||||
*.iml
|
||||
*.ipr
|
||||
*.iws
|
||||
@@ -41,6 +55,7 @@ src-tauri/dist/
|
||||
*.pid
|
||||
.DS_Store
|
||||
Thumbs.db
|
||||
logs/
|
||||
|
||||
# Docker itself
|
||||
Dockerfile*
|
||||
@@ -54,13 +69,33 @@ Dockerfile*
|
||||
# Test reports
|
||||
**/test-results/
|
||||
**/jacoco/
|
||||
test_*.pdf
|
||||
|
||||
# Testing and documentation (not needed in build)
|
||||
testing/
|
||||
docs/
|
||||
devGuide/
|
||||
devTools/
|
||||
*.md
|
||||
README*
|
||||
|
||||
# Separate projects not consumed by the Java/frontend build
|
||||
commonforms-onnx/
|
||||
|
||||
# Runtime mount points used by docker-compose volumes, not build input
|
||||
stirling/
|
||||
customFiles/
|
||||
configs/
|
||||
|
||||
# Claude Code workspace
|
||||
.claude/
|
||||
|
||||
# Python caches
|
||||
.pytest_cache/
|
||||
.ruff_cache/
|
||||
__pycache__/
|
||||
**/__pycache__/
|
||||
|
||||
# Local env
|
||||
.env
|
||||
.env.*
|
||||
@@ -73,4 +108,3 @@ README*
|
||||
*~
|
||||
.DS_Store
|
||||
.cache/
|
||||
.pytest_cache/
|
||||
|
||||
@@ -182,7 +182,9 @@ public class RequestUriUtils {
|
||||
"/api/v1/mobile-scanner/") // Mobile scanner endpoints (no auth)
|
||||
|| trimmedUri.startsWith("/v1/api-docs")
|
||||
// Workflow participant endpoints — access controlled by share tokens, not login
|
||||
|| trimmedUri.startsWith("/api/v1/workflow/participant/");
|
||||
|| trimmedUri.startsWith("/api/v1/workflow/participant/")
|
||||
// Share-link SPA bootstrap; data APIs remain protected
|
||||
|| trimmedUri.matches("^/share/[^/]+/?$");
|
||||
}
|
||||
|
||||
private static String stripContextPath(String contextPath, String requestURI) {
|
||||
|
||||
@@ -161,4 +161,46 @@ class RequestUriUtilsTest {
|
||||
void testIsPublicAuthEndpoint_withContextPath() {
|
||||
assertTrue(RequestUriUtils.isPublicAuthEndpoint("/app/login", "/app"));
|
||||
}
|
||||
|
||||
// --- share-link SPA bootstrap ---
|
||||
|
||||
@Test
|
||||
void testIsPublicAuthEndpoint_shareLinkToken() {
|
||||
assertTrue(
|
||||
RequestUriUtils.isPublicAuthEndpoint(
|
||||
"/share/00dcac3a-fc7a-4989-9c4f-97745484d62f", ""));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testIsPublicAuthEndpoint_shareLinkTokenTrailingSlash() {
|
||||
assertTrue(RequestUriUtils.isPublicAuthEndpoint("/share/abc123/", ""));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testIsPublicAuthEndpoint_shareLinkWithContextPath() {
|
||||
assertTrue(RequestUriUtils.isPublicAuthEndpoint("/app/share/abc123", "/app"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testIsPublicAuthEndpoint_shareRootNotPublic() {
|
||||
// Avoid matching bare "/share" or "/share/" — must have a token segment
|
||||
assertFalse(RequestUriUtils.isPublicAuthEndpoint("/share", ""));
|
||||
assertFalse(RequestUriUtils.isPublicAuthEndpoint("/share/", ""));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testIsPublicAuthEndpoint_shareNestedPathNotPublic() {
|
||||
// Guard against future additions like /share/<token>/download becoming accidentally public
|
||||
assertFalse(RequestUriUtils.isPublicAuthEndpoint("/share/abc123/download", ""));
|
||||
assertFalse(RequestUriUtils.isPublicAuthEndpoint("/share/abc/admin", ""));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testIsPublicAuthEndpoint_shareApiStillProtected() {
|
||||
// Share-link data APIs must NOT be public — they enforce auth + access checks
|
||||
assertFalse(RequestUriUtils.isPublicAuthEndpoint("/api/v1/storage/share-links/abc123", ""));
|
||||
assertFalse(
|
||||
RequestUriUtils.isPublicAuthEndpoint(
|
||||
"/api/v1/storage/share-links/abc123/metadata", ""));
|
||||
}
|
||||
}
|
||||
|
||||
+5
@@ -149,6 +149,11 @@ public class ReactRoutingController {
|
||||
return serveIndexHtml(request);
|
||||
}
|
||||
|
||||
@GetMapping(value = "/share/{token}", produces = MediaType.TEXT_HTML_VALUE)
|
||||
public ResponseEntity<String> serveShareLinkPage(HttpServletRequest request) {
|
||||
return serveIndexHtml(request);
|
||||
}
|
||||
|
||||
@GetMapping(value = "/auth/callback/tauri", produces = MediaType.TEXT_HTML_VALUE)
|
||||
public ResponseEntity<String> serveTauriAuthCallback(HttpServletRequest request) {
|
||||
// cachedCallbackHtml is always initialized in @PostConstruct
|
||||
|
||||
+13
@@ -82,6 +82,19 @@ class ReactRoutingControllerTest {
|
||||
assertTrue(response.getBody().contains("Stirling PDF"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void serveShareLinkPage_returnsIndexHtml() {
|
||||
controller.init();
|
||||
|
||||
ResponseEntity<String> response = controller.serveShareLinkPage(request);
|
||||
|
||||
assertEquals(HttpStatus.OK, response.getStatusCode());
|
||||
assertEquals(MediaType.TEXT_HTML, response.getHeaders().getContentType());
|
||||
String body = response.getBody();
|
||||
assertNotNull(body);
|
||||
assertTrue(body.contains("Stirling PDF"));
|
||||
}
|
||||
|
||||
// --- tauri auth callback ---
|
||||
|
||||
@Test
|
||||
|
||||
@@ -16,6 +16,35 @@ import {
|
||||
const recentSpecialByEndpoint: Record<string, number> = {};
|
||||
const SPECIAL_SUPPRESS_MS = 1500; // brief window to suppress generic duplicate after special toast
|
||||
|
||||
// Mirrors the key in proprietary/auth/springAuthClient.ts; AuthCallback consumes it.
|
||||
const POST_LOGIN_REDIRECT_STORAGE_KEY = "stirling_post_login_path";
|
||||
|
||||
function isSafePostLoginPath(path: string): boolean {
|
||||
if (
|
||||
!path.startsWith("/") ||
|
||||
path.startsWith("//") ||
|
||||
path.startsWith("/\\")
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
const lowered = path.toLowerCase();
|
||||
return (
|
||||
!lowered.startsWith("/login") &&
|
||||
!lowered.startsWith("/auth/") &&
|
||||
!lowered.startsWith("/oauth2") &&
|
||||
!lowered.startsWith("/saml2")
|
||||
);
|
||||
}
|
||||
|
||||
function stashPostLoginRedirect(path: string): void {
|
||||
try {
|
||||
if (typeof window === "undefined" || !isSafePostLoginPath(path)) return;
|
||||
window.sessionStorage.setItem(POST_LOGIN_REDIRECT_STORAGE_KEY, path);
|
||||
} catch {
|
||||
// sessionStorage unavailable (private mode) — fail open
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Handles HTTP errors with toast notifications and file error broadcasting
|
||||
* Returns true if the error should be suppressed (deduplicated), false otherwise
|
||||
@@ -42,9 +71,10 @@ export async function handleHttpError(error: any): Promise<boolean> {
|
||||
// If not on auth page, redirect to login with expired session message
|
||||
if (!isAuthPage && !skipAuthRedirect) {
|
||||
console.debug("[httpErrorHandler] 401 detected, redirecting to login");
|
||||
// Store the current location so we can redirect back after login
|
||||
// Spring 302-strips the ?from= query from /login, so stash the return
|
||||
// path in sessionStorage (AuthCallback reads it after SSO round-trip).
|
||||
const currentLocation = window.location.pathname + window.location.search;
|
||||
// Redirect to login with state (only show expired when a JWT existed)
|
||||
stashPostLoginRedirect(currentLocation);
|
||||
let hadStoredJwt = false;
|
||||
try {
|
||||
hadStoredJwt = Boolean(localStorage.getItem("stirling_jwt"));
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
import { describe, it, expect, beforeEach, vi, afterEach } from "vitest";
|
||||
import { springAuth } from "@app/auth/springAuthClient";
|
||||
import {
|
||||
consumePostLoginRedirectPath,
|
||||
isSafePostLoginRedirect,
|
||||
POST_LOGIN_REDIRECT_STORAGE_KEY,
|
||||
setPostLoginRedirectPath,
|
||||
springAuth,
|
||||
} from "@app/auth/springAuthClient";
|
||||
import { startOAuthNavigation } from "@app/extensions/oauthNavigation";
|
||||
import apiClient from "@app/services/apiClient";
|
||||
import {
|
||||
@@ -382,4 +388,95 @@ describe("SpringAuthClient", () => {
|
||||
expect(result.error).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("post-login redirect path", () => {
|
||||
beforeEach(() => {
|
||||
sessionStorage.clear();
|
||||
});
|
||||
|
||||
describe("isSafePostLoginRedirect", () => {
|
||||
it("accepts same-origin paths with a single leading slash", () => {
|
||||
expect(isSafePostLoginRedirect("/share/abc123")).toBe(true);
|
||||
expect(isSafePostLoginRedirect("/workbench")).toBe(true);
|
||||
expect(isSafePostLoginRedirect("/share/abc?x=1")).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects empty, null, or non-string values", () => {
|
||||
expect(isSafePostLoginRedirect("")).toBe(false);
|
||||
expect(isSafePostLoginRedirect(null)).toBe(false);
|
||||
expect(isSafePostLoginRedirect(undefined)).toBe(false);
|
||||
expect(isSafePostLoginRedirect(42 as unknown)).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects protocol-relative and absolute URLs (open-redirect guard)", () => {
|
||||
expect(isSafePostLoginRedirect("//evil.example")).toBe(false);
|
||||
expect(isSafePostLoginRedirect("http://evil.example")).toBe(false);
|
||||
expect(isSafePostLoginRedirect("https://evil.example/x")).toBe(false);
|
||||
expect(isSafePostLoginRedirect("/\\evil")).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects auth-plumbing paths to avoid login loops", () => {
|
||||
expect(isSafePostLoginRedirect("/login")).toBe(false);
|
||||
expect(isSafePostLoginRedirect("/login?foo=1")).toBe(false);
|
||||
expect(isSafePostLoginRedirect("/auth/callback")).toBe(false);
|
||||
expect(isSafePostLoginRedirect("/oauth2/authorization/google")).toBe(
|
||||
false,
|
||||
);
|
||||
expect(isSafePostLoginRedirect("/saml2/authenticate/x")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("setPostLoginRedirectPath", () => {
|
||||
it("stores a safe path in sessionStorage", () => {
|
||||
setPostLoginRedirectPath("/share/abc123");
|
||||
expect(sessionStorage.getItem(POST_LOGIN_REDIRECT_STORAGE_KEY)).toBe(
|
||||
"/share/abc123",
|
||||
);
|
||||
});
|
||||
|
||||
it("clears any existing entry when given an unsafe value", () => {
|
||||
sessionStorage.setItem(POST_LOGIN_REDIRECT_STORAGE_KEY, "/share/old");
|
||||
setPostLoginRedirectPath("//evil.example");
|
||||
expect(
|
||||
sessionStorage.getItem(POST_LOGIN_REDIRECT_STORAGE_KEY),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("clears any existing entry when given null", () => {
|
||||
sessionStorage.setItem(POST_LOGIN_REDIRECT_STORAGE_KEY, "/share/old");
|
||||
setPostLoginRedirectPath(null);
|
||||
expect(
|
||||
sessionStorage.getItem(POST_LOGIN_REDIRECT_STORAGE_KEY),
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("consumePostLoginRedirectPath", () => {
|
||||
it("returns the stored path and clears it (single-use)", () => {
|
||||
sessionStorage.setItem(
|
||||
POST_LOGIN_REDIRECT_STORAGE_KEY,
|
||||
"/share/abc123",
|
||||
);
|
||||
expect(consumePostLoginRedirectPath()).toBe("/share/abc123");
|
||||
expect(
|
||||
sessionStorage.getItem(POST_LOGIN_REDIRECT_STORAGE_KEY),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null (and still clears) when the stored value is unsafe", () => {
|
||||
sessionStorage.setItem(
|
||||
POST_LOGIN_REDIRECT_STORAGE_KEY,
|
||||
"//evil.example",
|
||||
);
|
||||
expect(consumePostLoginRedirectPath()).toBeNull();
|
||||
expect(
|
||||
sessionStorage.getItem(POST_LOGIN_REDIRECT_STORAGE_KEY),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when nothing is stored", () => {
|
||||
expect(consumePostLoginRedirectPath()).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -53,6 +53,8 @@ const OAUTH_REDIRECT_COOKIE = "stirling_redirect_path";
|
||||
const OAUTH_REDIRECT_COOKIE_MAX_AGE = 60 * 5; // 5 minutes
|
||||
const DEFAULT_REDIRECT_PATH = `${BASE_PATH || ""}/auth/callback`;
|
||||
|
||||
export const POST_LOGIN_REDIRECT_STORAGE_KEY = "stirling_post_login_path";
|
||||
|
||||
function normalizeRedirectPath(target?: string): string {
|
||||
if (!target || typeof target !== "string") {
|
||||
return DEFAULT_REDIRECT_PATH;
|
||||
@@ -80,6 +82,52 @@ function persistRedirectPath(path: string): void {
|
||||
}
|
||||
}
|
||||
|
||||
// Same-origin relative path, not pointing at auth plumbing. Rejects protocol-relative
|
||||
// URLs to guard against open-redirect abuse if the stored value is tampered with.
|
||||
export function isSafePostLoginRedirect(path: unknown): path is string {
|
||||
if (typeof path !== "string" || path.length === 0) return false;
|
||||
if (!path.startsWith("/") || path.startsWith("//")) return false;
|
||||
if (path.startsWith("/\\")) return false;
|
||||
const lowered = path.toLowerCase();
|
||||
if (
|
||||
lowered.startsWith("/login") ||
|
||||
lowered.startsWith("/auth/") ||
|
||||
lowered.startsWith("/oauth2") ||
|
||||
lowered.startsWith("/saml2")
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export function setPostLoginRedirectPath(
|
||||
path: string | null | undefined,
|
||||
): void {
|
||||
try {
|
||||
if (typeof window === "undefined") return;
|
||||
if (isSafePostLoginRedirect(path)) {
|
||||
window.sessionStorage.setItem(POST_LOGIN_REDIRECT_STORAGE_KEY, path);
|
||||
} else {
|
||||
window.sessionStorage.removeItem(POST_LOGIN_REDIRECT_STORAGE_KEY);
|
||||
}
|
||||
} catch (_error) {
|
||||
// sessionStorage unavailable (private mode) — fail open
|
||||
}
|
||||
}
|
||||
|
||||
export function consumePostLoginRedirectPath(): string | null {
|
||||
try {
|
||||
if (typeof window === "undefined") return null;
|
||||
const value = window.sessionStorage.getItem(
|
||||
POST_LOGIN_REDIRECT_STORAGE_KEY,
|
||||
);
|
||||
window.sessionStorage.removeItem(POST_LOGIN_REDIRECT_STORAGE_KEY);
|
||||
return isSafePostLoginRedirect(value) ? value : null;
|
||||
} catch (_error) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
// Auth types
|
||||
export interface User {
|
||||
id: string;
|
||||
|
||||
@@ -2,14 +2,23 @@ import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import { render, waitFor } from "@testing-library/react";
|
||||
import { BrowserRouter } from "react-router-dom";
|
||||
import AuthCallback from "@app/routes/AuthCallback";
|
||||
import { springAuth } from "@app/auth/springAuthClient";
|
||||
import {
|
||||
POST_LOGIN_REDIRECT_STORAGE_KEY,
|
||||
springAuth,
|
||||
} from "@app/auth/springAuthClient";
|
||||
|
||||
// Mock springAuth
|
||||
vi.mock("@app/auth/springAuthClient", () => ({
|
||||
springAuth: {
|
||||
getSession: vi.fn(),
|
||||
},
|
||||
}));
|
||||
// Mock springAuth; keep the real redirect-path helpers.
|
||||
vi.mock("@app/auth/springAuthClient", async () => {
|
||||
const actual = await vi.importActual<
|
||||
typeof import("@app/auth/springAuthClient")
|
||||
>("@app/auth/springAuthClient");
|
||||
return {
|
||||
...actual,
|
||||
springAuth: {
|
||||
getSession: vi.fn(),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
// Mock useNavigate
|
||||
const mockNavigate = vi.fn();
|
||||
@@ -24,6 +33,7 @@ vi.mock("react-router-dom", async () => {
|
||||
describe("AuthCallback", () => {
|
||||
beforeEach(() => {
|
||||
localStorage.clear();
|
||||
sessionStorage.clear();
|
||||
vi.clearAllMocks();
|
||||
// Reset window.location.hash
|
||||
window.location.hash = "";
|
||||
@@ -149,6 +159,82 @@ describe("AuthCallback", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("should navigate to the stored post-login path when one is present", async () => {
|
||||
const mockToken = "oauth-jwt-token";
|
||||
const mockUser = {
|
||||
id: "123",
|
||||
email: "[email protected]",
|
||||
username: "oauthuser",
|
||||
role: "USER",
|
||||
};
|
||||
|
||||
window.location.hash = `#access_token=${mockToken}`;
|
||||
sessionStorage.setItem(POST_LOGIN_REDIRECT_STORAGE_KEY, "/share/abc123");
|
||||
|
||||
vi.mocked(springAuth.getSession).mockResolvedValueOnce({
|
||||
data: {
|
||||
session: {
|
||||
user: mockUser,
|
||||
access_token: mockToken,
|
||||
expires_in: 3600,
|
||||
expires_at: Date.now() + 3600000,
|
||||
},
|
||||
},
|
||||
error: null,
|
||||
});
|
||||
|
||||
render(
|
||||
<BrowserRouter>
|
||||
<AuthCallback />
|
||||
</BrowserRouter>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockNavigate).toHaveBeenCalledWith("/share/abc123", {
|
||||
replace: true,
|
||||
});
|
||||
});
|
||||
// Stored path is single-use
|
||||
expect(sessionStorage.getItem(POST_LOGIN_REDIRECT_STORAGE_KEY)).toBeNull();
|
||||
});
|
||||
|
||||
it("should fall back to home when the stored post-login path is unsafe", async () => {
|
||||
const mockToken = "oauth-jwt-token";
|
||||
const mockUser = {
|
||||
id: "123",
|
||||
email: "[email protected]",
|
||||
username: "oauthuser",
|
||||
role: "USER",
|
||||
};
|
||||
|
||||
window.location.hash = `#access_token=${mockToken}`;
|
||||
// Protocol-relative URL — must not be followed
|
||||
sessionStorage.setItem(POST_LOGIN_REDIRECT_STORAGE_KEY, "//evil.example");
|
||||
|
||||
vi.mocked(springAuth.getSession).mockResolvedValueOnce({
|
||||
data: {
|
||||
session: {
|
||||
user: mockUser,
|
||||
access_token: mockToken,
|
||||
expires_in: 3600,
|
||||
expires_at: Date.now() + 3600000,
|
||||
},
|
||||
},
|
||||
error: null,
|
||||
});
|
||||
|
||||
render(
|
||||
<BrowserRouter>
|
||||
<AuthCallback />
|
||||
</BrowserRouter>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockNavigate).toHaveBeenCalledWith("/", { replace: true });
|
||||
});
|
||||
expect(sessionStorage.getItem(POST_LOGIN_REDIRECT_STORAGE_KEY)).toBeNull();
|
||||
});
|
||||
|
||||
it("should display loading state while processing", () => {
|
||||
window.location.hash = "#access_token=processing-token";
|
||||
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { useEffect, useRef } from "react";
|
||||
import { useNavigate } from "react-router-dom";
|
||||
import { springAuth } from "@app/auth/springAuthClient";
|
||||
import {
|
||||
consumePostLoginRedirectPath,
|
||||
springAuth,
|
||||
} from "@app/auth/springAuthClient";
|
||||
import { handleAuthCallbackSuccess } from "@app/extensions/authCallback";
|
||||
import styles from "@app/routes/AuthCallback.module.css";
|
||||
|
||||
@@ -157,12 +160,11 @@ export default function AuthCallback() {
|
||||
`[AuthCallback:${executionId}] Elapsed after stabilization wait: ${(performance.now() - startTime).toFixed(2)}ms`,
|
||||
);
|
||||
|
||||
const target = consumePostLoginRedirectPath() ?? "/";
|
||||
console.log(
|
||||
`[AuthCallback:${executionId}] Step 7: Navigating to home page`,
|
||||
`[AuthCallback:${executionId}] Step 7: Navigating to ${target}`,
|
||||
);
|
||||
|
||||
// Clear the hash from URL and redirect to home page
|
||||
navigate("/", { replace: true });
|
||||
navigate(target, { replace: true });
|
||||
|
||||
const duration = performance.now() - startTime;
|
||||
console.log(
|
||||
|
||||
@@ -40,13 +40,19 @@ vi.mock("@app/auth/UseSession", () => ({
|
||||
useAuth: vi.fn(),
|
||||
}));
|
||||
|
||||
// Mock springAuth
|
||||
vi.mock("@app/auth/springAuthClient", () => ({
|
||||
springAuth: {
|
||||
signInWithPassword: vi.fn(),
|
||||
signInWithOAuth: vi.fn(),
|
||||
},
|
||||
}));
|
||||
// Mock springAuth; keep the real redirect-path helpers.
|
||||
vi.mock("@app/auth/springAuthClient", async () => {
|
||||
const actual = await vi.importActual<
|
||||
typeof import("@app/auth/springAuthClient")
|
||||
>("@app/auth/springAuthClient");
|
||||
return {
|
||||
...actual,
|
||||
springAuth: {
|
||||
signInWithPassword: vi.fn(),
|
||||
signInWithOAuth: vi.fn(),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
// Mock useDocumentMeta
|
||||
vi.mock("@app/hooks/useDocumentMeta", () => ({
|
||||
@@ -692,4 +698,55 @@ describe("Login", () => {
|
||||
expect(submitButton).not.toBeDisabled();
|
||||
});
|
||||
});
|
||||
|
||||
it("should persist location.state.from.pathname before triggering SSO so the user returns to their original URL", async () => {
|
||||
const user = userEvent.setup();
|
||||
sessionStorage.clear();
|
||||
|
||||
vi.mocked(apiClient.get).mockResolvedValue({
|
||||
data: {
|
||||
enableLogin: true,
|
||||
providerList: {
|
||||
"/oauth2/authorization/authentik": "Authentik",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
vi.mocked(springAuth.signInWithOAuth).mockResolvedValueOnce({
|
||||
error: null,
|
||||
});
|
||||
|
||||
render(
|
||||
<TestWrapper>
|
||||
<MemoryRouter
|
||||
initialEntries={[
|
||||
{
|
||||
pathname: "/login",
|
||||
state: { from: { pathname: "/share/abc123" } },
|
||||
},
|
||||
]}
|
||||
>
|
||||
<Login />
|
||||
</MemoryRouter>
|
||||
</TestWrapper>,
|
||||
);
|
||||
|
||||
await waitFor(
|
||||
() => {
|
||||
expect(screen.getByText("Authentik")).toBeTruthy();
|
||||
},
|
||||
{ timeout: 3000 },
|
||||
);
|
||||
|
||||
await user.click(screen.getByText("Authentik"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(springAuth.signInWithOAuth).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Must be stashed before the cross-origin SSO redirect wipes location.state.
|
||||
expect(sessionStorage.getItem("stirling_post_login_path")).toBe(
|
||||
"/share/abc123",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,7 +6,10 @@ import {
|
||||
useSearchParams,
|
||||
} from "react-router-dom";
|
||||
import { Text, Stack, Alert } from "@mantine/core";
|
||||
import { springAuth } from "@app/auth/springAuthClient";
|
||||
import {
|
||||
setPostLoginRedirectPath,
|
||||
springAuth,
|
||||
} from "@app/auth/springAuthClient";
|
||||
import { useAuth } from "@app/auth/UseSession";
|
||||
import { useAppConfig } from "@app/contexts/AppConfigContext";
|
||||
import { useTranslation } from "react-i18next";
|
||||
@@ -34,6 +37,19 @@ export default function Login() {
|
||||
const location = useLocation();
|
||||
const [searchParams] = useSearchParams();
|
||||
const { session, loading } = useAuth();
|
||||
const resolveReturnPath = (): string | null => {
|
||||
const fromState = (
|
||||
location.state as { from?: { pathname?: string } } | null
|
||||
)?.from?.pathname;
|
||||
if (fromState) return fromState;
|
||||
const fromQuery = searchParams.get("from");
|
||||
if (!fromQuery) return null;
|
||||
try {
|
||||
return decodeURIComponent(fromQuery);
|
||||
} catch {
|
||||
return fromQuery;
|
||||
}
|
||||
};
|
||||
const { refetch } = useAppConfig();
|
||||
const { t } = useTranslation();
|
||||
const [isSigningIn, setIsSigningIn] = useState(false);
|
||||
@@ -166,15 +182,13 @@ export default function Login() {
|
||||
// Redirect immediately if user has valid session (JWT already validated by AuthProvider)
|
||||
useEffect(() => {
|
||||
if (!loading && session) {
|
||||
const returnPath = (
|
||||
location.state as { from?: { pathname?: string } } | null
|
||||
)?.from?.pathname;
|
||||
const returnPath = resolveReturnPath();
|
||||
console.debug("[Login] User already authenticated, redirecting to home", {
|
||||
returnPath,
|
||||
});
|
||||
navigate(returnPath || "/", { replace: true });
|
||||
}
|
||||
}, [session, loading, navigate, location.state]);
|
||||
}, [session, loading, navigate, location.state, searchParams]);
|
||||
|
||||
// If backend reports login is disabled, redirect to home (anonymous mode)
|
||||
useEffect(() => {
|
||||
@@ -267,6 +281,12 @@ export default function Login() {
|
||||
setError(null);
|
||||
clearLogoutBlock();
|
||||
|
||||
// Don't overwrite a path already stashed by httpErrorHandler on a prior 401.
|
||||
const returnPath = resolveReturnPath();
|
||||
if (returnPath) {
|
||||
setPostLoginRedirectPath(returnPath);
|
||||
}
|
||||
|
||||
console.log(`[Login] Signing in with provider: ${provider}`);
|
||||
|
||||
// Redirect to Spring OAuth2 endpoint using the actual provider ID from backend
|
||||
|
||||
@@ -66,6 +66,7 @@ services:
|
||||
- ../../../stirling/keycloak-saml-test/data:/usr/share/tessdata:rw
|
||||
- ../../../stirling/keycloak-saml-test/config:/configs:rw
|
||||
- ../../../stirling/keycloak-saml-test/logs:/logs:rw
|
||||
- ../../../stirling/keycloak-saml-test/storage:/storage:rw
|
||||
- ./keycloak-saml-cert.pem:/app/keycloak-saml-cert.pem:ro
|
||||
- ./saml-private-key.key:/app/saml-private-key.key:ro
|
||||
- ./saml-public-cert.crt:/app/saml-public-cert.crt:ro
|
||||
@@ -82,6 +83,18 @@ services:
|
||||
PREMIUM_ENABLED: "true"
|
||||
PREMIUM_PROFEATURES_SSOAUTOLOGIN: "${PREMIUM_PROFEATURES_SSOAUTOLOGIN:-false}"
|
||||
|
||||
# Storage + sharing (opt-in via start-saml-test.sh --with-storage)
|
||||
STORAGE_ENABLED: "${STORAGE_ENABLED:-false}"
|
||||
STORAGE_PROVIDER: "${STORAGE_PROVIDER:-local}"
|
||||
STORAGE_LOCAL_BASEPATH: "${STORAGE_LOCAL_BASEPATH:-/storage}"
|
||||
STORAGE_SHARING_ENABLED: "${STORAGE_SHARING_ENABLED:-false}"
|
||||
STORAGE_SHARING_LINKENABLED: "${STORAGE_SHARING_LINKENABLED:-false}"
|
||||
STORAGE_SHARING_EMAILENABLED: "${STORAGE_SHARING_EMAILENABLED:-false}"
|
||||
STORAGE_SHARING_LINKEXPIRATIONDAYS: "${STORAGE_SHARING_LINKEXPIRATIONDAYS:-3}"
|
||||
STORAGE_SIGNING_ENABLED: "${STORAGE_SIGNING_ENABLED:-false}"
|
||||
# Required for share-link creation (FileStorageService.isShareLinksEnabled)
|
||||
SYSTEM_FRONTENDURL: "${SYSTEM_FRONTENDURL:-}"
|
||||
|
||||
# Debug Logging
|
||||
LOGGING_LEVEL_ORG_SPRINGFRAMEWORK_SECURITY_SAML2: DEBUG
|
||||
LOGGING_LEVEL_ORG_OPENSAML: DEBUG
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
#!/bin/bash
|
||||
set -e
|
||||
|
||||
# Stop Git Bash / MSYS from mangling Unix-style paths (e.g. /storage) passed
|
||||
# to docker-compose.exe. No-op on native Linux/macOS.
|
||||
export MSYS_NO_PATHCONV=1
|
||||
export MSYS2_ARG_CONV_EXCL="*"
|
||||
|
||||
RED='\033[0;31m'
|
||||
GREEN='\033[0;32m'
|
||||
YELLOW='\033[1;33m'
|
||||
@@ -13,6 +18,7 @@ echo -e "${BLUE}╚════════════════════
|
||||
echo ""
|
||||
|
||||
AUTO_LOGIN=false
|
||||
WITH_STORAGE=false
|
||||
DEFAULT_LANGUAGE="en-US"
|
||||
COMPOSE_UP_ARGS=(-d --build)
|
||||
while [[ $# -gt 0 ]]; do
|
||||
@@ -21,6 +27,10 @@ while [[ $# -gt 0 ]]; do
|
||||
AUTO_LOGIN=true
|
||||
shift
|
||||
;;
|
||||
--with-storage)
|
||||
WITH_STORAGE=true
|
||||
shift
|
||||
;;
|
||||
--nobuild)
|
||||
COMPOSE_UP_ARGS=(-d)
|
||||
shift
|
||||
@@ -46,11 +56,13 @@ while [[ $# -gt 0 ]]; do
|
||||
shift 2
|
||||
;;
|
||||
-h|--help)
|
||||
echo "Usage: $0 [--auto] [--nobuild] [--language <locale>]"
|
||||
echo "Usage: $0 [--auto] [--with-storage] [--nobuild] [--language <locale>]"
|
||||
echo ""
|
||||
echo " --auto Enable SSO auto-login and force SAML-only login method"
|
||||
echo " --nobuild Skip building images (use existing images)"
|
||||
echo " --language Set system default locale (e.g. de-DE, sv-SE)"
|
||||
echo " --auto Enable SSO auto-login and force SAML-only login method"
|
||||
echo " --with-storage Enable the file storage + link-sharing feature"
|
||||
echo " (required to test /share/<token> flows)"
|
||||
echo " --nobuild Skip building images (use existing images)"
|
||||
echo " --language Set system default locale (e.g. de-DE, sv-SE)"
|
||||
exit 0
|
||||
;;
|
||||
*)
|
||||
@@ -89,6 +101,26 @@ if [ "$AUTO_LOGIN" = true ]; then
|
||||
echo ""
|
||||
fi
|
||||
|
||||
if [ "$WITH_STORAGE" = true ]; then
|
||||
export STORAGE_ENABLED=true
|
||||
export STORAGE_PROVIDER=local
|
||||
export STORAGE_LOCAL_BASEPATH=/storage
|
||||
export STORAGE_SHARING_ENABLED=true
|
||||
export STORAGE_SHARING_LINKENABLED=true
|
||||
export STORAGE_SHARING_EMAILENABLED=true
|
||||
export STORAGE_SHARING_LINKEXPIRATIONDAYS=3
|
||||
# storage.signing is a sibling of storage.sharing, not nested under it
|
||||
export STORAGE_SIGNING_ENABLED=true
|
||||
# Required for share-link creation (FileStorageService.isShareLinksEnabled)
|
||||
export SYSTEM_FRONTENDURL="http://localhost:8080"
|
||||
# Force recreate so env changes apply even with --nobuild
|
||||
if [[ ! " ${COMPOSE_UP_ARGS[*]} " =~ " --force-recreate " ]]; then
|
||||
COMPOSE_UP_ARGS+=(--force-recreate)
|
||||
fi
|
||||
echo -e "${GREEN}✓ Storage + link sharing enabled${NC}"
|
||||
echo ""
|
||||
fi
|
||||
|
||||
export SYSTEM_DEFAULTLOCALE="$DEFAULT_LANGUAGE"
|
||||
echo -e "${GREEN}✓ Default locale set to: ${SYSTEM_DEFAULTLOCALE}${NC}"
|
||||
echo ""
|
||||
@@ -199,6 +231,14 @@ echo -e " 1. Go to ${GREEN}http://localhost:8080${NC}"
|
||||
echo -e " 2. Click 'Login' and select SAML"
|
||||
echo -e " 3. Login with test credentials"
|
||||
echo ""
|
||||
if [ "$WITH_STORAGE" = true ]; then
|
||||
echo -e "${BLUE}🔗 Test share links:${NC}"
|
||||
echo -e " 1. Log in as ${GREEN}[email protected]${NC}, upload a PDF"
|
||||
echo -e " 2. Create a share link from the file manager"
|
||||
echo -e " 3. Open the share URL in an incognito/private window"
|
||||
echo -e " 4. Verify you land on the share page (not the home page) after SSO"
|
||||
echo ""
|
||||
fi
|
||||
echo -e "${BLUE}📊 View logs:${NC}"
|
||||
echo -e " docker-compose -f docker-compose-keycloak-saml.yml logs -f"
|
||||
echo ""
|
||||
|
||||
Reference in New Issue
Block a user