Skip to content
This repository has been archived by the owner on Aug 28, 2023. It is now read-only.

Cannot read property 'keys' of undefined #90

Closed
AlexandrTuniev opened this issue Mar 18, 2016 · 4 comments
Closed

Cannot read property 'keys' of undefined #90

AlexandrTuniev opened this issue Mar 18, 2016 · 4 comments
Assignees
Labels

Comments

@AlexandrTuniev
Copy link

Hey
We have a node js server, that running using your library for authentication.
There is a exception thrown from it.

[2016-03-18 16:58:08.180] [ERROR] errorHandler - Cannot read property 'keys' of undefined
TypeError: Cannot read property 'keys' of undefined
    at Metadata.generateOidcPEM (xxx\server\node_modules\passport-azure-ad\lib\passport-azure-ad\metadata.js:197:19)
    at Strategy.jwtVerify [as _verify] (xxx\server\node_modules\passport-azure-ad\lib\passport-azure-ad\bearerstrategy.js:176:36)
    at Strategy.authenticate (xxx\server\node_modules\passport-azure-ad\node_modules\passport-http-bearer\lib\strategy.js:132:10)
    at attempt (xxx\server\node_modules\passport\lib\middleware\authenticate.js:348:16)
    at authenticate (xxx\server\node_modules\passport\lib\middleware\authenticate.js:349:7)
    at Layer.handle [as handle_request] (xxx\server\node_modules\express\lib\router\layer.js:95:5)
    at trim_prefix (xxx\server\node_modules\express\lib\router\index.js:312:13)
    at xxx\server\node_modules\express\lib\router\index.js:280:7
    at Function.process_params (xxx\server\node_modules\express\lib\router\index.js:330:12)
    at next (xxx\server\node_modules\express\lib\router\index.js:271:10)
    at initialize (xxx\server\node_modules\passport\lib\middleware\initialize.js:53:5)
    at Layer.handle [as handle_request] (xxx\server\node_modules\express\lib\router\layer.js:95:5)
    at trim_prefix (xxx\server\node_modules\express\lib\router\index.js:312:13)
    at xxx\server\node_modules\express\lib\router\index.js:280:7
    at Function.process_params (xxx\server\node_modules\express\lib\router\index.js:330:12)
    at next (xxx\server\node_modules\express\lib\router\index.js:271:10)

Or some times it rejects valid json token, and messages, "An error was received validating the token secret or public key must be provided". I think it happens because at the time when token comes for validation the public key hasn't downloaded yet. For reproduce this issue I stop and start node server, and immediately after that make request. On later requests with the same token there isn't any issue.
Below is my node code.

if (env !== 'test') {
  app.use(passport.initialize());

  var authentication = require('./authentication/strategy');

  passport.use(authentication.bearerStrategy);

  app.use(passport.authenticate('oauth-bearer', {
    session: false
  }));
}

//Wire up routing
app.use('/dashboard/api/v1', router);
var env = process.env.NODE_ENV || 'development',
  adCredentials = require(__dirname + '/../config/config.json')[env].adCredentials,
  BearerStrategy = require('passport-azure-ad').BearerStrategy;

var options = {
  identityMetadata: adCredentials.identityMetadata,
  audience: adCredentials.audience,
  clientID: adCredentials.clientID,
  passReqToCallback: false,
  validateIssuer: true
};


exports.bearerStrategy = new BearerStrategy(options,
    function(token, done) {
        return done(null, token);
    }
);

Any help appreciated.
Thanks,
Sash

@AlexandrTuniev
Copy link
Author

After some investigation found that issue connected with this,

 this.metadata.fetch(function(err) {
        if (err) {
            throw new Error("Unable to fetch metadata: " + err);
        }

    });


    function jwtVerify(req, token, done) {

        if (!options.passReqToCallback) {
            token = arguments[0];
            done = arguments[1];
            req = null;
            log.info('got token - going in to verification');
        }


        var decoded = jws.decode(token);
        if (decoded == null) {
            done(null, false, "Invalid JWT token.");
        }

        log.info('token decoded:  ', decoded);


        // We have two different types of token signatures we have to validate here. One provides x5t and the other a kid.
        // We need to call the right one.

        if (decoded.header.x5t) {
            PEMkey = this.metadata.generateOidcPEM(decoded.header.x5t);
        } else if (decoded.header.kid) {
            PEMkey = this.metadata.generateOidcPEM(decoded.header.kid);
        } else {
            throw new TypeError('We did not reveive a token we know how to validate');
        }



        if (options.validateIssuer) {
            options.issuer = this.metadata.oidc.issuer;
        }
        options.algorithms = this.metadata.oidc.algorithms;

        jwt.verify(token, PEMkey, options, function(err, token) {
            if (err) {
                if (err instanceof jwt.TokenExpiredError) {
                    log.warn("Access token expired");
                    done(null, false, 'The access token expired');
                } else if (err instanceof jwt.JsonWebTokenError) {
                    log.warn("An error was received validating the token", err.message);
                    done(null, false, util.format('Invalid token (%s)', err.message));
                } else {
                    done(err, false);
                }
            } else {
                log.info(token, 'was token going out of verification');
                if (options.passReqToCallback) {
                    log.info("We did pass Req back to Callback");
                    verify(req, token, done);
                } else {
                    log.info("We did not pass Req back to Callback");
                    verify(token, done);
                }
            }
        });
    }

jwtVerify function doesn't wait for metadata.fetch method's successful completion.

