feat(auth): add basic authentication - #3008
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
WalkthroughAdds a "basic" username/password authentication flow: middleware, a login API, client-side login UI, context/session providers, detection logic, and minor routing/redirect updates to support Basic auth alongside Clerk and Kratos. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser as Browser
participant Middleware as Middleware
participant LoginAPI as "/api/auth/login"
participant Backend as Backend
User->>Browser: Request protected page
Browser->>Middleware: HTTP request
alt Path is public (/login,/registration,/api/auth/login)
Middleware->>Browser: Allow request through
else Has session cookie (ory_kratos_session or authorization)
Middleware->>Browser: Allow request through
else No session cookie
Middleware->>Browser: Redirect to /login?return_to=<encoded>
Browser->>User: Show login form (BasicLogin)
User->>Browser: Submit credentials
Browser->>LoginAPI: POST /api/auth/login with JSON
LoginAPI->>LoginAPI: Build Basic auth header
LoginAPI->>Backend: Forward request (10s timeout)
Backend-->>LoginAPI: Response + set-cookie
LoginAPI-->>Browser: Relay response + set-cookie
Browser->>Browser: Store session cookie and redirect to return_to
end
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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. Review rate limit: 0/1 reviews remaining, refill in 58 minutes and 51 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Around line 60-61: The global "*.png" entry in .gitignore is too broad and
hides all PNG assets; narrow or remove it by restricting the pattern to
generated/test report directories (for example replace "*.png" with
".playwright-mcp/*.png" or remove the line since .playwright-mcp/ is already
ignored), ensuring only generated Artifacts are ignored and legitimate PNG
assets remain trackable.
In `@middleware.basic.ts`:
- Around line 4-7: The public path check in isPublicPath currently uses
publicPaths = ["/login", "/api/", "/registration"] and pathname.startsWith(p)
which misses the exact "/api" root; update the logic in isPublicPath by either
adding "/api" to publicPaths or normalizing the API check (e.g., check p ===
"/api" or use pathname === "/api" || pathname.startsWith("/api/")) so requests
to "/api" are treated as public and won't be redirected to login.
- Around line 17-20: The current check that returns NextResponse.next() whenever
a token query param exists must be replaced with proper validation: instead of
just checking the token variable, call a token verification function (e.g.,
verifyToken or validateJwt) to decode and verify signature/expiry/claims and
only return NextResponse.next() if verification succeeds; on failure or missing
token, return the appropriate NextResponse (401/redirect) and avoid treating
presence as proof of auth. Update middleware.basic.ts to use the verified token
result to set any request context or headers as needed and handle exceptions
from verifyToken to ensure invalid tokens do not bypass auth.
In `@pages/api/`[...paths].ts:
- Around line 11-19: The current flow assumes getAuth(req).sessionClaims?.org_id
exists and that clerkClient.organizations.getOrganization(...) returns an org
with publicMetadata.backend_url, which can cause undefined target and 500s;
update the logic around getAuth and clerkClient.organizations.getOrganization to
(1) check that const orgId = getAuth(req).sessionClaims?.org_id is truthy before
calling clerkClient.organizations.getOrganization, (2) avoid non-null assertions
and wrap the org fetch in a try/catch, and (3) if org is missing or
org.publicMetadata?.backend_url is falsy, return process.env.BACKEND_URL as the
safe fallback; reference getAuth, clerkClient.organizations.getOrganization,
publicMetadata.backend_url and process.env.BACKEND_URL when making changes.
In `@pages/api/auth/login.ts`:
- Around line 13-15: Validate that username and password exist and are non-empty
strings before constructing the Basic auth header: after extracting const {
username, password } = req.body (in the login handler), check typeof username
=== "string" && username.trim() !== "" && typeof password === "string" &&
password.trim() !== ""; if the check fails return a 400 response (e.g.
res.status(400).json({ error: "Missing or invalid credentials" })), and only
then build const basicAuth =
Buffer.from(`${username}:${password}`).toString("base64").
- Around line 30-33: The code currently uses response.headers.get("set-cookie")
which only returns the first cookie; update the logic in pages/api/auth/login.ts
to call response.headers.getSetCookie() to obtain all Set-Cookie values (an
array) and forward them via res.setHeader("set-cookie", cookiesArray). Replace
the setCookie variable and the res.setHeader call that uses response.headers.get
with the array-based approach, and optionally add a small fallback to handle
environments without getSetCookie (e.g., use
response.headers.raw()?.['set-cookie'] if needed).
In `@src/components/Authentication/Basic/BasicAuthContextProvider.tsx`:
- Around line 44-46: The current check in BasicAuthContextProvider that returns
FullPageSkeletonLoader when (!payload) causes an indefinite spinner on non-401
whoami failures; update the render logic in BasicAuthContextProvider to inspect
the whoami response/error (in addition to payload) and, when an error exists and
its status !== 401, render an error UI or retry affordance instead of
FullPageSkeletonLoader (e.g., show an ErrorState component with retry callback
or a message and a "Try again" button). Keep FullPageSkeletonLoader for true
loading states, but replace the unconditional (!payload) return with a
conditional that distinguishes loading, 401 (unauthenticated) flows, and other
errors so users aren't stuck on a spinner.
- Around line 37-41: In BasicAuthContextProvider, remove the
window.location.href mutation from the render path and instead perform the 401
redirect inside a useEffect triggered when error changes; build the return_to
value using encodeURIComponent(window.location.pathname +
window.location.search) and call
window.location.assign(`/login?return_to=${encoded}`) inside that effect,
returning <FullPageSkeletonLoader /> while waiting for the effect to run. Also
ensure non-401 errors do not fall through to the infinite skeleton loader by
rendering an error state or fallback UI when error exists but
error.response?.status !== 401 so the real error is visible instead of hiding it
behind the loader.
In `@src/components/Authentication/Basic/BasicLogin.tsx`:
- Around line 15-40: The handler uses the user-controlled returnTo (from
searchParams.get("return_to")) directly in router.push, enabling open redirects;
before calling router.push in handleSubmit, validate and sanitize returnTo to
allow only safe internal paths (e.g., ensure it is a relative path that starts
with a single "/" and does not start with "//", does not contain "://" or a
hostname, and disallow path-traversal like "../"); if validation fails, fallback
to "/" (or the existing default) and then call router.push with the sanitized
value. Reference symbols: returnTo, searchParams.get, handleSubmit, and
router.push.
- Around line 3-15: The component imports and uses useSearchParams (App Router)
alongside useRouter (Pages Router); remove the useSearchParams import and
replace its usage by reading the return_to param from router.query inside
BasicLogin: derive returnTo from router.query.return_to (handle string |
string[] | undefined) and default to "/" if missing, and remove the
useSearchParams() call and any references to it so the component consistently
uses Pages Router APIs (functions/variables to locate: BasicLogin,
useSearchParams, useRouter, returnTo, router.query).
In `@src/components/Authentication/Kratos/KratosLogin.tsx`:
- Around line 36-37: The code reads an unvalidated query param return_to
(symbol: returnTo) and later performs a redirect using it; prevent
open-redirects by validating returnTo before using it in the navigation/redirect
call (where you currently redirect around line 118). Add a small validator
(e.g., isValidReturnTo) that only allows internal paths (reject values that
start with //, include a different origin, or are absolute URLs) or checks the
origin via new URL(...) against window.location.origin; if the value fails
validation, use a safe fallback (e.g., '/') instead. Update the redirect site to
call this validator and only navigate to returnTo when isValidReturnTo(returnTo)
is true. Ensure to reference the existing returnTo variable and the
redirect/navigation call when making the change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1626768a-8b89-408e-8ac3-30965c6cb8b3
📒 Files selected for processing (17)
.gitignoremiddleware.basic.tsmiddleware.clerk.tsmiddleware.kratos.tspackage.jsonpages/api/[...paths].tspages/api/auth/login.tspages/login/[[...index]].tsxsrc/api/axios.tssrc/components/Authentication/AuthProviderWrapper.tsxsrc/components/Authentication/AuthSessionChecker.tsxsrc/components/Authentication/Basic/BasicAuthContextProvider.tsxsrc/components/Authentication/Basic/BasicAuthSessionChecker.tsxsrc/components/Authentication/Basic/BasicLogin.tsxsrc/components/Authentication/Kratos/KratosLogin.tsxsrc/components/Authentication/useDetermineAuthSystem.tsxsrc/ui/SkeletonLoader/FullPageSkeletonLoader.tsx
adityathebe
force-pushed
the
feat/basic-auth
branch
from
April 30, 2026 10:00
fb3e905 to
67d19c1
Compare
7 tasks
moshloop
marked this pull request as draft
May 1, 2026 06:56
adityathebe
marked this pull request as ready for review
May 1, 2026 15:35
adityathebe
force-pushed
the
feat/basic-auth
branch
from
May 1, 2026 15:35
67d19c1 to
479b2db
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return; | ||
| } | ||
|
|
||
| router.push(returnTo); |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pages/auth-state-checker.tsx`:
- Around line 28-30: The Clerk branch in pages/auth-state-checker.tsx should
wait for Clerk's session load before performing redirects: inside the
ClerkAuthStateChecker flow (or before calling/returning <ClerkAuthStateChecker
/>) check Clerk's isLoaded (or equivalent) and if !isLoaded return null or a
loading placeholder so the existing effect cannot treat an unloaded session as
BAD_SESSION; once isLoaded is true proceed with the existing session checks and
redirects. Ensure you reference the Clerk isLoaded boolean from the Clerk
SDK/session hook used in this file and only run the BAD_SESSION redirect when
isLoaded === true.
In `@src/components/Authentication/Basic/BasicLogin.tsx`:
- Around line 65-68: The error div in the BasicLogin component currently renders
visible text but lacks assistive-tech semantics; update the error rendering (the
JSX block that checks error and returns the div) to include ARIA live-region
attributes such as role="alert" or aria-live="assertive" and aria-atomic="true"
so screen readers announce the auth failure immediately; ensure you only add
those attributes to the element that renders {error} (the error container) so
visual behavior is unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d6ed9bef-ff3b-4ae9-8495-3894a6f150a0
📒 Files selected for processing (18)
.gitignoremiddleware.basic.tsmiddleware.clerk.tsmiddleware.kratos.tspackage.jsonpages/api/[...paths].tspages/api/auth/login.tspages/auth-state-checker.tsxpages/login/[[...index]].tsxsrc/api/axios.tssrc/components/Authentication/AuthProviderWrapper.tsxsrc/components/Authentication/AuthSessionChecker.tsxsrc/components/Authentication/Basic/BasicAuthContextProvider.tsxsrc/components/Authentication/Basic/BasicAuthSessionChecker.tsxsrc/components/Authentication/Basic/BasicLogin.tsxsrc/components/Authentication/Kratos/KratosLogin.tsxsrc/components/Authentication/useDetermineAuthSystem.tsxsrc/ui/SkeletonLoader/FullPageSkeletonLoader.tsx
✅ Files skipped from review due to trivial changes (3)
- pages/login/[[...index]].tsx
- .gitignore
- pages/api/auth/login.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/Authentication/AuthSessionChecker.tsx
- pages/api/[...paths].ts
- middleware.basic.ts
- src/components/Authentication/Kratos/KratosLogin.tsx
Comment on lines
+65
to
+68
| {error && ( | ||
| <div className="rounded-md bg-red-50 p-3 text-sm text-red-700"> | ||
| {error} | ||
| </div> |
Contributor
There was a problem hiding this comment.
Announce login errors to assistive tech.
Add live-region semantics so auth failures are announced immediately.
Proposed fix
- <div className="rounded-md bg-red-50 p-3 text-sm text-red-700">
+ <div
+ role="alert"
+ aria-live="polite"
+ className="rounded-md bg-red-50 p-3 text-sm text-red-700"
+ >
{error}
</div>📝 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
| {error && ( | |
| <div className="rounded-md bg-red-50 p-3 text-sm text-red-700"> | |
| {error} | |
| </div> | |
| import FormSkeletonLoader from "@flanksource-ui/ui/SkeletonLoader/FormSkeletonLoader"; | |
| import Image from "next/image"; | |
| import { useRouter } from "next/router"; | |
| import { FormEvent, useState } from "react"; | |
| export default function BasicLogin() { | |
| const [username, setUsername] = useState(""); | |
| const [password, setPassword] = useState(""); | |
| const [error, setError] = useState<string>(); | |
| const [submitting, setSubmitting] = useState(false); | |
| const router = useRouter(); | |
| const returnTo = "/"; | |
| async function handleSubmit(e: FormEvent) { | |
| e.preventDefault(); | |
| setError(undefined); | |
| setSubmitting(true); | |
| try { | |
| const response = await fetch("/api/auth/login", { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ username, password }) | |
| }); | |
| if (!response.ok) { | |
| const data = await response.json().catch(() => null); | |
| setError( | |
| data?.message || | |
| data?.error || | |
| `${response.status}: ${response.statusText}` | |
| ); | |
| setSubmitting(false); | |
| return; | |
| } | |
| router.push(returnTo); | |
| } catch (err) { | |
| setError(String(err)); | |
| setSubmitting(false); | |
| } | |
| } | |
| return ( | |
| <div className="w-96"> | |
| <div> | |
| <Image | |
| height={288} | |
| width={75} | |
| alt="Mission Control" | |
| src="/images/logo.svg" | |
| className="m-auto h-auto w-72 rounded-8px p-2" | |
| /> | |
| <h2 className="mt-6 text-center text-3xl font-bold tracking-tight text-gray-900"> | |
| Sign In to your account | |
| </h2> | |
| <div className="mt-8 bg-white px-4 pb-8 pt-4 shadow sm:rounded-lg sm:px-10"> | |
| {submitting ? ( | |
| <FormSkeletonLoader /> | |
| ) : ( | |
| <form onSubmit={handleSubmit} className="space-y-6"> | |
| {error && ( | |
| <div | |
| role="alert" | |
| aria-live="polite" | |
| className="rounded-md bg-red-50 p-3 text-sm text-red-700" | |
| > | |
| {error} | |
| </div> | |
| )} | |
| <div> | |
| <label | |
| htmlFor="username" | |
| className="block text-sm font-medium text-gray-700" | |
| > | |
| Username | |
| </label> | |
| <input | |
| id="username" | |
| type="text" | |
| required | |
| autoComplete="username" | |
| value={username} | |
| onChange={(e) => setUsername(e.target.value)} | |
| className="mt-1 block w-full rounded-md border border-gray-300 px-3 py-2 shadow-sm focus:border-blue-500 focus:outline-none focus:ring-blue-500 sm:text-sm" | |
| /> | |
| </div> | |
| <div> | |
| <label | |
| htmlFor="password" | |
| className="block text-sm font-medium text-gray-700" | |
| > | |
| Password | |
| </label> | |
| <input | |
| id="password" | |
| type="password" | |
| required | |
| autoComplete="current-password" | |
| value={password} | |
| onChange={(e) => setPassword(e.target.value)} | |
| className="mt-1 block w-full rounded-md border border-gray-300 px-3 py-2 shadow-sm focus:border-blue-500 focus:outline-none focus:ring-blue-500 sm:text-sm" | |
| /> | |
| </div> | |
| <button | |
| type="submit" | |
| className="flex w-full justify-center rounded-md border border-transparent bg-blue-600 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-blue-700 focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2" | |
| > | |
| Sign in | |
| </button> | |
| </form> | |
| )} | |
| </div> | |
| </div> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Authentication/Basic/BasicLogin.tsx` around lines 65 - 68, The
error div in the BasicLogin component currently renders visible text but lacks
assistive-tech semantics; update the error rendering (the JSX block that checks
error and returns the div) to include ARIA live-region attributes such as
role="alert" or aria-live="assertive" and aria-atomic="true" so screen readers
announce the auth failure immediately; ensure you only add those attributes to
the element that renders {error} (the error container) so visual behavior is
unchanged.
Basic auth middleware was treating every /api/... path as public, which bypassed the middleware for proxied backend API requests. Only the login endpoint needs to be reachable before a session exists, so keep /api/auth/login public and require normal session checks for other API paths. Also make page route matching exact or prefix-safe.
The auth-state-checker page rendered the Clerk session checker for every non-Kratos auth mode. In basic auth builds this called useSession without a ClerkProvider and failed during prerender. Render the Clerk checker only when Clerk auth is active, and use the existing loading fallback for other auth modes.
Comment out the basic-auth middleware shortcut that allowed any request with a token query parameter to skip the session-cookie check. This keeps unauthenticated direct page loads on the normal login redirect path unless a valid session cookie exists.
When the basic auth whoami request failed with a non-401 response, the provider had no payload and fell through to the full-page skeleton indefinitely. Render the existing error page for non-401 failures while preserving the 401 redirect flow.
BasicLogin is rendered from the pages router login page but was reading return_to with next/navigation useSearchParams. This mixed app-router and pages-router APIs and could make the post-login redirect unreliable. Read return_to from next/router query instead, handling array and missing values before defaulting to /.
The basic login return_to parameter comes from the URL and was passed directly to router.push after login. That allowed external or unsafe URL values to control the redirect target. Only accept local absolute paths that start with / but not //, and fall back to / for all other values.
Clerk proxy requests could fail with a missing target when the session had no org id or the org metadata lacked backend_url. The proxy now falls back to BACKEND_URL when org lookup is unavailable or fails. Login redirects now sanitize return_to before using it and Clerk auth-state checking waits for the session state to load before redirecting.
adityathebe
force-pushed
the
feat/basic-auth
branch
from
May 4, 2026 06:19
1f26549 to
7930a63
Compare
adityathebe
enabled auto-merge (squash)
May 4, 2026 06:21
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
3 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
Adds support for basic authentication mode in the UI.
Changes
/api/auth/loginproxy endpointreturn_toredirects consistentlySummary by CodeRabbit
New Features
Refactor
UX