Replace popup auth flow with redirect flow (#1473)

* refactor: simplify login store using only authDetails variable * feat(frontend): replace popup auth with redirect auth * build: remove local compose config var VITE_FMTM_DOMAIN * refactor(frontend): add route login protection for additional pages
hotosm · Apr 24, 2024 · a9fa16a · a9fa16a
1 parent 785cb80
commit a9fa16a
Show file tree

Hide file tree

Showing 20 changed files with 145 additions and 196 deletions.
diff --git a/docker-compose.yml b/docker-compose.yml
@@ -122,7 +122,6 @@ services:
       - /app/node_modules/
     environment:
       - VITE_API_URL=http://api.${FMTM_DOMAIN}:${FMTM_DEV_PORT:-7050}
-      - VITE_FMTM_DOMAIN=${FMTM_DOMAIN}
     ports:
       - "7051:7051"
     networks:

diff --git a/src/frontend/src/App.tsx b/src/frontend/src/App.tsx
@@ -60,23 +60,23 @@ axios.interceptors.request.use(
 
 const GlobalInit = () => {
   const dispatch = useDispatch();
-  const storeUser = CoreModules.useAppSelector((state) => state.login);
+  const authDetails = CoreModules.useAppSelector((state) => state.login.authDetails);
 
-  const checkIfUserLoggedIn = () => {
+  const checkIfUserLoginValid = () => {
     fetch(`${import.meta.env.VITE_API_URL}/auth/introspect`, { credentials: 'include' })
       .then((resp) => {
         if (resp.status !== 200) {
-          dispatch(LoginActions.signOut(null));
+          dispatch(LoginActions.signOut());
           return;
         }
         return resp.json();
       })
       .then((apiUser) => {
         if (!apiUser) return;
 
-        if (apiUser.username !== storeUser?.loginToken?.username) {
+        if (apiUser.username !== authDetails?.username) {
           // Mismatch between store user and logged in user via api
-          dispatch(LoginActions.signOut(null));
+          dispatch(LoginActions.signOut());
         }
       })
       .catch((error) => {
@@ -103,7 +103,9 @@ const GlobalInit = () => {
 
     // Check current login state (omit callback url)
     if (!window.location.pathname.includes('osmauth')) {
-      checkIfUserLoggedIn();
+      // No need for introspect check if user details are not set
+      if (!authDetails) return;
+      checkIfUserLoginValid();
     }
 
     // Do things when component is unmounted

diff --git a/src/frontend/src/api/Login.ts b/src/frontend/src/api/Login.ts
@@ -1,15 +1,16 @@
 import axios from 'axios';
 import { getUserDetailsFromApi } from '@/utilfunctions/login';
 import { CommonActions } from '@/store/slices/CommonSlice';
+import { LoginActions } from '@/store/slices/LoginSlice';
 
 export const TemporaryLoginService: Function = (url: string) => {
   return async (dispatch) => {
     const getTemporaryLogin = async (url) => {
       // Sets a cookie in the browser that is used for auth
       await axios.get(url);
 
-      const loginSuccess = await getUserDetailsFromApi();
-      if (!loginSuccess) {
+      const apiUser = await getUserDetailsFromApi();
+      if (!apiUser) {
         dispatch(
           CommonActions.SetSnackBar({
             open: true,
@@ -18,7 +19,10 @@ export const TemporaryLoginService: Function = (url: string) => {
             duration: 2000,
           }),
         );
+        return;
       }
+
+      dispatch(LoginActions.setAuthDetails(apiUser));
     };
 
     await getTemporaryLogin(url);

diff --git a/src/frontend/src/api/OrganisationService.ts b/src/frontend/src/api/OrganisationService.ts
@@ -4,7 +4,7 @@ import { GetOrganisationDataModel, OrganisationModal } from '@/models/organisati
 import { CommonActions } from '@/store/slices/CommonSlice';
 import { OrganisationAction } from '@/store/slices/organisationSlice';
 import { API } from '.';
-import { createLoginWindow } from '@/utilfunctions/login';
+import { LoginActions } from '@/store/slices/LoginSlice';
 
 function appendObjectToFormData(formData, object) {
   for (const [key, value] of Object.entries(object)) {
@@ -52,7 +52,7 @@ export const OrganisationDataService: Function = (url: string) => {
       } catch (error) {
         dispatch(OrganisationAction.GetOrganisationDataLoading(false));
         if (error.response.status === 401) {
-          createLoginWindow('/');
+          dispatch(LoginActions.setLoginModalOpen(true));
         }
       }
     };

diff --git a/src/frontend/src/components/DialogTaskActions.jsx b/src/frontend/src/components/DialogTaskActions.jsx
@@ -13,7 +13,7 @@ export default function Dialog({ taskId, feature, map, view }) {
   const navigate = useNavigate();
   const projectInfo = CoreModules.useAppSelector((state) => state.project.projectInfo);
   const taskBoundaryData = CoreModules.useAppSelector((state) => state.project.projectTaskBoundries);
-  const token = CoreModules.useAppSelector((state) => state.login.loginToken);
+  const authDetails = CoreModules.useAppSelector((state) => state.login.authDetails);
   const loading = CoreModules.useAppSelector((state) => state.common.loading);
   const [list_of_task_status, set_list_of_task_status] = useState([]);
   const [task_status, set_task_status] = useState('READY');
@@ -54,10 +54,10 @@ export default function Dialog({ taskId, feature, map, view }) {
 
   const handleOnClick = (event) => {
     const status = task_priority_str[event.currentTarget.dataset.btnid];
-    const body = token != null ? { ...token } : {};
+    const authDetailsCopy = authDetails != null ? { ...authDetails } : {};
     const geoStyle = geojsonStyles[event.currentTarget.dataset.btnid];
     if (event.currentTarget.dataset.btnid != undefined) {
-      if (body.hasOwnProperty('id')) {
+      if (authDetailsCopy.hasOwnProperty('id')) {
         dispatch(
           ProjectTaskStatus(
             `${import.meta.env.VITE_API_URL}/tasks/${taskId}/new-status/${status}`,
@@ -68,7 +68,7 @@ export default function Dialog({ taskId, feature, map, view }) {
             map,
             view,
             taskId,
-            body,
+            authDetailsCopy,
             { project_id: currentProjectId },
           ),
         );
@@ -94,7 +94,7 @@ export default function Dialog({ taskId, feature, map, view }) {
     }
   };
   const checkIfTaskAssignedOrNot =
-    currentStatus?.locked_by_username === token?.username || currentStatus?.locked_by_username === null;
+    currentStatus?.locked_by_username === authDetails?.username || currentStatus?.locked_by_username === null;
 
   return (
     <div className="fmtm-flex fmtm-flex-col">

diff --git a/src/frontend/src/components/LoginPopup.tsx b/src/frontend/src/components/LoginPopup.tsx
@@ -2,8 +2,9 @@ import React from 'react';
 import CoreModules from '@/shared/CoreModules';
 import { Modal } from '@/components/common/Modal';
 import { useDispatch } from 'react-redux';
+import { useNavigate } from 'react-router-dom';
 import { LoginActions } from '@/store/slices/LoginSlice';
-import { createLoginWindow } from '@/utilfunctions/login';
+import { osmLoginRedirect } from '@/utilfunctions/login';
 import { TemporaryLoginService } from '@/api/Login';
 import AssetModules from '@/shared/AssetModules';
 import OSMImg from '@/assets/images/osm-logo.png';
@@ -33,14 +34,17 @@ const loginOptions: loginOptionsType[] = [
 
 const LoginPopup = () => {
   const dispatch = useDispatch();
+  const navigate = useNavigate();
 
   const loginModalOpen = CoreModules.useAppSelector((state) => state.login.loginModalOpen);
 
   const handleSignIn = (selectedOption: string) => {
     if (selectedOption === 'osm_account') {
-      createLoginWindow('/');
+      osmLoginRedirect();
     } else {
       dispatch(TemporaryLoginService(`${import.meta.env.VITE_API_URL}/auth/temp-login`));
+      navigate('/');
+      dispatch(LoginActions.setLoginModalOpen(false));
     }
   };
 

diff --git a/src/frontend/src/components/ProjectDetailsV2/TaskSelectionPopup.tsx b/src/frontend/src/components/ProjectDetailsV2/TaskSelectionPopup.tsx
@@ -26,16 +26,16 @@ const TaskSelectionPopup = ({ taskId, body, feature }: TaskSelectionPopupPropTyp
   //qrcodecomponent
   const projectName = CoreModules.useAppSelector((state) => state.project.projectInfo.title);
   const odkToken = CoreModules.useAppSelector((state) => state.project.projectInfo.odk_token);
-  const loginToken = CoreModules.useAppSelector((state) => state.login.loginToken);
+  const authDetails = CoreModules.useAppSelector((state) => state.login.authDetails);
   const selectedTask = {
     ...projectData?.[projectIndex]?.taskBoundries?.filter((indTask, i) => {
       return indTask.id == taskId;
     })?.[0],
   };
   const checkIfTaskAssignedOrNot =
-    selectedTask?.locked_by_username === loginToken?.username || selectedTask?.locked_by_username === null;
+    selectedTask?.locked_by_username === authDetails?.username || selectedTask?.locked_by_username === null;
 
-  const { qrcode } = GetProjectQrCode(odkToken, projectName, loginToken?.username);
+  const { qrcode } = GetProjectQrCode(odkToken, projectName, authDetails?.username);
   useEffect(() => {
     if (projectIndex != -1) {
       const currentStatus = {

diff --git a/src/frontend/src/components/organisation/OrganisationGridCard.tsx b/src/frontend/src/components/organisation/OrganisationGridCard.tsx
@@ -6,7 +6,7 @@ import { user_roles } from '@/types/enums';
 
 const OrganisationGridCard = ({ filteredData, allDataLength }) => {
   const navigate = useNavigate();
-  const token = CoreModules.useAppSelector((state) => state.login.loginToken);
+  const authDetails = CoreModules.useAppSelector((state) => state.login.authDetails);
   const cardStyle = {
     padding: '20px',
     display: 'flex',
@@ -27,7 +27,7 @@ const OrganisationGridCard = ({ filteredData, allDataLength }) => {
             key={index}
             sx={cardStyle}
             onClick={() => {
-              if (!data?.approved && token && token?.['role'] === user_roles.ADMIN) {
+              if (!data?.approved && authDetails && authDetails?.['role'] === user_roles.ADMIN) {
                 navigate(`/approve-organization/${data?.id}`);
               }
             }}
@@ -60,7 +60,7 @@ const OrganisationGridCard = ({ filteredData, allDataLength }) => {
                   {data.description}
                 </p>
               </div>
-              {token && token['role'] === user_roles.ADMIN && (
+              {authDetails && authDetails['role'] === user_roles.ADMIN && (
                 <div className="fmtm-w-full fmtm-flex fmtm-justify-end">
                   <div
                     className={`fmtm-bottom-5 fmtm-right-5 fmtm-px-2 fmtm-py-1 fmtm-rounded fmtm-w-fit ${

diff --git a/src/frontend/src/routes.jsx b/src/frontend/src/routes.jsx
@@ -8,7 +8,7 @@ import NotFoundPage from '@/views/NotFound404';
 import Organisation from '@/views/Organisation';
 import CreateEditOrganization from '@/views/CreateEditOrganization';
 import ApproveOrganization from '@/views/ApproveOrganization';
-import Authorized from '@/views/Authorized';
+import OsmAuth from '@/views/OsmAuth';
 import SubmissionDetails from '@/views/SubmissionDetails';
 import CreateNewProject from '@/views/CreateNewProject';
 import UnderConstruction from '@/views/UnderConstruction';
@@ -51,24 +51,22 @@ const routes = createBrowserRouter([
       {
         path: '/edit-organization/:id',
         element: (
-          <ErrorBoundary>
-            <CreateEditOrganization />
-          </ErrorBoundary>
+          <ProtectedRoute>
+            <ErrorBoundary>
+              <CreateEditOrganization />
+            </ErrorBoundary>
+          </ProtectedRoute>
         ),
       },
       {
         path: '/approve-organization/:id',
         element: (
-          <ErrorBoundary>
-            <ApproveOrganization />
-          </ErrorBoundary>
+          <ProtectedRoute>
+            <ErrorBoundary>
+              <ApproveOrganization />
+            </ErrorBoundary>
+          </ProtectedRoute>
         ),
-      },
-      // {
-      //     path: '/explore',
-      //     element: <Navigate to="/" />,
-      // },
-      {
         path: '/tabbed',
         element: (
           <ErrorBoundary>
@@ -101,11 +99,6 @@ const routes = createBrowserRouter([
           </ProtectedRoute>
         ),
       },
-      // {
-      //     path: "/recoveraccess",
-      //     element: <Forgot />,
-      // },
-
       {
         path: '/project/:projectId/tasks/:taskId/submission/:instanceId',
         element: (
@@ -133,11 +126,13 @@ const routes = createBrowserRouter([
       {
         path: '/project/:id',
         element: (
-          <Suspense fallback={<div>Loading...</div>}>
-            <ErrorBoundary>
-              <ProjectDetailsV2 />
-            </ErrorBoundary>
-          </Suspense>
+          <ProtectedRoute>
+            <Suspense fallback={<div>Loading...</div>}>
+              <ErrorBoundary>
+                <ProjectDetailsV2 />
+              </ErrorBoundary>
+            </Suspense>
+          </ProtectedRoute>
         ),
       },
       {
@@ -217,7 +212,7 @@ const routes = createBrowserRouter([
         element: (
           <Suspense fallback={<div>Loading...</div>}>
             <ErrorBoundary>
-              <Authorized />
+              <OsmAuth />
             </ErrorBoundary>
           </Suspense>
         ),

diff --git a/src/frontend/src/store/Store.ts b/src/frontend/src/store/Store.ts
@@ -25,7 +25,7 @@ export default function persist(key, whitelist, reducer) {
 
 const rootReducer = combineReducers({
   project: ProjectSlice.reducer,
-  login: persist('login', ['loginToken', 'authDetails'], LoginSlice.reducer),
+  login: persist('login', ['authDetails'], LoginSlice.reducer),
   //you can persist your auth reducer here similar to project reducer
   home: HomeSlice.reducer,
   theme: ThemeSlice.reducer,

diff --git a/src/frontend/src/store/slices/LoginSlice.ts b/src/frontend/src/store/slices/LoginSlice.ts
@@ -3,25 +3,21 @@ import storage from 'redux-persist/lib/storage';
 import { LoginStateTypes } from '@/store/types/ILogin';
 
 const initialState: LoginStateTypes = {
-  loginToken: {},
-  authDetails: {},
+  authDetails: null,
   loginModalOpen: false,
 };
 
 const LoginSlice = CoreModules.createSlice({
   name: 'login',
   initialState: initialState,
   reducers: {
-    SetLoginToken(state, action) {
-      state.loginToken = action.payload;
-    },
-    signOut(state, action) {
-      storage.removeItem('persist:login');
-      state.loginToken = action.payload;
-    },
     setAuthDetails(state, action) {
       state.authDetails = action.payload;
     },
+    signOut(state) {
+      storage.removeItem('persist:login');
+      state.authDetails = null;
+    },
     setLoginModalOpen(state, action) {
       state.loginModalOpen = action.payload;
     },

diff --git a/src/frontend/src/store/types/ILogin.ts b/src/frontend/src/store/types/ILogin.ts
@@ -1,14 +1,12 @@
 export type LoginStateTypes = {
-  loginToken: logintTokenType | {};
-  authDetails: {} | string;
+  authDetails: authDetailsType | null;
   loginModalOpen: false;
 };
 
-type logintTokenType = {
+type authDetailsType = {
   id: string;
-  osm_oauth_token: string;
-  picture: string;
+  img_url: string;
   role: string;
-  sessionToken: string | null;
   username: string;
+  // sessionToken: string | null;
 };