Skip to content

[FIX] Add '.value' in the SAML package to fix TypeErrors on SAML token validation#10084

Merged
sampaiodiego merged 5 commits intoRocketChat:developfrom
heysanil:develop
Apr 7, 2018
Merged

[FIX] Add '.value' in the SAML package to fix TypeErrors on SAML token validation#10084
sampaiodiego merged 5 commits intoRocketChat:developfrom
heysanil:develop

Conversation

@heysanil
Copy link
Copy Markdown
Contributor

@heysanil heysanil commented Mar 9, 2018

@RocketChat/core

Closes #10056

SAML broke with 0.62.1 and throws a TypeError while validating a token from the IdP. As observed by @chrosey in #10056, adding '.value' in two locations of the SAML package fixes the issue.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 9, 2018 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@heysanil heysanil changed the title Add '.value' in the SAML package to fix TypeErrors Add '.value' in the SAML package to fix TypeErrors on SAML token validation Mar 9, 2018
@heysanil heysanil changed the title Add '.value' in the SAML package to fix TypeErrors on SAML token validation [FIX] Add '.value' in the SAML package to fix TypeErrors on SAML token validation Mar 9, 2018
@heysanil heysanil mentioned this pull request Mar 9, 2018
Closed
@arminfelder
Copy link
Copy Markdown
Contributor

arminfelder commented Mar 19, 2018

I would suggest to test the latest version of the https://github.com/steffow/meteor-accounts-saml (I replaced the XML parsing code) and if it still fails, do a pull request there to prevent upstream issues

@pageb018
Copy link
Copy Markdown

pageb018 commented Mar 23, 2018

Any news on this fix? I have worked with OneLogin to develop a SAML app for RocketChat, but can't confirm it is working correctly with these errors.

We are waiting to deploy.

@rodrigok
Copy link
Copy Markdown
Member

rodrigok commented Apr 5, 2018

Guys, I did some changes, can you test and see if it is still working?

@pageb018
Copy link
Copy Markdown

pageb018 commented Apr 5, 2018

Happy to test, but I deploy via Docker. How long does it usually take for the updated dev image to get published on Docker Hub?

@sampaiodiego sampaiodiego added this to the 0.63.1 milestone Apr 5, 2018
@Hudell
Copy link
Copy Markdown
Contributor

Hudell commented Apr 6, 2018

I believe the lines 399 and 401 of the saml_utils.js file should also receive a .value, when accessing attributes' names.

@Hudell Hudell mentioned this pull request Apr 6, 2018
Closed
@sampaiodiego sampaiodiego merged commit ee1a3c9 into RocketChat:develop Apr 7, 2018
graywolf336 added a commit that referenced this pull request Apr 17, 2018
@graywolf336
Fix even more saml type errors (extension of #10084 and #9775)
929ff22
@Hudell Hudell mentioned this pull request Aug 9, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sampaiodiego sampaiodiego sampaiodiego approved these changes

+2 more reviewers

@gutmox gutmox gutmox approved these changes

@jahantech jahantech jahantech approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.63.1

Development

Successfully merging this pull request may close these issues.

SAML broken after recent update

9 participants

@heysanil @CLAassistant @arminfelder @pageb018 @rodrigok @Hudell @gutmox @sampaiodiego @jahantech