-
Notifications
You must be signed in to change notification settings - Fork 54
refactor(desktop): Manage auth token in renderer instead of main thread for cleaner code / better interop with better auth #741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,62 +1,113 @@ | ||
| import crypto from "node:crypto"; | ||
| import fs from "node:fs/promises"; | ||
| import { AUTH_PROVIDERS } from "@superset/shared/constants"; | ||
| import { observable } from "@trpc/server/observable"; | ||
| import { type AuthSession, authService } from "main/lib/auth"; | ||
| import { shell } from "electron"; | ||
| import { env } from "main/env.main"; | ||
| import { z } from "zod"; | ||
| import { publicProcedure, router } from "../.."; | ||
|
|
||
| /** Auth state emitted by onAuthState subscription */ | ||
| export type AuthState = (AuthSession & { token: string | null }) | null; | ||
| import { | ||
| authEvents, | ||
| loadToken, | ||
| saveToken, | ||
| stateStore, | ||
| TOKEN_FILE, | ||
| } from "./utils/auth-functions"; | ||
|
|
||
| export const createAuthRouter = () => { | ||
| return router({ | ||
| onAuthState: publicProcedure.subscription(() => { | ||
| return observable<AuthState>((emit) => { | ||
| const emitCurrent = () => { | ||
| const sessionData = authService.getSession(); | ||
| const token = authService.getAccessToken(); | ||
| /** | ||
| * Get initial token from encrypted disk storage. | ||
| * Called once on app startup for hydration. | ||
| */ | ||
| getStoredToken: publicProcedure.query(async () => { | ||
| return await loadToken(); | ||
| }), | ||
|
|
||
| if (!sessionData) { | ||
| emit.next(null); | ||
| return; | ||
| /** | ||
| * Persist token to encrypted disk storage. | ||
| * Called when renderer saves token to localStorage. | ||
| */ | ||
| persistToken: publicProcedure | ||
| .input( | ||
| z.object({ | ||
| token: z.string(), | ||
| expiresAt: z.string(), | ||
| }), | ||
| ) | ||
| .mutation(async ({ input }) => { | ||
| await saveToken(input); | ||
| return { success: true }; | ||
| }), | ||
|
|
||
| /** | ||
| * Subscribe to token changes from deep link callbacks. | ||
| * CRITICAL: Notifies renderer when OAuth callback saves new token. | ||
| * Without this, renderer wouldn't know to update localStorage after OAuth. | ||
| */ | ||
| onTokenChanged: publicProcedure.subscription(() => { | ||
| return observable<{ token: string; expiresAt: string } | null>((emit) => { | ||
| // Emit initial token on subscription | ||
| loadToken().then((initial) => { | ||
| if (initial.token && initial.expiresAt) { | ||
| emit.next({ token: initial.token, expiresAt: initial.expiresAt }); | ||
| } | ||
| }); | ||
|
|
||
| emit.next({ ...sessionData, token }); | ||
| const handler = (data: { token: string; expiresAt: string }) => { | ||
| emit.next(data); | ||
| }; | ||
|
|
||
| emitCurrent(); | ||
|
|
||
| const sessionHandler = () => { | ||
| emitCurrent(); | ||
| }; | ||
| const stateHandler = () => { | ||
| emitCurrent(); | ||
| }; | ||
|
|
||
| authService.on("session-changed", sessionHandler); | ||
| authService.on("state-changed", stateHandler); | ||
| authEvents.on("token-saved", handler); | ||
|
|
||
| return () => { | ||
| authService.off("session-changed", sessionHandler); | ||
| authService.off("state-changed", stateHandler); | ||
| authEvents.off("token-saved", handler); | ||
| }; | ||
| }); | ||
| }), | ||
|
|
||
| setActiveOrganization: publicProcedure | ||
| .input(z.object({ organizationId: z.string() })) | ||
| .mutation(async ({ input }) => { | ||
| await authService.setActiveOrganization(input.organizationId); | ||
| return { success: true }; | ||
| }), | ||
|
|
||
| /** | ||
| * Start OAuth sign-in flow. | ||
| * Opens browser for OAuth, token delivered via deep link callback. | ||
| */ | ||
| signIn: publicProcedure | ||
| .input(z.object({ provider: z.enum(AUTH_PROVIDERS) })) | ||
| .mutation(async ({ input }) => { | ||
| return authService.signIn(input.provider); | ||
| try { | ||
| const state = crypto.randomBytes(32).toString("base64url"); | ||
| stateStore.set(state, Date.now()); | ||
|
|
||
| // Clean up old states (older than 10 minutes) | ||
| const tenMinutesAgo = Date.now() - 10 * 60 * 1000; | ||
| for (const [s, ts] of stateStore) { | ||
| if (ts < tenMinutesAgo) stateStore.delete(s); | ||
| } | ||
|
|
||
| const connectUrl = new URL( | ||
| `${env.NEXT_PUBLIC_API_URL}/api/auth/desktop/connect`, | ||
| ); | ||
| connectUrl.searchParams.set("provider", input.provider); | ||
| connectUrl.searchParams.set("state", state); | ||
| await shell.openExternal(connectUrl.toString()); | ||
| return { success: true }; | ||
| } catch (err) { | ||
| return { | ||
| success: false, | ||
| error: | ||
| err instanceof Error ? err.message : "Failed to open browser", | ||
| }; | ||
| } | ||
| }), | ||
|
|
||
| /** | ||
| * Sign out - clears token from disk. | ||
| * Renderer should also clear localStorage and call authClient.signOut(). | ||
| */ | ||
| signOut: publicProcedure.mutation(async () => { | ||
| await authService.signOut(); | ||
| console.log("[auth] Clearing token"); | ||
| try { | ||
| await fs.unlink(TOKEN_FILE); | ||
| } catch {} | ||
| return { success: true }; | ||
| }), | ||
|
Comment on lines
106
to
112
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Empty catch block silently swallows errors. Per coding guidelines, errors should never be swallowed silently. The Suggested fix signOut: publicProcedure.mutation(async () => {
console.log("[auth] Clearing token");
try {
await fs.unlink(TOKEN_FILE);
- } catch {}
+ } catch (err) {
+ // ENOENT is expected if token file doesn't exist
+ if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
+ console.error("[auth/signOut] Failed to clear token file:", err);
+ }
+ }
return { success: true };
}),🤖 Prompt for AI Agents |
||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,99 @@ | ||||||||||||||||
| import { EventEmitter } from "node:events"; | ||||||||||||||||
| import fs from "node:fs/promises"; | ||||||||||||||||
| import { join } from "node:path"; | ||||||||||||||||
| import { PROTOCOL_SCHEMES } from "@superset/shared/constants"; | ||||||||||||||||
| import { SUPERSET_HOME_DIR } from "main/lib/app-environment"; | ||||||||||||||||
| import { decrypt, encrypt } from "./crypto-storage"; | ||||||||||||||||
|
|
||||||||||||||||
| interface StoredAuth { | ||||||||||||||||
| token: string; | ||||||||||||||||
| expiresAt: string; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| export const TOKEN_FILE = join(SUPERSET_HOME_DIR, "auth-token.enc"); | ||||||||||||||||
| export const stateStore = new Map<string, number>(); | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Event emitter for auth-related events. | ||||||||||||||||
| * Used by tRPC subscription to notify renderer of token changes. | ||||||||||||||||
| */ | ||||||||||||||||
| export const authEvents = new EventEmitter(); | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Load token from encrypted disk storage. | ||||||||||||||||
| */ | ||||||||||||||||
| export async function loadToken(): Promise<{ | ||||||||||||||||
| token: string | null; | ||||||||||||||||
| expiresAt: string | null; | ||||||||||||||||
| }> { | ||||||||||||||||
| try { | ||||||||||||||||
| const data = decrypt(await fs.readFile(TOKEN_FILE)); | ||||||||||||||||
| const parsed: StoredAuth = JSON.parse(data); | ||||||||||||||||
| console.log("[auth] Token loaded from disk"); | ||||||||||||||||
| return { token: parsed.token, expiresAt: parsed.expiresAt }; | ||||||||||||||||
| } catch { | ||||||||||||||||
| return { token: null, expiresAt: null }; | ||||||||||||||||
| } | ||||||||||||||||
|
Comment on lines
+34
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Silent error swallowing masks potential issues. Per coding guidelines, errors should never be swallowed silently. If token loading fails due to corruption, permission issues, or decryption errors, this should be logged for debugging. Proposed fix- } catch {
+ } catch (error) {
+ console.log("[auth] Failed to load token from disk:", error);
return { token: null, expiresAt: null };
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Persist token to encrypted disk storage and notify subscribers. | ||||||||||||||||
| */ | ||||||||||||||||
| export async function saveToken({ | ||||||||||||||||
| token, | ||||||||||||||||
| expiresAt, | ||||||||||||||||
| }: { | ||||||||||||||||
| token: string; | ||||||||||||||||
| expiresAt: string; | ||||||||||||||||
| }): Promise<void> { | ||||||||||||||||
| const storedAuth: StoredAuth = { token, expiresAt }; | ||||||||||||||||
| await fs.writeFile(TOKEN_FILE, encrypt(JSON.stringify(storedAuth))); | ||||||||||||||||
| console.log("[auth] Token saved to disk"); | ||||||||||||||||
|
|
||||||||||||||||
| // Emit event for onTokenChanged subscription | ||||||||||||||||
| authEvents.emit("token-saved", { token, expiresAt }); | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Handle OAuth callback from deep link. | ||||||||||||||||
| * Validates CSRF state and saves token. | ||||||||||||||||
| */ | ||||||||||||||||
| export async function handleAuthCallback(params: { | ||||||||||||||||
| token: string; | ||||||||||||||||
| expiresAt: string; | ||||||||||||||||
| state: string; | ||||||||||||||||
| }): Promise<{ success: boolean; error?: string }> { | ||||||||||||||||
| if (!stateStore.has(params.state)) { | ||||||||||||||||
| return { success: false, error: "Invalid or expired auth session" }; | ||||||||||||||||
| } | ||||||||||||||||
| stateStore.delete(params.state); | ||||||||||||||||
|
|
||||||||||||||||
| await saveToken({ token: params.token, expiresAt: params.expiresAt }); | ||||||||||||||||
|
|
||||||||||||||||
| return { success: true }; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| /** | ||||||||||||||||
| * Parse and validate auth deep link URL. | ||||||||||||||||
| */ | ||||||||||||||||
| export function parseAuthDeepLink( | ||||||||||||||||
| url: string, | ||||||||||||||||
| ): { token: string; expiresAt: string; state: string } | null { | ||||||||||||||||
| try { | ||||||||||||||||
| const parsed = new URL(url); | ||||||||||||||||
| const validProtocols = [ | ||||||||||||||||
| `${PROTOCOL_SCHEMES.PROD}:`, | ||||||||||||||||
| `${PROTOCOL_SCHEMES.DEV}:`, | ||||||||||||||||
| ]; | ||||||||||||||||
| if (!validProtocols.includes(parsed.protocol)) return null; | ||||||||||||||||
| if (parsed.host !== "auth" || parsed.pathname !== "/callback") return null; | ||||||||||||||||
|
|
||||||||||||||||
| const token = parsed.searchParams.get("token"); | ||||||||||||||||
| const expiresAt = parsed.searchParams.get("expiresAt"); | ||||||||||||||||
| const state = parsed.searchParams.get("state"); | ||||||||||||||||
| if (!token || !expiresAt || !state) return null; | ||||||||||||||||
| return { token, expiresAt, state }; | ||||||||||||||||
| } catch { | ||||||||||||||||
| return null; | ||||||||||||||||
| } | ||||||||||||||||
| } | ||||||||||||||||
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle potential rejection from initial token load.
The
loadToken().then()call has no error handler. IfloadToken()rejects, it will cause an unhandled promise rejection.Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents