wekan / wekan

The Open Source kanban (built with Meteor). Keep variable/table/field names camelCase. For translations, only add Pull Request changes to wekan/i18n/en.i18n.json , other translations are done at https://app.transifex.com/wekan/wekan only.
https://wekan.github.io
MIT License
19.44k stars 2.83k forks source link

Bugs: OAuth2 can not be disabled (was: OAuth2 maps only to the account "admin") #1874

Closed bnalpay closed 3 years ago

bnalpay commented 6 years ago

Issue

Server Setup Information:

Problem description: oauth maps only to the account "admin" regardless of the user credentials entered in oauth implemantation

xet7 commented 6 years ago

@salleman33

Can you look at this?

salleman33 commented 6 years ago

Hi, strange... the only way to make an account admin is when there is 0 account : the first created is admin.

xet7 commented 6 years ago

@salleman33

On my Wekan install, there is multiple existing users, but anyone logging in with oauth logs in as me, first admin user.

bnalpay commented 6 years ago

I wonder if the mapping logic checks the verification status (tab named "verified" in people section) of the account ? and even tough there's no verified account including the default admin account all users can login with their local accounts but not with oauth

xet7 commented 6 years ago

@bnalpay

Wekan user verifying is broken #1426 , users are not verified.

salleman33 commented 6 years ago

There may be a problem with datas returned by meteor-accounts-oidc. Could you check the content of user.services.oidc in models/users.js line 491 ?

xet7 commented 6 years ago

@salleman33

It seems to try to find only those that have verified true. But most have verified false on Wekan.

xet7 commented 6 years ago

@salleman33

I have 2 users at Rocket.Chat and Wekan: xet7 and test.

1) When I remove verified=true:

$ git diff
diff --git a/models/users.js b/models/users.js
index 01673e4..5c4c041 100644
--- a/models/users.js
+++ b/models/users.js
@@ -492,7 +492,7 @@ if (Meteor.isServer) {
       const email = user.services.oidc.email.toLowerCase();

       user.username = user.services.oidc.username;
-      user.emails = [{ address: email, verified: true }];
+      user.emails = [{ address: email }];
       const initials = user.services.oidc.fullname.match(/\b[a-zA-Z]/g).join('').toUpperCase();
       user.profile = { initials, fullname: user.services.oidc.fullname };

2) Then login using test user to to Rocket.Chat https://github.com/wekan/wekan/wiki/OAuth2

3) Then login with Oidc button using Rocket.Chat auth to Wekan

4) Then Wekan shows test user Authorize window

5) But when clicking Authorize, test user still logins as xet7 user.

salleman33 commented 6 years ago

do you test this with a private window ?

xet7 commented 6 years ago

@salleman33

Yes, in private window it behaves same way.

oauth-login-wrong-user

salleman33 commented 6 years ago

@xet7 Why you remove "verified: true" ? does it change the problem ?

xet7 commented 6 years ago

@salleman33

Because currrently in Standalone Wekan Admin Panel all users seems to be not verified, because setting somebody verified does not work. I was thinking that your code would only work for user that are set as verified. But removing "verified:true" did not change anything, and did not fix anything, everyone always logins as first admin user.

salleman33 commented 6 years ago

Verified property does not effect authentification. My code sets it to true because the account come from oAuth identity provider who checks the email. Could you display the content user.services.oidc in models/users.js line 491 when you log in please ?

xet7 commented 6 years ago

@salleman33

I don't get any output. I added these console.log lines:

diff --git a/models/users.js b/models/users.js
index 01673e4..4652395 100644
--- a/models/users.js
+++ b/models/users.js
@@ -490,7 +490,9 @@ if (Meteor.isServer) {

     if (user.services.oidc) {
       const email = user.services.oidc.email.toLowerCase();
-
+      console.log("Email: " + user.services.oidc.email);
+      console.log("Username: " + user.services.oidc.username);
+      console.log("Fullname: " + user.services.oidc.fullname);
       user.username = user.services.oidc.username;
       user.emails = [{ address: email, verified: true }];
       const initials = user.services.oidc.fullname.match(/\b[a-zA-Z]/g).join('').toUpperCase();

I don't even know where you check and validate info that is coming from oidc. There seems to be some code on create user, but I probably would need to go through oidc documentation to figure it out.

HLFH commented 6 years ago

It should be interesting to be able to disable the Oidc feature & button directly in the Wekan config.

xet7 commented 6 years ago

@HLFH

Yes sure. But first the feature needs to be fixed so that it works.

salleman33 commented 6 years ago

Could you give me an account to rocket.chat to debug this problem please ? Trial is disabled on rocket.chat currently...

xet7 commented 6 years ago

@salleman33

Yes, send me email to x@xet7.org

rjevnikar commented 6 years ago

Is it possible to hide the "Sign in with Oidc" button until this feature works and there is a way of disabling the feature?

xet7 commented 6 years ago

@rjevnikar

I will add sometime in near future check to code, that if OAuth2 is not configured, Oidc button is not shown.

xet7 commented 6 years ago

@HLFH @rjevnikar

Wekan v1.49 has now been released, it has IFTTT and OAuth2 removed. Their development continues at wekan repo edge branch, until they work.

This is how to use snap stable channel, that most Wekan users have installed:

sudo snap refresh wekan --stable --amend

This is how to use snap edge channel:

sudo snap refresh wekan --edge --amend
salleman33 commented 5 years ago

Hi, the problem is that identity providers (facebook, google, rocketchat) are there own data structure which is send to oauth client. When I wrote the oauth2 authentification for wekan, i used a personnal identity provider and the package salleman:accounts-oidc works for it. But if you want connect with facebook, google, etc., you have to use an other package like accounts-google for google, accounts-facebook for facebook, and accounts-rocketchat for rocket then, you have to put a service configuration in server/authentification.js in replace for exemple with rocketchat : ServiceConfiguration.configurations.upsert( // eslint-disable-line no-undef { service: 'oidc' }, by ServiceConfiguration.configurations.upsert( // eslint-disable-line no-undef { service: 'rocketchat' },

Could you test ?

xet7 commented 5 years ago

@salleman33

What personal identity provider you used for oidc?

Is it open source?

salleman33 commented 5 years ago

I use doorkeeper (https://github.com/doorkeeper-gem/doorkeeper)

xet7 commented 5 years ago

Thanks ! I will test.

xet7 commented 5 years ago

@salleman33

Does this work well for you with doorkeeper?

Does my change oidc username to preferred_username fix or break something? https://github.com/wekan/wekan/commit/734e4e5f3ff2c3dabf94c0fbfca561db066c4565

salleman33 commented 5 years ago

yes, it works well :) No, i think your change neigher breaks or fix anything

hever commented 5 years ago

Hi, this problem still exists.

Server Setup Information: Did you test in newest Wekan?: Yes Wekan version: 1.65.0 Deployment Method(snap/docker/sandstorm/mongodb bundle/source): Snap Node Version: 8.12.0

I have LDAP enabled too.

xet7 commented 5 years ago

Moved to here from #1977

Setting OAUTH2_ENABLED=false does not work.

@hever

Yes, every open issue usually still exists. I usually close those issues that are fixed.

I have also link at OAuth2 wiki page to this bug here.

I have not yet figured out how to fix this. PRs welcome.

tiagoefreitas commented 5 years ago

I fixed this for my setup, Nextcloud as OAuth2 provider, by just changing and user field mappings. The current mappings are for doorkeeper but they can be easily changed.

These should be made into parameters in the oauth2 configuration, example from here: https://github.com/hackmdio/codimd/blob/master/docs/guides/auth/nextcloud.md OAUTH2_USER_PROFILE_USERNAME_ATTR=ocs.data.id OAUTH2_USER_PROFILE_DISPLAY_NAME_ATTR=ocs.data.display-name OAUTH2_USER_PROFILE_EMAIL_ATTR=ocs.data.email

Or even better, have a OAuth settings page like Rocket.chat has.

How to fix for Nextcloud provider:

  - OAUTH2_AUTH_ENDPOINT=/index.php/apps/oauth2/authorize
  - OAUTH2_USERINFO_ENDPOINT=/ocs/v2.php/cloud/user?format=json
  - OAUTH2_TOKEN_ENDPOINT=/index.php/apps/oauth2/api/v1/token

In package switch_oidc (or change /build/programs/server/packages/salleman_oidc.js and restart docker)

And line 34:

profile.name = userinfodata['display-name']; profile.email = userinfodata.email;

horvan commented 5 years ago

So Next loud is fixed now. But for Rocketchat this bug still exists.

xet7 commented 5 years ago

@horvan

Those NextCloud changes have not been added to Wekan yet.

horvan commented 5 years ago

For sure, but @tiagoefreitas seems to provide a fix in here for nextcloud. May be we can find a fix for rocketchat-sso too before the next stable comes out.

xet7 commented 5 years ago

@horvan

OAuth2 above uses OIDC package, that is compatible with doorkeeper. Using similar code it's possible to add also rocketchat package, that is listed here: https://guide.meteor.com/accounts.html#accounts-ui

I have previously tried adding rocketchat package, it adds login button with text RocketChat.

By looking at comments above, and searching closed pull request, anyone can see related code.

Even beginners can get features done for Wekan, as can be seen here: https://blog.wekan.team/2018/05/wekan-v1-00-released/index.html

First step is to read the code, and ask questions.

Can you look at adding this?

horvan commented 5 years ago

@xet7 I am able to identify bugs but not a programmer so I am not able to add valid fixes. sorry.

xet7 commented 5 years ago

@horvan

Ok. Thanks for info!

gil0109 commented 5 years ago

Server Setup Information:

Did you test in newest Wekan?: Yes Wekan version: 2.10.0 If this is about old version of Wekan, what upgrade problem you have?: Operating System: Linux - Openshift Deployment Method(snap/docker/sandstorm/mongodb bundle/source): OpenShift Template Http frontend if any (Caddy, Nginx, Apache, see config examples from Wekan GitHub wiki first): None Node Version: 8.15.0 MongoDB Version: db version v3.2.10 ROOT_URL environment variable http(s)://(subdomain).example.com(/suburl): Prefer not to say.

Problem description: oauth maps only to the account "admin" regardless of the user credentials entered in oauth implementation using Keycloak.

Has anyone been able to get keycloak to work with wekan? I get the keycloak login page and then redirected to the admin user. No users are being created.

If anyone could tell me the mappings I need to create in keycloak, I would appreciate it.

I see this in the logs:

Exception while invoking method 'setEmail' Error: Did not check() all arguments during call to 'setEmail'

  | at ArgumentChecker.throwUnlessAllArgumentsHaveBeenChecked (packages/check.js:483:13)   | at Object._failIfArgumentsAreNotAllChecked (packages/check.js:131:16)   | at maybeAuditArgumentChecks (packages/ddp-server/livedata_server.js:1765:18)   | at DDP._CurrentMethodInvocation.withValue (packages/ddp-server/livedata_server.js:719:19)   | at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1186:15)   | at DDPServer._CurrentWriteFence.withValue (packages/ddp-server/livedata_server.js:717:46)   | at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1186:15)   | at Promise (packages/ddp-server/livedata_server.js:715:46)   | at new Promise ()   | at Session.method (packages/ddp-server/livedata_server.js:689:23)   | at packages/ddp-server/livedata_server.js:559:43

xet7 commented 5 years ago

Some related discussion from Vanila Chat.

From richard:

I am busy with an integration of Wekan into our custom web app (vue.js based) and would like to display Wekan in our frontend. We could easily do this via iframe etc. What we need to do is make it as seemless as possible, so when a user is logged into our side (we use an openID implementation) they are automatically logged into Wekan / account created if not existing already. Am I correct in assuming we can pass through a JWT to Wekan, which can then query the token check endpoint on ourside to validate the token and receive the user details? How complex would it be to add an integration like this?

xet7 is currently doing something similar for Auth0 without JWT. Some parameters maybe need to be added to wekan/server/authentication.js and there is some cookie checking code at login page javascript and some API to create user token from email address. Not all code is there yet, but point is to get user automatically logged when user comes to Wekan login page, if user has been already logged in to other app like yours, or Nextcloud/RocketChat/Friend/Keycloak etc. Something similar exists already for Sandstorm, where Wekan is on iframe and Sandstorm has SSO that logins to all apps at once.

There are 2 ways to do this: a) Use OIDC package from @salleman33 and add Nextcloud etc to it b) Use other meteor accounts packages from atmospherejs and/or rocketchat to have RocketChat and other logins. Each login package works a little differently, so it's mainly about using correct package.

