Repository navigation
Add Hackers' Pub GraphQL authentication provider - #155
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds HackersPub GraphQL authentication: start, callback (HTML), and callback API endpoints; in-memory session store with cleanup; server wiring; UI provider dialog; user resolution/creation and session issuance on successful challenge completion. Changes
Sequence DiagramsequenceDiagram
participant User as User/Client
participant App as App Server
participant HP as HackersPub Instance
participant DB as Database
User->>App: POST /api/auth/hackerspub/graphql-start (instance, username)
App->>App: validate instance, create session(state), build verifyUrl
App->>HP: POST /graphql (loginByUsername with verifyUrl)
HP-->>App: LoginChallenge (or error)
App-->>User: 200 OK (prompt user to check email)
User->>HP: Click email link -> redirect to /auth/hackerspub/callback?token=T&code=C&state=S
User->>App: GET /auth/hackerspub/callback
App-->>User: HTML with embedded payload + JS
User->>App: Client POST /api/auth/hackerspub/graphql-callback (token, code, state)
App->>App: verifyAndConsumeHackersPubSession(state)
App->>HP: POST /graphql (completeLoginChallenge with token, code)
HP-->>App: challenge result (account or errors)
App->>DB: lookup user by fediverse handle / migrate legacy / create user & link if needed
App->>DB: insert session token (30d)
App-->>User: 200 OK + Set-Cookie session_token
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/server/hackerspub-sessions.ts (1)
53-60: Consider adding debug logging for consistency with other session stores.The
miauth-sessions.tsversion ofcleanupExpiredSessionslogs the count of removed sessions, which aids debugging. This implementation silently removes expired entries.📝 Optional: Add debug logging
function cleanupExpiredSessions(): void { const now = Date.now(); + let count = 0; for (const [key, session] of sessions) { if (now > session.expiresAt) { sessions.delete(key); + count++; } } + if (count > 0) { + console.debug(`[HackersPub] Cleaned up ${count} expired sessions`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/hackerspub-sessions.ts` around lines 53 - 60, The cleanupExpiredSessions function silently deletes expired entries from the sessions Map; update it to track and log the number of removed sessions for consistency with the miauth-sessions implementation: iterate through sessions (symbol: sessions), increment a counter when deleting an expired session (symbol: cleanupExpiredSessions, sessions.delete), and at the end emit a debug/processLogger.debug log that includes the removed count and optionally the current sessions.size for context.src/routes/auth/hackerspub/-graphql-callback-api.ts (1)
45-64: Consider adding a timeout to the external GraphQL request.The
fetchcall to HackersPub's GraphQL endpoint has no timeout. If the remote server is slow or unresponsive, this could tie up server resources indefinitely.⏱️ Add AbortSignal timeout
const challengeRes = await fetch(graphqlUrl, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ query: ` mutation CompleteLoginChallenge($token: UUID!, $code: String!) { completeLoginChallenge(token: $token, code: $code) { id account { username handle name avatarUrl } } } `, variables: { token, code }, }), + signal: AbortSignal.timeout(10_000), // 10 second timeout });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/auth/hackerspub/-graphql-callback-api.ts` around lines 45 - 64, The fetch call that assigns challengeRes using graphqlUrl to post the CompleteLoginChallenge mutation currently has no timeout; wrap the request with an AbortController (create a controller, pass controller.signal into fetch) and start a timer (setTimeout) to call controller.abort() after a reasonable timeout (e.g., a few seconds), clear the timer on success, and handle the resulting abort/error when awaiting the fetch of the mutation that uses token and code so the server doesn't hang waiting for an unresponsive GraphQL endpoint.src/routes/auth/signin.tsx (1)
359-362: Consider extracting user-facing strings for i18n.Hardcoded strings like "Check your email!" and "We sent a verification link..." should ideally use Lingui for internationalization. This matches the existing pattern in other dialog forms but is technically a gap per coding guidelines.
As per coding guidelines: "Use Lingui (
@lingui/core) for all user-facing text strings and i18n catalog management."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/auth/signin.tsx` around lines 359 - 362, Extract the hardcoded user-facing strings in the signin component into Lingui translations: replace "Check your email!" and the paragraph starting "We sent a verification link..." with Lingui expressions (e.g., use t`...` or <Trans> from `@lingui/macro`) in the JSX where those strings are rendered (the block that currently interpolates @{username.trim()}); import the appropriate Lingui helper at the top of the file and interpolate username.trim() into the translated string using placeholders so the message remains identical in content but backed by the i18n catalog.src/routes/auth/hackerspub/-graphql-start.ts (1)
45-72: Consider adding a timeout to the external GraphQL request.Similar to the callback handler, this
fetchcall lacks a timeout. A slow or unresponsive HackersPub instance could block the request indefinitely.⏱️ Add AbortSignal timeout
const res = await fetch(graphqlUrl, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ query: ` mutation LoginByUsername($username: String!, $locale: Locale!, $verifyUrl: URITemplate!) { loginByUsername(username: $username, locale: $locale, verifyUrl: $verifyUrl) { ... on LoginChallenge { __typename token } ... on AccountNotFoundError { __typename message } } } `, variables: { username, locale: "en", verifyUrl, }, }), + signal: AbortSignal.timeout(10_000), // 10 second timeout });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/auth/hackerspub/-graphql-start.ts` around lines 45 - 72, The GraphQL fetch to graphqlUrl can hang; wrap it with an AbortController timeout: create an AbortController, start a setTimeout that calls controller.abort() after a chosen timeout (e.g., 5s), pass controller.signal into the fetch options, and clear the timeout after fetch completes; handle the abort error in the existing try/catch to return a sensible timeout response. Locate the fetch call that posts the LoginByUsername mutation (variables: username, locale, verifyUrl; graphqlUrl) and modify it to use AbortController and a cleared timer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/routes/auth/hackerspub/-graphql-callback-api.ts`:
- Around line 45-64: The fetch call that assigns challengeRes using graphqlUrl
to post the CompleteLoginChallenge mutation currently has no timeout; wrap the
request with an AbortController (create a controller, pass controller.signal
into fetch) and start a timer (setTimeout) to call controller.abort() after a
reasonable timeout (e.g., a few seconds), clear the timer on success, and handle
the resulting abort/error when awaiting the fetch of the mutation that uses
token and code so the server doesn't hang waiting for an unresponsive GraphQL
endpoint.
In `@src/routes/auth/hackerspub/-graphql-start.ts`:
- Around line 45-72: The GraphQL fetch to graphqlUrl can hang; wrap it with an
AbortController timeout: create an AbortController, start a setTimeout that
calls controller.abort() after a chosen timeout (e.g., 5s), pass
controller.signal into the fetch options, and clear the timeout after fetch
completes; handle the abort error in the existing try/catch to return a sensible
timeout response. Locate the fetch call that posts the LoginByUsername mutation
(variables: username, locale, verifyUrl; graphqlUrl) and modify it to use
AbortController and a cleared timer.
In `@src/routes/auth/signin.tsx`:
- Around line 359-362: Extract the hardcoded user-facing strings in the signin
component into Lingui translations: replace "Check your email!" and the
paragraph starting "We sent a verification link..." with Lingui expressions
(e.g., use t`...` or <Trans> from `@lingui/macro`) in the JSX where those strings
are rendered (the block that currently interpolates @{username.trim()}); import
the appropriate Lingui helper at the top of the file and interpolate
username.trim() into the translated string using placeholders so the message
remains identical in content but backed by the i18n catalog.
In `@src/server/hackerspub-sessions.ts`:
- Around line 53-60: The cleanupExpiredSessions function silently deletes
expired entries from the sessions Map; update it to track and log the number of
removed sessions for consistency with the miauth-sessions implementation:
iterate through sessions (symbol: sessions), increment a counter when deleting
an expired session (symbol: cleanupExpiredSessions, sessions.delete), and at the
end emit a debug/processLogger.debug log that includes the removed count and
optionally the current sessions.size for context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4e86374-9c68-4547-939e-495a180f29e1
📒 Files selected for processing (6)
src/routes/auth/hackerspub/-graphql-callback-api.tssrc/routes/auth/hackerspub/-graphql-callback.tssrc/routes/auth/hackerspub/-graphql-start.tssrc/routes/auth/signin.tsxsrc/server-entry.tssrc/server/hackerspub-sessions.ts
New auth flow using HackersPub's existing GraphQL API (zero changes to HackersPub needed): 1. User enters instance URL (default: hackers.pub) + username 2. Moim calls loginByUsername with verifyUrl pointing to Moim callback 3. HackersPub sends verification email with link to Moim 4. User clicks link → Moim calls completeLoginChallenge → gets session 5. Moim creates/links user from fediverse handle
malkoG
force-pushed
the
feature/hackerspub-auth
branch
from
March 22, 2026 04:22
5b2705d to
e9fe9ca
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/routes/auth/hackerspub/-graphql-callback-api.ts (2)
180-184: CookieSecureflag may block local development over HTTP.The
Secureflag is hardcoded, which is correct for production but will prevent the cookie from being set during local HTTP development. Consider making this conditional based on the environment.♻️ Suggested fix for development compatibility
+ const isSecure = env.federationProtocol === "https"; const headers = new Headers(); headers.set( "Set-Cookie", - `session_token=${sessionToken}; HttpOnly; Secure; Path=/; Max-Age=${30 * 24 * 60 * 60}; SameSite=Lax`, + `session_token=${sessionToken}; HttpOnly;${isSecure ? " Secure;" : ""} Path=/; Max-Age=${30 * 24 * 60 * 60}; SameSite=Lax`, );You'll need to import
envfrom~/server/envif not already present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/auth/hackerspub/-graphql-callback-api.ts` around lines 180 - 184, The Set-Cookie header for session_token is always adding the Secure flag which breaks local HTTP development; modify the cookie construction in the block that sets headers.set("Set-Cookie", ...) so Secure is only appended in production (e.g. check env.NODE_ENV === "production" or a dedicated env flag). Import env from ~/server/env if not present, build the cookie string conditionally (include "; Secure" only when production), and keep all other attributes (HttpOnly, Path, Max-Age, SameSite) unchanged.
143-166: Consider returning the created user from the transaction for clarity.The
user = createdassignment inside the transaction callback relies on closure mutation. While this works, returning the result from the transaction is more explicit and aligns with Drizzle's idiomatic patterns.♻️ Optional refactor for clarity
// Create new user if (!user) { - await db.transaction(async (tx) => { + user = await db.transaction(async (tx) => { const [created] = await tx .insert(users) .values({ handle: proxyHandle, fediverseHandle: handle, displayName, avatarUrl, summary: null, }) .returning(); await tx.insert(userFediverseAccounts).values({ userId: created.id, fediverseHandle: handle, proxyHandle, isPrimary: true, }); - user = created; + return created; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/auth/hackerspub/-graphql-callback-api.ts` around lines 143 - 166, The transaction mutates the outer `user` variable via closure which is implicit; change the `db.transaction(...)` call to return the created user (e.g., return `created` from the async callback) and assign that result to `user` (user = await db.transaction(...)). Update references in the block involving `users`, `userFediverseAccounts`, `proxyHandle`, `handle`, and the transaction callback so the created user is explicitly returned from the transaction and consumed by the outer scope.src/routes/auth/signin.tsx (2)
397-420: Consider addingautoFocusto the instance field when username is pre-filled, or vice versa.Currently
autoFocusis only on the username field (line 418). If users typically need to change the default instance (hackers.pub), having focus start there could improve UX. This is a minor enhancement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/auth/signin.tsx` around lines 397 - 420, Add conditional autofocus so the field that likely needs editing gets focus: when username is prefilled (check the username state/prop), move autoFocus to the instance Input by setting its autoFocus based on username (e.g., autoFocus={!!username && username.trim().length > 0}), and remove or set autoFocus on the username Input to the inverse (use setInstance/setUsername state values to determine). Update the two Input components (the one with id="hp-instance" and id="hp-username") so only the appropriate field receives autoFocus depending on whether username is already populated.
359-362: User-facing strings should use Lingui for i18n.The hardcoded strings here (and throughout the dialog) are not wrapped with Lingui's
tmacro orTranscomponent. While this follows the existing pattern in other provider dialogs, it's inconsistent with the coding guidelines.Consider wrapping user-facing text with Lingui for proper internationalization, e.g.:
import { t, Trans } from "@lingui/macro"; // ... <Trans>Check your email!</Trans>As per coding guidelines: "Use Lingui (
@lingui/core) for all user-facing text strings and i18n catalog management"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/auth/signin.tsx` around lines 359 - 362, Wrap all user-facing strings in the sign-in confirmation dialog with Lingui: import { t, Trans } from "@lingui/macro" and replace the hardcoded text nodes (e.g., "Check your email!" and the following paragraph that mentions username) with <Trans> usage and/or t() for inline messages, ensuring the username variable (username.trim()) is passed into the Trans interpolation rather than concatenated; update the JSX around the <p> elements and the <strong> that currently contains @{username.trim()} to use Trans interpolation so the text becomes i18n-compatible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/auth/hackerspub/-graphql-callback-api.ts`:
- Around line 45-64: The fetch to the external GraphQL endpoint (using
graphqlUrl) can hang indefinitely; wrap the request with a timeout by using an
AbortController/AbortSignal (e.g., AbortSignal.timeout or new AbortController
with setTimeout) when calling fetch so that the request is aborted after a
reasonable interval, update the fetch call that produces challengeRes to pass
the signal, and add error handling to catch AbortError specifically and return a
clear timeout response to the client (or convert it into a controlled error),
keeping the existing variables token and code unchanged.
In `@src/routes/auth/hackerspub/-graphql-start.ts`:
- Around line 48-72: The fetch to graphqlUrl has no timeout and can hang; wrap
the POST request in an AbortController with a configurable timeout (e.g.,
5–10s), pass controller.signal to fetch, and ensure the timer is cleared after
fetch completes; in the surrounding try/catch (where you handle this request)
catch the AbortError specifically and return a timeout-specific response so
callers can distinguish timeouts from other errors — locate the fetch call that
posts the loginByUsername mutation (variables: username, locale, verifyUrl) and
add the AbortController/setTimeout logic around it and the AbortError handling
in the same error path.
- Around line 32-39: Add the missing HACKERSPUB_SESSION_TTL_SECONDS variable to
the environment template so the code reading
process.env.HACKERSPUB_SESSION_TTL_SECONDS (used when computing ttlSeconds
before calling createHackersPubSession) is documented; update .env.example to
include a line defining HACKERSPUB_SESSION_TTL_SECONDS=600 (place it near the
existing OTP_TTL_SECONDS entry) and commit the change so the configuration
variable is clearly documented for developers.
---
Nitpick comments:
In `@src/routes/auth/hackerspub/-graphql-callback-api.ts`:
- Around line 180-184: The Set-Cookie header for session_token is always adding
the Secure flag which breaks local HTTP development; modify the cookie
construction in the block that sets headers.set("Set-Cookie", ...) so Secure is
only appended in production (e.g. check env.NODE_ENV === "production" or a
dedicated env flag). Import env from ~/server/env if not present, build the
cookie string conditionally (include "; Secure" only when production), and keep
all other attributes (HttpOnly, Path, Max-Age, SameSite) unchanged.
- Around line 143-166: The transaction mutates the outer `user` variable via
closure which is implicit; change the `db.transaction(...)` call to return the
created user (e.g., return `created` from the async callback) and assign that
result to `user` (user = await db.transaction(...)). Update references in the
block involving `users`, `userFediverseAccounts`, `proxyHandle`, `handle`, and
the transaction callback so the created user is explicitly returned from the
transaction and consumed by the outer scope.
In `@src/routes/auth/signin.tsx`:
- Around line 397-420: Add conditional autofocus so the field that likely needs
editing gets focus: when username is prefilled (check the username state/prop),
move autoFocus to the instance Input by setting its autoFocus based on username
(e.g., autoFocus={!!username && username.trim().length > 0}), and remove or set
autoFocus on the username Input to the inverse (use setInstance/setUsername
state values to determine). Update the two Input components (the one with
id="hp-instance" and id="hp-username") so only the appropriate field receives
autoFocus depending on whether username is already populated.
- Around line 359-362: Wrap all user-facing strings in the sign-in confirmation
dialog with Lingui: import { t, Trans } from "@lingui/macro" and replace the
hardcoded text nodes (e.g., "Check your email!" and the following paragraph that
mentions username) with <Trans> usage and/or t() for inline messages, ensuring
the username variable (username.trim()) is passed into the Trans interpolation
rather than concatenated; update the JSX around the <p> elements and the
<strong> that currently contains @{username.trim()} to use Trans interpolation
so the text becomes i18n-compatible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d548511-16ba-433e-9838-3c9851bc5b14
📒 Files selected for processing (6)
src/routes/auth/hackerspub/-graphql-callback-api.tssrc/routes/auth/hackerspub/-graphql-callback.tssrc/routes/auth/hackerspub/-graphql-start.tssrc/routes/auth/signin.tsxsrc/server-entry.tssrc/server/hackerspub-sessions.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/routes/auth/hackerspub/-graphql-callback.ts
- src/server/hackerspub-sessions.ts
- src/server-entry.ts
Comment on lines
+45
to
+64
| const challengeRes = await fetch(graphqlUrl, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| query: ` | ||
| mutation CompleteLoginChallenge($token: UUID!, $code: String!) { | ||
| completeLoginChallenge(token: $token, code: $code) { | ||
| id | ||
| account { | ||
| username | ||
| handle | ||
| name | ||
| avatarUrl | ||
| } | ||
| } | ||
| } | ||
| `, | ||
| variables: { token, code }, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Add a timeout to the external GraphQL fetch to prevent request hangs.
The fetch call to HackersPub's GraphQL endpoint has no timeout. If the external server is slow or unresponsive, this request could hang indefinitely, tying up resources.
🛡️ Proposed fix using AbortSignal.timeout
- const challengeRes = await fetch(graphqlUrl, {
+ const challengeRes = await fetch(graphqlUrl, {
+ signal: AbortSignal.timeout(10_000), // 10 second timeout
method: "POST",
headers: { "Content-Type": "application/json" },You may also want to catch AbortError specifically to return a more descriptive error to the client.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const challengeRes = await fetch(graphqlUrl, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ | |
| query: ` | |
| mutation CompleteLoginChallenge($token: UUID!, $code: String!) { | |
| completeLoginChallenge(token: $token, code: $code) { | |
| id | |
| account { | |
| username | |
| handle | |
| name | |
| avatarUrl | |
| } | |
| } | |
| } | |
| `, | |
| variables: { token, code }, | |
| }), | |
| }); | |
| const challengeRes = await fetch(graphqlUrl, { | |
| signal: AbortSignal.timeout(10_000), // 10 second timeout | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ | |
| query: ` | |
| mutation CompleteLoginChallenge($token: UUID!, $code: String!) { | |
| completeLoginChallenge(token: $token, code: $code) { | |
| id | |
| account { | |
| username | |
| handle | |
| name | |
| avatarUrl | |
| } | |
| } | |
| } | |
| `, | |
| variables: { token, code }, | |
| }), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/auth/hackerspub/-graphql-callback-api.ts` around lines 45 - 64,
The fetch to the external GraphQL endpoint (using graphqlUrl) can hang
indefinitely; wrap the request with a timeout by using an
AbortController/AbortSignal (e.g., AbortSignal.timeout or new AbortController
with setTimeout) when calling fetch so that the request is aborted after a
reasonable interval, update the fetch call that produces challengeRes to pass
the signal, and add error handling to catch AbortError specifically and return a
clear timeout response to the client (or convert it into a controlled error),
keeping the existing variables token and code unchanged.
Comment on lines
+32
to
+39
| // Store session for callback verification | ||
| const ttlSecondsEnv = parseInt( | ||
| process.env.HACKERSPUB_SESSION_TTL_SECONDS ?? "600", | ||
| 10, | ||
| ); | ||
| const ttlSeconds = | ||
| isFinite(ttlSecondsEnv) && ttlSecondsEnv > 0 ? ttlSecondsEnv : 600; // 10 min default (email delivery can be slow) | ||
| createHackersPubSession(state, instance, username, ttlSeconds); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if HACKERSPUB_SESSION_TTL_SECONDS is documented in .env.example
rg -n "HACKERSPUB_SESSION_TTL_SECONDS" .env.exampleRepository: moim-social/moim
Length of output: 42
🏁 Script executed:
# Check if .env.example exists and see its content structure
ls -la .env.example
# Show sample of what's in .env.example
head -50 .env.example
# Count total lines
wc -l .env.example
# Check if there are similar SESSION_TTL or TTL variables documented
rg -i "session|ttl" .env.exampleRepository: moim-social/moim
Length of output: 1127
🏁 Script executed:
# Verify the code file exists and contains the environment variable usage
cat -n src/routes/auth/hackerspub/-graphql-start.ts | sed -n '30,45p'Repository: moim-social/moim
Length of output: 857
Add HACKERSPUB_SESSION_TTL_SECONDS to .env.example.
The code uses process.env.HACKERSPUB_SESSION_TTL_SECONDS but this variable is not documented in .env.example, violating the guideline to define all configuration variables in the template. Add it alongside the existing OTP_TTL_SECONDS:
HACKERSPUB_SESSION_TTL_SECONDS=600
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/auth/hackerspub/-graphql-start.ts` around lines 32 - 39, Add the
missing HACKERSPUB_SESSION_TTL_SECONDS variable to the environment template so
the code reading process.env.HACKERSPUB_SESSION_TTL_SECONDS (used when computing
ttlSeconds before calling createHackersPubSession) is documented; update
.env.example to include a line defining HACKERSPUB_SESSION_TTL_SECONDS=600
(place it near the existing OTP_TTL_SECONDS entry) and commit the change so the
configuration variable is clearly documented for developers.
Comment on lines
+48
to
+72
| const res = await fetch(graphqlUrl, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| query: ` | ||
| mutation LoginByUsername($username: String!, $locale: Locale!, $verifyUrl: URITemplate!) { | ||
| loginByUsername(username: $username, locale: $locale, verifyUrl: $verifyUrl) { | ||
| ... on LoginChallenge { | ||
| __typename | ||
| token | ||
| } | ||
| ... on AccountNotFoundError { | ||
| __typename | ||
| query | ||
| } | ||
| } | ||
| } | ||
| `, | ||
| variables: { | ||
| username, | ||
| locale: "en", | ||
| verifyUrl, | ||
| }, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Add a timeout to the external GraphQL fetch.
Similar to the callback handler, this fetch call to the external HackersPub instance has no timeout, risking request hangs.
🛡️ Proposed fix
const res = await fetch(graphqlUrl, {
+ signal: AbortSignal.timeout(10_000), // 10 second timeout
method: "POST",
headers: { "Content-Type": "application/json" },Consider catching AbortError in the catch block to return a timeout-specific error message.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const res = await fetch(graphqlUrl, { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ | |
| query: ` | |
| mutation LoginByUsername($username: String!, $locale: Locale!, $verifyUrl: URITemplate!) { | |
| loginByUsername(username: $username, locale: $locale, verifyUrl: $verifyUrl) { | |
| ... on LoginChallenge { | |
| __typename | |
| token | |
| } | |
| ... on AccountNotFoundError { | |
| __typename | |
| query | |
| } | |
| } | |
| } | |
| `, | |
| variables: { | |
| username, | |
| locale: "en", | |
| verifyUrl, | |
| }, | |
| }), | |
| }); | |
| const res = await fetch(graphqlUrl, { | |
| signal: AbortSignal.timeout(10_000), // 10 second timeout | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ | |
| query: ` | |
| mutation LoginByUsername($username: String!, $locale: Locale!, $verifyUrl: URITemplate!) { | |
| loginByUsername(username: $username, locale: $locale, verifyUrl: $verifyUrl) { | |
| ... on LoginChallenge { | |
| __typename | |
| token | |
| } | |
| ... on AccountNotFoundError { | |
| __typename | |
| query | |
| } | |
| } | |
| } | |
| `, | |
| variables: { | |
| username, | |
| locale: "en", | |
| verifyUrl, | |
| }, | |
| }), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/auth/hackerspub/-graphql-start.ts` around lines 48 - 72, The fetch
to graphqlUrl has no timeout and can hang; wrap the POST request in an
AbortController with a configurable timeout (e.g., 5–10s), pass
controller.signal to fetch, and ensure the timer is cleared after fetch
completes; in the surrounding try/catch (where you handle this request) catch
the AbortError specifically and return a timeout-specific response so callers
can distinguish timeouts from other errors — locate the fetch call that posts
the loginByUsername mutation (variables: username, locale, verifyUrl) and add
the AbortController/setTimeout logic around it and the AbortError handling in
the same error path.
6 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds Hackers' Pub as a fourth auth provider using its existing GraphQL API — zero changes needed to HackersPub's codebase. The flow leverages HackersPub's
loginByUsernamemutation which accepts averifyUrlURI template, allowing Moim to receive the email verification callback directly.Flow: user enters instance URL (default: hackers.pub) + username → Moim calls
loginByUsernamewithverifyUrlpointing to Moim's callback → HackersPub sends verification email → user clicks link → Moim callscompleteLoginChallenge→ gets session + account info → creates/links Moim user.Test plan
/api/auth/hackerspub/graphql-startand shows "Check your email"/auth/hackerspub/callbackwhich completes authusername@instance)Summary by CodeRabbit