Skip to content

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

Merged
sampaiodiego merged 5 commits into
RocketChat: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 into
RocketChat:developfrom
heysanil:develop

Conversation

@heysanil

@heysanil heysanil commented Mar 9, 2018

Copy link
Copy Markdown
Contributor

@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

CLAassistant commented Mar 9, 2018

Copy link
Copy Markdown

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
@arminfelder

Copy link
Copy Markdown
Contributor

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

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

rodrigok commented Apr 5, 2018

Copy link
Copy Markdown
Member

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

@pageb018

pageb018 commented Apr 5, 2018

Copy link
Copy Markdown

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

Hudell commented Apr 6, 2018

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SAML broken after recent update

9 participants