For iframe, there is Custom CSS input boxes at Wekan Admin Panel/Layout, but those don't work yet. Plan is to embed iframe menus with HTML/CSS and have wekan root-url='https://example.com/boards' so that link to card would work correctly and not require redirects. There are also whitelabeling options at Admin Panel/Layout/Hide Logo and Custom Product Name, although that logo still flashes briefly at login screen.

@gil0109

Yes user logging in as first admin Wekan user is know issue in current OIDC/OAuth2 package that works correctly only with doorkeeper OAuth2 provider. Most likely it does not use correct username parameter, or there is some other wrong setting.

To fix this, correct username parameter should be found, by testing with doorkeeper.

If that does not work, then some other auth package should be added to Wekan, and used instead of OIDC package. Currently Wekan auth works with password and LDAP logins, so maybe some of it's code could be looked as example by searching from Wekan GitHub closed pull requests, or from Wekan's current code by using script wekan/find.sh for example like this ./find.sh LDAP.

gil0109 commented 5 years ago

@xet7 Thank you for the great summary and fast response. I will ingest your suggested options and go from there. If I find a solution, I would be happy to add a pull request or add the solution to your documentation.

Thanks for the great project !!

gil0109 commented 5 years ago

@xet7 Ok, I figured out what I need to do in Keycloak to have the application use keycloak. Do you mind if I update your wiki directly?

