entrouvert
/
authentic
16
0
Fork 0
Code Issues Pull Requests 35 Releases Activity

misc: consider user-initiated account creation as authn event (#82736) #161

Open
pmarillonnet wants to merge 1 commits from wip/82736-account-create-as-recent-authn into main
pull from: wip/82736-account-create-as-recent-authn
merge into: entrouvert:main
Conversation 4 Commits 1 Files Changed 10 +79 -54
pmarillonnet commented 2023-11-02 11:41:18 +01:00
Owner
Copy Link
No description provided.
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from 3a76524734 to ad261941c2 2023-11-02 12:08:34 +01:00 Compare
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from ad261941c2 to 6d4745a063 2023-11-02 14:26:18 +01:00 Compare
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from 6d4745a063 to ae490a74c9 2023-11-02 14:50:19 +01:00 Compare
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from ae490a74c9 to 555f2aefcb 2023-11-02 15:02:00 +01:00 Compare
pmarillonnet changed title from WIP: misc: consider user-initiated account creation as authn event (#82736) to misc: consider user-initiated account creation as authn event (#82736) 2023-11-02 15:02:06 +01:00
vdeniaud commented 2023-11-15 17:01:57 +01:00
Owner
Copy Link

Il me semble que le patch introduit une différence entre le « how » d'une inscription et le « how » d'une connexion, or il y a du code qui s'attend à ce que les deux soient pareils, genre src/authentic/src/authentic2/journal_event_types.py::login_method_label, mais pas sûr que ce soit le seul endroit. (mais dans cette méthode la valeur 'email' n'est pas traduite et pourtant je vois que c'est celle par défaut dans le code modifié, donc peut-être que ce qu'on fait est déjà lacunaire)

En tout cas une solution sans toucher au how qu'on enregistre serait carrément plus safe (en ajoutant éventuellement des valeurs pour couvrir le cas téléphone, bien sûr).

Il me semble que le patch introduit une différence entre le « how » d'une inscription et le « how » d'une connexion, or il y a du code qui s'attend à ce que les deux soient pareils, genre src/authentic/src/authentic2/journal_event_types.py::login_method_label, mais pas sûr que ce soit le seul endroit. (mais dans cette méthode la valeur 'email' n'est pas traduite et pourtant je vois que c'est celle par défaut dans le code modifié, donc peut-être que ce qu'on fait est déjà lacunaire) En tout cas une solution sans toucher au how qu'on enregistre serait carrément plus safe (en ajoutant éventuellement des valeurs pour couvrir le cas téléphone, bien sûr).
pmarillonnet commented 2023-11-16 14:23:59 +01:00
Author
Owner
Copy Link

Il me semble que le patch introduit une différence entre le « how » d'une inscription et le « how » d'une connexion, or il y a du code qui s'attend à ce que les deux soient pareils, genre src/authentic/src/authentic2/journal_event_types.py::login_method_label, mais pas sûr que ce soit le seul endroit. (mais dans cette méthode la valeur 'email' n'est pas traduite et pourtant je vois que c'est celle par défaut dans le code modifié, donc peut-être que ce qu'on fait est déjà lacunaire)

En tout cas une solution sans toucher au how qu'on enregistre serait carrément plus safe (en ajoutant éventuellement des valeurs pour couvrir le cas téléphone, bien sûr).

Ok fair enough, je n’étais pas parti dans cette direction qui laisserait l’existant inchangé parce que le code produit me paraissait un peu bancal (genre une valeur 'password-on-https' faussement générique car elle exclut l’authn tél, laquelle correspond à une autre valeur) mais on peut essayer. Je vais proposer une v2 qui laisse inchangées les valeurs existantes.

> Il me semble que le patch introduit une différence entre le « how » d'une inscription et le « how » d'une connexion, or il y a du code qui s'attend à ce que les deux soient pareils, genre src/authentic/src/authentic2/journal_event_types.py::login_method_label, mais pas sûr que ce soit le seul endroit. (mais dans cette méthode la valeur 'email' n'est pas traduite et pourtant je vois que c'est celle par défaut dans le code modifié, donc peut-être que ce qu'on fait est déjà lacunaire) > > En tout cas une solution sans toucher au how qu'on enregistre serait carrément plus safe (en ajoutant éventuellement des valeurs pour couvrir le cas téléphone, bien sûr). Ok fair enough, je n’étais pas parti dans cette direction qui laisserait l’existant inchangé parce que le code produit me paraissait un peu bancal (genre une valeur `'password-on-https'` faussement générique car elle exclut l’authn tél, laquelle correspond à une autre valeur) mais on peut essayer. Je vais proposer une v2 qui laisse inchangées les valeurs existantes.
pmarillonnet changed title from misc: consider user-initiated account creation as authn event (#82736) to WIP: misc: consider user-initiated account creation as authn event (#82736) 2023-11-16 14:24:19 +01:00
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from 555f2aefcb to 5343dad279 2023-12-06 11:15:04 +01:00 Compare
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from 5343dad279 to ab0594e248 2023-12-06 17:00:22 +01:00 Compare
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from 44dde3b307 to 49aff8862a 2023-12-07 15:35:29 +01:00 Compare
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from 49aff8862a to 7583b0ea65 2023-12-07 15:59:50 +01:00 Compare
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from 7583b0ea65 to df985386f7 2023-12-07 16:22:27 +01:00 Compare
pmarillonnet commented 2023-12-07 16:34:29 +01:00
Author
Owner
Copy Link

Il me semble que le patch introduit une différence entre le « how » d'une inscription et le « how » d'une connexion, or il y a du code qui s'attend à ce que les deux soient pareils, genre src/authentic/src/authentic2/journal_event_types.py::login_method_label, mais pas sûr que ce soit le seul endroit. (mais dans cette méthode la valeur 'email' n'est pas traduite et pourtant je vois que c'est celle par défaut dans le code modifié, donc peut-être que ce qu'on fait est déjà lacunaire)

