kanboard / plugin-oauth2

Generic OAuth2 authentication plugin
MIT License
27 stars 33 forks source link

Option to allow ldap users as oauth users #12

Open clauded opened 6 years ago

clauded commented 6 years ago

It would be nice to have an option that allows the plugin to recognize existing ldap users as oauth users. Right now, I the the following error message when using oauth for an existing ldap user:

"INSERT INTO users (username, name, email, oauth2_user_id, is_ldap_user, disable_login_form) VALUES..." "SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry..."

Another nice option would be to allow a custom text for the oauth login button.

Thulium-Drake commented 5 years ago

Better yet, allow for existing users from whatever authentication source to be uniquely identified by their username (which has to be unique anyway) so you can map multiple authentication methods to that useraccount.

This will allow for usecases such as a user that is:

without having to maintain 2 accounts.

Basically what Google is doing with the 'application passwords' -> https://support.google.com/accounts/answer/185833?hl=en

orware commented 4 years ago

Since this issue already exists I'm going to add to it here since I was just dealing with this yesterday/this morning and shared my results over here in this issue: https://github.com/kanboard/plugin-oauth2/issues/18#issuecomment-701500793

I'm mainly referring to the first comment above, since I had the same issue with existing LDAP users in my database which was resulting in a very generic "External Authentication Failed" error, and while I saw what was probably the same SQL error shown above, I wasn't sure how to fix that particular issue or knew that it was related to trying to add the oauth2_user_id value into the database (and couldn't, due to the existing LDAP user record).

The second suggestion may be a good thing to look into as well, but might be separate from this issue (it seems like the OAuth2 and also the CAS Plugins both tap into the existing LDAP Authentication options a bit and borrow from it, so not sure if the other SSO plugins do the same, which might result in some limitations).

In the notes I share in my link above though, I do make note of both LDAP + OAuth2 being able to coexist to some degree...however you would continue to run into an issue in the case where somebody logs into LDAP first and then OAuth2 the way the plugin is written currently, since it wouldn't be able to insert the oauth2_user_id value successfully. @Thulium-Drake not sure if you're using LDAP too and trying to transition to OAuth2, but I'm sure there's a bit more complexity in the extra use case you're describing (e.g. not sure if local Kanboard user accounts could coexist with OAuth2 ones currently either since I haven't ran that particular type of test).

Thulium-Drake commented 4 years ago

@orware My home setup for Kanboard uses OAuth, not LDAP. But I have run into issues with LDAP combined with Reverse Proxy authentication.

All worked well, users we're able to sign in with either authentication method, but, when a user signed in via ReverseProxy auth, their other details (which are not in the request sent by the proxy) are not collected from LDAP.

So a user signing in as RevProxy did not have groups for example, where LDAP users did (but there is an issue with AD https://github.com/kanboard/kanboard/issues/4553)

Which why I would advocate to make a PAM like mechanism where if a user, which can come from any authentication source, can prove he/she owns that username, will be checked against LDAP for additional attributes and group memberships.

orware commented 4 years ago

@Thulium-Drake That definitely sounds like something that might be worthwhile to pursue (just still feeling here that particular idea probably belongs in either a different issue or maybe might even potentially belong in the main Kanboard project perhaps as an enhancement idea since I could see it potentially affecting authentication in a larger way).

I think what @clauded was originally reporting here though is for the same situation I ran into during my testing earlier this week in that an existing LDAP user wasn't able to successfully authenticate as an OAuth2 user due to there being an issue with populating the oauth2_user_id value correctly in the table with the current OAuth2 plugin (if the value was populated manually the user could login).

In the PR I created on Wednesday, I added a check that would essentially help to populate the oauth2_user_id value automatically for an existing LDAP user, which I think addresses the original issue mentioned above (so for my needs now, I'm good to go with the fix shared in that PR).