xet7 commented 5 years ago

@gil0109

You can add Keycloak wiki page to https://github.com/wekan/wekan/wiki/Keycloak and add link to wiki menu near OAuth2. Wiki is community maintained, you can edit it directly. Thanks!

gil0109 commented 5 years ago

@xet7 I updated the page but I found an issue with the functionality when registering via OIDC. I added the the issue on the page. I was wondering if you had any idea why the issue is occuring.

I register with OIDC. The user is created with a full name, username, email, initials.

The "boardView" item is not being created in the user profile entry in the database. Any thoughts?

xet7 commented 5 years ago

@gil0109

Yes. When creating user, you need to add code to add boardView item.

xet7 commented 5 years ago

I would presume that with OIDC Keycloak working settings, similar would work also for OIDC Azure if username etc is mapped correctly, @danpatdav could check about this. The remaining would be to add to @salleman33 OIDC code or Wekan code that boardView item would be created too.

Please test etc.

gil0109 commented 5 years ago

@xet7 Your keycloak link is broken. I think you meant to use this one: Keycloak.

Let me know if you need anything else from me.

xet7 commented 5 years ago

@gil0109

I don't know yet who has time to look at adding boardView item when new user logins to Wekan using OIDC. While it would be nice for someone to try to make PR, I could also try it a little later. I don't have Keycloak or Azure setup yet, so it will take more time for me to get up to speed.

gil0109 commented 5 years ago

@xet7 This weekend, I will try to build your app and if I getting it working, if you point me to the working sample code, I can try to get it included. I haven't done coding in a while but I can give it a shot.

As for getting keycloak up and running, I have a docker instance that works.

And instructions are as follows:

Setup docker & install Keycloak:

You should be able to login in with admin/admin on http://localhost:8085/auth

xet7 commented 5 years ago

Wekan code is at https://github.com/wekan/wekan

There is rebuild-wekan.sh that 1 installs dependencies 2 builds wekan. There is also start-wekan.sh that starts Wekan with cd wekan/.build/bundle && node main.js Although in that case installing MongoDB 3.2.11 is also required. Other option is to cd wekan && WITH_API=true meteor --port 4000 so it rebuilds wekan while running, and also starts mongodb that is included with meteor. Those scripts should mostly work on Ubuntu 14.04, Debian 9, Ubuntu on Linux Subsystem for Windows, and Mac - some related Mac info is at https://github.com/wekan/wekan/wiki/Mac .

Wekan auth code is at wekan/server/authentication.js and some working LDAP login code is at https://github.com/wekan/wekan-ldap/blob/master/server/loginHandler.js

Some developer docs are at https://github.com/wekan/wekan/wiki/Developer-Documentation

There is wekan/find.sh that you can use for example to find boardView code this way:

cd wekan
./find.sh boardView
tiagoefreitas commented 5 years ago

I tried to set profile.boardView in several different places but didn't work...why isn't it done in the main create user function that is used for all the authentication methods? It should be independent on the authentication.

If you point where that function is, its just one line of code...

I don't have the time to look into the code or even install wekan development environment, so I am just updating the database manually when new users register.

db.users.updateMany({}, {$set: {'profile.boardView': 'board-view-lists'}})

xet7 commented 5 years ago

I think this is that profile error, because I got it too - maybe because of some wrong settings.

{"line":"431","file":"oauth.js","message":"Error in OAuth Server: Cannot use 'in' operator to search for 'profile' in null","time":{"$date":1549158981384},"level":"warn"}
Exception while invoking method 'login' TypeError: Cannot use 'in' operator to search for 'profile' in null
    at packages/underscore.js:894:15
    at Array.forEach (<anonymous>)
    at _.each._.forEach (packages/underscore.js:139:11)
    at Function._.pick (packages/underscore.js:893:5)
    at Object.handleOauthRequest (packages/salleman_oidc.js:48:20)
    at OAuth._requestHandlers.(anonymous function) (packages/oauth2.js:27:31)
    at middleware (packages/oauth.js:203:5)
    at packages/oauth.js:176:5