DGNum/django-allauth-ens
6
0
Fork 0
Code Issues 5 Pull requests Projects Releases Packages Wiki Activity Actions

LongTermClipper preserving LDAP data #14

Merged
delobell merged 1 commit from aureplop/archicubes_keep-ldap into Evarin/archicubes 2018-06-24 19:57:07 +02:00
Conversation 19 Commits 1 Files changed 6 +72 -20
delobell commented 2018-06-16 17:01:37 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

And use it to check promo (instead of usernames comparison).

And use it to check promo (instead of usernames comparison).
delobell commented 2018-06-16 17:02:38 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

  • 97ea4a8a - LongTermClipper preserving LDAP data

Compare with previous version

added 1 commit * 97ea4a8a - LongTermClipper preserving LDAP data [Compare with previous version](https://git.eleves.ens.fr/cof-geek/django-allauth-ens/merge_requests/8/diffs?diff_id=868&start_sha=08a47150db9fd84c30f0002619213b55db354a26)
delobell commented 2018-06-16 17:02:55 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

@champeno @tbastian

@champeno @tbastian
champeno commented 2018-06-16 17:41:58 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Looks good to me. Maybe update the README?

Looks good to me. Maybe update the README?
tbastian commented 2018-06-16 21:09:45 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

To me, the major question is whether we want to handle that at django-allauth level, or at a higher level. Handling it in django-allauth means that every single webapp using it at ENS will replicate archicube's data, which looks bad (no easy "please change my password", etc.).

I'd rather advocate for an archicube's CAS, somewhere (phare? archi.eleves?), handling those accounts. Thus, the only thing we would have to handle at django-allauth's level would be the transition from a cas.eleves account to those new accounts.

To me, the major question is whether we want to handle that at django-allauth level, or at a higher level. Handling it in django-allauth means that every single webapp using it at ENS will replicate archicube's data, which looks bad (no easy "please change my password", etc.). I'd rather advocate for an archicube's CAS, somewhere (phare? archi.eleves?), handling those accounts. Thus, the only thing we would have to handle at django-allauth's level would be the transition from a cas.eleves account to those new accounts.
delobell commented 2018-06-16 23:13:20 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

We can limit the data to the entrance year when connections are disabled, and fetched again on successful authentication.

What do you want to say in the "please change my password" part ? With allauth, a user can reset (or set) without being connected.
If you don't want to keep hashed passwords, you can use the admin to switch the connections to another CAS (if so, creating a provider in this package would be welcomed).

We can limit the data to the entrance year when connections are disabled, and fetched again on successful authentication. What do you want to say in the "please change my password" part ? With allauth, a user can reset (or set) without being connected. If you don't want to keep hashed passwords, you can use the admin to switch the connections to another CAS (if so, creating a provider in this package would be welcomed).
tbastian commented 2018-06-16 23:34:18 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

This is a design decision, so I won't just implement that here, it deserves some discussion :) The "please change my password" was to point out that whenever a user wants to change their password, data replication across every django-allauth-enabled webapp forces them to change their password on every single instance, instead of having something centralized. Or am I missing something?

This is a design decision, so I won't just implement that here, it deserves some discussion :) The "please change my password" was to point out that whenever a user wants to change their password, data replication across every django-allauth-enabled webapp forces them to change their password on every single instance, instead of having something centralized. Or am I missing something?
delobell commented 2018-06-17 00:39:44 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

You're totally right! As a project maintainer, you could open other providers provided by allauth, e.g. OpenID.

If you wanna create a CAS for archis, guess who is gourou 😃

You're totally right! As a project maintainer, you could open other providers provided by allauth, e.g. OpenID. If you wanna create a CAS for archis, guess who is gourou :smiley:
tbastian commented 2018-06-17 11:54:57 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

@champeno @mpepin @bclement and maybe others, I think this might be the right time to discuss how we want to handle that :)

@champeno @mpepin @bclement and maybe others, I think this might be the right time to discuss how we want to handle that :)
champeno commented 2018-06-17 12:50:54 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

I think this is a good design question but rather independent from the one we're addressing here. There can be a CAS for archicubes, and you may associate your account to this (future) third-party, but we will still need to handle the SPI CAS connexion and check whether it is still the same person or not. And the username disambiguation will be useful in any case.

