feat(desktop): add AniList account linking via OAuth implicit grant#139
feat(desktop): add AniList account linking via OAuth implicit grant#139Shironex wants to merge 1 commit into
Conversation
Foundation for the AniList sync chain: link an AniList account, persist the session securely, and expose an authenticated GraphQL path — no manual token handling and no client secret. - Implicit grant in a hardened popup BrowserWindow: isolated non-persistent session partition, contextIsolation/sandbox on, nodeIntegration off, window.open denied, navigation confined to AniList origins, redirect intercepted on will-redirect AND will-navigate with a canonical origin+pathname match, token read from the URL fragment (redirect page never loads). Token is never logged and never crosses IPC. - safeStorage-backed token store with an explicit, flagged plaintext fallback when OS encryption is unavailable; expiry-aware getToken/getStatus. - AniListTokenPort (provided from main) injected into AniListClient; new getViewer() runs authenticated queries while the unauthenticated path is unchanged. - IPC connect/disconnect/status (renderer only ever sees AniListAuthStatus), preload bridge, and a @global() AniListAuthModule wiring verified by a Nest boot test. - Renderer: useAniListAuthStore + AccountsSection settings panel (pl/en), connected/disconnected/loading/error states. Requires a maintainer-supplied public client_id (ANILIST_CLIENT_ID / VITE_ANILIST_CLIENT_ID); redirect URI https://shiroani.app/oauth/anilist. The feature is inert with a clear "not configured" error until set.
|
Warning Review limit reached
More reviews will be available in 28 minutes and 15 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (38)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
i18n
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces AniList OAuth (implicit grant) authentication, allowing users to securely connect their AniList accounts. Key changes include implementing an isolated Electron browser window for the OAuth flow, securely persisting the access token using Electron's 'safeStorage', and exposing the connection status to the renderer via IPC. The AniListClient has been updated to support authenticated requests and fetch the user's profile, accompanied by a new 'Accounts' settings section in the frontend. The review feedback suggests several robustness and security enhancements: explicitly clearing the OAuth session partition before starting the flow, extracting detailed error messages from the redirect URL when authorization fails, validating persisted session fields during retrieval, and adding defensive checks when parsing the GraphQL viewer response.
| clientId: string | ||
| ): Promise<{ accessToken: string; expiresIn: number }> { | ||
| return new Promise((resolve, reject) => { | ||
| const oauthSession = session.fromPartition(OAUTH_PARTITION, { cache: false }); |
There was a problem hiding this comment.
To prevent session reuse or credential leakage across multiple authorization attempts (e.g., if a user wants to switch accounts or retry after a failure), it is highly recommended to explicitly clear any leftover session data (cookies, localStorage, etc.) from the partition before initiating the OAuth flow.
| const oauthSession = session.fromPartition(OAUTH_PARTITION, { cache: false }); | |
| const oauthSession = session.fromPartition(OAUTH_PARTITION, { cache: false }); | |
| void oauthSession.clearStorageData(); |
| const parsed = parseTokenFragment(navUrl); | ||
| if (parsed) { | ||
| finish(parsed); | ||
| } else { | ||
| fail('AniList redirect did not contain an access token'); | ||
| } |
There was a problem hiding this comment.
When the AniList authorization fails or is denied by the user, AniList redirects with error details in the URL fragment (e.g., #error=access_denied&error_description=...). Extracting and forwarding these error details provides a much more helpful and actionable error message to the user instead of a generic 'did not contain an access token' error.
const parsed = parseTokenFragment(navUrl);
if (parsed) {
finish(parsed);
} else {
const hashIndex = navUrl.indexOf('#');
const fragment = hashIndex !== -1 ? navUrl.slice(hashIndex + 1) : '';
const params = new URLSearchParams(fragment);
const error = params.get('error');
const errorDesc = params.get('error_description');
if (error) {
fail('AniList authorization failed: ' + (errorDesc || error));
} else {
fail('AniList redirect did not contain an access token');
}
}| function readSession(): PersistedSession | null { | ||
| const raw = store.get(SESSION_KEY) as PersistedSession | undefined; | ||
| if (!raw || typeof raw.token !== 'string' || typeof raw.expiresAt !== 'number') { | ||
| return null; | ||
| } | ||
| return raw; | ||
| } |
There was a problem hiding this comment.
To prevent potential runtime errors or UI inconsistencies, validate that expiresAt is a valid finite number (not NaN) and that the viewer object is present and valid before returning the session. If viewer is missing, the UI might incorrectly treat the state as disconnected even though connected is true.
function readSession(): PersistedSession | null {
const raw = store.get(SESSION_KEY) as PersistedSession | undefined;
if (
!raw ||
typeof raw.token !== 'string' ||
typeof raw.expiresAt !== 'number' ||
!Number.isFinite(raw.expiresAt) ||
!raw.viewer ||
typeof raw.viewer !== 'object'
) {
return null;
}
return raw;
}| async getViewer(): Promise<AniListViewer> { | ||
| const data = await this.query<ViewerResponse>(VIEWER_QUERY); | ||
| const viewer = data.Viewer; | ||
| return { | ||
| id: viewer.id, | ||
| name: viewer.name, | ||
| avatar: viewer.avatar?.large, | ||
| bannerImage: viewer.bannerImage, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Apply defensive programming by verifying that the viewer object exists in the GraphQL response before accessing its properties. This prevents potential TypeError crashes if the API returns a successful response structure but with a null Viewer field.
async getViewer(): Promise<AniListViewer> {
const data = await this.query<ViewerResponse>(VIEWER_QUERY);
const viewer = data?.Viewer;
if (!viewer) {
throw new Error('AniList API returned no viewer data');
}
return {
id: viewer.id,
name: viewer.name,
avatar: viewer.avatar?.large,
bannerImage: viewer.bannerImage,
};
}
1 participant
What
Foundation for the AniList sync chain (the highest-leverage post-v1 item in the gap analysis): link an AniList account, persist the session securely, and expose an authenticated GraphQL path. No manual token handling, no client secret.
Built via a contract-first parallel workflow — a frozen cross-package contract, then package-isolated desktop + web slices, then a cross-package typecheck/test/lint gate and an adversarial security review (4 findings, all fixed before this PR).
How
Auth flow (
apps/desktop/src/main/auth/)BrowserWindow: isolated non-persistent session partition,contextIsolation:true/sandbox:true/nodeIntegration:false,window.opendenied, navigation confined to AniList origins, redirect intercepted on bothwill-redirectandwill-navigatewith a canonical origin+pathname match (not a prefix test), token read from the URL fragment — the redirect page never loads.safeStorage-backed token store with an explicit, flagged plaintext fallback when OS encryption is unavailable; expiry-awaregetToken/getStatus; missing/garbledexpires_infalls back to AniList's documented ~1-year TTL.Wiring
AniListTokenPort(abstract class, provided frommain) injected intoAniListClient; newgetViewer()runs authenticated queries while the existing unauthenticated path is unchanged.connect/disconnect/status(renderer only ever receivesAniListAuthStatus), preload bridge, and a@Global()AniListAuthModulewhose wiring is asserted by a NestJS boot test.Renderer
useAniListAuthStore(Zustand, guards missing bridge for the web build) +AccountsSectionsettings panel (pl/en) with connected / disconnected / loading / error states.Verification
pnpm typecheck: cleanpnpm lint: 0 errors (2 pre-existing warnings in unrelated test files)Register a developer app at anilist.co/settings/developer with redirect URI
https://shiroani.app/oauth/anilist, then set the public client id viaANILIST_CLIENT_ID(main) /VITE_ANILIST_CLIENT_ID(web). Until set, Połącz z AniList fails with a clear "not configured" error — the rest of the app is unaffected.Follow-ups (not in this PR)
Two-way list/progress/score sync, auto-scrobble write-back on detected episode completion, and auto cover/ID resolution — all of which this OAuth foundation unlocks.