feat: Multi-provider social auth, proxy updates, and RBAC fixes (vertex app)#3575
Merged
Conversation
Change isHCaptchaEnabled to allow hCaptcha on production as well as staging, but only if a hCaptcha site key is configured. The function now checks getEnvironment() (lowercased) and requires getHCaptchaSiteKey() to be truthy; the comment was updated to match the new behavior.
Replace the single-provider Google auth component with a new SocialAuthSection that supports Google, GitHub, LinkedIn and X. Extend oauth-session with supported provider types, helpers to resolve redirect URLs, and localStorage helpers to track the last-used provider. Update the login page to use the new social component, display an error banner for oauth failures, and clean the URL after showing the error. Add a redirect from /user/login to /login in next.config and update NEXT_PUBLIC_API_URL in the example env to the vertex staging API. Remove the old google-auth-section component.
Use user's last active module for social auth redirect (falls back to /admin/networks for admin or /home) and import getLastActiveModule in SocialAuthSection. Change middleware authorized callback to always return true. Update API proxy destination from staging-analytics.airqo.net to staging-vertex.airqo.net in both next.config files and add a redirect from /user/home to /home in next.config.mjs.
Extend isHCaptchaEnabled to also return true in development when an hCaptcha site key is present. Previously hCaptcha was only enabled for staging and production, which prevented local/dev testing; this change allows developers to test hCaptcha locally if the site key is configured.
Include PERMISSIONS.NETWORK.VIEW in the role's permission array in src/vertex/core/permissions/constants.ts so the system-wide role (canOverrideOrganization: false) is granted network view access.
Contributor
📝 WalkthroughWalkthroughReplaces Google-only auth with a multi-provider SocialAuthSection and OAuth-session helpers, migrates onboarding checklist syncing to a group-backed API with hooks and Redux propagation, updates token-handoff/middleware redirect logic, and aligns env/rewrite/permission/changelog entries for the release. ChangesMulti-Provider Social Authentication & Onboarding API
Sequence Diagram(s)sequenceDiagram
participant User
participant SocialAuthSection
participant OAuthSession as OAuthSessionUtils
participant OAuthProvider
participant TokenHandoffHandler
User->>SocialAuthSection: click provider button
SocialAuthSection->>OAuthSession: getLastUsedOAuthProvider()
OAuthSession-->>SocialAuthSection: last-used provider
SocialAuthSection->>OAuthSession: setLastUsedOAuthProvider(selected)
SocialAuthSection->>OAuthProvider: window.location.replace(buildOAuthInitiationUrl(...))
OAuthProvider-->>TokenHandoffHandler: redirect callback (with token / success)
TokenHandoffHandler->>TokenHandoffHandler: capture hash, decide router.replace vs full replace
TokenHandoffHandler->>User: navigate to destination
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. Comment |
Contributor
|
New azure vertex changes available for preview here |
![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: 6
🤖 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/vertex/app/changelog.md`:
- Around line 35-44: Update the "Files Modified" list in changelog.md to use
paths relative to the src/vertex directory (remove the leading "src/vertex/"
prefix) so entries like `src/vertex/app/login/page.tsx` become
`app/login/page.tsx`; open src/vertex/app/changelog.md, find the "Files
Modified" section and replace each listed path (`src/vertex/.env.example`,
`src/vertex/app/login/page.tsx`,
`src/vertex/components/features/auth/social-auth-section.tsx`,
`src/vertex/components/features/auth/google-auth-section.tsx`,
`src/vertex/core/auth/oauth-session.ts`,
`src/vertex/core/permissions/constants.ts`, `src/vertex/lib/envConstants.ts`,
`src/vertex/middleware.ts`, `src/vertex/next.config.mjs`) with their relative
counterparts (drop the `src/vertex/` prefix) to follow the vertex changelog
convention.
In `@src/vertex/app/login/page.tsx`:
- Around line 259-266: The email step renders SocialAuthSection which emits
scoped errors but there is no BannerSlot in that branch, so OAuth failures are
invisible; fix by either adding a BannerSlot into the email step JSX where step
=== 'email' is rendered (so scoped banners from SocialAuthSection surface) or
change SocialAuthSection to emit non-scoped banners for the login/email flow
(adjust its error emission to omit scoped: true or provide a prop like
useScopedBanners={false} and wire that into its banner calls); update the
SocialAuthSection usage or implementation accordingly (look for
SocialAuthSection, BannerSlot, and the step === 'email' rendering) to ensure
banners are visible.
- Around line 108-117: The code handling OAuth errors checks authError from
searchParams and clears the URL by replacing it with '/login', which drops any
existing callbackUrl; update the logic (in the block using authError,
searchParams, showBanner, and window.history.replaceState) to build a new URL
that preserves the existing callbackUrl (and any other relevant query params)
while only removing the 'error' param—e.g., reconstruct the current URL via URL
or URLSearchParams, delete the 'error' key, and call
window.history.replaceState({}, '', newUrl) so the callbackUrl remains intact
after clearing the error.
In `@src/vertex/components/features/auth/social-auth-section.tsx`:
- Around line 8-12: The component reads getLastUsedOAuthProvider but never
persists the provider the user just picked before performing the redirect; fix
by saving the selected provider immediately before redirecting (i.e. before the
call that uses buildOAuthInitiationUrl and window.location.assign/redirect), for
example call a persistence helper such as
setLastUsedOAuthProvider(selectedProvider) or
persistSelectedOAuthProvider(selectedProvider) (or update the same storage
mechanism used by getLastUsedOAuthProvider) right before
resolveOAuthRedirectAfterUrl/buildOAuthInitiationUrl is used so the “last used”
ordering/badge reflects this sign-in attempt.
In `@src/vertex/middleware.ts`:
- Line 17: The middleware currently disables auth by returning true from the
authorized callback; replace that with a real session/token check so only
authenticated users pass the gate. Update the authorized callback (the
authorized symbol) to verify NextAuth session or token (e.g., check for a valid
token or session object from getToken/session) and return a boolean based on its
presence (e.g., !!token or !!session), leaving config.matcher as-is so protected
routes are actually gated. Ensure the callback returns false when there is no
valid session/token so unauthenticated requests are redirected or blocked.
In `@src/vertex/next.config.mjs`:
- Around line 21-34: The project has legacy redirects defined in the redirects()
export (sources '/user/login' -> '/login' and '/user/home' -> '/home') in one
Next.js config but not the other; ensure a single source of truth by mirroring
those redirect entries into the missing config or remove the duplicate config so
only the file exporting redirects() contains the two redirect objects. Update
the redirects() implementation to include the two redirect objects (source
'/user/login' destination '/login' and source '/user/home' destination '/home')
in whichever config will be the canonical config, keeping the same permanent:
false flag and exact object shape so behavior is consistent across both builds.
🪄 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: 54984ca9-d47c-4876-a633-85aebad1b217
📒 Files selected for processing (11)
src/vertex/.env.examplesrc/vertex/app/changelog.mdsrc/vertex/app/login/page.tsxsrc/vertex/components/features/auth/google-auth-section.tsxsrc/vertex/components/features/auth/social-auth-section.tsxsrc/vertex/core/auth/oauth-session.tssrc/vertex/core/permissions/constants.tssrc/vertex/lib/envConstants.tssrc/vertex/middleware.tssrc/vertex/next.config.jssrc/vertex/next.config.mjs
💤 Files with no reviewable changes (1)
- src/vertex/components/features/auth/google-auth-section.tsx
Capture the initial OAuth hash and prevent UI unblocking during client-side token handoff: use a ref (isHandlingOAuthRef) to record if the URL initially contains a token, add a shouldUnblock flag so setIsBootstrapping(false) is skipped when the browser is redirected, and only clear the ref when safe. Also add waitForSession to the effect deps and use the ref for the initial loading guard. In middleware, relax server-side authorization for OAuth callback requests (allow when search param 'success' is present) so the client can process the token hash while still enforcing protection client-side if the token is invalid.
Refine TokenHandoffHandler to avoid unnecessary full-page reloads when the client is already on the redirect destination. The handler now compares the current pathname to the redirect target and uses client-side router.replace for same-path redirects (allowing the UI to unblock), while preserving window.location.replace for cross-page redirects and keeping the UI blocked to prevent flashing. Also update changelog to mention the optimized redirect logic.
Contributor
|
New azure vertex changes available for preview here |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/vertex/app/changelog.md (1)
15-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSection count is off by one in “Social Authentication & Routing”.
The heading says
(4), but there are 5 bullets in this block. Adjust the count to keep release notes internally consistent.🤖 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/vertex/app/changelog.md` around lines 15 - 21, Update the section heading "Social Authentication & Routing (4)" to reflect the correct number of changes by changing the count from (4) to (5) so it matches the five bullet points in that block; locate the heading line in changelog where "Social Authentication & Routing (4)" appears and edit it to "Social Authentication & Routing (5)".src/vertex/core/auth/authProvider.tsx (1)
629-639:⚠️ Potential issue | 🟠 MajorStop OAuth redirect when
waitForSession()yields no confirmed user
TokenHandoffHandlerredirects even whenwaitForSession()returnsnull(it falls back toemail = ''and proceeds). That can recreate the handoff race the refactor targets—especially sincemiddleware.tsexplicitly bypasses auth protection for routes containing?success=.... The credential flow insrc/vertex/app/login/page.tsxalready blocks on!session?.user, so the OAuth success branch should do the same and route to/auth-errorinstead of navigating.🤖 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/vertex/core/auth/authProvider.tsx` around lines 629 - 639, TokenHandoffHandler currently continues redirecting even when waitForSession() returns null; change the flow so after awaiting waitForSession() you check if session?.user exists and, if not, stop the handoff and route to /auth-error (log a warning) instead of computing email/getLastActiveModule or using fallbackUrl; only call getLastActiveModule(email) and compute redirectUrl when session.user is present. Ensure this uses the existing waitForSession and getLastActiveModule symbols and updates the redirect logic in TokenHandoffHandler accordingly.
🤖 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/vertex/app/changelog.md`:
- Around line 34-46: Update the changelog header "Files Modified (10)" to
reflect the actual number of listed entries (11) so it matches the diff; locate
the header line containing the exact text "Files Modified (10)" in
src/vertex/app/changelog.md and change the numeric value to "11" so the count
matches the listed items including both next.config.mjs and next.config.js.
In `@src/vertex/middleware.ts`:
- Around line 17-23: The authorized callback in withAuth (callbacks.authorized)
currently grants access whenever req.nextUrl.searchParams.has('success'), which
improperly bypasses token checks across the matcher; change this to only allow
the OAuth hand-off when the request path exactly matches the OAuth callback
route (e.g., compare req.nextUrl.pathname to your callback path used by your
auth provider) or, better, require a server-validated opaque/signed parameter
(verify a signature or nonce tied to a server-side session) before returning
true; update logic in middleware.ts to replace the broad
req.nextUrl.searchParams.has('success') check with either the exact pathname
check or a verification step that validates the signed param and falls back to
Boolean(token) otherwise.
---
Outside diff comments:
In `@src/vertex/app/changelog.md`:
- Around line 15-21: Update the section heading "Social Authentication & Routing
(4)" to reflect the correct number of changes by changing the count from (4) to
(5) so it matches the five bullet points in that block; locate the heading line
in changelog where "Social Authentication & Routing (4)" appears and edit it to
"Social Authentication & Routing (5)".
In `@src/vertex/core/auth/authProvider.tsx`:
- Around line 629-639: TokenHandoffHandler currently continues redirecting even
when waitForSession() returns null; change the flow so after awaiting
waitForSession() you check if session?.user exists and, if not, stop the handoff
and route to /auth-error (log a warning) instead of computing
email/getLastActiveModule or using fallbackUrl; only call
getLastActiveModule(email) and compute redirectUrl when session.user is present.
Ensure this uses the existing waitForSession and getLastActiveModule symbols and
updates the redirect logic in TokenHandoffHandler accordingly.
🪄 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: c1a1011f-d0d4-4d66-b67e-77febf93bbd1
📒 Files selected for processing (6)
src/vertex/app/changelog.mdsrc/vertex/app/login/page.tsxsrc/vertex/components/features/auth/social-auth-section.tsxsrc/vertex/core/auth/authProvider.tsxsrc/vertex/middleware.tssrc/vertex/next.config.js
🚧 Files skipped from review as they are similar to previous changes (3)
- src/vertex/next.config.js
- src/vertex/app/login/page.tsx
- src/vertex/components/features/auth/social-auth-section.tsx
Comment on lines
+17
to
+23
| authorized: ({ req, token }) => { | ||
| // Bypass server-side protection for OAuth callbacks so the client can process the token hash. | ||
| // Client-side AuthWrapper will still enforce protection if the token is invalid. | ||
| if (req.nextUrl.searchParams.has('success')) { | ||
| return true; | ||
| } | ||
| return Boolean(token); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show the middleware file with line numbers around the reported snippet
sed -n '1,120p' src/vertex/middleware.ts | cat -n
# 2) Locate any auth wrapper / withAuth enforcement mentioned in the comments
rg -n "AuthWrapper|withAuth|next-auth|authorized\s*:\s*\(" -S src/vertex src/platform/src src -g'*.ts' -g'*.tsx' || true
# 3) Inspect any config.matcher in the same middleware (to understand route scope)
rg -n "matcher\s*:" -S src/vertex/middleware.ts src/vertex -g'*.ts' -g'*.tsx' || true
# 4) Check how the token is produced/used (if this is next-auth middleware, look for next-auth config)
rg -n "NextAuth|callbacks|jwt|session|token" -S src/vertex src -g'*.ts' -g'*.tsx' || trueRepository: airqo-platform/AirQo-frontend
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Next.js auth gating in server layout
sed -n '1,220p' src/vertex/app/layout.tsx | cat -n
# Inspect any client/server AuthWrapper unauthorized handling
sed -n '330,560p' src/vertex/core/auth/authProvider.tsx | cat -n
# Find how/where ?success=... is used/produced
rg -n "success\\s*=" -S src/vertex -g'*.ts' -g'*.tsx'
rg -n "searchParams\\.(has|get)\\('success'\\)" -S src/vertex -g'*.ts' -g'*.tsx' || true
rg -n "nextUrl\\.searchParams\\.has\\(\"success\"\\)" -S src/vertex -g'*.ts' -g'*.tsx' || true
# Inspect oauth-session logic that comment mentions
sed -n '1,220p' src/vertex/core/auth/oauth-session.ts | cat -n
# Inspect the auth callback route if present (next-auth callback handling or custom OAuth)
rg -n "oauth|callback|authorize|token hash|hash" src/vertex/app -S -g'*.ts' -g'*.tsx'Repository: airqo-platform/AirQo-frontend
Length of output: 26902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the specific route mentioned: /admin/networks/requests
sed -n '1,220p' src/vertex/app/(authenticated)/admin/networks/requests/page.tsx | cat -n
# Find other server-side guards in protected routes (app/(authenticated))
rg -n "getServerSession\\(options\\)|Unauthorized|return NextResponse\\.json\\(\\{ message: \"Unauthorized\"\\}" -S src/vertex/app/'(authenticated)' -g'*.ts' -g'*.tsx'
# Inspect AuthWrapper's redirect behavior around protected routes
rg -n "Unprotected routes|Unauthenticated user on protected route|isPublicRoute|!session" -S src/vertex/core/auth/authProvider.tsx
sed -n '620,760p' src/vertex/core/auth/authProvider.tsx | cat -nRepository: airqo-platform/AirQo-frontend
Length of output: 135
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the referenced protected page
sed -n '1,200p' 'src/vertex/app/(authenticated)/admin/networks/requests/page.tsx' | cat -n
# 2) Find other app/(authenticated) pages that use getServerSession / enforce authorization server-side
rg -n "getServerSession\\(options\\)|getServerSession\\(" -S "src/vertex/app/(authenticated)" -g'*.ts' -g'*.tsx'
# 3) Look for explicit Unauthorized handling in those routes
rg -n "Unauthorized|status:\\s*401|return NextResponse\\.redirect|redirect\\(" -S "src/vertex/app/(authenticated)" -g'*.ts' -g'*.tsx' || true
# 4) Inspect the part of AuthWrapper that blocks protected rendering when there's no session
rg -n "Unauthenticated user on protected route|return <SessionLoadingState>|router\\.push\\('/login'\\)" -S src/vertex/core/auth/authProvider.tsx
sed -n '150,230p' src/vertex/core/auth/authProvider.tsx | cat -nRepository: airqo-platform/AirQo-frontend
Length of output: 5049
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# List authenticated app pages that use server-side session checks
rg -n --no-heading "getServerSession\\(options\\)" src/vertex/app/'(authenticated)' -g'*.ts' -g'*.tsx' \
| head -n 200
# Find pages that *don’t* mention getServerSession at all (heuristic: may rely on client AuthWrapper)
# We'll sample by listing top files under app/(authenticated) and filter out those containing getServerSession.
ls -1 src/vertex/app/'(authenticated)' 2>/dev/null || true
find src/vertex/app/'(authenticated)' -type f \( -name '*.ts' -o -name '*.tsx' \) | head -n 200 > /tmp/auth_files.txt
python3 - <<'PY'
import subprocess, pathlib, re
root = pathlib.Path("src/vertex/app/(authenticated)")
files = [line.strip() for line in open("/tmp/auth_files.txt") if line.strip()]
def has_getserver(f):
# use rg for speed
res = subprocess.run(["rg","-n","--no-heading","getServerSession\\(options\\)",f], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
return res.returncode==0
no_checks=[]
for f in files:
if not has_getserver(f):
no_checks.append(f)
print("Sample files without getServerSession(options):")
for f in no_checks[:60]:
print("-", f)
print("\nTotal sampled:", len(files), "without server check:", len(no_checks))
PY
# Inspect whether AuthWrapper is applied globally to all protected pages
rg -n "AuthWrapper\\s*\\{" -S src/vertex/core/auth/authProvider.tsx
rg -n "<AuthWrapper>\\{children\\}</AuthWrapper>" -S src/vertex/core/auth/authProvider.tsx
rg -n "AuthProvider" -S src/vertex/app -g'*.tsx' | head -n 50Repository: airqo-platform/AirQo-frontend
Length of output: 1839
🏁 Script executed:
#!/bin/bash
set -euo pipefail
targets=(
"src/vertex/app/(authenticated)/admin/layout.tsx"
"src/vertex/app/(authenticated)/admin/networks/page.tsx"
"src/vertex/app/(authenticated)/admin/cohorts/page.tsx"
"src/vertex/app/(authenticated)/admin/grids/page.tsx"
"src/vertex/app/(authenticated)/admin/shipping/page.tsx"
"src/vertex/app/(authenticated)/admin/sites/page.tsx"
"src/vertex/app/(authenticated)/home/page.tsx"
"src/vertex/app/(authenticated)/devices/my-devices/page.tsx"
"src/vertex/app/(authenticated)/devices/overview/page.tsx"
"src/vertex/app/(authenticated)/sites/overview/page.tsx"
)
for f in "${targets[@]}"; do
echo "===== $f ====="
if [ -f "$f" ]; then
sed -n '1,200p' "$f" | cat -n
else
echo "MISSING: $f"
fi
done
# Also inspect the OAuth/session handoff consumer and any mention of "success" in routing
sed -n '1,220p' src/vertex/core/auth/oauth-session.ts | cat -n
rg -n "OAUTH_SUCCESS_PROVIDER_KEY|\\?success=|searchParams\\.get\\('success'\\)|\\bsuccess\\b.*oauth" -S src/vertex -g'*.ts' -g'*.tsx'Repository: airqo-platform/AirQo-frontend
Length of output: 45792
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the RouteGuard implementation used by admin pages
rg -n "export (function|const) RouteGuard|function RouteGuard" -S src/vertex/components/layout/accessConfig -g'*.ts' -g'*.tsx'
fd -t f "route-guard" src/vertex/components/layout/accessConfig || true
sed -n '1,220p' src/vertex/components/layout/accessConfig/route-guard.tsx | cat -n 2>/dev/null || true
sed -n '1,260p' src/vertex/components/layout/accessConfig/route-guard.ts | cat -n 2>/dev/null || true
# Also check if any server-side API/protected endpoints are gated only by middleware
rg -n "getServerSession\\(options\\)|withAuth\\(" -S src/vertex/app/api -g'*.ts' -g'*.tsx' | head -n 200Repository: airqo-platform/AirQo-frontend
Length of output: 6448
Fix ?success= bypass in src/vertex/middleware.ts
withAuth’s callbacks.authorized returns true whenever req.nextUrl.searchParams.has('success'), bypassing Boolean(token) and effectively treating anonymous requests as authorized across the broad config.matcher scope. Client-side guards (e.g., RouteGuard/AuthWrapper) and some server code (e.g., admin/networks/requests/page.tsx re-checks getServerSession) mitigate current data exposure, but this still weakens the server-side auth boundary and is easy to misuse later. Scope the OAuth-hand-off exemption to a server-validated, signed/opaque signal and/or restrict it to the exact callback route instead of any matched route with ?success.**
🤖 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/vertex/middleware.ts` around lines 17 - 23, The authorized callback in
withAuth (callbacks.authorized) currently grants access whenever
req.nextUrl.searchParams.has('success'), which improperly bypasses token checks
across the matcher; change this to only allow the OAuth hand-off when the
request path exactly matches the OAuth callback route (e.g., compare
req.nextUrl.pathname to your callback path used by your auth provider) or,
better, require a server-validated opaque/signed parameter (verify a signature
or nonce tied to a server-side session) before returning true; update logic in
middleware.ts to replace the broad req.nextUrl.searchParams.has('success') check
with either the exact pathname check or a verification step that validates the
signed param and falls back to Boolean(token) otherwise.
Member
Author
There was a problem hiding this comment.
We evaluated routing the OAuth callback exclusively through a public route (like /login or a dedicated /oauth/callback) to avoid this middleware bypass. However, because our backend relies on an Implicit Flow-style handoff (passing the token via the URL hash fragment), the Next.js server cannot read the token.
If we don't bypass the middleware for the exact destination route (e.g., /home), the server forcefully intercepts the callback and throws a 307 redirect to /login. Even if we handle the handoff there and redirect back to /home, it causes a jarring UX degradation where the user's address bar visibly bounces through /login after a successful authentication.
We are intentionally keeping the ?success= bypass as a deliberate UX trade-off. It allows the token hash to reach the client directly on the intended protected route. Data exposure is securely mitigated because our client-side and Server Components (via getServerSession) still strictly evaluate the session before rendering any sensitive data.
Contributor
There was a problem hiding this comment.
@Codebmk, that's a clear and well-reasoned explanation — the hash-fragment constraint is a real limitation of the Implicit Flow model and the server genuinely can't participate in that handoff. The defense-in-depth you described (AuthWrapper blocking rendering client-side + getServerSession re-checks in server components) is a legitimate mitigation, and the UX argument against bouncing through /login is solid.
One lightweight hardening worth considering (not blocking, just a note): if the set of valid OAuth destination paths is small and known (e.g., /home, /admin/*), you could narrow the bypass further by also requiring req.nextUrl.pathname to match one of those paths alongside ?success:
authorized: ({ req, token }) => {
const OAUTH_CALLBACK_PATHS = ['/home', '/admin'];
const isOAuthCallback =
req.nextUrl.searchParams.has('success') &&
OAUTH_CALLBACK_PATHS.some(p => req.nextUrl.pathname.startsWith(p));
if (isOAuthCallback) return true;
return Boolean(token);
},This doesn't eliminate the bypass but reduces its attack surface to the specific routes the OAuth flow legitimately lands on. That said, this is purely optional — your current approach is a deliberate, documented trade-off and the existing guards cover the real risk surface. Acknowledging your design decision here.
✏️ Learnings added
Learnt from: Codebmk
Repo: airqo-platform/AirQo-frontend PR: 3575
File: src/vertex/middleware.ts:17-23
Timestamp: 2026-06-07T07:28:30.942Z
Learning: In `src/vertex/middleware.ts`, the `callbacks.authorized` bypass for `req.nextUrl.searchParams.has('success')` is intentional. The backend uses an Implicit Flow-style OAuth handoff where the token is passed via the URL hash fragment, which the Next.js server cannot read. Without this bypass, the middleware issues a 307 redirect to /login before the client can process the token, causing jarring UX (visible address-bar bounce). Data exposure is mitigated by the client-side `<AuthWrapper />` and server components that call `getServerSession` before rendering sensitive data. Do not flag this bypass as a security issue.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: airqo-platform/AirQo-frontend PR: 0
File: src/platform/AGENTS.md:0-0
Timestamp: 2026-04-20T11:59:09.099Z
Learning: Applies to src/platform/src/**/*.{ts,tsx} : Normalize OAuth tokens with `normalizeOAuthAccessToken` before storing
Learnt from: Codebmk
Repo: airqo-platform/AirQo-frontend PR: 2895
File: src/vertex/app/login/page.tsx:33-37
Timestamp: 2025-09-07T05:38:26.522Z
Learning: In the AirQo Vertex frontend, authentication error handling including toast display is centralized in the useAuth hook (src/vertex/core/hooks/users.ts). The hook's login mutation already includes onError handling with ReusableToast, so UI components using useAuth only need to handle the onSuccess case for navigation. No additional error handling should be added at the UI layer.
Learnt from: CR
Repo: airqo-platform/AirQo-frontend PR: 0
File: src/platform/AGENTS.md:0-0
Timestamp: 2026-04-20T11:59:09.099Z
Learning: Applies to src/platform/src/**/*.{ts,tsx} : After token refresh, update both ApiClient defaults and NextAuth session via the token-refreshed event
Learnt from: CR
Repo: airqo-platform/AirQo-frontend PR: 0
File: src/platform/AGENTS.md:0-0
Timestamp: 2026-04-20T11:59:09.099Z
Learning: Applies to src/platform/src/app/**/*.{ts,tsx} : Server fetches should use Next.js App Router conventions and `cache: 'no-store'` for auth/sensitive reads
Learnt from: CR
Repo: airqo-platform/AirQo-frontend PR: 0
File: src/platform/AGENTS.md:0-0
Timestamp: 2026-04-20T11:59:09.099Z
Learning: Applies to src/platform/src/**/*.{ts,tsx} : Keep NextAuth session tokens out of logs and sanitize error payloads
Learnt from: CR
Repo: airqo-platform/AirQo-frontend PR: 0
File: src/platform/AGENTS.md:0-0
Timestamp: 2026-04-20T11:59:09.099Z
Learning: Applies to src/platform/src/**/*.{ts,tsx} : Treat 401s as auth flow signals; only escalate after refresh attempts fail
Learnt from: Codebmk
Repo: airqo-platform/AirQo-frontend PR: 2955
File: src/platform/src/app/api/auth/[...nextauth]/options.js:2-2
Timestamp: 2025-09-24T07:27:06.228Z
Learning: User clarified that when working on jwt-decode imports in the AirQo frontend, the scope should be limited to the platform directory only, not the entire monorepo including netmanager or vertex directories.
Learnt from: CR
Repo: airqo-platform/AirQo-frontend PR: 0
File: src/platform/AGENTS.md:0-0
Timestamp: 2026-04-20T11:59:09.099Z
Learning: Applies to src/platform/src/**/*.{ts,tsx} : Use silent refresh on 401 with a single-flight queue to prevent parallel token refreshes
Learnt from: Codebmk
Repo: airqo-platform/AirQo-frontend PR: 3572
File: src/vertex/components/features/devices/device-details-modal.tsx:236-242
Timestamp: 2026-06-03T13:33:07.608Z
Learning: In `src/vertex/components/features/devices/device-details-modal.tsx`, the `onSubmit` routing between `onSubmitLocal` and `onSubmitGlobal` is intentionally: AirQo devices (`device.network === 'airqo'`) → `onSubmitLocal` (writes to ThingSpeak), non-AirQo/external devices → `onSubmitGlobal` (uses the soft-add API, no ThingSpeak update). Do not flag this routing as reversed.
Learnt from: OchiengPaul442
Repo: airqo-platform/AirQo-frontend PR: 2923
File: src/platform/package.json:40-40
Timestamp: 2025-09-14T21:47:35.917Z
Learning: PR `#2923` is focused on platform directory changes only - jwt-decode v4 upgrade and auth refactoring. Netmanager directory files are out of scope for this PR.
Learnt from: Codebmk
Repo: airqo-platform/AirQo-frontend PR: 2950
File: src/vertex/core/apis/organizations.ts:4-11
Timestamp: 2025-09-23T07:16:09.735Z
Learning: The AirQo-frontend repository contains multiple separate projects: vertex, platform, and website2. When analyzing PRs, focus only on the specific project being modified to avoid false positives from other projects in the same repository.
Learnt from: Codebmk
Repo: airqo-platform/AirQo-frontend PR: 2950
File: src/vertex/core/apis/organizations.ts:4-11
Timestamp: 2025-09-23T07:16:09.735Z
Learning: The AirQo-frontend repository contains multiple separate projects: vertex, platform, and website2. When analyzing PRs, focus only on the specific project being modified to avoid false positives from other projects in the same repository.
Learnt from: Codebmk
Repo: airqo-platform/AirQo-frontend PR: 3092
File: src/vertex/app/changelog.md:7-45
Timestamp: 2025-12-12T05:19:00.702Z
Learning: In the AirQo-frontend repository, the vertex project's changelog (src/vertex/app/changelog.md) uses paths relative to the src/vertex/ directory (e.g., "app/layout.tsx" instead of "src/vertex/app/layout.tsx"). When verifying changelog entries against PR changes, always check with the full repository path prefix (src/vertex/).
Fetch and sync organization onboarding checklist state from the server and persist updates. - Add API helpers: updateGroupOnboardingApi and getGroupDetailsApi to organizations API. - Update types: add onboarding_checklist to Group and User types. - Extend userSlice with updateActiveGroupOnboarding to update active group and userGroups state. - Revamp WelcomePage: fetch up-to-date group details, separate local (personal) and org checklist state, compute visuallyCompletedSteps, and handle marking/dismissing steps via API calls. Updates also update query cache and dispatch redux updates. Auto-complete logic for steps now accounts for org vs personal scope. - Misc: import/use useAppDispatch and react-query useQuery for group details. This enables backend-driven onboarding checklist for org workspaces while preserving local storage behavior for personal scope.
Replace createSecureApiClient with secureApiProxy in groupsApi and export a groupsApi object for callers. In the Welcome page, switch to importing groupsApi and destructure updateGroupOnboardingApi/getGroupDetailsApi. Add permission, device and cohort hooks to auto-detect existing devices/cohorts and surface those steps as completed (visually and persisted for personal scopes). Change auto-step syncing to sequential calls with error handling, unify how updated onboarding_checklist is read from API responses, and update the react-query cache accordingly. Remove duplicated logic, update effect dependencies, and add a delayed dismiss of the checklist after workspace setup completes.
Introduce useGroupDetails and useUpdateGroupOnboarding hooks (useQuery/useMutation wrappers) in useGroups.ts and refactor the authenticated Home/Welcome page to use them. Replace direct groupsApi calls and a raw useQuery with the new hooks, import Group type, wire mutateAsync for onboarding updates, tighten setQueryData typing, and add the mutation to the effect deps. This centralizes group-related react-query logic and removes unused imports.
Contributor
|
New azure vertex changes available for preview here |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/vertex/app/(authenticated)/home/page.tsx (1)
280-293: ⚖️ Poor tradeoffSequential auto-step syncing may fail partially.
The auto-step sync loop makes individual API calls for each missing step. If one call fails mid-loop, earlier steps succeed while later ones are skipped, leaving the checklist in an inconsistent state. The errors are logged but not surfaced to the user.
Consider whether partial failures are acceptable for your UX, or if you should either abort the main action when auto-sync fails or collect all sync errors and display them.
🤖 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/vertex/app/`(authenticated)/home/page.tsx around lines 280 - 293, The current sequential loop over missingAutoSteps calls updateGroupOnboarding for each step and logs failures but continues, which can cause partial syncs; change the implementation to collect errors rather than silently continue and surface them after attempting all steps: keep using missingAutoSteps and the sequential/await pattern with updateGroupOnboarding (so order is preserved), push any caught error info (use getApiErrorMessage(e)) into an errors array, and after the loop if errors.length > 0 either throw a combined Error or call the user-facing failure handler (e.g., set an error state or show a toast) so the caller of this code (and the user) is aware that auto-step syncing partially failed instead of only logging via logger.error.src/vertex/core/hooks/useGroups.ts (1)
46-53: ⚡ Quick winStrengthen typing for
useGroupDetails.The
optionsparameter and return type are typed asany, which bypasses TypeScript's type safety. Consider using React Query's built-in types for better IDE support and compile-time safety.♻️ Improved typing
+import type { UseQueryOptions } from "`@tanstack/react-query`"; + -export const useGroupDetails = (groupId: string, options?: any) => { - return useQuery<any>({ +export const useGroupDetails = ( + groupId: string, + options?: Omit<UseQueryOptions<any>, 'queryKey' | 'queryFn' | 'enabled'> +) => { + return useQuery({ queryKey: ["groupDetails", groupId], queryFn: () => groupsApi.getGroupDetailsApi(groupId), enabled: !!groupId, ...options, }); };🤖 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/vertex/core/hooks/useGroups.ts` around lines 46 - 53, The useGroupDetails hook currently types options and the return as any; update it to use React Query generics so options is typed as UseQueryOptions<GroupDetails, Error> (or the appropriate error type) and the hook returns UseQueryResult<GroupDetails, Error>; locate useGroupDetails and the queryFn calling groupsApi.getGroupDetailsApi and replace the any types with the concrete GroupDetails interface (or a response type from groupsApi) and the proper UseQueryOptions/UseQueryResult generics to preserve typing and IDE support.src/vertex/core/redux/slices/userSlice.ts (1)
210-220: 💤 Low valueMinor redundancy in optional chaining.
Line 215 uses
state.activeGroup?._idafter line 214 already establishedstate.activeGroup?._idis truthy. The second optional chaining on line 215 is redundant.♻️ Simplified version
updateActiveGroupOnboarding(state, action: PayloadAction<Group['onboarding_checklist']>) { if (state.activeGroup) { state.activeGroup.onboarding_checklist = action.payload; } if (state.activeGroup?._id) { - const groupIndex = state.userGroups.findIndex(g => g._id === state.activeGroup?._id); + const groupIndex = state.userGroups.findIndex(g => g._id === state.activeGroup!._id); if (groupIndex !== -1) { state.userGroups[groupIndex].onboarding_checklist = action.payload; } } },🤖 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/vertex/core/redux/slices/userSlice.ts` around lines 210 - 220, The updateActiveGroupOnboarding reducer has redundant optional chaining when checking state.activeGroup?._id after already verifying state.activeGroup; simplify by grabbing the active group id once (e.g., const activeId = state.activeGroup._id) or by checking state.activeGroup then using state.activeGroup._id (no ?.) in the findIndex call, and keep the rest of the logic that updates state.activeGroup.onboarding_checklist and state.userGroups[groupIndex].onboarding_checklist unchanged; updateActiveGroupOnboarding is the function to change.src/vertex/core/apis/organizations.ts (1)
21-36: 💤 Low valueConsider removing the empty try/catch blocks.
The try/catch blocks in both new methods (and existing ones) just rethrow the error without adding context or handling. This pattern adds no value and obscures the call stack. Consider either adding meaningful error context or removing the try/catch entirely and letting errors propagate naturally.
♻️ Cleaner approach
updateGroupOnboardingApi: async (groupId: string, payload: { action: 'mark_step_complete' | 'dismiss_checklist'; step_id?: string }) => { - try { - const response = await secureApiProxy.patch(`/users/groups/${groupId}/onboarding`, payload, { headers: { 'X-Auth-Type': 'JWT' } }); - return response.data; - } catch (error) { - throw error; - } + const response = await secureApiProxy.patch(`/users/groups/${groupId}/onboarding`, payload, { headers: { 'X-Auth-Type': 'JWT' } }); + return response.data; }, getGroupDetailsApi: async (groupId: string) => { - try { - const response = await secureApiProxy.get(`/users/groups/${groupId}`, { headers: { 'X-Auth-Type': 'JWT' } }); - return response.data; - } catch (error) { - throw error; - } + const response = await secureApiProxy.get(`/users/groups/${groupId}`, { headers: { 'X-Auth-Type': 'JWT' } }); + return response.data; }🤖 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/vertex/core/apis/organizations.ts` around lines 21 - 36, Remove the redundant try/catch wrappers around updateGroupOnboardingApi and getGroupDetailsApi so errors propagate naturally; call secureApiProxy.patch(`/users/groups/${groupId}/onboarding`, ...) and secureApiProxy.get(`/users/groups/${groupId}`, ...) directly and return response.data without catching and rethrowing, or if you want added context, catch errors and rethrow with additional message referencing the function name (updateGroupOnboardingApi / getGroupDetailsApi) and the original error.
🤖 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/vertex/app/`(authenticated)/home/page.tsx:
- Around line 655-657: The scheduled setTimeout that calls updateChecklist({
action: 'dismiss_checklist', dismissed: true }) must be cleaned up to avoid
running after the component unmounts; change the code that creates the timeout
(the setTimeout block in the Home page component in page.tsx) to store the timer
id in a variable (or ref) and call clearTimeout(timerId) in the component's
cleanup (useEffect return or equivalent unmount handler), or guard the callback
by an isMounted flag referenced before calling updateChecklist; ensure you
reference the updateChecklist call and clear the specific timer id created for
that setTimeout.
---
Nitpick comments:
In `@src/vertex/app/`(authenticated)/home/page.tsx:
- Around line 280-293: The current sequential loop over missingAutoSteps calls
updateGroupOnboarding for each step and logs failures but continues, which can
cause partial syncs; change the implementation to collect errors rather than
silently continue and surface them after attempting all steps: keep using
missingAutoSteps and the sequential/await pattern with updateGroupOnboarding (so
order is preserved), push any caught error info (use getApiErrorMessage(e)) into
an errors array, and after the loop if errors.length > 0 either throw a combined
Error or call the user-facing failure handler (e.g., set an error state or show
a toast) so the caller of this code (and the user) is aware that auto-step
syncing partially failed instead of only logging via logger.error.
In `@src/vertex/core/apis/organizations.ts`:
- Around line 21-36: Remove the redundant try/catch wrappers around
updateGroupOnboardingApi and getGroupDetailsApi so errors propagate naturally;
call secureApiProxy.patch(`/users/groups/${groupId}/onboarding`, ...) and
secureApiProxy.get(`/users/groups/${groupId}`, ...) directly and return
response.data without catching and rethrowing, or if you want added context,
catch errors and rethrow with additional message referencing the function name
(updateGroupOnboardingApi / getGroupDetailsApi) and the original error.
In `@src/vertex/core/hooks/useGroups.ts`:
- Around line 46-53: The useGroupDetails hook currently types options and the
return as any; update it to use React Query generics so options is typed as
UseQueryOptions<GroupDetails, Error> (or the appropriate error type) and the
hook returns UseQueryResult<GroupDetails, Error>; locate useGroupDetails and the
queryFn calling groupsApi.getGroupDetailsApi and replace the any types with the
concrete GroupDetails interface (or a response type from groupsApi) and the
proper UseQueryOptions/UseQueryResult generics to preserve typing and IDE
support.
In `@src/vertex/core/redux/slices/userSlice.ts`:
- Around line 210-220: The updateActiveGroupOnboarding reducer has redundant
optional chaining when checking state.activeGroup?._id after already verifying
state.activeGroup; simplify by grabbing the active group id once (e.g., const
activeId = state.activeGroup._id) or by checking state.activeGroup then using
state.activeGroup._id (no ?.) in the findIndex call, and keep the rest of the
logic that updates state.activeGroup.onboarding_checklist and
state.userGroups[groupIndex].onboarding_checklist unchanged;
updateActiveGroupOnboarding is the function to 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: cbb3bf8f-a577-41a6-a233-7e5ee871330e
📒 Files selected for processing (7)
src/vertex/app/(authenticated)/home/page.tsxsrc/vertex/app/changelog.mdsrc/vertex/app/types/groups.tssrc/vertex/app/types/users.tssrc/vertex/core/apis/organizations.tssrc/vertex/core/hooks/useGroups.tssrc/vertex/core/redux/slices/userSlice.ts
✅ Files skipped from review due to trivial changes (2)
- src/vertex/app/types/groups.ts
- src/vertex/app/changelog.md
Contributor
|
New azure vertex changes available for preview here |
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.
What does this PR do?
This PR introduces a comprehensive multi-provider social authentication system, updates our staging API proxy routing, enables dynamic hCaptcha testing across environments, fixes a critical RBAC bug that was preventing System Administrators from accessing network management features, and centralizes the onboarding checklist state for organizations.
Why is this change necessary?
AIRQO_ADMINrole were unable to view the "Sensor Manufacturers", "Shipping" admin panel due to missing network view permissions in the frontend's static role definition.localStorageleading to cross-device synchronization bugs, multi-admin conflicts, and inconsistent UI when resources were created or deleted.Type of Change
Technical Details & Changes
localStorageas the single source of truth for the organization-level onboarding checklist. The frontend now dynamically evaluates resource availability (e.g., existing devices or cohorts) and pre-populates completed steps usingGET /users/groups/:groupIdrequests.Promise.allSettled()requests with serializedfor...ofloops during onboarding state synchronization. This prevents a critical race condition where concurrentNextAuthsession lookups resulted in401 Unauthorizedresponses and unexpected user logouts. Also implemented anisMountedref check to prevent state updates on unmounted components after checklist dismissal.useGroupDetailsanduseUpdateGroupOnboarding) to centralize query management and eliminate redundant Axios client instantiations.SocialAuthSectionthat supports Google, GitHub, LinkedIn, and X. Addedoauth-sessiontracking to remember the user's last-used provider vialocalStorage.middleware.tsauthorized callback. Repointed API proxy destinations fromstaging-analyticstostaging-vertex.airqo.netin Next configs, and added legacy redirects for/user/homeand/user/login.isHCaptchaEnabledinenvConstants.tsto dynamically support local development as long as a valid site key is present.PERMISSIONS.NETWORK.VIEWto theAIRQO_ADMINrole array inconstants.tsto restore access to the Sensor Manufacturers panel.How to test
NEXT_PUBLIC_HCAPTCHA_SITE_KEYis set in your.env./loginpage:/home(or/admin/networksif applicable).AIRQO_ADMIN:/homeas an organization user.Self-Review Checklist
app/changelog.mdto reflect these changes.What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation