feat: sign in / sign up - #81
Merged
Merged
Conversation
* chore: install the @supabase/supabase-js and @supabase/ssr helper packages * feat: add Supabase browser client * feat: add Supabase server client * docs: add .env.example * chore: add .env.local to gitignore * docs: update README with project description, setup instructions, and environment variables * feat: integrate Supabase session refresh into proxy * test: add tests for Supabase browser client, server client, and proxy * chore: ensure .env.example is tracked * fix: log getClaims errors in proxy * test: fix after review * fix: pass response headers through setAll in proxy and add test coverage * feat: proxy route protection (#75) * feat: add page stubs for about, history, sign-in, and sign-up routes * feat: redirect authenticated users away from sign-in/sign-up routes * feat: protect private routes with 401 and redirect expired/invalid sessions * test: add tests for getAuthenticatedUser, Unauthorized page, and proxy auth-route redirects * fix: stricter route matching, error handling, cookie and locale preservation on redirect * fix: wrap getClaims() in try/catch in getAuthenticatedUser * refactor: use ROUTES constants and simplify locale/route matching * fix: forward setAll headers on auth-route redirect * docs: link to Supabase SSR guide in browser and server client files
* feat: add AuthProvider with signOut and reactive auth state * test: add unit tests for AuthProvider and AuthNavigation component * refactor: update ru sign-up label * chore: install zod, react-hook-form, @hookform/resolvers * feat: add sign-in and sign-up forms with zod validation * feat: connect SignIn and SignUp forms to Supabase auth * fix: redirect to home after sign out to leave protected routes * test: cover auth-schemas, auth-card, controlled-text-field, sign-in and sign-up forms * fix: handle rejected Supabase calls in auth flows
AlyaEngineer
added
the
feature-2-auth
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds Supabase authentication with sign-in/sign-up forms, validation, auth context, route protection, localized UI, authenticated history access, and auth-aware Swagger schema saving. Configuration, environment setup, documentation, and tests are updated. ChangesAuthentication foundation
Auth flow and application integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SignInForm
participant Supabase
participant AuthProvider
participant Router
User->>SignInForm: Submit email and password
SignInForm->>Supabase: signInWithPassword(credentials)
Supabase-->>SignInForm: Authentication result
SignInForm->>Router: Navigate to home on success
Supabase-->>AuthProvider: Auth state change
AuthProvider-->>Router: Redirect after sign-out
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
src/utils/auth/get-authenticated-user.test.ts (1)
8-10: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMock should throw to match real
unauthorized()control-flow behavior.
unauthorized()actually throws in production (see companion comment onget-authenticated-user.ts). Mocking it as a no-op here means the test suite can't detect ifunauthorized()calls end up being caught by an enclosingtry/catch, which is precisely the bug found in the implementation.♻️ Suggested mock update
vi.mock('next/navigation', () => ({ - unauthorized: vi.fn(), + unauthorized: vi.fn(() => { + throw new Error('NEXT_HTTP_ERROR_FALLBACK;401'); + }), }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/auth/get-authenticated-user.test.ts` around lines 8 - 10, The next/navigation unauthorized mock should throw when invoked to match production control flow. Update the vi.mock definition for unauthorized in get-authenticated-user tests so calls raise an error rather than acting as a no-op, allowing tests to detect accidental interception by enclosing try/catch logic.src/lib/client.ts (1)
5-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider validating required env vars instead of using non-null assertions.
process.env.NEXT_PUBLIC_SUPABASE_URL!and...PUBLISHABLE_KEY!silently passundefinedintocreateBrowserClientif unset, producing a confusing error deep inside the Supabase SDK rather than a clear failure at the call site. The same pattern is duplicated insrc/lib/server.ts.♻️ Suggested validation helper
+function getSupabaseEnv() { + const url = process.env.NEXT_PUBLIC_SUPABASE_URL; + const key = process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY; + + if (!url || !key) { + throw new Error('Missing NEXT_PUBLIC_SUPABASE_URL or NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY'); + } + + return { url, key }; +} + export function createClient() { - return createBrowserClient( - process.env.NEXT_PUBLIC_SUPABASE_URL!, - process.env.NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY!, - ); + const { url, key } = getSupabaseEnv(); + return createBrowserClient(url, key); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/client.ts` around lines 5 - 10, Update createClient and the corresponding server client setup to validate NEXT_PUBLIC_SUPABASE_URL and NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY before calling createBrowserClient or its server equivalent. Replace the non-null assertions with explicit checks that fail immediately using a clear configuration error when either variable is missing, while preserving the existing client construction for valid values.src/components/swagger-control/swagger-control.test.tsx (1)
25-31: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMake the auth mock configurable and cover signed-out behavior.
The hardcoded authenticated state prevents tests from exercising
SwaggerControl’s unauthenticated branch, where the save control must be hidden and saving must be rejected. MockuseAuthwithvi.fn()and set its return value per test, including a signed-out case.Suggested mock change
vi.mock('`@/providers/auth-provider/AuthProvider`', () => ({ - useAuth: () => ({ - isAuthenticated: true, - isAuthLoading: false, - signOut: vi.fn(), - }), + useAuth: vi.fn(), }));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/swagger-control/swagger-control.test.tsx` around lines 25 - 31, Update the AuthProvider mock used by SwaggerControl tests so useAuth is a vi.fn() with per-test return values instead of a hardcoded authenticated object. Configure authenticated state for existing tests and add a signed-out test verifying the save control is hidden and save attempts are rejected.src/components/controlled-text-field/controlled-text-field.tsx (1)
38-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLocalize the password-toggle
aria-label.The rest of the field's text (label, placeholder, error) goes through
useTranslations('authForm'), but"toggle password visibility"is a hardcoded English string, leaving RU screen-reader users with untranslated a11y text.♻️ Proposed fix
- aria-label="toggle password visibility" + aria-label={t('togglePasswordVisibility')}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/controlled-text-field/controlled-text-field.tsx` at line 38, Update the password visibility toggle in the controlled text field to obtain its aria-label through the existing useTranslations('authForm') flow instead of a hardcoded English string, and add or reuse the corresponding authForm translation key so screen readers receive the localized label.src/app/[locale]/unauthorized.tsx (1)
11-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using
ROUTES.homeinstead of a hardcoded path.Other files (e.g.
sign-up-form.tsx) referenceROUTES.homefor the home route; using the constant here keeps the route source-of-truth consistent.♻️ Suggested change
+import { ROUTES } from '`@/constants/routes`'; import { useRouter } from '`@/i18n/navigation`'; export default function Unauthorized() { const router = useRouter(); useEffect(() => { - router.replace('/'); + router.replace(ROUTES.home); }, [router]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[locale]/unauthorized.tsx at line 11, Update the redirect in the unauthorized page to use the shared ROUTES.home constant instead of the hardcoded "/" path, matching the route source of truth used by sign-up-form.tsx.src/providers/auth-provider/AuthProvider.test.tsx (1)
17-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Object.assigndoesn't capture the callback inmockSupabaseClient.
Object.assign(authStateChangeCallback, callback)copies enumerable own properties, not callable behavior — invokingauthStateChangeCallback(...)afterward would not run the real listener logic. This parameter is currently unused by any test (only{ user }is ever passed), which is why it hasn't caused a failure — the "onAuthStateChange fires" test (Lines 73-100) works around it by hand-rolling a separate mock with a correctcapturedCallback = callbackassignment instead of reusing this helper.Fix the assignment and reuse the helper to remove the duplicated mock setup:
🐛 Proposed fix
function mockSupabaseClient({ authStateChangeCallback, user, }: { - authStateChangeCallback?: (event: string, session: unknown) => void; + authStateChangeCallback?: { current?: (event: string, session: unknown) => void }; user: unknown; }) { const unsubscribe = vi.fn(); const signOut = vi.fn().mockResolvedValue({ error: null }); return { auth: { getUser: vi.fn().mockResolvedValue({ data: { user } }), onAuthStateChange: vi.fn((callback: (event: string, session: unknown) => void) => { if (authStateChangeCallback) { - Object.assign(authStateChangeCallback, callback); + authStateChangeCallback.current = callback; } return { data: { subscription: { unsubscribe } } }; }), signOut, }, } as unknown as ReturnType<typeof createClient>; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/auth-provider/AuthProvider.test.tsx` around lines 17 - 39, Fix mockSupabaseClient so onAuthStateChange stores the supplied callback in a callable captured reference rather than using Object.assign, then update the “onAuthStateChange fires” test to reuse this helper instead of its duplicated hand-rolled Supabase mock. Preserve the existing unsubscribe, getUser, and signOut behavior.src/proxy.test.ts (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
vi.restoreAllMocks()doesn't resetcreateServerClient's call history between tests.unlike mock.mockRestore, vi.restoreAllMocks will not clear mock history or reset the mock implementation for mocks not created via
vi.spyOn. SincecreateServerClienthere is avi.fn()from avi.mock()factory, its.mock.calls/.mock.resultskeep growing across tests in this file. Current assertions happen to be robust to this (first test uses index0, last test usescalls.length - 1, others usemockReturnValueOnce), but it's fragile to test reordering or additions.♻️ Suggested fix
describe('proxy', () => { afterEach(() => { - vi.restoreAllMocks(); + vi.restoreAllMocks(); + vi.mocked(createServerClient).mockClear(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/proxy.test.ts` around lines 20 - 22, Update the test cleanup around createServerClient so its vi.fn call history and mock implementation are reset between tests, using the mock’s reset/clear API in addition to the existing vi.restoreAllMocks() cleanup. Keep the existing restore behavior and ensure each test starts with an isolated createServerClient state.src/proxy.ts (1)
1-86: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMissing
config.matcher— proxy runs on every request, including static assets.Without a matcher, Proxy runs on every request, including static files (_next/static), image optimizations (_next/image), and assets in the public/ folder. This file has no
configexport, so every static asset request incurs a SupabasegetClaims()call and the next-intl pass unnecessarily.⚡ Suggested addition
function matchesRoute(pathname: string, routes: string[]) { const segments = pathname.split('/'); return routes.some((route) => segments.includes(route.replace('/', ''))); } + +export const config = { + matcher: ['/((?!api|_next/static|_next/image|favicon.ico).*)'], +};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/proxy.ts` around lines 1 - 86, Add a Next.js `config` export alongside `proxy` that defines a matcher excluding static assets, image optimization requests, and public-file requests while retaining application route handling. Keep the existing `proxy`, authentication, and internationalization logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`[locale]/unauthorized.tsx:
- Around line 1-15: The Unauthorized component should render the
401/unauthorized UI instead of silently redirecting via router.replace. Remove
the redirect effect and use the project’s existing unauthorized-state component
or presentation within Unauthorized, preserving the page’s client/server
requirements as needed.
In `@src/components/auth-card/auth-card.tsx`:
- Line 1: Remove the unnecessary 'use client' directive from the AuthCard
component so it remains server-compatible; preserve the component implementation
and rely on client callers to establish their own client boundary.
In `@src/components/controlled-text-field/controlled-text-field.tsx`:
- Around line 35-47: Set the password visibility IconButton in
passwordToggleButton to type="button" so it cannot submit surrounding forms;
leave its existing click and mouse-down behavior unchanged.
In `@src/proxy.ts`:
- Around line 13-53: The setAll callback in proxy must recreate intlResponse
with NextResponse.next({ request }) after applying refreshed cookies to
request.cookies, before copying response cookies and headers, so downstream
processing sees the updated session. Update the response variable handling while
preserving the existing cookie and header propagation.
---
Nitpick comments:
In `@src/app/`[locale]/unauthorized.tsx:
- Line 11: Update the redirect in the unauthorized page to use the shared
ROUTES.home constant instead of the hardcoded "/" path, matching the route
source of truth used by sign-up-form.tsx.
In `@src/components/controlled-text-field/controlled-text-field.tsx`:
- Line 38: Update the password visibility toggle in the controlled text field to
obtain its aria-label through the existing useTranslations('authForm') flow
instead of a hardcoded English string, and add or reuse the corresponding
authForm translation key so screen readers receive the localized label.
In `@src/components/swagger-control/swagger-control.test.tsx`:
- Around line 25-31: Update the AuthProvider mock used by SwaggerControl tests
so useAuth is a vi.fn() with per-test return values instead of a hardcoded
authenticated object. Configure authenticated state for existing tests and add a
signed-out test verifying the save control is hidden and save attempts are
rejected.
In `@src/lib/client.ts`:
- Around line 5-10: Update createClient and the corresponding server client
setup to validate NEXT_PUBLIC_SUPABASE_URL and
NEXT_PUBLIC_SUPABASE_PUBLISHABLE_KEY before calling createBrowserClient or its
server equivalent. Replace the non-null assertions with explicit checks that
fail immediately using a clear configuration error when either variable is
missing, while preserving the existing client construction for valid values.
In `@src/providers/auth-provider/AuthProvider.test.tsx`:
- Around line 17-39: Fix mockSupabaseClient so onAuthStateChange stores the
supplied callback in a callable captured reference rather than using
Object.assign, then update the “onAuthStateChange fires” test to reuse this
helper instead of its duplicated hand-rolled Supabase mock. Preserve the
existing unsubscribe, getUser, and signOut behavior.
In `@src/proxy.test.ts`:
- Around line 20-22: Update the test cleanup around createServerClient so its
vi.fn call history and mock implementation are reset between tests, using the
mock’s reset/clear API in addition to the existing vi.restoreAllMocks() cleanup.
Keep the existing restore behavior and ensure each test starts with an isolated
createServerClient state.
In `@src/proxy.ts`:
- Around line 1-86: Add a Next.js `config` export alongside `proxy` that defines
a matcher excluding static assets, image optimization requests, and public-file
requests while retaining application route handling. Keep the existing `proxy`,
authentication, and internationalization logic unchanged.
In `@src/utils/auth/get-authenticated-user.test.ts`:
- Around line 8-10: The next/navigation unauthorized mock should throw when
invoked to match production control flow. Update the vi.mock definition for
unauthorized in get-authenticated-user tests so calls raise an error rather than
acting as a no-op, allowing tests to detect accidental interception by enclosing
try/catch logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bbf0b2a8-c575-4e21-b715-99c2e0a41712
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (39)
.env.example.gitignoreREADME.mdnext.config.tspackage.jsonsrc/app/[locale]/about/page.tsxsrc/app/[locale]/history/page.tsxsrc/app/[locale]/layout.tsxsrc/app/[locale]/sign-in/page.tsxsrc/app/[locale]/sign-up/page.tsxsrc/app/[locale]/unauthorized.test.tsxsrc/app/[locale]/unauthorized.tsxsrc/components/auth-card/auth-card.test.tsxsrc/components/auth-card/auth-card.tsxsrc/components/auth-navigation/auth-navigation.test.tsxsrc/components/auth-navigation/auth-navigation.tsxsrc/components/controlled-text-field/controlled-text-field.test.tsxsrc/components/controlled-text-field/controlled-text-field.tsxsrc/components/sign-in-form/sign-in-form.test.tsxsrc/components/sign-in-form/sign-in-form.tsxsrc/components/sign-up-form/sign-up-form.test.tsxsrc/components/sign-up-form/sign-up-form.tsxsrc/components/swagger-control/swagger-control.test.tsxsrc/components/swagger-control/swagger-control.tsxsrc/components/swagger-editor/swagger-editor.test.tsxsrc/i18n/messages/en.jsonsrc/i18n/messages/ru.jsonsrc/lib/client.test.tssrc/lib/client.tssrc/lib/server.test.tssrc/lib/server.tssrc/providers/auth-provider/AuthProvider.test.tsxsrc/providers/auth-provider/AuthProvider.tsxsrc/proxy.test.tssrc/proxy.tssrc/utils/auth/auth-schemas.test.tssrc/utils/auth/auth-schemas.tssrc/utils/auth/get-authenticated-user.test.tssrc/utils/auth/get-authenticated-user.ts
AlyaEngineer
added
feature-1-header
…ental form submission
lexarudak
approved these changes
Jul 12, 2026
This was referenced Jul 13, 2026
Merged
Merged
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
2 participants
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
Implements authentication end-to-end: Supabase client setup, route protection (401 for private routes, redirect for logged-in users on auth routes, expired-token handling), and working Sign In / Sign Up forms with client-side validation.
Changes Made
Related issue
Closes #21
Feature
Feature 1: App Header
Feature 2: Sign In / Sign Up
Feature 7: General Requirements
Acceptance criteria
Feature 1: App Header
Feature 2: Sign In / Sign Up
Feature 3: Swagger Editor
Feature 4: Swagger Viewer
Feature 5: History and Analytics
Feature 6: About Page
Feature 7: General Requirements
Feature 8: YouTube Video
Screenshots
Sign up form
Sign in form
Header before signing in
Header after signing in + saving form button
401 response when an unauthenticated user tries to access /history
Redirect from /history to the Main page after signing out
Type of change
Checklist
any/@ts-ignoreSummary by CodeRabbit