wekan / ldap

LDAP support for Wekan code has been moved to https://github.com/wekan/wekan/tree/master/packages/wekan-ldap , issues to https://github.com/wekan/wekan/issues , and if PRs are needed please add them instead to https://github.com/wekan/wekan/pulls
https://github.com/wekan/wekan/tree/master/packages/wekan-ldap
MIT License
12 stars 10 forks source link

LDAP Background sync crashes with LDAP_BACKGROUND_SYNC_INTERVAL set improperly #50

Open benh57 opened 4 years ago

benh57 commented 4 years ago

Issue

With LDAP_BACKGROUND_SYNC_INTERVAL set, wekan fails to sync

Server Setup Information:

Problem description:

  1. Set up LDAP auth to work properly for your environment per the doucmentation.

Also enable background sync via:

LDAP_BACKGROUND_SYNC=true LDAP_BACKGROUND_SYNC_IMPORT_NEW_USERS=true

LDAP_BACKGROUND_SYNC_INTERVAL=500

  1. Start up wekan

---> error occurs:

Jul 26 19:09:31 wekan Wekan: [INFO] Enabling LDAP Background Sync Jul 26 19:09:31 wekan Wekan: Exception in callback of async function: TypeError: str.toLowerCase is not a function Jul 26 19:09:31 wekan Wekan: at Object.later.parse.text (/usr/local/wekan/build-0/programs/server/npm/node_modules/meteor/percolate_synced-cron/node_modules/later/later.js:1506:34) Jul 26 19:09:31 wekan Wekan: at Object.schedule (packages/wekan-ldap/server/sync.js:436:22) Jul 26 19:09:31 wekan Wekan: at scheduleEntry (packages/percolate_synced-cron.js:126:24) Jul 26 19:09:31 wekan Wekan: at packages/percolatesynced-cron.js:163:7 Jul 26 19:09:31 wekan Wekan: at Function..each._.forEach (packages/underscore.js:147:22) Jul 26 19:09:31 wekan Wekan: at packages/percolate_synced-cron.js:162:7 Jul 26 19:09:31 wekan Wekan: at Object.startup (packages/meteor.js:992:5) Jul 26 19:09:31 wekan Wekan: at Object.SyncedCron.start (packages/percolate_synced-cron.js:160:10) Jul 26 19:09:31 wekan Wekan: at addCronJobDebounced (packages/wekan-ldap/server/sync.js:4

xet7 commented 4 years ago

I did already fix this https://github.com/wekan/wekan/commit/fff144a8279ac36ce83e6b975f17f6dbc35f39d6

benh57 commented 4 years ago

It doesn't look like any changes were made. For me, it still crashes when set to "500" (double quotes)

benh57 commented 4 years ago

I'm running wekan via systemd using an .env file to set the environment variables like so. Maybe that eats the quotes?

14:52:05 root@wekan /usr/local/wekan $ cat wekan-settings.env NODE_ENV=production PWD=/usr/local/wekan/current PORT=3000 HTTP_FORWARDED_COUNT=1 MONGO_URL=-redacted- ROOT_URL=-redacted- MAIL_URL=-redacted- DEBUG=true LDAP_ENABLE=true LDAP_PORT=389 LDAP_TIMEOUT=120000 LDAP_CONNECT_TIMEOUT=120000 LDAP_IDLE_TIMEOUT=10000 LDAP_RECONNECT=true LDAP_SEARCH_PAGE_SIZE=0 LDAP_SEARCH_SIZE_LIMIT=1000 LDAP_HOST=-redacted- LDAP_ENCRYPTION=false LDAP_CA_CERT=-redacted- LDAP_BASEDN=-redacted- LDAP_LOGIN_FALLBACK=true LDAP_USER_AUTHENTICATION=false LDAP_AUTHENTIFICATION=true LDAP_AUTHENTIFICATION_USERDN=-redacted- LDAP_AUTHENTIFICATION_PASSWORD=-redacted- LDAP_LOG_ENABLED=true LDAP_INTERNAL_LOG_LEVEL=debug LDAP_BACKGROUND_SYNC=true LDAP_BACKGROUND_SYNC_IMPORT_NEW_USERS=true LDAP_USERNAME_FIELD=sAMAccountName LDAP_EMAIL_FIELD=mail LDAP_FULLNAME_FIELD=cn LDAP_UTF8_NAMES_SLUGIFY=true LDAP_USER_SEARCH_FIELD=sAMAccountName LDAP_USER_SEARCH_FILTER=(&(objectCategory=person)(objectClass=user)) LDAP_USER_SEARCH_SCOPE=sub LDAP_UNIQUE_IDENTIFIER_FIELD=sAMAccountName LDAP_GROUP_FILTER_ENABLE=false LDAP_GROUP_FILTER_GROUP_MEMBER_ATTRIBUTE=member LDAP_GROUP_FILTER_GROUP_MEMBER_FORMAT=dn LDAP_GROUP_FILTER_OBJECTCLASS=group LDAP_GROUP_FILTER_GROUP_ID_ATTRIBUTE=cn LDAP_GROUP_FILTER_GROUP_NAME=-redacted- LDAP_SYNC_ADMIN_STATUS=false LDAP_SYNC_ADMIN_GROUPS=-redacted- LDAP_SYNC_USER_DATA=true LDAP_SYNC_USER_DATA_FIELDMAP={"cn":"name", "mail":"email"} LDAP_BACKGROUND_SYNC_KEEP_EXISTANT_USERS_UPDATED=true LDAP_EMAIL_MATCH_ENABLE=false LDAP_EMAIL_MATCH_REQUIRE=false LDAP_BACKGROUND_SYNC_INTERVAL="500"

14:59:00 root@wekan /usr/lib/systemd/system $ cat wekan.service
[Unit] Description=Wekan Server Documentation=https://github.com/wekan/wekan After=network-online.target Wants=network-online.target

[Service] WorkingDirectory=/usr/local/wekan/current ExecStart=/bin/node /usr/local/wekan/current/main.js Restart=on-failure StartLimitInterval=86400 StartLimitBurst=5 RestartSec=10 ExecReload=/bin/kill -USR1 $MAINPID RestartSec=10 StandardOutput=syslog StandardError=syslog SyslogIdentifier=Wekan User=wekan Group=users EnvironmentFile=/usr/local/wekan/wekan-settings.env

[Install]

xet7 commented 4 years ago

Hmm, in my commit https://github.com/wekan/wekan/commit/fff144a8279ac36ce83e6b975f17f6dbc35f39d6 for empty values I did use single or double quotes. Maybe you have some quotes missing?

Previously when I did service file, I did it this way:

Environment=WITH_API=true SOMETHING=""

But I have not used EnvironmentFile option yet.

xet7 commented 4 years ago

The "fix" I added is only about changing LDAP_BACKGROUND_SYNC_INTERVAL to empty string, because it seems to work with that setting.

As mentioned at original pull request https://github.com/wekan/wekan/pull/2555 , getting it working is a hack. I think it does not support setting LDAP_BACKGROUND_SYNC_INTERVAL to any other value. If supporting that would be needed, then most likely it would require some new pull request to enable that.

pshunter commented 4 years ago

Hi, I was on vacation, sorry. LDAP_BACKGROUND_SYNC_INTERVAL should be set to a string like "every 10 minute". The SyncedCron page points to the later.js documentation where this is explained.

xet7 commented 4 years ago

@pshunter

Do you mean optionally? Can default be empty string?

pshunter commented 4 years ago

@xet7 No, if LDAP_BACKGROUND_SYNC is set to true, then it checks for LDAP_BACKGROUND_SYNC_INTERVAL. If the latter is unset, then the hardcoded "every 1 hour" rule will be used. If LDAP_BACKGROUND_SYNC_INTERVAL is set, it must be set to something valid. What can be put here is explained there : http://bunkat.github.io/later/parsers.html#text For example, you can use "every 1 hour" or "every 20 minute". You can get very creative, like so : "at 10:15 am also at 5:15pm except on Tuesday".

benh57 commented 4 years ago

Thanks! I'll try out that tomorrow.

benh57 commented 4 years ago

Indeed, it works with the form "every 10 minutes".

You can close this if you like, but it might be better to fail with a human readable error if the config setting cannot be parsed properly, rather than a crash stacktrace.