Skip to content

Vie privée: utilisation du ID public en place de PK dans les URLs#4395

Merged
calummackervoy merged 1 commit intomasterfrom
calum/candidate-slugs-access
Jul 11, 2024
Merged

Vie privée: utilisation du ID public en place de PK dans les URLs#4395
calummackervoy merged 1 commit intomasterfrom
calum/candidate-slugs-access

Conversation

@calummackervoy
Copy link
Contributor

🤔 Pourquoi ?

En tant qu’employeur, je ne dois pas récupérer les informations personnelles de tous les candidats des emplois

🍰 Comment ?

Remplacer l'utilisation des PKs du base de donnée avec les public_ids

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?

@calummackervoy
Copy link
Contributor Author

J'avais réflechis sur l'idée d'utiliser le public_id d'utilisateur en tant que PK, comme sur le modèle JobApplication. Mon instinct me dit non, mais ce serait un moyen sûr d'éviter ce genre de bug à la prochaine

Sinon, ce serait peut-être plus performante de mettre public_id en db_index=True?

@xavfernandez
Copy link
Contributor

Sinon, ce serait peut-être plus performante de mettre public_id en db_index=True?

Oui, il va falloir 👍

@calummackervoy
Copy link
Contributor Author

J'ai testé quelques URLs à la main, et j'ai ajouté quelques URLs dans mes changements ici qui n'étaient pas dans la spécification, mais où j'ai répliqué le bug

L'application est toujours nouvelle pour moi - ce serait efficace je pense si quelqu'un pourrait me faire un tutoriel sur le processus d'une candidature, et on pourrait voir ensemble s'il y a des autres vecteurs d'attaque

@calummackervoy
Copy link
Contributor Author

Sinon, ce serait peut-être plus performante de mettre public_id en db_index=True?

Oui, il va falloir 👍

#4397

@calummackervoy calummackervoy added the ajouté Ajouté dans le changelog. label Jul 9, 2024
)


class JobSeekerPrimaryKeyRedirectView(RedirectView):
Copy link
Contributor

Choose a reason for hiding this comment

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

Euh les redirect ne vont pas gérer les POST des formulaires donc toutes les personnes en cours de candidature/création de candidat/mise à jour de candidat vont perdre leur soumission de formulaire.
Pourquoi ne pas être parti sur <str:public_id> ?
Ou en alternative, conserver les doubles URLs:

path("edit_job_seeker_info/<int:job_seeker_public_id>", views.edit_job_seeker_info),
path("edit_job_seeker_info/<uuid:job_seeker_public_id>", views.edit_job_seeker_info, name="edit_job_seeker_info"),

Et gérer les deux formats dans les vues ?

Copy link
Contributor Author

@calummackervoy calummackervoy Jul 10, 2024

Choose a reason for hiding this comment

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

Après réflexion j'aimais pas la complexité de la gestion des différents types sur l'input, il y avait plus qu'un try/catch à faire - j'ai cherché une solution avec une redirection qui m'a semblé plus propre

Mince, merci de l'avoir signalé

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xavfernandez j'ai implementé ta suggestion : 13e5fa9

C'est digne de l'effort d'ajouter diverses tests sur ces endpoints tu penses, ou mieux d'aller plus vite avec des tests unitaires plus limités ?

Normalement j'aime pas trop les fichiers utils.py, mais j'ai choisis ce style afin de bien séparé la logique à supprimer dans quelques jours

Copy link
Contributor

Choose a reason for hiding this comment

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

Pour moi pas besoin de tests pour les vues avec job_seeker_pk: elles étaient testées jusque là et marchaient très bien + on va les supprimer dans quelques jours.

Copy link
Contributor

@xavfernandez xavfernandez left a comment

Choose a reason for hiding this comment

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

L'utils.py ne me choque pas 👍

makes it more difficult to use URLs to scrape applicant content from the server

reformat changes for PEP8

feat(submit_views): temporary redirects on depreceated job_seeker_pk urls

refactor(apply): PK views are supported without redirects

refactor(apply/urls): remove unused names on URL paths
@calummackervoy calummackervoy force-pushed the calum/candidate-slugs-access branch from 58b34e8 to f0ea7f2 Compare July 11, 2024 08:45
@calummackervoy calummackervoy added this pull request to the merge queue Jul 11, 2024
Merged via the queue into master with commit 4bb7c9b Jul 11, 2024
@calummackervoy calummackervoy deleted the calum/candidate-slugs-access branch July 11, 2024 15:16
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.

2 participants

Comments