fabi/PiCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(stage-2): audit dashboard TS alignment + login UX

Browse Source 
Closes 4 audit findings:

- Route TS interface now carries dispatch_mode (sync|async). Backend's
  shared::route::Route has had this since 0012_routes_dispatch_mode but
  the TS client silently always created sync routes and the list
  display dropped the field. Add a Dispatch select to the new-route
  form and an "ASYNC" badge in the route list.

- api.files.downloadUrl pointed to a never-registered backend endpoint.
  The dashboard's live Download button was hitting 405. Add the GET
  handler (AppFilesRead + FilesRepo::head + FilesRepo::get, Content-
  Disposition: inline) at the same path that delete already used.

- F-T-003: adminRequest's 401 handler called goto(login) without a
  recursion cap. If the login endpoint itself returned 401, the wrapper
  looped until browser nav limits. Track consecutive 401s within a 10s
  window and hard-reload to /login?reason=auth-loop after the third,
  showing the user an explanatory banner. Any 2xx resets the counter.

- Login flow now honors a ?returnTo= query parameter. adminRequest and
  the root layout both append the current location when redirecting to
  /login; the login page validates the value is a same-origin admin
  path (no open-redirect) and goto's there after successful sign-in.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-10 21:09:53 +02:00
parent 3c9816daf3
commit f466b2a15e
5 changed files with 166 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
crates/manager-core/src/files_api.rs
View File

