diff --git a/.taskfiles/backend.yml b/.taskfiles/backend.yml index 08a12b9535..3e43321937 100644 --- a/.taskfiles/backend.yml +++ b/.taskfiles/backend.yml @@ -40,12 +40,15 @@ tasks: AIENGINE_ENABLED: '{{.AIENGINE_ENABLED | default "false"}}' AIENGINE_TIMEOUTSECONDS: '{{.AIENGINE_TIMEOUTSECONDS | default "120"}}' SECURITY_ENABLELOGIN: '{{.SECURITY_ENABLELOGIN | default ""}}' + # Set by dev:linked. Inline rather than in `env:` so an empty value emits nothing + # and cannot blank the committed default. + ACCOUNT_LINK_SAAS_BASE_URL: '{{.ACCOUNT_LINK_SAAS_BASE_URL | default ""}}' env: SERVER_PORT: '{{.PORT}}' cmds: - - cmd: '{{if .AIENGINE_URL}}AIENGINE_URL={{.AIENGINE_URL}} AIENGINE_ENABLED={{.AIENGINE_ENABLED}} AIENGINE_TIMEOUTSECONDS={{.AIENGINE_TIMEOUTSECONDS}} {{end}}{{if .SECURITY_ENABLELOGIN}}SECURITY_ENABLELOGIN={{.SECURITY_ENABLELOGIN}} {{end}}cmd /c ".\gradlew.bat :stirling-pdf:bootRun"' + - cmd: '{{if .AIENGINE_URL}}AIENGINE_URL={{.AIENGINE_URL}} AIENGINE_ENABLED={{.AIENGINE_ENABLED}} AIENGINE_TIMEOUTSECONDS={{.AIENGINE_TIMEOUTSECONDS}} {{end}}{{if .SECURITY_ENABLELOGIN}}SECURITY_ENABLELOGIN={{.SECURITY_ENABLELOGIN}} {{end}}{{if .ACCOUNT_LINK_SAAS_BASE_URL}}STIRLING_BILLING_ACCOUNT_LINK_ENABLED=true STIRLING_BILLING_ACCOUNT_LINK_SAAS_BASE_URL={{.ACCOUNT_LINK_SAAS_BASE_URL}} {{end}}cmd /c ".\gradlew.bat :stirling-pdf:bootRun"' platforms: [windows] - - cmd: '{{if .AIENGINE_URL}}AIENGINE_URL={{.AIENGINE_URL}} AIENGINE_ENABLED={{.AIENGINE_ENABLED}} AIENGINE_TIMEOUTSECONDS={{.AIENGINE_TIMEOUTSECONDS}} {{end}}{{if .SECURITY_ENABLELOGIN}}SECURITY_ENABLELOGIN={{.SECURITY_ENABLELOGIN}} {{end}}./gradlew :stirling-pdf:bootRun' + - cmd: '{{if .AIENGINE_URL}}AIENGINE_URL={{.AIENGINE_URL}} AIENGINE_ENABLED={{.AIENGINE_ENABLED}} AIENGINE_TIMEOUTSECONDS={{.AIENGINE_TIMEOUTSECONDS}} {{end}}{{if .SECURITY_ENABLELOGIN}}SECURITY_ENABLELOGIN={{.SECURITY_ENABLELOGIN}} {{end}}{{if .ACCOUNT_LINK_SAAS_BASE_URL}}STIRLING_BILLING_ACCOUNT_LINK_ENABLED=true STIRLING_BILLING_ACCOUNT_LINK_SAAS_BASE_URL={{.ACCOUNT_LINK_SAAS_BASE_URL}} {{end}}./gradlew :stirling-pdf:bootRun' platforms: [linux, darwin] dev:bundled: @@ -84,6 +87,8 @@ tasks: AIENGINE_URL: '{{.AIENGINE_URL}}' AIENGINE_ENABLED: '{{.AIENGINE_ENABLED}}' AIENGINE_TIMEOUTSECONDS: '{{.AIENGINE_TIMEOUTSECONDS}}' + APP_BASE_URL: '{{.APP_BASE_URL}}' + BASE_PATH: '{{.BASE_PATH}}' staging:saas: desc: "Start SaaS backend against the shared v3 staging project" @@ -95,10 +100,47 @@ tasks: AIENGINE_URL: '{{.AIENGINE_URL}}' AIENGINE_ENABLED: '{{.AIENGINE_ENABLED}}' AIENGINE_TIMEOUTSECONDS: '{{.AIENGINE_TIMEOUTSECONDS}}' + APP_BASE_URL: '{{.APP_BASE_URL}}' + BASE_PATH: '{{.BASE_PATH}}' + + dev:linked: + desc: "Self-hosted backend linked to a locally running SaaS backend (see task linked:*)" + ignore_error: true + vars: + PORT: '{{.PORT | default "8080"}}' + SAAS_BASE_URL: '{{.SAAS_BASE_URL | default "http://localhost:8081"}}' + cmds: + - 'echo ">> self-hosted :{{.PORT}} linking to SaaS at {{.SAAS_BASE_URL}}"' + # The two backends run different STIRLING_FLAVOURs, which are different Gradle + # project graphs sharing one build/ tree. Waiting avoids overlapping builds; it + # does not make the sharing safe, so avoid rebuilding one while the other runs. + - cmd: | + n=0 + while [ "$n" -lt 150 ]; do + if curl -s -m 2 "{{.SAAS_BASE_URL}}" >/dev/null 2>&1; then + echo ">> SaaS backend is up, starting self-hosted" + break + fi + n=$((n + 1)) + {{if eq OS "windows"}}powershell -NoProfile -Command "Start-Sleep -Seconds 2"{{else}}sleep 2{{end}} + done + if [ "$n" -ge 150 ]; then + echo ">> SaaS backend never answered; starting anyway" + fi + - task: dev:proprietary + vars: + PORT: '{{.PORT}}' + ACCOUNT_LINK_SAAS_BASE_URL: '{{.SAAS_BASE_URL}}' _run:saas: internal: true - dotenv: ['app/.env.saas.local', 'app/.env.saas'] + # The frontend files are here only for RUN_SUBPATH, which the authorize URL needs. + # Last, because dotenv is set-if-absent: app/* still decides everything else. + dotenv: + - 'app/.env.saas.local' + - 'app/.env.saas' + - 'frontend/editor/.env.saas.local' + - 'frontend/editor/.env.saas' ignore_error: true vars: PORT: '{{.PORT | default "8080"}}' @@ -111,12 +153,29 @@ tasks: AIENGINE_URL: '{{.AIENGINE_URL | default ""}}' AIENGINE_ENABLED: '{{.AIENGINE_ENABLED | default "false"}}' AIENGINE_TIMEOUTSECONDS: '{{.AIENGINE_TIMEOUTSECONDS | default "120"}}' + # Empty is the same as unset: the property defaults to empty and is blank-checked. + APP_BASE_URL: '{{.APP_BASE_URL | default ""}}' + # Relocates configs/pipeline/logs, for a second backend in the same directory. + # Empty is the same as unset: the reader blank-checks it. + BASE_PATH: '{{.BASE_PATH | default ""}}' env: SERVER_PORT: '{{.PORT}}' STIRLING_FLAVOR: saas + STIRLING_BASE_PATH: '{{.BASE_PATH}}' AIENGINE_URL: '{{.AIENGINE_URL}}' AIENGINE_ENABLED: '{{.AIENGINE_ENABLED}}' AIENGINE_TIMEOUTSECONDS: '{{.AIENGINE_TIMEOUTSECONDS}}' + # Appends RUN_SUBPATH: the approval page is at /link, so a subpath build + # serves it at /app/link. An explicit value still wins. + SYSTEM_FRONTENDURL: + sh: | + if [ -n "${SYSTEM_FRONTENDURL:-}" ]; then + echo "${SYSTEM_FRONTENDURL}" + elif [ -n "{{.APP_BASE_URL}}" ] && [ -n "${RUN_SUBPATH:-}" ]; then + echo "{{.APP_BASE_URL}}/${RUN_SUBPATH}" + else + echo "{{.APP_BASE_URL}}" + fi cmds: # PROFILE_ARGS is empty when PROFILES=none, i.e. the bare `saas` profile # against SAAS_DB_* (production). diff --git a/.taskfiles/frontend.yml b/.taskfiles/frontend.yml index 844306824d..7867315037 100644 --- a/.taskfiles/frontend.yml +++ b/.taskfiles/frontend.yml @@ -121,17 +121,17 @@ tasks: sh: | case "${SAAS_ENV:-dev}" in staging) ref="${SAAS_STAGING_PROJECT_REF:?set it in app/.env.saas.local}" ;; - *) ref="${SAAS_DEV_PROJECT_REF:?set it in app/.env.saas.local, or run task staging:saas}" ;; + *) ref="${SAAS_DEV_PROJECT_REF:?set it in app/.env.saas.local, or pass SAAS_ENV=staging}" ;; esac echo "https://${ref}.supabase.co" VITE_SUPABASE_PUBLISHABLE_DEFAULT_KEY: sh: | case "${SAAS_ENV:-dev}" in staging) echo "${SAAS_STAGING_PUBLISHABLE_KEY:?set it in app/.env.saas.local}" ;; - *) echo "${SAAS_DEV_PUBLISHABLE_KEY:?set it in app/.env.saas.local}" ;; + *) echo "${SAAS_DEV_PUBLISHABLE_KEY:?set it in app/.env.saas.local, or pass SAAS_ENV=staging}" ;; esac cmds: - - 'echo ">> frontend Supabase target: $VITE_SUPABASE_URL"' + - 'echo ">> frontend {{.SAAS_ENV}}: Supabase $VITE_SUPABASE_URL, backend $BACKEND_URL"' - npx vite editor --mode saas --port {{.PORT}}{{if .OPEN}} --open{{end}} dev: @@ -173,6 +173,16 @@ tasks: OPEN: '{{.OPEN}}' SAAS_ENV: '{{.SAAS_ENV}}' + staging:saas: + desc: "Start frontend dev server against the shared v3 staging project" + cmds: + - task: dev:saas + vars: + SAAS_ENV: staging + PORT: '{{.PORT}}' + BACKEND_URL: '{{.BACKEND_URL}}' + OPEN: '{{.OPEN}}' + dev:desktop: desc: "Start frontend dev server in desktop mode" deps: diff --git a/Taskfile.yml b/Taskfile.yml index 92dcdcc742..e364242f1f 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -121,6 +121,92 @@ tasks: cmds: - task: dev:_all + # No engine: linking never calls it. + linked:staging: + desc: "SaaS on the shared v3 project + a self-hosted instance linked to it" + cmds: + - task: linked:_all + vars: { SAAS_ENV: staging } + + linked:dev: + desc: "SaaS on the current PR's preview branch + a self-hosted instance linked to it" + cmds: + - task: linked:_all + vars: { SAAS_ENV: dev } + + linked:_all: + internal: true + vars: + SAAS_ENV: '{{.SAAS_ENV | default "staging"}}' + PORTS: + sh: '{{if eq OS "windows"}}{{.FIND_FREE_PORT_PS}} 8081 5174 8080 5173{{else}}{{.FIND_FREE_PORT_SH}} 8081 5174 8080 5173{{end}}' + SAAS_BACKEND_PORT: '{{index (splitList "\n" .PORTS) 0}}' + SAAS_FRONTEND_PORT: '{{index (splitList "\n" .PORTS) 1}}' + APP_BACKEND_PORT: '{{index (splitList "\n" .PORTS) 2}}' + APP_FRONTEND_PORT: '{{index (splitList "\n" .PORTS) 3}}' + deps: + # APP_BASE_URL is the SaaS *frontend*: the approval page is served by vite, not + # by the API. BASE_PATH moves this backend's configs/pipeline aside so it does not + # race the self-hosted one, which keeps ./configs and its existing database. + - task: 'backend:{{.SAAS_ENV}}:saas' + vars: + PORT: '{{.SAAS_BACKEND_PORT}}' + APP_BASE_URL: 'http://localhost:{{.SAAS_FRONTEND_PORT}}' + BASE_PATH: 'tmp/linked-saas' + - task: frontend:dev:saas + vars: + PORT: '{{.SAAS_FRONTEND_PORT}}' + BACKEND_URL: 'http://localhost:{{.SAAS_BACKEND_PORT}}' + SAAS_ENV: '{{.SAAS_ENV}}' + - task: backend:dev:linked + vars: + PORT: '{{.APP_BACKEND_PORT}}' + SAAS_BASE_URL: 'http://localhost:{{.SAAS_BACKEND_PORT}}' + - task: frontend:dev:proprietary + vars: + PORT: '{{.APP_FRONTEND_PORT}}' + BACKEND_URL: 'http://localhost:{{.APP_BACKEND_PORT}}' + OPEN: "true" + - task: linked:_ready + vars: + SAAS_BACKEND_PORT: '{{.SAAS_BACKEND_PORT}}' + SAAS_FRONTEND_PORT: '{{.SAAS_FRONTEND_PORT}}' + APP_BACKEND_PORT: '{{.APP_BACKEND_PORT}}' + APP_FRONTEND_PORT: '{{.APP_FRONTEND_PORT}}' + + # Waits for all four to answer, then prints where they landed. + linked:_ready: + internal: true + cmds: + - cmd: | + n=0 + ok=0 + while [ "$n" -lt 150 ]; do + ok=1 + for u in "http://localhost:{{.SAAS_BACKEND_PORT}}" \ + "http://localhost:{{.SAAS_FRONTEND_PORT}}" \ + "http://localhost:{{.APP_BACKEND_PORT}}" \ + "http://localhost:{{.APP_FRONTEND_PORT}}"; do + # Not -o /dev/null: Windows curl.exe treats it as a real path and exits 23. + curl -s -m 2 "$u" >/dev/null 2>&1 || ok=0 + done + if [ "$ok" = 1 ]; then break; fi + n=$((n + 1)) + # `sleep` is a binary, not a builtin, and Windows has none. + {{if eq OS "windows"}}powershell -NoProfile -Command "Start-Sleep -Seconds 2"{{else}}sleep 2{{end}} + done + echo "" + if [ "$ok" = 1 ]; then + echo ">> all four answering" + else + echo ">> still waiting on one or more after 5 minutes; addresses below anyway" + fi + echo ">> self-hosted UI http://localhost:{{.APP_FRONTEND_PORT}}/processor" + echo ">> self-hosted api http://localhost:{{.APP_BACKEND_PORT}}" + echo ">> saas UI http://localhost:{{.SAAS_FRONTEND_PORT}}" + echo ">> saas api http://localhost:{{.SAAS_BACKEND_PORT}}" + echo "" + dev:_all: internal: true vars: diff --git a/app/core/src/main/resources/settings.yml.template b/app/core/src/main/resources/settings.yml.template index fdfe40b352..ecf7ea8538 100644 --- a/app/core/src/main/resources/settings.yml.template +++ b/app/core/src/main/resources/settings.yml.template @@ -186,7 +186,7 @@ system: maxDPI: 500 # Maximum allowed DPI for PDF to image conversion corsAllowedOrigins: [] # List of allowed origins for CORS (e.g. ['http://localhost:5173', 'https://app.example.com']). WARNING: leaving this empty falls back to allowing ALL origins (with credentials), it does NOT disable CORS. Set explicit origins to lock it down. backendUrl: "" # Backend base URL for SAML/OAuth/API callbacks (e.g. 'http://localhost:8080' for dev, 'https://api.example.com' for production). REQUIRED for SSO authentication to work correctly. This is where your IdP will send SAML responses and OAuth callbacks. Leave empty to default to 'http://localhost:8080' in development. - frontendUrl: "" # Frontend URL for invite email links (e.g. 'https://app.example.com'). Optional - if not set, will use backendUrl. This is the URL users click in invite emails. + frontendUrl: "" # Base URL of the web app, as a browser reaches it (e.g. 'https://app.example.com', or 'https://example.com/app' if served under a base path). Optional - if not set, will use backendUrl. Used for any link handed to a browser: invite emails, share links, mobile QR codes, and the account-link handshake. enableMobileScanner: true # Enable mobile phone QR code upload feature. Requires frontendUrl to be configured. enableMobileSignature: true # Enable drawing signatures on a phone via QR code from the Sign tool. Requires frontendUrl to be configured. mobileScannerSettings: diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkClient.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkClient.java index bdd9df10a8..558341745d 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkClient.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkClient.java @@ -24,22 +24,6 @@ import tools.jackson.databind.node.ObjectNode; /** * Outbound calls from a self-hosted instance to its linked SaaS backend (combined-billing "Mode * A"). - * - *

Calls: - * - *

- * - *

