Skip to content
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.

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

front: Initialize authorization #7870

Merged
merged 1 commit into from
Jul 19, 2024
Merged

Conversation

kmer2016
Copy link
Contributor

@kmer2016 kmer2016 commented Jun 27, 2024

Close #7779

This PR Adds:

1. ProtectedRoute Components:

  • This component act as route wrappers that verify if the user has the required role to access a specific route.
  • If the user does not meet the necessary role, they are redirected to the 403 error page.

2. RoleBasedContent and PermissionBasedContent Components:

  • These components act as component wrappers that verify if the user has the required role or permission to display/hide a wrapped component.

3. Custom Hooks: useUserRoleCheck and useUserPermissionsCheck:

  • Encapsulates the logic for checking if the user has the required roles or permissions.
  • Simplifies role and permission checks across the application.

4. Error 403 listener:

  • Intercept http response with 403 status and and redirect to 403 url.

Future Integration:

  • Currently, the components that depend on roles or permissions are not used and are incomplete. We have added comments marked with TODO AUTH to indicate where we need to complete them later.
  • Once the backend implementation is ready, the checks will be replaced with actual API calls to verify user roles and permissions.
  • As I haven't received a mockup, I propose a simple first implementation of the 403 page while we wait for a mockup to be provided.

@kmer2016 kmer2016 requested a review from a team as a code owner June 27, 2024 13:59
@kmer2016 kmer2016 force-pushed the cnh/initialize_authorization_front branch from d0659bf to 32321a0 Compare June 27, 2024 14:03
@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 12.25806% with 136 lines in your changes missing coverage. Please review.

Project coverage is 18.35%. Comparing base (987512c) to head (db5a9e7).
Report is 1 commits behind head on dev.

Files Patch % Lines
...t/src/common/authorization/components/Error403.tsx 0.00% 29 Missing and 1 partial ⚠️
...uthorization/components/PermissionBasedContent.tsx 0.00% 27 Missing and 1 partial ⚠️
...common/authorization/components/ProtectedRoute.tsx 0.00% 21 Missing and 1 partial ⚠️
...mmon/authorization/components/RoleBasedContent.tsx 0.00% 17 Missing and 1 partial ⚠️
...mon/authorization/hooks/useUserPermissionsCheck.ts 0.00% 14 Missing and 1 partial ⚠️
...src/common/authorization/hooks/useUserRoleCheck.ts 0.00% 11 Missing and 1 partial ⚠️
front/src/store/listeners/authorization.ts 62.50% 5 Missing and 1 partial ⚠️
front/src/main/app.jsx 0.00% 5 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (987512c) and HEAD (db5a9e7). Click for more details.

HEAD has 7 uploads less than BASE
Flag BASE (987512c) HEAD (db5a9e7)
railjson_generator 2 1
gateway 2 1
tests 2 1
front 2 1
core 2 1
editoast 2 0
Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #7870      +/-   ##
============================================
- Coverage     28.12%   18.35%   -9.77%     
  Complexity     2081     2081              
============================================
  Files          1289     1020     -269     
  Lines        157856   128591   -29265     
  Branches       3131     3138       +7     
============================================
- Hits          44390    23606   -20784     
+ Misses       111576   103088    -8488     
- Partials       1890     1897       +7     
Flag Coverage Δ
core 75.11% <ø> (ø)
editoast ?
front 9.98% <12.25%> (-0.01%) ⬇️
gateway 2.34% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 73.18% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kmer2016 kmer2016 force-pushed the cnh/initialize_authorization_front branch from 673cd5d to aa62516 Compare July 1, 2024 07:55
@kmer2016 kmer2016 marked this pull request as draft July 1, 2024 10:03
@kmer2016 kmer2016 force-pushed the cnh/initialize_authorization_front branch 5 times, most recently from 8ae5f30 to 9762b8b Compare July 2, 2024 13:09
@kmer2016 kmer2016 marked this pull request as ready for review July 2, 2024 13:19
Copy link
Member

@emersion emersion left a comment

Choose a reason for hiding this comment

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

Nice, I think that's a good start!

Should we start introducing types/enums for roles and permissions, instead of strings? Would avoid mixing things up and typos.

