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

Closed
wants to merge 1 commit into from

Conversation

HugoPoi
Copy link

@HugoPoi HugoPoi 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
Copy link

Josue-T 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
Copy link
Author

HugoPoi 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
Copy link

Josue-T 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
Copy link
Author

HugoPoi 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
Copy link
Author

HugoPoi 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
Copy link

Josue-T 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
Copy link
Author

HugoPoi 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.

@Josue-T
Copy link

Josue-T commented Sep 18, 2019

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
Copy link
Author

HugoPoi commented Sep 22, 2019

TODO

@Josue-T Josue-T mentioned this pull request Oct 27, 2019
@HugoPoi
Copy link
Author

HugoPoi commented Nov 5, 2019

This is superseded by #49

@HugoPoi HugoPoi closed this Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple accounts same username
2 participants