-
Notifications
You must be signed in to change notification settings - Fork 16
BearerStrategy callback returns null instead of a valid user object #3
Description
Please provide us with the following information:
This issue is for a: (mark with an x)
- [x ] bug report -> please search issues before submitting
- [ ] feature request
- [ ] documentation issue or request
- [ ] regression (a behavior that used to work and stopped in a new release)
Minimal steps to reproduce
- Create a App registration in Azure AD
- Provide client and tenant id
- run the sample
Any log messages given by the failure
Expected/desired behavior
BearerStrategy callback should return a valid user object. At least it should return any value. Passport considers the empty user object as false. This prevents proper authorization.
OS and Version?
Windows 7, 8 or 10. Linux (which distribution). macOS (Yosemite? El Capitan? Sierra?)
Versions
Mention any other details that might be useful
Please find a proposed fix below.
I also improved the config.js file to use tenantID instead of tenantName. With the ID you get the issuer and the identityMetadata.
diff --git a/app.js b/app.js
index ba8ad0b..b153cba 100644
--- a/app.js
+++ b/app.js
@@ -1,27 +1,28 @@
'use strict';
const
-
restify = require('restify') - , restifyPlugins = require ('restify').plugins
- , passport = require('passport')
- , BearerStrategy = require('passport-azure-ad').BearerStrategy
- , config = require('./config')
- , authenticatedUserTokens = []
- , serverPort = process.env.PORT || config.serverPort
-;
- restify = require('restify'),
- restifyPlugins = require('restify').plugins,
- passport = require('passport'),
- BearerStrategy = require('passport-azure-ad').BearerStrategy,
- config = require('./config'),
- authenticatedUsers = {},
- serverPort = process.env.PORT || config.serverPort;
const authenticationStrategy = new BearerStrategy(config.credentials, (token, done) => {
- let currentUser = null;
- let userToken = authenticatedUserTokens.find((user) => {
-
currentUser = user; -
user.sub === token.sub; - });
- if(!userToken) {
-
authenticatedUserTokens.push(token);
- //try to find user based on token.sub key
- let currentUser = authenticatedUsers[token.sub];
- if (!currentUser) {
-
//first time, this user is here. Create a new user object with the values we may have -
currentUser = { -
name: token.name || 'no-name', -
upn: token.upn || 'no-upn', -
unique_name: token.unique_name || 'no-unique_name', -
email: token.email || 'no-email@no-server.com' -
} -
//add the new user to the user store -
}
authenticatedUsers[token.sub]=currentUser;
- return done(null, currentUser, token);
});
diff --git a/config.js b/config.js
index f7c78de..a1b6a0e 100644
--- a/config.js
+++ b/config.js
@@ -1,12 +1,13 @@
'use strict';
-const tenantName = <YOUR_TENANT_NAME>;
+const tenantID = <YOUR_TENANT_ID>;
const clientID = <YOUR_CLIENT_ID>;
const serverPort = 3000;
module.exports.serverPort = serverPort;
module.exports.credentials = {
- identityMetadata:
https://login.microsoftonline.com/${tenantName}.onmicrosoft.com/.well-known/openid-configuration,
- identityMetadata:
https://login.microsoftonline.com/${tenantID}/v2.0/.well-known/openid-configuration, - issuer:
https://sts.windows.net/${tenantID}/,
clientID: clientID
};