Skip to content

feat(frontend): add reusable useUser hook and header auth UX#943

Open
skypank-coder wants to merge 2 commits into
OWASP:mainfrom
skypank-coder:feat/frontend-useuser-header-auth
Open

feat(frontend): add reusable useUser hook and header auth UX#943
skypank-coder wants to merge 2 commits into
OWASP:mainfrom
skypank-coder:feat/frontend-useuser-header-auth

Conversation

@skypank-coder

Copy link
Copy Markdown
Contributor

Scoped to non-MyOpenCRE auth UX per @Pa04rth 's steer (b): a reusable useUser hook + header login/user/logout state, following the existing chatbot auth pattern. Part of the Login MVP (@northdpole 's priority #1).

useUser probes /rest/v1/user and treats 401 as anonymous — no forced redirect, so public pages stay open to anonymous users (consistent with the public-reads-stay-open / v2 direction @ outlined).
Header shows the logged-in user + Logout, or a Login button when anonymous.
Frontend-only. No backend changes. No myopencre/capabilities coupling (left to Prateek).

Verified manually in dev: NO_LOGIN=1 shows logged-in state; unauthenticated shows Login with no redirect on load; login/logout flows work.
Closes #942 .

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: d1dcfe1f-4699-433e-bb5c-48849c9d63ad

📥 Commits

Reviewing files that changed from the base of the PR and between 4c0d5bc and d9c3f63.

📒 Files selected for processing (1)
  • application/frontend/src/hooks/useUser.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • application/frontend/src/hooks/useUser.ts

Summary by CodeRabbit

  • New Features
    • Added a user authentication hook that fetches the current user status, exposes isLoggedIn, and provides login/logout actions.
    • Updated the header to use the new authentication state to show the logged-in user with a Logout button, or Login when not authenticated, across both desktop and mobile views.
    • Mobile authentication actions now close the mobile menu before navigating to login/logout.

Walkthrough

A new useUser React hook is introduced that fetches the current user from ${apiUrl}/user, exposes { user, isLoggedIn, loading, login, logout }, and is re-exported from the hooks index. The Header component is updated to consume useUser and render conditional login/logout controls for both desktop and mobile layouts.

Changes

User Auth State and Header Integration

Layer / File(s) Summary
useUser hook and public export
application/frontend/src/hooks/useUser.ts, application/frontend/src/hooks/index.ts
Defines the UserState exported type ({ user: string | null, isLoggedIn: boolean, loading: boolean }). The useUser hook fetches ${apiUrl}/user on mount: HTTP 200 sets a trimmed username, 401 sets user to null (anonymous), and any other status or network error logs and degrades to user: null. Uses an active flag to prevent state updates after unmount and sets loading to false in a finally block. Derives isLoggedIn from user !== null and provides login/logout as window-location redirect functions to /login and /logout. Re-exported from the hooks barrel in index.ts.
Header desktop and mobile auth controls
application/frontend/src/scaffolding/Header/Header.tsx
Expands the lucide-react import to include LogOut and User icons. Adds the useUser import and calls it to destructure { user, isLoggedIn, loading: userLoading, login, logout }. Replaces previously commented-out placeholder blocks in both desktop and mobile layouts with conditional rendering gated on !userLoading: logged-in state displays user info and a Logout button; anonymous state displays a Login button. Mobile Logout and Login buttons also call closeMobileMenu() before invoking the auth actions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding a reusable useUser hook and integrating header authentication UX.
Description check ✅ Passed The description is directly related to the changeset, explaining the useUser hook implementation, header auth UX, and the rationale behind the design choices.
Linked Issues check ✅ Passed The PR fully addresses issue #942 requirements: implements a reusable useUser hook probing /rest/v1/user, treats 401 as anonymous without redirects, and integrates auth state into header UI.
Out of Scope Changes check ✅ Passed All changes are directly within scope: useUser hook creation, header auth UI integration, and hook export. No backend changes, MyOpenCRE coupling, or chatbot refactoring included.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

