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

req.user is undefined #457

Closed
Arithgale opened this issue Dec 9, 2019 · 6 comments
Closed

req.user is undefined #457

Arithgale opened this issue Dec 9, 2019 · 6 comments
Labels

Comments

@Arithgale
Copy link

Arithgale commented Dec 9, 2019

Originally Passport indicates that if login is successful the req.user property will be set. The documentation for this library says the same. When I use the library however req.user is undefined. Moreover, the verify function in the documentation is never called. I have put breakpoints on it while using Visual Studio Code and also tried to use console.log, but the function is never called. Serialize and deserialize functions passed to passport are never called either.

The library instead is returning the id_token and code (accessToken) in the req.body property. It also returns in that body the nonce and state. I know the passport session is working because it is effectively creating a session. The code below prints the session to the console. Does the documentation need to be updated to show that information is actually returned in req.body and contrary to Passport documentation, this library never sets req.user? Am I doing something wrong?

As of the moment of writing this I am using the latest version of all libraries.

Express version: 4.17.1
express-session version: 1.17.0
passport-azure-ad version: 4.2.0

Here is the code I have been using for testing. The clientID is not the actual app client ID and the tenantID in the identityMetadata url is not the actual tenantID for my test app.

const express = require('express');
var passport = require('passport');
var OIDCStrategy = require('passport-azure-ad').OIDCStrategy;
var session = require('express-session');
const app = express();


app.use(express.urlencoded({extended: true}));

app.use(session({ secret: 'randomly-generated_secret' }));
// Initialize passport
app.use(passport.initialize());
app.use(passport.session());


// Breakpoints here are never called when debugging
passport.serializeUser(function(user, done) {
  done (null, user);
});

// Breakpoints here are never called when debugging
passport.deserializeUser(function(user, done) {
  done(null, user);
});

const port = 3000;


// Show Hello World
app.get('/', (req, res) => {res.send('Hello World')});

/* Sign in */
app.get('/signin',
  function  (req, res, next) {
    passport.authenticate('azuread-openidconnect',
      {
        failureRedirect: '/',
      }
    )(req,res,next);
  }
);

// Called after sign in
app.post('/callback',
  function(req, res, next) {
    passport.authenticate('azuread-openidconnect',
      {
        failureRedirect: '/',
      }
    );
    next();
  },
  function(req, res) {
    console.log(req.user);
    console.log(req.body.user);
    console.dir('Req body: ' + JSON.stringify(req.body));
    console.log('\n\nReq session: ' + JSON.stringify(req.session));
    res.redirect('/');
  }
);

app.get('/logout', (req, res) => {
  req.session.destroy();
  res.send('Logged out.');
})

// This function is supposed to be called once sign-in is complete
// But is never called
async function signInComplete(iss, sub, profile, accessToken, refreshToken, params, done) {
  if (!profile.oid) {
    return done(new Error("No oid found"), null);
  }
  // asynchronous verification, for effect...
  process.nextTick(function () {
    findByOid(profile.oid, function(err, user) {
      if (err) {
        return done(err);
      }
      if (!user) {
        // "Auto-registration"
        users.push(profile);
        return done(null, profile);
      }
      return done(null, user);
    });
  });
}


// Strategy
var strategy = new OIDCStrategy({
    identityMetadata: 'https://login.microsoftonline.com/00112233-4455-6677-8899-aabbccddeeff/v2.0/.well-known/openid-configuration',
    clientID: '00112233-4455-6677-8899-aabbccddeeff',
    clientSecret: 'client-secret-taken-from-azure',
    scope: ['User.Read'],
    responseType: 'code id_token',
    responseMode: 'form_post',
    redirectUrl: 'https://localhost/callback',
}, signInComplete);

passport.use(strategy);

// Start listening on specified port
app.listen(port, () => {console.log(`Listening on port: ${port}`)});
@jasonnutter
Copy link
Contributor

jasonnutter commented Dec 10, 2019

@Arithgale My understanding is that passport.authenticate is asynchronous, but it looks like your code is invoking next synchronously after calling passport.authenticate, which I'm guessing is calling your next middleware before passport.authenticate has completed.

Try this:

