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

Faire que l’api générique crud sur les usagers gère l’unicité du numéro en cas d’authn tél activée (#83700) #181

Merged
pmarillonnet merged 1 commits from wip/83700-api-users-crud-phone-uniqueness-tel into main 2024-04-02 11:28:24 +02:00
Conversation 1 Commits 1 Files Changed 2 +97 -1
pmarillonnet commented 2023-11-20 10:24:12 +01:00
Owner
Copy Link
No description provided.
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from 2df0b9c7f5 to 3f6f4b4a3b 2023-11-20 15:18:56 +01:00 Compare
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from 3f6f4b4a3b to 3337c9e1ef 2023-11-20 15:36:27 +01:00 Compare
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from 3337c9e1ef to ea99d2bc15 2023-11-20 16:59:25 +01:00 Compare
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from ea99d2bc15 to df6d1011f0 2023-11-20 17:20:00 +01:00 Compare
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from df6d1011f0 to fe1634044f 2023-11-21 09:52:14 +01:00 Compare
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from fe1634044f to d5cf4091b7 2023-11-21 10:03:31 +01:00 Compare
pmarillonnet changed title from WIP: Faire que l’api générique crud sur les usagers gère l’unicité du numéro en cas d’authn tél activée (#83700) to Faire que l’api générique crud sur les usagers gère l’unicité du numéro en cas d’authn tél activée (#83700) 2023-11-21 10:06:00 +01:00
bdauvergne reviewed 2023-12-20 17:23:40 +01:00
debian/debian_config.py Outdated
@ -145,6 +145,7 @@ def extract_settings_from_environ():
'A2_CAN_RESET_PASSWORD',
'A2_REGISTRATION_CAN_DELETE_ACCOUNT',
'A2_REGISTRATION_EMAIL_IS_UNIQUE',
'A2_REGISTRATION_PHONE_IS_UNIQUE',
bdauvergne commented 2023-12-20 17:23:40 +01:00
Owner
Copy Link

Ça arrive de #82737 il me semble, à retirer.

Ça arrive de #82737 il me semble, à retirer.
pmarillonnet commented 2024-02-06 11:36:09 +01:00
Author
Owner
Copy Link

Ok, en effet, erreur de rebasage, je retire.

Ok, en effet, erreur de rebasage, je retire.
pmarillonnet marked this conversation as resolved
bdauvergne reviewed 2023-12-20 17:23:46 +01:00
src/authentic2/a2_rbac/admin.py Outdated
@ -68,6 +68,7 @@ class OrganizationalUnitAdmin(admin.ModelAdmin):
'description',
'username_is_unique',
'email_is_unique',
'phone_is_unique',
bdauvergne commented 2023-12-20 17:23:46 +01:00
Owner
Copy Link

Ça arrive de #82737 il me semble, à retirer.

Ça arrive de #82737 il me semble, à retirer.
pmarillonnet commented 2024-02-06 11:38:16 +01:00
Author
Owner
Copy Link

Là c’est un souci d’empilement de PR dans gitea. Cette présente PR a d’ailleurs été fermée par erreur de ma part, je vais rouvrir et configurer correctement pour que les modification (déjà mergées) de #82737 n’apparaissent plus ici.

Là c’est un souci d’empilement de PR dans gitea. Cette présente PR a d’ailleurs été fermée par erreur de ma part, je vais rouvrir et configurer correctement pour que les modification (déjà mergées) de #82737 n’apparaissent plus ici.
pmarillonnet marked this conversation as resolved
bdauvergne reviewed 2023-12-20 17:23:59 +01:00
src/authentic2/a2_rbac/migrations/0038_organizationalunit_phone_is_unique.py Outdated
@ -0,0 +1,17 @@
# Generated by Django 3.2.18 on 2023-11-06 15:47
bdauvergne commented 2023-12-20 17:23:59 +01:00
Owner
Copy Link

Ça arrive de #82737 il me semble, à retirer.

Ça arrive de #82737 il me semble, à retirer.
pmarillonnet marked this conversation as resolved
bdauvergne requested changes 2023-12-20 17:46:08 +01:00
Dismissed
src/authentic2/api_views.py Outdated
@ -545,0 +558,4 @@
ou = attrs.get('ou', None) or get_default_ou()
if ou.phone_is_unique:
qs = qs.annotate(
ou_id=models.expressions.RawSQL(
bdauvergne commented 2023-12-20 17:37:09 +01:00
Owner
Copy Link

On peut utiliser une subquery avec OuterRef pour ça :

ou_id=User.objects.filter(pk=models.OuterRef('object_id')).values_list('ou_id', flat=True),
On peut utiliser une subquery avec OuterRef pour ça : ``` ou_id=User.objects.filter(pk=models.OuterRef('object_id')).values_list('ou_id', flat=True), ```
pmarillonnet commented 2024-02-06 11:40:41 +01:00
Author
Owner
Copy Link

Oui complètement, je vais modifier.

Oui complètement, je vais modifier.
pmarillonnet commented 2024-02-06 16:03:19 +01:00
Author
Owner
Copy Link

C’est modifié.

C’est modifié.
src/authentic2/api_views.py Outdated
@ -545,0 +564,4 @@
(),
),
).filter(ou_id=ou.id)
elif not app_settings.A2_PHONE_IS_UNIQUE: # skip uniqueness checks
bdauvergne commented 2023-12-20 17:45:28 +01:00
Owner
Copy Link

Je ne m'embêterai pas avec ça on va considérer juste l'unicité au niveau de l'ou; en multico c'est ce qu'on veut de toute façon. On peut même virer les settings correspondant partout, pareil pour A2_REGISTRATION_PHONE_IS_UNIQUE. Tu pourrais déplacer le errors['attributes'] = ... directement dans le if ainsi et retirer le return attrs précoce.

Je ne m'embêterai pas avec ça on va considérer juste l'unicité au niveau de l'ou; en multico c'est ce qu'on veut de toute façon. On peut même virer les settings correspondant partout, pareil pour A2_REGISTRATION_PHONE_IS_UNIQUE. Tu pourrais déplacer le `errors['attributes'] = ...` directement dans le if ainsi et retirer le `return attrs` précoce.
pmarillonnet commented 2024-02-06 11:46:43 +01:00
Author
Owner
Copy Link

Ok, faisons simple, je vais virer tout ça.

Ok, faisons simple, je vais virer tout ça.
pmarillonnet commented 2024-02-06 16:16:38 +01:00
Author
Owner
Copy Link

Après réflexion je me dis qu’on pourrait laisser ces settings globaux maintenant qu’ils sont implémentés et testés, qu’il y a peut-être quand même des cas de multico où on veut quand même un téléphone unique partout ?

Après réflexion je me dis qu’on pourrait laisser ces settings globaux maintenant qu’ils sont implémentés et testés, qu’il y a peut-être quand même des cas de multico où on veut quand même un téléphone unique partout ?
pmarillonnet commented 2024-02-07 09:24:02 +01:00
Author
Owner
Copy Link

(Par contre bien sûr dans cette partie du code il y avait un bug, un return attrs en dépit de toute gestion des erreurs précédemment relevées, c’est corrigé dans cette nouvelle version de la PR.)

(Par contre bien sûr dans cette partie du code il y avait un bug, un `return attrs` en dépit de toute gestion des erreurs précédemment relevées, c’est corrigé dans cette nouvelle version de la PR.)
pmarillonnet closed this pull request 2024-01-15 08:38:19 +01:00
pmarillonnet reopened this pull request 2024-02-06 11:58:14 +01:00
pmarillonnet changed target branch from wip/82737-authn-tel-post-registration-account-selection-buggy-form to main 2024-02-06 11:58:21 +01:00
pmarillonnet changed title from Faire que l’api générique crud sur les usagers gère l’unicité du numéro en cas d’authn tél activée (#83700) to WIP: Faire que l’api générique crud sur les usagers gère l’unicité du numéro en cas d’authn tél activée (#83700) 2024-02-06 11:58:37 +01:00
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from f01cc42c60 to 41ea8c80c1 2024-02-06 12:19:17 +01:00 Compare
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from 41ea8c80c1 to 609167f6e5 2024-02-06 15:41:40 +01:00 Compare
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from 609167f6e5 to 70e2e2e6ce 2024-02-06 16:02:57 +01:00 Compare
pmarillonnet changed title from WIP: Faire que l’api générique crud sur les usagers gère l’unicité du numéro en cas d’authn tél activée (#83700) to Faire que l’api générique crud sur les usagers gère l’unicité du numéro en cas d’authn tél activée (#83700) 2024-02-06 16:16:48 +01:00
pmarillonnet requested review from bdauvergne 2024-02-06 16:16:52 +01:00
bdauvergne force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from 70e2e2e6ce to bfa0a62ed3 2024-03-20 20:05:00 +01:00 Compare
bdauvergne approved these changes 2024-03-21 15:16:23 +01:00
Dismissed
src/authentic2/api_views.py Outdated
@ -419,6 +419,7 @@ class BaseUserSerializer(serializers.ModelSerializer):
if ou and ou.email_is_unique and qs.filter(ou=ou, email__iexact=attrs['email']).exists():
already_used = True
authenticator = utils_misc.get_password_authenticator()
bdauvergne commented 2024-03-21 15:09:57 +01:00
Owner
Copy Link

Déplacer la ligne plus bas, il n'y a pas de raison de la mettre là et ça fait un changement plus compact.

Déplacer la ligne plus bas, il n'y a pas de raison de la mettre là et ça fait un changement plus compact.
pmarillonnet marked this conversation as resolved
src/authentic2/api_views.py Outdated
@ -434,6 +435,31 @@ class BaseUserSerializer(serializers.ModelSerializer):
hasher.safe_summary(attrs.get('hashed_password'))
except Exception:
errors['hashed_password'] = 'hash format error'
if authenticator.is_phone_authn_active and (
bdauvergne commented 2024-03-21 15:11:49 +01:00
Owner
Copy Link

En restant dans une logique Django ce code devrait faire partie d'un User.clean() (c'est d'ailleurs peut-être déjà le cas) et être réutilisé, je ne vais pas demander ça ici, mais il faudrait garder ça à l'esprit, les contraintes sur les modèles doivent rester autant que possible dans les modèles.

En restant dans une logique Django ce code devrait faire partie d'un User.clean() (c'est d'ailleurs peut-être déjà le cas) et être réutilisé, je ne vais pas demander ça ici, mais il faudrait garder ça à l'esprit, les contraintes sur les modèles doivent rester autant que possible dans les modèles.
pmarillonnet marked this conversation as resolved
src/authentic2/api_views.py Outdated
@ -437,0 +445,4 @@
if self.instance:
qs.exclude(object_id=self.instance.id)
else:
qs.exclude(object_id__isnull=True)
bdauvergne commented 2024-03-21 15:14:03 +01:00
Owner
Copy Link

Je pense que ça n'a pas d'effet, aucun attribut ne devrait être accroché à rien, la branche else peut être retirée, ou si c'est un vrai problème observé il faudrait nettoyer tous ces attributs qui traînent.

Je pense que ça n'a pas d'effet, aucun attribut ne devrait être accroché à rien, la branche else peut être retirée, ou si c'est un vrai problème observé il faudrait nettoyer tous ces attributs qui traînent.
pmarillonnet marked this conversation as resolved
src/authentic2/api_views.py Outdated
@ -437,0 +452,4 @@
if not ou.phone_is_unique:
qs = qs.none()
else:
qs = qs.annotate(
bdauvergne commented 2024-03-21 15:16:15 +01:00
Owner
Copy Link

Ça s'écrit plus simplement .filter(object_id__in=User.objects.filter(ou=ou).values_list('id')), je ne suis même pas certain que le values_list() soit nécessaire.

Ça s'écrit plus simplement `.filter(object_id__in=User.objects.filter(ou=ou).values_list('id'))`, je ne suis même pas certain que le values_list() soit nécessaire.
pmarillonnet marked this conversation as resolved
bdauvergne approved these changes 2024-03-21 15:16:42 +01:00
bdauvergne left a comment
Owner
Copy Link

Avec quelques remarques.

Avec quelques remarques.
bdauvergne pinned this 2024-03-22 00:21:48 +01:00
pmarillonnet commented 2024-04-02 09:51:59 +02:00
Author
Owner
Copy Link

Avec quelques remarques.

Merci, je vais inclure tes remarques puis pousser dans la branche main.

> Avec quelques remarques. Merci, je vais inclure tes remarques puis pousser dans la branche main.
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from bfa0a62ed3 to 44b323f87c 2024-04-02 10:14:27 +02:00 Compare
pmarillonnet added 1 commit 2024-04-02 10:24:16 +02:00
gitea/authentic/pipeline/head This commit looks good Details
a57583041a [wip/tosquash] modifs first batch
pmarillonnet added 1 commit 2024-04-02 10:25:17 +02:00
gitea/authentic/pipeline/head This commit looks good Details
8b566b819c [wip/tosquash] modifs, last batch
pmarillonnet force-pushed wip/83700-api-users-crud-phone-uniqueness-tel from 8b566b819c to 0abfbc9480 2024-04-02 11:22:49 +02:00 Compare
pmarillonnet merged commit 0abfbc9480 into main 2024-04-02 11:28:24 +02:00
pmarillonnet unpinned this 2024-04-16 11:21:32 +02:00
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
2 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#181
No description provided.
Delete Branch "wip/83700-api-users-crud-phone-uniqueness-tel"

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?