Invert logic of isAuthPreExpiry => isAuthExpired

mrc-ide · Nov 25, 2024 · 01cf3ac · 01cf3ac
1 parent 335837a
commit 01cf3ac
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 18 deletions.
diff --git a/app/src/app/components/routes/ProtectedRoute.tsx b/app/src/app/components/routes/ProtectedRoute.tsx
@@ -1,6 +1,6 @@
 import { useEffect } from "react";
 import { Outlet, useNavigate, useLocation } from "react-router-dom";
-import { authIsPreExpiry, isAuthenticated } from "../../../lib/isAuthenticated";
+import {authIsExpired, isAuthenticated} from "../../../lib/isAuthenticated";
 import { useAuthConfig } from "../providers/AuthConfigProvider";
 import { useUser } from "../providers/UserProvider";
 import { useRedirectOnLogin } from "../providers/RedirectOnLoginProvider";
@@ -19,7 +19,7 @@ export default function ProtectedRoute() {
       if (!loggingOut) {
         setRequestedUrl(pathname);
       }
-      if (user && !authIsPreExpiry(user)) {
+      if (user && authIsExpired(user)) {
         removeUser()
         navigate(`/login?info=${expiryMessage}`);
       } else {

diff --git a/app/src/lib/isAuthenticated.ts b/app/src/lib/isAuthenticated.ts
@@ -1,7 +1,8 @@
 import { AuthConfig } from "../app/components/providers/types/AuthConfigTypes";
 import { UserState } from "../app/components/providers/types/UserTypes";
 
-export const authIsPreExpiry = (user: UserState | null): boolean => !!user?.exp && user?.exp * 1000 >= Date.now();
+export const authIsExpired = (user: UserState | null): boolean =>
+  !user?.exp || user?.exp * 1000 < Date.now()
 
 export const isAuthenticated = (authConfig: AuthConfig | null, user: UserState | null): boolean =>
-  authConfig?.enableAuth === false || !!(authConfig?.enableAuth && user?.token && authIsPreExpiry(user));
+  authConfig?.enableAuth === false || !!(authConfig?.enableAuth && user?.token && !authIsExpired(user));
diff --git a/app/src/tests/components/routes/ProtectedRoute.test.tsx b/app/src/tests/components/routes/ProtectedRoute.test.tsx
@@ -12,10 +12,10 @@ jest.mock("react-router-dom", () => ({
   useNavigate: () => mockedUsedNavigate
 }));
 const mockIsAuthenticated = jest.fn();
-const mockAuthIsPreExpiry = jest.fn();
+const mockAuthIsExpired = jest.fn();
 jest.mock("../../../lib/isAuthenticated", () => ({
   isAuthenticated: () => mockIsAuthenticated(),
-  authIsPreExpiry: () => mockAuthIsPreExpiry()
+  authIsExpired: () => mockAuthIsExpired()
 }));
 
 const mockSetRequestedUrl = jest.fn();
@@ -53,7 +53,7 @@ describe("protected routes", () => {
 
   it("renders protected content when authenticated", async () => {
     mockIsAuthenticated.mockReturnValue(true);
-    mockAuthIsPreExpiry.mockReturnValue(true);
+    mockAuthIsExpired.mockReturnValue(false);
     renderElement();
 
     await waitFor(() => {
@@ -64,7 +64,7 @@ describe("protected routes", () => {
   it("should navigate to login and set requested url when unauthenticated", async () => {
     mockLoggingOut = false;
     mockIsAuthenticated.mockReturnValue(false);
-    mockAuthIsPreExpiry.mockReturnValue(true);
+    mockAuthIsExpired.mockReturnValue(false);
     renderElement();
 
     await waitFor(() => {
@@ -76,7 +76,7 @@ describe("protected routes", () => {
   it("navigates to login, sets requested url, & logs the user out when auth expiry time is in the past", async () => {
     mockLoggingOut = false;
     mockIsAuthenticated.mockReturnValue(false);
-    mockAuthIsPreExpiry.mockReturnValue(false);
+    mockAuthIsExpired.mockReturnValue(true);
     mockGetUserFromLocalStorage.mockReturnValueOnce(mockExpiredUserState());
     localStorage.setItem(LocalStorageKeys.USER, "mockUser");
     renderElement();
@@ -92,7 +92,7 @@ describe("protected routes", () => {
   it("should not set requested url when logging out", async () => {
     mockLoggingOut = true;
     mockIsAuthenticated.mockReturnValue(false);
-    mockAuthIsPreExpiry.mockReturnValue(true);
+    mockAuthIsExpired.mockReturnValue(false);
     renderElement();
 
     await waitFor(() => {

diff --git a/app/src/tests/lib/isAuthenticated.test.ts b/app/src/tests/lib/isAuthenticated.test.ts
@@ -1,5 +1,5 @@
 import { AuthConfig } from "../../app/components/providers/types/AuthConfigTypes";
-import { authIsPreExpiry, isAuthenticated } from "../../lib/isAuthenticated";
+import { authIsExpired, isAuthenticated } from "../../lib/isAuthenticated";
 import { mockUserState, mockExpiredUserState } from "../mocks";
 
 describe("isAuthenticated", () => {
@@ -28,18 +28,24 @@ describe("isAuthenticated", () => {
     const user = null;
     expect(isAuthenticated(authConfig, user)).toBe(false);
   });
+
+  it("returns false if authConfig is present and user is null", () => {
+    const authConfig = { enableAuth: true } as AuthConfig;
+    const user = null;
+    expect(isAuthenticated(authConfig, user)).toBe(false);
+  })
 });
 
-describe("authIsPreExpiry", () => {
-  it("returns false if user is null", () => {
-    expect(authIsPreExpiry(null)).toBe(false);
+describe("authIsExpired", () => {
+  it("returns true if user is null", () => {
+    expect(authIsExpired(null)).toBe(true);
   });
 
-  it("returns false if user.exp is in the past", () => {
-    expect(authIsPreExpiry(mockExpiredUserState())).toBe(false);
+  it("returns true if user.exp is in the past", () => {
+    expect(authIsExpired(mockExpiredUserState())).toBe(true);
   });
 
-  it("returns true if user.exp is in the future", () => {
-    expect(authIsPreExpiry(mockUserState())).toBe(true);
+  it("returns false if user.exp is in the future", () => {
+    expect(authIsExpired(mockUserState())).toBe(false);
   });
 });