-
Notifications
You must be signed in to change notification settings - Fork 7
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
SSO - Correction des erreurs de connexion #3763
Conversation
0e2721d
to
8194f0c
Compare
export function useGetUserAccount(): UserAccountContextType { | ||
// `| undefined` because it's undefined if the OIDC is disabled which is the case for Cypress tests | ||
const auth = useAuth() as AuthContextProps | undefined | ||
const { trackUserId } = useTracking() | ||
const { data: user } = useGetCurrentUserAuthorizationQueryOverride({ skip: !auth?.isAuthenticated }) | ||
const { data: user } = useGetCurrentUserAuthorizationQueryOverride({ skip: !IS_CYPRESS && !auth?.isAuthenticated }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'aimerais bien avoir une autre méthode que ça pour stubber cette requête, mais je n'arrive pas à utiliser cy.stub
pour useAuth
.
@ivangabriele je suis preneur d'une idée si ut en as!
@@ -0,0 +1,16 @@ | |||
export const paths = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tu trouves ça plus clair d'avoir les paths à part ? (je pose la question parce que le fait d'utiliser les routes "déclaratives" via un const routes = []
est déjà déclaratif en soi)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cela nous permet de ne pas avoir de magic string dans le code qui référence ces routes
import { LoadingSpinnerWall } from '../../ui/LoadingSpinnerWall' | ||
import { useGetUserAccount } from '../hooks/useGetUserAccount' | ||
|
||
export function RequireAuth({ children, redirect = false, requireSuperUser = false }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export function RequireAuth({ children, redirect = false, requireSuperUser = false }) { | |
export function Protceted({ children, redirect = false, mustBeSuperUser = false }) { |
100% taste, tout-à-fait optionnel ^^.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Après discussion, on reste sur ce nommage pour être iso avec Env
@@ -8,81 +8,135 @@ import { BackofficePage } from '@pages/BackofficePage' | |||
import { HomePage } from '@pages/HomePage' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je te propose de regrouper plutôt toutes les protected routes sous le même parent. Exemple : remix-run/react-router#10637 (comment).
Quality Gate passedIssues Measures |
Linked issues
<RequireAuth/>
, sur le même principe que sur MonitorEnv