[IMPROVE] SAML login process refactoring by kukkjanos · Pull Request #12891 · RocketChat/Rocket.Chat

kukkjanos · 2018-12-08T19:47:32Z

the eppn is primary identifier match parameter (if not exist then email is a secondary)
update eppn to user profile
support the eduPersonPrincipalName (eppn) usually required saml parameter
support the displayName optional saml parameter
support two new saml settings
- overwrite user fullname if need (use idp attribute [cn,username or displayName])
- overwrite user mail address if need (use idp attribute)

FIX NEW DIRECTORY

Closes #ISSUE_NUMBER

- the eppn is primary identifier match parameter (if not exist then email is a secondary) - update eppn to user profile - support the eduPersonPrincipalName (eppn) usually required saml parameter - support the displayName optional saml parameter - support two new saml settings - overwrite user fullname if need (use idp attribute [cn,username or displayName]) - overwrite user mail address if need (use idp attribute)

CLAassistant · 2018-12-08T19:47:39Z

All committers have signed the CLA.

misi · 2018-12-17T07:23:44Z

+1

rukverc · 2018-12-17T07:25:55Z

This is a very useful feature, strongly recommended.

EmiTrif · 2018-12-17T07:28:10Z

+1

northway · 2018-12-17T07:28:59Z

+1

rukverc

Contains necessary changes, seems legit code.

app/meteor-accounts-saml/server/saml_server.js

Toushe

Good job!

rAcHekLoS · 2019-05-29T14:45:34Z

a) This change destroy SAML with Microsoft ADFS. The login works, but the user maps always to the User Rocket.Cat.
b) generally a problem is that you cannot configure the authnContext. To enable Kerberos or NTLM login, the app.js code must be modified.

May 29 16:43:40 x06-rocketchat rocketchat[17255]: { actionName: 'authorize',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: serviceName: 'adfs',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: credentialToken: 'id-x93vGuoAPjFWxtEtr' }
May 29 16:43:40 x06-rocketchat rocketchat[17255]: [ { provider: 'adfs',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: entryPoint: 'https://adfs.edag.de/adfs/ls/',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: idpSLORedirectURL: 'https://adfs.edag.de/adfs/ls/',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: issuer: 'https://rocketchat.edag.de',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: cert: '',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: privateCert: false,
May 29 16:43:40 x06-rocketchat rocketchat[17255]: privateKey: false,
May 29 16:43:40 x06-rocketchat rocketchat[17255]: callbackUrl: 'https://rocketchat.edag.de/_saml/validate/adfs',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: id: 'wvWk7Zjf1YnlDjEdn',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: protocol: 'https://',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: path: '/saml/consume',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: identifierFormat: 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: authnContext: 'urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport' } ]
May 29 16:43:40 x06-rocketchat rocketchat[17255]: adfs
May 29 16:43:40 x06-rocketchat rocketchat[17255]: { actionName: 'validate',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: serviceName: 'adfs',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: credentialToken: undefined }
May 29 16:43:40 x06-rocketchat rocketchat[17255]: [ { provider: 'adfs',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: entryPoint: 'https://adfs.edag.de/adfs/ls/',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: idpSLORedirectURL: 'https://adfs.edag.de/adfs/ls/',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: issuer: 'https://rocketchat.edag.de',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: cert: '',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: privateCert: false,
May 29 16:43:40 x06-rocketchat rocketchat[17255]: privateKey: false,
May 29 16:43:40 x06-rocketchat rocketchat[17255]: callbackUrl: 'https://rocketchat.edag.de/_saml/validate/adfs',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: id: 'id-x93vGuoAPjFWxtEtr',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: protocol: 'https://',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: path: '/saml/consume',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: identifierFormat: 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress',
May 29 16:43:40 x06-rocketchat rocketchat[17255]: authnContext: 'urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport' } ]
May 29 16:43:40 x06-rocketchat rocketchat[17255]: adfs
May 29 16:43:40 x06-rocketchat rocketchat[17255]: RESULT :{"profile":{"inResponseToId":"id-x93vGuoAPjFWxtEtr","issuer":"http://adfs.edag.de/adfs/services/trust","nameID":"[email protected]","nameIDFormat":"urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress","sessionIndex":"_e4c81ddb-ddc0-49e4-bd51-36b501895c66","email":"[email protected]","http://schemas-xmlsoap-org/ws/2005/05/identity/claims/name":"Test, Test","http://schemas-xmlsoap-org/ws/2005/05/identity/claims/emailaddress":"[email protected]"}}

Hudell · 2019-05-29T15:15:17Z

Does it need a different authnContext value to work? What value would that be?

rAcHekLoS · 2019-05-29T15:49:03Z

request += '<samlp:RequestedAuthnContext xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" Comparison="exact">' + '<saml:AuthnContextClassRef xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">urn:federation:authentication:windows</saml:AuthnContextClassRef></samlp:RequestedAuthnContext>\n' + '</samlp:AuthnRequest>';

