SAML broken after recent update #10056

NickStallman · 2018-03-08T03:37:30Z

Description:

A recent update to rocketchat-server 0.62.1 seems to have broken SAML.

The SAML process works correctly until the SAML token is returned back to Rocket Chat, where the below error occurs. The previous version (unsure exactly which one as snap handles updates) worked correctly.

Server Setup Information:

Version of Rocket.Chat Server: 0.62.1
Operating System: Ubuntu 16.04
Deployment Method(snap/docker/tar/etc): snap
Number of Running Instances: 1
DB Replicaset Oplog:
Node Version: v4.2.6
mongoDB Version:

Relevant logs:

Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: Exception in defer callback: TypeError: Cannot read property 'indexOf' of undefined
Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: at RocketChat.models.Users.getDynamicView.data.filter.record (/snap/rocketchat-server/1232/programs/server/packages/rocketchat_lib.js:9492:105)
Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: at Array.filter (:null:null)
Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: at Object.fetch (/snap/rocketchat-server/1232/programs/server/packages/rocketchat_lib.js:9492:78)
Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: at /snap/rocketchat-server/1232/programs/server/packages/rocketchat_lib.js:2837:8
Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: at /snap/rocketchat-server/1232/programs/server/packages/rocketchat_lib.js:1162:30
Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: at Array.reduce (:null:null)
Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: at Object.RocketChat.callbacks.run (/snap/rocketchat-server/1232/programs/server/packages/rocketchat_lib.js:1155:8)
Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: at Meteor.defer (/snap/rocketchat-server/1232/programs/server/packages/rocketchat_lib.js:5250:35)
Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1186:15)
Mar 07 13:03:50 demo rocketchat-server.rocketchat-server[6728]: at packages/meteor.js:502:25

megamaced · 2018-03-08T10:48:44Z

+1, was working on 0.61.0 with Microsoft ADFS.. Now it's not

jahantech · 2018-03-08T11:04:53Z

+1
Something to do with: Rocket.Chat-0.62.1/packages/meteor-accounts-saml/saml_utils.js

if (!profile.email && profile.nameID && profile.nameIDFormat && profile.nameIDFormat.indexOf('emailAddress') >= 0) {

That particular file hasn't changed I believe for last few releases , but under lying package could be causing this.

chrosey · 2018-03-08T13:50:03Z

Working with SimpleSamlPHP I could reproduce that.

I had to Change 2 Spots in steffo_meteor-accounts-saml.js to get it working:

if (!profile.email && profile.nameID && profile.nameIDFormat && profile.nameIDFormat.indexOf('emailAddress') >= 0)
to
if (!profile.email && profile.nameID && profile.nameIDFormat && profile.nameIDFormat.value.indexOf('emailAddress') >= 0)

and

const credentialToken = profile.inResponseToId || profile.InResponseTo || samlObject.credentialToken;
to
const credentialToken = profile.inResponseToId.value || profile.InResponseTo || samlObject.credentialToken;

see those '.value' I had to append to make it work.

jeanfpoulin · 2018-03-08T18:06:42Z

+1 We're seeing this as well on 0.62.1

theorenck · 2018-03-08T22:07:21Z

@rodrigok can you have a look into this one?

heysanil · 2018-03-09T05:16:49Z

+1, SAML with Auth0 is not working for me either

heysanil · 2018-03-09T19:08:15Z

Submitted #10084 implementing @chrosey's fix for this issue.

NickStallman · 2018-03-18T21:01:54Z

Is there any progress with this issue? It looks like the pull request needs a review.

I've also discovered that manually editing a snap is an absolute pain, and downgrading to the previous rocket chat version likes to spew out database schema errors making this a very inconvenient and disruptive breakage.

rasos · 2018-03-19T19:42:25Z

We can not confirm that 0.62.2 has broken SAML after an upgrade 5 days ago yet (maybe all tokens are still alive). We are using NodeJS v8.9.3, though.

inksis · 2018-03-19T20:58:46Z

@rasos it seems to persist according to my issue #10146
After add .value I've got a error 500 about "SAML Profile did not contain an email address" but I do… (cf my issue for details)

rasos · 2018-03-20T19:12:48Z

@inksis sorry, we use oauth (with keycloak), which still works. I mixed up the SSO method, sorry for the confusion.

But I was told by @arminfelder that he improved the parsing in the SAML meteor library. So better look there instead of patching here.

pageb018 · 2018-03-23T14:40:00Z

Any idea when this will be pushed and a new version released? We are waiting to deploy RocketChat, but can't without SAML working.

arminfelder · 2018-03-24T04:46:17Z

@pageb018 just created a pull request #10209, which fixes the issue in a clean way,
you could also test the fix by checking out my fork: https://github.com/arminfelder/Rocket.Chat/tree/0.62.2-samlhotfix
or download the package: https://cloud.felder-edv.at/index.php/s/xjWH8SJNNQJEi3F

pageb018 · 2018-03-26T14:39:13Z

@arminfelder I deploy via docker-compose. Not sure if I can use your hot fix that way. Is it in a docker repository?

arminfelder · 2018-03-26T15:20:16Z

@pageb018 no its not, just take the Dockerfile from .docker/Dockerfile, place it in the same folder as the tar.gz and replace the first RUN with ADD rc0.62.2-samlhotfix.tar.gz /app/

pageb018 · 2018-03-26T18:04:44Z

@arminfelder getting closer. But the docker build fails with the following error:
Step 4/10 : ADD rc0.62.2-samlhotfix.tar.gz /app/ && curl -SLf "https://releases.rocket.chat/${RC_VERSION}/download/" -o rocket.chat.tgz && curl -SLf "https://releases.rocket.chat/${RC_VERSION}/asc" -o rocket.chat.tgz.asc && gpg --verify rocket.chat.tgz.asc && mkdir -p /app && tar -zxf rocket.chat.tgz -C /app && rm rocket.chat.tgz rocket.chat.tgz.asc && cd /app/bundle/programs/server && npm install && npm cache clear --force && chown -R rocketchat:rocketchat /app
ADD failed: stat /var/lib/docker/tmp/docker-builder911997288/app: no such file or directory

I am pretty new to docker, so I may be making a obvious mistake. I have both the tar and dockerfile in the same directory.

arminfelder · 2018-03-26T19:23:19Z

@pageb018 you need to replace the whole first RUN block including the && curl -SLf..., I uploaded my Dockerfile: https://cloud.felder-edv.at/index.php/s/wsYK4P4a7g2MZEy

pageb018 · 2018-03-26T19:27:35Z

@arminfelder thank you! Let me give it a shot.

ChessSpider · 2018-03-27T12:35:45Z

👍 please fix this guys, i want to use RocketChat again

pageb018 · 2018-03-30T14:02:11Z

Will the fix be in .63? Or anytime soon?

anicoa · 2018-03-30T16:28:54Z

can not confirm that #10209 fixes SAML login. Had to apply changes of #10084 (append .value two times) which "solved" the problem.

ChessSpider · 2018-03-30T16:31:19Z

Automatic test cases are useful. Op vr 30 mrt. 2018 18:29 schreef anicoa <notifications@github.com>:

…

can not confirm that #10209 <#10209> fixes SAML login. Had to apply changes of #10084 <#10084> (append .value two times) which "solved" the problem. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#10056 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACEWNK5f0pPpwycGPotbb4FsxP3BXcjFks5tjl1agaJpZM4SiFuw> .

arminfelder · 2018-03-30T16:33:38Z

@anicoa sounds strange, as the new meteor-accounts-saml package, which is used in my fix is using a different XML parsing code, are you shure, you checked out the correct code? @pageb018 does my version work for you?

arminfelder · 2018-03-30T16:35:01Z

@ChessSpider fully agree

pageb018 · 2018-03-30T16:35:42Z

@arminfelder I had a few issues running the docker file you sent over, then I got pulled to a different project. Plan is jump back in on Monday. I will post the error I am getting, I am sure it's my lack of docker knowledge.

anicoa · 2018-03-30T17:03:39Z

@arminfelder you are right, sorry. First checked only last modified timestamp of files which showed me the changes; checking the files' content showed me that the pr wasn't applied. will try again later and report...

anicoa · 2018-03-30T20:08:08Z

@arminfelder I applied #10210 to branch develop configured saml and now I get the following error: TypeError: values is not iterable; if i change in line 430 in saml_utils.js the

for (const attributeValue of values) {
to
for (const attributeValue in values) {

it seems to work as expected.

arminfelder · 2018-03-31T01:45:59Z

@anicoa thx for checking, I replaced the for of with a regular for loop

ChessSpider · 2018-04-03T07:34:28Z

soooo..... im surprised this hasn't been fixed yet. I predict a riot.

profrowe · 2018-04-03T12:26:59Z

This is breaking for me. If anyone has a guide on implementing the fixes to a snap, I would be grateful. I'm happy to help and experienced with coding, just new to snaps. (Running 0.62.2, 1239 candidate)

arminfelder · 2018-04-03T12:48:57Z

@profrowe I haven't tried, but I guess you can
1.) clone https://github.com/arminfelder/Rocket.Chat.git
2.) checkout hotfix-develop-saml
3.) meteor build
4.) checkout https://github.com/RocketChat/rocketchat-server-snap
5.) change ln:67 to point to your Rocket.Chat.tar.gz, which was just created by meteor build