Except if what you're talking about is a CAS of CAS, where the login process would be handled by a more global CAS, that itself handles the "SPI ou archicube" distinction, and outputs a single user connexion. But it doesn't sound to me like a good idea -- every project has its own way to handle archicubes (or not), not sure unifying it will really simplify things.

I think this is a good design question but rather independent from the one we're addressing here. There can be a CAS for archicubes, and you may associate your account to this (future) third-party, but we will still need to handle the SPI CAS connexion and check whether it is still the same person or not. And the username disambiguation will be useful in any case. Except if what you're talking about is a CAS of CAS, where the login process would be handled by a more global CAS, that itself handles the "SPI ou archicube" distinction, and outputs a single user connexion. But it doesn't sound to me like a good idea -- every project has its own way to handle archicubes (or not), not sure unifying it will really simplify things.
tbastian commented 2018-06-17 12:52:42 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

@champeno I was not talking about a CAS of CAS, and it indeed looks like a bad idea. I just probably misunderstood what the PR does :)

@champeno I was not talking about a CAS of CAS, and it indeed looks like a bad idea. I just probably misunderstood what the PR does :)
champeno commented 2018-06-17 13:07:10 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

This exact MR is just for handling user disambiguation a better way than mine: I was doing it using the username (champeno@18 != champeno@12), @delobell proposes to do it storing the LDAP data in SocialAccount's extra_data, which is indeed better.

My MR is globally to better use LDAP infos (with fewer queries), and invalidate SPI CAS <-> user association when the user has no more clipper account (so a new champeno will not inherit the account of the old one). It will be useful even if we create a new archicube CAS association later (though it will require specific work to handle the username policy)

This exact MR is just for handling user disambiguation a better way than mine: I was doing it using the username (`champeno@18 != champeno@12`), @delobell proposes to do it storing the LDAP data in SocialAccount's `extra_data`, which is indeed better. My MR is globally to better use LDAP infos (with fewer queries), and invalidate SPI CAS <-> user association when the user has no more clipper account (so a new `champeno` will not inherit the account of the old one). It will be useful even if we create a new archicube CAS association later (though it will require specific work to handle the username policy)
champeno commented 2018-06-22 19:21:26 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Actually, is that really what we want? Couldn't the test be on the presence of ldap in clipper_account.extra_data and in extra_data?

(This is debatable, as it will result in a systematic database query if we do so)

Actually, is that really what we want? Couldn't the test be on the presence of `ldap` in `clipper_account.extra_data` and in `extra_data`? (This is debatable, as it will result in a systematic database query if we do so)
delobell commented 2018-06-24 12:42:21 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Indeed, it's safer.

Indeed, it's safer.
delobell commented 2018-06-24 12:43:36 +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](https://git.eleves.ens.fr/cof-geek/django-allauth-ens/merge_requests/8/diffs?diff_id=872&start_sha=97ea4a8a244f4e94434afcf8dde9c415f6d40bc7#3dcb1bc63bc39a0e6ee7d4ecc6d51da34ba93ac6_52_47)
delobell commented 2018-06-24 12:43:37 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

added 1 commit

  • 534e0b18 - LongTermClipper preserving LDAP data

Compare with previous version

added 1 commit * 534e0b18 - LongTermClipper preserving LDAP data [Compare with previous version](https://git.eleves.ens.fr/cof-geek/django-allauth-ens/merge_requests/8/diffs?diff_id=872&start_sha=97ea4a8a244f4e94434afcf8dde9c415f6d40bc7)
champeno commented 2018-06-24 19:54:28 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

resolved all discussions

resolved all discussions
champeno commented 2018-06-24 19:56:58 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

Ok, looks fine, I merge to my branch.

(Btw why did you modify your commit rather than making a new one? I had trouble checking the branch out after that :/)

Ok, looks fine, I merge to my branch. (Btw why did you modify your commit rather than making a new one? I had trouble checking the branch out after that :/)
champeno commented 2018-06-24 19:57:07 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

merged

merged
champeno commented 2018-06-24 19:57:08 +02:00 (Migrated from git.eleves.ens.fr)
Copy link

mentioned in commit 6e77b31e0d

mentioned in commit 6e77b31e0dfed7659776d2b572e1c8621250f77e
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

   
Doing

   
Good first issue

   
Question

   
status -- need review

   
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
bug
Doing
Good first issue
Question
status -- need review
To Do
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
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/django-allauth-ens#14
No description provided.
Delete branch "aureplop/archicubes_keep-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?