Uses {@code java.net.http.HttpClient} (the established self-hosted outbound pattern; see - * {@code AiEngineClient}); base URL + client are injectable so tests can stub SaaS. */ @Slf4j @Service @@ -72,13 +56,7 @@ public class AccountLinkClient { this.httpClient = httpClient; } - /** The device credential a successful {@link #register} returns. */ - public record RegisterResult(String deviceId, String deviceSecret, Long teamId) {} - - /** - * A non-2xx reply from the SaaS account-link API. Carries the upstream status so the caller can - * map auth failures (401/403) through rather than masking everything as a 502. - */ + /** A non-2xx reply from the SaaS account-link API. */ public static class UpstreamException extends IOException { private final int status; @@ -92,11 +70,7 @@ public class AccountLinkClient { } } - /** - * Authoritative deny (401/403) — the device credential is revoked or invalid. Unlike a - * transport/server failure (which returns {@code null} and fails open), the cache must BLOCK on - * this. Unchecked so it propagates through {@link #fetchEntitlement}'s transport try/catch. - */ + /** Authoritative deny (401/403) — the device credential is revoked or invalid. */ public static final class RevokedException extends RuntimeException { private final int status; @@ -110,46 +84,142 @@ public class AccountLinkClient { } } + /** What the SaaS side hands back when it records a connect handshake. */ + public record ConnectRequestResult( + String requestId, int expiresInSeconds, String authorizeUrl) {} + + public enum ConnectClaimOutcome { + /** Approved and collected; the credential fields are populated. */ + GRANTED, + /** A re-authentication was approved. */ + CONFIRMED, + /** No human decision yet. */ + PENDING, + /** Declined, expired or already used. */ + REJECTED, + /** SaaS unreachable or erroring. */ + UNAVAILABLE + } + + public record ConnectClaimResult( + ConnectClaimOutcome outcome, String deviceId, String deviceSecret, Long teamId) { + static ConnectClaimResult of(ConnectClaimOutcome outcome) { + return new ConnectClaimResult(outcome, null, null, null); + } + } + + /** Opens a connect handshake. */ + public ConnectRequestResult connectRequest( + String name, String callbackUrl, String nonce, String claimSecret) throws IOException { + return connectRequest(name, callbackUrl, nonce, claimSecret, null); + } + /** - * Relays the admin Supabase JWT to the SaaS register endpoint and returns the minted - * credential. - * - * @throws IOException on transport failure or a non-2xx response (caller surfaces to the - * admin). + * As {@link #connectRequest}, but presenting an existing device credential so the SaaS side + * treats this as a re-authentication and pins the handshake to the team we already belong to. */ - public RegisterResult register(String supabaseJwt, String instanceName) throws IOException { - String body = - instanceName == null || instanceName.isBlank() - ? "{}" - : "{\"name\":" + mapper.writeValueAsString(instanceName) + "}"; - HttpRequest request = + public ConnectRequestResult connectRequest( + String name, + String callbackUrl, + String nonce, + String claimSecret, + DeviceCredential credential) + throws IOException { + ObjectNode root = mapper.createObjectNode(); + if (name != null && !name.isBlank()) { + root.put("name", name); + } + root.put("callbackUrl", callbackUrl); + root.put("nonce", nonce); + root.put("claimSecret", claimSecret); + + HttpRequest.Builder builder = HttpRequest.newBuilder() - .uri(uri("/api/v1/account-link/register")) - .header("Authorization", "Bearer " + supabaseJwt) + .uri(uri("/api/v1/account-link/connect/request")) .header("Content-Type", "application/json") .header("Accept", "application/json") .timeout(timeout()) - .POST(HttpRequest.BodyPublishers.ofString(body)) - .build(); + .POST(HttpRequest.BodyPublishers.ofString(mapper.writeValueAsString(root))); + if (credential != null) { + builder.header(HEADER_DEVICE_ID, credential.getDeviceId()) + .header(HEADER_DEVICE_SECRET, credential.getDeviceSecret()); + } - HttpResponse response = send(request); + HttpResponse response = send(builder.build()); if (response.statusCode() / 100 != 2) { throw new UpstreamException(response.statusCode(), response.body()); } - JsonNode root = mapper.readTree(response.body()); - String deviceId = text(root, "deviceId"); - String deviceSecret = text(root, "deviceSecret"); - if (deviceId == null || deviceSecret == null) { - throw new IOException("SaaS register response missing deviceId/deviceSecret"); + JsonNode body = mapper.readTree(response.body()); + String requestId = text(body, "requestId"); + if (requestId == null) { + throw new IOException("SaaS connect response missing requestId"); + } + String authorizeUrl = text(body, "authorizeUrl"); + if (authorizeUrl == null || !isAbsoluteHttpUrl(authorizeUrl)) { + throw new IOException("SaaS connect response carried no usable authorizeUrl"); + } + return new ConnectRequestResult(requestId, body.path("expiresIn").asInt(0), authorizeUrl); + } + + /** + * Collects the device credential for an approved handshake, proving possession of the claim + * secret. + */ + public ConnectClaimResult connectClaim(String requestId, String claimSecret) { + HttpResponse response; + try { + ObjectNode root = mapper.createObjectNode(); + root.put("requestId", requestId); + root.put("claimSecret", claimSecret); + HttpRequest request = + HttpRequest.newBuilder() + .uri(uri("/api/v1/account-link/connect/claim")) + .header("Content-Type", "application/json") + .header("Accept", "application/json") + .timeout(timeout()) + .POST( + HttpRequest.BodyPublishers.ofString( + mapper.writeValueAsString(root))) + .build(); + response = send(request); + } catch (Exception e) { + log.debug("Connect claim failed (transport): {}", e.getMessage()); + return ConnectClaimResult.of(ConnectClaimOutcome.UNAVAILABLE); + } + int status = response.statusCode(); + if (status == 202) { + return ConnectClaimResult.of(ConnectClaimOutcome.PENDING); + } + if (status >= 500 && status <= 599) { + return ConnectClaimResult.of(ConnectClaimOutcome.UNAVAILABLE); + } + if (status < 200 || status > 299) { + return ConnectClaimResult.of(ConnectClaimOutcome.REJECTED); + } + try { + JsonNode body = mapper.readTree(response.body()); + Long teamId = body.hasNonNull("teamId") ? body.get("teamId").asLong() : null; + // A re-authentication says so explicitly and carries no credential, so an absent + // credential is only an error when we were expecting one. + if ("confirmed".equals(text(body, "status"))) { + return new ConnectClaimResult(ConnectClaimOutcome.CONFIRMED, null, null, teamId); + } + String deviceId = text(body, "deviceId"); + String deviceSecret = text(body, "deviceSecret"); + if (deviceId == null || deviceSecret == null) { + log.warn("Connect claim succeeded but the reply carried no credential"); + return ConnectClaimResult.of(ConnectClaimOutcome.REJECTED); + } + return new ConnectClaimResult( + ConnectClaimOutcome.GRANTED, deviceId, deviceSecret, teamId); + } catch (RuntimeException e) { + log.debug("Connect claim parse failed: {}", e.getMessage()); + return ConnectClaimResult.of(ConnectClaimOutcome.REJECTED); } - Long teamId = root.hasNonNull("teamId") ? root.get("teamId").asLong() : null; - return new RegisterResult(deviceId, deviceSecret, teamId); } /** * Revokes this instance's own credential on the SaaS side, authenticated by that credential. - * Best-effort: returns {@code false} if SaaS is unreachable or rejects, so the caller (local - * unlink) can still clear locally and log the orphan for follow-up. Idempotent on SaaS. */ public boolean revokeSelf(String deviceId, String deviceSecret) { try { @@ -174,17 +244,7 @@ public class AccountLinkClient { } } - /** - * Fetches the current entitlement using the stored device credential. Three outcomes: - * - *

- */ + /** Fetches the current entitlement using the stored device credential. */ public InstanceEntitlement fetchEntitlement(String deviceId, String deviceSecret) { HttpResponse response; try { @@ -224,9 +284,6 @@ public class AccountLinkClient { /** * Reports the period's cumulative per-category units to {@code POST /api/v1/instance/sync} and * returns the fresh entitlement in the same reply — one round-trip both reports and refreshes. - * SaaS bills the delta against its last-seen cumulative, so resending the same totals is - * idempotent. Same three outcomes as {@link #fetchEntitlement}; on {@code null} the caller must - * not advance its last-synced markers so the usage retries next sync. */ public InstanceEntitlement reportUsage( String deviceId, @@ -360,4 +417,19 @@ public class AccountLinkClient { private static String text(JsonNode node, String field) { return node.hasNonNull(field) ? node.get(field).asText() : null; } + + /** Absolute http(s) with a host. */ + static boolean isAbsoluteHttpUrl(String candidate) { + try { + URI uri = URI.create(candidate.strip()); + String scheme = uri.getScheme(); + return uri.isAbsolute() + && scheme != null + && ("http".equalsIgnoreCase(scheme) || "https".equalsIgnoreCase(scheme)) + && uri.getHost() != null + && !uri.getHost().isBlank(); + } catch (IllegalArgumentException e) { + return false; + } + } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkController.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkController.java index 52af366df4..b1826b7cca 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkController.java @@ -16,21 +16,11 @@ import org.springframework.web.bind.annotation.RestController; import io.swagger.v3.oas.annotations.Hidden; +import jakarta.servlet.http.HttpServletRequest; + import lombok.extern.slf4j.Slf4j; -/** - * Same-origin account-link surface on the self-hosted instance (combined-billing "Mode A"). - * - *

The portal (served from this same origin, admin authenticated by the existing self-hosted - * security chain) calls these. {@code POST /link} relays the admin's Supabase JWT to the SaaS - * backend, which mints + returns a device credential we store locally. {@code GET /status} backs - * the portal's link card; {@code GET /usage} exposes locally-accrued unsynced usage the portal adds - * to SaaS-synced spend; {@code POST /sync-now} forces an immediate usage sync (ops "reconcile now" - * / test aid). - * - *

Admin-only, {@code @Profile("!saas")}, gated behind {@code - * stirling.billing.account-link.enabled} — off → bean absent → 404. - */ +/** Same-origin account-link surface on the self-hosted instance (combined billing). */ @Slf4j @Hidden @RestController @@ -41,51 +31,110 @@ import lombok.extern.slf4j.Slf4j; public class AccountLinkController { private final AccountLinkService service; + private final ConnectService connectService; private final LocalUsageService localUsageService; // Present only when metering is on (its own flag); absent → /sync-now reports 409. private final ObjectProvider syncServiceProvider; public AccountLinkController( AccountLinkService service, + ConnectService connectService, LocalUsageService localUsageService, ObjectProvider syncServiceProvider) { this.service = service; + this.connectService = connectService; this.localUsageService = localUsageService; this.syncServiceProvider = syncServiceProvider; } - /** {@code supabaseJwt} is the admin's short-lived token the portal already holds. */ - public record LinkRequest(String supabaseJwt, String name) {} + /** {@code callbackUrl} is the portal telling us where its own callback route lives. */ + public record ConnectStartRequest(String name, String callbackUrl) {} - @PostMapping("/link") - public ResponseEntity link(@RequestBody LinkRequest req) { - if (req == null || req.supabaseJwt() == null || req.supabaseJwt().isBlank()) { - return ResponseEntity.badRequest() - .body(java.util.Map.of("error", "supabaseJwt is required")); - } + /** {@code nonce} comes from the callback fragment the approval page redirected to. */ + public record ConnectCompleteRequest(String nonce) {} + + /** + * Opens a browser-mediated link handshake and returns the approval URL to send the admin to. + */ + @PostMapping("/connect/start") + public ResponseEntity connectStart( + @RequestBody(required = false) ConnectStartRequest req, HttpServletRequest http) { try { - return ResponseEntity.ok(service.link(req.supabaseJwt(), req.name())); + return ResponseEntity.ok( + connectService.start(req != null ? req.name() : null, callbackHint(req, http))); } catch (AccountLinkClient.UpstreamException e) { - // Auth failures are the admin's token, not a gateway fault: surface 401/403 as-is so - // the portal can prompt a re-sign-in. Anything else upstream → 502. Don't echo the - // raw upstream body back to the browser. - HttpStatus status = - e.status() == HttpStatus.UNAUTHORIZED.value() - || e.status() == HttpStatus.FORBIDDEN.value() - ? HttpStatus.valueOf(e.status()) - : HttpStatus.BAD_GATEWAY; - log.warn("Account-link register rejected upstream: HTTP {}", e.status()); - return ResponseEntity.status(status).body(java.util.Map.of("error", "LINK_FAILED")); - } catch (IOException e) { - // Don't echo e.getMessage() to the browser: a DNS/connection/TLS failure can carry the - // configured SaaS host/IP. Log it server-side; return the same opaque body the - // UpstreamException branch does. - log.warn("Account-link failed (transport): {}", e.getMessage()); + log.warn("Account-link connect rejected upstream: HTTP {}", e.status()); return ResponseEntity.status(HttpStatus.BAD_GATEWAY) - .body(java.util.Map.of("error", "LINK_FAILED")); + .body(java.util.Map.of("error", "CONNECT_FAILED")); + } catch (IOException e) { + // Same reasoning as /link: a transport message can carry the configured SaaS host. + log.warn("Account-link connect failed (transport): {}", e.getMessage()); + return ResponseEntity.status(HttpStatus.BAD_GATEWAY) + .body(java.util.Map.of("error", "CONNECT_FAILED")); } } + /** Re-establishes the admin's SaaS session for a server that is already linked. */ + @PostMapping("/connect/reauth") + public ResponseEntity connectReauth( + @RequestBody(required = false) ConnectStartRequest req, HttpServletRequest http) { + try { + return ResponseEntity.ok(connectService.startReauth(callbackHint(req, http))); + } catch (AccountLinkClient.UpstreamException e) { + log.warn("Account-link reauth rejected upstream: HTTP {}", e.status()); + return ResponseEntity.status(HttpStatus.BAD_GATEWAY) + .body(java.util.Map.of("error", "CONNECT_FAILED")); + } catch (IOException e) { + log.warn("Account-link reauth failed: {}", e.getMessage()); + return ResponseEntity.status(HttpStatus.BAD_GATEWAY) + .body(java.util.Map.of("error", "CONNECT_FAILED")); + } + } + + /** Called by the callback page with the nonce it found in the fragment. */ + @PostMapping("/connect/complete") + public ResponseEntity connectComplete( + @RequestBody(required = false) ConnectCompleteRequest req) { + return ResponseEntity.ok(connectService.complete(req != null ? req.nonce() : null)); + } + + /** Everything we know about where the admin's browser is, for the callback. */ + private static ConnectService.CallbackHint callbackHint( + ConnectStartRequest req, HttpServletRequest http) { + return new ConnectService.CallbackHint( + req != null ? req.callbackUrl() : null, http.getHeader("Origin"), baseUrlOf(http)); + } + + /** + * This instance's base URL as the browser reached it, including any context path so a subpath + * deployment builds a callback that actually resolves. + */ + private static String baseUrlOf(HttpServletRequest request) { + String forwardedProto = firstHop(request.getHeader("X-Forwarded-Proto")); + String forwardedHost = firstHop(request.getHeader("X-Forwarded-Host")); + String scheme = forwardedProto != null ? forwardedProto : request.getScheme(); + String hostPort; + if (forwardedHost != null) { + hostPort = forwardedHost; + } else { + int port = request.getServerPort(); + boolean defaultPort = + ("http".equals(scheme) && port == 80) + || ("https".equals(scheme) && port == 443); + hostPort = defaultPort ? request.getServerName() : request.getServerName() + ":" + port; + } + String context = request.getContextPath() == null ? "" : request.getContextPath(); + return scheme + "://" + hostPort + context; + } + + private static String firstHop(String headerValue) { + if (headerValue == null || headerValue.isBlank()) { + return null; + } + String first = headerValue.split(",")[0].strip(); + return first.isEmpty() ? null : first; + } + @GetMapping("/status") public ResponseEntity status() { return ResponseEntity.ok(service.status()); @@ -106,12 +155,7 @@ public class AccountLinkController { return ResponseEntity.ok(localUsageService.currentPeriodUnsynced()); } - /** - * Forces an immediate usage sync to SaaS — the same work the daily scheduler does. An admin - * "reconcile now" action (and a test aid so you don't wait on the scheduler). Idempotent: - * re-reports the current cumulative, so a repeat trigger bills nothing. {@code 204} once run; - * {@code 409} when metering is off (the sync bean is absent). - */ + /** Forces an immediate usage sync to SaaS — the same work the daily scheduler does. */ @PostMapping("/sync-now") public ResponseEntity syncNow() { UsageSyncService sync = syncServiceProvider.getIfAvailable(); diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkProperties.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkProperties.java index 6d1f1fb151..619aa10e86 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkProperties.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkProperties.java @@ -8,29 +8,17 @@ import org.springframework.stereotype.Component; import lombok.Getter; import lombok.Setter; -/** - * Self-hosted side of combined-billing "Mode A" (connected self-hosted). - * - *

Binds the {@code stirling.billing.account-link.*} keys. {@link #enabled} mirrors the same flag - * the gated beans test with {@code @ConditionalOnProperty}; it is kept here only so non-conditional - * code (e.g. the gate's flag-off short-circuit, exposed status) can read it. The whole feature is - * off by default and dark — when off nothing gates and the link endpoints 404. - */ +/** Self-hosted side of combined billing: this instance bills through a linked SaaS team. */ @Getter @Setter @Component @ConfigurationProperties(prefix = "stirling.billing.account-link") public class AccountLinkProperties { - /** Master switch. When {@code false} (default) the feature is fully inert. */ + /** Master switch. */ private boolean enabled = false; - /** - * Base URL of the SaaS backend this instance links to (register + entitlement live there). - * - *

STUB: defaults to the public cloud host; an operator overrides it for staging. There is no - * existing SaaS-base-url property in the self-hosted profile, so this is introduced here. - */ + /** Base URL of the SaaS backend this instance links to (register + entitlement live there). */ private String saasBaseUrl = "https://stirling.com/app"; /** Cached entitlement is reused for this long before a refresh is attempted. */ @@ -39,20 +27,18 @@ public class AccountLinkProperties { /** Connect/read timeout for the outbound SaaS calls. */ private int requestTimeoutSeconds = 10; - /** Phase 2 usage metering + daily sync. Keyed under {@code …account-link.metering.*}. */ + /** Phase 2 usage metering + daily sync. */ private final Metering metering = new Metering(); /** - * Dedicated billing switch, separate from {@link #enabled} so the link plumbing can be - * enabled (e.g. to test linking) without ever turning on real usage metering, reporting, or cap - * enforcement. Both default off; metering requires the master flag too. This is the production - * safety key — flipping it on is what actually bills linked instances. + * Separate from {@link #enabled} so linking can be exercised without billing anything. Both + * default off, and metering needs the master flag as well. */ @Getter @Setter public static class Metering { - /** Turns on usage metering, the daily sync, and cap enforcement. Default off. */ + /** Turns on usage metering, the daily sync, and cap enforcement. */ private boolean enabled = false; /** @@ -65,12 +51,7 @@ public class AccountLinkProperties { */ private int graceDays = 3; - /** - * Dedup window for identical input sets. A re-run of the same inputs within this window is - * treated as workflow chaining and not re-charged; the same inputs run again after it are - * billed afresh. Mirrors the cloud's {@code payg.lineage.workflow-window} so the same op - * costs the same on the instance and in the cloud. - */ + /** Dedup window for identical input sets. */ private Duration workflowWindow = Duration.ofMinutes(5); } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkService.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkService.java index 1bb27d9cd6..e1283d83ba 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkService.java @@ -1,6 +1,5 @@ package stirling.software.proprietary.accountlink; -import java.io.IOException; import java.util.Optional; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -9,13 +8,7 @@ import org.springframework.stereotype.Service; import lombok.extern.slf4j.Slf4j; -/** - * Linking orchestrator (self-hosted side of combined-billing "Mode A"). - * - *

{@link #link} is the same-origin action the portal triggers: it relays the admin's Supabase - * JWT to the SaaS register endpoint, then persists the returned device credential secure-at-rest. - * The credential — not the JWT — authenticates all later unattended entitlement calls. - */ +/** Linking orchestrator (self-hosted side of combined billing). */ @Slf4j @Service @Profile("!saas") @@ -38,24 +31,9 @@ public class AccountLinkService { /** Status of this instance's link, for the portal's "Account link" card. */ public record LinkStatus(boolean linked, String deviceId, Long teamId, String linkedAt) {} - /** - * Registers this instance with the SaaS team behind {@code supabaseJwt} and stores the - * credential. - * - * @throws IOException if the SaaS register call fails (surfaced to the admin as a link error). - */ - public LinkStatus link(String supabaseJwt, String instanceName) throws IOException { - AccountLinkClient.RegisterResult result = client.register(supabaseJwt, instanceName); - credentialStore.save(result.deviceId(), result.deviceSecret(), result.teamId()); - entitlementCache.invalidate(); - log.info("Account-link: instance linked to team {}", result.teamId()); - return status(); - } - /** * Unlinks this instance — best-effort tells SaaS to revoke first (so the row gets {@code - * revoked_at} set), then clears locally regardless. If SaaS is unreachable the local clear - * still proceeds (admin's intent must win); the orphan row can be revoked from the portal. + * revoked_at} set), then clears locally regardless. */ public void unlink() { credentialStore diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkSyncState.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkSyncState.java index fbac6a8603..2715b4743b 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkSyncState.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkSyncState.java @@ -12,7 +12,7 @@ import lombok.NoArgsConstructor; import lombok.Setter; /** - * Singleton row holding this instance's daily-sync bookkeeping (combined-billing "Mode A"). + * Singleton row holding this instance's daily-sync bookkeeping (combined billing). * *

{@link #lastSyncSeq} is reserved (incremented + persisted) before each report so it * is strictly monotonic across restarts and partial failures — SaaS dedups replays by comparing it, diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkSyncStateRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkSyncStateRepository.java index 15b5e3842d..d0cdf36f90 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkSyncStateRepository.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/AccountLinkSyncStateRepository.java @@ -2,5 +2,5 @@ package stirling.software.proprietary.accountlink; import org.springframework.data.jpa.repository.JpaRepository; -/** Persistence for the singleton {@link AccountLinkSyncState} (combined-billing "Mode A"). */ +/** Persistence for the singleton {@link AccountLinkSyncState} (combined billing). */ public interface AccountLinkSyncStateRepository extends JpaRepository {} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/ConnectService.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/ConnectService.java new file mode 100644 index 0000000000..978d6d494e --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/ConnectService.java @@ -0,0 +1,276 @@ +package stirling.software.proprietary.accountlink; + +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.SecureRandom; +import java.time.Duration; +import java.time.LocalDateTime; +import java.util.Base64; +import java.util.Locale; +import java.util.Optional; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Profile; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import lombok.extern.slf4j.Slf4j; + +import stirling.software.common.model.ApplicationProperties; + +/** Browser-mediated account linking, instance side. */ +@Slf4j +@Service +@Profile("!saas") +@ConditionalOnProperty(name = "stirling.billing.account-link.enabled", havingValue = "true") +public class ConnectService { + + /** Frontend route that consumes the callback fragment. */ + static final String CALLBACK_PATH = "/account-link/callback"; + + private static final int SECRET_BYTES = 32; + + private final AccountLinkClient client; + private final ConnectStateRepository stateRepo; + private final DeviceCredentialStore credentialStore; + private final EntitlementCache entitlementCache; + private final ApplicationProperties applicationProperties; + private final SecureRandom random = new SecureRandom(); + + public ConnectService( + AccountLinkClient client, + ConnectStateRepository stateRepo, + DeviceCredentialStore credentialStore, + EntitlementCache entitlementCache, + ApplicationProperties applicationProperties) { + this.client = client; + this.stateRepo = stateRepo; + this.credentialStore = credentialStore; + this.entitlementCache = entitlementCache; + this.applicationProperties = applicationProperties; + } + + public enum Phase { + /** Nothing in flight and not linked. */ + NONE, + /** A handshake is open, waiting for a leader to approve it on the SaaS site. */ + PENDING, + /** Linked. */ + LINKED, + /** The handshake outlived its window; start a new one. */ + EXPIRED, + /** Declined or already used; start a new one. */ + REJECTED, + /** SaaS could not be reached; the handshake is still valid and can be retried. */ + UNAVAILABLE + } + + /** What the portal renders. */ + public record ConnectStatus( + Phase phase, String authorizeUrl, Long secondsRemaining, Long teamId) { + static ConnectStatus of(Phase phase) { + return new ConnectStatus(phase, null, null, null); + } + } + + /** Everything we know about where the admin's browser actually is, in decreasing authority. */ + public record CallbackHint( + String requestedCallbackUrl, String browserOrigin, String derivedBaseUrl) {} + + /** Opens a handshake and returns where to send the admin. */ + @Transactional + public ConnectStatus start(String name, CallbackHint hint) throws IOException { + if (credentialStore.isLinked()) { + return status(); + } + return open(name, hint, null); + } + + /** + * Opens a handshake that only re-establishes the admin's browser session, for an instance that + * is already linked. + */ + @Transactional + public ConnectStatus startReauth(CallbackHint hint) throws IOException { + DeviceCredential credential = + credentialStore + .get() + .orElseThrow( + () -> + new IOException( + "This server is not linked, so there is no session" + + " to re-establish")); + return open(credential.getDeviceId(), hint, credential); + } + + private ConnectStatus open(String name, CallbackHint hint, DeviceCredential credential) + throws IOException { + String callbackUrl = resolveCallbackUrl(hint); + if (callbackUrl == null) { + throw new IOException( + "Cannot determine where to send the admin back to; set system.frontendUrl"); + } + String nonce = randomSecret(); + String claimSecret = randomSecret(); + + AccountLinkClient.ConnectRequestResult created = + client.connectRequest(name, callbackUrl, nonce, claimSecret, credential); + + LocalDateTime now = LocalDateTime.now(); + ConnectState state = new ConnectState(); + state.setId(ConnectState.SINGLETON_ID); + state.setRequestId(created.requestId()); + state.setNonce(nonce); + state.setClaimSecret(claimSecret); + state.setCallbackUrl(callbackUrl); + state.setAuthorizeUrl(created.authorizeUrl()); + state.setCreatedAt(now); + state.setExpiresAt( + now.plusSeconds(created.expiresInSeconds() > 0 ? created.expiresInSeconds() : 900)); + stateRepo.save(state); + + log.info("Account-link connect: handshake {} opened", created.requestId()); + return pendingStatus(state, now); + } + + /** Finishes a handshake from the callback the approval page redirected to. */ + @Transactional + public ConnectStatus complete(String nonce) { + Optional found = stateRepo.findById(ConnectState.SINGLETON_ID); + if (found.isEmpty()) { + // Already finished (a double-submitted callback) or never started. + return status(); + } + ConnectState state = found.get(); + if (state.isExpired(LocalDateTime.now())) { + stateRepo.delete(state); + return ConnectStatus.of(Phase.EXPIRED); + } + if (nonce == null || !nonceMatches(nonce, state.getNonce())) { + log.warn( + "Account-link connect: callback for handshake {} had a bad nonce", + state.getRequestId()); + return ConnectStatus.of(Phase.REJECTED); + } + + AccountLinkClient.ConnectClaimResult claim = + client.connectClaim(state.getRequestId(), state.getClaimSecret()); + return switch (claim.outcome()) { + case GRANTED -> { + credentialStore.save(claim.deviceId(), claim.deviceSecret(), claim.teamId()); + entitlementCache.invalidate(); + stateRepo.delete(state); + log.info("Account-link connect: linked to team {}", claim.teamId()); + yield new ConnectStatus(Phase.LINKED, null, null, claim.teamId()); + } + case CONFIRMED -> { + stateRepo.delete(state); + log.info( + "Account-link connect: session re-established for team {}", claim.teamId()); + yield new ConnectStatus(Phase.LINKED, null, null, claim.teamId()); + } + case PENDING -> + // The admin reached the callback before the approval committed. The row stays, + // so a retry finishes it. + ConnectStatus.of(Phase.PENDING); + case REJECTED -> { + stateRepo.delete(state); + yield ConnectStatus.of(Phase.REJECTED); + } + case UNAVAILABLE -> ConnectStatus.of(Phase.UNAVAILABLE); + }; + } + + @Transactional(readOnly = true) + public ConnectStatus status() { + Optional credential = credentialStore.get(); + if (credential.isPresent()) { + return new ConnectStatus(Phase.LINKED, null, null, credential.get().getTeamId()); + } + Optional state = stateRepo.findById(ConnectState.SINGLETON_ID); + if (state.isEmpty()) { + return ConnectStatus.of(Phase.NONE); + } + LocalDateTime now = LocalDateTime.now(); + if (state.get().isExpired(now)) { + return ConnectStatus.of(Phase.EXPIRED); + } + return pendingStatus(state.get(), now); + } + + private static ConnectStatus pendingStatus(ConnectState state, LocalDateTime now) { + long remaining = Duration.between(now, state.getExpiresAt()).toSeconds(); + return new ConnectStatus( + Phase.PENDING, state.getAuthorizeUrl(), Math.max(remaining, 0), null); + } + + /** Decides the callback, preferring knowledge over inference. */ + String resolveCallbackUrl(CallbackHint hint) { + String configured = applicationProperties.getSystem().getFrontendUrl(); + if (configured != null && !configured.isBlank()) { + return trimTrailingSlash(configured.strip()) + CALLBACK_PATH; + } + String browserOrigin = originOf(hint.browserOrigin()); + if (browserOrigin != null) { + String requested = hint.requestedCallbackUrl(); + if (requested != null && browserOrigin.equals(originOf(requested))) { + return requested.strip(); + } + return browserOrigin + CALLBACK_PATH; + } + return hint.derivedBaseUrl() == null || hint.derivedBaseUrl().isBlank() + ? null + : trimTrailingSlash(hint.derivedBaseUrl().strip()) + CALLBACK_PATH; + } + + /** Scheme, host and port of an absolute http(s) URL; null if it is not one. */ + private static String originOf(String candidate) { + if (candidate == null || candidate.isBlank()) { + return null; + } + URI uri; + try { + uri = new URI(candidate.strip()); + } catch (URISyntaxException e) { + return null; + } + if (uri.getScheme() == null || uri.getHost() == null) { + return null; + } + String scheme = uri.getScheme().toLowerCase(Locale.ROOT); + if (!"http".equals(scheme) && !"https".equals(scheme)) { + return null; + } + int port = uri.getPort(); + boolean defaultPort = + port == -1 + || ("http".equals(scheme) && port == 80) + || ("https".equals(scheme) && port == 443); + return defaultPort + ? scheme + "://" + uri.getHost() + : scheme + "://" + uri.getHost() + ":" + port; + } + + private static String trimTrailingSlash(String value) { + return value.replaceAll("/+$", ""); + } + + private String randomSecret() { + byte[] buf = new byte[SECRET_BYTES]; + random.nextBytes(buf); + return Base64.getUrlEncoder().withoutPadding().encodeToString(buf); + } + + /** Constant-time so a caller cannot probe the nonce a character at a time. */ + private static boolean nonceMatches(String candidate, String expected) { + if (expected == null) { + return false; + } + return MessageDigest.isEqual( + candidate.getBytes(StandardCharsets.UTF_8), + expected.getBytes(StandardCharsets.UTF_8)); + } +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/ConnectState.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/ConnectState.java new file mode 100644 index 0000000000..c0dcea032e --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/ConnectState.java @@ -0,0 +1,60 @@ +package stirling.software.proprietary.accountlink; + +import java.io.Serializable; +import java.time.LocalDateTime; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Table; + +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +/** The one in-flight "connect this server" handshake, instance side. */ +@Entity +@Table(name = "account_link_connect_state") +@NoArgsConstructor +@Getter +@Setter +public class ConnectState implements Serializable { + + private static final long serialVersionUID = 1L; + + public static final Long SINGLETON_ID = 1L; + + @Id + @Column(name = "id") + private Long id = SINGLETON_ID; + + /** Opaque handle the SaaS side gave us; identifies the handshake on both sides. */ + @Column(name = "request_id", nullable = false, length = 64) + private String requestId; + + /** Correlator we minted. */ + @Column(name = "nonce", nullable = false, length = 128) + private String nonce; + + /** Secret we minted and sent to SaaS server to server. */ + @Column(name = "claim_secret", nullable = false, length = 128) + private String claimSecret; + + /** Where we asked the approval page to send the admin back to. */ + @Column(name = "callback_url", nullable = false, length = 2048) + private String callbackUrl; + + /** The approval URL handed to the browser, so a reload can offer it again. */ + @Column(name = "authorize_url", nullable = false, length = 2048) + private String authorizeUrl; + + @Column(name = "created_at", nullable = false) + private LocalDateTime createdAt; + + @Column(name = "expires_at", nullable = false) + private LocalDateTime expiresAt; + + public boolean isExpired(LocalDateTime now) { + return expiresAt != null && expiresAt.isBefore(now); + } +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/ConnectStateRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/ConnectStateRepository.java new file mode 100644 index 0000000000..995dfccde1 --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/ConnectStateRepository.java @@ -0,0 +1,6 @@ +package stirling.software.proprietary.accountlink; + +import org.springframework.data.jpa.repository.JpaRepository; + +/** Data access for the singleton {@link ConnectState} row. */ +public interface ConnectStateRepository extends JpaRepository {} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/DeviceCredential.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/DeviceCredential.java index 4625572310..7da486b741 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/DeviceCredential.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/DeviceCredential.java @@ -13,8 +13,8 @@ import lombok.NoArgsConstructor; import lombok.Setter; /** - * The device credential this self-hosted instance received when it linked a SaaS account - * (combined-billing "Mode A"). Singleton — one instance links to exactly one SaaS team. + * The device credential this self-hosted instance received when it linked a SaaS account (combined + * billing). Singleton — one instance links to exactly one SaaS team. * *

Unlike the SaaS side (which stores only a hash), the instance must keep the plaintext {@code * deviceSecret} so it can present it on every unattended entitlement call. It lives in the local diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/InstanceEntitlementGate.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/InstanceEntitlementGate.java index 018684b73f..c0cc901e1f 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/InstanceEntitlementGate.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/InstanceEntitlementGate.java @@ -8,7 +8,7 @@ import org.springframework.context.annotation.Profile; import org.springframework.stereotype.Service; /** - * Decides whether a request may proceed under combined-billing "Mode A" on a self-hosted instance. + * Decides whether a request may proceed under combined billing on a self-hosted instance. * *

Rules (in order): * diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/InstanceEntitlementInterceptor.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/InstanceEntitlementInterceptor.java index 0cd71c9bda..9e73267561 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/InstanceEntitlementInterceptor.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/InstanceEntitlementInterceptor.java @@ -39,8 +39,8 @@ import stirling.software.proprietary.policy.controller.PolicyRunRoutes; import stirling.software.proprietary.security.model.ApiKeyAuthenticationToken; /** - * Request-time gate + meter for combined-billing "Mode A". {@code preHandle} blocks billable (API / - * AI / automation) work when the instance is unlinked or over its limit; manual tools pass through. + * Request-time gate + meter for combined billing. {@code preHandle} blocks billable (API / AI / + * automation) work when the instance is unlinked or over its limit; manual tools pass through. * {@code afterCompletion} meters a successful billable op into the per-period cumulative counter. * *

Blocking responds {@code 402} with a machine-readable body the FE maps to a "link to activate" diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/MeteredInputSignature.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/MeteredInputSignature.java index 1ed49d6a8b..278380a5da 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/MeteredInputSignature.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/MeteredInputSignature.java @@ -16,11 +16,11 @@ import lombok.NoArgsConstructor; /** * The last time the instance metered a given input set this period — the local equivalent of the - * cloud's lineage join (combined-billing "Mode A"). The meter dedups on a rolling workflow - * window: an identical input set re-submitted within the window (see {@link - * AccountLinkProperties.Metering}) is treated as workflow chaining and not re-charged, while the - * same inputs run again after the window are billed afresh — matching the cloud's 5-minute open-job - * window so the same operation costs the same on the instance and in the cloud. + * cloud's lineage join (combined billing). The meter dedups on a rolling workflow window: an + * identical input set re-submitted within the window (see {@link AccountLinkProperties.Metering}) + * is treated as workflow chaining and not re-charged, while the same inputs run again after the + * window are billed afresh — matching the cloud's 5-minute open-job window so the same operation + * costs the same on the instance and in the cloud. * *

{@code lastMeteredAt} is refreshed on every sighting (the window slides, as recording a cloud * artifact touches its job). One row per {@code (period, signature)}; the unique constraint also diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/MeteredInputSignatureRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/MeteredInputSignatureRepository.java index 863f503f61..a31310a7b3 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/MeteredInputSignatureRepository.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/MeteredInputSignatureRepository.java @@ -5,7 +5,7 @@ import java.util.Optional; import org.springframework.data.jpa.repository.JpaRepository; -/** Persistence for the per-period metered input-set signatures (combined-billing "Mode A"). */ +/** Persistence for the per-period metered input-set signatures (combined billing). */ public interface MeteredInputSignatureRepository extends JpaRepository { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageCounter.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageCounter.java index b90af07958..4c06089040 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageCounter.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageCounter.java @@ -17,10 +17,10 @@ import lombok.NoArgsConstructor; import stirling.software.proprietary.billing.BillingCategory; /** - * Durable per-(billing period, category) cumulative usage counter for combined-billing "Mode A". - * Each successful billable op increments its row; the daily sync reports the cumulative totals and - * SaaS bills the delta since the last sync. The cumulative model is idempotent (a resend bills - * nothing) and tamper-evident (a counter that drops is a signal). One row per {@code (period_start, + * Durable per-(billing period, category) cumulative usage counter for combined billing. Each + * successful billable op increments its row; the daily sync reports the cumulative totals and SaaS + * bills the delta since the last sync. The cumulative model is idempotent (a resend bills nothing) + * and tamper-evident (a counter that drops is a signal). One row per {@code (period_start, * category)}, auto-created by Hibernate; only the flag-gated {@link UsageMeterService} writes it. */ @Entity diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageCounterRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageCounterRepository.java index 2140775abc..3013d00a52 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageCounterRepository.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageCounterRepository.java @@ -9,7 +9,7 @@ import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; import org.springframework.transaction.annotation.Transactional; -/** Persistence for the per-period/per-category usage counters (combined-billing "Mode A"). */ +/** Persistence for the per-period/per-category usage counters (combined billing). */ public interface UsageCounterRepository extends JpaRepository { /** diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageSyncService.java b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageSyncService.java index 4c4ce2377c..a12a26eb8a 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageSyncService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/accountlink/UsageSyncService.java @@ -18,8 +18,8 @@ import lombok.extern.slf4j.Slf4j; import stirling.software.proprietary.billing.BillingCategory; /** - * Daily usage sender for combined-billing "Mode A". Reports each period's cumulative per-category - * usage to SaaS, which bills the delta against its own last-seen totals. + * Daily usage sender for combined billing. Reports each period's cumulative per-category usage to + * SaaS, which bills the delta against its own last-seen totals. * *

Resilience: the sync seq is persisted before the report so it never regresses across * restarts/failures; a transport failure leaves the {@code lastSyncedUnits} markers untouched so diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/billing/ContentHasher.java b/app/proprietary/src/main/java/stirling/software/proprietary/billing/ContentHasher.java index 232dd499dc..182c162f6f 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/billing/ContentHasher.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/billing/ContentHasher.java @@ -11,9 +11,9 @@ import java.util.HexFormat; /** * SHA-256 content fingerprint shared by the SaaS charge path and the linked self-hosted instance's - * meter (combined-billing "Mode A"), so both derive an identical signature for the same - * bytes — the basis for lineage dedup. Pure, no Spring: fixed 64 KiB buffer (allocation independent - * of file size), hardware-accelerated by the JVM where available. + * meter (combined billing), so both derive an identical signature for the same bytes — the + * basis for lineage dedup. Pure, no Spring: fixed 64 KiB buffer (allocation independent of file + * size), hardware-accelerated by the JVM where available. * *

Lives in {@code :proprietary} (not {@code :common}) so it stays out of the community core * build yet is reachable from {@code :saas} (which depends on {@code :proprietary}). diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkClientTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkClientTest.java index 969111e9f5..2bce17d732 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkClientTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkClientTest.java @@ -21,9 +21,9 @@ import org.mockito.ArgumentCaptor; import tools.jackson.databind.ObjectMapper; /** - * Stubs the {@link HttpClient} so the SaaS endpoint is never actually called. Confirms register - * relays the JWT and parses the credential, and that entitlement parsing + the fail-open (null on - * unreachable) behaviour hold. + * Stubs the {@link HttpClient} so the SaaS endpoint is never actually called. Confirms the connect + * handshake refuses an authorize URL it would not navigate to and carries no user token, and that + * entitlement parsing + the fail-open (null on unreachable) behaviour hold. */ class AccountLinkClientTest { @@ -48,39 +48,79 @@ class AccountLinkClientTest { return resp; } + // register() is gone with the JWT relay, and with it the two tests that asserted this client + // sends an Authorization: Bearer header. Nothing here carries a user token any more. + @Test @SuppressWarnings("unchecked") - void registerRelaysJwtAndParsesCredential() throws Exception { - // Build the stub response first: nesting response() inside when() trips Mockito's - // unfinished-stubbing check (inner when() runs mid outer when()). + void connectRequestRefusesAnAuthorizeUrlItWouldNotNavigateTo() throws Exception { + // The reply drives a browser navigation, so a non-absolute or non-http(s) value must fail + // loudly here rather than reach the admin. HttpResponse resp = - response(201, "{\"deviceId\":\"dev-1\",\"deviceSecret\":\"sec-1\",\"teamId\":42}"); - ArgumentCaptor captor = ArgumentCaptor.forClass(HttpRequest.class); - when(httpClient.send(captor.capture(), any(HttpResponse.BodyHandler.class))) - .thenReturn(resp); + response(201, "{\"requestId\":\"req-1\",\"authorizeUrl\":\"/link?request=req-1\"}"); + when(httpClient.send(any(), any(HttpResponse.BodyHandler.class))).thenReturn(resp); - AccountLinkClient.RegisterResult result = client.register("jwt-token", "My Server"); - - assertEquals("dev-1", result.deviceId()); - assertEquals("sec-1", result.deviceSecret()); - assertEquals(42L, result.teamId()); - - HttpRequest sent = captor.getValue(); - assertEquals("Bearer jwt-token", sent.headers().firstValue("Authorization").orElse(null)); - assertEquals( - "https://saas.example.com/api/v1/account-link/register", sent.uri().toString()); + assertThrows( + java.io.IOException.class, + () -> client.connectRequest("n", "https://pdf.example.com/cb", "nonce", "secret")); } @Test @SuppressWarnings("unchecked") - void registerThrowsUpstreamExceptionWithStatusOnNon2xx() throws Exception { - HttpResponse resp = response(401, "{\"error\":\"unauthorized\"}"); + void connectRequestParsesTheAuthorizeUrlItIsGiven() throws Exception { + HttpResponse resp = + response( + 201, + "{\"requestId\":\"req-1\",\"expiresIn\":900," + + "\"authorizeUrl\":\"https://app.example.com/link?request=req-1\"}"); + ArgumentCaptor captor = ArgumentCaptor.forClass(HttpRequest.class); + when(httpClient.send(captor.capture(), any(HttpResponse.BodyHandler.class))) + .thenReturn(resp); + + AccountLinkClient.ConnectRequestResult result = + client.connectRequest("n", "https://pdf.example.com/cb", "nonce", "secret"); + + assertEquals("req-1", result.requestId()); + assertEquals("https://app.example.com/link?request=req-1", result.authorizeUrl()); + // No user token on this call, by design. + assertEquals(null, captor.getValue().headers().firstValue("Authorization").orElse(null)); + } + + @Test + @SuppressWarnings("unchecked") + void connectClaimGrantsTheCredentialOnSuccess() throws Exception { + HttpResponse resp = + response(200, "{\"deviceId\":\"dev-1\",\"deviceSecret\":\"sec-1\",\"teamId\":7}"); when(httpClient.send(any(), any(HttpResponse.BodyHandler.class))).thenReturn(resp); - AccountLinkClient.UpstreamException ex = - assertThrows( - AccountLinkClient.UpstreamException.class, - () -> client.register("jwt", null)); - assertEquals(401, ex.status()); + + AccountLinkClient.ConnectClaimResult result = client.connectClaim("req-1", "secret"); + + assertEquals(AccountLinkClient.ConnectClaimOutcome.GRANTED, result.outcome()); + assertEquals("dev-1", result.deviceId()); + assertEquals("sec-1", result.deviceSecret()); + } + + @Test + @SuppressWarnings("unchecked") + void connectClaimMapsTheStatusItIsGiven() throws Exception { + // The whole point of these four: a claim consumes the request server-side, so + // reading 200 as anything but success loses the credential irrecoverably. + assertEquals(AccountLinkClient.ConnectClaimOutcome.PENDING, claimOutcome(202, "{}")); + assertEquals(AccountLinkClient.ConnectClaimOutcome.UNAVAILABLE, claimOutcome(503, "{}")); + assertEquals(AccountLinkClient.ConnectClaimOutcome.REJECTED, claimOutcome(400, "{}")); + assertEquals( + AccountLinkClient.ConnectClaimOutcome.CONFIRMED, + claimOutcome(200, "{\"status\":\"confirmed\",\"teamId\":7}")); + } + + @SuppressWarnings("unchecked") + private AccountLinkClient.ConnectClaimOutcome claimOutcome(int status, String body) + throws Exception { + // Built before the when(), not inside it: response() stubs a mock of its own, and + // Mockito cannot have that happen mid-stubbing. + HttpResponse resp = response(status, body); + when(httpClient.send(any(), any(HttpResponse.BodyHandler.class))).thenReturn(resp); + return client.connectClaim("req-1", "secret").outcome(); } @Test diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkControllerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkControllerTest.java index 41f544de7e..6e13021a40 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkControllerTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkControllerTest.java @@ -1,6 +1,7 @@ package stirling.software.proprietary.accountlink; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -14,16 +15,15 @@ import org.springframework.beans.factory.ObjectProvider; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; -import stirling.software.proprietary.accountlink.AccountLinkController.LinkRequest; - /** - * The local (self-hosted) account-link controller's error mapping: an upstream auth rejection - * surfaces as 401/403 (so the portal can prompt a re-sign-in) while other upstream / transport - * faults are a 502. + * The local (self-hosted) account-link controller's error mapping. Every upstream or transport + * failure is a 502, and the response body never echoes the exception, because a DNS or TLS message + * can carry the configured SaaS host. */ class AccountLinkControllerTest { private AccountLinkService service; + private ConnectService connectService; private UsageSyncService syncService; private ObjectProvider syncProvider; private AccountLinkController controller; @@ -32,47 +32,54 @@ class AccountLinkControllerTest { @SuppressWarnings("unchecked") void setUp() { service = mock(AccountLinkService.class); + connectService = mock(ConnectService.class); syncService = mock(UsageSyncService.class); syncProvider = mock(ObjectProvider.class); controller = - new AccountLinkController(service, mock(LocalUsageService.class), syncProvider); + new AccountLinkController( + service, connectService, mock(LocalUsageService.class), syncProvider); } - @Test - void link_missingJwt_returns400() { - ResponseEntity resp = controller.link(new LinkRequest(" ", null)); - assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); - } + // These asserted POST /link's error mapping, which distinguished 401/403 so the portal could + // prompt a re-sign-in. That endpoint is gone with the JWT relay, and the distinction went with + // it: connect/start carries no user token, so an upstream refusal is never the admin's session + // and everything non-transport is a plain gateway failure. @Test - void link_upstreamUnauthorized_maps401() throws Exception { - when(service.link("jwt", null)) - .thenThrow(new AccountLinkClient.UpstreamException(401, "bad token")); - ResponseEntity resp = controller.link(new LinkRequest("jwt", null)); - assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); - } - - @Test - void link_upstreamForbidden_maps403() throws Exception { - when(service.link("jwt", null)) - .thenThrow(new AccountLinkClient.UpstreamException(403, "forbidden")); - ResponseEntity resp = controller.link(new LinkRequest("jwt", null)); - assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); - } - - @Test - void link_upstreamServerError_maps502() throws Exception { - when(service.link("jwt", null)) + void connectStart_upstreamFailure_maps502() throws Exception { + when(connectService.start(any(), any())) .thenThrow(new AccountLinkClient.UpstreamException(500, "boom")); - ResponseEntity resp = controller.link(new LinkRequest("jwt", null)); + + ResponseEntity resp = controller.connectStart(null, request()); + assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.BAD_GATEWAY); } @Test - void link_transportFailure_maps502() throws Exception { - when(service.link("jwt", null)).thenThrow(new IOException("connection refused")); - ResponseEntity resp = controller.link(new LinkRequest("jwt", null)); + void connectStart_transportFailure_maps502WithoutLeakingTheHost() throws Exception { + when(connectService.start(any(), any())) + .thenThrow(new IOException("connection refused to saas.internal:8081")); + + ResponseEntity resp = controller.connectStart(null, request()); + assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.BAD_GATEWAY); + // The body must not echo the exception: a DNS/TLS message can carry the configured SaaS + // host. + assertThat(String.valueOf(resp.getBody())).doesNotContain("saas.internal"); + } + + @Test + void connectReauth_onAnUnlinkedServer_maps502() throws Exception { + when(connectService.startReauth(any())).thenThrow(new IOException("not linked")); + + ResponseEntity resp = controller.connectReauth(null, request()); + + assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.BAD_GATEWAY); + } + + /** Minimal request: the controller only reads Origin and the forwarded/host details from it. */ + private static jakarta.servlet.http.HttpServletRequest request() { + return new org.springframework.mock.web.MockHttpServletRequest(); } @Test diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkServiceTest.java index b909fb37a0..410ef8a8c7 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkServiceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/AccountLinkServiceTest.java @@ -3,12 +3,10 @@ package stirling.software.proprietary.accountlink; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.io.IOException; import java.time.LocalDateTime; import java.util.Optional; @@ -30,33 +28,25 @@ class AccountLinkServiceTest { service = new AccountLinkService(client, store, cache); } + // The two link() tests here are gone with the JWT relay. Storing a credential and invalidating + // the entitlement cache is now ConnectService's job and is covered by ConnectServiceTest; what + // remains in this service is status and unlink. + @Test - void link_storesCredentialAndInvalidatesCache() throws IOException { - when(client.register("jwt", "name")) - .thenReturn(new AccountLinkClient.RegisterResult("dev-1", "sec-1", 7L)); + void status_linkedFromTheStoredCredential() { DeviceCredential stored = new DeviceCredential(); stored.setDeviceId("dev-1"); stored.setTeamId(7L); stored.setLinkedAt(LocalDateTime.now()); when(store.get()).thenReturn(Optional.of(stored)); - AccountLinkService.LinkStatus status = service.link("jwt", "name"); + AccountLinkService.LinkStatus status = service.status(); - verify(store).save("dev-1", "sec-1", 7L); - verify(cache).invalidate(); assertTrue(status.linked()); assertEquals("dev-1", status.deviceId()); assertEquals(7L, status.teamId()); } - @Test - void link_propagatesRegisterFailure() throws IOException { - when(client.register(any(), any())).thenThrow(new IOException("boom")); - org.junit.jupiter.api.Assertions.assertThrows( - IOException.class, () -> service.link("jwt", null)); - verify(cache, org.mockito.Mockito.never()).invalidate(); - } - @Test void status_unlinkedWhenNoCredential() { when(store.get()).thenReturn(Optional.empty()); diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/ConnectServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/ConnectServiceTest.java new file mode 100644 index 0000000000..5e040e4ad7 --- /dev/null +++ b/app/proprietary/src/test/java/stirling/software/proprietary/accountlink/ConnectServiceTest.java @@ -0,0 +1,408 @@ +package stirling.software.proprietary.accountlink; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import java.time.LocalDateTime; +import java.util.Optional; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; + +import stirling.software.common.model.ApplicationProperties; +import stirling.software.proprietary.accountlink.AccountLinkClient.ConnectClaimOutcome; +import stirling.software.proprietary.accountlink.AccountLinkClient.ConnectClaimResult; +import stirling.software.proprietary.accountlink.AccountLinkClient.ConnectRequestResult; +import stirling.software.proprietary.accountlink.ConnectService.Phase; + +/** Unit tests for the instance half of the connect handshake. */ +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +class ConnectServiceTest { + + private static final String NONCE = "the-nonce"; + private static final String CLAIM_SECRET = "the-claim-secret"; + private static final String AUTHORIZE_URL = "https://app.example.com/link?request=req-1"; + + @Mock private AccountLinkClient client; + @Mock private ConnectStateRepository stateRepo; + @Mock private DeviceCredentialStore credentialStore; + @Mock private EntitlementCache entitlementCache; + + private ApplicationProperties applicationProperties; + private ConnectService service; + + @BeforeEach + void setUp() { + applicationProperties = new ApplicationProperties(); + service = + new ConnectService( + client, + stateRepo, + credentialStore, + entitlementCache, + applicationProperties); + } + + private void configureFrontendUrl(String url) { + applicationProperties.getSystem().setFrontendUrl(url); + } + + @Test + void start_advertisesTheConfiguredFrontendUrlInPreferenceToTheRequest() throws Exception { + configureFrontendUrl("https://pdf.example.com/"); + stubCreate(); + + service.start("prod-1", fromRequest("http://10.0.0.5:8080")); + + verify(client) + .connectRequest( + anyString(), + // Trailing slash trimmed, and the request's own view ignored. + org.mockito.ArgumentMatchers.eq( + "https://pdf.example.com" + ConnectService.CALLBACK_PATH), + anyString(), + anyString(), + // A first link carries no credential; that is what makes it a first link. + org.mockito.ArgumentMatchers.isNull()); + } + + @Test + void start_fallsBackToTheAddressTheRequestArrivedOn() throws Exception { + stubCreate(); + + service.start(null, fromRequest("https://pdf.internal:8443/stirling")); + + ArgumentCaptor callback = ArgumentCaptor.forClass(String.class); + verify(client).connectRequest(any(), callback.capture(), anyString(), anyString(), any()); + // Context path preserved, so a subpath deployment gets a callback that resolves. + assertThat(callback.getValue()) + .isEqualTo("https://pdf.internal:8443/stirling" + ConnectService.CALLBACK_PATH); + } + + @Test + void start_withNoAddressAtAllFailsRatherThanGuessing() { + assertThat(catchIo(() -> service.start(null, fromRequest(null)))) + .hasMessageContaining("system.frontendUrl"); + verifyNoInteractions(client); + } + + @Test + void resolveCallback_honoursThePortalsOwnCallbackWhenTheBrowserOriginAgrees() { + // The frontend is the only party that knows its router's base path. + String requested = "http://localhost:5173/app/account-link/callback"; + + assertThat( + service.resolveCallbackUrl( + new ConnectService.CallbackHint( + requested, + "http://localhost:5173", + "http://localhost:8080"))) + .isEqualTo(requested); + } + + @Test + void resolveCallback_ignoresACallbackFromADifferentOrigin() { + assertThat( + service.resolveCallbackUrl( + new ConnectService.CallbackHint( + "https://evil.example.com/steal", + "http://localhost:5173", + "http://localhost:8080"))) + .isEqualTo("http://localhost:5173" + ConnectService.CALLBACK_PATH); + } + + @Test + void resolveCallback_prefersTheBrowserOriginOverTheApiRequest() { + // The whole point: :5173 is where the admin is, :8080 is where the call landed. + assertThat( + service.resolveCallbackUrl( + new ConnectService.CallbackHint( + null, "http://localhost:5173", "http://localhost:8080"))) + .isEqualTo("http://localhost:5173" + ConnectService.CALLBACK_PATH); + } + + @Test + void resolveCallback_letsConfigurationBeatEverything() { + configureFrontendUrl("https://pdf.example.com/"); + + assertThat( + service.resolveCallbackUrl( + new ConnectService.CallbackHint( + "http://localhost:5173/account-link/callback", + "http://localhost:5173", + "http://localhost:8080"))) + .isEqualTo("https://pdf.example.com" + ConnectService.CALLBACK_PATH); + } + + @Test + void resolveCallback_ignoresAnUnusableOriginHeader() { + // "null" is what a browser sends for an opaque origin; it must not become a callback. + assertThat( + service.resolveCallbackUrl( + new ConnectService.CallbackHint( + null, "null", "http://localhost:8080"))) + .isEqualTo("http://localhost:8080" + ConnectService.CALLBACK_PATH); + } + + @Test + void start_sendsTheAdminWhereverSaaSSaidToSendThem() throws Exception { + stubCreate(); + + ConnectService.ConnectStatus status = + service.start(null, fromRequest("https://pdf.example.com")); + + assertThat(status.phase()).isEqualTo(Phase.PENDING); + // Not composed here: only the SaaS side knows where its approval page lives, so an + // instance configuring that could only get it wrong. + assertThat(status.authorizeUrl()).isEqualTo(AUTHORIZE_URL); + } + + @Test + void start_keepsTheNonceAndClaimSecretItSent() throws Exception { + stubCreate(); + + service.start(null, fromRequest("https://pdf.example.com")); + + ArgumentCaptor nonce = ArgumentCaptor.forClass(String.class); + ArgumentCaptor secret = ArgumentCaptor.forClass(String.class); + verify(client).connectRequest(any(), anyString(), nonce.capture(), secret.capture(), any()); + + ArgumentCaptor saved = ArgumentCaptor.forClass(ConnectState.class); + verify(stateRepo).save(saved.capture()); + assertThat(saved.getValue().getNonce()).isEqualTo(nonce.getValue()); + assertThat(saved.getValue().getClaimSecret()).isEqualTo(secret.getValue()); + // Two independent secrets, not one value used twice. + assertThat(nonce.getValue()).isNotEqualTo(secret.getValue()); + } + + @Test + void start_whenAlreadyLinkedDoesNothing() throws Exception { + when(credentialStore.isLinked()).thenReturn(true); + when(credentialStore.get()).thenReturn(Optional.of(credential(7L))); + + ConnectService.ConnectStatus status = + service.start(null, fromRequest("https://pdf.example.com")); + + assertThat(status.phase()).isEqualTo(Phase.LINKED); + verifyNoInteractions(client); + verify(stateRepo, never()).save(any()); + } + + @Test + void complete_withTheRightNonceStoresTheCredentialAndClearsTheHandshake() { + ConnectState state = openHandshake(); + when(stateRepo.findById(ConnectState.SINGLETON_ID)).thenReturn(Optional.of(state)); + when(client.connectClaim("req-1", CLAIM_SECRET)) + .thenReturn(new ConnectClaimResult(ConnectClaimOutcome.GRANTED, "dev", "sec", 7L)); + + ConnectService.ConnectStatus status = service.complete(NONCE); + + assertThat(status.phase()).isEqualTo(Phase.LINKED); + assertThat(status.teamId()).isEqualTo(7L); + verify(credentialStore).save("dev", "sec", 7L); + verify(entitlementCache).invalidate(); + verify(stateRepo).delete(state); + } + + @Test + void complete_withAWrongNonceClaimsNothingAndLeavesTheHandshakeAlone() { + ConnectState state = openHandshake(); + when(stateRepo.findById(ConnectState.SINGLETON_ID)).thenReturn(Optional.of(state)); + + ConnectService.ConnectStatus status = service.complete("not-the-nonce"); + + assertThat(status.phase()).isEqualTo(Phase.REJECTED); + // The important half: an unverified caller cannot cancel a legitimate handshake. + verify(stateRepo, never()).delete(any()); + verifyNoInteractions(credentialStore); + verify(client, never()).connectClaim(anyString(), anyString()); + } + + @Test + void complete_withNoNonceAtAllIsRejected() { + when(stateRepo.findById(ConnectState.SINGLETON_ID)) + .thenReturn(Optional.of(openHandshake())); + + assertThat(service.complete(null).phase()).isEqualTo(Phase.REJECTED); + verify(client, never()).connectClaim(anyString(), anyString()); + } + + @Test + void complete_whenSaaSHasNotCommittedTheApprovalKeepsTheHandshake() { + when(stateRepo.findById(ConnectState.SINGLETON_ID)) + .thenReturn(Optional.of(openHandshake())); + when(client.connectClaim(anyString(), anyString())) + .thenReturn(ConnectClaimResult.of(ConnectClaimOutcome.PENDING)); + + assertThat(service.complete(NONCE).phase()).isEqualTo(Phase.PENDING); + verify(stateRepo, never()).delete(any()); + } + + @Test + void complete_whenSaaSIsUnreachableKeepsTheHandshakeForARetry() { + when(stateRepo.findById(ConnectState.SINGLETON_ID)) + .thenReturn(Optional.of(openHandshake())); + when(client.connectClaim(anyString(), anyString())) + .thenReturn(ConnectClaimResult.of(ConnectClaimOutcome.UNAVAILABLE)); + + assertThat(service.complete(NONCE).phase()).isEqualTo(Phase.UNAVAILABLE); + verify(stateRepo, never()).delete(any()); + verifyNoInteractions(credentialStore); + } + + @Test + void complete_whenDeclinedClearsTheHandshake() { + ConnectState state = openHandshake(); + when(stateRepo.findById(ConnectState.SINGLETON_ID)).thenReturn(Optional.of(state)); + when(client.connectClaim(anyString(), anyString())) + .thenReturn(ConnectClaimResult.of(ConnectClaimOutcome.REJECTED)); + + assertThat(service.complete(NONCE).phase()).isEqualTo(Phase.REJECTED); + verify(stateRepo).delete(state); + verifyNoInteractions(credentialStore); + } + + @Test + void complete_onAnExpiredHandshakeClearsItWithoutClaiming() { + ConnectState state = openHandshake(); + state.setExpiresAt(LocalDateTime.now().minusSeconds(1)); + when(stateRepo.findById(ConnectState.SINGLETON_ID)).thenReturn(Optional.of(state)); + + assertThat(service.complete(NONCE).phase()).isEqualTo(Phase.EXPIRED); + verify(stateRepo).delete(state); + verify(client, never()).connectClaim(anyString(), anyString()); + } + + @Test + void startReauth_presentsTheCredentialSoSaaSCanPinTheTeam() throws Exception { + when(credentialStore.get()).thenReturn(Optional.of(credential(7L))); + when(client.connectRequest(any(), anyString(), anyString(), anyString(), any())) + .thenReturn(new ConnectRequestResult("req-1", 900, AUTHORIZE_URL)); + + service.startReauth(fromRequest("https://pdf.example.com")); + + // Sending the credential is what makes the pinning trustworthy: the team comes from + // something only this instance holds. + verify(client) + .connectRequest( + any(), + anyString(), + anyString(), + anyString(), + org.mockito.ArgumentMatchers.argThat( + c -> c != null && "dev".equals(c.getDeviceId()))); + } + + @Test + void startReauth_onAnUnlinkedServerFails() { + assertThat(catchIo(() -> service.startReauth(fromRequest("https://pdf.example.com")))) + .hasMessageContaining("not linked"); + verifyNoInteractions(client); + } + + @Test + void complete_onAConfirmedReauthKeepsTheExistingCredential() { + ConnectState state = openHandshake(); + when(stateRepo.findById(ConnectState.SINGLETON_ID)).thenReturn(Optional.of(state)); + when(client.connectClaim(anyString(), anyString())) + .thenReturn(new ConnectClaimResult(ConnectClaimOutcome.CONFIRMED, null, null, 7L)); + + ConnectService.ConnectStatus status = service.complete(NONCE); + + assertThat(status.phase()).isEqualTo(Phase.LINKED); + assertThat(status.teamId()).isEqualTo(7L); + // Nothing to store: a second credential would orphan the one we already hold. + verify(credentialStore, never()).save(anyString(), anyString(), any()); + verify(stateRepo).delete(state); + } + + @Test + void status_reportsNothingInFlightWhenThereIsNoHandshakeOrCredential() { + assertThat(service.status().phase()).isEqualTo(Phase.NONE); + } + + @Test + void status_reportsAnExpiredHandshakeRatherThanOfferingAStaleLink() { + ConnectState state = openHandshake(); + state.setExpiresAt(LocalDateTime.now().minusSeconds(1)); + when(stateRepo.findById(ConnectState.SINGLETON_ID)).thenReturn(Optional.of(state)); + + ConnectService.ConnectStatus status = service.status(); + + assertThat(status.phase()).isEqualTo(Phase.EXPIRED); + assertThat(status.authorizeUrl()).isNull(); + } + + @Test + void status_countsDownWhileAHandshakeIsOpen() { + when(stateRepo.findById(ConnectState.SINGLETON_ID)) + .thenReturn(Optional.of(openHandshake())); + + ConnectService.ConnectStatus status = service.status(); + + assertThat(status.phase()).isEqualTo(Phase.PENDING); + assertThat(status.secondsRemaining()).isPositive(); + assertThat(status.authorizeUrl()).isEqualTo("https://app.example.com/link?request=req-1"); + } + + // --------------------------------------------------------------------------------------- + + /** A start with nothing but the reconstructed request URL, as a headless caller would send. */ + private static ConnectService.CallbackHint fromRequest(String derivedBaseUrl) { + return new ConnectService.CallbackHint(null, null, derivedBaseUrl); + } + + private void stubCreate() throws Exception { + // The five-argument overload: a first link passes a null credential rather than none. + when(client.connectRequest(any(), anyString(), anyString(), anyString(), any())) + .thenReturn(new ConnectRequestResult("req-1", 900, AUTHORIZE_URL)); + } + + private static ConnectState openHandshake() { + ConnectState state = new ConnectState(); + state.setId(ConnectState.SINGLETON_ID); + state.setRequestId("req-1"); + state.setNonce(NONCE); + state.setClaimSecret(CLAIM_SECRET); + state.setCallbackUrl("https://pdf.example.com/account-link/callback"); + state.setAuthorizeUrl("https://app.example.com/link?request=req-1"); + state.setCreatedAt(LocalDateTime.now()); + state.setExpiresAt(LocalDateTime.now().plusMinutes(10)); + return state; + } + + private static DeviceCredential credential(Long teamId) { + DeviceCredential credential = new DeviceCredential(); + credential.setDeviceId("dev"); + credential.setDeviceSecret("sec"); + credential.setTeamId(teamId); + credential.setLinkedAt(LocalDateTime.now()); + return credential; + } + + /** Runs a throwing call and returns the exception, so the assertion reads in one line. */ + private static Throwable catchIo(ThrowingCall call) { + try { + call.run(); + throw new AssertionError("expected the call to fail"); + } catch (Exception e) { + return e; + } + } + + private interface ThrowingCall { + void run() throws Exception; + } +} diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/AccountLinkController.java b/app/saas/src/main/java/stirling/software/saas/accountlink/AccountLinkController.java index 646791f76e..4703c32496 100644 --- a/app/saas/src/main/java/stirling/software/saas/accountlink/AccountLinkController.java +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/AccountLinkController.java @@ -4,14 +4,12 @@ import java.util.List; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Profile; -import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.Authentication; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; -import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; @@ -19,25 +17,9 @@ import io.swagger.v3.oas.annotations.Hidden; import lombok.extern.slf4j.Slf4j; -import stirling.software.common.model.enumeration.TeamRole; -import stirling.software.proprietary.model.TeamMembership; -import stirling.software.proprietary.security.database.repository.UserRepository; -import stirling.software.proprietary.security.model.User; -import stirling.software.proprietary.security.repository.TeamMembershipRepository; -import stirling.software.saas.util.AuthenticationUtils; +import stirling.software.saas.accountlink.LeaderTeamResolver.LeaderTeam; -/** - * Account-link registration surface (combined-billing "Mode A"). - * - *

A self-hosted instance's local backend calls {@code POST /register} with the admin's - * short-lived Supabase JWT (validated by the existing {@code SupabaseSecurityConfig} chain — no new - * auth here). We resolve the caller's team, mint a device credential bound to it, and return the - * secret exactly once. Ongoing entitlement reads authenticate with that device credential, not this - * JWT. - * - *

Whole surface gated behind {@code stirling.billing.account-link.enabled}: off → beans absent → - * 404. Leader-only, and the team is always derived from the caller (never the request body). - */ +/** Team-wide management of linked instances (combined billing). */ @Slf4j @Hidden @RestController @@ -47,25 +29,13 @@ import stirling.software.saas.util.AuthenticationUtils; public class AccountLinkController { private final AccountLinkService service; - private final TeamMembershipRepository memberRepo; - private final UserRepository userRepository; + private final LeaderTeamResolver leaderTeams; - public AccountLinkController( - AccountLinkService service, - TeamMembershipRepository memberRepo, - UserRepository userRepository) { + public AccountLinkController(AccountLinkService service, LeaderTeamResolver leaderTeams) { this.service = service; - this.memberRepo = memberRepo; - this.userRepository = userRepository; + this.leaderTeams = leaderTeams; } - /** Optional display name for the instance (hostname / label). */ - public record RegisterRequest(String name) {} - - /** {@code deviceSecret} is plaintext and returned exactly once — the caller must store it. */ - public record RegisterResponse( - Long instanceId, Long teamId, String deviceId, String deviceSecret, String name) {} - public record InstanceRow( Long instanceId, String deviceId, @@ -74,31 +44,10 @@ public class AccountLinkController { String lastSeenAt, boolean revoked) {} - @PostMapping("/register") - @PreAuthorize("isAuthenticated()") - public ResponseEntity register( - @RequestBody(required = false) RegisterRequest req, Authentication auth) { - LeaderTeam lt = resolveLeaderTeam(auth); - if (lt.error() != null) { - return ResponseEntity.status(lt.error()).build(); - } - String name = req != null ? req.name() : null; - AccountLinkService.RegisteredInstance reg = - service.register(lt.teamId(), lt.userId(), name); - return ResponseEntity.status(HttpStatus.CREATED) - .body( - new RegisterResponse( - reg.instanceId(), - lt.teamId(), - reg.deviceId(), - reg.deviceSecret(), - reg.name())); - } - @GetMapping("/instances") @PreAuthorize("isAuthenticated()") public ResponseEntity> list(Authentication auth) { - LeaderTeam lt = resolveLeaderTeam(auth); + LeaderTeam lt = leaderTeams.resolve(auth); if (lt.error() != null) { return ResponseEntity.status(lt.error()).build(); } @@ -124,38 +73,11 @@ public class AccountLinkController { @PostMapping("/instances/{instanceId}/revoke") @PreAuthorize("isAuthenticated()") public ResponseEntity revoke(@PathVariable Long instanceId, Authentication auth) { - LeaderTeam lt = resolveLeaderTeam(auth); + LeaderTeam lt = leaderTeams.resolve(auth); if (lt.error() != null) { return ResponseEntity.status(lt.error()).build(); } boolean ok = service.revoke(lt.teamId(), instanceId); return ok ? ResponseEntity.noContent().build() : ResponseEntity.notFound().build(); } - - // --------------------------------------------------------------------------------------- - // Helpers — team always derived from the caller; instance linking is a leader (billing) action. - // --------------------------------------------------------------------------------------- - - /** - * Resolved caller team, or an {@code error} status to return (teamId/userId null when error). - */ - private record LeaderTeam(Long teamId, Long userId, HttpStatus error) {} - - private LeaderTeam resolveLeaderTeam(Authentication auth) { - User user; - try { - user = AuthenticationUtils.getCurrentUser(auth, userRepository); - } catch (SecurityException e) { - return new LeaderTeam(null, null, HttpStatus.UNAUTHORIZED); - } - List rows = memberRepo.findPrimaryMembership(user.getId()); - if (rows.isEmpty()) { - return new LeaderTeam(null, null, HttpStatus.FORBIDDEN); - } - TeamMembership m = rows.getFirst(); - if (m.getRole() != TeamRole.LEADER) { - return new LeaderTeam(null, null, HttpStatus.FORBIDDEN); - } - return new LeaderTeam(m.getTeam().getId(), user.getId(), null); - } } diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/AccountLinkService.java b/app/saas/src/main/java/stirling/software/saas/accountlink/AccountLinkService.java index c31fc1b03e..fe6a0cc0e6 100644 --- a/app/saas/src/main/java/stirling/software/saas/accountlink/AccountLinkService.java +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/AccountLinkService.java @@ -18,16 +18,7 @@ import org.springframework.transaction.annotation.Transactional; import lombok.extern.slf4j.Slf4j; -/** - * Account-link instance registration + lifecycle (combined-billing "Mode A"). - * - *

Mints a {@code device_id} (public) + {@code device_secret} (high-entropy, returned once) bound - * to a team, persisting only the SHA-256 hash of the secret. The instance authenticates its - * unattended entitlement reads with that credential. - * - *

Gated behind {@code stirling.billing.account-link.enabled}: when off the bean is absent, so - * {@link AccountLinkController} (which depends on it) drops out too and its endpoints 404. - */ +/** Account-link instance registration + lifecycle (combined billing). */ @Slf4j @Service @Profile("saas") @@ -80,10 +71,7 @@ public class AccountLinkService { return repo.findByTeamIdOrderByCreatedAtDesc(teamId); } - /** - * Revokes an instance iff it belongs to {@code teamId}. Returns false if not found or owned by - * a different team (so a caller can never revoke another team's instance). Idempotent. - */ + /** Revokes an instance iff it belongs to {@code teamId}. */ @Transactional public boolean revoke(Long teamId, Long instanceId) { Optional found = repo.findById(instanceId); @@ -99,13 +87,30 @@ public class AccountLinkService { return true; } + /** + * Resolves an active instance from a device credential, or empty if it does not authenticate. + */ + @Transactional(readOnly = true) + public Optional resolveActiveInstance(String deviceId, String deviceSecret) { + if (deviceId == null || deviceSecret == null) { + return Optional.empty(); + } + return repo.findByDeviceIdAndRevokedAtIsNull(deviceId) + .filter( + instance -> + MessageDigest.isEqual( + sha256Hex(deviceSecret).getBytes(StandardCharsets.UTF_8), + instance.getDeviceSecretHash() + .getBytes(StandardCharsets.UTF_8))); + } + private String randomSecret() { byte[] buf = new byte[SECRET_BYTES]; random.nextBytes(buf); return Base64.getUrlEncoder().withoutPadding().encodeToString(buf); } - /** SHA-256 hex of a value. The device secret is high-entropy, so no salt is required. */ + /** SHA-256 hex of a value. */ static String sha256Hex(String value) { try { MessageDigest md = MessageDigest.getInstance("SHA-256"); diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectController.java b/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectController.java new file mode 100644 index 0000000000..95f2ba5de7 --- /dev/null +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectController.java @@ -0,0 +1,277 @@ +package stirling.software.saas.accountlink; + +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import java.util.Map; +import java.util.Optional; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Profile; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.Authentication; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import io.swagger.v3.oas.annotations.Hidden; + +import jakarta.servlet.http.HttpServletRequest; + +import lombok.extern.slf4j.Slf4j; + +import stirling.software.common.model.ApplicationProperties; +import stirling.software.saas.accountlink.LeaderTeamResolver.LeaderTeam; + +/** Browser-mediated "connect this server" handshake. */ +@Slf4j +@Hidden +@RestController +@RequestMapping("/api/v1/account-link/connect") +@Profile("saas") +@ConditionalOnProperty(name = "stirling.billing.account-link.enabled", havingValue = "true") +public class ConnectController { + + /** Same headers the device-credential filter uses on the {@code /api/v1/instance} paths. */ + static final String HEADER_DEVICE_ID = "X-Device-Id"; + + static final String HEADER_DEVICE_SECRET = "X-Device-Secret"; + + /** Frontend route serving the approval page. */ + static final String LINK_PATH = "/link"; + + private final ConnectRequestService service; + private final LeaderTeamResolver leaderTeams; + private final AccountLinkService accountLinkService; + private final ApplicationProperties applicationProperties; + + public ConnectController( + ConnectRequestService service, + LeaderTeamResolver leaderTeams, + AccountLinkService accountLinkService, + ApplicationProperties applicationProperties) { + this.service = service; + this.leaderTeams = leaderTeams; + this.accountLinkService = accountLinkService; + this.applicationProperties = applicationProperties; + } + + /** Sent by the instance's own backend, before it holds any credential. */ + public record CreateBody(String name, String callbackUrl, String nonce, String claimSecret) {} + + /** {@code authorizeUrl} is where the instance should send its admin. */ + public record CreateResponse(String requestId, int expiresIn, String authorizeUrl) {} + + /** What the approval page renders. */ + public record ViewResponse( + String requestId, + String name, + String callbackOrigin, + boolean insecureTransport, + String mode, + String status) {} + + /** Where the approver's browser goes next, and the correlator the instance is waiting on. */ + public record ApproveResponse(String callbackUrl, String nonce) {} + + public record ClaimBody(String requestId, String claimSecret) {} + + public record ClaimResponse(String deviceId, String deviceSecret, Long teamId) {} + + /** Opens a handshake. */ + @PostMapping("/request") + public ResponseEntity request( + @RequestBody(required = false) CreateBody body, HttpServletRequest http) { + if (body == null) { + return ResponseEntity.badRequest().body(Map.of("error", "BAD_REQUEST")); + } + String deviceId = http.getHeader(HEADER_DEVICE_ID); + String deviceSecret = http.getHeader(HEADER_DEVICE_SECRET); + boolean reauthRequested = deviceId != null || deviceSecret != null; + + ConnectRequestService.CreateResult result; + if (reauthRequested) { + Long pinnedTeamId = + accountLinkService + .resolveActiveInstance(deviceId, deviceSecret) + .map(LinkedInstance::getTeamId) + .orElse(null); + result = + service.createReauth( + body.name(), + body.callbackUrl(), + body.nonce(), + body.claimSecret(), + clientIp(http), + pinnedTeamId); + } else { + result = + service.create( + body.name(), + body.callbackUrl(), + body.nonce(), + body.claimSecret(), + clientIp(http)); + } + if (result.isRejected()) { + return switch (result.rejection()) { + case RATE_LIMITED -> + ResponseEntity.status(HttpStatus.TOO_MANY_REQUESTS) + .body(Map.of("error", "RATE_LIMITED")); + case BAD_CALLBACK -> + ResponseEntity.badRequest().body(Map.of("error", "BAD_CALLBACK")); + case BAD_NONCE -> ResponseEntity.badRequest().body(Map.of("error", "BAD_NONCE")); + case BAD_SECRET -> ResponseEntity.badRequest().body(Map.of("error", "BAD_SECRET")); + // A credential was offered and did not authenticate. Same answer as any other bad + // credential, and deliberately not distinguishable from "revoked". + case NOT_LINKED -> + ResponseEntity.status(HttpStatus.UNAUTHORIZED) + .body(Map.of("error", "NOT_LINKED")); + }; + } + return ResponseEntity.status(HttpStatus.CREATED) + .body( + new CreateResponse( + result.requestId(), + result.expiresInSeconds(), + authorizeUrl(result.requestId(), http))); + } + + /** + * Where to send the admin to approve a handshake. {@code system.frontendUrl} is the web app's + * own base URL, including any base path; without it the API's origin has to serve the app too. + */ + private String authorizeUrl(String requestId, HttpServletRequest http) { + String frontendUrl = applicationProperties.getSystem().getFrontendUrl(); + String base = + frontendUrl != null && !frontendUrl.isBlank() + ? frontendUrl.strip().replaceAll("/+$", "") + : requestOrigin(http); + return base + + LINK_PATH + + "?request=" + + URLEncoder.encode(requestId, StandardCharsets.UTF_8); + } + + /** Scheme, host and context path as the browser reached us, honouring a reverse proxy. */ + private static String requestOrigin(HttpServletRequest request) { + String proto = firstHop(request.getHeader("X-Forwarded-Proto")); + String host = firstHop(request.getHeader("X-Forwarded-Host")); + String scheme = proto != null ? proto : request.getScheme(); + // A forwarded host already carries its own port, if it needs one. + String hostPort = + host != null + ? host + : Origins.hostPort( + scheme, request.getServerName(), request.getServerPort()); + String context = request.getContextPath() == null ? "" : request.getContextPath(); + return scheme + "://" + hostPort + context; + } + + private static String firstHop(String headerValue) { + if (headerValue == null || headerValue.isBlank()) { + return null; + } + String first = headerValue.split(",")[0].strip(); + return first.isEmpty() ? null : first; + } + + /** Detail for the approval page. */ + @GetMapping("/{requestId}") + @PreAuthorize("isAuthenticated()") + public ResponseEntity view(@PathVariable String requestId) { + return service.lookup(requestId) + .map( + v -> + ResponseEntity.ok( + new ViewResponse( + v.requestId(), + v.name(), + v.callbackOrigin(), + v.insecureTransport(), + v.mode().name(), + v.status().name()))) + .orElseGet(() -> ResponseEntity.notFound().build()); + } + + /** Approves a handshake. */ + @PostMapping("/{requestId}/approve") + @PreAuthorize("isAuthenticated()") + public ResponseEntity approve(@PathVariable String requestId, Authentication auth) { + Optional view = service.lookup(requestId); + if (view.isEmpty()) { + return ResponseEntity.notFound().build(); + } + boolean reauth = view.get().mode() == ConnectRequest.Mode.REAUTH; + LeaderTeam lt = reauth ? leaderTeams.resolveMember(auth) : leaderTeams.resolve(auth); + if (lt.isError()) { + return ResponseEntity.status(lt.error()).build(); + } + ConnectRequestService.ApproveResult result = + service.approve(requestId, lt.teamId(), lt.userId()); + if (result.isRejected()) { + return switch (result.rejection()) { + // Named separately so the page can say "you are signed in to a different account" + // rather than implying the request itself was bad. + case WRONG_TEAM -> + ResponseEntity.status(HttpStatus.CONFLICT) + .body(Map.of("error", "WRONG_TEAM")); + case UNAVAILABLE -> ResponseEntity.notFound().build(); + }; + } + return ResponseEntity.ok( + new ApproveResponse(result.target().callbackUrl(), result.target().nonce())); + } + + @PostMapping("/{requestId}/deny") + @PreAuthorize("isAuthenticated()") + public ResponseEntity deny(@PathVariable String requestId, Authentication auth) { + LeaderTeam lt = leaderTeams.resolve(auth); + if (lt.isError()) { + return ResponseEntity.status(lt.error()).build(); + } + return service.deny(requestId) + ? ResponseEntity.noContent().build() + : ResponseEntity.notFound().build(); + } + + /** Collects the device credential. */ + @PostMapping("/claim") + public ResponseEntity claim(@RequestBody(required = false) ClaimBody body) { + if (body == null) { + return ResponseEntity.badRequest().body(Map.of("error", "BAD_REQUEST")); + } + ConnectRequestService.ClaimResult result = + service.claim(body.requestId(), body.claimSecret()); + return switch (result.outcome()) { + case GRANTED -> + ResponseEntity.ok( + new ClaimResponse( + result.deviceId(), result.deviceSecret(), result.teamId())); + // A re-authentication carries no credential: the instance already has one. It only + // needs to know the browser leg succeeded, and which team it was confirmed against. + case CONFIRMED -> + ResponseEntity.ok(Map.of("status", "confirmed", "teamId", result.teamId())); + case PENDING -> + ResponseEntity.status(HttpStatus.ACCEPTED).body(Map.of("status", "pending")); + case REJECTED -> ResponseEntity.badRequest().body(Map.of("error", "CONNECT_REJECTED")); + }; + } + + /** + * Source address for the creation cap. + * + *

Deliberately not reading {@code X-Forwarded-For}: the caller sets it, so keying a cap on + * it lets one rotate fake addresses and have no cap at all. {@code + * server.forward-headers-strategy} is NATIVE, so the container has already resolved the real + * client from trusted proxies. + */ + private static String clientIp(HttpServletRequest request) { + String remote = request.getRemoteAddr(); + return remote == null || remote.length() <= 45 ? remote : remote.substring(0, 45); + } +} diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequest.java b/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequest.java new file mode 100644 index 0000000000..f9c204eef3 --- /dev/null +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequest.java @@ -0,0 +1,103 @@ +package stirling.software.saas.accountlink; + +import java.time.LocalDateTime; + +import org.hibernate.annotations.CreationTimestamp; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.EnumType; +import jakarta.persistence.Enumerated; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.Index; +import jakarta.persistence.Table; + +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +/** One in-flight "connect this server" handshake. Short lived and single use. */ +@Entity +@Table( + name = "account_link_connect_request", + indexes = @Index(name = "idx_alcr_ip_created", columnList = "requester_ip,created_at")) +@Getter +@Setter +@NoArgsConstructor +public class ConnectRequest { + + public enum Mode { + LINK, + REAUTH + } + + public enum Status { + PENDING, + APPROVED, + DENIED, + CONSUMED + } + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + + @Column(name = "request_id", nullable = false, unique = true, length = 64) + private String requestId; + + @Column(name = "name", length = 255) + private String name; + + /** + * Read back from here on approval, never from the request: that is what stops an open redirect. + */ + @Column(name = "callback_url", nullable = false, length = 2048) + private String callbackUrl; + + @Column(name = "callback_origin", nullable = false, length = 255) + private String callbackOrigin; + + @Column(name = "nonce", nullable = false, length = 128) + private String nonce; + + /** SHA-256; the secret itself is never stored. */ + @Column(name = "claim_secret_hash", nullable = false, length = 64) + private String claimSecretHash; + + @Enumerated(EnumType.STRING) + @Column(name = "mode", nullable = false, length = 16) + private Mode mode = Mode.LINK; + + @Enumerated(EnumType.STRING) + @Column(name = "status", nullable = false, length = 16) + private Status status = Status.PENDING; + + /** LINK: set on approval. REAUTH: pinned at creation, so approval can only confirm it. */ + @Column(name = "team_id") + private Long teamId; + + @Column(name = "approved_by_user_id") + private Long approvedByUserId; + + @Column(name = "requester_ip", length = 45) + private String requesterIp; + + @CreationTimestamp + @Column(name = "created_at", nullable = false, updatable = false) + private LocalDateTime createdAt; + + @Column(name = "expires_at", nullable = false) + private LocalDateTime expiresAt; + + @Column(name = "approved_at") + private LocalDateTime approvedAt; + + @Column(name = "consumed_at") + private LocalDateTime consumedAt; + + public boolean isExpired(LocalDateTime now) { + return expiresAt != null && expiresAt.isBefore(now); + } +} diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequestCleanupService.java b/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequestCleanupService.java new file mode 100644 index 0000000000..7712ee297a --- /dev/null +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequestCleanupService.java @@ -0,0 +1,47 @@ +package stirling.software.saas.accountlink; + +import java.time.LocalDateTime; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Profile; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +/** + * Removes connect requests that are past use. + * + *

Needed rather than merely tidy: anyone can create a row on {@code POST /connect/request}, and + * nothing else deletes one. Requests hold a callback URL and the requester's address, so they are + * swept soon after expiry rather than kept. + */ +@Slf4j +@Service +@Profile("saas") +@ConditionalOnProperty(name = "stirling.billing.account-link.enabled", havingValue = "true") +@RequiredArgsConstructor +public class ConnectRequestCleanupService { + + /** Long enough to answer "what happened to my link?" the next morning, and no longer. */ + private static final int RETAIN_HOURS = 24; + + private final ConnectRequestRepository repo; + + @Scheduled(cron = "0 30 3 * * *") + @Transactional + public void purgeExpired() { + try { + LocalDateTime cutoff = LocalDateTime.now().minusHours(RETAIN_HOURS); + int deleted = repo.deleteByExpiresAtBefore(cutoff); + if (deleted > 0) { + log.info("Account-link connect: purged {} expired requests", deleted); + } + } catch (Exception e) { + // A failed sweep must not take the scheduler down; the next run retries. + log.error("Account-link connect: purge failed", e); + } + } +} diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequestRepository.java b/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequestRepository.java new file mode 100644 index 0000000000..6c5dec098b --- /dev/null +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequestRepository.java @@ -0,0 +1,28 @@ +package stirling.software.saas.accountlink; + +import java.time.LocalDateTime; +import java.util.Optional; + +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Lock; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; + +import jakarta.persistence.LockModeType; + +/** Data access for {@link ConnectRequest}. */ +public interface ConnectRequestRepository extends JpaRepository { + + Optional findByRequestId(String requestId); + + /** Row-locking read used by approve, deny and claim. */ + @Lock(LockModeType.PESSIMISTIC_WRITE) + @Query("SELECT r FROM ConnectRequest r WHERE r.requestId = :requestId") + Optional findByRequestIdForUpdate(@Param("requestId") String requestId); + + /** Backs the per-IP creation cap, since creating a request needs no authentication. */ + long countByRequesterIpAndCreatedAtAfter(String requesterIp, LocalDateTime after); + + /** Sweeps rows past use, whatever they settled as. Anyone can create these. */ + int deleteByExpiresAtBefore(LocalDateTime cutoff); +} diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequestService.java b/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequestService.java new file mode 100644 index 0000000000..64abc12fa6 --- /dev/null +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/ConnectRequestService.java @@ -0,0 +1,391 @@ +package stirling.software.saas.accountlink; + +import java.net.URI; +import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.SecureRandom; +import java.time.LocalDateTime; +import java.util.Base64; +import java.util.Locale; +import java.util.Optional; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Profile; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import lombok.extern.slf4j.Slf4j; + +/** The "connect this server" handshake, SaaS side. */ +@Slf4j +@Service +@Profile("saas") +@ConditionalOnProperty(name = "stirling.billing.account-link.enabled", havingValue = "true") +public class ConnectRequestService { + + /** + * Long enough for the approver to sign in, pick the right account and read the origin. Sized + * for the slowest real route: signing up, waiting for a confirmation email, and coming back. + */ + static final int LIFETIME_MINUTES = 30; + + /** Creating a request needs no authentication, so the only brake is per-source volume. */ + static final int MAX_REQUESTS_PER_IP = 10; + + private static final int REQUEST_ID_BYTES = 32; + private static final int MAX_NONCE_LENGTH = 128; + private static final int MAX_CALLBACK_LENGTH = 2048; + private static final int MAX_NAME_LENGTH = 255; + + private final ConnectRequestRepository repo; + private final AccountLinkService accountLinkService; + private final SecureRandom random = new SecureRandom(); + + public ConnectRequestService( + ConnectRequestRepository repo, AccountLinkService accountLinkService) { + this.repo = repo; + this.accountLinkService = accountLinkService; + } + + /** Rejected creation attempts, so the controller can pick a status without parsing messages. */ + public enum CreateRejection { + BAD_CALLBACK, + BAD_NONCE, + BAD_SECRET, + RATE_LIMITED, + /** + * A re-authentication was asked for by something that could not prove it is a linked + * instance. + */ + NOT_LINKED + } + + /** Either a created request id, or the reason we would not create one. */ + public record CreateResult(String requestId, int expiresInSeconds, CreateRejection rejection) { + static CreateResult ok(String requestId, int expiresInSeconds) { + return new CreateResult(requestId, expiresInSeconds, null); + } + + static CreateResult rejected(CreateRejection rejection) { + return new CreateResult(null, 0, rejection); + } + + public boolean isRejected() { + return rejection != null; + } + } + + /** What the approval page shows. */ + public record ConnectView( + String requestId, + String name, + String callbackOrigin, + boolean insecureTransport, + ConnectRequest.Mode mode, + ConnectRequest.Status status) {} + + /** Where to send the browser once approved, plus the correlator the instance is expecting. */ + public record ApprovalTarget(String callbackUrl, String nonce) {} + + public enum ClaimOutcome { + /** Approved and collected; {@code credential} is populated. */ + GRANTED, + /** A re-authentication was approved. */ + CONFIRMED, + /** Still waiting on a human. */ + PENDING, + /** Declined, expired, unknown, already collected, or a bad claim secret. */ + REJECTED + } + + public record ClaimResult( + ClaimOutcome outcome, String deviceId, String deviceSecret, Long teamId) { + static ClaimResult of(ClaimOutcome outcome) { + return new ClaimResult(outcome, null, null, null); + } + } + + /** Records a handshake on behalf of an instance that has no credential yet. */ + @Transactional + public CreateResult create( + String name, String callbackUrl, String nonce, String claimSecret, String requesterIp) { + return create(name, callbackUrl, nonce, claimSecret, requesterIp, null); + } + + /** + * As {@link #create}, but for an instance that is already linked and only needs its admin's + * browser signed in again. + */ + @Transactional + public CreateResult createReauth( + String name, + String callbackUrl, + String nonce, + String claimSecret, + String requesterIp, + Long pinnedTeamId) { + if (pinnedTeamId == null) { + return CreateResult.rejected(CreateRejection.NOT_LINKED); + } + return create(name, callbackUrl, nonce, claimSecret, requesterIp, pinnedTeamId); + } + + private CreateResult create( + String name, + String callbackUrl, + String nonce, + String claimSecret, + String requesterIp, + Long pinnedTeamId) { + if (nonce == null || nonce.isBlank() || nonce.length() > MAX_NONCE_LENGTH) { + return CreateResult.rejected(CreateRejection.BAD_NONCE); + } + if (claimSecret == null || claimSecret.isBlank()) { + return CreateResult.rejected(CreateRejection.BAD_SECRET); + } + Optional parsed = validateCallback(callbackUrl); + if (parsed.isEmpty()) { + return CreateResult.rejected(CreateRejection.BAD_CALLBACK); + } + LocalDateTime now = LocalDateTime.now(); + if (requesterIp != null + && repo.countByRequesterIpAndCreatedAtAfter(requesterIp, now.minusHours(1)) + >= MAX_REQUESTS_PER_IP) { + return CreateResult.rejected(CreateRejection.RATE_LIMITED); + } + + URI uri = parsed.get(); + ConnectRequest request = new ConnectRequest(); + request.setRequestId(randomToken()); + request.setName(trim(name, MAX_NAME_LENGTH)); + request.setCallbackUrl(uri.toString()); + request.setCallbackOrigin(originOf(uri)); + request.setNonce(nonce); + request.setClaimSecretHash(sha256Hex(claimSecret)); + request.setStatus(ConnectRequest.Status.PENDING); + request.setMode( + pinnedTeamId == null ? ConnectRequest.Mode.LINK : ConnectRequest.Mode.REAUTH); + request.setTeamId(pinnedTeamId); + request.setRequesterIp(requesterIp); + request.setExpiresAt(now.plusMinutes(LIFETIME_MINUTES)); + repo.save(request); + + // Never log the nonce or the claim secret; both are live. The request id is the safe + // handle for correlating a support request against this row. + log.info( + "Account-link connect: request {} created for origin {}", + request.getRequestId(), + request.getCallbackOrigin()); + return CreateResult.ok(request.getRequestId(), LIFETIME_MINUTES * 60); + } + + /** The approver's view of a handshake. */ + @Transactional(readOnly = true) + public Optional lookup(String requestId) { + return repo.findByRequestId(requestId) + .filter(r -> !r.isExpired(LocalDateTime.now())) + .map( + r -> + new ConnectView( + r.getRequestId(), + r.getName(), + r.getCallbackOrigin(), + !"https".equals(schemeOf(r.getCallbackOrigin())), + r.getMode(), + r.getStatus())); + } + + /** Why an approval was refused, so the page can say something useful. */ + public enum ApproveRejection { + /** Unknown, expired, or already settled. */ + UNAVAILABLE, + /** The approver's team is not the team this server already belongs to. */ + WRONG_TEAM + } + + public record ApproveResult(ApprovalTarget target, ApproveRejection rejection) { + public boolean isRejected() { + return target == null; + } + } + + /** Binds a pending handshake to the approver's team and returns where to send them next. */ + @Transactional + public ApproveResult approve(String requestId, Long teamId, Long userId) { + Optional found = repo.findByRequestIdForUpdate(requestId); + if (found.isEmpty()) { + return new ApproveResult(null, ApproveRejection.UNAVAILABLE); + } + ConnectRequest request = found.get(); + LocalDateTime now = LocalDateTime.now(); + if (request.isExpired(now) || request.getStatus() != ConnectRequest.Status.PENDING) { + return new ApproveResult(null, ApproveRejection.UNAVAILABLE); + } + Long pinned = request.getTeamId(); + if (pinned != null && !pinned.equals(teamId)) { + log.warn( + "Account-link connect: request {} approved by team {} but is pinned to team {};" + + " refusing", + requestId, + teamId, + pinned); + return new ApproveResult(null, ApproveRejection.WRONG_TEAM); + } + request.setStatus(ConnectRequest.Status.APPROVED); + request.setTeamId(teamId); + request.setApprovedByUserId(userId); + request.setApprovedAt(now); + repo.save(request); + log.info( + "Account-link connect: request {} approved for team {} ({})", + requestId, + teamId, + request.getMode()); + return new ApproveResult( + new ApprovalTarget(request.getCallbackUrl(), request.getNonce()), null); + } + + /** Declines a pending handshake. */ + @Transactional + public boolean deny(String requestId) { + Optional found = repo.findByRequestIdForUpdate(requestId); + if (found.isEmpty()) { + return false; + } + ConnectRequest request = found.get(); + if (request.getStatus() != ConnectRequest.Status.PENDING) { + return false; + } + request.setStatus(ConnectRequest.Status.DENIED); + repo.save(request); + log.info("Account-link connect: request {} denied", requestId); + return true; + } + + /** Collects the device credential for an approved handshake. */ + @Transactional + public ClaimResult claim(String requestId, String claimSecret) { + if (requestId == null || claimSecret == null) { + return ClaimResult.of(ClaimOutcome.REJECTED); + } + Optional found = repo.findByRequestIdForUpdate(requestId); + if (found.isEmpty()) { + return ClaimResult.of(ClaimOutcome.REJECTED); + } + ConnectRequest request = found.get(); + if (!secretMatches(claimSecret, request.getClaimSecretHash())) { + // Same answer as an unknown id: a caller probing ids learns nothing from the + // difference. + log.warn("Account-link connect: claim for request {} had a bad secret", requestId); + return ClaimResult.of(ClaimOutcome.REJECTED); + } + if (request.isExpired(LocalDateTime.now())) { + return ClaimResult.of(ClaimOutcome.REJECTED); + } + return switch (request.getStatus()) { + case PENDING -> ClaimResult.of(ClaimOutcome.PENDING); + case APPROVED -> mint(request); + case DENIED, CONSUMED -> ClaimResult.of(ClaimOutcome.REJECTED); + }; + } + + /** Settles an approved handshake. */ + private ClaimResult mint(ConnectRequest request) { + if (request.getMode() == ConnectRequest.Mode.REAUTH) { + request.setStatus(ConnectRequest.Status.CONSUMED); + request.setConsumedAt(LocalDateTime.now()); + repo.save(request); + log.info( + "Account-link connect: request {} re-authenticated for team {}", + request.getRequestId(), + request.getTeamId()); + return new ClaimResult(ClaimOutcome.CONFIRMED, null, null, request.getTeamId()); + } + AccountLinkService.RegisteredInstance registered = + accountLinkService.register( + request.getTeamId(), request.getApprovedByUserId(), request.getName()); + request.setStatus(ConnectRequest.Status.CONSUMED); + request.setConsumedAt(LocalDateTime.now()); + repo.save(request); + log.info( + "Account-link connect: request {} claimed, instance {} bound to team {}", + request.getRequestId(), + registered.instanceId(), + request.getTeamId()); + return new ClaimResult( + ClaimOutcome.GRANTED, + registered.deviceId(), + registered.deviceSecret(), + request.getTeamId()); + } + + /** Absolute http(s) URL, with a host, no credentials and no fragment of its own. */ + static Optional validateCallback(String candidate) { + if (candidate == null || candidate.isBlank() || candidate.length() > MAX_CALLBACK_LENGTH) { + return Optional.empty(); + } + URI uri; + try { + uri = new URI(candidate.strip()); + } catch (URISyntaxException e) { + return Optional.empty(); + } + if (!uri.isAbsolute() || uri.getScheme() == null) { + return Optional.empty(); + } + String scheme = uri.getScheme().toLowerCase(Locale.ROOT); + if (!"http".equals(scheme) && !"https".equals(scheme)) { + return Optional.empty(); + } + if (uri.getHost() == null || uri.getHost().isBlank()) { + return Optional.empty(); + } + if (uri.getUserInfo() != null || uri.getFragment() != null) { + return Optional.empty(); + } + return Optional.of(uri); + } + + /** Scheme, host and port, with the default port omitted so origins compare cleanly. */ + static String originOf(URI uri) { + String scheme = uri.getScheme().toLowerCase(Locale.ROOT); + return scheme + "://" + Origins.hostPort(scheme, uri.getHost(), uri.getPort()); + } + + private static String schemeOf(String origin) { + int sep = origin.indexOf("://"); + return sep < 0 ? "" : origin.substring(0, sep); + } + + private static String trim(String value, int max) { + if (value == null) { + return null; + } + String stripped = value.strip(); + if (stripped.isEmpty()) { + return null; + } + return stripped.length() <= max ? stripped : stripped.substring(0, max); + } + + private String randomToken() { + byte[] buf = new byte[REQUEST_ID_BYTES]; + random.nextBytes(buf); + return Base64.getUrlEncoder().withoutPadding().encodeToString(buf); + } + + /** Constant-time comparison so a claim cannot be brute-forced a byte at a time. */ + private static boolean secretMatches(String candidate, String expectedHash) { + if (expectedHash == null) { + return false; + } + return MessageDigest.isEqual( + sha256Hex(candidate).getBytes(StandardCharsets.UTF_8), + expectedHash.getBytes(StandardCharsets.UTF_8)); + } + + private static String sha256Hex(String value) { + return AccountLinkService.sha256Hex(value); + } +} diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/DeviceCredentialAuthenticationFilter.java b/app/saas/src/main/java/stirling/software/saas/accountlink/DeviceCredentialAuthenticationFilter.java index a2fd13a095..17cb340d60 100644 --- a/app/saas/src/main/java/stirling/software/saas/accountlink/DeviceCredentialAuthenticationFilter.java +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/DeviceCredentialAuthenticationFilter.java @@ -19,7 +19,7 @@ import jakarta.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; /** - * Authenticates a linked self-hosted instance by its device credential (combined-billing "Mode A"). + * Authenticates a linked self-hosted instance by its device credential (combined billing). * *

Reads {@code X-Device-Id} + {@code X-Device-Secret}, looks up the active {@link * LinkedInstance}, and constant-time compares the SHA-256 of the presented secret against the diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/InstanceController.java b/app/saas/src/main/java/stirling/software/saas/accountlink/InstanceController.java index 7c98d5e59b..9c313a1dfa 100644 --- a/app/saas/src/main/java/stirling/software/saas/accountlink/InstanceController.java +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/InstanceController.java @@ -32,9 +32,9 @@ import stirling.software.saas.payg.policy.PricingPolicy; import stirling.software.saas.payg.policy.PricingPolicyService; /** - * Instance-facing surface (combined-billing "Mode A"), authenticated by the device - * credential — not a user JWT. Separate path prefix ({@code /api/v1/instance/**}) so the device - * credential is scoped here and nowhere else. + * Instance-facing surface (combined billing), authenticated by the device credential — not a + * user JWT. Separate path prefix ({@code /api/v1/instance/**}) so the device credential is scoped + * here and nowhere else. * *

{@code GET /whoami} is the MVP round-trip proof: a registered instance presenting a valid * device credential gets back its resolved {@code instanceId} + {@code teamId}. {@code GET diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/LeaderTeamResolver.java b/app/saas/src/main/java/stirling/software/saas/accountlink/LeaderTeamResolver.java new file mode 100644 index 0000000000..7118562a28 --- /dev/null +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/LeaderTeamResolver.java @@ -0,0 +1,68 @@ +package stirling.software.saas.accountlink; + +import java.util.List; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Profile; +import org.springframework.http.HttpStatus; +import org.springframework.security.core.Authentication; +import org.springframework.stereotype.Component; + +import stirling.software.common.model.enumeration.TeamRole; +import stirling.software.proprietary.model.TeamMembership; +import stirling.software.proprietary.security.database.repository.UserRepository; +import stirling.software.proprietary.security.model.User; +import stirling.software.proprietary.security.repository.TeamMembershipRepository; +import stirling.software.saas.util.AuthenticationUtils; + +/** Who is allowed to bind a self-hosted instance to a team. */ +@Component +@Profile("saas") +@ConditionalOnProperty(name = "stirling.billing.account-link.enabled", havingValue = "true") +public class LeaderTeamResolver { + + private final TeamMembershipRepository memberRepo; + private final UserRepository userRepository; + + public LeaderTeamResolver(TeamMembershipRepository memberRepo, UserRepository userRepository) { + this.memberRepo = memberRepo; + this.userRepository = userRepository; + } + + /** + * Resolved caller, or an {@code error} status to return ({@code teamId}/{@code userId} null). + */ + public record LeaderTeam(Long teamId, Long userId, HttpStatus error) { + public boolean isError() { + return error != null; + } + } + + /** Caller must lead their team. */ + public LeaderTeam resolve(Authentication auth) { + return resolve(auth, true); + } + + /** Caller need only belong to a team. */ + public LeaderTeam resolveMember(Authentication auth) { + return resolve(auth, false); + } + + private LeaderTeam resolve(Authentication auth, boolean requireLeader) { + User user; + try { + user = AuthenticationUtils.getCurrentUser(auth, userRepository); + } catch (SecurityException e) { + return new LeaderTeam(null, null, HttpStatus.UNAUTHORIZED); + } + List rows = memberRepo.findPrimaryMembership(user.getId()); + if (rows.isEmpty()) { + return new LeaderTeam(null, null, HttpStatus.FORBIDDEN); + } + TeamMembership membership = rows.getFirst(); + if (requireLeader && membership.getRole() != TeamRole.LEADER) { + return new LeaderTeam(null, null, HttpStatus.FORBIDDEN); + } + return new LeaderTeam(membership.getTeam().getId(), user.getId(), null); + } +} diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/LinkedInstance.java b/app/saas/src/main/java/stirling/software/saas/accountlink/LinkedInstance.java index ec92c97758..f460e82d5b 100644 --- a/app/saas/src/main/java/stirling/software/saas/accountlink/LinkedInstance.java +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/LinkedInstance.java @@ -16,7 +16,7 @@ import lombok.NoArgsConstructor; import lombok.Setter; /** - * One self-hosted instance that has linked a SaaS account (combined-billing "Mode A", {@code + * One self-hosted instance that has linked a SaaS account (combined billing, {@code * linked_instance}, V22). * *

Created by {@code POST /api/v1/account-link/register}, authenticated with the admin's diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/LinkedInstanceAuthenticationToken.java b/app/saas/src/main/java/stirling/software/saas/accountlink/LinkedInstanceAuthenticationToken.java index af883bd66a..e393a162db 100644 --- a/app/saas/src/main/java/stirling/software/saas/accountlink/LinkedInstanceAuthenticationToken.java +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/LinkedInstanceAuthenticationToken.java @@ -6,7 +6,7 @@ import org.springframework.security.authentication.AbstractAuthenticationToken; import org.springframework.security.core.authority.SimpleGrantedAuthority; /** - * Authentication for a linked self-hosted instance (combined-billing "Mode A"). + * Authentication for a linked self-hosted instance (combined billing). * *

Deliberately not a user: the principal is the instance ({@code instanceId}) bound to * a {@code teamId}, with the single authority {@code ROLE_LINKED_INSTANCE}. It carries no {@code diff --git a/app/saas/src/main/java/stirling/software/saas/accountlink/Origins.java b/app/saas/src/main/java/stirling/software/saas/accountlink/Origins.java new file mode 100644 index 0000000000..629e0f7fc1 --- /dev/null +++ b/app/saas/src/main/java/stirling/software/saas/accountlink/Origins.java @@ -0,0 +1,22 @@ +package stirling.software.saas.accountlink; + +/** + * Origin formatting shared by the connect handshake. + * + *

One place on purpose: the origin a request arrives on and the origin parsed out of a callback + * URL are compared with each other, so if either side stopped omitting the default port the + * comparison would start failing quietly. + */ +final class Origins { + + private Origins() {} + + /** {@code host} or {@code host:port}, dropping a port that is the scheme's default. */ + static String hostPort(String scheme, String host, int port) { + boolean isDefault = + port <= 0 + || ("http".equals(scheme) && port == 80) + || ("https".equals(scheme) && port == 443); + return isDefault ? host : host + ":" + port; + } +} diff --git a/app/saas/src/main/java/stirling/software/saas/config/SaasSchemaOwnership.java b/app/saas/src/main/java/stirling/software/saas/config/SaasSchemaOwnership.java index dffd1eccf1..7853c56856 100644 --- a/app/saas/src/main/java/stirling/software/saas/config/SaasSchemaOwnership.java +++ b/app/saas/src/main/java/stirling/software/saas/config/SaasSchemaOwnership.java @@ -41,6 +41,7 @@ public final class SaasSchemaOwnership { */ public static final Set MIGRATION_OWNED = Set.of( + "account_link_connect_request", "ai_create_sessions", "audit_events", "authorities", @@ -78,6 +79,7 @@ public final class SaasSchemaOwnership { */ public static final Set HIBERNATE_MANAGED = Set.of( + "account_link_connect_state", "account_link_device_credential", "account_link_metered_signature", "account_link_sync_state", diff --git a/app/saas/src/main/java/stirling/software/saas/payg/instance/InstanceUsageIngestService.java b/app/saas/src/main/java/stirling/software/saas/payg/instance/InstanceUsageIngestService.java index c672540870..67e9c581d7 100644 --- a/app/saas/src/main/java/stirling/software/saas/payg/instance/InstanceUsageIngestService.java +++ b/app/saas/src/main/java/stirling/software/saas/payg/instance/InstanceUsageIngestService.java @@ -18,12 +18,12 @@ import stirling.software.saas.payg.model.ProcessType; import stirling.software.saas.payg.repository.PaygInstanceUsageRepository; /** - * Ingests a linked instance's daily usage sync (combined-billing "Mode A"). The instance reports a - * monotonic cumulative unit total per {@link BillingCategory}; we bill only the delta since the - * last sync via {@link JobChargeService#chargeStandalone} (reusing the in-cloud free-grant split, - * ledger DEBIT, Stripe meter and idempotency). Idempotent (a resend → delta 0 → no charge) and - * tamper-evident (a backwards total is refused; a monotonic {@code syncSeq} dedups replays). The - * cap is enforced at the instance gate, not here. Gated behind {@code account-link.enabled}. + * Ingests a linked instance's daily usage sync (combined billing). The instance reports a monotonic + * cumulative unit total per {@link BillingCategory}; we bill only the delta since the last sync via + * {@link JobChargeService#chargeStandalone} (reusing the in-cloud free-grant split, ledger DEBIT, + * Stripe meter and idempotency). Idempotent (a resend → delta 0 → no charge) and tamper-evident (a + * backwards total is refused; a monotonic {@code syncSeq} dedups replays). The cap is enforced at + * the instance gate, not here. Gated behind {@code account-link.enabled}. */ @Slf4j @Service diff --git a/app/saas/src/main/java/stirling/software/saas/payg/instance/PaygInstanceUsage.java b/app/saas/src/main/java/stirling/software/saas/payg/instance/PaygInstanceUsage.java index 45a6435746..345cf46b75 100644 --- a/app/saas/src/main/java/stirling/software/saas/payg/instance/PaygInstanceUsage.java +++ b/app/saas/src/main/java/stirling/software/saas/payg/instance/PaygInstanceUsage.java @@ -19,9 +19,9 @@ import lombok.Setter; /** * Last-seen cumulative usage a linked self-hosted instance has reported for one {@code (team, - * billing period, category)} (combined-billing "Mode A"). The instance reports monotonic cumulative - * unit totals on its daily sync; SaaS bills {@code reportedCumulative - lastCumulativeUnits} via - * the standard charge path and advances this row. {@code lastSyncSeq} dedups replays. + * billing period, category)} (combined billing). The instance reports monotonic cumulative unit + * totals on its daily sync; SaaS bills {@code reportedCumulative - lastCumulativeUnits} via the + * standard charge path and advances this row. {@code lastSyncSeq} dedups replays. */ @Entity @Table( diff --git a/app/saas/src/main/java/stirling/software/saas/security/SupabaseSecurityConfig.java b/app/saas/src/main/java/stirling/software/saas/security/SupabaseSecurityConfig.java index 36f3a4c06e..ea1c9bca9b 100644 --- a/app/saas/src/main/java/stirling/software/saas/security/SupabaseSecurityConfig.java +++ b/app/saas/src/main/java/stirling/software/saas/security/SupabaseSecurityConfig.java @@ -16,6 +16,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Profile; import org.springframework.core.annotation.Order; +import org.springframework.core.env.Environment; import org.springframework.http.HttpMethod; import org.springframework.security.authentication.AbstractAuthenticationToken; import org.springframework.security.config.Customizer; @@ -71,6 +72,7 @@ public class SupabaseSecurityConfig { private final SaasTeamService saasTeamService; private final ApplicationProperties applicationProperties; private final ApiKeyAuthenticationService apiKeyAuthenticationService; + private final Environment environment; @Value("${app.supabase.issuer:}") private String issuer; @@ -105,6 +107,17 @@ public class SupabaseSecurityConfig { .permitAll() .requestMatchers("/actuator/health", "/api/v1/config/**") .permitAll() + // Account-link connect handshake: an instance calls these + // before it holds any credential, so there is nothing to + // authenticate with yet. Neither grants anything on its + // own — /request records an intent a human must approve, + // and /claim requires a secret only the instance that + // created the request has ever held. + .requestMatchers( + HttpMethod.POST, + "/api/v1/account-link/connect/request", + "/api/v1/account-link/connect/claim") + .permitAll() .requestMatchers( req -> RequestUriUtils.isStaticResource( @@ -144,7 +157,7 @@ public class SupabaseSecurityConfig { SupabaseSecurityConfig ::toAuthentication))); - // Device-credential auth for linked self-hosted instances (combined-billing Mode A). + // Device-credential auth for linked self-hosted instances (combined billing). // The filter bean exists only when stirling.billing.account-link.enabled=true; when off it // is absent here, so the instance surface cannot authenticate at all until release. DeviceCredentialAuthenticationFilter deviceFilter = @@ -268,6 +281,28 @@ public class SupabaseSecurityConfig { } } + /** + * Loopback on any port, as Spring origin patterns. Only added outside production; see {@link + * #corsConfigurationSource()}. + */ + private static final List LOOPBACK_ANY_PORT = + List.of("http://localhost:[*]", "http://127.0.0.1:[*]"); + + /** + * Profiles that mean "a developer's machine or a preview environment", never the production + * deployment. Production runs the bare {@code saas} profile. + */ + private static final List NON_PRODUCTION_PROFILES = List.of("dev", "staging", "local"); + + private boolean isNonProduction() { + for (String profile : environment.getActiveProfiles()) { + if (NON_PRODUCTION_PROFILES.contains(profile)) { + return true; + } + } + return false; + } + @Bean CorsConfigurationSource corsConfigurationSource() { CorsConfiguration cfg = new CorsConfiguration(); @@ -297,7 +332,23 @@ public class SupabaseSecurityConfig { origins.add(desktopOrigin); } } - if (origins.stream().anyMatch(o -> o.contains("*"))) { + // Outside production, allow loopback on ANY port. Several dev servers run side by side + // (editor, saas web app, one per flavour under test) and their ports move, so pinning a + // list means every new local environment shows up as an opaque CORS failure. Unlike a + // wildcard subdomain, a wildcard port on loopback cannot be taken over: nothing but this + // machine can answer on it, so there is no lapsed-DNS or abandoned-vhost risk. Absent in + // production, where the profile check below is false. + if (!operatorOverride && isNonProduction()) { + origins.addAll(LOOPBACK_ANY_PORT); + log.info( + "Non-production profile active: allowing loopback CORS origins on any port {}", + LOOPBACK_ANY_PORT); + } + // Loopback port wildcards are exempt: the warning below is about hostname takeover, which + // does not apply to an origin only this machine can serve. + if (origins.stream() + .filter(o -> !LOOPBACK_ANY_PORT.contains(o)) + .anyMatch(o -> o.contains("*"))) { log.warn( "CORS origins contain a wildcard paired with allowCredentials=true: {}." + " Wildcard subdomains can be taken over by an attacker (lapsed DNS," diff --git a/app/saas/src/main/java/stirling/software/saas/service/SaasTeamService.java b/app/saas/src/main/java/stirling/software/saas/service/SaasTeamService.java index 0038cca42a..d81e0d9ee7 100644 --- a/app/saas/src/main/java/stirling/software/saas/service/SaasTeamService.java +++ b/app/saas/src/main/java/stirling/software/saas/service/SaasTeamService.java @@ -519,8 +519,8 @@ public class SaasTeamService { * membership and its wallet) rather than deleting it, so a plain team is never orphaned. The * only real hazard is a team the user is the last leader of that still carries live * billing: an active paid/PAYG subscription, or a non-revoked linked self-hosted instance - * ("Mode A"). Those block the join until the plan is cancelled / leadership transferred / - * instances revoked. An unpaid, unlinked team (personal or shared) no longer blocks. + * (combined billing). Those block the join until the plan is cancelled / leadership transferred + * / instances revoked. An unpaid, unlinked team (personal or shared) no longer blocks. * *

The home team and the team being joined are excluded: neither is left by the join (home is * parked, the joined team is kept), so their live billing cannot be stranded. diff --git a/app/saas/src/test/java/stirling/software/saas/accountlink/AccountLinkControllerTest.java b/app/saas/src/test/java/stirling/software/saas/accountlink/AccountLinkControllerTest.java index e12888edb3..b9c6f22305 100644 --- a/app/saas/src/test/java/stirling/software/saas/accountlink/AccountLinkControllerTest.java +++ b/app/saas/src/test/java/stirling/software/saas/accountlink/AccountLinkControllerTest.java @@ -1,6 +1,7 @@ package stirling.software.saas.accountlink; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @@ -23,8 +24,7 @@ import stirling.software.proprietary.model.TeamMembership; import stirling.software.proprietary.security.database.repository.UserRepository; import stirling.software.proprietary.security.model.User; import stirling.software.proprietary.security.repository.TeamMembershipRepository; -import stirling.software.saas.accountlink.AccountLinkController.RegisterRequest; -import stirling.software.saas.accountlink.AccountLinkController.RegisterResponse; +import stirling.software.saas.accountlink.AccountLinkController.InstanceRow; import stirling.software.saas.util.AuthenticationUtils; /** @@ -44,20 +44,27 @@ class AccountLinkControllerTest { @BeforeEach void setUp() { - controller = new AccountLinkController(service, memberRepo, userRepository); + // Real resolver over the mocked repositories: the leader ladder moved into + // LeaderTeamResolver, and these tests are still asserting that ladder's behaviour + // through the controller. + controller = + new AccountLinkController( + service, new LeaderTeamResolver(memberRepo, userRepository)); auth = new AnonymousAuthenticationToken( "k", "anonymousUser", List.of(new SimpleGrantedAuthority("ROLE_USER"))); } + // The leader ladder used to be asserted through POST /register, which has been removed along + // with the JWT relay. It is exercised through /instances instead: same resolver, same rungs. + @Test - void register_unauthenticated_returns401() { + void list_unauthenticated_returns401() { try (var mocked = org.mockito.Mockito.mockStatic(AuthenticationUtils.class)) { mocked.when(() -> AuthenticationUtils.getCurrentUser(auth, userRepository)) .thenThrow(new SecurityException("not authenticated")); - ResponseEntity resp = - controller.register(new RegisterRequest("host"), auth); + ResponseEntity> resp = controller.list(auth); assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); verifyNoInteractions(service); @@ -65,14 +72,14 @@ class AccountLinkControllerTest { } @Test - void register_noMembership_returns403() { + void list_noMembership_returns403() { User user = mockUser(42L); try (var mocked = org.mockito.Mockito.mockStatic(AuthenticationUtils.class)) { mocked.when(() -> AuthenticationUtils.getCurrentUser(auth, userRepository)) .thenReturn(user); when(memberRepo.findPrimaryMembership(42L)).thenReturn(List.of()); - ResponseEntity resp = controller.register(null, auth); + ResponseEntity> resp = controller.list(auth); assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); verifyNoInteractions(service); @@ -80,7 +87,7 @@ class AccountLinkControllerTest { } @Test - void register_nonLeader_returns403() { + void list_nonLeader_returns403() { User user = mockUser(42L); TeamMembership member = membership(7L, TeamRole.MEMBER); try (var mocked = org.mockito.Mockito.mockStatic(AuthenticationUtils.class)) { @@ -88,7 +95,7 @@ class AccountLinkControllerTest { .thenReturn(user); when(memberRepo.findPrimaryMembership(42L)).thenReturn(List.of(member)); - ResponseEntity resp = controller.register(null, auth); + ResponseEntity> resp = controller.list(auth); assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN); verifyNoInteractions(service); @@ -96,27 +103,20 @@ class AccountLinkControllerTest { } @Test - void register_leader_mintsCredentialForCallerTeam() { + void list_leader_readsOnlyTheCallersTeam() { User user = mockUser(42L); TeamMembership leader = membership(7L, TeamRole.LEADER); - when(service.register(7L, 42L, "host")) - .thenReturn( - new AccountLinkService.RegisteredInstance(99L, "dev-x", "sec-x", "host")); + when(service.list(7L)).thenReturn(List.of()); try (var mocked = org.mockito.Mockito.mockStatic(AuthenticationUtils.class)) { mocked.when(() -> AuthenticationUtils.getCurrentUser(auth, userRepository)) .thenReturn(user); when(memberRepo.findPrimaryMembership(42L)).thenReturn(List.of(leader)); - ResponseEntity resp = - controller.register(new RegisterRequest("host"), auth); + ResponseEntity> resp = controller.list(auth); - assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.CREATED); - RegisterResponse body = resp.getBody(); - assertThat(body).isNotNull(); - // Team comes from the caller's membership and is surfaced in the response. - assertThat(body.teamId()).isEqualTo(7L); - assertThat(body.instanceId()).isEqualTo(99L); - assertThat(body.deviceSecret()).isEqualTo("sec-x"); + assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK); + // The team comes from the caller's membership, never from the request. + verify(service).list(7L); } } diff --git a/app/saas/src/test/java/stirling/software/saas/accountlink/ConnectControllerTest.java b/app/saas/src/test/java/stirling/software/saas/accountlink/ConnectControllerTest.java new file mode 100644 index 0000000000..d58c4c4098 --- /dev/null +++ b/app/saas/src/test/java/stirling/software/saas/accountlink/ConnectControllerTest.java @@ -0,0 +1,129 @@ +package stirling.software.saas.accountlink; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.when; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.springframework.mock.web.MockHttpServletRequest; + +import stirling.software.common.model.ApplicationProperties; +import stirling.software.saas.accountlink.ConnectController.CreateBody; +import stirling.software.saas.accountlink.ConnectController.CreateResponse; + +/** + * The authorize URL the instance is told to send its admin to. Everything else on this controller + * delegates; this is the only decision it makes on its own. + */ +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +class ConnectControllerTest { + + private static final CreateBody BODY = + new CreateBody("prod-1", "https://pdf.example.com/account-link/callback", "n", "s"); + + @Mock private ConnectRequestService service; + @Mock private LeaderTeamResolver leaderTeams; + @Mock private AccountLinkService accountLinkService; + + private ApplicationProperties applicationProperties; + private ConnectController controller; + + @BeforeEach + void setUp() { + applicationProperties = new ApplicationProperties(); + controller = + new ConnectController( + service, leaderTeams, accountLinkService, applicationProperties); + when(service.create(anyString(), anyString(), anyString(), anyString(), any())) + .thenReturn(ConnectRequestService.CreateResult.ok("req-1", 1800)); + } + + private String authorizeUrl(MockHttpServletRequest request) { + Object body = controller.request(BODY, request).getBody(); + assertThat(body).isInstanceOf(CreateResponse.class); + return ((CreateResponse) body).authorizeUrl(); + } + + private static MockHttpServletRequest request(String scheme, String host, int port) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setScheme(scheme); + request.setServerName(host); + request.setServerPort(port); + return request; + } + + @Test + void prefersTheConfiguredFrontendUrl() { + applicationProperties.getSystem().setFrontendUrl("https://app.example.com/app/"); + + // Trailing slash trimmed, base path kept, and the API's own origin ignored. + assertThat(authorizeUrl(request("https", "api.example.com", 443))) + .isEqualTo("https://app.example.com/app/link?request=req-1"); + } + + @Test + void fallsBackToTheOriginTheApiWasReachedOn() { + assertThat(authorizeUrl(request("https", "api.example.com", 443))) + .isEqualTo("https://api.example.com/link?request=req-1"); + } + + @Test + void keepsANonDefaultPortAndTheContextPath() { + MockHttpServletRequest request = request("http", "localhost", 8081); + request.setContextPath("/stirling"); + + assertThat(authorizeUrl(request)) + .isEqualTo("http://localhost:8081/stirling/link?request=req-1"); + } + + @Test + void honoursTheForwardedSchemeAndHost() { + MockHttpServletRequest request = request("http", "10.0.0.5", 8080); + request.addHeader("X-Forwarded-Proto", "https"); + request.addHeader("X-Forwarded-Host", "api.example.com"); + + assertThat(authorizeUrl(request)).isEqualTo("https://api.example.com/link?request=req-1"); + } + + @Test + void takesOnlyTheFirstForwardedHop() { + MockHttpServletRequest request = request("http", "10.0.0.5", 8080); + request.addHeader("X-Forwarded-Proto", "https, http"); + request.addHeader("X-Forwarded-Host", "api.example.com, evil.example.com"); + + assertThat(authorizeUrl(request)).isEqualTo("https://api.example.com/link?request=req-1"); + } + + @Test + void percentEncodesTheRequestId() { + when(service.create(anyString(), anyString(), anyString(), anyString(), any())) + .thenReturn(ConnectRequestService.CreateResult.ok("a b&c", 1800)); + + assertThat(authorizeUrl(request("https", "api.example.com", 443))) + .isEqualTo("https://api.example.com/link?request=a+b%26c"); + } + + @Test + void aBodylessRequestIsRejectedBeforeAnythingIsRecorded() { + assertThat(controller.request(null, request("https", "api.example.com", 443)).getBody()) + .isEqualTo(java.util.Map.of("error", "BAD_REQUEST")); + } + + @Test + void offeringNoCredentialTakesTheFirstLinkPath() { + authorizeUrl(request("https", "api.example.com", 443)); + + // createReauth is the credentialled path; a first link must not reach it. + org.mockito.Mockito.verify(service, org.mockito.Mockito.never()) + .createReauth(anyString(), anyString(), anyString(), anyString(), any(), isNull()); + } +} diff --git a/app/saas/src/test/java/stirling/software/saas/accountlink/ConnectRequestServiceTest.java b/app/saas/src/test/java/stirling/software/saas/accountlink/ConnectRequestServiceTest.java new file mode 100644 index 0000000000..f75c96becb --- /dev/null +++ b/app/saas/src/test/java/stirling/software/saas/accountlink/ConnectRequestServiceTest.java @@ -0,0 +1,358 @@ +package stirling.software.saas.accountlink; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import java.time.LocalDateTime; +import java.util.Optional; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; + +import stirling.software.saas.accountlink.ConnectRequestService.ClaimOutcome; +import stirling.software.saas.accountlink.ConnectRequestService.CreateRejection; + +/** + * Unit tests for the connect handshake's security properties, which are the reason this flow is + * safe rather than an open redirect: the callback is validated once and then read back from + * storage, the claim secret authenticates the collection, and one approval mints exactly one + * credential. + */ +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +class ConnectRequestServiceTest { + + private static final String CALLBACK = "https://pdf.example.com/account-link/callback"; + private static final String NONCE = "nonce-value"; + private static final String CLAIM_SECRET = "claim-secret-value"; + + @Mock private ConnectRequestRepository repo; + @Mock private AccountLinkService accountLinkService; + + private ConnectRequestService service; + + @BeforeEach + void setUp() { + service = new ConnectRequestService(repo, accountLinkService); + } + + @Test + void create_storesTheValidatedCallbackAndItsOrigin() { + ConnectRequestService.CreateResult result = + service.create("prod-1", CALLBACK, NONCE, CLAIM_SECRET, "10.0.0.1"); + + assertThat(result.isRejected()).isFalse(); + assertThat(result.requestId()).isNotBlank(); + + ArgumentCaptor saved = ArgumentCaptor.forClass(ConnectRequest.class); + verify(repo).save(saved.capture()); + ConnectRequest row = saved.getValue(); + assertThat(row.getCallbackUrl()).isEqualTo(CALLBACK); + assertThat(row.getCallbackOrigin()).isEqualTo("https://pdf.example.com"); + assertThat(row.getStatus()).isEqualTo(ConnectRequest.Status.PENDING); + assertThat(row.getName()).isEqualTo("prod-1"); + // The claim secret is only ever stored as a hash. + assertThat(row.getClaimSecretHash()).isNotEqualTo(CLAIM_SECRET).hasSize(64); + } + + @Test + void create_keepsANonDefaultPortInTheOrigin() { + service.create( + null, "http://pdf.internal:8080/account-link/callback", NONCE, CLAIM_SECRET, null); + + ArgumentCaptor saved = ArgumentCaptor.forClass(ConnectRequest.class); + verify(repo).save(saved.capture()); + assertThat(saved.getValue().getCallbackOrigin()).isEqualTo("http://pdf.internal:8080"); + } + + @ParameterizedTest + @ValueSource( + strings = { + "/account-link/callback", // not absolute + "ftp://pdf.example.com/cb", // wrong scheme + "javascript:alert(1)", // not a hierarchical http(s) URL + "https://user:pw@pdf.example.com/cb", // credentials in the URL + "https://pdf.example.com/cb#already", // would collide with our fragment + "https:///cb" // no host + }) + void create_refusesCallbacksWeWouldNotWantToRedirectTo(String callback) { + ConnectRequestService.CreateResult result = + service.create(null, callback, NONCE, CLAIM_SECRET, null); + + assertThat(result.rejection()).isEqualTo(CreateRejection.BAD_CALLBACK); + verify(repo, never()).save(any()); + } + + @Test + void create_refusesAMissingNonce() { + assertThat(service.create(null, CALLBACK, " ", CLAIM_SECRET, null).rejection()) + .isEqualTo(CreateRejection.BAD_NONCE); + verify(repo, never()).save(any()); + } + + @Test + void create_namesTheSecretWhenTheSecretIsWhatIsMissing() { + assertThat(service.create(null, CALLBACK, NONCE, " ", null).rejection()) + .isEqualTo(CreateRejection.BAD_SECRET); + verify(repo, never()).save(any()); + } + + @Test + void create_isCappedPerSourceAddress() { + when(repo.countByRequesterIpAndCreatedAtAfter(anyString(), any())) + .thenReturn((long) ConnectRequestService.MAX_REQUESTS_PER_IP); + + ConnectRequestService.CreateResult result = + service.create(null, CALLBACK, NONCE, CLAIM_SECRET, "10.0.0.1"); + + assertThat(result.rejection()).isEqualTo(CreateRejection.RATE_LIMITED); + verify(repo, never()).save(any()); + } + + @Test + void lookup_flagsPlaintextTransportSoTheApproverCanSeeIt() { + ConnectRequest row = pending(); + row.setCallbackOrigin("http://pdf.internal:8080"); + when(repo.findByRequestId("req")).thenReturn(Optional.of(row)); + + assertThat(service.lookup("req")).get().extracting("insecureTransport").isEqualTo(true); + } + + @Test + void lookup_hidesAnExpiredHandshake() { + ConnectRequest row = pending(); + row.setExpiresAt(LocalDateTime.now().minusMinutes(1)); + when(repo.findByRequestId("req")).thenReturn(Optional.of(row)); + + assertThat(service.lookup("req")).isEmpty(); + } + + @Test + void approve_bindsTheTeamAndReturnsTheStoredCallback() { + ConnectRequest row = pending(); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + + ConnectRequestService.ApproveResult result = service.approve("req", 7L, 42L); + + assertThat(result.isRejected()).isFalse(); + // The destination comes from the row, never from the caller. + assertThat(result.target().callbackUrl()).isEqualTo(CALLBACK); + assertThat(result.target().nonce()).isEqualTo(NONCE); + assertThat(row.getStatus()).isEqualTo(ConnectRequest.Status.APPROVED); + assertThat(row.getTeamId()).isEqualTo(7L); + assertThat(row.getApprovedByUserId()).isEqualTo(42L); + // Approval on its own must not mint anything. + verifyNoInteractions(accountLinkService); + } + + @Test + void approve_isSingleUse() { + ConnectRequest row = pending(); + row.setStatus(ConnectRequest.Status.APPROVED); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + + assertThat(service.approve("req", 7L, 42L).isRejected()).isTrue(); + } + + @Test + void approve_refusesAnExpiredHandshake() { + ConnectRequest row = pending(); + row.setExpiresAt(LocalDateTime.now().minusSeconds(1)); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + + assertThat(service.approve("req", 7L, 42L).isRejected()).isTrue(); + } + + @Test + void createReauth_pinsTheTeamItWasToldByTheCredential() { + ConnectRequestService.CreateResult result = + service.createReauth(null, CALLBACK, NONCE, CLAIM_SECRET, null, 7L); + + assertThat(result.isRejected()).isFalse(); + ArgumentCaptor saved = ArgumentCaptor.forClass(ConnectRequest.class); + verify(repo).save(saved.capture()); + assertThat(saved.getValue().getMode()).isEqualTo(ConnectRequest.Mode.REAUTH); + assertThat(saved.getValue().getTeamId()).isEqualTo(7L); + } + + @Test + void createReauth_withoutAnAuthenticatedInstanceIsRefused() { + // The controller passes null when the offered device credential did not authenticate. + assertThat( + service.createReauth(null, CALLBACK, NONCE, CLAIM_SECRET, null, null) + .rejection()) + .isEqualTo(CreateRejection.NOT_LINKED); + verify(repo, never()).save(any()); + } + + @Test + void create_leavesTheTeamOpenForAFirstLink() { + service.create("n", CALLBACK, NONCE, CLAIM_SECRET, null); + + ArgumentCaptor saved = ArgumentCaptor.forClass(ConnectRequest.class); + verify(repo).save(saved.capture()); + assertThat(saved.getValue().getMode()).isEqualTo(ConnectRequest.Mode.LINK); + // Approval is what decides the team on a first link. + assertThat(saved.getValue().getTeamId()).isNull(); + } + + @Test + void approve_refusesAnApproverFromADifferentTeam() { + ConnectRequest row = reauthPinnedTo(7L); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + + ConnectRequestService.ApproveResult result = service.approve("req", 99L, 42L); + + // This is the "signed in to the wrong account" case, and it must not silently rebind. + assertThat(result.rejection()).isEqualTo(ConnectRequestService.ApproveRejection.WRONG_TEAM); + assertThat(row.getStatus()).isEqualTo(ConnectRequest.Status.PENDING); + assertThat(row.getTeamId()).isEqualTo(7L); + } + + @Test + void approve_acceptsTheTeamTheServerAlreadyBelongsTo() { + ConnectRequest row = reauthPinnedTo(7L); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + + assertThat(service.approve("req", 7L, 42L).isRejected()).isFalse(); + assertThat(row.getStatus()).isEqualTo(ConnectRequest.Status.APPROVED); + } + + @Test + void claim_onAReauthConfirmsWithoutMintingASecondCredential() { + ConnectRequest row = reauthPinnedTo(7L); + row.setStatus(ConnectRequest.Status.APPROVED); + row.setApprovedByUserId(42L); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + + ConnectRequestService.ClaimResult result = service.claim("req", CLAIM_SECRET); + + assertThat(result.outcome()).isEqualTo(ClaimOutcome.CONFIRMED); + assertThat(result.deviceId()).isNull(); + assertThat(result.deviceSecret()).isNull(); + assertThat(result.teamId()).isEqualTo(7L); + assertThat(row.getStatus()).isEqualTo(ConnectRequest.Status.CONSUMED); + // A second credential would orphan the one the instance already holds. + verifyNoInteractions(accountLinkService); + } + + @Test + void claim_mintsOnceForAnApprovedHandshake() { + ConnectRequest row = approved(); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + when(accountLinkService.register(anyLong(), anyLong(), any())) + .thenReturn( + new AccountLinkService.RegisteredInstance(9L, "dev-id", "dev-secret", "n")); + + ConnectRequestService.ClaimResult result = service.claim("req", CLAIM_SECRET); + + assertThat(result.outcome()).isEqualTo(ClaimOutcome.GRANTED); + assertThat(result.deviceId()).isEqualTo("dev-id"); + assertThat(result.deviceSecret()).isEqualTo("dev-secret"); + assertThat(result.teamId()).isEqualTo(7L); + assertThat(row.getStatus()).isEqualTo(ConnectRequest.Status.CONSUMED); + verify(accountLinkService).register(7L, 42L, "n"); + } + + @Test + void claim_refusesASecondCollection() { + ConnectRequest row = approved(); + row.setStatus(ConnectRequest.Status.CONSUMED); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + + assertThat(service.claim("req", CLAIM_SECRET).outcome()).isEqualTo(ClaimOutcome.REJECTED); + verifyNoInteractions(accountLinkService); + } + + @Test + void claim_withTheWrongSecretMintsNothing() { + ConnectRequest row = approved(); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + + assertThat(service.claim("req", "not-the-secret").outcome()) + .isEqualTo(ClaimOutcome.REJECTED); + assertThat(row.getStatus()).isEqualTo(ConnectRequest.Status.APPROVED); + verifyNoInteractions(accountLinkService); + } + + @Test + void claim_beforeApprovalTellsTheInstanceToWait() { + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(pending())); + + assertThat(service.claim("req", CLAIM_SECRET).outcome()).isEqualTo(ClaimOutcome.PENDING); + verifyNoInteractions(accountLinkService); + } + + @Test + void claim_afterDenialIsTerminal() { + ConnectRequest row = pending(); + row.setStatus(ConnectRequest.Status.DENIED); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + + assertThat(service.claim("req", CLAIM_SECRET).outcome()).isEqualTo(ClaimOutcome.REJECTED); + verifyNoInteractions(accountLinkService); + } + + @Test + void claim_onAnExpiredHandshakeMintsNothing() { + ConnectRequest row = approved(); + row.setExpiresAt(LocalDateTime.now().minusSeconds(1)); + when(repo.findByRequestIdForUpdate("req")).thenReturn(Optional.of(row)); + + assertThat(service.claim("req", CLAIM_SECRET).outcome()).isEqualTo(ClaimOutcome.REJECTED); + verifyNoInteractions(accountLinkService); + } + + @Test + void claim_forAnUnknownIdLooksTheSameAsABadSecret() { + when(repo.findByRequestIdForUpdate("nope")).thenReturn(Optional.empty()); + + assertThat(service.claim("nope", CLAIM_SECRET).outcome()).isEqualTo(ClaimOutcome.REJECTED); + } + + // --------------------------------------------------------------------------------------- + + private static ConnectRequest pending() { + ConnectRequest row = new ConnectRequest(); + row.setRequestId("req"); + row.setName("n"); + row.setCallbackUrl(CALLBACK); + row.setCallbackOrigin("https://pdf.example.com"); + row.setNonce(NONCE); + row.setClaimSecretHash(AccountLinkService.sha256Hex(CLAIM_SECRET)); + row.setStatus(ConnectRequest.Status.PENDING); + row.setExpiresAt(LocalDateTime.now().plusMinutes(10)); + return row; + } + + /** A re-authentication whose team came from the instance's credential, not from a browser. */ + private static ConnectRequest reauthPinnedTo(Long teamId) { + ConnectRequest row = pending(); + row.setMode(ConnectRequest.Mode.REAUTH); + row.setTeamId(teamId); + return row; + } + + private static ConnectRequest approved() { + ConnectRequest row = pending(); + row.setStatus(ConnectRequest.Status.APPROVED); + row.setTeamId(7L); + row.setApprovedByUserId(42L); + row.setApprovedAt(LocalDateTime.now()); + return row; + } +} diff --git a/app/saas/src/test/java/stirling/software/saas/security/SupabaseSecurityConfigMoreTest.java b/app/saas/src/test/java/stirling/software/saas/security/SupabaseSecurityConfigMoreTest.java index da719e7ba5..4727d78463 100644 --- a/app/saas/src/test/java/stirling/software/saas/security/SupabaseSecurityConfigMoreTest.java +++ b/app/saas/src/test/java/stirling/software/saas/security/SupabaseSecurityConfigMoreTest.java @@ -14,6 +14,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.core.env.Environment; +import org.springframework.mock.env.MockEnvironment; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.oauth2.jwt.Jwt; import org.springframework.security.oauth2.jwt.JwtDecoder; @@ -48,13 +50,19 @@ class SupabaseSecurityConfigMoreTest { apiKeyAuthenticationService; private SupabaseSecurityConfig config(ApplicationProperties props) { + return config(props, new MockEnvironment()); + } + + /** Loopback CORS origins are only added outside production, so the environment decides. */ + private SupabaseSecurityConfig config(ApplicationProperties props, Environment environment) { return new SupabaseSecurityConfig( userService, teamService, supabaseUserService, saasTeamService, props, - apiKeyAuthenticationService); + apiKeyAuthenticationService, + environment); } @Nested @@ -204,6 +212,50 @@ class SupabaseSecurityConfigMoreTest { .hasSize(1); } + @Test + @DisplayName("production does not allow loopback on arbitrary ports") + void productionHasNoLoopbackWildcard() { + CorsConfiguration cfg = + cors(config(new ApplicationProperties()).corsConfigurationSource()); + + assertThat(cfg.getAllowedOriginPatterns()) + .doesNotContain("http://localhost:[*]", "http://127.0.0.1:[*]"); + } + + @Test + @DisplayName("non-production allows loopback on any port so dev servers can move") + void devAllowsAnyLoopbackPort() { + // Several dev servers run side by side and their ports change; pinning a list turns + // every new local environment into an opaque CORS failure. + MockEnvironment dev = new MockEnvironment(); + dev.setActiveProfiles("saas", "dev"); + + CorsConfiguration cfg = + cors(config(new ApplicationProperties(), dev).corsConfigurationSource()); + + assertThat(cfg.getAllowedOriginPatterns()) + .contains("http://localhost:[*]", "http://127.0.0.1:[*]") + // Still credentialed, which is the reason the pattern form matters. + .contains("https://stirling.com"); + assertThat(cfg.getAllowCredentials()).isTrue(); + } + + @Test + @DisplayName("an operator origin list is respected verbatim even in dev") + void operatorOverrideSuppressesLoopbackWildcard() { + ApplicationProperties props = new ApplicationProperties(); + props.getSystem().setCorsAllowedOrigins(List.of("https://custom.example.com")); + MockEnvironment dev = new MockEnvironment(); + dev.setActiveProfiles("saas", "dev"); + + CorsConfiguration cfg = cors(config(props, dev).corsConfigurationSource()); + + // An operator who set the list meant it; we do not widen it behind their back. + assertThat(cfg.getAllowedOriginPatterns()) + .contains("https://custom.example.com") + .doesNotContain("http://localhost:[*]"); + } + @Test @DisplayName("operator override replaces the default origin list") void operatorOverrideUsed() { diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml index 1b81a551f1..4af1be3359 100644 --- a/frontend/editor/public/locales/en-US/translation.toml +++ b/frontend/editor/public/locales/en-US/translation.toml @@ -3240,7 +3240,7 @@ enterEmailConfirm = "To confirm deletion, please type your email address ({{emai guestDescription = "You are signed in as a guest. Consider upgrading your account above." label = "Overview" manageAccountPreferences = "Manage your account preferences" -signedInAs = "Signed in as" +signedInAs = "Account" title = "Account Settings" [config.account.profilePicture] @@ -3351,6 +3351,39 @@ integration = "Integration Configuration" security = "Security Configuration" system = "System Configuration" +[connect] +loading = "Checking this request." +redirecting = "Returning you to your server." + +[connect.confirm] +acknowledge = "I recognise this address and want to connect it to my team" +approve = "Connect server" +deny = "Decline" +lead = "A Stirling server is asking to connect to your team. Check the address below is yours before you approve." +originLabel = "Address" +signedInAs = "Signed in as" +switchAccount = "Use a different account" +title = "Connect this server?" +unknownAccount = "an unknown account" + +[connect.confirm.insecure] +body = "This address does not use HTTPS, so your sign-in will be sent over an unencrypted connection. Only approve it on a network you trust." +label = "Not an encrypted address" + +[connect.declined] +body = "Nothing was connected. You can close this page." +title = "Request declined" + +[connect.error] +failed = "That did not go through. Only a team owner can connect a server." + +[connect.meta] +title = "Connect a server" + +[connect.notFound] +body = "This connection request is not valid. It may have expired, or already been used. Start another one from your server." +title = "Request not valid" + [convert] autoRotate = "Auto Rotate" autoRotateDescription = "Automatically rotate images to better fit the PDF page" @@ -6487,6 +6520,34 @@ after = "to enable account linking against the hosted Stirling account. In dev y before = "Set" title = "SaaS login not configured" +[portal.accountLink.connect.callback] +continue = "Continue" +linkedNotSignedIn = "You are not signed in to Stirling in this browser, so usage and billing will ask you to sign in." +modalTitle = "Connecting this server" +retry = "Try again" +signedInAnyway = "You are signed in to Stirling, so billing and usage will load. Only the server link is incomplete." +working = "Finishing the connection." + +[portal.accountLink.connect.callback.expired] +body = "Connection requests are short lived. Start another one." +title = "Request expired" + +[portal.accountLink.connect.callback.linked] +body = "This server is connected to your Stirling account." +title = "Server connected" + +[portal.accountLink.connect.callback.malformed] +body = "This page was opened without a valid connection response. Start the connection from settings." +title = "Could not read the response" + +[portal.accountLink.connect.callback.rejected] +body = "This request was declined or has already been used. Start another one if that was not intended." +title = "Connection not completed" + +[portal.accountLink.connect.callback.unfinished] +body = "Stirling did not confirm the connection. This is usually temporary." +title = "Not finished yet" + [portal.accountLink.gate] action = "Link account" description = "Link this org's Stirling account to use billable features." @@ -6520,17 +6581,24 @@ minutesAgo_other = "{{count}}m ago" never = "never" [portal.accountLink.modal] -linkSubtitle = "Sign in to the account this server should bill against." -linkTitle = "Link your Stirling account" -reauthSubtitle = "Your session expired — sign back in to your Stirling account. Your instance stays linked." +cancel = "Cancel" +continueLink = "Continue to Stirling" +continueReauth = "Sign in again" +linkSubtitle = "Connect this server to the Stirling account it should bill against." +linkTitle = "Connect your Stirling account" +noAuthorizeUrl = "Stirling did not return somewhere to continue. Try again in a moment." +reauthSubtitle = "Your Stirling session expired. Sign in again to keep seeing usage and billing. This server stays connected either way." reauthTitle = "Sign in again" -simulateSignIn = "Simulate sign-in (dev)" +startFailed = "Could not reach Stirling to start the connection. Check this server's outbound network access, then try again." +step1 = "We send you to stirling.com to sign in. Any sign-in method works there, including Google and single sign-on." +step2 = "You check this server's address and approve it. A team owner has to do this the first time." +step3 = "Stirling brings you straight back here and finishes up." [portal.accountLink.modal.loginNotConfigured] -after = "to enable in-app linking against the hosted Stirling account." +after = "so this server can finish the connection when you come back." and = "and" before = "Set" -title = "SaaS login not configured" +title = "Stirling connection not configured" [portal.accountLink.panel] instancesSub = "Every self-hosted instance registered to this org. Revoke a credential to immediately cut off its unattended access." @@ -11797,6 +11865,10 @@ title = "Watermark Text" image = "Image" text = "Text" +[workbench.sessionRestore] +none = "Your previous files are no longer stored on this device." +partial = "Restored {{restored}} of {{total}} files. The rest are no longer stored on this device." + [workbenchBar] activeFiles = "Active Files" annotations = "Annotations" diff --git a/frontend/editor/src/core/components/AppProviders.tsx b/frontend/editor/src/core/components/AppProviders.tsx index 01b2e42a82..ed1194eb4b 100644 --- a/frontend/editor/src/core/components/AppProviders.tsx +++ b/frontend/editor/src/core/components/AppProviders.tsx @@ -39,6 +39,7 @@ import { RedactionProvider } from "@app/contexts/RedactionContext"; import { FormFillProvider } from "@app/tools/formFill/FormFillContext"; import { FolderFileContextProvider } from "@app/contexts/FolderFileContext"; import { FolderProvider } from "@app/contexts/FolderContext"; +import { WorkbenchSessionPersistence } from "@app/components/session/WorkbenchSessionPersistence"; // Component to initialize scarf tracking (must be inside AppConfigProvider) function ScarfTrackingInitializer() { @@ -163,6 +164,7 @@ export function AppProviders({ + {children} diff --git a/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.test.tsx b/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.test.tsx new file mode 100644 index 0000000000..b05928a93e --- /dev/null +++ b/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.test.tsx @@ -0,0 +1,475 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, waitFor, act } from "@testing-library/react"; + +const mocks = vi.hoisted(() => ({ + getLeafStirlingFileStubs: vi.fn(), + alert: vi.fn(), + setActiveFileId: vi.fn(), + restoreWorkbench: vi.fn(), + workbench: "viewer" as string, + authUser: null as { id: string } | null, + authLoading: false, + pathname: "/editor", + activeFileId: null as string | null, +})); + +vi.mock("@app/services/fileStorage", () => ({ + fileStorage: { getLeafStirlingFileStubs: mocks.getLeafStirlingFileStubs }, +})); +vi.mock("@app/components/toast", () => ({ alert: mocks.alert })); +vi.mock("@app/contexts/NavigationContext", () => ({ + useNavigationState: () => ({ workbench: mocks.workbench }), + useNavigationActions: () => ({ + actions: { restoreWorkbench: mocks.restoreWorkbench }, + }), +})); +vi.mock("react-router-dom", () => ({ + useLocation: () => ({ pathname: mocks.pathname }), +})); +vi.mock("@app/auth/UseSession", () => ({ + useAuth: () => ({ user: mocks.authUser, loading: mocks.authLoading }), +})); +vi.mock("@app/contexts/ViewerContext", () => ({ + useViewer: () => ({ + activeFileId: mocks.activeFileId, + setActiveFileId: mocks.setActiveFileId, + }), +})); + +import { WorkbenchSessionPersistence } from "@app/components/session/WorkbenchSessionPersistence"; +import { fingerprintOwner } from "@app/services/workbenchSession"; +import { + FileStoreContext, + FileActionsContext, +} from "@app/contexts/file/contexts"; +import type { StirlingFileStub } from "@app/types/fileContext"; + +const SESSION_KEY = "stirling.workbench.session"; + +function stub( + id: string, + originalFileId: string, + versionNumber = 1, +): StirlingFileStub { + return { id, originalFileId, versionNumber, name: `${id}.pdf` } as never; +} + +// A minimal stand-in for the FileContext store: mutable state plus subscribers. +function makeStore(open: StirlingFileStub[] = [], selected: string[] = []) { + const listeners = new Set<() => void>(); + const state = { + files: { + ids: open.map((s) => s.id), + byId: Object.fromEntries(open.map((s) => [s.id, s])), + }, + ui: { selectedFileIds: selected }, + }; + return { + state, + getState: () => state as never, + subscribe: (listener: () => void) => { + listeners.add(listener); + return () => listeners.delete(listener); + }, + // reopenView waits on this to know the restored bytes have landed. + selectors: { + getFiles: (ids: string[]) => ids.map((id) => ({ id })), + } as never, + notify: () => listeners.forEach((listener) => listener()), + }; +} + +const actions = { + addStirlingFileStubs: vi.fn().mockResolvedValue([]), + setSelectedFiles: vi.fn(), +}; + +function mount(store: ReturnType) { + return render( + + + + + , + ); +} + +beforeEach(() => { + // The shared setup stubs crypto.subtle.digest to one constant for every input, so every account + // would fingerprint alike - and ownership is exactly what these tests are about. + vi.spyOn(globalThis.crypto.subtle, "digest").mockImplementation( + async (_algorithm: AlgorithmIdentifier, data: BufferSource) => { + const bytes = ArrayBuffer.isView(data) + ? new Uint8Array(data.buffer, data.byteOffset, data.byteLength) + : new Uint8Array(data); + let hash = 0x811c9dc5; + for (const byte of bytes) { + hash = Math.imul(hash ^ byte, 0x01000193) >>> 0; + } + const out = new Uint8Array(32); + for (let i = 0; i < out.length; i++) { + hash = Math.imul(hash ^ i, 0x01000193) >>> 0; + out[i] = hash & 0xff; + } + return out.buffer; + }, + ); + sessionStorage.clear(); + vi.clearAllMocks(); + actions.addStirlingFileStubs.mockResolvedValue([]); + mocks.getLeafStirlingFileStubs.mockResolvedValue([]); + mocks.workbench = "viewer"; + mocks.authUser = null; + mocks.authLoading = false; + mocks.pathname = "/editor"; + mocks.activeFileId = null; +}); +afterEach(() => vi.useRealTimers()); + +describe("restore", () => { + it("refills an empty workbench with each file's current leaf, in saved order", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a", "root-b"], + selectedFileIds: ["root-b"], + }), + ); + // root-a forked while the user was away: v3 must win over the stale v1 leaf. + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("a-v1", "root-a", 1), + stub("a-v3", "root-a", 3), + stub("root-b", "root-b", 1), + ]); + + mount(makeStore()); + + await waitFor(() => + expect(actions.addStirlingFileStubs).toHaveBeenCalled(), + ); + const restored = actions.addStirlingFileStubs.mock.calls[0][0]; + expect(restored.map((s: StirlingFileStub) => s.id)).toEqual([ + "a-v3", + "root-b", + ]); + expect(actions.setSelectedFiles).toHaveBeenCalledWith(["root-b"]); + expect(mocks.alert).not.toHaveBeenCalled(); + }); + + it("does not touch a workbench that already holds files", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ v: 2, fileIds: ["root-a"], selectedFileIds: [] }), + ); + mount(makeStore([stub("already-open", "already-open")])); + + await act(async () => {}); + expect(actions.addStirlingFileStubs).not.toHaveBeenCalled(); + }); + + it("restores what still exists and says how much is gone", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a", "gone"], + selectedFileIds: [], + }), + ); + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + mount(makeStore()); + + await waitFor(() => expect(mocks.alert).toHaveBeenCalled()); + expect(actions.addStirlingFileStubs.mock.calls[0][0]).toHaveLength(1); + expect(mocks.alert.mock.calls[0][0].alertType).toBe("warning"); + }); + + it("does not say 'the rest' when nothing at all could be restored", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["gone-1", "gone-2"], + selectedFileIds: [], + }), + ); + mocks.getLeafStirlingFileStubs.mockResolvedValue([]); + + mount(makeStore()); + + await waitFor(() => expect(mocks.alert).toHaveBeenCalled()); + expect(mocks.alert.mock.calls[0][0].title).toBe( + "workbench.sessionRestore.none", + ); + }); + + it("does nothing when no session was recorded", async () => { + mount(makeStore()); + await act(async () => {}); + expect(actions.addStirlingFileStubs).not.toHaveBeenCalled(); + expect(mocks.getLeafStirlingFileStubs).not.toHaveBeenCalled(); + }); + + it("reopens the document the user was viewing, at its current version", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a"], + selectedFileIds: ["root-a"], + workbench: "fileEditor", + activeFileId: "root-a", + }), + ); + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("a-v2", "root-a", 2), + ]); + + mount(makeStore()); + + await waitFor(() => expect(mocks.setActiveFileId).toHaveBeenCalled()); + expect(mocks.setActiveFileId).toHaveBeenCalledWith("a-v2"); + expect(mocks.restoreWorkbench).toHaveBeenCalledWith("fileEditor"); + }); + + it("leaves a URL-owned view to the return path", async () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a"], + selectedFileIds: [], + workbench: "myFiles", + }), + ); + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + mount(makeStore()); + + await waitFor(() => + expect(actions.addStirlingFileStubs).toHaveBeenCalled(), + ); + expect(mocks.restoreWorkbench).not.toHaveBeenCalled(); + }); +}); + +describe("whose workbench it is", () => { + // Records hold a fingerprint of the owner, never the account id. + const record = async (userId: string | null) => + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a"], + selectedFileIds: [], + userId: userId == null ? null : await fingerprintOwner(userId), + }), + ); + + it("does not open one user's workbench for the next person in the tab", async () => { + await record("user-a"); + mocks.authUser = { id: "user-b" }; + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + mount(makeStore()); + + await act(async () => {}); + expect(actions.addStirlingFileStubs).not.toHaveBeenCalled(); + // The record is theirs now - the previous person's files are gone from it, so they cannot + // resurface later in the session. + const taken = JSON.parse(sessionStorage.getItem(SESSION_KEY)!); + expect(taken.fileIds).toEqual([]); + expect(taken.userId).toBe(await fingerprintOwner("user-b")); + }); + + it("reopens it for the user who left it", async () => { + await record("user-a"); + mocks.authUser = { id: "user-a" }; + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + mount(makeStore()); + + await waitFor(() => + expect(actions.addStirlingFileStubs).toHaveBeenCalled(), + ); + }); + + it("waits for the session before deciding", async () => { + await record("user-a"); + mocks.authUser = null; + mocks.authLoading = true; + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + mount(makeStore()); + + await act(async () => {}); + // Neither restored nor discarded - who is signed in is not known yet. + expect(actions.addStirlingFileStubs).not.toHaveBeenCalled(); + expect(sessionStorage.getItem(SESSION_KEY)).not.toBeNull(); + }); +}); + +describe("a lost session that comes back", () => { + const rerenderWith = ( + view: ReturnType, + store: ReturnType, + ) => + view.rerender( + + + + + , + ); + + it("survives a blip on the identity check", async () => { + // A failed /auth/me - flaky wifi, a backend redeploy, a refreshSession() that did not land - + // briefly reads as nobody signed in. It must not be mistaken for signing out. + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 2, + fileIds: ["root-a"], + selectedFileIds: [], + userId: await fingerprintOwner("user-a"), + }), + ); + mocks.authUser = { id: "user-a" }; + const store = makeStore([stub("f1", "f1")]); + const view = mount(store); + await act(async () => {}); + + mocks.authUser = null; + rerenderWith(view, store); + await act(async () => {}); + + expect(sessionStorage.getItem(SESSION_KEY)).not.toBeNull(); + + // ...and once the identity is back, the workbench is still being recorded. + mocks.authUser = { id: "user-a" }; + rerenderWith(view, store); + // Let the fingerprint land: writes hold off while a known identity has none yet. + await act(async () => {}); + store.state.files.ids = ["f2" as never]; + store.state.files.byId = { f2: stub("f2", "root-b") } as never; + act(() => store.notify()); + view.unmount(); + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!).fileIds).toEqual([ + "root-b", + ]); + }); +}); + +describe("on the login screen", () => { + it("neither restores nor records - signing out must not rebuild the workbench there", async () => { + mocks.pathname = "/login"; + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ v: 2, fileIds: ["root-a"], selectedFileIds: [] }), + ); + mocks.getLeafStirlingFileStubs.mockResolvedValue([ + stub("root-a", "root-a"), + ]); + + const store = makeStore(); + const { unmount } = mount(store); + await act(async () => {}); + expect(actions.addStirlingFileStubs).not.toHaveBeenCalled(); + + // And the unmount flush must not write either. + store.state.files.ids = ["f1" as never]; + store.state.files.byId = { f1: stub("f1", "f1") } as never; + act(() => store.notify()); + unmount(); + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!).fileIds).toEqual([ + "root-a", + ]); + }); +}); + +describe("writer", () => { + it("mirrors the open files and selection as original ids, debounced", async () => { + vi.useFakeTimers(); + const store = makeStore(); + mount(store); + + store.state.files.ids = ["v2" as never]; + store.state.files.byId = { v2: stub("v2", "root-a", 2) } as never; + store.state.ui.selectedFileIds = ["v2"]; + act(() => store.notify()); + + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!)).toMatchObject({ + fileIds: ["root-a"], + selectedFileIds: ["root-a"], + workbench: "viewer", + }); + }); + + it("records the current view, so the return lands where the user left", async () => { + vi.useFakeTimers(); + mocks.workbench = "fileEditor"; + const store = makeStore([stub("f1", "f1")]); + mount(store); + + act(() => store.notify()); + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!).workbench).toBe( + "fileEditor", + ); + }); + + it("writes nothing until the restore has settled", () => { + vi.useFakeTimers(); + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ v: 2, fileIds: ["root-a"], selectedFileIds: [] }), + ); + // Restore is still awaiting storage, so this mount's empty state is not the truth. + mocks.getLeafStirlingFileStubs.mockReturnValue(new Promise(() => {})); + + const store = makeStore(); + const { unmount } = mount(store); + act(() => store.notify()); + unmount(); + + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!).fileIds).toEqual([ + "root-a", + ]); + }); + + it("flushes on unmount, so the state at the shell switch survives", () => { + vi.useFakeTimers(); + const store = makeStore(); + const { unmount } = mount(store); + + store.state.files.ids = ["f1" as never]; + store.state.files.byId = { f1: stub("f1", "f1") } as never; + act(() => store.notify()); + unmount(); + + expect(JSON.parse(sessionStorage.getItem(SESSION_KEY)!).fileIds).toEqual([ + "f1", + ]); + }); +}); diff --git a/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.tsx b/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.tsx new file mode 100644 index 0000000000..86bf72502c --- /dev/null +++ b/frontend/editor/src/core/components/session/WorkbenchSessionPersistence.tsx @@ -0,0 +1,301 @@ +// The editor/processor shell switch unmounts every editor provider, and a reload starts from nothing: +// this mirrors the workbench into sessionStorage and refills an empty one from that record on mount. +import { useCallback, useContext, useEffect, useRef, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { + FileStoreContext, + type FileStateStore, +} from "@app/contexts/file/contexts"; +import { useFileActions } from "@app/contexts/FileContext"; +import { + useNavigationActions, + useNavigationState, +} from "@app/contexts/NavigationContext"; +import { useViewer } from "@app/contexts/ViewerContext"; +import { useAuth } from "@app/auth/UseSession"; +import { useLocation } from "react-router-dom"; +import { isAuthRoute } from "@app/constants/routes"; +import { fileStorage } from "@app/services/fileStorage"; +import { alert } from "@app/components/toast"; +import { WORKBENCH_SESSION_RESTORE } from "@app/constants/featureFlags"; +import { + beginRestoredView, + clearWorkbenchSession, + fingerprintOwner, + resumeWorkbenchSession, + endRestoredView, + isSeedableView, + originalIdOf, + readWorkbenchSession, + writeWorkbenchSession, +} from "@app/services/workbenchSession"; +import type { WorkbenchType } from "@app/types/workbench"; +import type { StirlingFileStub } from "@app/types/fileContext"; +import type { FileId } from "@app/types/file"; + +const WRITE_DEBOUNCE_MS = 300; + +// Current leaf per original id; a forked chain resolves to the highest version. +function leafByOriginalId( + leaves: StirlingFileStub[], +): Map { + const map = new Map(); + for (const leaf of leaves) { + const key = originalIdOf(leaf); + const current = map.get(key); + if (!current || (leaf.versionNumber ?? 1) > (current.versionNumber ?? 1)) { + map.set(key, leaf); + } + } + return map; +} + +/** How long to wait for the NEXT file to hydrate before giving up on holding the view. Restarted on + * each arrival, so a slow device with large documents keeps the view as long as it makes progress. */ +const SETTLE_TIMEOUT_MS = 5000; + +/** Released a beat late, so effects reacting to the same commit still see the restore in progress. */ +const RELEASE_GRACE_MS = 250; + +/** + * Reopen the recorded view, then hold the restore guard until the files have hydrated. + * + * The view is written ONCE. Re-asserting it after hydration would also overwrite a view the user + * picked in the meantime; holding the guard is what keeps HomePage's defaults off it instead. + */ +function reopenView( + store: FileStateStore, + reopen: (view: WorkbenchType) => void, + { + view, + fileCount, + token, + }: { view: WorkbenchType; fileCount: number; token: number }, +): void { + reopen(view); + const loaded = () => + store.selectors.getFiles(store.getState().files.ids).length; + + const release = () => + setTimeout(() => endRestoredView(token), RELEASE_GRACE_MS); + if (loaded() >= fileCount) { + release(); + return; + } + + let timer: ReturnType; + const stop = () => { + clearTimeout(timer); + unsubscribe(); + release(); + }; + const waitForNext = () => { + clearTimeout(timer); + timer = setTimeout(stop, SETTLE_TIMEOUT_MS); + }; + + let seen = loaded(); + const unsubscribe = store.subscribe(() => { + const now = loaded(); + if (now >= fileCount) return stop(); + // Progress, not completion: give the remaining files a fresh window. + if (now > seen) { + seen = now; + waitForNext(); + } + }); + waitForNext(); +} + +export function WorkbenchSessionPersistence() { + const store = useContext(FileStoreContext); + const { actions } = useFileActions(); + const { workbench } = useNavigationState(); + const { actions: navigationActions } = useNavigationActions(); + const { activeFileId, setActiveFileId } = useViewer(); + const { user, loading: authLoading } = useAuth(); + // Login/signup mount the editor's providers too. Nothing there is the user's workbench, so this + // records nothing and restores nothing - otherwise signing out rebuilds it on the login screen. + const onAuthRoute = isAuthRoute(useLocation().pathname); + const userId = user?.id != null ? String(user.id) : null; + // Fingerprinted, never stored raw - see fingerprintOwner. Computed asynchronously, so writes + // hold off until it lands rather than stamping the record "nobody's" and then failing its own + // ownership check. + const [owner, setOwner] = useState(null); + useEffect(() => { + if (userId == null) { + setOwner(null); + return; + } + let cancelled = false; + void fingerprintOwner(userId).then((fingerprint) => { + if (!cancelled) setOwner(fingerprint); + }); + return () => { + cancelled = true; + }; + }, [userId]); + const { t } = useTranslation(); + // Captured before the writer below can overwrite it with the empty boot state. + const [saved] = useState(readWorkbenchSession); + const restoreStarted = useRef(false); + // Until the restore has run, this mount's empty state is not the truth to record. + const restoreSettled = useRef(false); + + // Published so a build's restore setting is legible without reading the bundle. + useEffect(() => { + document.documentElement.dataset.workbenchRestore = String( + WORKBENCH_SESSION_RESTORE, + ); + }, []); + + const write = useCallback(() => { + if (!store || !restoreSettled.current) return; + // A known identity whose fingerprint has not landed yet: wait, do not stamp it as nobody's. + if (userId != null && owner == null) return; + const state = store.getState(); + const toOriginal = (id: FileId): string | null => { + const stub = state.files.byId[id]; + return stub ? originalIdOf(stub) : null; + }; + const isPresent = (id: string | null): id is string => id !== null; + writeWorkbenchSession({ + fileIds: state.files.ids.map(toOriginal).filter(isPresent), + selectedFileIds: state.ui.selectedFileIds + .map(toOriginal) + .filter(isPresent), + workbench, + userId: owner, + activeFileId: activeFileId + ? (toOriginal(activeFileId as FileId) ?? undefined) + : undefined, + }); + }, [store, workbench, activeFileId, userId, owner]); + + // Read by the file subscription, which must not resubscribe on every view change. + const writeRef = useRef(write); + writeRef.current = write; + + useEffect(() => { + if (!store || onAuthRoute) return; + // This mount is a new session: undo any suspension left by a sign-out in this page's lifetime. + resumeWorkbenchSession(); + let timer: ReturnType | undefined; + const unsubscribe = store.subscribe(() => { + clearTimeout(timer); + timer = setTimeout(() => writeRef.current(), WRITE_DEBOUNCE_MS); + }); + return () => { + clearTimeout(timer); + // Flush, so the state at the moment of the shell switch is what survives. + writeRef.current(); + unsubscribe(); + }; + }, [store, onAuthRoute]); + + // Changing view touches no file state, so the subscription above never sees it. + useEffect(() => write(), [write]); + + useEffect(() => { + if (restoreStarted.current) return; + if (onAuthRoute) return; + // Who is signed in decides whether this record is theirs to reopen, so settle that first. + if (authLoading) return; + restoreStarted.current = true; + + const nothingToDo = + !WORKBENCH_SESSION_RESTORE || + !store || + !saved || + saved.fileIds.length === 0 || + store.getState().files.ids.length > 0; + if (nothingToDo) { + restoreSettled.current = true; + return; + } + + void (async () => { + // A tab can outlive a sign-out (the logout clears it, but a 401 bounce or an expiry does + // not), and the next person to sign in here must not open the last person's documents. + const currentOwner = + userId == null ? null : await fingerprintOwner(userId); + if ((saved.userId ?? null) !== currentOwner) { + clearWorkbenchSession(); + restoreSettled.current = true; + return; + } + + // Held while the files land: they are added one at a time, and each landing re-runs the + // default-view heuristic, which must not overwrite the recorded view mid-restore. + let held: number | null = null; + try { + // Resolve each id to its CURRENT leaf: a policy or another tab may have versioned it since. + const leaves = leafByOriginalId( + await fileStorage.getLeafStirlingFileStubs(), + ); + const stubs = saved.fileIds + .map((id) => leaves.get(id)) + .filter((stub): stub is StirlingFileStub => stub !== undefined); + + if (stubs.length > 0) { + const view = isSeedableView(saved.workbench) ? saved.workbench : null; + if (view) held = beginRestoredView(); + // The same entry point My Files uses, so a restored file is governed by the same rules as + // any other file entering the workbench - including whether a policy has already run on it. + await actions.addStirlingFileStubs(stubs); + const selected = saved.selectedFileIds + .map((id) => leaves.get(id)?.id) + .filter((id): id is FileId => id !== undefined); + if (selected.length > 0) actions.setSelectedFiles(selected); + // After the files land: the viewer drops an active id it cannot find. + const active = saved.activeFileId + ? leaves.get(saved.activeFileId)?.id + : undefined; + if (active) setActiveFileId(active as string); + if (view && held !== null) { + reopenView(store, navigationActions.restoreWorkbench, { + view, + fileCount: stubs.length, + token: held, + }); + held = null; // reopenView owns the release from here. + } + } + + const missing = saved.fileIds.length - stubs.length; + if (missing > 0) { + alert({ + alertType: "warning", + title: + stubs.length === 0 + ? t( + "workbench.sessionRestore.none", + "Your previous files are no longer stored on this device.", + ) + : t( + "workbench.sessionRestore.partial", + "Restored {{restored}} of {{total}} files. The rest are no longer stored on this device.", + { restored: stubs.length, total: saved.fileIds.length }, + ), + }); + } + } finally { + if (held !== null) endRestoredView(held); + // Even a failed restore must release the writer, or the record freezes for the session. + restoreSettled.current = true; + } + })(); + }, [ + saved, + store, + actions, + navigationActions, + setActiveFileId, + t, + authLoading, + userId, + onAuthRoute, + ]); + + return null; +} diff --git a/frontend/editor/src/core/constants/featureFlags.ts b/frontend/editor/src/core/constants/featureFlags.ts index a60770ac3a..8981dce2d6 100644 --- a/frontend/editor/src/core/constants/featureFlags.ts +++ b/frontend/editor/src/core/constants/featureFlags.ts @@ -11,3 +11,6 @@ // Annotated as `boolean` (not the literal `false`) so call sites aren't treated // as constant/unreachable conditions by the type checker and linter. export const WATCHED_FOLDERS_ENABLED: boolean = false; + +// Refill an empty workbench from the tab's last session (survives a provider remount or a reload). +export const WORKBENCH_SESSION_RESTORE: boolean = true; diff --git a/frontend/editor/src/core/contexts/NavigationContext.tsx b/frontend/editor/src/core/contexts/NavigationContext.tsx index a7cb95d349..f158991378 100644 --- a/frontend/editor/src/core/contexts/NavigationContext.tsx +++ b/frontend/editor/src/core/contexts/NavigationContext.tsx @@ -94,6 +94,9 @@ export interface NavigationWarningHandlers { // Navigation context actions interface export interface NavigationContextActions { setWorkbench: (workbench: WorkbenchType) => void; + /** Reopen a view the user already had, bypassing the unsaved-changes prompt that + * guards a user-initiated switch - a restore is not the user leaving anything. */ + restoreWorkbench: (workbench: WorkbenchType) => void; setSelectedTool: (toolId: ToolId | null) => void; setToolAndWorkbench: ( toolId: ToolId | null, @@ -221,6 +224,10 @@ export const NavigationProvider: React.FC<{ [state.workbench, state.hasUnsavedChanges], ); + const restoreWorkbench = useCallback((workbench: WorkbenchType) => { + dispatch({ type: "SET_WORKBENCH", payload: { workbench } }); + }, []); + const setSelectedTool = useCallback((toolId: ToolId | null) => { dispatch({ type: "SET_SELECTED_TOOL", payload: { toolId } }); }, []); @@ -402,6 +409,7 @@ export const NavigationProvider: React.FC<{ const actions: NavigationContextActions = useMemo( () => ({ setWorkbench, + restoreWorkbench, setSelectedTool, setToolAndWorkbench, setHasUnsavedChanges, @@ -419,6 +427,7 @@ export const NavigationProvider: React.FC<{ }), [ setWorkbench, + restoreWorkbench, setSelectedTool, setToolAndWorkbench, setHasUnsavedChanges, diff --git a/frontend/editor/src/core/extensions/accountLogout.ts b/frontend/editor/src/core/extensions/accountLogout.ts index e4eddd7274..df4e8f4273 100644 --- a/frontend/editor/src/core/extensions/accountLogout.ts +++ b/frontend/editor/src/core/extensions/accountLogout.ts @@ -1,3 +1,5 @@ +import { suspendWorkbenchSession } from "@app/services/workbenchSession"; + type SignOutFn = () => Promise; interface AccountLogoutDeps { @@ -21,6 +23,10 @@ export function useAccountLogout() { "1", ); } + // The tab outlives the session; the next person to sign in here must not + // inherit this workbench. Suspends writing too - signing out unmounts the + // editor, and its flush would otherwise write the record straight back. + suspendWorkbenchSession(); await signOut(); } finally { redirectToLogin(); diff --git a/frontend/editor/src/core/pages/HomePage.tsx b/frontend/editor/src/core/pages/HomePage.tsx index e0c5303447..0d7a571f98 100644 --- a/frontend/editor/src/core/pages/HomePage.tsx +++ b/frontend/editor/src/core/pages/HomePage.tsx @@ -15,6 +15,7 @@ import { useNavigationState, useNavigationActions, } from "@app/contexts/NavigationContext"; +import { isApplyingRestoredView } from "@app/services/workbenchSession"; import { useViewer } from "@app/contexts/ViewerContext"; import { useLocation, useNavigate } from "react-router-dom"; import AppsIcon from "@mui/icons-material/AppsRounded"; @@ -161,8 +162,14 @@ export default function HomePage() { if (navigationState.workbench !== "myFiles") { actions.setWorkbench("myFiles"); } - } else if (navigationState.workbench === "myFiles") { - // Leaving the file manager - drop back to a sensible default. + } else if ( + navigationState.workbench === "myFiles" && + !isApplyingRestoredView() + ) { + // The URL no longer supports the file manager - drop back to a sensible default. Stays a + // state check rather than a transition one: HomePage remounts without NavigationContext + // (a share link, a login bounce), and the view has to be corrected on arrival too. + // Skipped mid-restore, which is reopening a recorded view onto files still loading. actions.setWorkbench(activeFiles.length > 1 ? "fileEditor" : "viewer"); } }, [ @@ -204,7 +211,9 @@ export default function HomePage() { navigationState.workbench, ); - if (action) { + // A session restore fills an empty workbench too, but it already knows which view the user + // left - so it wins over this heuristic rather than being overwritten by it. + if (action && !isApplyingRestoredView()) { actions.setWorkbench(action.workbench); if (typeof action.activeFileIndex === "number") { setActiveFileIndex(action.activeFileIndex); diff --git a/frontend/editor/src/core/services/workbenchSession.test.ts b/frontend/editor/src/core/services/workbenchSession.test.ts new file mode 100644 index 0000000000..d9feec8d1c --- /dev/null +++ b/frontend/editor/src/core/services/workbenchSession.test.ts @@ -0,0 +1,193 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { + originalIdOf, + readWorkbenchSession, + writeWorkbenchSession, + saveEditorReturnPath, + takeEditorReturnPath, + isSeedableView, + clearWorkbenchSession, + suspendWorkbenchSession, + resumeWorkbenchSession, +} from "@app/services/workbenchSession"; +import type { StirlingFileStub } from "@app/types/fileContext"; + +const SESSION_KEY = "stirling.workbench.session"; + +beforeEach(() => { + sessionStorage.clear(); + resumeWorkbenchSession(); +}); + +describe("workbench session record", () => { + it("round-trips the open files and selection", () => { + writeWorkbenchSession({ fileIds: ["a", "b"], selectedFileIds: ["b"] }); + expect(readWorkbenchSession()).toMatchObject({ + fileIds: ["a", "b"], + selectedFileIds: ["b"], + }); + }); + + it("returns null when nothing was recorded", () => { + expect(readWorkbenchSession()).toBeNull(); + }); + + it("rejects a malformed record instead of throwing", () => { + sessionStorage.setItem(SESSION_KEY, "not json"); + expect(readWorkbenchSession()).toBeNull(); + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ v: 2, fileIds: "nope" }), + ); + expect(readWorkbenchSession()).toBeNull(); + }); + + it("drops non-string ids and defaults a missing selection", () => { + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ v: 2, fileIds: ["a", 7, null, "b"] }), + ); + expect(readWorkbenchSession()).toMatchObject({ + fileIds: ["a", "b"], + selectedFileIds: [], + }); + }); +}); + +describe("editor return path", () => { + it("is consumed by the first take", () => { + saveEditorReturnPath("/compress?x=1"); + expect(takeEditorReturnPath()).toBe("/compress?x=1"); + expect(takeEditorReturnPath()).toBeNull(); + }); +}); + +describe("originalIdOf", () => { + it("prefers the original id and falls back to the file id", () => { + expect( + originalIdOf({ id: "v3", originalFileId: "root" } as StirlingFileStub), + ).toBe("root"); + expect( + originalIdOf({ id: "v1", originalFileId: "" } as StirlingFileStub), + ).toBe("v1"); + }); +}); + +describe("record hygiene", () => { + it("discards a record written by an older schema", () => { + // No version stamp: a shape this build no longer understands. + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ fileIds: ["a"], selectedFileIds: [] }), + ); + expect(readWorkbenchSession()).toBeNull(); + + // v1 recorded userId before it meant anything, so those must go too rather than + // look like a workbench that legitimately belongs to an anonymous session. + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ + v: 1, + fileIds: ["a"], + selectedFileIds: [], + userId: null, + }), + ); + expect(readWorkbenchSession()).toBeNull(); + }); + + it("drops the previous record when a write fails, rather than leaving it stale", () => { + writeWorkbenchSession({ fileIds: ["old"], selectedFileIds: [] }); + const setItem = vi + .spyOn(Storage.prototype, "setItem") + .mockImplementation(() => { + throw new Error("QuotaExceededError"); + }); + + writeWorkbenchSession({ fileIds: ["new"], selectedFileIds: [] }); + setItem.mockRestore(); + + // Better to restore nothing than to restore a workbench the user has moved on from. + expect(readWorkbenchSession()).toBeNull(); + }); + + it("records who the workbench belonged to", () => { + writeWorkbenchSession({ + fileIds: ["a"], + selectedFileIds: [], + userId: "user-1", + }); + expect(readWorkbenchSession()?.userId).toBe("user-1"); + }); + + it("stays gone after sign-out, even though the teardown writes once more", () => { + writeWorkbenchSession({ fileIds: ["a", "b"], selectedFileIds: [] }); + + suspendWorkbenchSession(); + // Signing out unmounts the editor, whose flush writes the workbench one last time - + // with no user attached. Clearing alone let that recreate the record. + writeWorkbenchSession({ + fileIds: ["a", "b"], + selectedFileIds: [], + userId: null, + }); + + expect(sessionStorage.getItem(SESSION_KEY)).toBeNull(); + }); + + it("keeps the owner when the identity is momentarily unknown", () => { + // A sign-out teardown and a failed /auth/me both write with no user attached. Losing the + // owner here would make the record unrestorable for the person it belongs to. + writeWorkbenchSession({ + fileIds: ["a", "b"], + selectedFileIds: [], + userId: "user-a", + }); + + writeWorkbenchSession({ + fileIds: ["a", "b"], + selectedFileIds: [], + userId: null, + }); + + expect(readWorkbenchSession()?.userId).toBe("user-a"); + }); + + it("still records for a genuinely anonymous session", () => { + // Core has no auth at all, so null is the normal owner there and must keep working. + writeWorkbenchSession({ + fileIds: ["a"], + selectedFileIds: [], + userId: null, + }); + expect(readWorkbenchSession()?.fileIds).toEqual(["a"]); + }); + + it("records again once a new editor session starts", () => { + suspendWorkbenchSession(); + resumeWorkbenchSession(); + writeWorkbenchSession({ fileIds: ["a"], selectedFileIds: [] }); + expect(readWorkbenchSession()?.fileIds).toEqual(["a"]); + }); + + it("clears on request", () => { + writeWorkbenchSession({ fileIds: ["a"], selectedFileIds: [] }); + clearWorkbenchSession(); + expect(readWorkbenchSession()).toBeNull(); + }); +}); + +describe("views the restore may reopen", () => { + it("accepts the workbench views a session can land on", () => { + expect(isSeedableView("viewer")).toBe(true); + expect(isSeedableView("fileEditor")).toBe(true); + expect(isSeedableView("pageEditor")).toBe(true); + }); + + it("leaves URL-owned and tool-owned views alone", () => { + // HomePage pins myFiles to /files and bounces it elsewhere; custom views belong to a tool. + expect(isSeedableView("myFiles")).toBe(false); + expect(isSeedableView("custom:compare")).toBe(false); + expect(isSeedableView(undefined)).toBe(false); + }); +}); diff --git a/frontend/editor/src/core/services/workbenchSession.ts b/frontend/editor/src/core/services/workbenchSession.ts new file mode 100644 index 0000000000..67f6d1abfc --- /dev/null +++ b/frontend/editor/src/core/services/workbenchSession.ts @@ -0,0 +1,173 @@ +// The tab's last editor session (open files, selection, view), so a processor switch or reload +// does not cost the user their workbench. sessionStorage on purpose: per-tab, tabs never clobber. + +import type { StirlingFileStub } from "@app/types/fileContext"; + +const SESSION_KEY = "stirling.workbench.session"; +/** Bumped when the record's shape or meaning changes, so an old one is discarded rather than + * half-read. v2: `userId` became meaningful - v1 records were written without a real owner and + * would otherwise look like they belonged to an anonymous session forever. */ +const SESSION_VERSION = 2; +const RETURN_PATH_KEY = "stirling.workbench.editorReturnPath"; + +// All ids are ORIGINAL file ids - a file's stable identity across versions. +export interface WorkbenchSession { + fileIds: string[]; + selectedFileIds: string[]; + /** Which view was on screen. Absent for a record written before this was tracked. */ + workbench?: string; + activeFileId?: string; + /** Fingerprint of who the workbench belonged to, so the next person in this tab does not + * inherit it. Never the account id itself - see {@link fingerprintOwner}. */ + userId?: string | null; +} + +/** A file's stable identity across versions - what the session records. */ +export function originalIdOf(stub: StirlingFileStub): string { + return stub.originalFileId || (stub.id as string); +} + +export function readWorkbenchSession(): WorkbenchSession | null { + try { + const raw = sessionStorage.getItem(SESSION_KEY); + if (!raw) return null; + const parsed = JSON.parse(raw) as Partial & { + v?: number; + }; + if (parsed.v !== SESSION_VERSION) return null; + if (!Array.isArray(parsed.fileIds)) return null; + return { + fileIds: parsed.fileIds.filter((id) => typeof id === "string"), + selectedFileIds: Array.isArray(parsed.selectedFileIds) + ? parsed.selectedFileIds.filter((id) => typeof id === "string") + : [], + workbench: + typeof parsed.workbench === "string" ? parsed.workbench : undefined, + activeFileId: + typeof parsed.activeFileId === "string" + ? parsed.activeFileId + : undefined, + userId: typeof parsed.userId === "string" ? parsed.userId : null, + }; + } catch { + return null; + } +} + +// Sign-out clears the record, but signing out also tears the editor down - and that teardown +// flushes the workbench one last time, recreating what we just deleted (with no user attached). +// So a sign-out has to stop writing too, not merely clear. +let writesSuspended = false; + +/** Sign-out: drop the record and stop recording, so the teardown cannot put it back. */ +export function suspendWorkbenchSession(): void { + writesSuspended = true; + clearWorkbenchSession(); +} + +/** A fresh editor mount is a new session, so recording starts again. */ +export function resumeWorkbenchSession(): void { + writesSuspended = false; +} + +export function writeWorkbenchSession(session: WorkbenchSession): void { + if (writesSuspended) return; + try { + // Never downgrade a known owner to "nobody". Signing out and a failed identity check both + // read as no user, and dropping the owner would either hand the workbench to whoever signs + // in next or lose it for the person it belongs to. Keeping the owner leaves the restore's + // ownership check to decide, which it does with a settled identity. + const owner = session.userId ?? readWorkbenchSession()?.userId ?? null; + sessionStorage.setItem( + SESSION_KEY, + JSON.stringify({ ...session, userId: owner, v: SESSION_VERSION }), + ); + } catch { + // Storage refused (quota, privacy mode). setItem is atomic, so the PREVIOUS record would + // survive and restore an older workbench - drop it, so the failure is "no restore" instead. + clearWorkbenchSession(); + } +} + +/** + * A one-way fingerprint of the signed-in user. Owners are only ever compared, never read back, so + * the account id itself never needs to reach storage. Falls back to a non-cryptographic digest + * where SubtleCrypto is absent (a self-hosted instance served over plain http): the fingerprint + * only has to tell two accounts sharing one tab apart, and the files it gates are reachable from + * My Files regardless, since IndexedDB is per-origin. + */ +export async function fingerprintOwner(userId: string): Promise { + if (globalThis.crypto?.subtle) { + const digest = await globalThis.crypto.subtle.digest( + "SHA-256", + new TextEncoder().encode(userId), + ); + return Array.from(new Uint8Array(digest)) + .map((byte) => byte.toString(16).padStart(2, "0")) + .join("") + .slice(0, 32); + } + let hash = 0x811c9dc5; + for (let i = 0; i < userId.length; i++) { + hash ^= userId.charCodeAt(i); + hash = Math.imul(hash, 0x01000193) >>> 0; + } + return `fnv-${hash.toString(16)}`; +} + +/** Drop the record: on sign-out, and whenever it would otherwise be restored for the wrong person. */ +export function clearWorkbenchSession(): void { + try { + sessionStorage.removeItem(SESSION_KEY); + } catch { + // A record we cannot remove is also one we cannot read. + } +} + +/** Views a restore may seed directly. "myFiles" is URL-owned (HomePage pins it to /files) and a + * custom view belongs to its tool - the editor return path restores those instead. */ +const SEEDABLE_VIEWS = ["viewer", "fileEditor", "pageEditor"]; + +// Raised while a restore is applying its recorded view, so writers that pick a default view from +// whatever is loaded at the time defer to the restore rather than race it. +let applyingRestoredView = false; +let restoreGeneration = 0; + +/** Returns a token for endRestoredView, so a stale release cannot end a newer restore. */ +export function beginRestoredView(): number { + applyingRestoredView = true; + return ++restoreGeneration; +} + +export function endRestoredView(token: number): void { + if (token === restoreGeneration) applyingRestoredView = false; +} + +export function isApplyingRestoredView(): boolean { + return applyingRestoredView; +} + +export function isSeedableView( + view: string | undefined, +): view is "viewer" | "fileEditor" | "pageEditor" { + return view !== undefined && SEEDABLE_VIEWS.includes(view); +} + +export function saveEditorReturnPath(path: string): void { + try { + sessionStorage.setItem(RETURN_PATH_KEY, path); + } catch { + // Best-effort: the switch back just lands on the editor root. + } +} + +/** One-shot: consumed by the switch back so a stale path cannot linger. */ +export function takeEditorReturnPath(): string | null { + try { + const path = sessionStorage.getItem(RETURN_PATH_KEY); + if (path !== null) sessionStorage.removeItem(RETURN_PATH_KEY); + return path; + } catch { + return null; + } +} diff --git a/frontend/editor/src/core/tests/stubbed/engine-capabilities.spec.ts b/frontend/editor/src/core/tests/stubbed/engine-capabilities.spec.ts index 6107fb5276..c5b24dd6ac 100644 --- a/frontend/editor/src/core/tests/stubbed/engine-capabilities.spec.ts +++ b/frontend/editor/src/core/tests/stubbed/engine-capabilities.spec.ts @@ -107,6 +107,12 @@ test.describe("engine capabilities", { tag: "@engine-capability" }, () => { await uploadFiles(page, SAMPLE_PDF); + // Dropped before the reload boots, so it cannot reopen the file for us: the eye + // below toggles, and whether the restore runs is a build flag this spec does not own. + await page.addInitScript(() => + sessionStorage.removeItem("stirling.workbench.session"), + ); + // Full reload: FileContext rehydrates from IndexedDB, not from memory. await page.reload({ waitUntil: "domcontentloaded" }); diff --git a/frontend/editor/src/core/tests/stubbed/workbench-session-restore.spec.ts b/frontend/editor/src/core/tests/stubbed/workbench-session-restore.spec.ts new file mode 100644 index 0000000000..48765392fe --- /dev/null +++ b/frontend/editor/src/core/tests/stubbed/workbench-session-restore.spec.ts @@ -0,0 +1,207 @@ +import { test, expect } from "@app/tests/helpers/stub-test-base"; +import { uploadFiles } from "@app/tests/helpers/ui-helpers"; +import path from "path"; + +const FIXTURES_DIR = path.join(import.meta.dirname, "../test-fixtures"); +const SAMPLES = [ + "compare_sample_a.pdf", + "compare_sample_b.pdf", + "sample.pdf", + "rotated-pages.pdf", + "annotations_out_of_order.pdf", +].map((name) => path.join(FIXTURES_DIR, name)); + +// Read from the running app, not imported: a spec resolves @app/* to a different layer than +// the browser build does, so an imported WORKBENCH_SESSION_RESTORE can disagree with reality. +async function restoreEnabled( + page: import("@playwright/test").Page, +): Promise { + await page.waitForFunction( + () => document.documentElement.dataset.workbenchRestore !== undefined, + null, + { timeout: 20000 }, + ); + return page.evaluate( + () => document.documentElement.dataset.workbenchRestore === "true", + ); +} + +const NO_RESTORE = "this build ships the workbench restore off"; + +// Switching editor -> processor unmounts every editor provider; the session record +// in sessionStorage is what brings the workbench back on return. +test.describe("Workbench survives the editor/processor switch", () => { + test.use({ + stubOptions: { + enableLogin: true, + user: { + id: 44, + username: "owner", + email: "owner@example.com", + role: "ROLE_USER", + portalAccess: true, + }, + }, + seedJwt: true, + }); + + test("open files and the library return after a round-trip", async ({ + page, + }) => { + test.skip(!(await restoreEnabled(page)), NO_RESTORE); + + // Portal endpoints the processor shell fetches on mount. + for (const [pattern, json] of [ + ["**/api/v1/policies", []], + ["**/api/v1/policies/runs", []], + ["**/api/v1/policies/overview", { pipelines: [] }], + ["**/api/v1/sources", { sources: [] }], + ["**/api/v1/team/my", []], + ] as const) { + await page.route(pattern, (route) => route.fulfill({ json })); + } + + await uploadFiles(page, SAMPLES); + await expect(page.locator(".file-sidebar-file-item")).toHaveCount( + SAMPLES.length, + { timeout: 15000 }, + ); + + // Uploading lands on the file grid, not the viewer - so the return has a + // view it can get wrong (NavigationContext boots to "viewer"). + await expect( + page.getByRole("radio", { name: /Active Files/i }), + ).toBeChecked(); + + // Out through the sidebar footer switch - the real user path. + await page.getByRole("button", { name: "Open PDF Processor" }).click(); + await expect(page).toHaveURL(/\/processor/, { timeout: 15000 }); + + // Split the two halves of the feature: if this fails the writer is at fault, + // if it passes but the view below is wrong the seeding is. + expect( + await page.evaluate(() => ({ + session: JSON.parse( + sessionStorage.getItem("stirling.workbench.session") ?? "{}", + ), + returnPath: sessionStorage.getItem( + "stirling.workbench.editorReturnPath", + ), + })), + ).toMatchObject({ + session: { workbench: "fileEditor" }, + returnPath: "/editor", + }); + + // Load the editor cold. Every provider mounts from nothing here, which is + // the loss the restore has to cover on the way back. + await page.goto("/editor", { waitUntil: "domcontentloaded" }); + + await expect(page.locator(".file-sidebar-file-item")).toHaveCount( + SAMPLES.length, + { timeout: 20000 }, + ); + await expect(page.getByText(/compare_sample_a/i).first()).toBeVisible(); + await expect( + page.getByRole("radio", { name: /Active Files/i }), + ).toBeChecked({ timeout: 15000 }); + await expect(page.locator(".file-sidebar-loading")).toHaveCount(0, { + timeout: 15000, + }); + }); +}); + +test.describe("The view survives a reload", () => { + test.use({ + stubOptions: { + enableLogin: true, + user: { + id: 44, + username: "owner", + email: "o@e.com", + role: "ROLE_USER", + portalAccess: true, + }, + }, + seedJwt: true, + }); + + const currentView = (page: import("@playwright/test").Page) => + page.evaluate(() => { + const r = Array.from( + document.querySelectorAll("input[type=radio]"), + ).find((x) => x.checked); + return r?.value ?? "none"; + }); + + test("comes back on the same view the user left", async ({ page }) => { + test.skip(!(await restoreEnabled(page)), NO_RESTORE); + + await uploadFiles(page, SAMPLES.slice(0, 3)); + await expect(page.locator(".file-sidebar-file-item")).toHaveCount(3, { + timeout: 15000, + }); + + // Open a document, so the view under test is the viewer rather than the grid. + await page + .getByRole("button", { name: /Open in Viewer/i }) + .first() + .click({ force: true }); + await expect + .poll(() => currentView(page), { timeout: 10000 }) + .toBe("viewer"); + + // Whatever the workbench settled on is what a reload must reproduce. + const before = await currentView(page); + await page.waitForTimeout(600); + + await page.reload({ waitUntil: "domcontentloaded" }); + await expect(page.locator(".file-sidebar-file-item")).toHaveCount(3, { + timeout: 20000, + }); + await page.waitForTimeout(3000); + expect(await currentView(page)).toBe(before); + }); + + // The conjunction neither neighbour covers: the spec above proves the VIEW comes back, + // engine-capabilities proves stored bytes decode, and nothing proved that the file the + // restore reopened is one whose pixels actually arrive. + test("a file the restore reopened renders its pages", async ({ page }) => { + test.setTimeout(120_000); + test.skip(!(await restoreEnabled(page)), NO_RESTORE); + + await uploadFiles(page, SAMPLES.slice(0, 3)); + await expect(page.locator(".file-sidebar-file-item")).toHaveCount(3, { + timeout: 15_000, + }); + + await page + .getByRole("button", { name: /Open in Viewer/i }) + .first() + .click({ force: true }); + await expect + .poll(() => currentView(page), { timeout: 10_000 }) + .toBe("viewer"); + // Let the record settle: the writer debounces, so a reload can outrun it. + await page.waitForTimeout(600); + + await page.reload({ waitUntil: "domcontentloaded" }); + await expect(page.locator(".file-sidebar-file-item")).toHaveCount(3, { + timeout: 30_000, + }); + + // A tile that decoded has non-zero naturalWidth. The restore resolves each recorded id + // to its current leaf, so an empty tile here means it reopened something unreadable. + const tile = page + .locator('[data-page-index="0"]') + .first() + .locator('img[src^="blob:"]') + .first(); + await expect(tile).toBeAttached({ timeout: 30_000 }); + await expect + .poll(() => tile.evaluate((img: HTMLImageElement) => img.naturalWidth), { + timeout: 30_000, + }) + .toBeGreaterThan(0); + }); +}); diff --git a/frontend/editor/src/desktop/components/session/WorkbenchSessionPersistence.tsx b/frontend/editor/src/desktop/components/session/WorkbenchSessionPersistence.tsx new file mode 100644 index 0000000000..2957ac3735 --- /dev/null +++ b/frontend/editor/src/desktop/components/session/WorkbenchSessionPersistence.tsx @@ -0,0 +1,4 @@ +// Stub: desktop opens OS-launched files on boot; a session restore would collide with that. +export function WorkbenchSessionPersistence() { + return null; +} diff --git a/frontend/editor/src/desktop/extensions/accountLogout.ts b/frontend/editor/src/desktop/extensions/accountLogout.ts index b97b06ec6e..ca75c9c5e9 100644 --- a/frontend/editor/src/desktop/extensions/accountLogout.ts +++ b/frontend/editor/src/desktop/extensions/accountLogout.ts @@ -1,4 +1,5 @@ import { connectionModeService } from "@app/services/connectionModeService"; +import { suspendWorkbenchSession } from "@app/services/workbenchSession"; type SignOutFn = () => Promise; @@ -16,6 +17,10 @@ export function useAccountLogout() { redirectToLogin, }: AccountLogoutDeps): Promise => { try { + // The tab outlives the session; the next person to sign in here must not + // inherit this workbench. Suspends writing too - signing out unmounts the + // editor, and its flush would otherwise write the record straight back. + suspendWorkbenchSession(); await signOut(); const currentConfig = await connectionModeService.getCurrentConfig(); diff --git a/frontend/editor/src/portal/PortalProviders.tsx b/frontend/editor/src/portal/PortalProviders.tsx index 35c9fc7e88..a82f1c7405 100644 --- a/frontend/editor/src/portal/PortalProviders.tsx +++ b/frontend/editor/src/portal/PortalProviders.tsx @@ -1,52 +1,24 @@ import { TierProvider } from "@portal/contexts/TierContext"; -import { LinkProvider, useLink } from "@portal/contexts/LinkContext"; +import { LinkProvider } from "@portal/contexts/LinkContext"; import { UIProvider, useUI } from "@portal/contexts/UIContext"; -import type { SupabaseLoginSession } from "@app/auth/ui/useSupabaseLogin"; import { LinkAccountModal } from "@portal/components/account-link/LinkAccountModal"; -import { - AccountLinkProvider, - useAccountLinkContext, -} from "@portal/contexts/AccountLinkContext"; +import { AccountLinkProvider } from "@portal/contexts/AccountLinkContext"; +import { ConnectCallbackHost } from "@portal/components/account-link/ConnectCallbackHost"; import { PortalChrome } from "@portal/components/PortalChrome"; -/** - * The one and only account-link login modal. Mounted at the app root (never - * nested in another overlay) and driven by UIContext, so any "Link account" CTA - * — sidebar, billing prompt, feature gate, Settings panel — opens this exact - * instance. Linking is finished by the shared {@link useAccountLinkContext} - * orchestration. - */ +/** The one and only account-link modal. */ function LinkModalHost() { const { linkModalOpen, linkModalMode, closeLinkModal } = useUI(); - const { markSaasSessionChanged } = useLink(); - const link = useAccountLinkContext(); - // "reauth" only refreshes the browser SaaS session for attended reads — the - // sign-in already applied it to the Supabase client, so we just signal a - // refetch. It must NOT call completeLink (that re-registers → duplicate row). - const onLinked = - linkModalMode === "reauth" - ? () => markSaasSessionChanged() - : (session: SupabaseLoginSession) => link.completeLink(session); return ( ); } -/** - * Self-hosted provider stack. The account-link layer (LinkProvider + - * AccountLinkProvider + the login modal) wraps the shared chrome; the tier is - * derived from the link/subscription state (see usePlanTier). TierProvider sits - * inside LinkProvider because the self-hosted usePlanTier reads useLink. - * - * The SaaS build shadows this file to drop the account-link layer entirely — the - * signed-in account IS the SaaS account, so there is nothing to link and the - * tier comes from the wallet. - */ +/** Self-hosted provider stack. */ export function PortalProviders() { return ( @@ -55,6 +27,7 @@ export function PortalProviders() { + diff --git a/frontend/editor/src/portal/api/link.test.ts b/frontend/editor/src/portal/api/link.test.ts index e03d5bf25b..9c05952549 100644 --- a/frontend/editor/src/portal/api/link.test.ts +++ b/frontend/editor/src/portal/api/link.test.ts @@ -34,7 +34,6 @@ import { fetchInstances, fetchLocalUsage, fetchStatus, - linkInstance, revokeInstance, unlinkInstance, } from "@portal/api/link"; @@ -55,21 +54,14 @@ describe("api/link — local backend (this instance)", () => { expect(status.linked).toBe(false); }); - it("links this instance via the local endpoint, never returning a secret", async () => { - const status = await linkInstance({ - supabaseJwt: "jwt_abc", - name: "node-1", - }); - expect(status.linked).toBe(true); - expect(status.name).toBe("node-1"); - // Contract: the device secret is stored server-side, never sent to the portal. + it("never exposes the device credential in a status read", async () => { + // Contract: the device secret is stored server-side and the portal never sees it. + const status = await fetchStatus(); expect(status).not.toHaveProperty("deviceSecret"); expect(status).not.toHaveProperty("deviceId"); - expect(await (await fetchStatus()).linked).toBe(true); }); it("unlinks this instance", async () => { - await linkInstance({ supabaseJwt: "jwt_abc" }); // unlink returns 204 (no body); the status is read back separately. await unlinkInstance(); expect((await fetchStatus()).linked).toBe(false); @@ -84,18 +76,6 @@ describe("api/link — local backend (this instance)", () => { ); expect(usage.totalUnsyncedUnits).toBeGreaterThanOrEqual(0); }); - - it("forwards the SaaS JWT in the link body", async () => { - let seenBody: unknown = null; - server.events.on("request:start", async ({ request }) => { - if (request.method === "POST" && request.url.endsWith("/link")) { - seenBody = await request.clone().json(); - } - }); - await linkInstance({ supabaseJwt: "jwt_xyz", name: "n" }); - expect(seenBody).toMatchObject({ supabaseJwt: "jwt_xyz" }); - server.events.removeAllListeners(); - }); }); describe("api/link — SaaS backend (team-wide)", () => { diff --git a/frontend/editor/src/portal/api/link.ts b/frontend/editor/src/portal/api/link.ts index 696ccbe5b2..c3684b3dd2 100644 --- a/frontend/editor/src/portal/api/link.ts +++ b/frontend/editor/src/portal/api/link.ts @@ -1,13 +1,5 @@ import { apiClient } from "@portal/api/http"; -/** Body for POST /api/v1/account-link/link — the SaaS JWT + optional name. */ -export interface LinkInstanceRequest { - /** Admin's SaaS session JWT, obtained via the hosted-login popup. */ - supabaseJwt: string; - /** Optional label for this instance. */ - name?: string; -} - /** Link status for this instance (GET /api/v1/account-link/status). */ export interface LinkStatus { linked: boolean; @@ -15,12 +7,7 @@ export interface LinkStatus { name: string | null; } -/** - * Locally-accrued usage not yet reported to SaaS (GET /api/v1/account-link/usage). - * The portal adds this on top of the SaaS-synced spend so "current usage" - * includes work done since the last daily sync. Per-category unsynced units for - * the current period; all zero when metering is off or nothing is pending. - */ +/** Locally-accrued usage not yet reported to SaaS (GET /api/v1/account-link/usage). */ export interface LocalUsage { /** ISO timestamp of the current period start; null when unknown (not yet synced). */ periodStart: string | null; @@ -42,93 +29,88 @@ export interface LinkedInstanceRow { revoked: boolean; } -/** - * Account-link client (combined-billing "Mode A"). Two distinct surfaces: - * - * THIS instance — apiClient.local (Spring admin bearer auto-attached): - * - POST /api/v1/account-link/link — hand the local backend the admin's - * SaaS JWT in the body. It registers - * with SaaS + stores the device - * secret SERVER-SIDE; the portal - * NEVER receives or renders it. - * - GET /api/v1/account-link/status — Linked / Not-linked for this - * instance. - * - POST /api/v1/account-link/unlink — drop this instance's link (local - * backend best-effort tells SaaS). - * - * TEAM-WIDE management — apiClient.saas (admin's Supabase JWT auto-attached - * from the in-app account-link login): - * - GET /api/v1/account-link/instances — every linked instance - * - POST /api/v1/account-link/instances/{id}/revoke - * - * The team-wide endpoints are served by the hosted SaaS Java backend (the - * local backend has no such routes), so they go through apiClient.saas. In - * Storybook/tests, wildcard MSW handlers match both the local and absolute - * SaaS URLs. - */ +/** Account-link client (combined billing). */ const BASE = "/api/v1/account-link"; -/** - * Link THIS instance. The local backend takes the SaaS JWT, registers with - * SaaS, and persists the device secret itself; the response carries only the - * resulting link status. No secret is returned. - */ -export async function linkInstance( - req: LinkInstanceRequest, -): Promise { - return apiClient.local.json(`${BASE}/link`, { - method: "POST", - body: req, - }); -} - /** Linked / Not-linked for this instance. */ export async function fetchStatus(): Promise { return apiClient.local.json(`${BASE}/status`); } /** - * Locally-accrued usage not yet reported to SaaS — the portal adds this on top - * of the SaaS-synced spend so "current usage" includes work done since the last - * daily sync. Local-backend call; returns zeros when metering is off. + * Locally-accrued usage not yet reported to SaaS — the portal adds this on top of the SaaS-synced spend so "current usage" includes work done since the last daily sync. */ export async function fetchLocalUsage(): Promise { return apiClient.local.json(`${BASE}/usage`); } -/** - * Drop this instance's link. The local backend best-effort tells SaaS to - * revoke before clearing the credential locally, then returns 204 — there's no - * body, so the caller sets the known unlinked status itself. - */ +/** Drop this instance's link. */ export async function unlinkInstance(): Promise { await apiClient.local.json(`${BASE}/unlink`, { method: "POST" }); } -/** - * Nudge the local backend to sync + refresh its cached entitlement now. Called - * right after a checkout completes so the instance's request-time gate reflects - * the new subscription immediately instead of waiting out its entitlement-cache - * TTL. Best-effort — the caller swallows failures (metering off → 409, or the - * local backend unreachable); the scheduled sync / TTL refresh is the backstop. - */ +/** Nudge the local backend to sync + refresh its cached entitlement now. */ export async function triggerLocalSync(): Promise { await apiClient.local.json(`${BASE}/sync-now`, { method: "POST" }); } +/** Where a browser-mediated connect handshake has got to. */ +export type ConnectPhase = + | "NONE" + | "PENDING" + | "LINKED" + | "EXPIRED" + | "REJECTED" + | "UNAVAILABLE"; + +export interface ConnectStatus { + phase: ConnectPhase; + /** Approval page to send the admin to. */ + authorizeUrl: string | null; + secondsRemaining: number | null; + teamId: number | null; +} + +const CONNECT = `${BASE}/connect`; + +/** Open a handshake and get the approval URL to send the admin to. */ +export async function startConnect( + name?: string, + callbackUrl?: string, +): Promise { + return apiClient.local.json(`${CONNECT}/start`, { + method: "POST", + body: { name, callbackUrl }, + }); +} + +/** Re-establish the SaaS session for a server that is already linked. */ +export async function startReauth( + callbackUrl?: string, +): Promise { + return apiClient.local.json(`${CONNECT}/reauth`, { + method: "POST", + body: { callbackUrl }, + }); +} + +/** Finish a handshake using the nonce the approval page put in the callback fragment. */ +export async function completeConnect(nonce: string): Promise { + return apiClient.local.json(`${CONNECT}/complete`, { + method: "POST", + body: { nonce }, + }); +} + /** - * Every linked instance for the team — SaaS-direct call with the admin's - * Supabase JWT (no longer takes an accessToken parameter; the saas client - * resolves the live session itself). + * Every linked instance for the team — SaaS-direct call with the admin's Supabase JWT (no longer takes an accessToken parameter; the saas client resolves the live session itself). */ export async function fetchInstances(): Promise { return apiClient.saas.json(`${BASE}/instances`); } -/** - * Revoke a linked instance — SaaS-direct call with the admin's Supabase JWT. - */ +/** Revoke a linked instance — SaaS-direct call with the admin's Supabase JWT. */ export async function revokeInstance(instanceId: number): Promise { await apiClient.saas.json(`${BASE}/instances/${instanceId}/revoke`, { method: "POST", diff --git a/frontend/editor/src/portal/auth/saasSupabase.ts b/frontend/editor/src/portal/auth/saasSupabase.ts index 1005d63a63..45598c369b 100644 --- a/frontend/editor/src/portal/auth/saasSupabase.ts +++ b/frontend/editor/src/portal/auth/saasSupabase.ts @@ -22,11 +22,12 @@ const key = import.meta.env.VITE_SUPABASE_PUBLISHABLE_DEFAULT_KEY; export const isSaasSupabaseConfigured = Boolean(url && key); -/** OAuth providers the hosted SaaS login offers (mirrors the SaaS editor login). */ -export const SAAS_OAUTH_PROVIDERS = ["google", "github", "apple", "azure"]; - -/** sessionStorage marker set before an SSO redirect so the return can finish the link. */ -export const PENDING_LINK_KEY = "stirling-account-link-pending"; +/* + * SAAS_OAUTH_PROVIDERS and PENDING_LINK_KEY are gone. They served an in-portal SSO sign-in that + * could not work: the provider only redirects to allow-listed URLs, so a customer's origin was + * never returned to and the admin was left on stirling.com. Provider choice now happens on our own + * origin during the connect handshake, where the redirect can actually complete. + */ let configured = false; diff --git a/frontend/editor/src/portal/components/Sidebar.tsx b/frontend/editor/src/portal/components/Sidebar.tsx index 8ce7008d67..685fa32722 100644 --- a/frontend/editor/src/portal/components/Sidebar.tsx +++ b/frontend/editor/src/portal/components/Sidebar.tsx @@ -14,6 +14,7 @@ import { useUI } from "@portal/contexts/UIContext"; import { LinkAccountFooterItem } from "@portal/components/LinkAccountFooterItem"; import { EDITOR_URL, EDITOR_IS_SAME_APP } from "@portal/auth/editorUrl"; import { EDITOR_BASENAME } from "@app/routes/editorBasename"; +import { takeEditorReturnPath } from "@app/services/workbenchSession"; import { CloseIcon } from "@portal/components/icons"; import { GROUP_PROCESSOR, @@ -57,7 +58,7 @@ export function Sidebar() { // the switch stays client-side; an absolute EDITOR_URL (dev cross-app setup) // needs a full page load. const goToEditor = () => { - if (EDITOR_IS_SAME_APP) navigate(EDITOR_BASENAME); + if (EDITOR_IS_SAME_APP) navigate(takeEditorReturnPath() ?? EDITOR_BASENAME); else window.location.href = EDITOR_URL; }; diff --git a/frontend/editor/src/portal/components/account-link/ConnectCallbackHost.tsx b/frontend/editor/src/portal/components/account-link/ConnectCallbackHost.tsx new file mode 100644 index 0000000000..fab501da4a --- /dev/null +++ b/frontend/editor/src/portal/components/account-link/ConnectCallbackHost.tsx @@ -0,0 +1,184 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { useLocation, useNavigate } from "react-router-dom"; +import { useTranslation } from "react-i18next"; +import { Modal } from "@app/ui"; +import { PORTAL_BASENAME } from "@app/routes/portalBasename"; +import { withBasePath } from "@app/constants/app"; +import { + completeConnect, + startConnect, + type ConnectPhase, +} from "@portal/api/link"; +import { ensureSaasSupabase } from "@portal/auth/saasSupabase"; +import { useAccountLinkContext } from "@portal/contexts/AccountLinkContext"; +import { + ConnectCallbackView, + type ConnectCallbackState, +} from "@portal/components/account-link/ConnectCallbackView"; +import "@portal/views/ConnectCallback.css"; + +/** What the callback route hands over, read from the URL fragment before stripping it. */ +export interface AccountLinkReturn { + type: string | null; + nonce: string | null; + accessToken: string | null; + refreshToken: string | null; +} + +interface LocationState { + accountLinkReturn?: AccountLinkReturn; +} + +/** + * Finishes the handshake and reports the outcome, over the portal the admin + * started from. + * + * Mounted alongside the other portal-wide modal rather than being its own route: + * the result is a step in a task, so the page behind it should still be there. + */ +export function ConnectCallbackHost() { + const location = useLocation(); + const navigate = useNavigate(); + const { t } = useTranslation(); + const { refresh } = useAccountLinkContext(); + const handover = (location.state as LocationState | null)?.accountLinkReturn; + + const [state, setState] = useState(null); + const [sessionRestored, setSessionRestored] = useState(false); + const nonceRef = useRef(null); + const startedRef = useRef(false); + + const finish = useCallback( + async (nonce: string) => { + setState("working"); + try { + const outcome = toViewState((await completeConnect(nonce)).phase); + setState(outcome); + // The portal read its status on mount, before this existed. Without this + // the page behind the modal still says unlinked until a reload. + if (outcome === "linked") await refresh(); + } catch { + // Could not reach our own backend. The handshake is still open, so this + // is worth another attempt rather than a restart. + setState("retry"); + } + }, + [refresh], + ); + + useEffect(() => { + if (!handover || startedRef.current) return; + startedRef.current = true; + + const { type, nonce, accessToken, refreshToken } = handover; + if (type !== "link" || !nonce) { + setState("malformed"); + return; + } + nonceRef.current = nonce; + + void (async () => { + if (accessToken && refreshToken) { + try { + const supabase = ensureSaasSupabase(); + // Logged, not swallowed: silently this resurfaces later as "session + // expired" on the usage page, with nothing tying it back here. + if (!supabase) { + console.warn( + "[account-link] no Supabase client: VITE_SUPABASE_URL / VITE_SUPABASE_PUBLISHABLE_DEFAULT_KEY are not set for this build", + ); + } else { + const { error } = await supabase.auth.setSession({ + access_token: accessToken, + refresh_token: refreshToken, + }); + if (error) { + console.warn("[account-link] setSession failed:", error.message); + } else { + setSessionRestored(true); + } + } + } catch (e) { + console.warn("[account-link] session hand-off threw:", e); + } + } else { + console.warn( + "[account-link] callback carried no tokens; the approval page had no session to pass", + ); + } + await finish(nonce); + })(); + }, [handover, finish]); + + /** + * Retry means different things either side of a still-valid handshake: finish the one we have, or open a new one when it is past saving. + */ + const onRetry = useCallback(() => { + if (state === "retry" && nonceRef.current) { + void finish(nonceRef.current); + return; + } + setState("working"); + // Same callback the modal sends. Without it the backend falls back to the bare + // origin, which drops the app's base path and lands the return on nothing. + void startConnect( + window.location.hostname, + new URL( + withBasePath("/account-link/callback"), + window.location.origin, + ).toString(), + ) + .then((status) => { + if (status.authorizeUrl) { + window.location.assign(status.authorizeUrl); + } else { + setState("rejected"); + } + }) + .catch(() => setState("retry")); + }, [state, finish]); + + // Drops the handover with it, so a back navigation does not reopen the result. + const done = useCallback(() => { + setState(null); + navigate(PORTAL_BASENAME, { replace: true }); + }, [navigate]); + + if (!state) return null; + + return ( + + + + ); +} + +/** + * PENDING and UNAVAILABLE collapse into one "try again" state: both mean the handshake is intact but unfinished, which is the same thing to do about it. + */ +function toViewState(phase: ConnectPhase): ConnectCallbackState { + switch (phase) { + case "LINKED": + return "linked"; + case "EXPIRED": + return "expired"; + case "PENDING": + case "UNAVAILABLE": + return "retry"; + default: + return "rejected"; + } +} diff --git a/frontend/editor/src/portal/components/account-link/ConnectCallbackView.tsx b/frontend/editor/src/portal/components/account-link/ConnectCallbackView.tsx new file mode 100644 index 0000000000..4a22592e45 --- /dev/null +++ b/frontend/editor/src/portal/components/account-link/ConnectCallbackView.tsx @@ -0,0 +1,162 @@ +import { useTranslation } from "react-i18next"; +import { Banner, Button, Spinner } from "@app/ui"; + +/** Outcomes of returning from the approval page. */ +export type ConnectCallbackState = + | "working" + | "linked" + | "retry" + | "expired" + | "rejected" + | "malformed"; + +export interface ConnectCallbackViewProps { + state: ConnectCallbackState; + /** True once the SaaS session landed, regardless of how the link itself went. */ + sessionRestored: boolean; + onRetry: () => void; + onDone: () => void; +} + +/** Presentation for the account-link callback. */ +export function ConnectCallbackView({ + state, + sessionRestored, + onRetry, + onDone, +}: ConnectCallbackViewProps) { + const { t } = useTranslation(); + + if (state === "working") { + return ( +

+ +

+ {t( + "portal.accountLink.connect.callback.working", + "Finishing the connection.", + )} +

+
+ ); + } + + if (state === "linked") { + return ( +
+ + {t( + "portal.accountLink.connect.callback.linked.body", + "This server is connected to your Stirling account.", + )} + + {/* The inverse of the failure note below: the link took but the sign-in did + not, which otherwise only shows up later as "session expired" on a page + that gives no hint the two are related. */} + {sessionRestored ? null : ( +

+ {t( + "portal.accountLink.connect.callback.linkedNotSignedIn", + "You are not signed in to Stirling in this browser, so usage and billing will ask you to sign in.", + )} +

+ )} + +
+ ); + } + + const { tone, title, body, retryable } = failure(state, t); + return ( +
+ + {body} + + {/* The SaaS sign-in and the server link are separate outcomes. Say so when + one worked and the other did not, or the admin re-runs the whole thing + to fix a problem that is already half solved. */} + {sessionRestored ? ( +

+ {t( + "portal.accountLink.connect.callback.signedInAnyway", + "You are signed in to Stirling, so billing and usage will load. Only the server link is incomplete.", + )} +

+ ) : null} + +
+ ); +} + +type Translate = ReturnType["t"]; + +function failure(state: ConnectCallbackState, t: Translate) { + switch (state) { + case "expired": + return { + tone: "warning" as const, + title: t( + "portal.accountLink.connect.callback.expired.title", + "Request expired", + ), + body: t( + "portal.accountLink.connect.callback.expired.body", + "Connection requests are short lived. Start another one.", + ), + retryable: true, + }; + case "rejected": + return { + tone: "warning" as const, + title: t( + "portal.accountLink.connect.callback.rejected.title", + "Connection not completed", + ), + body: t( + "portal.accountLink.connect.callback.rejected.body", + "This request was declined or has already been used. Start another one if that was not intended.", + ), + retryable: true, + }; + case "malformed": + return { + tone: "danger" as const, + title: t( + "portal.accountLink.connect.callback.malformed.title", + "Could not read the response", + ), + body: t( + "portal.accountLink.connect.callback.malformed.body", + "This page was opened without a valid connection response. Start the connection from settings.", + ), + retryable: false, + }; + default: + return { + tone: "warning" as const, + // Not "retry.*": that key is the button label, and TOML cannot hold a + // value and a table under the same name. + title: t( + "portal.accountLink.connect.callback.unfinished.title", + "Not finished yet", + ), + body: t( + "portal.accountLink.connect.callback.unfinished.body", + "Stirling did not confirm the connection. This is usually temporary.", + ), + retryable: true, + }; + } +} diff --git a/frontend/editor/src/portal/components/account-link/LinkAccountCard.stories.tsx b/frontend/editor/src/portal/components/account-link/LinkAccountCard.stories.tsx index af949e9dc9..f20f777280 100644 --- a/frontend/editor/src/portal/components/account-link/LinkAccountCard.stories.tsx +++ b/frontend/editor/src/portal/components/account-link/LinkAccountCard.stories.tsx @@ -9,8 +9,9 @@ const base: UseAccountLink = { status: { linked: false, name: null }, phase: "idle", error: null, - completeLink: async () => {}, + unlink: async () => {}, + refresh: async () => {}, }; const meta: Meta = { diff --git a/frontend/editor/src/portal/components/account-link/LinkAccountModal.css b/frontend/editor/src/portal/components/account-link/LinkAccountModal.css new file mode 100644 index 0000000000..bc96172f0a --- /dev/null +++ b/frontend/editor/src/portal/components/account-link/LinkAccountModal.css @@ -0,0 +1,26 @@ +/* Connect-account modal. Imported by the component rather than relying on the + account-link view's stylesheet: this modal is mounted at the app root, so it + renders on pages that never import that view. */ + +.portal-link__modal-body { + display: flex; + flex-direction: column; + gap: 1rem; +} + +.portal-link__steps { + display: flex; + flex-direction: column; + gap: 0.5rem; + margin: 0; + padding-left: 1.25rem; + font-size: 0.875rem; + line-height: 1.5; + color: var(--c-text-muted); +} + +.portal-link__modal-actions { + display: flex; + justify-content: flex-end; + gap: 0.5rem; +} diff --git a/frontend/editor/src/portal/components/account-link/LinkAccountModal.stories.tsx b/frontend/editor/src/portal/components/account-link/LinkAccountModal.stories.tsx index 8228e3b1bd..b7ba6647bf 100644 --- a/frontend/editor/src/portal/components/account-link/LinkAccountModal.stories.tsx +++ b/frontend/editor/src/portal/components/account-link/LinkAccountModal.stories.tsx @@ -8,16 +8,19 @@ const meta: Meta = { args: { open: true, onClose: () => {}, - onLinked: async () => {}, }, }; export default meta; type Story = StoryObj; -/** Default "link" mode — sign in to register this instance against a Stirling account. */ +/** + * "link" mode — explains the trip to Stirling and starts the handshake. There is no + * sign-in form: a sign-in started on a self-hosted origin cannot complete, because + * the provider will not redirect back to a hostname it does not know. + */ export const Default: Story = {}; -/** "reauth" mode — an already-linked instance's session expired and needs a fresh sign-in. */ +/** "reauth" mode — the server stays linked; only the browser session is renewed. */ export const Reauth: Story = { args: { mode: "reauth" }, }; diff --git a/frontend/editor/src/portal/components/account-link/LinkAccountModal.test.tsx b/frontend/editor/src/portal/components/account-link/LinkAccountModal.test.tsx new file mode 100644 index 0000000000..018d435b31 --- /dev/null +++ b/frontend/editor/src/portal/components/account-link/LinkAccountModal.test.tsx @@ -0,0 +1,128 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { act, render, waitFor } from "@testing-library/react"; +import { MantineProvider } from "@mantine/core"; + +/** The modal every "link account" CTA in the portal opens. */ +const { startConnect, startReauth } = vi.hoisted(() => ({ + startConnect: vi.fn(), + startReauth: vi.fn(), +})); + +vi.mock("@portal/api/link", () => ({ startConnect, startReauth })); +vi.mock("@portal/auth/saasSupabase", () => ({ + isSaasSupabaseConfigured: true, +})); + +import { LinkAccountModal } from "@portal/components/account-link/LinkAccountModal"; + +const AUTHORIZE = "http://localhost:5174/link?request=req-1"; + +function renderModal(mode?: "link" | "reauth") { + return render( + + {}} mode={mode} /> + , + ); +} + +/** Clicks the primary action (the secondary one is Cancel). */ +function clickContinue(getAllByRole: (role: string) => HTMLElement[]) { + const buttons = getAllByRole("button"); + act(() => buttons[buttons.length - 1].click()); +} + +describe("LinkAccountModal", () => { + let assign: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + startConnect.mockResolvedValue({ + phase: "PENDING", + authorizeUrl: AUTHORIZE, + secondsRemaining: 900, + teamId: null, + }); + startReauth.mockResolvedValue({ + phase: "PENDING", + authorizeUrl: AUTHORIZE, + secondsRemaining: 900, + teamId: null, + }); + assign = vi.fn(); + Object.defineProperty(window, "location", { + configurable: true, + value: { + origin: "http://localhost:5173", + hostname: "localhost", + assign, + }, + }); + }); + + it("offers no sign-in form, because a sign-in started here cannot complete", () => { + const { container } = renderModal(); + + // The provider buttons this modal used to carry sent the admin to Stirling and + // abandoned them there. Nothing should collect credentials on this origin. + expect(container.querySelector("input[type=password]")).toBeNull(); + expect(container.querySelector("input[type=email]")).toBeNull(); + }); + + it("starts a link handshake and hands the browser to Stirling", async () => { + const { getAllByRole } = renderModal(); + + clickContinue(getAllByRole); + + await waitFor(() => expect(startConnect).toHaveBeenCalled()); + // Callback built from this page's own origin, which the backend then checks + // against the request's Origin header. + expect(startConnect).toHaveBeenCalledWith( + "localhost", + "http://localhost:5173/account-link/callback", + ); + await waitFor(() => expect(assign).toHaveBeenCalledWith(AUTHORIZE)); + expect(startReauth).not.toHaveBeenCalled(); + }); + + it("uses the reauth endpoint when only the session needs renewing", async () => { + const { getAllByRole } = renderModal("reauth"); + + clickContinue(getAllByRole); + + // A different endpoint on purpose: reauth presents the device credential so + // Stirling pins the handshake to the team that already owns this server. + await waitFor(() => + expect(startReauth).toHaveBeenCalledWith( + "http://localhost:5173/account-link/callback", + ), + ); + expect(startConnect).not.toHaveBeenCalled(); + await waitFor(() => expect(assign).toHaveBeenCalledWith(AUTHORIZE)); + }); + + it("stays put and explains itself when the handshake cannot start", async () => { + startConnect.mockRejectedValue(new Error("offline")); + + const { getAllByRole } = renderModal(); + clickContinue(getAllByRole); + + await waitFor(() => expect(startConnect).toHaveBeenCalled()); + expect(assign).not.toHaveBeenCalled(); + }); + + it("does not navigate when there is nothing to navigate to", async () => { + // Already linked: the backend reports status without an authorize URL. + startConnect.mockResolvedValue({ + phase: "LINKED", + authorizeUrl: null, + secondsRemaining: null, + teamId: 7, + }); + + const { getAllByRole } = renderModal(); + clickContinue(getAllByRole); + + await waitFor(() => expect(startConnect).toHaveBeenCalled()); + expect(assign).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/editor/src/portal/components/account-link/LinkAccountModal.tsx b/frontend/editor/src/portal/components/account-link/LinkAccountModal.tsx index e46504d309..8bd47a9eb4 100644 --- a/frontend/editor/src/portal/components/account-link/LinkAccountModal.tsx +++ b/frontend/editor/src/portal/components/account-link/LinkAccountModal.tsx @@ -1,63 +1,60 @@ -import { useEffect } from "react"; +import { useCallback, useState } from "react"; import { useTranslation } from "react-i18next"; import { Banner, Button, Modal } from "@app/ui"; -import SupabaseLoginForm from "@app/auth/ui/SupabaseLoginForm"; -import { - useSupabaseLogin, - type SupabaseLoginSession, -} from "@app/auth/ui/useSupabaseLogin"; -import "@app/auth/ui/auth-theme.css"; -import { - ensureSaasSupabase, - isSaasSupabaseConfigured, - PENDING_LINK_KEY, - SAAS_OAUTH_PROVIDERS, -} from "@portal/auth/saasSupabase"; +import { withBasePath } from "@app/constants/app"; +import { startConnect, startReauth } from "@portal/api/link"; +import { isSaasSupabaseConfigured } from "@portal/auth/saasSupabase"; +import "@portal/components/account-link/LinkAccountModal.css"; interface Props { open: boolean; onClose: () => void; /** - * "link" registers this instance against the signed-in account; "reauth" only - * refreshes an expired SaaS session (the instance is already linked). The mode - * is persisted across the OAuth redirect so the SSO-return handler doesn't - * re-register on a reauth. + * "link" connects this server to a team for the first time; "reauth" only re-establishes the browser's Stirling session for a server that is already linked. */ mode?: "link" | "reauth"; - /** Called with the SaaS session after a successful sign-in. */ - onLinked: (session: SupabaseLoginSession) => void | Promise; } -/** - * In-app account-link login. Signs the admin in to their Stirling (SaaS) account - * via the shared Supabase login (SSO + email/password), then hands the resulting - * session to the caller to register this instance. No popup; the device secret - * never reaches the browser. SSO redirects away and is finished by useAccountLink - * on return. - */ -export function LinkAccountModal({ - open, - onClose, - mode = "link", - onLinked, -}: Props) { +/** Sends the admin off to Stirling to connect this server. */ +export function LinkAccountModal({ open, onClose, mode = "link" }: Props) { const { t } = useTranslation(); - useEffect(() => { - if (open) ensureSaasSupabase(); - }, [open]); - const reauth = mode === "reauth"; - const login = useSupabaseLogin({ - providers: SAAS_OAUTH_PROVIDERS, - // Return to the current page after SSO; the SSO-return handler in - // useAccountLink reads the persisted mode so it links vs. only refreshes. - redirectTo: window.location.href, - onBeforeOAuth: () => sessionStorage.setItem(PENDING_LINK_KEY, mode), - onSuccess: async (session) => { - await onLinked(session); - onClose(); - }, - }); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + + const begin = useCallback(async () => { + setBusy(true); + setError(null); + try { + const callbackUrl = new URL( + withBasePath("/account-link/callback"), + window.location.origin, + ).toString(); + const status = reauth + ? await startReauth(callbackUrl) + : await startConnect(window.location.hostname, callbackUrl); + if (status.authorizeUrl) { + window.location.assign(status.authorizeUrl); + return; + } + // Already linked, or a handshake we cannot act on. Nothing to navigate to. + setError( + t( + "portal.accountLink.modal.noAuthorizeUrl", + "Stirling did not return somewhere to continue. Try again in a moment.", + ), + ); + } catch { + setError( + t( + "portal.accountLink.modal.startFailed", + "Could not reach Stirling to start the connection. Check this server's outbound network access, then try again.", + ), + ); + } finally { + setBusy(false); + } + }, [reauth, t]); return ( - {isSaasSupabaseConfigured ? ( - - ) : ( -
+
+
    +
  1. + {t( + "portal.accountLink.modal.step1", + "We send you to stirling.com to sign in. Any sign-in method works there, including Google and single sign-on.", + )} +
  2. +
  3. + {t( + "portal.accountLink.modal.step2", + "You check this server's address and approve it. A team owner has to do this the first time.", + )} +
  4. +
  5. + {t( + "portal.accountLink.modal.step3", + "Stirling brings you straight back here and finishes up.", + )} +
  6. +
+ + {!isSaasSupabaseConfigured && ( {t("portal.accountLink.modal.loginNotConfigured.before", "Set")}{" "} @@ -101,25 +117,27 @@ export function LinkAccountModal({ VITE_SUPABASE_PUBLISHABLE_DEFAULT_KEY{" "} {t( "portal.accountLink.modal.loginNotConfigured.after", - "to enable in-app linking against the hosted Stirling account.", + "so this server can finish the connection when you come back.", )} - {import.meta.env.DEV && ( - - )} + )} + + {error && {error}} + +
+ +
- )} +
); } diff --git a/frontend/editor/src/portal/contexts/AccountLinkContext.tsx b/frontend/editor/src/portal/contexts/AccountLinkContext.tsx index d3f4bf3678..0ab5fc4da7 100644 --- a/frontend/editor/src/portal/contexts/AccountLinkContext.tsx +++ b/frontend/editor/src/portal/contexts/AccountLinkContext.tsx @@ -5,15 +5,7 @@ import { } from "@portal/hooks/useAccountLink"; /** - * Single app-wide {@link useAccountLink} instance. The link flow is orchestrated - * in exactly one place so that: - * - status is fetched once on mount (not per consumer), and - * - the SSO-return effect fires once — two instances would both call - * {@link UseAccountLink.completeLink} on return and re-register the device - * credential, leaving a duplicate linked_instance row. - * - * Consumers (the top-level link modal host, the Settings account-link panel, - * the link card) read this shared instance instead of calling the hook again. + * Single app-wide {@link useAccountLink} instance, so status is fetched once on mount rather than per consumer. */ const AccountLinkContext = createContext(null); diff --git a/frontend/editor/src/portal/contexts/LinkContext.tsx b/frontend/editor/src/portal/contexts/LinkContext.tsx index d088647586..7fd7cfe536 100644 --- a/frontend/editor/src/portal/contexts/LinkContext.tsx +++ b/frontend/editor/src/portal/contexts/LinkContext.tsx @@ -8,7 +8,7 @@ import { } from "react"; /** - * The "linked" dimension of the account-link surface (combined-billing "Mode A"), + * The "linked" dimension of the account-link surface (combined billing), * a sibling to TierContext. It answers one question the rest of the portal asks: * has this self-hosted org linked its SaaS account, and if so, is it on the free * grant or actively subscribed? @@ -59,13 +59,6 @@ interface LinkContextValue { isLinked: boolean; /** Convenience for `LINK_INFO[linkState].unlocked` — billable features usable. */ featuresUnlocked: boolean; - /** - * Bumps whenever the browser's SaaS session changes (e.g. a re-sign-in after - * expiry). Attended SaaS reads (the wallet) key off this to refetch with the - * fresh token without re-establishing the instance link. - */ - saasSessionNonce: number; - markSaasSessionChanged: () => void; } const LinkContext = createContext(null); @@ -78,11 +71,6 @@ export function LinkProvider({ initialState?: LinkState; }) { const [linkState, setLinkState] = useState(initialState); - const [saasSessionNonce, setSaasSessionNonce] = useState(0); - const markSaasSessionChanged = useCallback( - () => setSaasSessionNonce((n) => n + 1), - [], - ); const value = useMemo(() => { const unlocked = LINK_INFO[linkState].unlocked; return { @@ -90,10 +78,8 @@ export function LinkProvider({ setLinkState, isLinked: linkState !== "unlinked", featuresUnlocked: unlocked, - saasSessionNonce, - markSaasSessionChanged, }; - }, [linkState, saasSessionNonce, markSaasSessionChanged]); + }, [linkState]); return {children}; } diff --git a/frontend/editor/src/portal/hooks/useAccountLink.test.tsx b/frontend/editor/src/portal/hooks/useAccountLink.test.tsx deleted file mode 100644 index b327322778..0000000000 --- a/frontend/editor/src/portal/hooks/useAccountLink.test.tsx +++ /dev/null @@ -1,75 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { act, render, waitFor } from "@testing-library/react"; -import { LinkProvider } from "@portal/contexts/LinkContext"; - -/** - * The SSO-return path is mode-aware: a "reauth" return must only refresh the - * session, NOT re-register the instance (re-registering mints a duplicate device - * credential). This is the exact regression that slipped through once, so it gets - * a dedicated guard. - */ -const { linkInstance, fetchStatus, unlinkInstance, getSession } = vi.hoisted( - () => ({ - linkInstance: vi.fn(), - fetchStatus: vi.fn(), - unlinkInstance: vi.fn(), - getSession: vi.fn(), - }), -); - -vi.mock("@portal/api/link", () => ({ - linkInstance, - fetchStatus, - unlinkInstance, -})); -vi.mock("@portal/auth/saasSupabase", () => ({ - PENDING_LINK_KEY: "stirling_pending_link", - isSaasSupabaseConfigured: true, - SAAS_OAUTH_PROVIDERS: [], - ensureSaasSupabase: () => ({ auth: { getSession } }), -})); - -import { useAccountLink } from "@portal/hooks/useAccountLink"; -import { PENDING_LINK_KEY } from "@portal/auth/saasSupabase"; - -function Probe() { - useAccountLink(); - return null; -} - -const renderHook = () => - render( - - - , - ); - -beforeEach(() => { - linkInstance.mockReset().mockResolvedValue({ linked: true, name: null }); - fetchStatus.mockReset().mockResolvedValue({ linked: true, name: null }); - unlinkInstance.mockReset(); - getSession.mockReset().mockResolvedValue({ - data: { session: { access_token: "tok" } }, - }); - sessionStorage.clear(); -}); -afterEach(() => sessionStorage.clear()); - -describe("useAccountLink — SSO return", () => { - it("reauth mode refreshes the session WITHOUT re-registering", async () => { - sessionStorage.setItem(PENDING_LINK_KEY, "reauth"); - renderHook(); - await act(async () => { - await Promise.resolve(); - await Promise.resolve(); - }); - expect(linkInstance).not.toHaveBeenCalled(); - }); - - it("link mode registers the instance with the returned token", async () => { - sessionStorage.setItem(PENDING_LINK_KEY, "link"); - renderHook(); - await waitFor(() => expect(linkInstance).toHaveBeenCalledTimes(1)); - expect(linkInstance.mock.calls[0][0].supabaseJwt).toBe("tok"); - }); -}); diff --git a/frontend/editor/src/portal/hooks/useAccountLink.ts b/frontend/editor/src/portal/hooks/useAccountLink.ts index dd4894f6d6..3e47df5220 100644 --- a/frontend/editor/src/portal/hooks/useAccountLink.ts +++ b/frontend/editor/src/portal/hooks/useAccountLink.ts @@ -1,33 +1,9 @@ import { useCallback, useEffect, useState } from "react"; -import type { SupabaseLoginSession } from "@app/auth/ui/useSupabaseLogin"; -import { - ensureSaasSupabase, - isSaasSupabaseConfigured, - PENDING_LINK_KEY, -} from "@portal/auth/saasSupabase"; -import { - fetchStatus, - linkInstance, - unlinkInstance, - type LinkStatus, -} from "@portal/api/link"; -import { useApplyLinkFacts, useLink } from "@portal/contexts/LinkContext"; +import { isSaasSupabaseConfigured } from "@portal/auth/saasSupabase"; +import { fetchStatus, unlinkInstance, type LinkStatus } from "@portal/api/link"; +import { useApplyLinkFacts } from "@portal/contexts/LinkContext"; -/** - * Orchestrates the account-link flow for THIS instance: - * - * 1. The admin signs in to their Stirling account IN-APP (LinkAccountModal → - * shared Supabase login), minting a short-term SaaS JWT. - * 2. {@link completeLink} POSTs that JWT to the LOCAL backend (api/link.ts), - * which registers with SaaS and stores the device secret server-side. - * 3. The resulting Linked / Not-linked status is read back. - * - * Email/password resolves inline (the modal calls completeLink). SSO redirects - * the browser to the provider and back; the returned session is finished here on - * mount (see the pending-link effect). The device secret is never received or - * rendered. Subscription state is resolved separately from the wallet, so a fresh - * link marks the org linked-free. - */ +/** Reads and clears THIS instance's link status. */ export type LinkPhase = "idle" | "linking" | "error"; @@ -38,84 +14,32 @@ export interface UseAccountLink { status: LinkStatus | null; phase: LinkPhase; error: string | null; - /** Finish linking THIS instance with a SaaS session minted by the login modal. */ - completeLink: (session: SupabaseLoginSession, name?: string) => Promise; /** Unlink this instance. */ unlink: () => Promise; + /** Re-read the status, for when something outside this hook changed it. */ + refresh: () => Promise; } export function useAccountLink(): UseAccountLink { const applyLinkFacts = useApplyLinkFacts(); - const { markSaasSessionChanged } = useLink(); const [status, setStatus] = useState(null); const [phase, setPhase] = useState("idle"); const [error, setError] = useState(null); - const completeLink = useCallback( - async (session: SupabaseLoginSession, name?: string) => { - setPhase("linking"); - setError(null); - try { - const next = await linkInstance({ - supabaseJwt: session.access_token, - name, - }); - setStatus(next); - setPhase("idle"); - if (next.linked) applyLinkFacts(true, false); - } catch (e) { - setError(e instanceof Error ? e.message : String(e)); - setPhase("error"); - } - }, - [applyLinkFacts], - ); - - // Read the current link status on mount. - useEffect(() => { - let cancelled = false; - void fetchStatus() - .then((s) => { - if (!cancelled) { - setStatus(s); - // A linked instance is at least linked-free; subscription comes from the wallet. - if (s.linked) applyLinkFacts(true, false); - } - }) - .catch(() => { - // Status endpoint absent (flag off) / unreachable → leave status null, - // which renders as "Not linked". Don't surface an error or leak an - // unhandled rejection for the expected flag-off case. - if (!cancelled) setStatus({ linked: false, name: null }); - }); - return () => { - cancelled = true; - }; + const refresh = useCallback(async () => { + try { + const s = await fetchStatus(); + setStatus(s); + // A linked instance is at least linked-free; subscription comes from the wallet. + if (s.linked) applyLinkFacts(true, false); + } catch { + setStatus({ linked: false, name: null }); + } }, [applyLinkFacts]); - // SSO return: an SSO sign-in we kicked off has redirected back and the SaaS - // session is now in the shared Supabase client. The pending marker carries the - // mode: "reauth" only refreshes attended reads (the instance is already linked - // — re-registering would mint a duplicate credential); anything else links. useEffect(() => { - const supabase = ensureSaasSupabase(); - const pending = sessionStorage.getItem(PENDING_LINK_KEY); - if (!supabase || pending === null) return; - let cancelled = false; - void supabase.auth.getSession().then(({ data }) => { - sessionStorage.removeItem(PENDING_LINK_KEY); - const token = data.session?.access_token; - if (!token || cancelled) return; - if (pending === "reauth") { - markSaasSessionChanged(); - } else { - void completeLink({ access_token: token }); - } - }); - return () => { - cancelled = true; - }; - }, [completeLink, markSaasSessionChanged]); + void refresh(); + }, [refresh]); const unlink = useCallback(async () => { setPhase("linking"); @@ -136,7 +60,7 @@ export function useAccountLink(): UseAccountLink { status, phase, error, - completeLink, unlink, + refresh, }; } diff --git a/frontend/editor/src/portal/mocks/handlers/link.ts b/frontend/editor/src/portal/mocks/handlers/link.ts index 1724ce375f..bd7e6844f2 100644 --- a/frontend/editor/src/portal/mocks/handlers/link.ts +++ b/frontend/editor/src/portal/mocks/handlers/link.ts @@ -1,5 +1,4 @@ import { http, HttpResponse, delay } from "msw"; -import type { LinkInstanceRequest } from "@portal/api/link"; import { getLocalStatus, getLocalUsage, @@ -12,14 +11,13 @@ import { /** * Account-link MSW handlers. Two surfaces: * - * - LOCAL backend (this instance): link / status / unlink. `link` mutates the - * in-memory store and flips local status so the surface behaves like a real - * backend within a session. The device secret stays server-side — never - * returned over the wire, matching the real contract. + * - LOCAL backend (this instance): the connect handshake, status and unlink. + * `connect/complete` mutates the in-memory store and flips local status so the + * surface behaves like a real backend within a session. The device secret stays + * server-side — never returned over the wire, matching the real contract. * - SaaS backend (team-wide): instances / revoke. * - * Mirrors the real AccountLinkController paths so MSW can be dropped with no code - * change. + * Mirrors the real controller paths so MSW can be dropped with no code change. */ export const linkHandlers = [ http.get("/api/v1/account-link/status", async () => { @@ -27,15 +25,38 @@ export const linkHandlers = [ return HttpResponse.json(getLocalStatus()); }), - http.post("/api/v1/account-link/link", async ({ request }) => { + // Opening a handshake hands back where to send the admin. The real backend gets + // that URL from SaaS rather than composing it, so the mock returns one too. + http.post("*/api/v1/account-link/connect/start", async () => { await delay(120); - let name: string | undefined; - try { - name = ((await request.json()) as LinkInstanceRequest)?.name; - } catch { - // empty body — name stays undefined - } - return HttpResponse.json(linkLocal(name), { status: 201 }); + return HttpResponse.json({ + phase: "PENDING", + authorizeUrl: "https://app.stirling.test/link?request=mock-request", + secondsRemaining: 900, + teamId: null, + }); + }), + + http.post("*/api/v1/account-link/connect/reauth", async () => { + await delay(120); + return HttpResponse.json({ + phase: "PENDING", + authorizeUrl: "https://app.stirling.test/link?request=mock-reauth", + secondsRemaining: 900, + teamId: null, + }); + }), + + // The callback's completion step. Flips the store to linked, as a real claim would. + http.post("*/api/v1/account-link/connect/complete", async () => { + await delay(120); + linkLocal("mock-server"); + return HttpResponse.json({ + phase: "LINKED", + authorizeUrl: null, + secondsRemaining: null, + teamId: 7, + }); }), http.get("/api/v1/account-link/usage", async () => { diff --git a/frontend/editor/src/portal/mocks/link.ts b/frontend/editor/src/portal/mocks/link.ts index 92b4b90f2c..eb29ef5ba4 100644 --- a/frontend/editor/src/portal/mocks/link.ts +++ b/frontend/editor/src/portal/mocks/link.ts @@ -2,7 +2,7 @@ * Account-link fixtures. Types live in api/link.ts (the backend contract); * this module only builds fake data for Storybook and tests. * - * "Mode A" combined billing: a self-hosted instance links the org's SaaS account + * Combined billing: a self-hosted instance links the org's SaaS account * so its unattended calls bill against the org wallet. Two surfaces: * * - THIS instance: the local backend (`POST /api/v1/account-link/link`, diff --git a/frontend/editor/src/portal/views/ConnectCallback.css b/frontend/editor/src/portal/views/ConnectCallback.css new file mode 100644 index 0000000000..35e9b09ef7 --- /dev/null +++ b/frontend/editor/src/portal/views/ConnectCallback.css @@ -0,0 +1,32 @@ +/* Account-link callback. A transient page the admin passes through, so it is + centred and says one thing rather than trying to be a settings screen. */ + +.portal-connect-callback { + display: flex; + flex-direction: column; + align-items: center; + gap: 1rem; + max-width: 30rem; + margin: 4rem auto; + padding: 0 1rem; + text-align: center; +} + +.portal-connect-callback > * { + width: 100%; +} + +/* The button is the one thing that should not stretch to the banner's width. */ +.portal-connect-callback button { + width: auto; +} + +.portal-connect-callback p { + margin: 0; + font-size: 0.875rem; + color: var(--c-text-muted); +} + +.portal-connect-callback__note { + font-size: 0.8125rem; +} diff --git a/frontend/editor/src/portal/views/ConnectCallback.test.tsx b/frontend/editor/src/portal/views/ConnectCallback.test.tsx new file mode 100644 index 0000000000..bdadee03e4 --- /dev/null +++ b/frontend/editor/src/portal/views/ConnectCallback.test.tsx @@ -0,0 +1,175 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { act, render, waitFor } from "@testing-library/react"; +import { MemoryRouter, Route, Routes } from "react-router-dom"; +import { MantineProvider } from "@mantine/core"; + +/** + * The callback handles a live session token in a URL fragment, so the behaviour worth pinning is what it does with it: strip it immediately, refuse anything it cannot verify, and keep the two outcomes (SaaS sign-in, server link) independent of each other. + */ +const { completeConnect, startConnect, setSession, refresh } = vi.hoisted( + () => ({ + completeConnect: vi.fn(), + startConnect: vi.fn(), + setSession: vi.fn(), + refresh: vi.fn(), + }), +); + +vi.mock("@portal/api/link", () => ({ completeConnect, startConnect })); +vi.mock("@portal/auth/saasSupabase", () => ({ + ensureSaasSupabase: () => ({ auth: { setSession } }), +})); +vi.mock("@portal/contexts/AccountLinkContext", () => ({ + useAccountLinkContext: () => ({ refresh }), +})); + +import ConnectCallback from "@portal/views/ConnectCallback"; +import { ConnectCallbackHost } from "@portal/components/account-link/ConnectCallbackHost"; + +const NONCE = "the-nonce"; + +function landOn(fragment: string) { + window.history.replaceState(null, "", `/account-link/callback${fragment}`); +} + +/** + * Route and host together: the route reads the fragment, the portal renders the + * outcome. Exercising them apart would test the hand-off rather than the flow. + */ +function renderFlow() { + return render( + + + + + } /> + } /> + + + , + ); +} + +describe("account-link callback", () => { + beforeEach(() => { + vi.clearAllMocks(); + completeConnect.mockResolvedValue({ + phase: "LINKED", + authorizeUrl: null, + secondsRemaining: null, + teamId: 7, + }); + setSession.mockResolvedValue({ error: null }); + }); + + it("removes the token-bearing fragment from the URL", async () => { + landOn(`#type=link&nonce=${NONCE}&access_token=at&refresh_token=rt`); + + renderFlow(); + + // Synchronous, before any await: the fragment must not survive long enough + // to be read from the address bar or land in a history entry. + expect(window.location.hash).toBe(""); + await waitFor(() => expect(completeConnect).toHaveBeenCalled()); + }); + + it("lands on the portal rather than leaving the result on a bare page", async () => { + landOn(`#type=link&nonce=${NONCE}&access_token=at&refresh_token=rt`); + + const { getByTestId } = renderFlow(); + + await waitFor(() => expect(getByTestId("portal")).toBeTruthy()); + }); + + it("re-reads the link status, so the page behind agrees with the modal", async () => { + landOn(`#type=link&nonce=${NONCE}`); + + renderFlow(); + + await waitFor(() => expect(refresh).toHaveBeenCalled()); + }); + + it("deposits the session and then finishes the link with the nonce", async () => { + landOn(`#type=link&nonce=${NONCE}&access_token=at&refresh_token=rt`); + + renderFlow(); + + await waitFor(() => + expect(setSession).toHaveBeenCalledWith({ + access_token: "at", + refresh_token: "rt", + }), + ); + await waitFor(() => expect(completeConnect).toHaveBeenCalledWith(NONCE)); + }); + + it("finishes the link even when the session hand-off fails", async () => { + landOn(`#type=link&nonce=${NONCE}&access_token=at&refresh_token=rt`); + setSession.mockRejectedValue(new Error("nope")); + + renderFlow(); + + // The two outcomes are independent: a failed sign-in must not strand the + // server unlinked. + await waitFor(() => expect(completeConnect).toHaveBeenCalledWith(NONCE)); + }); + + it("links without a session when the fragment carries no tokens", async () => { + landOn(`#type=link&nonce=${NONCE}`); + + renderFlow(); + + await waitFor(() => expect(completeConnect).toHaveBeenCalledWith(NONCE)); + expect(setSession).not.toHaveBeenCalled(); + }); + + it("refuses a fragment with no nonce", async () => { + landOn("#type=link&access_token=at&refresh_token=rt"); + + renderFlow(); + + await waitFor(() => expect(window.location.hash).toBe("")); + expect(completeConnect).not.toHaveBeenCalled(); + expect(setSession).not.toHaveBeenCalled(); + }); + + it("refuses a fragment that is not a link response", async () => { + landOn(`#type=something-else&nonce=${NONCE}&access_token=at`); + + renderFlow(); + + await waitFor(() => expect(window.location.hash).toBe("")); + expect(completeConnect).not.toHaveBeenCalled(); + }); + + it("refuses a bare page load", async () => { + landOn(""); + + renderFlow(); + + expect(completeConnect).not.toHaveBeenCalled(); + expect(setSession).not.toHaveBeenCalled(); + }); + + it("offers a retry rather than a restart while the handshake is still open", async () => { + landOn(`#type=link&nonce=${NONCE}`); + completeConnect.mockResolvedValue({ + phase: "UNAVAILABLE", + authorizeUrl: null, + secondsRemaining: null, + teamId: null, + }); + + const { getAllByRole } = renderFlow(); + + await waitFor(() => expect(completeConnect).toHaveBeenCalledTimes(1)); + // Last button, not the only one: the modal shell contributes a close button. + const buttons = getAllByRole("button"); + act(() => buttons[buttons.length - 1].click()); + + // Retries the existing handshake; starting a new one would waste the + // approval a human just gave. + await waitFor(() => expect(completeConnect).toHaveBeenCalledTimes(2)); + expect(startConnect).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/editor/src/portal/views/ConnectCallback.tsx b/frontend/editor/src/portal/views/ConnectCallback.tsx new file mode 100644 index 0000000000..2fecd5244e --- /dev/null +++ b/frontend/editor/src/portal/views/ConnectCallback.tsx @@ -0,0 +1,41 @@ +import { useEffect, useRef } from "react"; +import { useNavigate } from "react-router-dom"; +import { PORTAL_BASENAME } from "@app/routes/portalBasename"; +import type { AccountLinkReturn } from "@portal/components/account-link/ConnectCallbackHost"; + +/** + * Return leg of the account-link handshake. Stirling redirects here with the + * admin's session in the URL fragment. + * + * This route only reads the fragment and hands it to the portal, which owns the + * rest. Rendering the outcome here would put it on an empty page; the portal is + * where the admin started, so that is where the result belongs. + */ +export default function ConnectCallback() { + const navigate = useNavigate(); + const startedRef = useRef(false); + + useEffect(() => { + if (startedRef.current) return; + startedRef.current = true; + + const params = new URLSearchParams(window.location.hash.replace(/^#/, "")); + // Before anything else: the fragment carries a live session token. + window.history.replaceState( + null, + "", + `${window.location.pathname}${window.location.search}`, + ); + + const accountLinkReturn: AccountLinkReturn = { + type: params.get("type"), + nonce: params.get("nonce"), + accessToken: params.get("access_token"), + refreshToken: params.get("refresh_token"), + }; + // Router state, not the URL: the tokens are live and must not be re-shareable. + navigate(PORTAL_BASENAME, { replace: true, state: { accountLinkReturn } }); + }, [navigate]); + + return null; +} diff --git a/frontend/editor/src/proprietary/auth/spring/UseSession.tsx b/frontend/editor/src/proprietary/auth/spring/UseSession.tsx index 289f2ec895..27a722b2d7 100644 --- a/frontend/editor/src/proprietary/auth/spring/UseSession.tsx +++ b/frontend/editor/src/proprietary/auth/spring/UseSession.tsx @@ -12,6 +12,7 @@ import { type AuthUser, type AuthTranslate, } from "@app/auth/types"; +import { suspendWorkbenchSession } from "@app/services/workbenchSession"; /** * Strip the configured base path so route comparisons work under subpath @@ -97,6 +98,11 @@ export function SpringAuthProvider({ const signOut = useCallback(async () => { try { setError(null); + // Signing out is deliberate, unlike an identity check that merely failed: drop the + // workbench record here and stop recording, so the teardown that follows cannot + // write it back for whoever signs in next. + suspendWorkbenchSession(); + const { error } = await springAuth.signOut(); // Always clear the in-memory session: springAuth.signOut() removes the diff --git a/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.reentry.test.tsx b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.reentry.test.tsx new file mode 100644 index 0000000000..dc8ff6397d --- /dev/null +++ b/frontend/editor/src/proprietary/components/policies/usePolicyAutoRun.reentry.test.tsx @@ -0,0 +1,119 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; + +// A file can re-enter the workbench without being a new upload (My Files reopen, session restore). +// The persisted dispatch record must stop the upload policy (and its billing) firing a second time. + +const mocks = vi.hoisted(() => ({ + workspace: [] as Array<{ id: string; derivedFromTool?: boolean }>, + runStoredPolicy: vi.fn(), + getPolicyRun: vi.fn(), + listPolicyRuns: vi.fn(), + getStirlingFile: vi.fn(), +})); + +vi.mock("@app/hooks/useAiEngineEnabled", () => ({ + useAiEngineEnabled: () => true, +})); +vi.mock("@app/contexts/FileContext", () => ({ + useAllFiles: () => ({ fileStubs: mocks.workspace }), + useFileManagement: () => ({ + addFiles: vi.fn(), + updateStirlingFileStub: vi.fn(), + }), + useFileContext: () => ({ consumeFiles: vi.fn() }), +})); +vi.mock("@app/contexts/IndexedDBContext", () => ({ + useIndexedDB: () => ({ bumpRevision: vi.fn() }), +})); +vi.mock("@app/hooks/usePolicies", () => ({ + usePolicies: () => ({ + policies: { + security: { + configured: true, + status: "active", + backendId: "backend-security", + runOn: "upload", + order: 0, + outputMode: "new_version", + outputName: "", + }, + }, + }), +})); +vi.mock("@app/services/policyApi", () => ({ + runStoredPolicy: mocks.runStoredPolicy, + getPolicyRun: mocks.getPolicyRun, + listPolicyRuns: mocks.listPolicyRuns, + downloadPolicyOutput: vi.fn(), + resolvePolicyRunTarget: () => "saas", +})); +vi.mock("@app/services/fileStorage", () => ({ + fileStorage: { + getStirlingFile: mocks.getStirlingFile, + getStirlingFileStub: vi.fn().mockResolvedValue(null), + persistVersionedOutputs: vi.fn(), + updateFileMetadata: vi.fn().mockResolvedValue(true), + }, +})); + +import { usePolicyAutoRun } from "@app/components/policies/usePolicyAutoRun"; +import { + markDispatched, + resetPolicyRuns, +} from "@app/components/policies/policyRunStore"; + +beforeEach(() => { + localStorage.clear(); + resetPolicyRuns(); + vi.clearAllMocks(); + mocks.listPolicyRuns.mockResolvedValue([]); + mocks.getStirlingFile.mockResolvedValue( + new File(["x"], "doc.pdf", { type: "application/pdf" }), + ); + mocks.runStoredPolicy.mockResolvedValue("run-0"); + // Completed with no outputs: the run settles without the import machinery. + mocks.getPolicyRun.mockResolvedValue({ + runId: "run-0", + policyId: null, + status: "COMPLETED", + currentStep: 1, + stepCount: 1, + error: null, + outputs: [], + }); +}); + +describe("upload policies and files re-entering the workbench", () => { + it("does not re-run on a file the policy already ran on", async () => { + markDispatched("security", "already-enforced"); + mocks.workspace = [{ id: "already-enforced" }, { id: "fresh-upload" }]; + + renderHook(() => usePolicyAutoRun()); + + await waitFor(() => expect(mocks.runStoredPolicy).toHaveBeenCalledTimes(1)); + expect(mocks.getStirlingFile).toHaveBeenCalledWith("fresh-upload"); + expect(mocks.getStirlingFile).not.toHaveBeenCalledWith("already-enforced"); + }); + + it("stays silent when every file in the workbench has already been enforced", async () => { + markDispatched("security", "one"); + markDispatched("security", "two"); + mocks.workspace = [{ id: "one" }, { id: "two" }]; + + renderHook(() => usePolicyAutoRun()); + + // Give the dispatch effect a tick to (wrongly) fire before asserting silence. + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(mocks.runStoredPolicy).not.toHaveBeenCalled(); + }); + + it("still skips a policy's own output, which is not an upload at all", async () => { + mocks.workspace = [{ id: "policy-output", derivedFromTool: true }]; + + renderHook(() => usePolicyAutoRun()); + + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(mocks.runStoredPolicy).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/editor/src/proprietary/constants/featureFlags.ts b/frontend/editor/src/proprietary/constants/featureFlags.ts index e432b34599..84af388005 100644 --- a/frontend/editor/src/proprietary/constants/featureFlags.ts +++ b/frontend/editor/src/proprietary/constants/featureFlags.ts @@ -13,3 +13,6 @@ * Watched Folders implementation to navigate to). */ export const WATCHED_FOLDERS_ENABLED: boolean = false; + +// Refill an empty workbench from the tab's last session (survives the editor/processor switch). +export const WORKBENCH_SESSION_RESTORE: boolean = true; diff --git a/frontend/editor/src/proprietary/extensions/accountLogout.ts b/frontend/editor/src/proprietary/extensions/accountLogout.ts index 6ae83b8e26..8c4d9f15d7 100644 --- a/frontend/editor/src/proprietary/extensions/accountLogout.ts +++ b/frontend/editor/src/proprietary/extensions/accountLogout.ts @@ -1,3 +1,5 @@ +import { suspendWorkbenchSession } from "@app/services/workbenchSession"; + type SignOutFn = () => Promise; interface AccountLogoutDeps { @@ -21,6 +23,10 @@ export function useAccountLogout() { "1", ); } + // The tab outlives the session; the next person to sign in here must not + // inherit this workbench. Suspends writing too - signing out unmounts the + // editor, and its flush would otherwise write the record straight back. + suspendWorkbenchSession(); await signOut(); } finally { redirectToLogin(); diff --git a/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.test.tsx b/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.test.tsx new file mode 100644 index 0000000000..c2a81c53d9 --- /dev/null +++ b/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.test.tsx @@ -0,0 +1,55 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook } from "@testing-library/react"; + +const mocks = vi.hoisted(() => ({ + navigate: vi.fn(), + requestNavigation: vi.fn(), + portalAccess: true, +})); + +vi.mock("react-router-dom", () => ({ + useNavigate: () => mocks.navigate, + useLocation: () => ({ pathname: "/compress", search: "?mode=fast" }), +})); +vi.mock("@app/auth/context", () => ({ + useAuth: () => ({ portalAccess: mocks.portalAccess }), +})); +vi.mock("@app/contexts/NavigationContext", () => ({ + useNavigationActions: () => ({ + actions: { requestNavigation: mocks.requestNavigation }, + }), +})); + +import { useOtherAppSwitch } from "@app/hooks/useOtherAppSwitch"; +import { takeEditorReturnPath } from "@app/services/workbenchSession"; + +beforeEach(() => { + sessionStorage.clear(); + vi.clearAllMocks(); + mocks.portalAccess = true; +}); + +describe("useOtherAppSwitch", () => { + it("offers no switch without portal access", () => { + mocks.portalAccess = false; + const { result } = renderHook(() => useOtherAppSwitch()); + expect(result.current).toBeNull(); + }); + + it("routes the switch through the unsaved-changes guard", () => { + const { result } = renderHook(() => useOtherAppSwitch()); + result.current?.onOpen(); + + expect(mocks.navigate).not.toHaveBeenCalled(); + expect(mocks.requestNavigation).toHaveBeenCalledTimes(1); + }); + + it("records where to return to, then navigates to the processor", () => { + const { result } = renderHook(() => useOtherAppSwitch()); + result.current?.onOpen(); + mocks.requestNavigation.mock.calls[0][0](); + + expect(takeEditorReturnPath()).toBe("/compress?mode=fast"); + expect(mocks.navigate).toHaveBeenCalledWith("/processor"); + }); +}); diff --git a/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.ts b/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.ts index 8bf07b5c2f..dff36a06f4 100644 --- a/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.ts +++ b/frontend/editor/src/proprietary/hooks/useOtherAppSwitch.ts @@ -1,6 +1,8 @@ -import { useNavigate } from "react-router-dom"; +import { useLocation, useNavigate } from "react-router-dom"; import { useAuth } from "@app/auth/context"; +import { useNavigationActions } from "@app/contexts/NavigationContext"; import { PORTAL_BASENAME } from "@app/routes/portalBasename"; +import { saveEditorReturnPath } from "@app/services/workbenchSession"; import { type NavFooterAppLink } from "@app/components/shared/navFooter/NavFooter"; /** @@ -10,6 +12,16 @@ import { type NavFooterAppLink } from "@app/components/shared/navFooter/NavFoote export function useOtherAppSwitch(): NavFooterAppLink | null { const { portalAccess } = useAuth(); const navigate = useNavigate(); + const location = useLocation(); + const { actions } = useNavigationActions(); if (!portalAccess) return null; - return { app: "processor", onOpen: () => navigate(PORTAL_BASENAME) }; + return { + app: "processor", + onOpen: () => + // Through the guard, so unsaved edits get the same warning as any other navigation. + actions.requestNavigation(() => { + saveEditorReturnPath(location.pathname + location.search); + navigate(PORTAL_BASENAME); + }), + }; } diff --git a/frontend/editor/src/proprietary/routes/adminRouteExtensions.tsx b/frontend/editor/src/proprietary/routes/adminRouteExtensions.tsx index 7f60fbdd3b..22ce46f03c 100644 --- a/frontend/editor/src/proprietary/routes/adminRouteExtensions.tsx +++ b/frontend/editor/src/proprietary/routes/adminRouteExtensions.tsx @@ -3,13 +3,6 @@ import type { ReactElement } from "react"; import { Route } from "react-router-dom"; import { PORTAL_BASENAME } from "@app/routes/portalBasename"; -// The portal ships as a lazy chunk of the editor. It's included in dev (so it's -// always available to work on) and in production builds made with -// VITE_INCLUDE_PORTAL=true (set by -PbuildWithPortal in the JAR, and by the deploy -// GHA when the portal or AI layers change). Vite replaces the env with a literal at -// build time, so when it's off the dynamic import below is tree-shaken out and the -// portal chunk isn't emitted. PortalApp stays module-level so it isn't recreated on -// each render. const includePortal = import.meta.env.VITE_INCLUDE_PORTAL === "true" || import.meta.env.DEV; @@ -21,18 +14,28 @@ const PortalApp = includePortal : null; /** - * The portal mounts as an admin-only route-set at PORTAL_BASENAME (/processor/*). - * Access is gated inside PortalApp (its own AuthProvider + AuthGate, plus server - * enforcement), so this just wires the lazy route into the editor's router when - * the portal is included in this build. + * Return leg of the account-link handshake, which Stirling redirects to with the admin's session in the URL fragment. */ +const ConnectCallback = includePortal + ? lazy(async () => { + const m = await import("@portal/views/ConnectCallback"); + return { default: m.default }; + }) + : null; + +/** The portal mounts as an admin-only route-set at PORTAL_BASENAME (/processor/*). */ export function getAdminRouteExtensions(): ReactElement[] { - if (!PortalApp) return []; + if (!PortalApp || !ConnectCallback) return []; return [ } />, + } + />, ]; } diff --git a/frontend/editor/src/saas/App.tsx b/frontend/editor/src/saas/App.tsx index 67dce869fd..2e091ebf05 100644 --- a/frontend/editor/src/saas/App.tsx +++ b/frontend/editor/src/saas/App.tsx @@ -11,10 +11,12 @@ import { LoadingFallback } from "@app/components/shared/LoadingFallback"; import OnboardingTour from "@app/components/onboarding/OnboardingTour"; import Landing from "@app/routes/Landing"; import Login from "@app/routes/Login"; +import { ResumePendingConnect } from "@app/routes/ResumePendingConnect"; import Signup from "@app/routes/Signup"; import AuthCallback from "@app/routes/AuthCallback"; import ResetPassword from "@app/routes/ResetPassword"; import OAuthConsent from "@app/routes/OAuthConsent"; +import ConnectApprove from "@app/routes/ConnectApprove"; import ShareLinkPage from "@app/routes/ShareLinkPage"; import { getAdminRouteExtensions } from "@app/routes/adminRouteExtensions"; import OnboardingBootstrap from "@app/components/OnboardingBootstrap"; @@ -110,12 +112,17 @@ export default function App() { > + } /> } /> } /> } /> } /> + {/* Human half of the self-hosted account-link handshake. It + lives on this origin because a customer hostname can + never be in the provider's redirect allow-list. */} + } /> {/* Shared-file links. Team invites are NOT routed here: on SaaS they are accepted in-app via the Supabase team invitation banner, not the Spring password-based diff --git a/frontend/editor/src/saas/auth/AuthProvider.test.tsx b/frontend/editor/src/saas/auth/AuthProvider.test.tsx index ffeca5433d..bfd3bef352 100644 --- a/frontend/editor/src/saas/auth/AuthProvider.test.tsx +++ b/frontend/editor/src/saas/auth/AuthProvider.test.tsx @@ -1,6 +1,7 @@ import { act, render, waitFor } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type { Session, User } from "@supabase/supabase-js"; +import { expectConsole } from "@app/tests/failOnConsole"; /** * Request-count tests for {@link AuthProvider}'s data loading. It used to fetch @@ -16,6 +17,7 @@ const createSignedUrl = vi.fn(); const storageFrom = vi.fn((_bucket: string) => ({ createSignedUrl })); const getSession = vi.fn(); const onAuthStateChange = vi.fn(); +const supabaseSignOut = vi.fn(); const unsubscribe = vi.fn(); vi.mock("@app/auth/supabase", () => ({ @@ -26,7 +28,7 @@ vi.mock("@app/auth/supabase", () => ({ refreshSession: vi .fn() .mockResolvedValue({ data: { session: null }, error: null }), - signOut: vi.fn().mockResolvedValue({ error: null }), + signOut: () => supabaseSignOut(), }, rpc: (...args: unknown[]) => rpc(...args), storage: { from: (bucket: string) => storageFrom(bucket) }, @@ -53,6 +55,8 @@ vi.mock("@app/services/userService", () => ({ // Imported after the mocks so the provider picks them up. const { AuthProvider, useAuth } = await import("@app/auth/UseSession"); +const { writeWorkbenchSession, readWorkbenchSession, resumeWorkbenchSession } = + await import("@app/services/workbenchSession"); /** Surfaces `loading` so a test can assert on it rather than on the container. */ function LoadingProbe() { @@ -337,3 +341,55 @@ describe("AuthProvider user-data loading", () => { expect(rpc).toHaveBeenCalledTimes(1); }); }); + +/** + * Signing out suspends workbench recording before the request, so a teardown cannot write the + * record back for whoever signs in next. When the request fails the session stands, and a + * still-signed-in user must not be left silently not recording. + */ +describe("a sign-out that fails", () => { + beforeEach(() => { + vi.clearAllMocks(); + sessionStorage.clear(); + resumeWorkbenchSession(); + getSession.mockResolvedValue({ + data: { session: makeSession() }, + error: null, + }); + onAuthStateChange.mockImplementation(() => ({ + data: { subscription: { unsubscribe } }, + })); + rpc.mockResolvedValue({ data: null, error: null }); + getProfilePictureMetadata.mockResolvedValue(null); + syncOAuthAvatar.mockResolvedValue(undefined); + synchronizeUserUpgrade.mockResolvedValue(undefined); + }); + + it("leaves the workbench still being recorded", async () => { + expectConsole.error(/\[Auth Debug\] Sign out error/); + supabaseSignOut.mockResolvedValue({ error: new Error("network down") }); + + let signOut: (() => Promise) | null = null; + function SignOutProbe() { + signOut = useAuth().signOut; + return null; + } + render( + + + , + ); + await waitFor(() => expect(signOut).not.toBeNull()); + + await act(async () => { + await signOut!(); + }); + + writeWorkbenchSession({ + fileIds: ["still-here"], + selectedFileIds: [], + userId: "user-a", + }); + expect(readWorkbenchSession()?.fileIds).toEqual(["still-here"]); + }); +}); diff --git a/frontend/editor/src/saas/auth/UseSession.tsx b/frontend/editor/src/saas/auth/UseSession.tsx index a1c4db2396..4bd3b99346 100644 --- a/frontend/editor/src/saas/auth/UseSession.tsx +++ b/frontend/editor/src/saas/auth/UseSession.tsx @@ -22,6 +22,10 @@ import { getProviderAvatarUrl, type ProfilePictureMetadata, } from "@app/services/avatarSyncService"; +import { + resumeWorkbenchSession, + suspendWorkbenchSession, +} from "@app/services/workbenchSession"; // Extend Supabase User to include optional username for compatibility export type User = SupabaseUser & { username?: string }; @@ -355,11 +359,19 @@ export function AuthProvider({ children }: { children: ReactNode }) { const signOut = async () => { try { setError(null); + // Signing out is deliberate, unlike an identity check that merely failed: drop the + // workbench record here and stop recording, so the teardown that follows cannot + // write it back for whoever signs in next. + suspendWorkbenchSession(); + const { error } = await supabase.auth.signOut(); if (error) { console.error("[Auth Debug] Sign out error:", error); setError(error); + // The sign-out did not happen and the session stands, so keep recording: + // otherwise a still-signed-in user silently stops persisting their workbench. + resumeWorkbenchSession(); } else { console.debug("[Auth Debug] Signed out successfully"); setSession(null); @@ -367,6 +379,7 @@ export function AuthProvider({ children }: { children: ReactNode }) { } catch (err) { console.error("[Auth Debug] Unexpected error during sign out:", err); setError(err as AuthError); + resumeWorkbenchSession(); } }; diff --git a/frontend/editor/src/saas/hooks/useOtherAppSwitch.ts b/frontend/editor/src/saas/hooks/useOtherAppSwitch.ts index 824e612bd8..67d0aa7d49 100644 --- a/frontend/editor/src/saas/hooks/useOtherAppSwitch.ts +++ b/frontend/editor/src/saas/hooks/useOtherAppSwitch.ts @@ -1,6 +1,8 @@ -import { useNavigate } from "react-router-dom"; +import { useLocation, useNavigate } from "react-router-dom"; import { usePortalAccess } from "@app/hooks/usePortalAccess"; +import { useNavigationActions } from "@app/contexts/NavigationContext"; import { PORTAL_BASENAME } from "@app/routes/portalBasename"; +import { saveEditorReturnPath } from "@app/services/workbenchSession"; import { type NavFooterAppLink } from "@app/components/shared/navFooter/NavFooter"; /** @@ -11,6 +13,16 @@ import { type NavFooterAppLink } from "@app/components/shared/navFooter/NavFoote export function useOtherAppSwitch(): NavFooterAppLink | null { const portalAccess = usePortalAccess(); const navigate = useNavigate(); + const location = useLocation(); + const { actions } = useNavigationActions(); if (!portalAccess) return null; - return { app: "processor", onOpen: () => navigate(PORTAL_BASENAME) }; + return { + app: "processor", + onOpen: () => + // Through the guard, so unsaved edits get the same warning as any other navigation. + actions.requestNavigation(() => { + saveEditorReturnPath(location.pathname + location.search); + navigate(PORTAL_BASENAME); + }), + }; } diff --git a/frontend/editor/src/saas/routes/AuthCallback.tsx b/frontend/editor/src/saas/routes/AuthCallback.tsx index 8fbe925ff2..b69619b85d 100644 --- a/frontend/editor/src/saas/routes/AuthCallback.tsx +++ b/frontend/editor/src/saas/routes/AuthCallback.tsx @@ -4,6 +4,7 @@ import { resolveLandingPath } from "@app/utils/loginLanding"; import { supabase } from "@app/auth/supabase"; import { Button } from "@app/ui/Button"; import { withBasePath } from "@app/constants/app"; +import { readPendingConnect } from "@app/routes/pendingConnect"; import { AuthShell } from "@app/auth/ui/AuthShell"; import ErrorMessage from "@app/auth/ui/ErrorMessage"; import { Spinner } from "@app/ui/Spinner"; @@ -133,10 +134,20 @@ export default function AuthCallback() { // URL can't bounce the user off-origin after sign-in. // No explicit destination: land team leads on the processor and everyone // else on the editor. + // Explicit `next` first, so a sign-in started for another reason is not + // hijacked by a remembered connect request. + const explicitNext = url.searchParams.get("next"); + const pendingConnect = readPendingConnect(); const destination = - next.startsWith("/") && !next.startsWith("//") - ? next - : await resolveLandingPath(); + explicitNext && + explicitNext.startsWith("/") && + !explicitNext.startsWith("//") + ? explicitNext + : pendingConnect + ? `/link?request=${encodeURIComponent(pendingConnect)}` + : next.startsWith("/") && !next.startsWith("//") + ? next + : await resolveLandingPath(); console.log("[Auth Callback Debug] Redirecting to:", destination); setTimeout(() => navigate(destination, { replace: true }), 1500); diff --git a/frontend/editor/src/saas/routes/ConnectApprove.tsx b/frontend/editor/src/saas/routes/ConnectApprove.tsx new file mode 100644 index 0000000000..6374738713 --- /dev/null +++ b/frontend/editor/src/saas/routes/ConnectApprove.tsx @@ -0,0 +1,172 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { useNavigate, useSearchParams } from "react-router-dom"; +import apiClient from "@app/services/apiClient"; +import { useAuth } from "@app/auth/UseSession"; +import { useTranslation } from "@app/hooks/useTranslation"; +import { useDocumentMeta } from "@app/hooks/useDocumentMeta"; +import { withBasePath } from "@app/constants/app"; +import { + clearPendingConnect, + rememberPendingConnect, +} from "@app/routes/pendingConnect"; +import loginHeader from "@app/assets/brand/modern-logo/LoginLightModeHeader.svg"; +import AuthLayout from "@app/routes/authShared/AuthLayout"; +import { + ConnectApproveView, + type ApprovePhase, + type PendingConnect, +} from "@app/routes/ConnectApproveView"; +import "@app/routes/authShared/saas-auth.css"; +import "@app/routes/connect.css"; + +interface ApproveResponse { + callbackUrl: string; + nonce: string; +} + +/** Wider than the view renders: only PENDING is still actionable. */ +interface ConnectLookup extends PendingConnect { + status: "PENDING" | "APPROVED" | "DENIED" | "CONSUMED"; +} + +/** Approve a self-hosted server's request to connect to a team. */ +export default function ConnectApprove() { + const { t } = useTranslation(); + const navigate = useNavigate(); + const { session, user, loading, signOut } = useAuth(); + const [params] = useSearchParams(); + const requestId = params.get("request"); + + const [phase, setPhase] = useState("loading"); + const [pending, setPending] = useState(null); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(null); + const lookedUpRef = useRef(false); + + useDocumentMeta({ title: t("connect.meta.title", "Connect a server") }); + + // On arrival, not only when signed out: an approver who is already signed in can + // still be sent away to re-authenticate, and needs the same way back. + useEffect(() => { + if (requestId) rememberPendingConnect(requestId); + }, [requestId]); + + useEffect(() => { + if (loading || session) return; + // No basename: every consumer of `next` reaches it through navigate(), which + // applies the basename itself, so carrying it here yields /app/app/link. + const next = `/link${requestId ? `?request=${encodeURIComponent(requestId)}` : ""}`; + navigate(`/login?next=${encodeURIComponent(next)}`, { replace: true }); + }, [loading, session, requestId, navigate]); + + useEffect(() => { + if (loading || !session || lookedUpRef.current) return; + lookedUpRef.current = true; + if (!requestId) { + setPhase("notFound"); + return; + } + void (async () => { + try { + const res = await apiClient.get( + `/api/v1/account-link/connect/${encodeURIComponent(requestId)}`, + ); + // Approving a settled request fails server-side, so offering the form again + // would only produce a dead end. + if (res.data.status !== "PENDING") { + clearPendingConnect(); + setPhase(res.data.status === "DENIED" ? "declined" : "notFound"); + return; + } + setPending(res.data); + setPhase("confirm"); + } catch { + clearPendingConnect(); + setPhase("notFound"); + } + })(); + }, [loading, session, requestId]); + + const onDecide = useCallback( + async (approve: boolean) => { + if (!requestId) return; + setBusy(true); + setError(null); + const path = `/api/v1/account-link/connect/${encodeURIComponent(requestId)}`; + try { + if (!approve) { + await apiClient.post(`${path}/deny`); + clearPendingConnect(); + setPhase("declined"); + return; + } + const res = await apiClient.post(`${path}/approve`); + clearPendingConnect(); + setPhase("redirecting"); + window.location.replace(returnUrl(res.data, session)); + } catch { + setError( + t( + "connect.error.failed", + "That did not go through. Only a team owner can connect a server.", + ), + ); + } finally { + setBusy(false); + } + }, + [requestId, session, t], + ); + + /** + * Sign out, then let the signed-out effect above send them to login with the request preserved. + */ + const onSwitchAccount = useCallback(() => { + void signOut(); + }, [signOut]); + + if (loading || !session) return null; + + return ( + + {/* Same header as the sibling auth pages: an admin arriving from another + screen should be able to tell at a glance they are on our site and not + somewhere that merely looks like it. */} +
+ Stirling PDF + Stirling PDF +
+ + void onDecide(approve)} + onSwitchAccount={onSwitchAccount} + /> +
+ ); +} + +/** The callback with the session appended as a fragment. */ +function returnUrl( + approval: ApproveResponse, + session: { access_token?: string; refresh_token?: string } | null, +): string { + const fragment = new URLSearchParams({ type: "link", nonce: approval.nonce }); + if (session?.access_token && session?.refresh_token) { + fragment.set("access_token", session.access_token); + fragment.set("refresh_token", session.refresh_token); + } + return `${approval.callbackUrl}#${fragment.toString()}`; +} diff --git a/frontend/editor/src/saas/routes/ConnectApproveView.tsx b/frontend/editor/src/saas/routes/ConnectApproveView.tsx new file mode 100644 index 0000000000..26a2fe1150 --- /dev/null +++ b/frontend/editor/src/saas/routes/ConnectApproveView.tsx @@ -0,0 +1,182 @@ +import { useState } from "react"; +import { useTranslation } from "@app/hooks/useTranslation"; +import { Banner, Button, Checkbox, Spinner } from "@app/ui"; +import { LocalIcon } from "@app/components/shared/LocalIcon"; +import { Tooltip } from "@app/components/shared/Tooltip"; + +export type ApprovePhase = + | "loading" + | "confirm" + | "redirecting" + | "declined" + | "notFound"; + +/** What the approver is being asked to connect. */ +export interface PendingConnect { + requestId: string; + callbackOrigin: string; + insecureTransport: boolean; +} + +export interface ConnectApproveViewProps { + phase: ApprovePhase; + pending: PendingConnect | null; + /** Email of the account the server would be connected to. */ + signedInEmail: string | null; + busy: boolean; + error: string | null; + onDecide: (approve: boolean) => void; + /** Sign out and come back here, keeping the request so it survives the detour. */ + onSwitchAccount: () => void; +} + +/** Presentation for the connect approval page. */ +export function ConnectApproveView({ + phase, + pending, + signedInEmail, + busy, + error, + onDecide, + onSwitchAccount, +}: ConnectApproveViewProps) { + const { t } = useTranslation(); + // Gates the primary action: anyone can create a request, so the approver reading + // the address is the only thing between one and a linked team. + const [acknowledged, setAcknowledged] = useState(false); + + if (phase === "loading" || phase === "redirecting") { + return ( +
+ +

+ {phase === "redirecting" + ? t("connect.redirecting", "Returning you to your server.") + : t("connect.loading", "Checking this request.")} +

+
+ ); + } + + if (phase === "notFound") { + return ( +
+ + {t( + "connect.notFound.body", + "This connection request is not valid. It may have expired, or already been used. Start another one from your server.", + )} + +
+ ); + } + + if (phase === "declined") { + return ( +
+ + {t( + "connect.declined.body", + "Nothing was connected. You can close this page.", + )} + +
+ ); + } + + return ( +
+

+ {t("connect.confirm.title", "Connect this server?")} +

+

+ {t( + "connect.confirm.lead", + "A Stirling server is asking to connect to your team. Check the address below is yours before you approve.", + )} +

+ + {/* One panel, because the account and the address are two halves of the same + decision: right server, wrong account is still wrong. */} +
+
{t("connect.confirm.signedInAs", "Account")}
+
+ {signedInEmail ?? + t("connect.confirm.unknownAccount", "an unknown account")} + +
+ {/* The reported name is deliberately not shown. The requester chooses it on an + unauthenticated endpoint, so it is the field an attacker would set to look + familiar, and its honest value is the hostname already in the address. It + still labels the server in the linked-instances list, after the decision. */} +
+ {t("connect.confirm.originLabel", "Address")} + {pending?.insecureTransport ? ( + + + + + + ) : null} +
+
{pending?.callbackOrigin}
+
+ + {error ? {error} : null} + + setAcknowledged(e.currentTarget.checked)} + label={t( + "connect.confirm.acknowledge", + "I recognise this address and want to connect it to my team", + )} + /> + +
+ + +
+
+ ); +} diff --git a/frontend/editor/src/saas/routes/ResumePendingConnect.tsx b/frontend/editor/src/saas/routes/ResumePendingConnect.tsx new file mode 100644 index 0000000000..37950047bd --- /dev/null +++ b/frontend/editor/src/saas/routes/ResumePendingConnect.tsx @@ -0,0 +1,38 @@ +import { useEffect, useRef } from "react"; +import { useLocation, useNavigate } from "react-router-dom"; +import { useAuth } from "@app/auth/UseSession"; +import { readPendingConnect } from "@app/routes/pendingConnect"; + +/** + * Sends a newly signed-in visitor back to the approval page they were pulled away + * from. + * + * Mounted app-wide, not only in the auth callback: a confirmation email can land the + * visitor anywhere in the app with a session, and only the ones below resolve the + * request themselves. + */ +export function ResumePendingConnect() { + const { session, loading } = useAuth(); + const navigate = useNavigate(); + const location = useLocation(); + const handled = useRef(false); + + useEffect(() => { + if (loading || !session || handled.current) return; + if ( + location.pathname === "/link" || + location.pathname === "/auth/callback" + ) { + return; + } + handled.current = true; + const requestId = readPendingConnect(); + if (requestId) { + navigate(`/link?request=${encodeURIComponent(requestId)}`, { + replace: true, + }); + } + }, [loading, session, location.pathname, navigate]); + + return null; +} diff --git a/frontend/editor/src/saas/routes/connect.css b/frontend/editor/src/saas/routes/connect.css new file mode 100644 index 0000000000..5a709abfbe --- /dev/null +++ b/frontend/editor/src/saas/routes/connect.css @@ -0,0 +1,93 @@ +/* Connect-approval page. The origin is the thing the approver has to actually + read, so it gets the visual weight and everything else stays quiet. */ + +.saas-connect { + display: flex; + flex-direction: column; + gap: 1rem; + text-align: left; +} + +.saas-connect__title { + margin: 0; + font-size: 1.25rem; + font-weight: 600; + color: var(--c-text); +} + +.saas-connect__lead { + margin: 0; + font-size: 0.875rem; + color: var(--c-text-muted); +} + +/* Sits inside the facts panel rather than beside the email: at this width a + right-aligned action wraps onto its own line and reads as a third field. */ +.saas-connect__switch { + display: block; + margin-top: 0.125rem; + padding: 0; + border: 0; + background: none; + font: inherit; + font-size: 0.8125rem; + color: var(--c-accent-text); + cursor: pointer; +} + +.saas-connect__switch:hover:not(:disabled) { + text-decoration: underline; +} + +.saas-connect__switch:disabled { + color: var(--c-text-muted); + cursor: default; +} + +.saas-connect__facts { + display: grid; + grid-template-columns: auto 1fr; + gap: 0.375rem 1rem; + margin: 0; + padding: 0.875rem; + background: var(--c-surface-sunken); + border: 1px solid var(--c-border); + border-radius: 0.375rem; + font-size: 0.875rem; +} + +.saas-connect__facts dt { + margin: 0; + color: var(--c-text-muted); +} + +.saas-connect__facts dd { + margin: 0; + color: var(--c-text); + overflow-wrap: anywhere; +} + +/* Monospaced so a lookalike hostname is harder to skim past. */ +.saas-connect__origin { + font-family: var(--font-mono, ui-monospace, monospace); + font-weight: 600; +} + +.saas-connect__origin-label { + display: flex; + align-items: center; + gap: 0.375rem; +} + +.saas-connect__insecure { + display: inline-flex; + flex: none; + color: var(--c-warning); + cursor: help; +} + +.saas-connect__actions { + display: flex; + justify-content: flex-end; + gap: 0.5rem; +} diff --git a/frontend/editor/src/saas/routes/pendingConnect.ts b/frontend/editor/src/saas/routes/pendingConnect.ts new file mode 100644 index 0000000000..c44af277be --- /dev/null +++ b/frontend/editor/src/saas/routes/pendingConnect.ts @@ -0,0 +1,61 @@ +/** + * Remembers that the visitor arrived wanting to connect a server, so a sign-in + * detour can return them to the approval page. + * + * localStorage, not sessionStorage: the confirmation email opens a new tab, and + * sessionStorage is per-tab — empty exactly when it is needed. + * + * Only the request id, which is already in the URL and carries no secret. This + * decides where the approver lands, never whether the link happens. + * + * Reading does not consume it: the request may be open in another tab, or the page + * closed and reopened, or the reader mounted twice. Only a recorded decision, or a + * request that is settled or gone, retires it. + */ +const KEY = "stirling-pending-connect"; + +/** Matches the server's request lifetime, so a stale intent cannot hijack a later sign-in. */ +const TTL_MS = 30 * 60 * 1000; + +interface Stored { + requestId: string; + at: number; +} + +export function rememberPendingConnect(requestId: string): void { + try { + const value: Stored = { requestId, at: Date.now() }; + window.localStorage.setItem(KEY, JSON.stringify(value)); + } catch { + // Private browsing or a full quota; nothing to fall back to. + } +} + +/** Drops the intent without reading it, once it has been acted on. */ +export function clearPendingConnect(): void { + try { + window.localStorage.removeItem(KEY); + } catch { + // Unwritable store; nothing to remove. + } +} + +/** The pending request, or null when absent or expired. Leaves it in place. */ +export function readPendingConnect(): string | null { + try { + const raw = window.localStorage.getItem(KEY); + if (!raw) return null; + const value = JSON.parse(raw) as Stored; + if (typeof value?.requestId !== "string" || typeof value?.at !== "number") { + clearPendingConnect(); + return null; + } + if (Date.now() - value.at > TTL_MS) { + clearPendingConnect(); + return null; + } + return value.requestId; + } catch { + return null; + } +} diff --git a/frontend/editor/vite.config.ts b/frontend/editor/vite.config.ts index 96ce29c480..22b8ce4e24 100644 --- a/frontend/editor/vite.config.ts +++ b/frontend/editor/vite.config.ts @@ -259,6 +259,16 @@ export default defineConfig(async ({ mode, command }) => { }; return { + // Per-mode: the default is one shared node_modules/.vite, so two dev servers in + // different modes re-optimize over each other and the browser 504s on a stale dep + // hash. Anchored to frontend/ because a relative path resolves against the vite + // root (editor/) and would create a second node_modules there. + cacheDir: resolve( + import.meta.dirname, + "..", + "node_modules", + `.vite-${effectiveMode}`, + ), define: { __DEV_WORKTREE_LABEL__: JSON.stringify(devWorktreeLabel), },