feat: optional Google sign-in - #192
Merged
Merged
Conversation
✅ Deploy Preview for dev-bayanflow ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
documentation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds optional Google sign-in via Supabase OIDC across schema, services, React auth state, UI, CSP/build configuration, and matching tests/docs. ChangesGoogle OIDC Auth via Supabase
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Preview for Bayan Flow Staging ready!
Preview alias |
ayoub3bidi
force-pushed
the
feature/supabase-auth-google
branch
from
June 27, 2026 17:19
3a05616 to
159e91a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
src/pages/LandingPage.test.jsx (1)
52-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the mocked
UserMenuis rendered on the landing page.Right now this stub prevents the suite from catching a regression where the landing-page sign-in entry disappears entirely. Add a simple
getByTestId('user-menu')assertion in the render checks so the PR’s new auth entrypoint stays covered.🤖 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/pages/LandingPage.test.jsx` around lines 52 - 54, The LandingPage test currently mocks UserMenu but never verifies it is actually rendered, leaving the sign-in entrypoint untested. In LandingPage.test.jsx, update the render assertions to include a getByTestId('user-menu') check against the mocked UserMenu so the landing page coverage explicitly confirms the auth entrypoint remains present.src/main.jsx (1)
40-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
AUTH_CALLBACK_PATHfor this route.
authService.getOAuthCallbackUrl()already builds the OAuth redirect fromAUTH_CALLBACK_PATH. Import the same constant here so the router and redirect cannot drift apart later.Suggested fix
import { ThemeProvider } from './contexts/ThemeContext.jsx'; import { AuthProvider } from './contexts/AuthProvider.jsx'; +import { AUTH_CALLBACK_PATH } from './services/authService.js'; @@ - <Route path="/auth/callback" element={<AuthCallback />} /> + <Route path={AUTH_CALLBACK_PATH} element={<AuthCallback />} />🤖 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/main.jsx` at line 40, The auth callback route is hardcoded instead of reusing the shared path constant. Update the router in main.jsx to import and use AUTH_CALLBACK_PATH for the AuthCallback route so it stays aligned with authService.getOAuthCallbackUrl() and cannot drift from the OAuth redirect path.
🤖 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 `@AGENTS.md`:
- Around line 9-16: Update AGENTS.md to remove the time-sensitive auth
rollout/status snapshot or rewrite it to reflect the current post-PR contract.
The current “auth not implemented yet” / “no Supabase project exists” notes are
stale and conflict with this patch, so edit the relevant bullet(s) in AGENTS.md
to keep only durable repo rules and reference the existing auth-related guidance
consistently.
In `@scripts/cspHeaders.js`:
- Around line 87-105: The auth CSP check in assertAuthCspDirectives is
hardcoding a single Supabase host instead of using the configured backend URL.
Update the connect-src validation to derive the required Supabase origin from
the same VITE_SUPABASE_URL used by src/lib/supabaseClient.js (and ensure the
build workflows pass the correct environment value for staging/production), then
validate against that computed origin rather than a fixed string.
In `@src/components/ui/Tooltip.jsx`:
- Around line 39-62: The Tooltip component in `show`, `hide`, and
`clearShowTimeout` leaves a pending hover timer active if the component unmounts
before the delay completes, which can trigger a stale `setVisible(true)` call.
Add an unmount cleanup for `showTimeoutRef` in the `Tooltip` component (for
example via the existing React effect cleanup) so any scheduled timeout is
cleared when the component is removed.
In `@src/components/UserMenu.jsx`:
- Around line 120-132: The landing sign-in CTA in UserMenu should stay fully
visible on small screens instead of collapsing to icon-only. Update the sign-in
button markup in the UserMenu component so the text label tied to
t('auth.sign_in_google') is no longer hidden by the responsive classes on
mobile, while keeping the existing motion.button behavior and Google icon
intact.
In `@src/content/legal/privacy.en.js`:
- Line 32: The privacy copy in privacy.en.js currently hard-codes that the
Supabase PostgreSQL profile row is hosted in the EU, which may be inaccurate
until the project is provisioned. Update the wording in the optional Google
sign-in paragraph to be region-neutral, or change it to the actual Supabase
region once known, so the statement matches the deployed setup. Locate the text
in the privacy policy entry and adjust only that hosting/location claim.
In `@src/contexts/AuthProvider.jsx`:
- Around line 53-64: `refreshProfile` in `AuthProvider` can commit an
out-of-date `profileRow` after `getProfile()` resolves, so a slower fetch for an
old user may overwrite the current session. Update the `refreshProfile` flow,
the hydrate path, and the `onAuthStateChange()` callback so each request is tied
to the latest active user/request token and only calls `setProfileRow` when the
fetched row still matches that latest request. Keep the existing null/reset
handling for signed-out users and stale/error cases.
In `@src/services/authService.js`:
- Around line 77-105: The popup callbacks in authService’s onMessage listener
and pollTimer currently await getSession() without handling rejections, which
can leave the popup promise pending and hide the real error. Update both
callbacks to catch any getSession() failure and route it to fail(error)
immediately, while keeping the existing success path that calls finish() when a
session is returned.
In `@src/test/supabaseMock.js`:
- Around line 46-55: The Supabase mock reset helper only clears call history for
several mocks, so prior return-value stubs can leak into later tests. Update
resetSupabaseMocks to use mockReset on the auth and from mocks in supabaseMock,
then reapply the default implementations for isSupabaseConfigured,
getSupabaseClient, and the Supabase auth/client helpers so tests like
AuthProvider.test.jsx start from a clean baseline.
In `@supabase/migrations/20260627170000_profiles_display_name_avatar_url.sql`:
- Around line 33-46: The current backfill only updates existing public.profiles
rows, so users without a profile row are still missed. Update the migration to
also insert missing profile records for any auth.users entries that do not yet
have a matching public.profiles row, then apply the existing display_name and
avatar_url backfill logic; use the public.profiles and auth.users tables in the
migration so src/services/profileService.js can find a row for every user.
---
Nitpick comments:
In `@src/main.jsx`:
- Line 40: The auth callback route is hardcoded instead of reusing the shared
path constant. Update the router in main.jsx to import and use
AUTH_CALLBACK_PATH for the AuthCallback route so it stays aligned with
authService.getOAuthCallbackUrl() and cannot drift from the OAuth redirect path.
In `@src/pages/LandingPage.test.jsx`:
- Around line 52-54: The LandingPage test currently mocks UserMenu but never
verifies it is actually rendered, leaving the sign-in entrypoint untested. In
LandingPage.test.jsx, update the render assertions to include a
getByTestId('user-menu') check against the mocked UserMenu so the landing page
coverage explicitly confirms the auth entrypoint remains present.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 25c4017a-0fc0-42ae-a659-d3e382f000b0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (45)
.env.example.github/workflows/deploy-cloudflare.yml.github/workflows/preview-cloudflare.ymlAGENTS.mdnetlify.tomlpackage.jsonpublic/_headerspublic/auth.mdscripts/cspHeaders.jssrc/components/Header.jsxsrc/components/UserAvatar.jsxsrc/components/UserAvatar.test.jsxsrc/components/UserMenu.jsxsrc/components/UserMenu.test.jsxsrc/components/ui/Tooltip.jsxsrc/components/ui/Tooltip.test.jsxsrc/content/legal/privacy.en.jssrc/content/legal/privacy.en.test.jssrc/contexts/AuthContextDefinition.jssrc/contexts/AuthProvider.jsxsrc/contexts/AuthProvider.test.jsxsrc/hooks/useAuth.jssrc/i18n/locales/ar/translation.jsonsrc/i18n/locales/en/translation.jsonsrc/i18n/locales/fr/translation.jsonsrc/lib/supabaseClient.jssrc/main.jsxsrc/pages/AuthCallback.jsxsrc/pages/LandingPage.jsxsrc/pages/LandingPage.test.jsxsrc/pages/PrivacyPolicy.test.jsxsrc/security/cspHeaders.test.jssrc/services/authService.jssrc/services/authService.test.jssrc/services/entitlementService.jssrc/services/profileService.jssrc/test/framerMotionMock.jsxsrc/test/setup.jssrc/test/supabaseMock.jssrc/utils/resolveUserAvatar.jssrc/utils/resolveUserAvatar.test.jssupabase/migrations/20260627160007_create_profiles_auth_trigger_rls.sqlsupabase/migrations/20260627160026_revoke_handle_new_user_rpc_execute.sqlsupabase/migrations/20260627170000_profiles_display_name_avatar_url.sqlvite.config.js
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/components/UserAvatar.jsx`:
- Around line 27-37: The avatar fallback logic in UserAvatar currently only
switches on error, so a missing profile.avatarSrc can leave the image blank.
Update the image source handling around useEffect, imageSrc, and usedFallback so
that the DiceBear fallback (fallbackSrc from generateAvatarDataUri) is used
whenever profile.avatarSrc is empty/undefined, not just inside onError. Keep the
state reset in sync when profile.avatarSrc changes so the component immediately
renders the fallback for missing avatars.
In `@src/pages/AuthCallback.jsx`:
- Around line 9-10: The AuthCallback page currently calls Supabase directly and
only handles the success path of getSession, which can leave the popup stuck if
the promise rejects. Update AuthCallback to use authService.getSession() instead
of importing getSupabaseClient, and add rejection handling in the callback flow
so failures still notify the opener and close or redirect appropriately. Keep
the session logic inside the auth service layer and preserve the existing
AUTH_COMPLETE_MESSAGE behavior via the AuthCallback component’s callback
handling.
In `@src/utils/resolveUserAvatar.js`:
- Around line 34-35: The remote avatar URL validator currently allows insecure
http:// values in isHttpUrl, which should be restricted to https:// only. Update
the URL check used by resolveUserAvatar and any related call sites in the avatar
resolution flow to accept only secure https URLs, keeping the existing
string/trim validation intact. Make sure any logic that depends on this helper
(including the referenced avatar resolution paths) continues to reject non-HTTPS
remote avatars.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a6ad3f0e-e21f-4bff-95fe-6e8a4f105668
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (45)
.env.example.github/workflows/deploy-cloudflare.yml.github/workflows/preview-cloudflare.ymlAGENTS.mdnetlify.tomlpackage.jsonpublic/_headerspublic/auth.mdscripts/cspHeaders.jssrc/components/Header.jsxsrc/components/UserAvatar.jsxsrc/components/UserAvatar.test.jsxsrc/components/UserMenu.jsxsrc/components/UserMenu.test.jsxsrc/components/ui/Tooltip.jsxsrc/components/ui/Tooltip.test.jsxsrc/content/legal/privacy.en.jssrc/content/legal/privacy.en.test.jssrc/contexts/AuthContextDefinition.jssrc/contexts/AuthProvider.jsxsrc/contexts/AuthProvider.test.jsxsrc/hooks/useAuth.jssrc/i18n/locales/ar/translation.jsonsrc/i18n/locales/en/translation.jsonsrc/i18n/locales/fr/translation.jsonsrc/lib/supabaseClient.jssrc/main.jsxsrc/pages/AuthCallback.jsxsrc/pages/LandingPage.jsxsrc/pages/LandingPage.test.jsxsrc/pages/PrivacyPolicy.test.jsxsrc/security/cspHeaders.test.jssrc/services/authService.jssrc/services/authService.test.jssrc/services/entitlementService.jssrc/services/profileService.jssrc/test/framerMotionMock.jsxsrc/test/setup.jssrc/test/supabaseMock.jssrc/utils/resolveUserAvatar.jssrc/utils/resolveUserAvatar.test.jssupabase/migrations/20260627160007_create_profiles_auth_trigger_rls.sqlsupabase/migrations/20260627160026_revoke_handle_new_user_rpc_execute.sqlsupabase/migrations/20260627170000_profiles_display_name_avatar_url.sqlvite.config.js
✅ Files skipped from review due to trivial changes (9)
- src/pages/PrivacyPolicy.test.jsx
- src/components/UserAvatar.test.jsx
- src/pages/LandingPage.test.jsx
- src/utils/resolveUserAvatar.test.js
- src/services/profileService.js
- .github/workflows/preview-cloudflare.yml
- src/i18n/locales/ar/translation.json
- src/i18n/locales/en/translation.json
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (26)
- public/_headers
- src/services/authService.test.js
- src/test/framerMotionMock.jsx
- src/components/ui/Tooltip.test.jsx
- src/hooks/useAuth.js
- src/main.jsx
- src/pages/LandingPage.jsx
- netlify.toml
- src/security/cspHeaders.test.js
- package.json
- .github/workflows/deploy-cloudflare.yml
- scripts/cspHeaders.js
- supabase/migrations/20260627160026_revoke_handle_new_user_rpc_execute.sql
- src/components/UserMenu.test.jsx
- src/contexts/AuthContextDefinition.js
- supabase/migrations/20260627160007_create_profiles_auth_trigger_rls.sql
- src/contexts/AuthProvider.test.jsx
- src/i18n/locales/fr/translation.json
- src/content/legal/privacy.en.test.js
- vite.config.js
- public/auth.md
- src/test/setup.js
- src/content/legal/privacy.en.js
- supabase/migrations/20260627170000_profiles_display_name_avatar_url.sql
- src/components/Header.jsx
- src/test/supabaseMock.js
Comment on lines
+9
to
+10
| import { getSupabaseClient } from '@/lib/supabaseClient'; | ||
| import { AUTH_COMPLETE_MESSAGE } from '@/services/authService'; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Catch callback session lookup failures and keep this page on the auth service layer.
Line 24 only handles the resolved path of supabase.auth.getSession(). If that promise rejects, the popup never notifies the opener or closes, so the user can get stranded on /auth/callback. Using authService.getSession() here fixes the repo contract at the same time.
Suggested fix
import { useEffect } from 'react';
import { useTranslation } from 'react-i18next';
-import { getSupabaseClient } from '`@/lib/supabaseClient`';
-import { AUTH_COMPLETE_MESSAGE } from '`@/services/authService`';
+import {
+ AUTH_COMPLETE_MESSAGE,
+ getSession,
+} from '`@/services/authService`';
function AuthCallback() {
const { t } = useTranslation();
useEffect(() => {
- const supabase = getSupabaseClient();
- if (!supabase) {
- window.close();
- return undefined;
- }
-
let isMounted = true;
-
- supabase.auth.getSession().then(({ data: { session }, error }) => {
- if (!isMounted) {
- return;
- }
-
- if (error) {
- console.error('Auth callback failed:', error);
- }
-
- if (session && window.opener && !window.opener.closed) {
- window.opener.postMessage(
- { type: AUTH_COMPLETE_MESSAGE },
- window.location.origin
- );
- }
-
- window.close();
- });
+ const completeAuth = async () => {
+ try {
+ const session = await getSession();
+ if (!isMounted) {
+ return;
+ }
+
+ if (session && window.opener && !window.opener.closed) {
+ window.opener.postMessage(
+ { type: AUTH_COMPLETE_MESSAGE },
+ window.location.origin
+ );
+ }
+ } catch (error) {
+ if (isMounted) {
+ console.error('Auth callback failed:', error);
+ }
+ } finally {
+ if (isMounted) {
+ window.close();
+ }
+ }
+ };
+
+ void completeAuth();
return () => {
isMounted = false;
};
}, []);As per coding guidelines, "Use src/services/authService.js and src/services/profileService.js as the auth service layer; components must use AuthContext/useAuth and never import Supabase directly." Based on learnings, "Use getSession() and onAuthStateChange() for session handling."
Also applies to: 15-41
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 7-10: src/pages/AuthCallback.jsx#L7-L10
Added lines #L7 - L10 were not covered by tests
🤖 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/pages/AuthCallback.jsx` around lines 9 - 10, The AuthCallback page
currently calls Supabase directly and only handles the success path of
getSession, which can leave the popup stuck if the promise rejects. Update
AuthCallback to use authService.getSession() instead of importing
getSupabaseClient, and add rejection handling in the callback flow so failures
still notify the opener and close or redirect appropriately. Keep the session
logic inside the auth service layer and preserve the existing
AUTH_COMPLETE_MESSAGE behavior via the AuthCallback component’s callback
handling.
Sources: Coding guidelines, Learnings
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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/components/GoogleOneTap.jsx`:
- Around line 35-43: Guard the redirect in GoogleOneTap.jsx by re-checking the
effect lifetime after the awaited authService.signInWithGoogleIdToken call
inside initOneTap’s onCredential handler. The issue is that navigation can still
happen after cleanup if the sign-in finishes after the component unmounts or the
effect is invalidated; fix it by verifying isMounted again before calling
navigate('/app') in the onCredential callback.
In `@src/lib/googleIdentity.js`:
- Around line 187-214: The hidden Google sign-in host created in
openGoogleSignInButton is not being removed after the sign-in flow completes,
which leaves stale offscreen DOM nodes behind on repeated attempts. Update the
popup/completion handling around openGoogleSignInButton and the related success,
timeout, and reject paths to always clean up the appended host element. Make
sure any early failure path still removes the host before rejecting, and
preserve the existing click/render behavior while ensuring the hidden node is
detached after use.
In `@src/services/authService.js`:
- Around line 93-100: The sign-in flow in signInWithGoogle only guards on
isGoogleAuthConfigured(), so it can still open the Google popup when Supabase
auth is not fully configured and then fail later. Update signInWithGoogle to
gate the popup on the full auth configuration check used by the rest of the auth
flow, and keep the early throw before requestGoogleSignInPopup is called so the
popup is never launched unless both Google and Supabase auth are ready.
- Around line 29-40: The JWT payload decoding in parseGoogleIdTokenClaims
currently uses atob() directly, which can corrupt non-ASCII Google profile names
before they reach sync logic in authService. Update parseGoogleIdTokenClaims to
decode the base64 payload as UTF-8 before JSON parsing, and keep the existing
fallback behavior for invalid tokens so full_name/name are stored correctly in
the Google sign-in flow.
In `@src/services/authService.test.js`:
- Around line 92-94: The isAuthConfigured test only covers the happy path and
does not exercise the false branches implied by its name. Update
authService.test.js around the isAuthConfigured assertion to also verify cases
where Supabase is present without Google client ID and where Google client ID is
present without Supabase, so the authService.isAuthConfigured() behavior is
validated for both required inputs and not just the combined success case.
- Around line 51-54: Prettier is flagging the JWT payload setup in
authService.test.js, so reformat the payload construction to match the project’s
style. Update the btoa(JSON.stringify(...)) chain in the authService test setup
by breaking it into the preferred multiline layout (or equivalent formatted
form) while keeping the same logic and variable name payload.
In `@src/services/googleTokenExchange.js`:
- Around line 34-40: The token exchange flow in googleTokenExchange.js parses
the response body before checking the HTTP status, which can turn non-JSON error
responses into parse failures. Update the token exchange logic around the
response handling to inspect response.ok first, and only call response.json()
when the body is expected to be JSON; for failures, safely read the error text
or otherwise guard parsing before using payload.error_description in the token
exchange error path.
In `@vitest.config.js`:
- Around line 19-22: The test.env block in vitest.config.js is not formatted to
match Prettier, causing CI to fail. Reformat the env object in the vitest config
so it matches the project’s Prettier style, keeping the
VITE_GOOGLE_WEB_CLIENT_ID entry under the test.env configuration.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 24f2e144-1579-4552-9780-9ffa7026fe02
📒 Files selected for processing (24)
.env.example.github/workflows/deploy-cloudflare.yml.github/workflows/preview-cloudflare.ymlAGENTS.mdnetlify.tomlpublic/_headerspublic/auth.mdscripts/cspHeaders.jssrc/components/GoogleOneTap.jsxsrc/components/GoogleOneTap.test.jsxsrc/components/UserAvatar.jsxsrc/constants/pyodideCdn.test.jssrc/contexts/AuthProvider.jsxsrc/lib/googleIdentity.jssrc/lib/googleIdentity.test.jssrc/lib/supabaseClient.jssrc/main.jsxsrc/pages/GoogleAuthCallback.jsxsrc/services/authService.jssrc/services/authService.test.jssrc/services/googleTokenExchange.jssrc/services/googleTokenExchange.test.jssrc/test/supabaseMock.jsvitest.config.js
✅ Files skipped from review due to trivial changes (4)
- src/services/googleTokenExchange.test.js
- .env.example
- AGENTS.md
- public/auth.md
🚧 Files skipped from review as they are similar to previous changes (4)
- src/lib/supabaseClient.js
- public/_headers
- src/components/UserAvatar.jsx
- src/contexts/AuthProvider.jsx
…d Supabase URL in CSP - Add test files for supabaseClient, GoogleAuthCallback, entitlementService (new) - Expand tests for AuthProvider, GoogleOneTap, UserMenu, Tooltip, authService, resolveUserAvatar (edge cases, error paths, state branches) - Replace hardcoded Supabase project URL in cspHeaders.js assertion with dynamic env read via process.env.VITE_SUPABASE_URL - Fix formatting in vitest.config.js
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/cspHeaders.js (1)
85-113: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon't make auth CSP validation unconditionally depend on
VITE_SUPABASE_URL.These helpers are exercised against committed header files, and the current test suite calls them without build env injected. As written,
getSupabaseOrigin()throws beforeassertAuthCspDirectives()can validate anything, which is exactly why the CSP tests are red in CI. Please make the expected Supabase origin injectable from the caller/test setup, or otherwise avoid hard-failing this validator when the env is absent outside the build path.🤖 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 `@scripts/cspHeaders.js` around lines 85 - 113, The auth CSP validator currently hard-depends on VITE_SUPABASE_URL via getSupabaseOrigin(), which makes assertAuthCspDirectives() fail before it can validate committed headers in tests. Update the helper flow so the expected Supabase origin is injectable from the caller/test setup (or otherwise only required in the build path), and adjust assertAuthCspDirectives() to use that injected value instead of unconditionally reading process.env. Keep the existing validation logic and error messages, but ensure getSupabaseOrigin() is no longer a mandatory runtime dependency for CI/header-file checks.Sources: Linters/SAST tools, Pipeline failures
🤖 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/components/ui/Tooltip.test.jsx`:
- Around line 46-58: The “hides tooltip on mouse leave” test is using
focus/blur, so it duplicates the keyboard-dismiss path and never covers the
mouse-leave behavior. Update the Tooltip test to use the same rendered setup but
trigger hover/mouse enter to show the tooltip and then fire mouse leave on the
button (using the Tooltip component and its button child) so the test verifies
the actual onMouseLeave path instead of blur.
In `@src/components/UserMenu.test.jsx`:
- Around line 110-131: The sign-in error test in UserMenu.test.jsx only verifies
that signInWithGoogle is called, so it does not catch an incorrect redirect
after a rejected login. Update the handleSignIn error-path coverage by asserting
that no navigation to /app occurs when signInWithGoogle rejects, using the
existing render(<UserMenu variant="landing" />) setup and the signInWithGoogle
mock. Keep the current rejection case, but add an expectation on the
router/navigation mock so this test fails if handleSignIn still redirects on
error.
In `@src/lib/supabaseClient.test.js`:
- Around line 13-52: The supabaseClient tests are importing the module without
stubbing the required Supabase env vars, so isSupabaseConfigured() stays false
and getSupabaseClient() never creates the client. Update the supabaseClient test
setup to stub VITE_SUPABASE_URL and VITE_SUPABASE_ANON_KEY before importing
supabaseClient.js, and isolate each test with vi.resetModules() plus
vi.unstubAllEnvs() so the cached singleton doesn’t leak between cases. Use the
existing getSupabaseClient and isSupabaseConfigured test blocks as the place to
apply this setup.
---
Outside diff comments:
In `@scripts/cspHeaders.js`:
- Around line 85-113: The auth CSP validator currently hard-depends on
VITE_SUPABASE_URL via getSupabaseOrigin(), which makes assertAuthCspDirectives()
fail before it can validate committed headers in tests. Update the helper flow
so the expected Supabase origin is injectable from the caller/test setup (or
otherwise only required in the build path), and adjust assertAuthCspDirectives()
to use that injected value instead of unconditionally reading process.env. Keep
the existing validation logic and error messages, but ensure getSupabaseOrigin()
is no longer a mandatory runtime dependency for CI/header-file checks.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3edd6582-ae30-4fe3-8da3-13b708028f5f
📒 Files selected for processing (11)
scripts/cspHeaders.jssrc/components/GoogleOneTap.test.jsxsrc/components/UserMenu.test.jsxsrc/components/ui/Tooltip.test.jsxsrc/contexts/AuthProvider.test.jsxsrc/lib/supabaseClient.test.jssrc/pages/GoogleAuthCallback.test.jsxsrc/services/authService.test.jssrc/services/entitlementService.test.jssrc/utils/resolveUserAvatar.test.jsvitest.config.js
🚧 Files skipped from review as they are similar to previous changes (3)
- vitest.config.js
- src/services/authService.test.js
- src/utils/resolveUserAvatar.test.js
- getSupabaseOrigin() returns null (instead of throwing) when VITE_SUPABASE_URL is not set, allowing CI build to proceed - assertAuthCspDirectives skips the Supabase origin check when getSupabaseOrigin() returns null; other validations remain enforced - This unblocks the CI workflow's Build job which doesn't have access to Supabase secrets, while deploy/preview workflows with secrets still validate CSP fully
- Tooltip.test.jsx: replace focus/blur with mouseEnter/mouseLeave in 'hides tooltip on mouse leave' test - UserMenu.test.jsx: assert no navigation occurs on sign-in error - LandingPage.test.jsx: assert mocked UserMenu is rendered - googleTokenExchange.js: check response.ok before parsing JSON body to avoid masking HTTP errors - resolveUserAvatar.js: restrict isHttpUrl to https:// only - authService.js: decode JWT payload as UTF-8 via TextDecoder for correct handling of non-ASCII characters in Google profile names
…t files Addresses all 16 remaining CodeRabbit issues from PR #192: Auth & services: - main.jsx: import AUTH_CALLBACK_PATH constant instead of hardcoded path - authService.js: signInWithGoogle checks isAuthConfigured() (Supabase + Google) - AuthProvider.jsx: stale refreshProfile fix with request token - GoogleAuthCallback.jsx: use getSession() with error handling - googleIdentity.js: clean up sign-in host element after use in finish() - GoogleOneTap.jsx: re-check isMounted before navigating after await Components: - UserAvatar.jsx: DiceBear fallback used immediately when avatarSrc empty - UserMenu.jsx: sign-in label visible on mobile (remove hidden sm:inline) - Tooltip.jsx: useEffect cleanup clears pending showTimeoutRef on unmount Data/content: - privacy.en.js: remove hardcoded "hosted in the EU" - Migration: INSERT missing profile rows before backfill UPDATE Tests: - supabaseMock.js: mockClear -> mockReset with re-applied defaults - supabaseClient.test.js: env stubs with vi.stubEnv + resetModules - authService.test.js: add isAuthConfigured Supabase-not-configured branch - GoogleAuthCallback.test.jsx: update tests for getSession() behavior
33 tasks
35 tasks
5 tasks
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
1 participant
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.
Contribution workflow
develop: This PR targetsdevelop, notmain.Description
Adds optional user accounts for Bayan Flow v0.5.0: Google OIDC sign-in via Supabase Auth, session persistence, sign-out, and minimal profile plumbing. Unauthenticated users keep full access to all visualizations and existing panels — no feature gating in this PR.
Sign-in uses a popup OAuth flow (Google account picker in a small window,
/auth/callbackcompletes the session and closes the popup). From the landing page, successful sign-in redirects to/app. On/app, the control is a compact G icon to the right of the theme toggle, with a styled tooltip on hover.Type of Change
Related Issues
Fixes #
Changes Made
Auth stack
@supabase/supabase-js,@dicebear/core,@dicebear/styles(package.json)src/lib/supabaseClient.js— singleton client +isSupabaseConfigured()src/services/authService.js— popup OAuth (skipBrowserRedirect), session helpers, sign-outsrc/services/profileService.js— read ownprofilesrow (RLS)src/services/entitlementService.js— stub for future plan gatingsrc/contexts/AuthProvider.jsx+src/hooks/useAuth.js— session hydration, profile view modelsrc/main.jsx— wrap app inAuthProvider; route/auth/callback→AuthCallback.jsx.env.example—VITE_SUPABASE_URL,VITE_SUPABASE_ANON_KEY(publishable anon key only)Profile UI
src/utils/resolveUserAvatar.js— Google metadata →profiles.avatar_url→ DiceBear generated avatar fallbacksrc/components/UserAvatar.jsx— avatar with ring + error fallbacksrc/components/UserMenu.jsx— sign-in / account dropdownvariant="landing"— full “Sign in with Google” in top-right cluster; navigates to/appafter sign-invariant="compact"— icon-only G button on/app; no scale hover; custom tooltipsrc/components/ui/Tooltip.jsx— styled, animated tooltip (replaces nativetitleon compact control)Header.jsx(compact, after theme toggle) andLandingPage.jsx(landing variant)Database (Supabase)
supabase/migrations/— mirrored SQL applied to projectbayan-flow(qketsapzqpzmccljfjcm, eu-central-1):profilestable (plandefaultfree, provider, timestamps)handle_new_usertrigger onauth.usersinsertSELECTown row only; client cannot writeplandisplay_name,avatar_urlcolumns + backfill from Google metadataCSP, CI, and hosting
public/_headers,netlify.toml:connect-src+ Supabase origin;img-src+https://lh3.googleusercontent.comscripts/cspHeaders.js—assertAuthCspDirectives(); validated at build invite.config.jsandsrc/security/cspHeaders.test.js.github/workflows/deploy-cloudflare.yml+preview-cloudflare.yml— injectVITE_SUPABASE_URL/VITE_SUPABASE_ANON_KEYfrom GitHub secretsvite.config.js—@/alias for Vite build (already in Vitest)Docs and i18n
AGENTS.md— auth architecture contractspublic/auth.md— popup flow, callback redirect URL requirementsrc/content/legal/privacy.en.js— optional Google sign-in, profile datasign_in_google,sign_out,account_menu_label,completing_sign_in, etc.Tests
authService,AuthProvider,UserMenu,UserAvatar,resolveUserAvatar,Tooltip,supabaseMockLandingPage,PrivacyPolicy,cspHeaders,framerMotionMock(useReducedMotion)Algorithm Details (if applicable)
N/A — no algorithm changes.
Testing
pnpm test:run) — 1598 testsTest Results
Manual verification
Prerequisites
.env.example→.env.localwith Supabase URL + anon key{origin}/auth/callback(e.g.http://localhost:5173/auth/callback,https://dev.bayanflow.com/auth/callback)VITE_SUPABASE_URLandVITE_SUPABASE_ANON_KEYset for Cloudflare deploy workflowsLanding (
/)/appApp (
/app)Without env vars
isSupabaseConfigured()false); app behaves as beforeRTL (
ar)endalignment; verify visually on stagingScreenshots/GIFs
/app; full button on landingAdd screenshots from local/staging QA before merge if desired.
Code Quality
pnpm lint)pnpm format)Performance Impact
Accessibility
aria-label,role="menu",role="tooltip",aria-describedby)Breaking Changes
Checklist
Additional Notes
Explicitly out of scope (future releases)
planwebhooks, signup abuse hooksDeploy / ops checklist for reviewer
VITE_SUPABASE_URL,VITE_SUPABASE_ANON_KEY/auth/callback)supabase/migrations/if deploying to a fresh Supabase projectOAuth branding note
Google consent screen shows
*.supabase.coas the redirect host — expected with Supabase as OIDC broker; improve via Google consent branding; optional Supabase custom auth domain later.Reviewer Guidelines:
Summary by CodeRabbit