Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Email header from SSOWat #46

Open
wants to merge 1 commit into
base: testing
from

Conversation

@HugoPoi
Copy link

commented Aug 29, 2019

Fix #44

I think we may just want rewrite the header variable now in the patch ?

Tested it work like charm.

We need to add a note to WARN user because it will "break" SSO for wrongly created account with username@seafiledomain if the user haven't the email in the seafile domain.

@Josue-T

This comment has been minimized.

Copy link

commented Aug 30, 2019

Hello,

Thanks for your work. Ok it fix #44 but not #5 I think.

We need to add a note to WARN user because it will "break" SSO for wrongly created account with username@seafiledomain if the user haven't the email in the seafile domain.

Maybe we need to keep the old solution for the actual install because it might be a big issue for some users.

@HugoPoi

This comment has been minimized.

Copy link
Author

commented Aug 30, 2019

Yes I confirm not fixing #5 because we need a filter in LDAP to identified alias vs main mail address.

  • check if already created account with the old solution can be access without SSO

If it's the case, my fix will just break the SSO, so the admin can manage to reassign account if there is way in Seafile ?

@Josue-T

This comment has been minimized.

Copy link

commented Aug 31, 2019

Yes I confirm not fixing #5 because we need a filter in LDAP to identified alias vs main mail address.

Can you clarify your idea about that ? On yunohost all email and alias are the same attribute in the LDAP database. On the Yunohost side the main email is just the first email entry...

If it's the case, my fix will just break the SSO, so the admin can manage to reassign account if there is way in Seafile ?

As I know it's not possible on the Yunohost side.

I thought also about the change_url script which might be broken because actually we change the domain of all user email when we change the domain of seafile.

@HugoPoi

This comment has been minimized.

Copy link
Author

commented Aug 31, 2019

Ok I test some scenario.

  • The new config doesn't impact account with same domain as Seafile installation. if you have user@mydomain.net and Seafile URL is https://mydomain.net/seafile, your are all good.

  • If the user doesn't have the corresponding alias he will can't connect to the corresponding account neither by SSO nor directly through Seafile URL. ( Seafile admin can check here https://seafiledomain.example/seafile/sys/useradmin/ then check in yunohost users )

  • The new SSO config will used the main email address to auth through SSO over Seafile and if the account doesn't exists it will be created (empty obviously). The admin should transfer Library at this point or add alias and explain the change to the users.

Remediation

  • Yunohost admin can add the alias to all accounts to keep allow login on old created accounts.
  • Seafile admin can transfert property of Library from the old account to the new. ( I think the best solution )
@HugoPoi

This comment has been minimized.

Copy link
Author

commented Aug 31, 2019

Can you clarify your idea about that ? On yunohost all email and alias are the same attribute in the LDAP database. On the Yunohost side the main email is just the first email entry...

there is a FILTER option in Seafile for LDAP, I doesn't know if we can just select the first entry of mail attribute with that, I'm noob about LDAP stuff.

Edit:
After digging LDAP attribute are given unordered so we can't do it without a proper tagging on the attribute. https://stackoverflow.com/questions/36871734/how-to-retrieve-value-of-first-matching-attribute-in-ldapsearch

@Josue-T

This comment has been minimized.

Copy link

commented Aug 31, 2019

The admin should transfer Library at this point or add alias and explain the change to the users.

This is what I don't really like because I don't how many user this impact...

By this you need to move each library and reconfigure all client.

there is a FILTER option in Seafile for LDAP, I doesn't know if we can just select the first entry of mail attribute with that, I'm noob about LDAP stuff.

Yes there are a Filter option but, you just can say mail=user@domain.tld or maybe (need to be tested) mail=*@domain.tld. For more specific filter I really don't know how to do this and if it's possible.

@HugoPoi

This comment has been minimized.

Copy link
Author

commented Sep 1, 2019

This is what I don't really like because I don't how many user this impact...

I think only a few users are in this case because the users impacted already facing this issues

  1. SSO Seafile account isn't the same as if they use the main email address in Seafile clients
  2. Users needs to use a weird email like username@seafiledomain for login with Seafile clients

The yunohost setup with only one domain are NOT impacted.

@HugoPoi

This comment has been minimized.

Copy link
Author

commented Sep 14, 2019

So what do we do about this PR ? we can discuss on IRC maybe ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.