Merge pull request #350 from AzureAD/dev

release 3.0.9

CHANGELOG.md

@@ -1,4 +1,22 @@
-<a name="3.0.8"></a>
+<a name="3.0.9"></a>
+# 3.0.9
+
+## OIDCStrategy
+
+### Bug fixes
+
+* [#338](https://github.com/AzureAD/passport-azure-ad/issues/338) Allow query parameters in the identityMetadata config key
+
+* [#346](https://github.com/AzureAD/passport-azure-ad/pull/346) Fix: Cannot set cookie on the response 
+
+## BearerStrategy
+
+### Bug fixes
+
+* [#333](https://github.com/AzureAD/passport-azure-ad/issues/333) jwt should not check sub for access token 
+
+* [#338](https://github.com/AzureAD/passport-azure-ad/issues/338) Allow query parameters in the identityMetadata config key
+
 # 3.0.8
 
 ## OIDCStrategy

README.md

@@ -16,7 +16,7 @@ and with [Microsoft Active Directory Federation Services](http://en.wikipedia.or
 _passport-azure-ad_ has a known security vulnerability affecting versions <1.4.6 and 2.0.0. Please update to >=1.4.6 or >=2.0.1 immediately. For more details, see the [security notice](https://github.com/AzureAD/passport-azure-ad/blob/master/SECURITY-NOTICE.MD).
 
 ## 2. Versions
-Current version - 3.0.8  
+Current version - 3.0.9  
 Minimum  recommended version - 1.4.6  
 You can find the changes for each version in the [change log](https://github.com/AzureAD/passport-azure-ad/blob/master/CHANGELOG.md).
 
@@ -371,7 +371,7 @@ the strategy.
 
 * `domain_hint`: if you want to specify the domain that the user should use to sign in. This option is not supported for B2C tenant.
 
-* `login_hint`: if you want to prefill the username with a given value in the login page. The value should be the `upn` of an user, not the email (most times they are the same though). 
+* `login_hint`: if you want to prefill the username with a given value in the login page. The value should be the `upn` of a user, not the email (most times they are the same though). 
 
 * `prompt`: v1 and v2 endpoint support `login`, `consent` and `admin_consent`; B2C endpoint only supports `login`. 
 
@@ -402,10 +402,7 @@ Passport framework uses session to keep a persistent login session. As a plug in
     ...
   });
 
-  // must pass the response object to passport.authenticate, since we will use response object to set cookie
-  app.get('/login', function(req, res, next) => {
-    passport.authenticate('azuread-openidconnect', { session: false, response: res })(req, res, next);
-  });
+  app.get('/login', passport.authenticate('azuread-openidconnect', { session: false }));
 
 ```
 
@@ -583,8 +580,7 @@ First you need to register one application in v1 tenant, one in v2 tenant and on
 
 For the v2 application, you should register it at https://apps.dev.microsoft.com/ instead of Azure Portal.
 
-For the B2C application, create four policies named 'B2C_1_signin', 'B2C_1_signup', 'B2C_1_updateprofile', 
-'B2C_1_resetpassword'. For each policy, select 'Local Account' as the identity provider, and select the
+For the B2C application, create policies named 'B2C_1_signin', 'B2C_1_signup'. For each policy, select 'Local Account' as the identity provider, and select the
 following:
 
 * 'B2C_1_signup': 
@@ -593,26 +589,10 @@ following:
 
   * Application claims: 'Display Name', Email Addresses', 'Given Name', 'Identity Provider', 'Surname', 'Users Object ID'
 
-* 'B2C_1_updateprofile': 
-
-  * Profile attributes: 'Display Name', 'Given Name', 'Surname'
-
-  * Application claims: 'Display Name', Email Addresses', 'Given Name', 'Identity Provider', 'Surname', 'Users Object ID'
-
 * 'B2C_1_signin': 
 
   * Application claims: 'Display Name', Email Addresses', 'Given Name', 'Identity Provider', 'Surname', 'Users Object ID'
 
-* 'B2C_1_signin_acr': 
-
-  * Application claims: 'Display Name', Email Addresses', 'Given Name', 'Identity Provider', 'Surname', 'Users Object ID'
-
-  * After creating this policy, go the blade of this policy, click 'Edit' and then 'Token, session & SSO config'. Now switch the 'Claim representing policy ID' from 'tfp' to 'acr' and save the change.
-
-* 'B2C_1_resetpassword': 
-
-  * Application claims: 'Email Addresses', 'Given Name', 'Users Object ID'
-
 You will also need to click the 'Run now' button in the 'B2C_1_signup' blade to create an user.
 
 For B2C application, you will also need to create at least one scope and provide it to test parameters. See [how to create scope for B2C access token](https://azure.microsoft.com/en-us/blog/azure-ad-b2c-access-tokens-now-in-public-preview/). In the bearer_b2c_test, We will use OIDCStrategy to get a B2C
@@ -622,7 +602,7 @@ access token for the scope, and use BearerStrategy to validate the scope. Note f
 
 #### 6.2.2. Fill the test parameters 
 
-Open `test/End_to_end_test/script.js`, set `is_test_parameters_completed` parameter to true. For `test_parameters` variable, fill in the tenant id/client id/client secret of your applications, and the username/password of your application user. The 'oid' value is the object id of your application user. To find the 'oid' value, go to your tenant, click 'Users and groups', find your user and click it. The Object ID value will show up in the new blade.
+Open `test/End_to_end_test/script.js`, set `is_test_parameters_completed` parameter to true. For `test_parameters` variable, fill in the tenant id/client id/client secret of your applications, and the username/password of your application user. 
 
 For `thumbprint` and `privatePEMKey` parameters, you need to specify a certificate for your app and register the public key in Azure Active Directory. `thumbprint` is the base64url format of the thumbprint of the public key, and `privatePEMKey` is the private pem key string. For a v1 tenant, you can follow [this post](http://www.andrewconnell.com/blog/user-app-app-only-permissions-client-credentials-grant-flow-in-azure-ad-office-365-apis) to generate a certificate and register the public key. For a v2 tenant, you can go to your application page in the [v2 portal](https://apps.dev.microsoft.com) and click `Generate New Key Pair`. A certificate will be generated for you to download. The corresponding public key is automatically registered in this case.  
 

lib/aadutils.js

@@ -253,3 +253,16 @@ exports.getErrorMessage = (err) => {
   return str;
 };
 
+exports.concatUrl = (url, rest) => {
+  if (typeof rest === 'string' || rest instanceof String) {
+    rest = [rest];
+  }
+
+  if (!url) {
+    return `?${rest.join('&')}`;
+  }
+
+  var hasParam = url.indexOf('?') !== -1;
+  return rest ? url.concat(hasParam ? '&' : '?').concat(rest.join('&')) : url;
+};
+

lib/bearerstrategy.js

@@ -386,12 +386,15 @@ Strategy.prototype.authenticate = function authenticateStrategy(req) {
 
     // compute metadataUrl
     (next) => {
-      params.metadataURL = self._options.identityMetadata
-            .concat(`?${aadutils.getLibraryProductParameterName()}=${aadutils.getLibraryProduct()}`)
-            .concat(`&${aadutils.getLibraryVersionParameterName()}=${aadutils.getLibraryVersion()}`); ;
+      params.metadataURL = aadutils.concatUrl(self._options.identityMetadata,
+        [
+          `${aadutils.getLibraryProductParameterName()}=${aadutils.getLibraryProduct()}`,
+          `${aadutils.getLibraryVersionParameterName()}=${aadutils.getLibraryVersion()}`
+        ]
+      );
 
       if (self._options.isB2C)
-        params.metadataURL = params.metadataURL.concat(`&p=${self._options.policyName}`)
+        params.metadataURL = aadutils.concatUrl(params.metadataURL, `p=${self._options.policyName}`);
 
       params.cacheKey = params.metadataURL;
 
@@ -432,6 +435,9 @@ Strategy.prototype.authenticate = function authenticateStrategy(req) {
       if (self._options.scope)
         optionsToValidate.scope = self._options.scope;
 
+      // Beaer token is considered as an access_token.  
+      optionsToValidate.isAccessToken = true;
+
       log.info(`In Strategy.prototype.authenticate: we will validate the following options: ${optionsToValidate}`);
 
       return next();

lib/constants.js

@@ -28,8 +28,6 @@
 var CONSTANTS = {};
 
 CONSTANTS.POLICY_REGEX = /^b2c_1a?_[0-9a-z._-]+$/i;    // policy is case insensitive
-CONSTANTS.TENANTNAME_REGEX = /^[0-9a-zA-Z]+.onmicrosoft.com$/;
-CONSTANTS.TENANTID_REGEX = /^[0-9a-zA-Z-]+$/;
 
 CONSTANTS.CLOCK_SKEW = 300; // 5 minutes
 

lib/jsonWebToken.js

@@ -48,6 +48,7 @@ var hasCommonElem = (array1, array2) => {
  *   - issuer (required if validateIssuer is true)
  *   - subject (optional, validate if provided)
  *   - ignoreExpiration (optional, if not set true we will validate expiration)
+ *   - isAccessToken (optional. Specify if the token is id_token or access_token. The default value is false.)
  * @callback
  */
 exports.verify = function(jwtString, PEMKey, options, callback) {
@@ -164,13 +165,15 @@ exports.verify = function(jwtString, PEMKey, options, callback) {
       return done(new Error('jwt issuer is invalid. expected: ' + options.issuer));
   }
 
-  // (2) subject
+  // (2) subject (id_token only. We don't check subject for access_token)
   //   - check the existence and the format of payload.sub
   //   - validate if options.subject is set
-  if (typeof payload.sub !== 'string' || payload.sub === '')
-    return done(new Error('invalid sub value in payload'));
-  if (options.subject && options.subject !== payload.sub)
-      return done(new Error('jwt subject is invalid. expected: ' + options.subject));
+  if (options.isAccessToken !== true) {
+    if (typeof payload.sub !== 'string' || payload.sub === '')
+      return done(new Error('invalid sub value in payload'));
+    if (options.subject && options.subject !== payload.sub)
+        return done(new Error('jwt subject is invalid. expected: ' + options.subject));
+  }
 
   // (3) audience
   //   - always validate

lib/oidcstrategy.js

@@ -418,8 +418,13 @@ function Strategy(options, verify) {
     options.isB2C = false;
 
   // add telemetry
-  options.identityMetadata = options.identityMetadata.concat(`?${aadutils.getLibraryProductParameterName()}=${aadutils.getLibraryProduct()}`)
-  .concat(`&${aadutils.getLibraryVersionParameterName()}=${aadutils.getLibraryVersion()}`);
+  options.identityMetadata = aadutils.concatUrl(
+    options.identityMetadata,
+    [
+      `${aadutils.getLibraryProductParameterName()}=${aadutils.getLibraryProduct()}`,
+      `${aadutils.getLibraryVersionParameterName()}=${aadutils.getLibraryVersion()}`
+    ]
+  );
 
   /****************************************************************************************
    * Take care of issuer and audience
@@ -573,11 +578,7 @@ Strategy.prototype.authenticate = function authenticateStrategy(req, options) {
   var prompt = options && options.prompt;
   var extraAuthReqQueryParams = options && options.extraAuthReqQueryParams;
   var extraTokenReqQueryParams = options && options.extraTokenReqQueryParams;
-  var response = options && options.response;
-
-  // validate tenantIdOrName if it is provided
-  if (tenantIdOrName && !CONSTANTS.TENANTNAME_REGEX.test(tenantIdOrName) && !CONSTANTS.TENANTID_REGEX.test(tenantIdOrName))
-    return self.failWithLog(`In passport.authenticate: invalid tenantIdOrName ${tenantIdOrName}`);
+  var response = options && options.response || req.res;
 
   // 'params': items we get from the request or metadata, such as id_token, code, policy, metadata, cacheKey, etc
   var params = { 'tenantIdOrName': tenantIdOrName, 'extraAuthReqQueryParams': extraAuthReqQueryParams, 'extraTokenReqQueryParams': extraTokenReqQueryParams }; 
@@ -1331,13 +1332,8 @@ Strategy.prototype._flowInitializationHandler = function flowInitializationHandl
   // add telemetry
   params[aadutils.getLibraryProductParameterName()] = aadutils.getLibraryProduct();
   params[aadutils.getLibraryVersionParameterName()] = aadutils.getLibraryVersion();
-  let location;
-
-  // Implement support for standard OpenID Connect params (display, prompt, etc.)
-  if (self._options.isB2C)
-    location = `${oauthConfig.authorization_endpoint}&${querystring.stringify(params)}`;
-  else
-    location = `${oauthConfig.authorization_endpoint}?${querystring.stringify(params)}`;
+
+  const location = aadutils.concatUrl(oauthConfig.authorization_endpoint, querystring.stringify(params));
 
   return self.redirect(location);
 };

package.json

@@ -1,6 +1,6 @@
 {
   "name": "passport-azure-ad",
-  "version": "3.0.8",
+  "version": "3.0.9",
   "license": "MIT",
   "keywords": [
     "azure active directory",

test/Chai-passport_test/aadutils_test.js

@@ -54,3 +54,33 @@ describe('uid test', function() {
     done();
   });
 });
+
+describe('concatUrl test', function() {
+  it('should generate a valid url if a query parameter is already in place', function(done) {
+    const currentUrl = 'http://example.com?foo=bar';
+    const newUrl = aadutils.concatUrl(currentUrl, ['bar=foo']);
+    expect(newUrl).to.equal(`${currentUrl}&bar=foo`);
+    done();
+  });
+
+  it('should generate a valid url if no query parameter is present', function(done) {
+    const currentUrl = 'http://example.com';
+    const newUrl = aadutils.concatUrl(currentUrl, 'bar=foo');
+    expect(newUrl).to.equal(`${currentUrl}?bar=foo`);
+    done();
+  });
+
+  it('should return the bare url if no additional arguments are present', function(done) {
+    const currentUrl = 'http://example.com';
+    const newUrl = aadutils.concatUrl(currentUrl);
+    expect(newUrl).to.equal(currentUrl);
+    done();
+  });
+
+  it('should return parseable query parameters if no url is present', function(done) {
+    const parameters = ['bar=foo&foo=bar'];
+    const newUrl = aadutils.concatUrl(undefined, parameters)
+    expect(newUrl).to.equal(`?${parameters}`);
+    done();
+  });
+})

test/Chai-passport_test/constants_test.js

@@ -29,9 +29,6 @@ var CONSTANTS = require('../../lib/constants');
 
 const TEST_TIMEOUT = 1000000; // 1000 seconds
 
-CONSTANTS.TENANTNAME_REGEX = /^[0-9a-zA-Z]+.onmicrosoft.com$/;
-CONSTANTS.TENANTID_REGEX = /^[0-9a-zA-Z-]+$/;
-
 describe('policy checking', function() {
   this.timeout(TEST_TIMEOUT);
 
@@ -61,36 +58,3 @@ describe('policy checking', function() {
     done();
   });
 });
-
-describe('tenant name checking', function() {
-  this.timeout(TEST_TIMEOUT);
-
-  it('should pass with good tenant name', function(done) {
-    expect(CONSTANTS.TENANTNAME_REGEX.test('contoso123COMPANY.onmicrosoft.com')).to.equal(true);
-    done();
-  });
-
-  it('should fail with bad tenant name', function(done) {
-    expect(CONSTANTS.TENANTNAME_REGEX.test('contoso.onmicrosoft.comm')).to.equal(false);
-    expect(CONSTANTS.TENANTNAME_REGEX.test('contoso123COMPANY')).to.equal(false);
-    expect(CONSTANTS.TENANTNAME_REGEX.test('.onmicrosoft.com')).to.equal(false);
-    expect(CONSTANTS.TENANTNAME_REGEX.test('contoso123COMPANY.ONMICROSOFT.com')).to.equal(false);
-    expect(CONSTANTS.TENANTNAME_REGEX.test('contoso_company.onmicrosoft.com')).to.equal(false);
-    done();
-  });
-});
-
-describe('tenant id checking', function() {
-  this.timeout(TEST_TIMEOUT);
-
-  it('should pass with good tenant id', function(done) {
-    expect(CONSTANTS.TENANTID_REGEX.test('683eAd13-3193-43f0-9677-d727c25a588f')).to.equal(true);
-    done();
-  });
-
-  it('should fail with bad tenant id', function(done) {
-    expect(CONSTANTS.TENANTID_REGEX.test('23_12')).to.equal(false);
-    done();
-  });
-});
-

test/Chai-passport_test/json_web_token_test.js

@@ -0,0 +1,55 @@
+/**
+ * Copyright (c) Microsoft Corporation
+ *  All Rights Reserved
+ *  MIT License
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this
+ * software and associated documentation files (the "Software"), to deal in the Software
+ * without restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS
+ * OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT
+ * OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+'use strict';
+
+var chai = require('chai');
+var jws = require('jws');
+var jwt = require('../../lib/jsonWebToken');
+
+const TEST_TIMEOUT = 1000000; // 1000 seconds
+
+const secret = "12345678901234567890123456789012"; // 512 bit symmetric key
+
+describe('json web token test', function() {
+  this.timeout(TEST_TIMEOUT);
+
+  var signStream = jws.createSign(
+  { 
+    'header': { 'alg': 'HS256', 'typ': 'JWT'}, 
+    'payload': { 'nbf': Date.now() / 1000, 'exp': Date.now() / 1000 +  300,  'iat': Date.now() / 1000, 'iss': 'https://example.com', 'aud': 'audience' }, 
+    'secret': secret 
+  });
+
+  var jwtString = signStream.sign(); // create the corresponding json web token
+  var options = { audience: 'audience', algorithms: ['HS256'], issuer: 'https://example.com' }; // validation options
+
+  it('should fail with missing sub error', function(done) {
+    jwt.verify(jwtString, secret, options, (err, token) => { chai.expect(err.message).to.equal('invalid sub value in payload'); done(); });
+  });
+
+  it('should succeed if testing access token', function(done) {
+    options.isAccessToken = true;
+    jwt.verify(jwtString, secret, options, (err, token) => { chai.expect(err).to.equal(null); done(); });
+  });
+});