application/frontend/src/hooks/useUser.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
application/frontend/src/hooks/useUser.ts (1)

16-39: Use async/await instead of Promise chaining in this new TS hook.

This new frontend TypeScript code uses raw .then/.catch/.finally; please rewrite to async/await for consistency with project standards.
As per coding guidelines, "Prefer async/await over raw Promise chains or callbacks in new TypeScript frontend code."

Suggested refactor
-  useEffect(() => {
-    let active = true;
-    fetch(`${apiUrl}/user`, { method: 'GET' })
-      .then((res) => {
-        if (res.status === 200) {
-          return res.text();
-        }
-        return null; // 401 or anything else => treated as not logged in
-      })
-      .then((value) => {
-        if (active) {
-          setUser(value && value.trim() !== '' ? value : null);
-        }
-      })
-      .catch(() => {
-        if (active) {
-          setUser(null); // network error => treat as anonymous, do NOT redirect
-        }
-      })
-      .finally(() => {
-        if (active) {
-          setLoading(false);
-        }
-      });
-    return () => {
-      active = false;
-    };
-  }, [apiUrl]);
+  useEffect(() => {
+    let active = true;
+    const loadUser = async () => {
+      try {
+        const res = await fetch(`${apiUrl}/user`, { method: 'GET' });
+        if (res.status === 200) {
+          const value = await res.text();
+          if (active) setUser(value.trim() !== '' ? value : null);
+          return;
+        }
+        if (active) setUser(null);
+      } catch {
+        if (active) setUser(null);
+      } finally {
+        if (active) setLoading(false);
+      }
+    };
+    void loadUser();
+    return () => {
+      active = false;
+    };
+  }, [apiUrl]);
🤖 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 `@application/frontend/src/hooks/useUser.ts` around lines 16 - 39, The
useEffect hook in the useUser function uses Promise chaining with
.then/.catch/.finally methods, which should be converted to async/await syntax
for consistency with project standards. Create an async function inside the
useEffect that performs the fetch call to the user endpoint, use try/catch
blocks instead of .catch(), and replace the .then() chaining with await
statements. Ensure the active flag check is maintained at each operation (after
fetch, after setting user state, and in the cleanup) to prevent state updates on
unmounted components, and keep the setLoading(false) call in a finally block or
after the try/catch completes.

Source: Coding guidelines

🤖 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 `@application/frontend/src/hooks/useUser.ts`:
- Around line 20-24: In the useUser hook, the current logic treats all non-200
HTTP responses as anonymous by returning null. Instead of collapsing all non-200
responses together, you should differentiate between them: return res.text() for
status 200, return null specifically for status 401 (unauthorized/anonymous),
and for all other status codes (like 5xx errors), throw an error or handle them
as actual failures rather than masking them as normal logged-out state. Apply
this same correction to both occurrences mentioned in the diff (lines 20-24 and
lines 30-33).

---

Nitpick comments:
In `@application/frontend/src/hooks/useUser.ts`:
- Around line 16-39: The useEffect hook in the useUser function uses Promise
chaining with .then/.catch/.finally methods, which should be converted to
async/await syntax for consistency with project standards. Create an async
function inside the useEffect that performs the fetch call to the user endpoint,
use try/catch blocks instead of .catch(), and replace the .then() chaining with
await statements. Ensure the active flag check is maintained at each operation
(after fetch, after setting user state, and in the cleanup) to prevent state
updates on unmounted components, and keep the setLoading(false) call in a
finally block or after the try/catch completes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 8e57915e-65e6-459b-95f0-ef4cd0130eb7

📥 Commits

Reviewing files that changed from the base of the PR and between 13d2f04 and 4c0d5bc.

📒 Files selected for processing (3)
  • application/frontend/src/hooks/index.ts
  • application/frontend/src/hooks/useUser.ts
  • application/frontend/src/scaffolding/Header/Header.tsx

Comment thread application/frontend/src/hooks/useUser.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Login MVP: add reusable user/auth state + header login/logout UX

1 participant