isometry · 2019-05-29T22:34:38Z

a) This change destroy SAML with Microsoft ADFS. The login works, but the user maps always to the User Rocket.Cat.

This change also breaks SAML with OneLogin IdP; same symptoms as reported here, but OneLogin does support the hard-coded authnContext of urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport

Hudell · 2019-05-29T22:47:22Z

You mean login working but with the wrong user? That was fixed by #14686 and included on the 1.1.1 hotfix release.

isometry · 2019-05-30T18:14:46Z

You mean login working but with the wrong user? That was fixed by #14686 and included on the 1.1.1 hotfix release.

Confirmed fixed. Thanks!

rAcHekLoS · 2019-05-31T14:40:48Z

The problem #14686 is fixed, THX.

For AuthContext, I would suggest that you make an array to specify multiple AuthContext. This way you can support multiple variants.

kukkjanos and others added 16 commits September 13, 2018 13:31

Lint fix ;)

a401c14

Merge branch 'develop' into develop

3aa9566

Merge branch 'develop' into develop

cab2ac9

Merge branch 'develop' into develop

34bd975

Merge branch 'develop' into develop

235524b

Merge branch 'develop' into develop

a203d74

Merge branch 'develop' into develop

9e8d54d

Merge branch 'develop' into develop

7aeb514

Merge branch 'develop' into develop

376a561

Merge branch 'develop' into develop

ba5252d

Merge branch 'develop' into develop

55a5f9e

Merge branch 'develop' into develop

26fa0b8

Merge branch 'develop' into develop

589f1d0

Merge branch 'develop' into develop

7a4cba7

kukkjanos added 8 commits December 10, 2018 16:56

Merge branch 'develop' into develop

937e94d

Merge branch 'develop' into develop

d35a04b

Merge branch 'develop' into develop

1036d85

Merge branch 'develop' into develop

b03a041

Merge branch 'develop' into develop

e876daa

Merge branch 'develop' into develop

969688f

Merge branch 'develop' into develop

b5c97ab

Merge branch 'develop' into develop

b3a6a12

rukverc previously approved these changes Dec 17, 2018

View reviewed changes

Hudell approved these changes Apr 29, 2019

View reviewed changes

Hudell previously approved these changes Apr 29, 2019

View reviewed changes

app/meteor-accounts-saml/server/saml_server.js Outdated Show resolved Hide resolved

app/meteor-accounts-saml/server/saml_server.js Outdated Show resolved Hide resolved

app/meteor-accounts-saml/server/saml_server.js Outdated Show resolved Hide resolved

Remove console log

ee6afa0

kukkjanos dismissed stale reviews from Hudell and Toushe via ee6afa0 April 29, 2019 13:11

Toushe approved these changes Apr 29, 2019

View reviewed changes

Hudell added this to the 1.1.0 milestone Apr 29, 2019

Hudell approved these changes Apr 29, 2019

View reviewed changes

kukkjanos added 10 commits April 29, 2019 18:52

Merge branch 'develop' into develop

abbb69c

Merge branch 'develop' into develop

7baa8f2

Merge branch 'develop' into develop

3b5f231

Merge branch 'develop' into develop

321c34f

Merge branch 'develop' into develop

c5d065b

Merge branch 'develop' into develop

ed3c271

Merge branch 'develop' into develop

98b4ded

Merge branch 'develop' into develop

2a52710

Merge branch 'develop' into develop

6a2ed1b

Merge branch 'develop' into develop

0564c0f

sampaiodiego changed the title ~~[NEW] SAML login process refactoring~~ [IMPROVE] SAML login process refactoring May 7, 2019

sampaiodiego merged commit 34e58da into RocketChat:develop May 7, 2019

sampaiodiego mentioned this pull request May 28, 2019

Release 1.1.0 #14653

Merged

Hudell mentioned this pull request May 29, 2019

[NEW] Setting to configure custom authn context on SAML requests #14675

Merged

pkolmann mentioned this pull request Jun 26, 2019

Bug introduced with Improvement #12891 #14876

Closed

Conversation

kukkjanos commented Dec 8, 2018

Uh oh!

CLAassistant commented Dec 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

misi commented Dec 17, 2018

Uh oh!

rukverc commented Dec 17, 2018

Uh oh!

EmiTrif commented Dec 17, 2018

Uh oh!

northway commented Dec 17, 2018

Uh oh!

rukverc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Toushe left a comment

Choose a reason for hiding this comment

Uh oh!

rAcHekLoS commented May 29, 2019

Uh oh!

Hudell commented May 29, 2019

Uh oh!

rAcHekLoS commented May 29, 2019

Uh oh!

isometry commented May 29, 2019

Uh oh!

Hudell commented May 29, 2019

Uh oh!

isometry commented May 30, 2019

Uh oh!

rAcHekLoS commented May 31, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

CLAassistant commented Dec 8, 2018 •

edited

Loading