pageb018 · 2018-04-04T18:01:00Z

@arminfelder circling back to the dockerfile.. When I try and build, I get the following using your dockerfile.

root@ip-172-30-0-209:/var/www/rocket.chat# docker build -t rocketchat_saml .
Sending build context to Docker daemon  578.4MB
Step 1/10 : FROM rocketchat/base:8
 ---> 39f13643a004
Step 2/10 : ENV RC_VERSION 0.62.2
 ---> Running in b5a4b5b04ec3
Removing intermediate container b5a4b5b04ec3
 ---> 8cd343e2b32f
Step 3/10 : MAINTAINER buildmaster@rocket.chat
 ---> Running in 352514dcba77
Removing intermediate container 352514dcba77
 ---> 8b80236e2d33
Step 4/10 : ADD rc0.62.2-samlhotfix.tar.gz /app/ && curl -SLf "https://releases.rocket.chat/${RC_VERSION}/download/" -o rocket.chat.tgz && curl -SLf "https://releases.rocket.chat/${RC_VERSION}/asc" -o rocket.chat.tgz.asc && gpg --verify rocket.chat.tgz.asc && mkdir -p /app && tar -zxf rocket.chat.tgz -C /app && rm rocket.chat.tgz rocket.chat.tgz.asc && cd /app/bundle/programs/server && npm install && npm cache clear --force && chown -R rocketchat:rocketchat /app
ADD failed: stat /var/lib/docker/tmp/docker-builder665927484/app: no such file or directory

arminfelder · 2018-04-04T18:08:55Z

@pageb018 thas does not look like my Dockerfile, where does this "&& curl -SLf "https://releases.rocke..." come from?

here is mine:

FROM rocketchat/base:8

ENV RC_VERSION 0.62.2-samlFix

MAINTAINER buildmaster@rocket.chat

RUN set -x

ADD rc0.62.2-samlhotfix.tar.gz /app 

RUN cd /app/bundle/programs/server \
 && npm install \
 && npm cache clear --force \
 && chown -R rocketchat:rocketchat /app

USER rocketchat

VOLUME /app/uploads

WORKDIR /app/bundle

# needs a mongoinstance - defaults to container linking with alias 'mongo'
ENV DEPLOY_METHOD=docker \
    NODE_ENV=production \
    MONGO_URL=mongodb://mongo:27017/rocketchat \
    HOME=/tmp \
    PORT=3000 \
    ROOT_URL=http://localhost:3000 \
    Accounts_AvatarStorePath=/app/uploads

EXPOSE 3000

CMD ["node", "main.js"]

pageb018 · 2018-04-04T18:13:57Z

Weird. Ok. So using the above docker file, I get the follow:

root@ip-172-30-0-209:/var/www/rocket.chat# docker build -t rocket_saml . Sending build context to Docker daemon 590.5MB Step 1/12 : FROM rocketchat/base:8 ---> 39f13643a004 Step 2/12 : ENV RC_VERSION 0.62.2-samlFix ---> Using cache ---> a9a73e154dc2 Step 3/12 : MAINTAINER buildmaster@rocket.chat ---> Using cache ---> 2a3484e5a16a Step 4/12 : RUN set -x ---> Using cache ---> 8dc8b294e4ed Step 5/12 : ADD rc0.62.2-samlhotfix.tar.gz /app ---> Using cache ---> 9117ee56f813 Step 6/12 : RUN cd /app/bundle/programs/server && npm install && npm cache clear --force && chown -R rocketchat:rocketchat /app ---> Running in 06b949b52e59 /bin/sh: 1: cd: can't cd to /app/bundle/programs/server The command '/bin/sh -c cd /app/bundle/programs/server && npm install && npm cache clear --force && chown -R rocketchat:rocketchat /app' returned a non-zero code: 2

