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

[FIX][AUTH] Fix Public Access with CAS INPN and other bugs #3360

Merged
merged 3 commits into from
Feb 6, 2025

Conversation

VincentCauchois
Copy link
Member

@VincentCauchois VincentCauchois commented Feb 4, 2025

Several fixes:

  • Fix direct public access with query param when INPN CAS authentication is configured
  • Fix flaky login and logout - ref Accès public à GeoNature - Lien ne fonctionne pas au premier chargement #3353
  • Use AuthGuard rather than UserCasGuard on NavHomeComponent, and remove UserCasGuard definition
  • Handle the case where a user is no more authenticated server-side but still authenticated client-side, by calling frontend logout()
  • Fix login and logout for Firefox clients :
    • Replace legacy library Moment.js with library Luxon
    • Fix frontend logout() by waiting for backend logout return before redirection to login page

Chore:

  • Remove unused Router and AuthService from ModuleService constructor

@VincentCauchois VincentCauchois self-assigned this Feb 4, 2025
@VincentCauchois VincentCauchois force-pushed the fix/public-access-with-cas-inpn branch 2 times, most recently from a8b95d9 to 757c928 Compare February 4, 2025 14:35
@edelclaux
Copy link
Contributor

Nice ! Tu as trouvé la source du problème de l'accès public ?

@VincentCauchois VincentCauchois force-pushed the fix/public-access-with-cas-inpn branch 4 times, most recently from 82aa6b6 to 204b9fe Compare February 5, 2025 16:23
@VincentCauchois VincentCauchois changed the title [WIP]fix(login): fix public access with cas inpn authentification [FIX][AUTH] Fix Public Access with CAS INPN and other bugs Feb 5, 2025
@VincentCauchois VincentCauchois marked this pull request as ready for review February 5, 2025 16:24
@VincentCauchois
Copy link
Member Author

VincentCauchois commented Feb 5, 2025

Nice ! Tu as trouvé la source du problème de l'accès public ?

Oui, à l'aide de breakpoints j'ai fini par voir que ce problème - que tu as relevé dans #3353 - était dû à l'appel de .toPromise() sur le retour de authService.manageUser(data) ici :

await authService.manageUser(data).toPromise();
. La fonction manageUser -
manageUser(data): any {
this.setSession(data);
const userForFront = {
user_login: data.user.identifiant,
prenom_role: data.user.prenom_role,
id_role: data.user.id_role,
nom_role: data.user.nom_role,
nom_complet: data.user.nom_role + ' ' + data.user.prenom_role,
id_organisme: data.user.id_organisme,
};
this.setCurrentUser(userForFront);
this.loginError = false;
}
- en question ne renvoie rien, et une exception pouvait donc être observée à l'appel de .toPromise(). La ligne suivante, qui s'occupe du chargement des routes des modules et de la redirection après authentification, n'était, par suite, pas exécutée. Toutefois, l'exécution de manageUser aboutissant bien par ailleurs, l'utilisateur était en fait bien logged in, et un second chargement permettait donc d'aboutir correctement à GeoNature en étant authentifié en public.

Il y avait quand même, par ailleurs, beaucoup d'autres anomalies avec l'authentification, comme tu peux le voir sur cette PR, du moins en 2.14.2, et je vais m'occuper de mettre les correctifs nécessaires, dont celui sur develop dès que possible.

@edelclaux
Copy link
Contributor

J'ai pris juste la partie de l'accès public, dépoyée en hotfix sur l'instance en question. Et c'est niquel !
Merci pour le débroussaillage !

Several fixes:
- Fix direct public access with query param when INPN CAS authentification is configured
- Fix flaky login and logout - ref #3353
- Use AuthGuard rather than UserCasGuard on NavHomeComponent, and remove UserCasGuard definition
- Handle the case where a user is no more authentified server-side but still authentified client-side, by calling frontend logout()

Chore:
- Remove unused Router and AuthService from ModuleService constructor
Fix login and logout for Firefox clients.

The library Moment.js is, if not deprecated, at least outdated and not fully functional on every client.
For more information, see: https://momentjs.com/docs/#/-project-status/

The current commit replace use of Moment.js with Luxon in `auth.service.ts` to ensure correct functioning of functions:
- `isLoggedIn()`
- `isLoggedOut()`
- `getExpiration()`
Previous implementation of logout(), with routing to login page after asynchronous request to backend /auth/logout endpoint, would not work on every client.
Also, while still functional on Chrome clients, the previous implementation was weird as one would get redirected to the login page while possibly not yet fully logged out.

This new implementation executes routing to login page after backend logout is done.
@VincentCauchois VincentCauchois force-pushed the fix/public-access-with-cas-inpn branch from 204b9fe to b4dddd0 Compare February 6, 2025 15:15
@Christophe-Ramet Christophe-Ramet merged commit d9bc297 into fix/ginco-2.14 Feb 6, 2025
4 checks passed
@Christophe-Ramet Christophe-Ramet deleted the fix/public-access-with-cas-inpn branch February 6, 2025 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants