[Authentification] ajout de Cerbère #1388
Conversation
thoomasbro
force-pushed
the
thomas/ajout-cerbere
branch
2 times, most recently
from
18c437c
to
11a611a
Compare
thoomasbro
force-pushed
the
thomas/ajout-cerbere
branch
from
e21d133
to
06fdc80
Compare
thoomasbro
force-pushed
the
thomas/ajout-cerbere
branch
from
06fdc80
to
e66f18e
Compare
backend/src/main/kotlin/fr/gouv/cacem/monitorenv/config/OIDCProperties.kt
Show resolved
Hide resolved
| // add the names of the request headers into the list | ||
| val n = e.nextElement() | ||
| set.add(n) | ||
| } |
There was a problem hiding this comment.
c'est possible de remplacer les variables e et n par des noms un peu plus explicites?
There was a problem hiding this comment.
oui. C'est une copie du code de fish.
.../gouv/cacem/monitorenv/infrastructure/api/endpoints/security/UserAuthorizationCheckFilter.kt
Outdated
Show resolved
Hide resolved
| @@ -2,10 +2,11 @@ | |||
| <html lang="fr"> | |||
| <head> | |||
| <meta charset="utf-8" /> | |||
| <link rel="preload" as="image" href="landing_background.png"> | |||
There was a problem hiding this comment.
penser à la remplacer? Ajouter un TODO ?
| return ( | ||
| <Wrapper data-cy="first-loader"> | ||
| <FulfillingBouncingCircleSpinner className="update-vessels" color={THEME.color.lightGray} size={48} /> | ||
| {isVesselShowed && <Icon.Vessel />} |
There was a problem hiding this comment.
petit détail à voir avec Adeline aussi pour l'icône
|
|
||
| ################################################################################ | ||
| # MonitorFish | ||
| MONITORFISH_API_KEY="" |
There was a problem hiding this comment.
on en a plus besoin de celui-ci normalement
There was a problem hiding this comment.
les api publiques de MonitorFish ont toujours besoin d'une clé, que MonitorEnv soit cerberisé ou non.
backend/src/main/kotlin/fr/gouv/cacem/monitorenv/config/SecurityConfig.kt
Outdated
Show resolved
Hide resolved
| "/error", | ||
| "/api/**", | ||
| "/version", | ||
| // TODO: secure SSE endpoints |
There was a problem hiding this comment.
todo: tu comptais le faire dans cette PR ou un autre ticket est existant ?
There was a problem hiding this comment.
il faut le faire dans une autre PR car c'est pas si évident. EventSource ne permet pas de passer les headers, mais pourrait passer correctement les cookies. Donc soit on change le back pour faire une sécu via cookies plutot que headers, soit on doit changer l'implem de EventSource coté client.
| import org.springframework.boot.context.properties.ConfigurationProperties | ||
| import org.springframework.stereotype.Component | ||
|
|
||
| @Component |
There was a problem hiding this comment.
| @Component | |
| @Configuration |
|
|
||
| return try { | ||
| userAuthorizationRepository.findByHashedEmail(hashedEmail) | ||
| } catch (e: Throwable) { |
There was a problem hiding this comment.
question: as-tu une raison particulière de catch les Error ?
There was a problem hiding this comment.
Effectivement, autoriser un type null en sortie de findByHashedEmail serait plus propre pour distinguer les erreurs de db du cas où juste l'utilisateur n'est pas en base.
| return try { | ||
| userAuthorizationRepository.findByHashedEmail(hashedEmail) | ||
| } catch (e: Throwable) { | ||
| logger.info("User $hashedEmail not found, defaulting to super-user=false") |
There was a problem hiding this comment.
question: On laisse les utilisateurs anonymes en lecture seule, si je comprends bien
There was a problem hiding this comment.
question: Y-a-t'il moyen d'avoir une image plus légère ?
| const auth = useAuth() | ||
| const { data: user } = useGetCurrentUserAuthorizationQueryOverride(undefined, { skip: !auth?.isAuthenticated }) | ||
|
|
||
| if (!oidcConfig.IS_OIDC_ENABLED) { |
There was a problem hiding this comment.
suggestion: un peu de refacto des conditions, sinon je m'y perds un peu. Qu'en penses-tu ?
| if (!oidcConfig.IS_OIDC_ENABLED) { | |
| if (!oidcConfig.IS_OIDC_ENABLED) { | |
| return children; | |
| } | |
| if (!auth.isAuthenticated) { | |
| return handleRedirect("/login", redirect); | |
| } | |
| if (requireSuperUser && !user?.isSuperUser) { | |
| return handleRedirect("/register", redirect); | |
| } | |
| return children; | |
| } | |
| const handleRedirect = (path, redirect) => { | |
| if (redirect) { | |
| return <Navigate replace to={path} />; | |
| } | |
| return null; | |
| } |
#1379