En tout cas une solution sans toucher au how qu'on enregistre serait carrément plus safe (en ajoutant éventuellement des valeurs pour couvrir le cas téléphone, bien sûr).

Ok fair enough, je n’étais pas parti dans cette direction qui laisserait l’existant inchangé parce que le code produit me paraissait un peu bancal (genre une valeur 'password-on-https' faussement générique car elle exclut l’authn tél, laquelle correspond à une autre valeur) mais on peut essayer. Je vais proposer une v2 qui laisse inchangées les valeurs existantes.

Pour info, j’ai creusé la piste qui laisserait l’existant inchangé, et ça me paraissait quand même super bancale et pas élégant du tout. Je vais revenir à la charge avec une version qui refond l’existant et propose des nouveaux “how” cohérents, que ce soit pour l’inscription ou la connexion, cette fois-ci sans lacunes dans le code.

> > Il me semble que le patch introduit une différence entre le « how » d'une inscription et le « how » d'une connexion, or il y a du code qui s'attend à ce que les deux soient pareils, genre src/authentic/src/authentic2/journal_event_types.py::login_method_label, mais pas sûr que ce soit le seul endroit. (mais dans cette méthode la valeur 'email' n'est pas traduite et pourtant je vois que c'est celle par défaut dans le code modifié, donc peut-être que ce qu'on fait est déjà lacunaire) > > > > En tout cas une solution sans toucher au how qu'on enregistre serait carrément plus safe (en ajoutant éventuellement des valeurs pour couvrir le cas téléphone, bien sûr). > > Ok fair enough, je n’étais pas parti dans cette direction qui laisserait l’existant inchangé parce que le code produit me paraissait un peu bancal (genre une valeur `'password-on-https'` faussement générique car elle exclut l’authn tél, laquelle correspond à une autre valeur) mais on peut essayer. Je vais proposer une v2 qui laisse inchangées les valeurs existantes. Pour info, j’ai creusé la piste qui laisserait l’existant inchangé, et ça me paraissait quand même super bancale et pas élégant du tout. Je vais revenir à la charge avec une version qui refond l’existant et propose des nouveaux “how” cohérents, que ce soit pour l’inscription ou la connexion, cette fois-ci sans lacunes dans le code.
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from df985386f7 to 096c325250 2023-12-13 14:19:40 +01:00 Compare
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from 096c325250 to 129408d6d2 2023-12-13 14:24:51 +01:00 Compare
pmarillonnet commented 2023-12-13 14:27:11 +01:00
Author
Owner
Copy Link

Voilà une façon à mon avis plus cohérente de procéder, qui implique de revoir l’existant.
Je crois que le prix à payer est de perdre le label human-friendly des événements d’authentification survenus avant l’application du patch, ça n’a pas l’air bien méchant.

Voilà une façon à mon avis plus cohérente de procéder, qui implique de revoir l’existant. Je crois que le prix à payer est de perdre le label human-friendly des événements d’authentification survenus avant l’application du patch, ça n’a pas l’air bien méchant.
pmarillonnet changed title from WIP: misc: consider user-initiated account creation as authn event (#82736) to misc: consider user-initiated account creation as authn event (#82736) 2023-12-13 14:27:15 +01:00
pmarillonnet force-pushed wip/82736-account-create-as-recent-authn from 129408d6d2 to 2f7f569808 2023-12-20 14:54:19 +01:00 Compare
bdauvergne requested changes 2024-01-16 17:25:49 +01:00
bdauvergne left a comment
Owner
Copy Link

Est-ce qu'on ne pouvait pas rajouter 'phone' et 'email' dans la liste de can_authenticate_with_password pour rester dans l'intitulé du ticket et s'éviter un patch aussi long (je découvre le ticket désolé).

Est-ce qu'on ne pouvait pas rajouter 'phone' et 'email' dans la liste de can_authenticate_with_password pour rester dans l'intitulé du ticket et s'éviter un patch aussi long (je découvre le ticket désolé).
All checks were successful
gitea/authentic/pipeline/head This commit looks good
Details
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b wip/82736-account-create-as-recent-authn main
git pull origin wip/82736-account-create-as-recent-authn

Step 2:

Merge the changes and update on Gitea.
git checkout main
git merge --no-ff wip/82736-account-create-as-recent-authn
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
bdauvergne
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/authentic#161
No description provided.
Delete Branch "wip/82736-account-create-as-recent-authn"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?