// Called after sign in
app.post('/callback',
   passport.authenticate('azuread-openidconnect',
      {
        failureRedirect: '/',
      }
    ),
  function(req, res) {
    console.log(req.user);
    console.log(req.body.user);
    console.dir('Req body: ' + JSON.stringify(req.body));
    console.log('\n\nReq session: ' + JSON.stringify(req.session));
    res.redirect('/');
  }
);

See: http://www.passportjs.org/docs/authenticate/

@jasonnutter jasonnutter added question and removed bug labels Dec 10, 2019
@Arithgale
Copy link
Author

Thanks. That was indeed what was causing the problem with the signInComplete function not being called. Although now that it is being called, profile.oid is undefined which causes it to return No oid found. When I put a breakpoint on the signInComplete function it displays that the profile variable exist, but the oid is undefined. I'll have to figure out why it is not setting the oid.

@Arithgale
Copy link
Author

Arithgale commented Dec 10, 2019

Figured it out.

To make profile.oid be defined, the scope in the Strategy must have 'profile'. For example scope: ['User.Read', 'profile'].

Given that the documentation marks the scope to be optional, shouldn't it instead specify that it is a mandatory field with profile in it if and only if the verification function is to be used with the profile.oid? Because profile.oid will be undefined unless one chooses to ask for it on the scope. For example:

If the profile argument is going to be used on the verification function, the scope must specify ['profile'] as such.

If it helps to anyone, once the signInComplete function is called, serializeUser is called. The signInComplete function will pass to the user parameter of serializeUser function what you want to serialize. Then, on the callback function provided on the strategy redirectUrl, the function that comes after calling passport.authenticate will have the req.user on it which on the code below is just the whole user profile. Because that is what we passed to the serializeUser function from the signInComplete function.

const express = require('express');
var passport = require('passport');
var OIDCStrategy = require('passport-azure-ad').OIDCStrategy;
var session = require('express-session');
const app = express();


app.use(express.urlencoded({extended: true}));

app.use(session({ secret: 'randomly-generated_secret' }));
// Initialize passport
app.use(passport.initialize());
app.use(passport.session());

passport.serializeUser(function(user, done) {
  done (null, user);
});

passport.deserializeUser(function(user, done) {
  done(null, user);
});

const port = 3000;

// Show Hello World
app.get('/', (req, res) => {res.send('Hello World')});

/* Sign in */
app.get('/signin',
  function  (req, res, next) {
    passport.authenticate('azuread-openidconnect',
      {
        failureRedirect: '/',
      }
    )(req,res,next);
  }
);

// Called after sign in
app.post('/callback',
   passport.authenticate('azuread-openidconnect',
      {
        failureRedirect: '/',
      }
    ),
  function(req, res) {
    console.log(req.user);
    res.redirect('/');
  }
);

app.get('/logout', (req, res) => {
  req.session.destroy();
  res.send('Logged out.');
})

// After sign in
async function signInComplete(iss, sub, profile, accessToken, refreshToken, params, done) {
  if (!profile.oid) {
    return done(new Error("No oid found"), null);
  }
  return done(null, profile);
}


// Strategy
var strategy = new OIDCStrategy({
    identityMetadata: 'https://login.microsoftonline.com/00112233-4455-6677-8899-aabbccddeeff/v2.0/.well-known/openid-configuration',
    clientID: '00112233-4455-6677-8899-aabbccddeeff',
    clientSecret: 'client-secret-taken-from-azure',
    scope: ['User.Read', 'profile'],
    responseType: 'code id_token',
    responseMode: 'form_post',
    redirectUrl: 'https://localhost/callback',
    passReqToCallback: false
}, signInComplete);

passport.use(strategy);

// Start listening on specified port
app.listen(port, () => {console.log(`Listening on port: ${port}`)});

@jasonnutter
Copy link
Contributor

@Arithgale Thanks, that's good feedback. In our other libraries we always provide profile openid when making requests, so I'll see if we can update the docs to reflect that.

@Arithgale
Copy link
Author

Since it was just a question I closed the issue. Thanks for the help everyone. It's working now.

@tylernix
Copy link

tylernix commented Aug 3, 2020

Changing my scope to ['User.Read', 'profile'] fixed my "req.user is undefined" issue since none of the user's information was being returned in the id_token until adding Profile in the scope.

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