arminfelder · 2018-04-04T18:21:22Z

@pageb018 I guess its a problem with the cache, try "docker build --no-cache rocket_saml ." and obviously, the Dockerfile and the rc0.62.2-samlhotfix.tar.gz need to be in the same folder

pageb018 · 2018-04-04T18:37:03Z

@arminfelder unfortunately same issue with the no cache flag. Hmm...

tezukzai · 2018-04-04T18:51:12Z

+1 for this to be prioritised for the next release.

arminfelder · 2018-04-04T18:56:01Z

@pageb018 it just uploaded my build to dockerhub: docker pull afelder/rocketchat:0.62.2-samlfix

pageb018 · 2018-04-04T20:35:01Z

@arminfelder thank you! I am up and running again with SAML back and working perfectly. Appreciate all the help.

graywolf336 · 2018-04-07T06:57:12Z

Let us know if the v0.63.1 release doesn't fix the issue.

MartinSchoeler added type: bug subj: auth - saml labels Mar 8, 2018

heysanil mentioned this issue Mar 9, 2018

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

Merged

jeanfpoulin mentioned this issue Mar 19, 2018

SAML No matching login attempt found after update 0.62.2 #10146

Closed

arminfelder mentioned this issue Mar 24, 2018

[HOTFIX]0.62.2 samlhotfix #10206

Closed

This was referenced Mar 24, 2018

[HOTFIX][RC] SAML repair #10211

Closed

[FIX] SAML repair #10210

Closed

[HOTFIX][MASTER] SAML repair #10209

Closed

[HOTFIX][RC] SAML repair #10212

Closed

engelgabriel added this to the 0.64.0 milestone Apr 6, 2018

sampaiodiego closed this as completed in #10084 Apr 7, 2018

SAML broken after recent update #10056

SAML broken after recent update #10056

Comments

NickStallman commented Mar 8, 2018

Description:

Server Setup Information:

Relevant logs:

megamaced commented Mar 8, 2018

jahantech commented Mar 8, 2018 • edited Loading

chrosey commented Mar 8, 2018

jeanfpoulin commented Mar 8, 2018

theorenck commented Mar 8, 2018

heysanil commented Mar 9, 2018

heysanil commented Mar 9, 2018

NickStallman commented Mar 18, 2018

rasos commented Mar 19, 2018

inksis commented Mar 19, 2018

rasos commented Mar 20, 2018

pageb018 commented Mar 23, 2018

arminfelder commented Mar 24, 2018 • edited Loading

pageb018 commented Mar 26, 2018

arminfelder commented Mar 26, 2018 • edited Loading

pageb018 commented Mar 26, 2018

arminfelder commented Mar 26, 2018

pageb018 commented Mar 26, 2018

ChessSpider commented Mar 27, 2018

pageb018 commented Mar 30, 2018

anicoa commented Mar 30, 2018

ChessSpider commented Mar 30, 2018 via email

arminfelder commented Mar 30, 2018 • edited Loading

arminfelder commented Mar 30, 2018

pageb018 commented Mar 30, 2018

anicoa commented Mar 30, 2018

anicoa commented Mar 30, 2018

arminfelder commented Mar 31, 2018 • edited Loading

ChessSpider commented Apr 3, 2018

profrowe commented Apr 3, 2018 • edited Loading

arminfelder commented Apr 3, 2018

pageb018 commented Apr 4, 2018

arminfelder commented Apr 4, 2018

pageb018 commented Apr 4, 2018

arminfelder commented Apr 4, 2018 • edited Loading

pageb018 commented Apr 4, 2018

tezukzai commented Apr 4, 2018

arminfelder commented Apr 4, 2018

pageb018 commented Apr 4, 2018

graywolf336 commented Apr 7, 2018

jahantech commented Mar 8, 2018 •

edited

Loading

arminfelder commented Mar 24, 2018 •

edited

Loading

arminfelder commented Mar 26, 2018 •

edited

Loading

arminfelder commented Mar 30, 2018 •

edited

Loading

arminfelder commented Mar 31, 2018 •

edited

Loading

profrowe commented Apr 3, 2018 •

edited

Loading

arminfelder commented Apr 4, 2018 •

edited

Loading