Fix for issue #185 - BearerStrategy profileName issue. #186
Conversation
| @@ -293,8 +293,9 @@ Strategy.prototype.authenticate = function authenticateStrategy(req) { | |||
|
|
|||
| var metadataURL = self._options.identityMetadata; | |||
|
|
|||
| if (self._options.policyName) { | |||
| log.info('B2C: using policy %s', self._options.policyName); | |||
| if (!self._options.policyName) { | |||
There was a problem hiding this comment.
Should we really be erroring any time there's no policyName? What if we're not using B2C- there'd be no policy here, right? And why remove the log info on the policy?
There was a problem hiding this comment.
You are right, the change can be reverted here. Will update.
There was a problem hiding this comment.
So what I realize I did when I first added B2C was use policyName as the trigger for doing B2C work, so logging that it isn't there isn't correct behavior. Reverted.
| @@ -294,7 +294,7 @@ Strategy.prototype.authenticate = function authenticateStrategy(req) { | |||
| var metadataURL = self._options.identityMetadata; | |||
|
|
|||
| if (self._options.policyName) { | |||
| log.info('B2C: using policy %s', self._options.policyName); | |||
| log.info('B2C: using policy %s', self._options.policyName); | |||
There was a problem hiding this comment.
Why does the indentation look off here? Where are we closing the open bracket on the if line above?
There was a problem hiding this comment.
As I said before, this is the signal to use the B2C flow, so this is if statement is quite large (it sets the stage for B2C, including creating a new caching key for policy). The end of the if statement is line number 312:
passport-azure-ad/lib/bearerstrategy.js
Line 312 in e7305c2
There was a problem hiding this comment.
@brandwe There should be two extra blanks in front of 297.
There was a problem hiding this comment.
@brandwe You should just revert the change on line 297, as @lovemaths says.
|
@lovemaths Please test this against a B2C tenant before we merge the change. Looks like we might need a fix to #187 before the E2E will work. |
No description provided.