YunoHost-Apps / seafile_ynh

Seafile package for YunoHost
https://seafile.com
MIT License
14 stars 19 forks source link

Use Email header from SSOWat #46

Closed HugoPoi closed 4 years ago

HugoPoi commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

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 commented 5 years ago

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 commented 5 years ago

Ok I test some scenario.

Remediation

HugoPoi commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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

Josue-T commented 4 years ago

Hello, sorry for the delay.

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

Yes, but I don't have IRC. I've matrix or jabber.

HugoPoi commented 4 years ago

TODO

HugoPoi commented 4 years ago

This is superseded by #49