Skip to content

Permissions : Création de groupes pour l'administration de GPS et Pilotage#5309

Merged
rsebille merged 8 commits into
masterfrom
rsebille/more-groups-and-perms
Dec 31, 2024
Merged

Permissions : Création de groupes pour l'administration de GPS et Pilotage#5309
rsebille merged 8 commits into
masterfrom
rsebille/more-groups-and-perms

Conversation

@rsebille

@rsebille rsebille commented Dec 23, 2024

Copy link
Copy Markdown
Contributor

🤔 Pourquoi ?

Idées directrices (long terme)

  • Ne plus mettre tout le monde super-admin #Sécurité
  • Gérer les droits plus finement en fonction du besoin de la personne ou de son équipe #RGPD #PDI
  • Pouvoir voir le plus facilement possible qui a accès à quoi #Auditabilité

Besoin immédiat

Ne pas avoir de droits personnels sans donner accès à tout le reste via le groupe itou-admin
image

Simplifier la gestion manuelle de HIJACK_ALLOWED_USER_EMAILS qui au final contient les emails des personnes concernés par ces 2 groupes

@rsebille rsebille added the ajouté Ajouté dans le changelog. label Dec 23, 2024
@rsebille rsebille self-assigned this Dec 23, 2024
@rsebille
rsebille force-pushed the rsebille/more-groups-and-perms branch from 00dbe84 to aa208be Compare December 30, 2024 11:06
@rsebille rsebille changed the title Permissions : Création d'un groupe pour l'administration de GPS Permissions : Création de groupes pour l'administration de GPS et Pilotage Dec 30, 2024
@rsebille
rsebille force-pushed the rsebille/more-groups-and-perms branch from aa208be to 3e13ecd Compare December 30, 2024 11:13
gps_models.FollowUpGroup: PERMS_ALL,
gps_models.FollowUpGroupMembership: PERMS_ALL,
users_models.User: PERMS_ADD | PERMS_HIJACK,
users_models.JobSeekerProfile: PERMS_EDIT,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pourquoi permettre à GPS de mettre à jour les informations de profil ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Car je me suis dit que si GPS a besoin d'ajouter des bénéficiaires donc des comptes candidats alors la modification du profil était aussi nécessaire, c'est quelque chose qui a été demandé la semaine dernière par Camille car LJ est en congés et vu qu'il est dans itou-admin bah il a toujours pû 😅.
C'est pour ça que j'ai mis la team GPS pour la relecture, car je sais pas du tout ce qui est OK ou pas 😏.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Je comprends mieux.
L'idée me met plutôt mal à l'aise car on risque de se retrouver avec des utilisateurs des emplois qui n'ont rien à voir avec le service. Nos règles de gestion peuvent aussi évoluer sans que GPS ne soit au courant. Il y a un risque de données incohérentes. Bon, après, le risque est faible car les cas concernés seront marginaux.
Pour garder une trace des User et UserProfile créés par GPS, je propose que le created_by soit renseigné automatiquement en indiquant le compte de Louis-Jean ou de Camille. Qu'en penses-tu ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Traité pour User() dans cette PR : #5321
Pour les jobSeekerProfile() ce n'est pas nécessaire car leur création est lié à la création d'un User() quand celui-ci est un candidat, il n'y a d'ailleurs pas de champs created_by sur le modèle, et de toute façon les permissions n'autorisent justement pas la création de ce type d'objet.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Merci !

Comment thread itou/users/models.py
),
),
]
permissions = [("hijack_user", "Can impersonate (hijack) other accounts")]

@celine-m-s celine-m-s Dec 30, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Excellente idée ! Je suppose donc qu'une autre PR viendra mettre à jour la permission des utilisateurs concernés puis supprimer HIJACK_ALLOWED_USER_EMAILS de itou-secrets ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

La mise à jour des permissions ne sera pas faite dans une PR pour ne pas faire fuiter les emails mais effectivement une PR suivra pour supprimer HIJACK_ALLOWED_USER_EMAILS sauf si quelqu'un trouve un cas qui demanderais de garder cette liste blanche plutôt que d'utiliser la permission Django.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👌 Si le cas existe, en tous cas, il ne me saute pas aux yeux.

@celine-m-s celine-m-s left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ezgif com-webp-to-gif-converter

@tonial tonial left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread itou/utils/perms/user.py Outdated

# Only whitelisted staff members can hijack other accounts
if hijacker.is_staff and hijacker.email.lower() in settings.HIJACK_ALLOWED_USER_EMAILS:
if hijacker.is_staff and (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Je pense que tu aurais pu directement retirer le settings, et ajouter la permission dans la foulée.
La probabilité que tu gènes quelqu'un pendant ces quelques minutes est faible, non ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Je l'ai pas tant fait pour gêner ou pas que j'avais un doute de si on pouvais encore avoir besoin du settings, et vu que j'accumule les PR ouvertes je voulais pas en "bloquer" une avec une action synchrone, mais je vais rajouter le commit et la passer cette semaine vu que j'ai eu des tampons ;)

@rsebille
rsebille force-pushed the rsebille/more-groups-and-perms branch from 3e13ecd to 2257bf6 Compare December 31, 2024 08:23
@rsebille
rsebille added this pull request to the merge queue Dec 31, 2024
Merged via the queue into master with commit d7b66d0 Dec 31, 2024
@rsebille
rsebille deleted the rsebille/more-groups-and-perms branch December 31, 2024 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ajouté Ajouté dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants