DGNum/authens
17
0
Fork 0
Code Issues 2 Pull requests Projects Releases 2 Packages Wiki Activity

Helper pour lier un compte Django existant à un compte CAS #35

Merged
mpepin merged 0 commits from kerl/ldap into master 2020-06-22 10:10:55 +02:00
Conversation 25 Commits - Files changed -
mpepin commented 2020-06-15 19:13:05 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Répond partiellement à #13. Je ne sais pas si on a besoin d'un helper pour OldCASAccount aussi 🤔

Petit side-effect : je prépare le terrain pour avoir des settings propres à authens.
Ça risque d'être très peu utilisé anyways, mais ça me semble plus propre de mettre l'url du ldap et celle du CAS dans un settings en fait, avec la bonne valeur par défaut bien sûr.

Répond partiellement à #13. Je ne sais pas si on a besoin d'un helper pour `OldCASAccount` aussi :thinking: Petit side-effect : je prépare le terrain pour avoir des settings propres à authens. Ça risque d'être très peu utilisé anyways, mais ça me semble plus propre de mettre l'url du ldap et celle du CAS dans un settings en fait, avec la bonne valeur par défaut bien sûr.
mpepin commented 2020-06-15 19:13:06 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

changed milestone to %1

changed milestone to %1
mpepin commented 2020-06-15 19:15:31 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

changed the description

changed the description
lstephan commented 2020-06-19 16:32:25 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Tu voudrais le rendre optionnel comment ?

Tu voudrais le rendre optionnel comment ?
lstephan commented 2020-06-19 16:44:45 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Il y a un avantage à faire ça plutôt que directement LDAP_SERVER_URL = ... ?

Il y a un avantage à faire ça plutôt que directement `LDAP_SERVER_URL = ...` ?
lstephan commented 2020-06-19 16:49:37 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Ça ne devrait jamais arriver, mais mettre un try...except au cas où le LDAP merde et renvoie 2 résultats ?

Ça ne devrait jamais arriver, mais mettre un `try...except` au cas où le LDAP merde et renvoie 2 résultats ?
lstephan commented 2020-06-19 16:51:55 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Ça renvoie une erreur pas ultra lisible (NoneType is not subscriptable, je pense) si cas_login ne correspond pas à un utilisateur.

Ça renvoie une erreur pas ultra lisible (`NoneType is not subscriptable`, je pense) si `cas_login` ne correspond pas à un utilisateur.
lstephan commented 2020-06-19 16:52:49 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

C'est faux dans l'implem actuelle, le cas_login n'a pas de valeur par défaut.

C'est faux dans l'implem actuelle, le `cas_login` n'a pas de valeur par défaut.
lstephan commented 2020-06-19 16:57:22 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Il me semble que tu testes deux fois la même chose ici ? Tu ne peux pas différencier la ValueError qui vient du fait que le login est réutilisé, et celle qui vient du fait que l'utilisateur a déjà un compte; il faudrait un 2e user pour le 1er test.

Il me semble que tu testes deux fois la même chose ici ? Tu ne peux pas différencier la `ValueError` qui vient du fait que le login est réutilisé, et celle qui vient du fait que l'utilisateur a déjà un compte; il faudrait un 2e user pour le 1er test.
lstephan commented 2020-06-19 16:57:41 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Idem, mais ici un des 2 tests est inutile.

Idem, mais ici un des 2 tests est inutile.
mpepin commented 2020-06-20 17:15:53 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

En fait j'hésite…

On peut faire en sorte que tout le code sauf ce helper fonctionne même si python-ldap n'est pas installé (avec un try-catch autour du import ldap). Ça élimine une dépendance pour quelqu'un qui voudrait utiliser authens sur un site non-hebergé à l'ENS et donc sans accès au LDAP, ou encore pour quelqu'un qui n'utilise juste pas cette feature. Vu que python-ldap a besoin de dépendances système pour tourner, c'est pas une petite dépendance.

Mais bon c'est peut-être un peu overkill, peut-être qu'on aura jamais ce use-case 🤔

En fait j'hésite… On peut faire en sorte que tout le code sauf ce helper fonctionne même si `python-ldap` n'est pas installé (avec un try-catch autour du `import ldap`). Ça élimine une dépendance pour quelqu'un qui voudrait utiliser `authens` sur un site non-hebergé à l'ENS et donc sans accès au LDAP, ou encore pour quelqu'un qui n'utilise juste pas cette feature. Vu que `python-ldap` a besoin de dépendances système pour tourner, c'est pas une petite dépendance. Mais bon c'est peut-être un peu overkill, peut-être qu'on aura jamais ce use-case :thinking:
mpepin commented 2020-06-20 17:17:48 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

euh non effectivement, je ne sais pas pourquoi j'ai fait ça, je change ça tout de suite