Small patch to try out `ProtectedRoute` for testers
diff --git a/front/src/common/authorization/hooks/useCheckUserRole.ts b/front/src/common/authorization/hooks/useCheckUserRole.ts
index 38b3664b3d0e..618066f4883e 100644
--- a/front/src/common/authorization/hooks/useCheckUserRole.ts
+++ b/front/src/common/authorization/hooks/useCheckUserRole.ts
@@ -8,7 +8,7 @@ const useCheckUserRole = (requiredRoles: string[] = []) => {
   // - uncomment when user roles are implemented
   // const userRoles: string[] = [];
   // return requiredRoles.some((role) => userRoles.includes(role))
-  return true;
+  return false;
 };
 
 export default useCheckUserRole;
diff --git a/front/src/main/app.jsx b/front/src/main/app.jsx
index b9dfe7630a67..1c3586de627c 100644
--- a/front/src/main/app.jsx
+++ b/front/src/main/app.jsx
@@ -29,6 +29,7 @@ import { stdcmConfSlice } from 'reducers/osrdconf/stdcmConf';
 import stdcmConfSelectors from 'reducers/osrdconf/stdcmConf/selectors';
 import { useAppDispatch } from 'store';
 import useAuth from 'utils/hooks/OsrdAuth';
+import ProtectedRoute from 'common/authorization/components/ProtectedRoute.tsx';
 
 import('@sncf/bootstrap-sncf.metier.reseau/dist/css/bootstrap-sncf.min.css');
 
@@ -68,7 +69,7 @@ const router = createBrowserRouter([
     children: [
       {
         path: '*',
-        element: <HomeStdcm />,
+        element: <ProtectedRoute requiredRoles={['nope']}><HomeStdcm /></ProtectedRoute>,
       },
     ],
   },

front/src/common/authorization/components/Error403.tsx Outdated Show resolved Hide resolved
front/src/common/authorization/hooks/useCheckUserRole.ts Outdated Show resolved Hide resolved
front/src/reducers/authorization/listener.ts Outdated Show resolved Hide resolved
@kmer2016 kmer2016 changed the title Cnh/initialize authorization front front: Initialize authorization Jul 4, 2024
@kmer2016 kmer2016 force-pushed the cnh/initialize_authorization_front branch from 11748a4 to 97322f8 Compare July 8, 2024 15:00
@kmer2016 kmer2016 force-pushed the cnh/initialize_authorization_front branch 2 times, most recently from 66d197d to d5c14c4 Compare July 16, 2024 09:55
@kmer2016 kmer2016 requested a review from a team as a code owner July 16, 2024 14:22
.github/workflows/build.yml Outdated Show resolved Hide resolved
@kmer2016 kmer2016 force-pushed the cnh/initialize_authorization_front branch from 4619893 to 172abcc Compare July 16, 2024 15:05
@kmer2016 kmer2016 requested a review from a team as a code owner July 16, 2024 15:05
@kmer2016 kmer2016 force-pushed the cnh/initialize_authorization_front branch from 172abcc to 129cead Compare July 16, 2024 15:38
@flomonster flomonster removed the request for review from a team July 17, 2024 07:52
@kmer2016 kmer2016 force-pushed the cnh/initialize_authorization_front branch from 129cead to 237f6f8 Compare July 17, 2024 10:05
Copy link
Contributor

@clarani clarani left a comment

Choose a reason for hiding this comment

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

LGTM, tested

@kmer2016 kmer2016 force-pushed the cnh/initialize_authorization_front branch 2 times, most recently from e8aaa6d to 4c7b8d5 Compare July 19, 2024 08:44
@kmer2016 kmer2016 force-pushed the cnh/initialize_authorization_front branch from 4c7b8d5 to db5a9e7 Compare July 19, 2024 08:47
@kmer2016 kmer2016 added this pull request to the merge queue Jul 19, 2024
Merged via the queue into dev with commit e879445 Jul 19, 2024
20 checks passed
@kmer2016 kmer2016 deleted the cnh/initialize_authorization_front branch July 19, 2024 09:20
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.

Initialize auhorization
6 participants