Skip to content

Fix refresh token logic #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
elvisdragonmao merged 2 commits into from
Feb 24, 2026
Merged

Fix refresh token logic #23

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8dbca18
fix: implement auth token refresh logic and remove obsolete hook
Kyle9410-Chen Feb 24, 2026
7a50fd2
fix: remove obsolete refresh token logic and clean up related code
Kyle9410-Chen Feb 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/features/auth/hooks/useAuth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { authService } from "@/features/auth/services/authService";
import type { UserOnboardingRequest } from "@nycu-sdc/core-system-sdk";
import { authRefreshToken } from "@nycu-sdc/core-system-sdk";
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
import { useEffect } from "react";

const DEFAULT_AUTH_REFRESH_INTERVAL = 5 * 60 * 1000;

export const useMe = () =>
useQuery({
Expand Down Expand Up @@ -30,3 +34,15 @@
isLoading
};
};

export const useAuthRefreshInterval = () => {
useEffect(() => {
console.log("Setting up auth refresh interval");
const interval = setInterval(() => {
console.log("Refreshing auth token");
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

These console.log statements will run for every mount and every refresh tick, creating noisy logs (and potentially leaking auth-related timing info in production). Please remove them or route through the app’s logging facility behind an environment/debug flag.

Suggested change
console.log("Setting up auth refresh interval");
const interval = setInterval(() => {
console.log("Refreshing auth token");
const interval = setInterval(() => {

Copilot uses AI. Check for mistakes.
authRefreshToken("");

Check failure on line 43 in src/features/auth/hooks/useAuth.ts

View workflow job for this annotation

GitHub Actions / Build Vite production 

Type '""' has no properties in common with type 'RequestInit'.
}, DEFAULT_AUTH_REFRESH_INTERVAL);
Comment on lines +40 to +42
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

authRefreshToken("") is invoked with an empty refresh token and without the request options used elsewhere (e.g. credentials: "include"). This bypasses the existing authService.refreshAccessToken() logic (stored token lookup, rotation handling, 404 expiry handling) and will likely make refresh fail or behave incorrectly. Use authService.refreshAccessToken() here (or pass the stored refresh token + the same default request options) and handle the returned promise.

Copilot uses AI. Check for mistakes.
Comment on lines +40 to +42
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

The interval is set up unconditionally and the callback doesn’t guard against overlapping refreshes or unauthenticated sessions. The previous implementation checked getStoredRefreshToken() and de-duped in-flight refreshes; without that, this can spam refresh requests (including for logged-out users) if the request is slow or multiple layouts mount. Consider reintroducing the stored-token check and an in-flight guard (e.g., via useRef) before calling refresh.

Copilot uses AI. Check for mistakes.

return () => clearInterval(interval);
}, []);
Comment on lines +39 to +45
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

There’s no success/error handling around the refresh call to keep React Query auth state consistent. Previously, refresh failures cleared ["user","me"] and refresh success invalidated it; without similar handling, useAuth() may stay authenticated with stale cached user data even when the refresh token expires and subsequent requests start returning 401.

Suggested change
useEffect(() => {
console.log("Setting up auth refresh interval");
const interval = setInterval(() => {
console.log("Refreshing auth token");
authRefreshToken("");
}, DEFAULT_AUTH_REFRESH_INTERVAL);
return () => clearInterval(interval);
}, []);
const qc = useQueryClient();
useEffect(() => {
console.log("Setting up auth refresh interval");
const interval = setInterval(() => {
console.log("Refreshing auth token");
authRefreshToken("")
.then(() => {
// Refresh succeeded: ensure user data is up to date.
qc.invalidateQueries({ queryKey: ["user", "me"] });
})
.catch(() => {
// Refresh failed (e.g., expired/invalid token): clear cached user.
qc.removeQueries({ queryKey: ["user", "me"] });
});
}, DEFAULT_AUTH_REFRESH_INTERVAL);
return () => clearInterval(interval);
}, [qc]);

Copilot uses AI. Check for mistakes.
};
5 changes: 3 additions & 2 deletions src/layouts/AdminLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import { useAuthRefreshInterval } from "@/features/auth/hooks/useAuth";
import { Footer } from "@/shared/components/Footer/Footer";
import type { ReactNode } from "react";
import { useState } from "react";
import styles from "./AdminLayout.module.css";
import { AdminNav } from "./AdminNav";
import { useAuthRefreshInterval } from "./useAuthRefreshInterval";
interface AdminLayoutProps {
children: ReactNode;
}

export const AdminLayout = ({ children }: AdminLayoutProps) => {
useAuthRefreshInterval();
const [isNavOpen, setIsNavOpen] = useState(false);

useAuthRefreshInterval();

return (
<div className={styles.container}>
<AdminNav isOpen={isNavOpen} setIsOpen={setIsNavOpen} />
Expand Down
2 changes: 1 addition & 1 deletion src/layouts/UserLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useAuthRefreshInterval } from "@/features/auth/hooks/useAuth";
import { Footer } from "@/shared/components/Footer/Footer";
import type { ReactNode } from "react";
import styles from "./UserLayout.module.css";
import { useAuthRefreshInterval } from "./useAuthRefreshInterval";

interface UserLayoutProps {
children: ReactNode;
Expand Down
46 changes: 0 additions & 46 deletions src/layouts/useAuthRefreshInterval.ts
View file
Open in desktop

This file was deleted.

Loading