Add Expo authentication and notes authorization workflow#1
Add Expo authentication and notes authorization workflow#1
Conversation
There was a problem hiding this comment.
Pull request overview
Adds session-based authentication to the Expo app, gates the existing animals area behind authenticated sessions, and introduces private notes with ownership-based authorization.
Changes:
- Introduces users/sessions schema + register/login/logout/current-user APIs using secure, httpOnly cookies.
- Updates animals data access and API routes to require a valid session and adds client redirects on 401.
- Adds notes table, notes API routes, and UI screens (list, detail, create) scoped to the logged-in user.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| util/validation.ts | Adds Zod error aggregation + returnTo sanitization helper for auth redirects. |
| util/cookies.ts | Centralizes secure session cookie options and (de)serialization helpers. |
| pnpm-lock.yaml | Locks new dependencies used for auth (bcryptjs, cookie). |
| package.json | Adds bcryptjs and cookie dependencies for auth/session management. |
| migrations/00004-createTableNotes.ts | Introduces notes table + Zod schema/type for notes payloads. |
| migrations/00003-createTableSessions.ts | Introduces sessions table for session-token auth. |
| migrations/00002-createTableUsers.ts | Introduces users table + Zod schemas/types for auth payloads. |
| database/users.ts | Adds user queries for session-backed current-user and auth flows. |
| database/sessions.ts | Adds session creation/validation/deletion queries. |
| database/notes.ts | Adds note CRUD queries scoped to session owner. |
| database/animals.ts | Adds session-gated animal CRUD queries alongside insecure versions. |
| components/NoteItem.tsx | UI component for rendering a note card linking to detail view. |
| components/AnimalItem.tsx | Adds 401 handling on delete to redirect to login. |
| app/notes/newNote.tsx | New note creation screen with auth guard + POST to notes API. |
| app/notes/[noteId].tsx | Note detail modal screen fetching note by id with auth handling. |
| app/api/user+api.ts | Current-user endpoint used by auth guards and profile screen. |
| app/api/notes/index+api.ts | Notes list + create endpoints enforcing authentication. |
| app/api/notes/[noteId]+api.ts | Note-by-id endpoint enforcing ownership-based authorization. |
| app/api/animals/index+api.ts | Animals list + create endpoints updated to require sessions. |
| app/api/animals/[animalId]+api.ts | Animal read/update/delete endpoints updated to require sessions. |
| app/animals/new.tsx | Adds auth guard + handles auth failures when creating animals. |
| app/animals/[animalId].tsx | Adds 401 handling for fetch/update/delete flows with login redirects. |
| app/_layout.tsx | Adds auth stack group and note modal routes to navigation. |
| app/(tabs)/profile.tsx | New profile tab showing user and enabling logout. |
| app/(tabs)/notes.tsx | New notes tab fetching the user’s private notes. |
| app/(tabs)/animals.tsx | Adds 401 redirect handling when loading animals list. |
| app/(tabs)/_layout.tsx | Adds Notes and Profile tabs + “add note” header action. |
| app/(auth)/register.tsx | Register screen with returnTo handling and auto-redirect when logged in. |
| app/(auth)/login.tsx | Login screen with returnTo handling and auto-redirect when logged in. |
| app/(auth)/api/register+api.ts | Register API: create user, create session, set session cookie. |
| app/(auth)/api/logout+api.ts | Logout API: delete session and clear session cookie. |
| app/(auth)/api/login+api.ts | Login API: verify password, create session, set session cookie. |
| app/(auth)/_layout.tsx | Auth stack layout for login/register screens. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| notes (user_id, title, text_content) ( | ||
| SELECT | ||
| sessions.user_id, | ||
| ${newNote.title}, | ||
| ${newNote.textContent} | ||
| FROM | ||
| sessions | ||
| WHERE | ||
| sessions.token = ${sessionToken} | ||
| AND sessions.expiry_timestamp > now() | ||
| ) |
There was a problem hiding this comment.
The INSERT ... SELECT statement in createNote has an extra pair of parentheses around the SELECT (notes (...) ( SELECT ... )). PostgreSQL expects INSERT INTO notes (...) SELECT ... without parentheses, so this query will fail at runtime. Remove the parentheses and use the standard INSERT ... SELECT form.
| notes (user_id, title, text_content) ( | |
| SELECT | |
| sessions.user_id, | |
| ${newNote.title}, | |
| ${newNote.textContent} | |
| FROM | |
| sessions | |
| WHERE | |
| sessions.token = ${sessionToken} | |
| AND sessions.expiry_timestamp > now() | |
| ) | |
| notes (user_id, title, text_content) | |
| SELECT | |
| sessions.user_id, | |
| ${newNote.title}, | |
| ${newNote.textContent} | |
| FROM | |
| sessions | |
| WHERE | |
| sessions.token = ${sessionToken} | |
| AND sessions.expiry_timestamp > now() |
| import { z } from 'zod'; | ||
| import type { $ZodIssue } from 'zod/v4/core'; | ||
|
|
||
| export function getCombinedErrorMessage(issues: $ZodIssue[]) { | ||
| return issues.map((issue) => issue.message).join(', '); |
There was a problem hiding this comment.
Avoid importing Zod types from internal paths like 'zod/v4/core' (the $ZodIssue type is not part of the public API and may change between releases). Prefer the public type from 'zod' (eg ZodIssue or z.ZodIssue) so this utility remains stable across Zod upgrades.
| }, | ||
| ); | ||
| const animals = await getAnimals(sessionToken); | ||
|
|
There was a problem hiding this comment.
GET /api/animals returns { animals: [] } with 200 when a sessionToken is present but expired/invalid (because getAnimals filters everything out via EXISTS). This hides authentication failures and breaks the client redirect logic. Consider validating the session token and returning 401 when the session is not valid (not only when the cookie is missing).
| if (!animals || animals.length === 0) { | |
| return ExpoApiResponse.json({ animals: [] }, { status: 401 }); | |
| } |
| @@ -15,23 +16,16 @@ export type AnimalsResponseBodyGet = { | |||
| export async function GET( | |||
| request: Request, | |||
| ): Promise<ExpoApiResponse<AnimalsResponseBodyGet>> { | |||
| // 1. Simple way to read cookies | |||
| const cookie = request.headers.get('cookie'); | |||
| console.log('cookie:', cookie); | |||
| const cookies = parse(request.headers.get('cookie') || ''); | |||
| const sessionToken = cookies.sessionToken; | |||
|
|
|||
| const animals = await getAnimalsInsecure(); | |||
| if (!sessionToken) { | |||
| return ExpoApiResponse.json({ animals: [] }, { status: 401 }); | |||
| } | |||
There was a problem hiding this comment.
AnimalsResponseBodyGet is typed as always returning { animals: Animal[] }, but this handler also returns a 401 response. Either include an error variant in the response type (like other endpoints) or always return a consistent error shape for 401 to avoid misleading typing for API consumers.
| userWithPasswordHash.passwordHash = ''; | ||
|
|
There was a problem hiding this comment.
Because getUserWithPasswordHashInsecure is wrapped in react cache(), mutating the returned object (userWithPasswordHash.passwordHash = '') can have surprising effects if the cached value is reused within the same request. Prefer not mutating cached DB results; instead, avoid returning the password hash from this function (eg map/alias in SQL) or create a separate object for the response without modifying the fetched record.
| userWithPasswordHash.passwordHash = ''; |
| if (!animal) { | ||
| return ExpoApiResponse.json( | ||
| { | ||
| error: 'Cannot delete animal', | ||
| }, | ||
| { | ||
| status: 500, | ||
| status: 401, | ||
| }, |
There was a problem hiding this comment.
DELETE returns status 401 when deleteAnimal returns no row (eg expired/invalid session), but 'Cannot delete animal' reads like an authorization/business failure. Consider returning an auth-specific message when responding with 401 so the UI can distinguish auth expiry from other errors.
| export async function GET( | ||
| request: Request, | ||
| ): Promise<ExpoApiResponse<NotesResponseBodyGet>> { | ||
| const cookies = parse(request.headers.get('cookie') || ''); | ||
| const sessionToken = cookies.sessionToken; | ||
|
|
||
| if (!sessionToken) { | ||
| return ExpoApiResponse.json( | ||
| { error: 'Authentication required' }, | ||
| { status: 401 }, | ||
| ); | ||
| } | ||
|
|
||
| const notes = await getNotes(sessionToken); | ||
|
|
||
| return ExpoApiResponse.json({ notes }); |
There was a problem hiding this comment.
GET /api/notes only checks for the presence of a sessionToken cookie; if the token is expired/invalid, getNotes will just return an empty array and this endpoint will respond 200. This makes the client treat an unauthenticated state as "no notes" instead of redirecting. Consider validating the session (eg via getUser/getValidSession) and returning 401 when the session is not valid.
| const cookies = parse(request.headers.get('cookie') || ''); | ||
| const sessionToken = cookies.sessionToken; | ||
|
|
||
| if (!sessionToken) { | ||
| return ExpoApiResponse.json( | ||
| { | ||
| error: 'Authentication required', | ||
| }, | ||
| { | ||
| status: 401, | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| const animal = await getAnimal(sessionToken, Number(animalId)); | ||
|
|
||
| if (!animal) { | ||
| const existingAnimal = await getAnimalInsecure(Number(animalId)); | ||
|
|
||
| return ExpoApiResponse.json( | ||
| { | ||
| error: `No animal with id ${animalId} found`, | ||
| error: existingAnimal | ||
| ? 'Access denied' | ||
| : `No animal with id ${animalId} found`, | ||
| }, | ||
| { | ||
| status: 404, | ||
| status: existingAnimal ? 403 : 404, | ||
| }, |
There was a problem hiding this comment.
In GET /api/animals/[animalId], a present-but-expired/invalid session token will cause getAnimal(...) to return undefined and then fall through to the existingAnimal check, returning 403 "Access denied". Since animals aren't per-user resources, this should be treated as an authentication failure (401) rather than authorization (403). Validate the session first (eg getValidSession) or change the logic so invalid sessions return 401 consistently.
app/api/notes/[noteId]+api.ts
Outdated
| if (!(await selectNoteExists(parsedNoteId))) { | ||
| return ExpoApiResponse.json( | ||
| { error: `No note with id ${noteId} found` }, | ||
| { status: 404 }, | ||
| ); | ||
| } | ||
|
|
||
| const note = await getNote(sessionToken, parsedNoteId); | ||
|
|
||
| if (!note) { | ||
| return ExpoApiResponse.json( | ||
| { error: `Access denied to note with id ${noteId}` }, | ||
| { status: 403 }, | ||
| ); |
There was a problem hiding this comment.
This endpoint makes a separate selectNoteExists(...) call before getNote(...), which both adds an extra query and leaks note existence via 404 vs 403 to any authenticated user. Consider doing a single authorized SELECT (join on sessions) and returning 404 when no row is returned, to avoid enumeration and reduce DB round-trips.
| if (!note || errorMessage) { | ||
| return ( | ||
| <View style={styles.container}> | ||
| <Text style={styles.errorTitle}>Error loading note</Text> | ||
| <Text style={styles.errorText}>{errorMessage}</Text> | ||
| <Link href="/(tabs)/notes" style={styles.backLink}> | ||
| Back to notes | ||
| </Link> |
There was a problem hiding this comment.
The render guard if (!note || errorMessage) will show the "Error loading note" UI on the initial render (when note is still undefined and no error has occurred yet), leading to an error flash with an empty message. Consider separating "loading" vs "error" states (eg show a loading indicator when note is undefined and errorMessage is undefined).
Summary
Testing