From b6a7b00e9fee944852d15712055959927e804819 Mon Sep 17 00:00:00 2001 From: Luis Leineweber Date: Sun, 4 Oct 2026 23:05:59 +0200 Subject: [PATCH] feat(auth): improve OAuth callback pages - Share one branded callback page for Spotify and Discogs - Return immediately after successful login --- src/main/frontend/styles/oauth-callback.css | 116 ++++++++++ .../java/Server/http/OAuthCallbackPage.java | 114 +++++++++ src/main/java/Server/routes/AuthRoutes.java | 216 +----------------- .../java/Server/routes/DiscogsRoutes.java | 86 +------ .../java/Server/routes/AuthRoutesTest.java | 34 +++ .../java/Server/routes/DiscogsRoutesTest.java | 15 ++ 6 files changed, 283 insertions(+), 298 deletions(-) create mode 100644 src/main/frontend/styles/oauth-callback.css create mode 100644 src/main/java/Server/http/OAuthCallbackPage.java diff --git a/src/main/frontend/styles/oauth-callback.css b/src/main/frontend/styles/oauth-callback.css new file mode 100644 index 0000000..8b2824b --- /dev/null +++ b/src/main/frontend/styles/oauth-callback.css @@ -0,0 +1,116 @@ +@import url('./base.css'); + +:root { + --font-body: "Space Mono", "IBM Plex Mono", monospace; + --font-display: "Archivo Black", "Arial Black", sans-serif; + --bg-primary: #f5f5f0; + --bg-card: #ffffff; + --text-primary: #0a0a0a; + --text-secondary: #666666; + --text-inverse: #ffffff; + --border-default: #0a0a0a; + --accent: #ff3300; +} + +:root[data-theme="dark"] { + --accent: #ff3300; + --bg-primary: #0a0a0a; + --bg-card: #1a1a1a; + --text-primary: #f5f5f0; + --text-secondary: #a3a3a3; + --text-inverse: #0a0a0a; + --border-default: #f5f5f0; +} + +@media (prefers-color-scheme: dark) { + :root:not([data-theme]) { + --accent: #ff3300; + --bg-primary: #0a0a0a; + --bg-card: #1a1a1a; + --text-primary: #f5f5f0; + --text-secondary: #a3a3a3; + --text-inverse: #0a0a0a; + --border-default: #f5f5f0; + } +} + +body { + min-height: 100vh; + min-height: 100svh; + display: grid; + place-items: center; + padding: 24px; +} + +.callback-page { + width: min(100%, 480px); + padding: 32px; + border: 3px solid var(--border-default); + border-top-color: var(--accent); + background: var(--bg-card); + box-shadow: 6px 6px 0 var(--border-default); + overflow-wrap: anywhere; +} + +.callback-brand { + margin-bottom: 32px; + font-family: var(--font-display); + font-size: 14px; + text-transform: uppercase; +} + +h1 { + margin-bottom: 16px; + font-size: 28px; + line-height: 1.2; + letter-spacing: -0.02em; + text-wrap: balance; +} + +.callback-message { + line-height: 1.6; + text-wrap: pretty; +} + +.callback-hint { + margin-top: 12px; + font-size: 14px; + line-height: 1.6; + color: var(--text-secondary); +} + +.callback-action { + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; + min-height: 48px; + margin-top: 28px; + padding: 12px 16px; + border: 2px solid var(--border-default); + background: var(--text-primary); + color: var(--text-inverse); + font-size: 14px; + font-weight: 700; + text-decoration: none; +} + +.callback-action:hover { + background: var(--bg-card); + color: var(--text-primary); +} + +.callback-action:focus-visible { + outline: 3px solid var(--text-primary); + outline-offset: 4px; +} + +@media (max-width: 380px) { + .callback-page { + padding: 24px 20px; + } + + h1 { + font-size: 24px; + } +} diff --git a/src/main/java/Server/http/OAuthCallbackPage.java b/src/main/java/Server/http/OAuthCallbackPage.java new file mode 100644 index 0000000..d238837 --- /dev/null +++ b/src/main/java/Server/http/OAuthCallbackPage.java @@ -0,0 +1,114 @@ +package Server.http; + +import com.sun.net.httpserver.HttpExchange; + +import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; + +/** Renders the return page for Spotify and Discogs login. */ +public final class OAuthCallbackPage { + private OAuthCallbackPage() {} + + public enum Provider { + SPOTIFY("Spotify", "spotify-auth-callback", "/"), + DISCOGS("Discogs", "discogs-auth-callback", "/playlist.html"); + + private final String name; + private final String eventType; + private final String returnPath; + + Provider(String name, String eventType, String returnPath) { + this.name = name; + this.eventType = eventType; + this.returnPath = returnPath; + } + } + + public static void send(HttpExchange exchange, Provider provider, boolean success, String message) throws IOException { + byte[] body = render(provider, success, message).getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().set("Content-Type", "text/html; charset=utf-8"); + exchange.getResponseHeaders().set("Cache-Control", "no-store"); + exchange.sendResponseHeaders(200, body.length); + try (OutputStream os = exchange.getResponseBody()) { + os.write(body); + } + } + + public static String render(Provider provider, boolean success, String message) { + String title = provider.name + (success ? " connected" : " connection failed"); + return """ + + + + + + VinylMatch - %s + + + + + +
+

Vinyl Match

+

%s

+

%s

+

%s

+ Return to VinylMatch +
+ + + """.formatted(success, provider.eventType, provider.returnPath, escapeHtml(message), title, + title, success ? "Returning to your music." : escapeHtml(message), + success ? "If this window stays open, use the link below." + : "Return to the app to try again.", provider.returnPath); + } + + private static String escapeHtml(String value) { + if (value == null) return ""; + return value.replace("&", "&") + .replace("<", "<") + .replace(">", ">") + .replace("\"", """) + .replace("'", "'"); + } +} diff --git a/src/main/java/Server/routes/AuthRoutes.java b/src/main/java/Server/routes/AuthRoutes.java index a492722..41219ea 100644 --- a/src/main/java/Server/routes/AuthRoutes.java +++ b/src/main/java/Server/routes/AuthRoutes.java @@ -4,6 +4,7 @@ import Server.cache.PlaylistCache; import Server.http.ApiFilters; import Server.http.HttpUtils; +import Server.http.OAuthCallbackPage; import Server.session.SpotifySession; import Server.session.SpotifySessionStore; import com.sun.net.httpserver.HttpExchange; @@ -12,10 +13,8 @@ import org.slf4j.LoggerFactory; import java.io.IOException; -import java.io.OutputStream; import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.StandardCharsets; import java.util.Map; /** @@ -319,218 +318,7 @@ public boolean isAuthenticated(HttpExchange exchange) { } private void sendCallbackHtml(HttpExchange exchange, boolean success, String message) throws IOException { - String status = success ? "Spotify connected" : "Spotify connection failed"; - String badge = success ? "SUCCESS" : "ERROR"; - String closeHint = success ? "This window closes automatically in a moment." : "You can close this window and try again."; - String safeMessage = escapeHtml(message); - String html = """ - - - - - VinylMatch - %s - - - - -
-

VinylMatch

-
%s
-

%s

-

%s

-

%s

-
- Back to app -
-
- - - - """.formatted( - status, - success ? "badge-success" : "badge-error", - badge, - status, - safeMessage, - closeHint, - success - ); - - byte[] body = html.getBytes(StandardCharsets.UTF_8); - exchange.getResponseHeaders().add("Content-Type", "text/html; charset=utf-8"); - exchange.sendResponseHeaders(200, body.length); - try (OutputStream os = exchange.getResponseBody()) { - os.write(body); - } - } - - private static String escapeHtml(String value) { - if (value == null) return ""; - return value - .replace("&", "&") - .replace("<", "<") - .replace(">", ">") - .replace("\"", """) - .replace("'", "'"); + OAuthCallbackPage.send(exchange, OAuthCallbackPage.Provider.SPOTIFY, success, message); } public record AccessTokenResolution(String token, boolean userAuthenticated) {} diff --git a/src/main/java/Server/routes/DiscogsRoutes.java b/src/main/java/Server/routes/DiscogsRoutes.java index c3d6dbb..7b7ecbb 100644 --- a/src/main/java/Server/routes/DiscogsRoutes.java +++ b/src/main/java/Server/routes/DiscogsRoutes.java @@ -2,6 +2,7 @@ import Server.auth.DiscogsOAuthService; import Server.http.HttpUtils; +import Server.http.OAuthCallbackPage; import Server.http.ApiFilters; import Server.http.filters.AdminOnlyFilter; import Server.session.DiscogsSession; @@ -23,10 +24,8 @@ import org.slf4j.LoggerFactory; import java.io.IOException; -import java.io.OutputStream; import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -747,88 +746,7 @@ private static URI deriveLoopbackDiscogsCallback(HttpExchange exchange) { } private void sendOAuthCallbackHtml(HttpExchange exchange, boolean success, String message) throws IOException { - String status = success ? "Discogs Login Successful" : "Discogs Login Failed"; - String color = success ? "#1f7a3f" : "#b23333"; - String action = success ? "Closing window..." : "You can close this window."; - String safeMessage = escapeHtml(message); - - String html = """ - - - - - - %s - - - -
-

%s

-

%s

-

%s

-
- - - - """.formatted( - status, - color, - safeMessage, - status, - safeMessage, - action, - success - ); - - byte[] body = html.getBytes(StandardCharsets.UTF_8); - exchange.getResponseHeaders().set("Content-Type", "text/html; charset=utf-8"); - exchange.sendResponseHeaders(200, body.length); - try (OutputStream os = exchange.getResponseBody()) { - os.write(body); - } - } - - private static String escapeHtml(String value) { - if (value == null) { - return ""; - } - return value - .replace("&", "&") - .replace("<", "<") - .replace(">", ">") - .replace("\"", """) - .replace("'", "'"); + OAuthCallbackPage.send(exchange, OAuthCallbackPage.Provider.DISCOGS, success, message); } private Integer parseYear(Object value) { diff --git a/src/test/java/Server/routes/AuthRoutesTest.java b/src/test/java/Server/routes/AuthRoutesTest.java index bbb045e..46cd824 100644 --- a/src/test/java/Server/routes/AuthRoutesTest.java +++ b/src/test/java/Server/routes/AuthRoutesTest.java @@ -15,13 +15,47 @@ import java.io.ByteArrayOutputStream; import java.io.InputStream; import java.io.OutputStream; +import java.lang.reflect.Method; import java.net.InetSocketAddress; import java.net.URI; +import java.nio.charset.StandardCharsets; import static org.junit.jupiter.api.Assertions.*; class AuthRoutesTest { + @Test + void callbackReturnsImmediatelyAfterSuccessfulLogin() throws Exception { + AuthRoutes routes = new AuthRoutes(new PlaylistCache(new ObjectMapper()), + new SpotifySessionStore(), new TestSpotifyOAuthService()); + Method method = AuthRoutes.class.getDeclaredMethod("sendCallbackHtml", HttpExchange.class, boolean.class, String.class); + method.setAccessible(true); + FakeExchange exchange = new FakeExchange("GET", URI.create("http://127.0.0.1/api/auth/callback")); + + method.invoke(routes, exchange, true, "Spotify connected."); + + String body = exchange.responseBody.toString(StandardCharsets.UTF_8); + assertFalse(body.contains("setTimeout"), "Login must not add a fixed wait before returning"); + assertTrue(body.contains("window.location.replace("), "Returning must remove the callback from browser history"); + assertTrue(body.indexOf(""; + + method.invoke(routes, exchange, false, message); + + String body = exchange.responseBody.toString(StandardCharsets.UTF_8); + assertFalse(body.contains(message)); + assertTrue(body.contains("</script><script>alert('error')</script>")); + } + @Test void getAccessTokenRefreshesAndPersistsSession() { RecordingSessionStore sessionStore = new RecordingSessionStore(); diff --git a/src/test/java/Server/routes/DiscogsRoutesTest.java b/src/test/java/Server/routes/DiscogsRoutesTest.java index f303b78..c7c3948 100644 --- a/src/test/java/Server/routes/DiscogsRoutesTest.java +++ b/src/test/java/Server/routes/DiscogsRoutesTest.java @@ -115,6 +115,21 @@ void disabledFeatureDoesNotCallJevOrAddJevMetadata() throws Exception { assertEquals(0, calls.get()); } + @Test + void callbackReturnsImmediatelyAfterSuccessfulLogin() throws Exception { + DiscogsRoutes routes = new DiscogsRoutes(() -> null, new DiscogsSessionStore(), new SpotifySessionStore()); + Method method = DiscogsRoutes.class.getDeclaredMethod("sendOAuthCallbackHtml", HttpExchange.class, boolean.class, String.class); + method.setAccessible(true); + FakeExchange exchange = new FakeExchange("GET", URI.create("http://127.0.0.1/api/discogs/oauth/callback")); + + method.invoke(routes, exchange, true, "Discogs connected."); + + String body = exchange.responseBodyAsString(); + assertFalse(body.contains("setTimeout"), "Login must not add a fixed wait before returning"); + assertTrue(body.contains("window.location.replace("), "Returning must remove the callback from browser history"); + assertTrue(body.indexOf("