PLT-5813 adding SAML sync settings to System Console #207

coreyhulen · 2017-10-26T20:38:06Z

This needs mattermost/mattermost#7668 from the server first.

jasonblais · 2017-10-31T19:08:33Z

@coreyhulen Any thoughts on why I'm getting errors on the Spinmint? I've tried a few different configurations for LDAP login and/or sync, each time giving me the same error. I tried increasing the timeout to 600 seconds but that didn't help.

[2017/10/31 19:00:32 UTC] [EROR] /api/v4/ldap/test:connect code=500 rid=7yeyqardmtrkdn78nd3cyesqgc uid=o3h3uuomibromd9g1dod864qzw ip=173.35.115.69 Unable to connect to AD/LDAP server [details: LDAP Result Code 200 "": dial tcp 10.10.0.64:389: i/o timeout]
[2017/10/31 19:00:32 UTC] [EROR] /api/v4/logs:client code=0 rid=f89uhc8fmfgp3rixo6j583q3cy uid=o3h3uuomibromd9g1dod864qzw ip=173.35.115.69 {"message":"Unable to connect to AD/LDAP server","server_error_id":"ent.ldap.do_login.unable_to_connect.app_error","status_code":500,"url":"http:https://i-0cf24f3295360b8e9.spinmint.com/api/v4/ldap/test"} [details: ]

I've used config settings from both bladekick and pre-release where the LDAP sync seems to work without issues.

Also wanted to confirm that if a server upgrades to v4.4 and had LDAP enabled, LDAP sync will be enabled by default, otherwise LDAP sync is disabled?

coreyhulen · 2017-10-31T20:31:23Z

@jasonblais you cannot reach our LDAP server from a spinmint. They are on different AWS accounts. I think it would be best to test this with jumpcloud LDAP.

jasonblais · 2017-10-31T20:46:19Z

@coreyhulen Okay, I'll try that instead

jasonblais · 2017-10-31T22:19:01Z

@coreryhulen Functionally it works. Some minor text tweaks might be made, but I can do those either in this PR or separately.

Wondering if the SAML sync setting is necessary? I remember you mentioned there was a technical difficulty with having just a single setting, was that why we have both LDAP and SAML sync settings, instead of just the LDAP sync one?
Does the LDAP sync setting do anything for LDAP log in? Seems the synchronization works if LDAP login is enabled, regardless of whether LDAP sync is enabled or not.
If SAML login is enabled, but "Enable synchronizing SAML accounts with AD/LDAP" is disabled, my SAML account is deactivated after performing an LDAP sync if my account was previously on JumpCloud. Steps:

Enable SAML login
Enable LDAP login
Don't enable any LDAP sync settings
Add a user to both SAML Okta and LDAP Jumpcloud accounts
Sign in with SAML Okta to Mattermost
Delete the user from LDAP Jumpcloud
Perform LDAP sync
Observed: User deactivated

EDIT: Tested various cases with LDAP login on/off, LDAP sync on/off, SAML login on/off, SAML-LDAP sync on/off.

coreyhulen · 2017-10-31T23:07:38Z

You can technically have SAML and LDAP setup to 2 different auth systems. If you're SAML instance doesn't point to the same LDAP (or have the same users) they will all get disabled. So you must explicitly tell us to sync SAML users.
Login is independent. This is because you want to enable sync with LDAP but only allow sign-in with SAML. As a side benefit: some users want to disable LDAP sync so we would recommend them setting the sync time to some very large number, now you do not have too.
This might be a bug.

jasonblais · 2017-11-01T13:45:47Z

Thanks @coreyhulen, 1 & 2 sound good.

Combining my findings in 2 & 3, it then appears that

if LDAP login is enabled, sync is always working regardless of the LDAP sync setting on AD/LDAP page
if SAML login is enabled, LDAP sync is enabled on AD/LDAP page, but "Enable synchronizing SAML accounts with AD/LDAP" is disabled, LDAP sync still applies to the SAML accounts.

^So appears that might be a bug. Happy to test further / give more info as needed.

jasonblais · 2017-11-02T12:04:09Z

@coreyhulen Did you have a chance to investigate the potential bug yet? Let me know if I can be of any help.

coreyhulen · 2017-11-02T15:03:26Z

The bug is on the backend, nothing todo with this PR. @jasonblais can you file a jira tix and we cn merge this PR.

coreyhulen · 2017-11-02T16:22:06Z

PR for the bug fix is at mattermost/mattermost#7763

jasonblais · 2017-11-02T16:27:34Z

@coreyhulen Okay great. I just tweaked help text a little. I'll test it to make sure it's rendered properly and we can merge this PR after

jasonblais · 2017-11-02T23:06:36Z

Sorry for the delay, good to merge @coreyhulen

* PLT-5813 adding SAML sync settings to System Console * PLT-5813 adding SAML sync option * Update ldap_settings.jsx * Update saml_settings.jsx * Update en.json * Fix build failure * Update saml_settings.jsx * Update en.json

