feat(website): replace auth-astro with better-auth #1191
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,38 @@ | ||||||
| import { betterAuth } from 'better-auth'; | ||||||
|
|
||||||
| import { getGitHubClientId, getGitHubClientSecret } from './config'; | ||||||
|
|
||||||
| export const auth = betterAuth({ | ||||||
| // TODO - maybe we can check again if this is read automatically? Should be, according to the docs. | ||||||
|
|
||||||
| secret: process.env.AUTH_SECRET, | ||||||
| session: { | ||||||
|
Contributor
There was a problem hiding this comment. 🔵 nit: Consider making session: {
expiresIn: 60 * 60 * 24 * 7, // 7 days
cookieCache: { ... },
}, |
||||||
| cookieCache: { | ||||||
| enabled: true, | ||||||
| maxAge: 60 * 60, | ||||||
| strategy: 'jwe', | ||||||
| }, | ||||||
|
Contributor
There was a problem hiding this comment. 🔴 bug: Missing The stateless session docs explicitly show cookieCache: {
enabled: true,
maxAge: 60 * 60,
strategy: 'jwe',
refreshCache: true, // ← add this
}, |
||||||
| }, | ||||||
|
Contributor
There was a problem hiding this comment. 🟡 risk: No account: {
storeAccountCookie: true,
}, |
||||||
| user: { | ||||||
| additionalFields: { | ||||||
| githubId: { | ||||||
| type: 'string', | ||||||
| input: false, | ||||||
| }, | ||||||
| }, | ||||||
| }, | ||||||
| socialProviders: { | ||||||
| github: { | ||||||
| clientId: getGitHubClientId(), | ||||||
| clientSecret: getGitHubClientSecret(), | ||||||
| // store the GitHub user ID (i.e. 45882389) in the 'user' object, | ||||||
| // which is easy to access from context later on. | ||||||
| // the 'id' property cannot be overwritten. | ||||||
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-conversion -- profile.id is typed as string but GitHub returns a number at runtime; String() ensures it's always stored as a string for consistent ownership checks | ||||||
| mapProfileToUser: (profile) => ({ githubId: String(profile.id) }), | ||||||
| }, | ||||||
| }, | ||||||
| advanced: { | ||||||
| trustedProxyHeaders: true, | ||||||
| cookiePrefix: 'gen-spectrum', | ||||||
|
Contributor
There was a problem hiding this comment. Nitpick: I think usually we use either GenSpectrum or genspectrum
Suggested change
|
||||||
| }, | ||||||
| }); | ||||||
|
Contributor
There was a problem hiding this comment. 🔵 nit: better-auth logs to console by default. The project uses winston via import { getInstanceLogger } from './logger';
const logger = getInstanceLogger('auth');
export const auth = betterAuth({
// ...
logger: {
log: (level, message, ...args) => {
logger[level === 'warn' ? 'warn' : level === 'error' ? 'error' : 'info'](message, ...args);
},
},
});Low priority — but without it, auth errors/warnings go to stdout while everything else goes through structured logging. |
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,5 @@ | ||
| import { auth } from '../auth'; | ||
|
|
||
| export async function logout(request: Request) { | ||
| return auth.api.signOut({ headers: request.headers, asResponse: true }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,11 @@ | ||
| import { createAuthClient } from 'better-auth/client'; | ||
|
Contributor
There was a problem hiding this comment. The docs mention that there is also a specific import for React: Did you try whether it makes a difference? The docs don't say what to expect. |
||
|
|
||
| import { getClientLogger } from '../../clientLogger.ts'; | ||
| import { getErrorLogMessage } from '../../util/getErrorLogMessage.ts'; | ||
| import { useErrorToast } from '../ErrorReportInstruction.tsx'; | ||
|
|
||
| const logger = getClientLogger('LoginButton'); | ||
| const authClient = createAuthClient(); | ||
|
|
||
| export function LoginButton() { | ||
| const { showErrorToast } = useErrorToast(logger); | ||
|
|
@@ -13,13 +14,18 @@ export function LoginButton() { | |
| const callbackUrlThatDoesNotImmediatelyLogoutAgain = new URL(window.location.href).pathname.endsWith('/logout') | ||
| ? new URL('/', window.location.href).toString() | ||
| : undefined; | ||
| authClient.signIn | ||
| .social({ | ||
| provider: 'github', | ||
| callbackURL: callbackUrlThatDoesNotImmediatelyLogoutAgain, | ||
| }) | ||
| .catch((error: unknown) => { | ||
| showErrorToast({ | ||
| error: error instanceof Error ? error : new Error(String(error)), | ||
| logMessage: `Login failed: ${getErrorLogMessage(error)}`, | ||
| errorToastMessages: ['Login failed. Please try again.'], | ||
| }); | ||
|
Comment on lines
+17
to
+27
Contributor
There was a problem hiding this comment. Preexisting issue, maybe not worth fixing here: I wonder why we didn't put this into a |
||
| }); | ||
| }; | ||
|
|
||
| return ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,8 +1,16 @@ | ||||||
| /// <reference path="../.astro/types.d.ts" /> | ||||||
| /// <reference types="astro/client" /> | ||||||
|
|
||||||
| declare namespace App { | ||||||
| // Note: 'import {} from ""' syntax does not work in .d.ts files. | ||||||
|
Contributor
There was a problem hiding this comment. AI claims that it works syntactically, but that such an import would turn the file into a module which apparently break the global availability of the declared types. |
||||||
| // We derive the User type from the auth config so that additionalFields (e.g. githubId) are included. | ||||||
| type AuthUser = NonNullable<Awaited<ReturnType<(typeof import('./auth').auth)['api']['getSession']>>>['user']; | ||||||
|
|
||||||
| interface Locals { | ||||||
| user: AuthUser | null; | ||||||
|
Contributor
There was a problem hiding this comment. https://better-auth.com/docs/integrations/astro#astro-locals-types has this:
Suggested change
|
||||||
| session: import('better-auth').Session | null; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| interface ImportMetaEnv { | ||||||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,11 @@ | ||
| import { sequence } from 'astro:middleware'; | ||
|
|
||
| import { authMiddleware } from './middleware/authMiddleware.ts'; | ||
| import { errorMiddleware } from './middleware/errorMiddleware.ts'; | ||
| import { loggingMiddleware } from './middleware/loggingMiddleware.ts'; | ||
| import setupDayjs from './util/setupDayjs.ts'; | ||
|
|
||
| // Workaround since Astro doesn't seem to support a global startup script for stuff that needs to be done exactly once | ||
| setupDayjs(); | ||
|
|
||
| export const onRequest = sequence(errorMiddleware, loggingMiddleware, authMiddleware); |
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.
🟡 risk: No
baseURLconfigured. The docs recommend always setting it explicitly:The app runs on multiple domains (
genspectrum.org,staging.genspectrum.org,localhost:4321). Options:BETTER_AUTH_URLenv var per deployment, orbaseURLobject withallowedHostsfor multi-domain supportWithout it, better-auth infers the URL from the request, which depends on
trustedProxyHeadersbeing correctly set and the proxy forwarding correct headers.