@@ -15,10 +15,12 @@
use std::sync::Arc;
use axum::body::Body;
use axum::extract::{Path, Query, State};
use axum::http::header::{CONTENT_DISPOSITION, CONTENT_LENGTH, CONTENT_TYPE};
use axum::http::StatusCode;
use axum::response::{IntoResponse, Json, Response};
use axum::routing::{delete, get};
use axum::routing::get;
use axum::{Extension, Router};
use picloud_shared::{AppId, Principal};
use serde::{Deserialize, Serialize};
@@ -41,7 +43,7 @@ pub fn files_admin_router(state: FilesAdminState) -> Router {
.route("/apps/{app_id}/files", get(list_files))
.route(
"/apps/{app_id}/files/{collection}/{file_id}",
delete(delete_file),
get(get_file).delete(delete_file),
)
.with_state(state)
}
@@ -120,6 +122,48 @@ async fn list_files(
}))
}
/// `GET /apps/{id}/files/{collection}/{file_id}` — stream the file's
/// bytes inline. The dashboard's Download button hits this; the audit
/// flagged the missing handler (callers were 405'ing). Metadata gives
/// the Content-Type + Content-Disposition; bytes come from
/// `FilesRepo::get` (which checksum-verifies on read).
async fn get_file(
State(s): State<FilesAdminState>,
Extension(principal): Extension<Principal>,
Path((id_or_slug, collection, file_id)): Path<(String, String, String)>,
) -> Result<Response, FilesApiError> {
let app_id = resolve_app(&*s.apps, &id_or_slug).await?;
require(
s.authz.as_ref(),
&principal,
Capability::AppFilesRead(app_id),
)
.await?;
let id = Uuid::parse_str(&file_id).map_err(|_| FilesApiError::NotFound)?;
let meta = s
.files
.head(app_id, &collection, id)
.await?
.ok_or(FilesApiError::NotFound)?;
let bytes = s
.files
.get(app_id, &collection, id)
.await?
.ok_or(FilesApiError::NotFound)?;
let disposition = format!(
"inline; filename=\"{}\"",
meta.name.replace('"', "").replace(['\r', '\n'], "")
);
let len = bytes.len();
Ok(Response::builder()
.status(StatusCode::OK)
.header(CONTENT_TYPE, meta.content_type)
.header(CONTENT_DISPOSITION, disposition)
.header(CONTENT_LENGTH, len)
.body(Body::from(bytes))
.expect("build files download response"))
}
async fn delete_file(
State(s): State<FilesAdminState>,
Extension(principal): Extension<Principal>,

51
dashboard/src/lib/api.ts
View File

@@ -100,6 +100,7 @@ export interface PatchAppInput {
export type HostKind = 'any' | 'strict' | 'wildcard';
export type PathKind = 'exact' | 'prefix' | 'param';
export type DispatchMode = 'sync' | 'async';
export interface Route {
id: string;
@@ -111,6 +112,7 @@ export interface Route {
path_kind: PathKind;
path: string;
method: string | null;
dispatch_mode: DispatchMode;
created_at: string;
}
@@ -121,6 +123,7 @@ export interface RouteInput {
path_kind: PathKind;
path: string;
method?: string | null;
dispatch_mode?: DispatchMode;
}
export interface CheckRouteResponse {
@@ -461,6 +464,27 @@ export class ApiError extends Error {
}
}
// F-T-003: if the server keeps returning 401 (e.g. the bounce to
// /login itself triggered another 401-emitting request), the previous
// implementation would just keep calling `goto(login)` — a tight loop.
// Track consecutive 401s; after the threshold within the window, hard-
// reload to a static auth-loop page so the user sees something other
// than a spinning dashboard. Any 2xx response resets the count.
let consecutive401Count = 0;
let first401At = 0;
const AUTH_LOOP_THRESHOLD = 3;
const AUTH_LOOP_WINDOW_MS = 10_000;
function loginUrlWithReturnTo(): string {
if (!browser) return `${base}/login`;
const here = `${window.location.pathname}${window.location.search}`;
// Don't loop returnTo back to /login itself.
if (here === `${base}/login` || here.startsWith(`${base}/login?`)) {
return `${base}/login`;
}
return `${base}/login?returnTo=${encodeURIComponent(here)}`;
}
async function adminRequest<T>(path: string, init?: RequestInit): Promise<T> {
const headers: Record<string, string> = {
'content-type': 'application/json',
@@ -478,9 +502,27 @@ async function adminRequest<T>(path: string, init?: RequestInit): Promise<T> {
// and bounce to login — unless we're already on it, in which
// case throw and let the login form render the error.
clearSession();
if (browser && !window.location.pathname.endsWith('/login')) {
void goto(`${base}/login`);
const now = Date.now();
if (now - first401At > AUTH_LOOP_WINDOW_MS) {
first401At = now;
consecutive401Count = 1;
} else {
consecutive401Count += 1;
}
if (browser && !window.location.pathname.endsWith('/login')) {
if (consecutive401Count >= AUTH_LOOP_THRESHOLD) {
// Hard-reload via location.assign — Svelte's goto keeps
// the SPA state which is what's keeping the loop alive.
window.location.assign(`${base}/login?reason=auth-loop`);
} else {
void goto(loginUrlWithReturnTo());
}
}
} else if (res.ok) {
// Any successful response resets the loop counter — the user has
// a valid session again.
consecutive401Count = 0;
first401At = 0;
}
if (!res.ok) {
const message =
@@ -908,8 +950,9 @@ export const api = {
`/api/v1/admin/apps/${encodeURIComponent(idOrSlug)}/files/${encodeURIComponent(collection)}/${fileId}`,
{ method: 'DELETE' }
),
/// F-U-009: build the admin download URL. GET returns the file
/// bytes inline; the browser handles content-disposition.
// Build the admin download URL. GET returns the file bytes
// inline with Content-Disposition set from the stored filename;
// the browser handles the download.
downloadUrl: (idOrSlug: string, collection: string, fileId: string): string =>
`/api/v1/admin/apps/${encodeURIComponent(idOrSlug)}/files/${encodeURIComponent(collection)}/${fileId}`
},

10
dashboard/src/routes/+layout.svelte
View File

@@ -14,6 +14,14 @@
const isLoginRoute = $derived(page.url.pathname.endsWith('/login'));
function loginRedirectTarget(): string {
const here = `${page.url.pathname}${page.url.search}`;
if (here === `${base}/login` || here.startsWith(`${base}/login?`)) {
return `${base}/login`;
}
return `${base}/login?returnTo=${encodeURIComponent(here)}`;
}
onMount(async () => {
// Hydrate the session: if there's a token, ask the server who we
// are. On 401 the fetch wrapper already redirects to /login and
@@ -21,7 +29,7 @@
const tok = getToken();
if (!tok) {
if (!isLoginRoute) {
await goto(`${base}/login`);
await goto(loginRedirectTarget());
}
booting = false;
return;

40
dashboard/src/routes/login/+page.svelte
View File

@@ -9,13 +9,37 @@
let password = $state('');
let pending = $state(false);
let error = $state<string | null>(null);
let notice = $state<string | null>(null);
// Honor returnTo from the auth bounce so deep links survive a
// re-login. Validate it's a same-origin admin path (no open
// redirect): must start with the admin base prefix, no scheme, no
// host. Falls back to the dashboard root.
function safeReturnTo(): string {
if (typeof window === 'undefined') return `${base}/`;
const params = new URLSearchParams(window.location.search);
const raw = params.get('returnTo');
if (!raw) return `${base}/`;
if (raw.includes('://') || raw.startsWith('//')) return `${base}/`;
if (!raw.startsWith(`${base}/`) && !raw.startsWith('/')) return `${base}/`;
// Don't bounce back into /login.
if (raw === `${base}/login` || raw.startsWith(`${base}/login?`)) return `${base}/`;
return raw;
}
onMount(async () => {
if (typeof window !== 'undefined') {
const reason = new URLSearchParams(window.location.search).get('reason');
if (reason === 'auth-loop') {
notice =
'Your session keeps being rejected. Sign in again — if this keeps happening, the admin token issuer may be misconfigured.';
}
}
// Already signed in? Skip the form.
if (!getToken()) return;
try {
await api.auth.me();
await goto(`${base}/`);
await goto(safeReturnTo());
} catch {
// stale token; let the form render
}
@@ -27,7 +51,7 @@
pending = true;
try {
await api.auth.login(username, password);
await goto(`${base}/`);
await goto(safeReturnTo());
} catch (e) {
error = e instanceof ApiError ? e.message : 'Login failed';
} finally {
@@ -65,6 +89,9 @@
<button type="submit" disabled={pending}>
{pending ? 'Signing in…' : 'Sign in →'}
</button>
{#if notice}
<div class="notice">{notice}</div>
{/if}
{#if error}
<div class="error">{error}</div>
{/if}
@@ -156,6 +183,15 @@
font-size: 0.85rem;
}
.notice {
background: #422006;
border: 1px solid #b45309;
color: #fde68a;
padding: 0.5rem 0.75rem;
border-radius: 0.375rem;
font-size: 0.85rem;
}
.hint {
margin: 0;
font-size: 0.75rem;

27
dashboard/src/routes/scripts/[id]/+page.svelte
View File

@@ -209,6 +209,7 @@
// canonical display form for an unrestricted host.
let newRouteHost = $state('*');
let newRouteMethod = $state('');
let newRouteDispatchMode = $state<'sync' | 'async'>('sync');
let pathKindAutoUpdate = $state(true);
let creatingRoute = $state(false);
let createRouteError = $state<string | null>(null);
@@ -255,13 +256,15 @@
host: parsedHost.host,
path_kind: newRoutePathKind,
path: newRoutePath.trim(),
method: newRouteMethod.trim() || null
method: newRouteMethod.trim() || null,
dispatch_mode: newRouteDispatchMode
};
await api.routes.create(id, input);
showAddRoute = false;
newRoutePath = '/';
newRouteHost = '*';
newRouteMethod = '';
newRouteDispatchMode = 'sync';
pathKindAutoUpdate = true;
await loadRoutes();
} catch (e) {
@@ -619,6 +622,13 @@
<option value="PATCH">PATCH</option>
</select>
</label>
<label>
<span>Dispatch</span>
<select bind:value={newRouteDispatchMode}>
<option value="sync">sync (wait for response)</option>
<option value="async">async (202 + run later)</option>
</select>
</label>
</div>
<label class="full">
<span class="host-label">
@@ -695,6 +705,9 @@
: r.host}
</span>
<span class="path">{r.path}</span>
{#if r.dispatch_mode === 'async'}
<span class="dispatch dispatch-async" title="Dispatch mode">async</span>
{/if}
{#if canWrite}
<button type="button" class="link danger" onclick={() => requestRemoveRoute(r)}>
remove
@@ -1258,6 +1271,18 @@
color: #e2e8f0;
flex: 1;
}
.route-row .dispatch {
font-size: 0.6rem;
font-weight: 700;
text-transform: uppercase;
padding: 0.1rem 0.4rem;
border-radius: 0.25rem;
letter-spacing: 0.03em;
}
.dispatch-async {
background: #422006;
color: #fcd34d;
}
.route-url {
font-family:
ui-monospace, SFMono-Regular, Menlo, Consolas, 'Liberation Mono', monospace;