… on mobile (#207) * RN-152 Updated createGroupChannel's return value to match createDirectChannel * RN-152 Added constants and exported functions needed for creating GMs on mobile * Updated unit tests

coreyhulen added 2 commits October 26, 2017 13:18

PLT-5813 adding SAML sync settings to System Console

66c6ad4

PLT-5813 adding SAML sync option

fcf308b

coreyhulen added the 2: Dev Review Requires review by a core commiter label Oct 26, 2017

coreyhulen requested review from grundleborg, crspeller and jasonblais October 26, 2017 20:38

coreyhulen added the Work in Progress Not yet ready for review label Oct 26, 2017

jasonblais self-assigned this Oct 26, 2017

jasonblais added the 1: PM Review Requires review by a product manager label Oct 26, 2017

crspeller approved these changes Oct 30, 2017

View reviewed changes

coreyhulen removed the Work in Progress Not yet ready for review label Oct 31, 2017

coreyhulen closed this Oct 31, 2017

coreyhulen reopened this Oct 31, 2017

jasonblais added the Setup Old Test Server Triggers the creation of a test server label Oct 31, 2017

jasonblais added this to the v4.4.0 milestone Oct 31, 2017

grundleborg approved these changes Oct 31, 2017

View reviewed changes

grundleborg removed the 2: Dev Review Requires review by a core commiter label Oct 31, 2017

jasonblais added the Awaiting Submitter Action Blocked on the author label Oct 31, 2017

coreyhulen added 2: Dev Review Requires review by a core commiter and removed 1: PM Review Requires review by a product manager Awaiting Submitter Action Blocked on the author Setup Old Test Server Triggers the creation of a test server labels Nov 2, 2017

Update en.json

27cff1e

Merge branch 'master' into PLT-5813

b9785e3

jasonblais added 3 commits November 2, 2017 12:34

Fix build failure

f2ff827

Update saml_settings.jsx

93b6d72

Update en.json

8f58366

jasonblais added the Setup Old Test Server Triggers the creation of a test server label Nov 2, 2017

mattermost deleted a comment from mattermod Nov 2, 2017

jasonblais added Setup Old Test Server Triggers the creation of a test server and removed Setup Old Test Server Triggers the creation of a test server labels Nov 2, 2017

jasonblais approved these changes Nov 2, 2017

View reviewed changes

jasonblais removed the Setup Old Test Server Triggers the creation of a test server label Nov 2, 2017

mattermost deleted a comment from mattermod Nov 2, 2017

jasonblais added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Nov 2, 2017

jasonblais removed their assignment Nov 2, 2017

coreyhulen merged commit 98c7fa8 into master Nov 2, 2017

coreyhulen deleted the PLT-5813 branch November 2, 2017 23:07

jasonblais added Changelog/Done Required changelog entry has been written Docs/Needed Requires documentation labels Nov 6, 2017

lindalumitchell added the Tests/Done Release tests have been written label Nov 7, 2017

jasonblais added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Nov 15, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PLT-5813 adding SAML sync settings to System Console #207

PLT-5813 adding SAML sync settings to System Console #207

coreyhulen commented Oct 26, 2017 •

edited

Loading

jasonblais commented Oct 31, 2017 •

edited

Loading

coreyhulen commented Oct 31, 2017

jasonblais commented Oct 31, 2017

jasonblais commented Oct 31, 2017 •

edited

Loading

coreyhulen commented Oct 31, 2017 •

edited

Loading

jasonblais commented Nov 1, 2017

jasonblais commented Nov 2, 2017 •

edited

Loading

coreyhulen commented Nov 2, 2017

coreyhulen commented Nov 2, 2017

jasonblais commented Nov 2, 2017

jasonblais commented Nov 2, 2017

PLT-5813 adding SAML sync settings to System Console #207

PLT-5813 adding SAML sync settings to System Console #207

Conversation

coreyhulen commented Oct 26, 2017 • edited Loading

jasonblais commented Oct 31, 2017 • edited Loading

coreyhulen commented Oct 31, 2017

jasonblais commented Oct 31, 2017

jasonblais commented Oct 31, 2017 • edited Loading

coreyhulen commented Oct 31, 2017 • edited Loading

jasonblais commented Nov 1, 2017

jasonblais commented Nov 2, 2017 • edited Loading

coreyhulen commented Nov 2, 2017

coreyhulen commented Nov 2, 2017

jasonblais commented Nov 2, 2017

jasonblais commented Nov 2, 2017

coreyhulen commented Oct 26, 2017 •

edited

Loading

jasonblais commented Oct 31, 2017 •

edited

Loading

jasonblais commented Oct 31, 2017 •

edited

Loading

coreyhulen commented Oct 31, 2017 •

edited

Loading

jasonblais commented Nov 2, 2017 •

edited

Loading