euh non effectivement, je ne sais pas pourquoi j'ai fait ça, je change ça tout de suite
mpepin commented 2020-06-20 17:20:22 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

changed this line in version 2 of the diff

changed this line in [version 2 of the diff](/klub-dev-ens/authens/merge_requests/13/diffs?diff_id=1787&start_sha=1cdc4a12d6059c3511e6990966b725affac6fc7b#57388df5918c1b4ef072990e4a604df18aab4024_1_1)
mpepin commented 2020-06-20 17:20:23 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

  • 744f12dc - More idiomatic default settings

Compare with previous version

added 1 commit <ul><li>744f12dc - More idiomatic default settings</li></ul> [Compare with previous version](/klub-dev-ens/authens/merge_requests/13/diffs?diff_id=1787&start_sha=1cdc4a12d6059c3511e6990966b725affac6fc7b)
mpepin commented 2020-06-20 17:20:36 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Je peux !

Je peux !
mpepin commented 2020-06-20 17:23:47 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

  • 1fc2a040 - More informative error message when LDAP messes up

Compare with previous version

added 1 commit <ul><li>1fc2a040 - More informative error message when LDAP messes up</li></ul> [Compare with previous version](/klub-dev-ens/authens/merge_requests/13/diffs?diff_id=1788&start_sha=744f12dccfccb8cf54392573f64c06e4e4b4cfbc)
mpepin commented 2020-06-20 17:24:31 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Ah oui bien vu, j'avais pas pensé à ce cas là

Ah oui bien vu, j'avais pas pensé à ce cas là
mpepin commented 2020-06-20 17:26:41 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

changed this line in version 4 of the diff

changed this line in [version 4 of the diff](/klub-dev-ens/authens/merge_requests/13/diffs?diff_id=1789&start_sha=1fc2a04040372877391dc4f26b9ef0a09586563f#7e72c4ca1df45f94d364a7dc86676affdd118b22_62_62)
mpepin commented 2020-06-20 17:26:42 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

  • 351b0844 - Handle non-existing users in register_cas_account

Compare with previous version

added 1 commit <ul><li>351b0844 - Handle non-existing users in register_cas_account</li></ul> [Compare with previous version](/klub-dev-ens/authens/merge_requests/13/diffs?diff_id=1789&start_sha=1fc2a04040372877391dc4f26b9ef0a09586563f)
mpepin commented 2020-06-20 17:27:34 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Oups, j'ai changé d'avis en cours de route et j'ai oublié de mettre à jour la docstring

Oups, j'ai changé d'avis en cours de route et j'ai oublié de mettre à jour la docstring
mpepin commented 2020-06-20 17:29:34 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

changed this line in version 5 of the diff

changed this line in [version 5 of the diff](/klub-dev-ens/authens/merge_requests/13/diffs?diff_id=1790&start_sha=351b084419d5d370e6c164c0b6189a10d5a5f543#f6bac0c117d1ea291a31c990c15a8e3661dcb55f_25_25)
mpepin commented 2020-06-20 17:29:34 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

Compare with previous version

added 1 commit <ul><li>a71688bb - Update erroneous docstring</li></ul> [Compare with previous version](/klub-dev-ens/authens/merge_requests/13/diffs?diff_id=1790&start_sha=351b084419d5d370e6c164c0b6189a10d5a5f543)
mpepin commented 2020-06-20 17:32:44 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

J'ai pas très envie de tester sur le message d'erreur. Ça ne me paraît fragile et pas très utile. Mais effectivement là les tests sont redondants, je fais mieux séparer.

J'ai pas très envie de tester sur le message d'erreur. Ça ne me paraît fragile et pas très utile. Mais effectivement là les tests sont redondants, je fais mieux séparer.
mpepin commented 2020-06-20 17:46:06 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

  • 73cf7315 - More relevant tests for register_cas_account

Compare with previous version

added 1 commit <ul><li>73cf7315 - More relevant tests for register_cas_account</li></ul> [Compare with previous version](/klub-dev-ens/authens/merge_requests/13/diffs?diff_id=1792&start_sha=a71688bbd50b306c31d84126e08d1eb264f67bc6)
lstephan commented 2020-06-22 10:10:55 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

merged

merged
lstephan commented 2020-06-22 10:10:56 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

mentioned in commit bf99e93cf1

mentioned in commit bf99e93cf14e6518e5527759ca6c251d8c702fda
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
documentation

   
Doing

   
Doing

   
Good first issue

   
To Do

   
To Do
  
bug

Something is not working

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
question

More information is needed

  
wontfix

This won't be fixed

No labels
documentation
Doing
Doing
Good first issue
To Do
To Do
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
v1
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
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: DGNum/authens#35
No description provided.
Delete branch "kerl/ldap"

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?