Merge branch 'upstream' into upstream_v1.3.5_merge

* upstream:
  docs: remove badges broken by project rename.
  bump version to 1.3.5
  deps: really bump xml-encryption for node-forge sub-dep upgrade to address vuln.
  docs: Update package.json / README to reflect site move.
  deps: bump xml-encryption to address node-forge sub-dep vuln.
  Update issue templates
  Update issue templates
  Bump lodash from 4.17.15 to 4.17.20 (node-saml#449)
  Bump acorn from 7.1.0 to 7.4.0 (node-saml#448)
  Return object for XML-valued AttributeValues (node-saml#447)
  Revert "doc: announce site move." (node-saml#446)
  doc: announce site move.
  add yarn-error.log to .gitignore
  bump version.
  Fix multi saml strategy race conditions (node-saml#426)
  v1.3.3
  v1.3.2

# Conflicts:
#	.gitignore
#	README.md
#	package.json

Author: walokra

.github/ISSUE_TEMPLATE/bug-report.md

@@ -0,0 +1,43 @@
+---
+name: Bug report
+about: Create a report to help us improve
+title: "[BUG]"
+labels: bug
+assignees: ''
+
+---
+
+<!--
+Thanks for submitting a bug report or featureq request to help us improve.
+
+If you have a support question about how to use the module, no one is monitoring the issues
+to answer those. Consider posting on StackOverflow instead using the "passport-saml" tag.
+-->
+
+** Spec-driven development **
+
+This project is focused on compliance with the SAML 2.0 specification. For any bug report that
+involves the SAML spec, please link to the related parts of the spec and quote the passages too.
+
+Start here: http://saml.xml.org/saml-specifications
+
+You might also check the spec to confirm that it doesn't address your particular bug and mention
+that you found no references in the spec concerning your issue.
+
+** Community development model **
+
+passport-saml is maintained by a number of current users. There is no author or primary maintainer
+waiting to write your tests and documentation for you. To increase the odds that your issue
+is promptly dealt with, consider a pull request to address the issue that includes test coverage
+and updated documentation.
+
+**To Reproduce**
+
+Steps to reproduce the behavior. Ideally, expressesd through an automated test.
+
+**Expected behavior**
+A clear and concise description of what you expected to happen.
+
+**Environment**
+ - Node.js version:
+ - passport-saml version:

.github/ISSUE_TEMPLATE/feature_request.md

@@ -0,0 +1,41 @@
+---
+name: Feature request
+about: Suggest an idea for this project
+title: "[ENHANCE]"
+labels: enhancement
+assignees: ''
+
+---
+
+<!--
+Thanks for submitting a bug report or featureq request to help us improve.
+
+If you have a support question about how to use the module, no one is monitoring the issues
+to answer those. Consider posting on StackOverflow instead using the "passport-saml" tag.
+-->
+
+** Spec-driven development **
+
+This project is focused on compliance with the SAML 2.0 specification. For any bug report that
+involves the SAML spec, please link to the related parts of the spec and quote the passages too.
+
+Start here: http://saml.xml.org/saml-specifications
+
+You might also check the spec to confirm that it doesn't address your particular bug and mention
+that you found no references in the spec concerning your issue.
+
+** Community development model **
+
+passport-saml is maintained by a number of current users. There is no author or primary maintainer
+waiting to write your tests and documentation for you. To increase the odds that your issue
+is promptly dealt with, consider a pull request to address the issue that includes test coverage
+and updated documentation.
+
+**Is your feature request related to a problem? Please describe.**
+A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
+
+**Describe the solution you'd like**
+A clear and concise description of what you want to happen.
+
+**Describe alternatives you've considered**
+A clear and concise description of any alternative solutions or features you've considered.

.gitignore

@@ -1,2 +1,4 @@
 node_modules/
 .tern-port
+.idea
+yarn-error.log

README.md

@@ -85,7 +85,10 @@ The options passed when the `MultiSamlStrategy` is initialized are also passed a
 e.g. If you provide an `issuer` on `MultiSamlStrategy`, this will be also a default value for every provider.
 You can override these defaults by passing a new value through the `getSamlOptions` function.
 
-Using multiple providers supports `validateInResponseTo`, but all the `InResponse` values are stored on the same Cache. This means, if you're using the default `InMemoryCache`, that all providers have access to it and a provider might get its response validated against another's request. [Issue Report](!https://github.com/bergie/passport-saml/issues/334). To amend this you should provide a different cache provider per SAML provider, through the `getSamlOptions` function.
+Using multiple providers supports `validateInResponseTo`, but all the `InResponse` values are stored on the same Cache. This means, if you're using the default `InMemoryCache`, that all providers have access to it and a provider might get its response validated against another's request. [Issue Report](!https://github.com/node-saml/passport-saml/issues/334). To amend this you should provide a different cache provider per SAML provider, through the `getSamlOptions` function.
+
+> :warning: **There's a race condition [bug](https://github.com/node-saml/passport-saml/issues/425) in versions < 1.3.3 which makes it vulnerable to DOS attacks**: Please use > 1.3.3 if you want to use this issue
+
 
 #### The profile object:
 
@@ -132,7 +135,7 @@ type Profile = {
   * `identifierFormat`: if truthy, name identifier format to request from identity provider (default: `urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress`)
   * `acceptedClockSkewMs`: Time in milliseconds of skew that is acceptable between client and server when checking `OnBefore` and `NotOnOrAfter` assertion condition validity timestamps.  Setting to `-1` will disable checking these conditions entirely.  Default is `0`.
   * `attributeConsumingServiceIndex`: optional `AttributeConsumingServiceIndex` attribute to add to AuthnRequest to instruct the IDP which attribute set to attach to the response ([link](http://blog.aniljohn.com/2014/01/data-minimization-front-channel-saml-attribute-requests.html))
-  * `disableRequestedAuthnContext`: if truthy, do not request a specific authentication context. This is [known to help when authenticating against Active Directory](https://github.com/bergie/passport-saml/issues/226) (AD FS) servers.
+  * `disableRequestedAuthnContext`: if truthy, do not request a specific authentication context. This is [known to help when authenticating against Active Directory](https://github.com/node-saml/passport-saml/issues/226) (AD FS) servers.
   * `authnContext`: if truthy, name identifier format to request auth context (default: `urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport`); array of values is also supported
   * `RACComparison`: Requested Authentication Context comparison type. Possible values are 'exact','minimum','maximum','better'. Default is 'exact'.
 

lib/passport-saml/saml.js

@@ -1243,7 +1243,7 @@ SAML.prototype.processValidlySignedAssertion = function(xml, samlResponseXml, in
                                );
 
       var attrValueMapper = function(value) {
-        return typeof value === 'string' ? value : value._;
+        return value._ ? value._ : value;
       };
 
       if (attributes) {

multiSamlStrategy.js

@@ -31,8 +31,10 @@ MultiSamlStrategy.prototype.authenticate = function (req, options) {
       return self.error(err);
     }
 
-    self._saml = new saml.SAML(Object.assign({}, self._options, samlOptions));
-    self.constructor.super_.prototype.authenticate.call(self, req, options);
+    var samlService = new saml.SAML(Object.assign({}, self._options, samlOptions));
+    var strategy = Object.assign({}, self, {_saml: samlService});
+    Object.setPrototypeOf(strategy, self);
+    self.constructor.super_.prototype.authenticate.call(strategy, req, options);
   });
 };
 
@@ -44,8 +46,10 @@ MultiSamlStrategy.prototype.logout = function (req, callback) {
       return callback(err);
     }
 
-    self._saml = new saml.SAML(Object.assign({}, self._options, samlOptions));
-    self.constructor.super_.prototype.logout.call(self, req, callback);
+    var samlService = new saml.SAML(Object.assign({}, self._options, samlOptions));
+    var strategy = Object.assign({}, self, {_saml: samlService});
+    Object.setPrototypeOf(strategy, self);
+    self.constructor.super_.prototype.logout.call(strategy, req, callback);
   });
 };
 
@@ -61,8 +65,10 @@ MultiSamlStrategy.prototype.generateServiceProviderMetadata = function( req, dec
       return callback(err);
     }
 
-    self._saml = new saml.SAML(Object.assign({}, self._options, samlOptions));
-    return callback(null, self.constructor.super_.prototype.generateServiceProviderMetadata.call(self, decryptionCert, signingCert ));
+    var samlService = new saml.SAML(Object.assign({}, self._options, samlOptions));
+    var strategy = Object.assign({}, self, {_saml: samlService});
+    Object.setPrototypeOf(strategy, self);
+    return callback(null, self.constructor.super_.prototype.generateServiceProviderMetadata.call(strategy, decryptionCert, signingCert));
   });
 };
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "suomifi-passport-saml",
-  "version": "1.3.3-sfi.0",
+  "version": "1.3.5-sfi.0",
   "license": "MIT",
   "keywords": [
     "saml",
@@ -31,7 +31,7 @@
     "passport-strategy": "*",
     "q": "^1.5.0",
     "xml-crypto": "^1.4.0",
-    "xml-encryption": "^1.0.0",
+    "xml-encryption": "1.2.1",
     "xml2js": "0.4.x",
     "xmlbuilder": "^11.0.0",
     "xmldom": "0.1.x"

test/multiSamlStrategy.js

@@ -68,6 +68,7 @@ describe('strategy#authenticate', function() {
   });
 
   it('uses given options to setup internal saml provider', function(done) {
+    var superAuthenticateStub = this.superAuthenticateStub;
     var samlOptions = {
       issuer: 'http://foo.issuer',
       callbackUrl: 'http://foo.callback',
@@ -84,7 +85,9 @@ describe('strategy#authenticate', function() {
     function getSamlOptions (req, fn) {
       try {
         fn(null, samlOptions);
-        strategy._saml.options.should.containEql(Object.assign({},
+        sinon.assert.calledOnce(superAuthenticateStub)
+        superAuthenticateStub.calledWith(Object.assign(
+          {},
           { cacheProvider: 'mock cache provider' },
           samlOptions
         ));
@@ -104,19 +107,19 @@ describe('strategy#authenticate', function() {
 
 describe('strategy#logout', function() {
   beforeEach(function() {
-    this.superAuthenticateStub = sinon.stub(SamlStrategy.prototype, 'logout');
+    this.superLogoutMock = sinon.stub(SamlStrategy.prototype, 'logout');
   });
 
   afterEach(function() {
-    this.superAuthenticateStub.restore();
+    this.superLogoutMock.restore();
   });
 
   it('calls super with request and auth options', function(done) {
-    var superAuthenticateStub = this.superAuthenticateStub;
+    var superLogoutMock = this.superLogoutMock;
     function getSamlOptions (req, fn) {
       try {
         fn();
-        sinon.assert.calledOnce(superAuthenticateStub);
+        sinon.assert.calledOnce(superLogoutMock);
         done();
       } catch (err2) {
         done(err2);
@@ -148,6 +151,7 @@ describe('strategy#logout', function() {
   });
 
   it('uses given options to setup internal saml provider', function(done) {
+    var superLogoutMock = this.superLogoutMock;
     var samlOptions = {
       issuer: 'http://foo.issuer',
       callbackUrl: 'http://foo.callback',
@@ -164,7 +168,11 @@ describe('strategy#logout', function() {
     function getSamlOptions (req, fn) {
       try {
         fn(null, samlOptions);
-        strategy._saml.options.should.containEql(samlOptions);
+        sinon.assert.calledOnce(superLogoutMock)
+        superLogoutMock.calledWith(Object.assign(
+          {},
+          samlOptions
+        ));
         done();
       } catch (err2) {
         done(err2);

test/tests.js

@@ -1380,6 +1380,40 @@ describe( 'passport-saml /', function() {
             }
           });
         });
+
+        it( 'XML AttributeValue should return object', function( done ) {
+          const nameid_opaque_string = '*******************************'
+          const nameQualifier = 'https://idp.example.org/idp/saml'
+          const spNameQualifier = 'https://sp.example.org/sp/entity'
+          const format = 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'
+          const xml = 
+          '<Response>' +
+            '<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" Version="2.0">' +
+              '<saml2:AttributeStatement>' +
+                  '<saml2:Attribute FriendlyName="eduPersonTargetedID" Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.10" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">' +
+                      '<saml2:AttributeValue>' +
+                          '<saml2:NameID Format="'+format+'" NameQualifier="'+nameQualifier+'" SPNameQualifier="'+spNameQualifier+'">' +
+                          nameid_opaque_string +
+                          '</saml2:NameID>' +
+                      '</saml2:AttributeValue>' +
+                  '</saml2:Attribute>' +
+              '</saml2:AttributeStatement>' +
+            '</saml2:Assertion>' +
+          '</Response>';
+          var base64xml = Buffer.from( xml ).toString('base64');
+          var container = { SAMLResponse: base64xml };
+          var samlObj = new SAML();
+          samlObj.validatePostResponse( container, function( err, profile, logout ) {
+            should.not.exist( err );
+            const eptid = profile['urn:oid:1.3.6.1.4.1.5923.1.1.1.10'];
+            const nameid = eptid['NameID'][0]
+            nameid._.should.eql(nameid_opaque_string)
+            nameid.$.NameQualifier.should.equal(nameQualifier)
+            nameid.$.SPNameQualifier.should.equal(spNameQualifier)
+            nameid.$.Format.should.equal(format)
+            done();
+          });
+        });
       });