@brandwe
Copy link
Contributor

brandwe commented Apr 14, 2016

Strange. This can be fixed with a waterfall and I thought I'd had done so. I'll look in to this now and get a fix in by EOW.

@AlexandrTuniev
Copy link
Author

Are there any updates on this?

lovemaths added a commit that referenced this issue Jul 6, 2016
Metadata loading is async, passport may use the metadata content even if the metadata loading is not finished, for instance, keys. In this fix, the following are done:

(1) load metadata dynamically at `authenticate` time, from server or memory cache
(2) solved async issue using waterfall
(3) added a couple of unit tests
lovemaths added a commit that referenced this issue Jul 7, 2016
Metadata loading is async, passport may use the metadata content even if the metadata loading is not finished, for instance, keys. In this fix, the following are done:

(1) load metadata dynamically at `authenticate` time, from server or memory cache
(2) solved async issue using waterfall
(3) added a couple of unit tests
lovemaths added a commit that referenced this issue Jul 13, 2016
BearerStrategy used to inherit from passport-http-bearer strategy
   https://github.com/jaredhanson/passport-http-bearer/
when BearerStrategy is created, it reads the info (from developer and metadata). In the authentication time, passport-http-bearer does all the work using these info read at the BearerStrategy's creation time.

This is not good since:
(1) for B2C, different policy may have different metadata, so the metadata should be loaded dynamically (and cached) in the authentication time based on the request
(2) async issue, metadata loading is async, passport-http-bearer's `authenticate` function depends on the metadata, and it might be called before the metadata loading finishes. This is in fact the cause of this bug, where we validate the token before we load the keys from metadata url.

In this fix, the following are done:

(1) load metadata dynamically at `authenticate` time, from server or memory cache
(2) solved async issue using waterfall
(3) added a couple of unit tests
lovemaths added a commit that referenced this issue Jul 13, 2016
BearerStrategy used to inherit from passport-http-bearer strategy
   https://github.com/jaredhanson/passport-http-bearer/
when BearerStrategy is created, it reads the info (from developer and metadata). In the authentication time, passport-http-bearer does all the work using these info read at the BearerStrategy's creation time.

This is not good since:
(1) for B2C, different policy may have different metadata, so the metadata should be loaded dynamically (and cached) in the authentication time based on the request
(2) async issue, metadata loading is async, passport-http-bearer's `authenticate` function depends on the metadata, and it might be called before the metadata loading finishes. This is in fact the cause of this bug, where we validate the token before we load the keys from metadata url.

In this fix, the following are done:

(1) load metadata dynamically at `authenticate` time, from server or memory cache
(2) solved async issue using waterfall
(3) added a couple of unit tests
lovemaths added a commit that referenced this issue Jul 13, 2016
BearerStrategy used to inherit from passport-http-bearer strategy
   https://github.com/jaredhanson/passport-http-bearer/
when BearerStrategy is created, it reads the info (from developer and metadata). In the authentication time, passport-http-bearer does all the work using these info read at the BearerStrategy's creation time.

This is not good since:
(1) for B2C, different policy may have different metadata, so the metadata should be loaded dynamically (and cached) in the authentication time based on the request
(2) async issue, metadata loading is async, passport-http-bearer's `authenticate` function depends on the metadata, and it might be called before the metadata loading finishes. This is in fact the cause of this bug, where we validate the token before we load the keys from metadata url.

In this fix, the following are done:

(1) load metadata dynamically at `authenticate` time, from server or memory cache
(2) solved async issue using waterfall
(3) added a couple of unit tests
lovemaths added a commit that referenced this issue Jul 13, 2016
BearerStrategy used to inherit from passport-http-bearer strategy
   https://github.com/jaredhanson/passport-http-bearer/
when BearerStrategy is created, it reads the info (from developer and metadata). In the authentication time, passport-http-bearer does all the work using these info read at the BearerStrategy's creation time.

This is not good since:
(1) for B2C, different policy may have different metadata, so the metadata should be loaded dynamically (and cached) in the authentication time based on the request
(2) async issue, metadata loading is async, passport-http-bearer's `authenticate` function depends on the metadata, and it might be called before the metadata loading finishes. This is in fact the cause of this bug, where we validate the token before we load the keys from metadata url.

In this fix, the following are done:

(1) load metadata dynamically at `authenticate` time, from server or memory cache
(2) solved async issue using waterfall
(3) added a couple of unit tests
lovemaths added a commit that referenced this issue Jul 14, 2016
BearerStrategy used to inherit from passport-http-bearer strategy
   https://github.com/jaredhanson/passport-http-bearer/
when BearerStrategy is created, it reads the info (from developer and metadata). In the authentication time, passport-http-bearer does all the work using these info read at the BearerStrategy's creation time.

This is not good since:
(1) for B2C, different policy may have different metadata, so the metadata should be loaded dynamically (and cached) in the authentication time based on the request
(2) async issue, metadata loading is async, passport-http-bearer's `authenticate` function depends on the metadata, and it might be called before the metadata loading finishes. This is in fact the cause of this bug, where we validate the token before we load the keys from metadata url.

In this fix, the following are done:

(1) load metadata dynamically at `authenticate` time, from server or memory cache
(2) solved async issue using waterfall
(3) added a couple of unit tests
lovemaths added a commit that referenced this issue Jul 14, 2016
Solved #90: Cannot read property 'keys' of undefined
@lovemaths
Copy link
Contributor

done

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants