We help IT Professionals succeed at work.

Ticket says, "Redirect from form is not validated..."

Bruce Gust
Bruce Gust asked
on
I'm working on a ticket whose title is, "Redirect from form is not validated."

Not sure what that means.

Here's what I know:
The app is using "express-session"
I'm storing the session data in "user-sessions"

try {
  let sessionObj = session({
    secret: process.env.SESSION_SECRET,
    store: new MongoStore({
      mongooseConnection: global.db,
      collection: 'user_sessions'
    }),
    cookie: {
      maxAge: (60 * 60) * 1000, // 1 hour - milliseconds
    },
    rolling: true, // resets the cookie max age on each request
    resave: false,
    saveUninitialized: true
  });

  app.use(sessionObj);
} catch (err) {
  console.log('Error: ', err);
  return false;
}

Open in new window

After successfully logging in, I can do a "console.log(req.session);" and see all of my session data.

Here's the file coming out of my "services..."

  async auth(email, password) {
    const isDev = (process.env.ENV == 'local' || process.env.ENV == 'dev');
    let match = {
      email: email,
      active: true
    };

    try {
        let user = await this.loadUser(match);        
      // validate the password hash
      if (!isDev) {
        await this.checkPasswordHash(password, user.password);
      }

      return this.success(user);
    } catch (err) {
      console.log('Error: ', err);
      return this.error(err);
    }
  }

Open in new window


And here's the login route:

router.post('/login/auth', async (req, res) => {
  if (typeof req.body.email == 'undefined' || typeof req.body.password == 'undefined') {
    flash.add(req, 'Please enter a valid email address and password before trying again.', 'danger');
    return res.redirect('/login');
  }

  let resp = await user.auth(req.body.email, req.body.password);

  if (resp.error) {
    flash.add(req, 'The provided email address and password combination is invalid. Please try again. If you need further assistance, please call 855.581.9910.', 'danger');
    return res.redirect('/login');
  }

  req.session.user = resp.data;
    await user.lastLogin();//Update lastLogin date
  let redirect = (req.body.redirect != '') ? req.body.redirect : '/';

  res.redirect(redirect);
});

Open in new window


This may all be golden and I just don't know it. But before I go asking around, I wanted to do my due diligence and make sure that I wasn't missing something.

Does this look OK? If something is jacked up, what's lacking and how can I fix it?
Comment
Watch Question

View Solution Only

Bruce GustPHP Developer

Author

Commented:
I've gotten a little more feedback, but again, I want to try to figure this out using my own resources before I just start bellyaching...

The two lines of code that are specified in the ticket belong to two different files respectively. First is the "login.js" file and the line that's targeted as a problem is the "let redirect = (req.body.redirect != '') ? req.body.redirect : '/';" line.

router.post('/login/auth', async (req, res) => {
  if (typeof req.body.email == 'undefined' || typeof req.body.password == 'undefined') {
    flash.add(req, 'Please enter a valid email address and password before trying again.', 'danger');
    return res.redirect('/login');
  }

  let resp = await user.auth(req.body.email, req.body.password);

  if (resp.error) {
    flash.add(req, 'The provided email address and password combination is invalid. Please try again. If you need further assistance, please call 855.581.9910.', 'danger');
    return res.redirect('/login');
  }

  req.session.user = resp.data;
    await user.lastLogin();//Update lastLogin date
  let redirect = (req.body.redirect != '') ? req.body.redirect : '/';

  res.redirect(redirect);
});

Open in new window



I know that let redirect = (req.body.redirect != '') ? req.body.redirect : '/'; is a ternary IF statement, but what does it mean? I've seen this where the result is a Boolean value, but that doesn't make sense here. What is it asking / proving? And why is this a problem if the session variables have already been established? Why would my teammate raise the concern that it hasn't been "validated?"

Second scenario is line #19 in the "index.js" page:

// load the environment
app.use((req, res, next) => {
  // set a global reference to the session so that we have some access outside
  // of the routers without having to pass it around everywhere
  if (typeof req.session != 'undefined' && typeof req.session.user != 'undefined') {
    // the object ids do not survey the serialization process, so we need to fix
    // that here for ease of use elsewhere
    let user = req.session.user;
    user._id = new ObjectId(user._id);
    user.account._id = new ObjectId(user.account._id);

    // assign the global reference
    global._user = user;

    // set the default timezone based on the user's settings
      moment.tz.setDefault(req.session.user.settings.timezone == 'undefined' ? 'America/Chicago' : req.session.user.settings.timezone);
  } else {
    if (!/^\/login/i.test(req.url)) {
      [b]return res.redirect('/login?redirect=' + encodeURIComponent(req.url));[/b]
    }

    moment.tz.setDefault('America/Chicago');
  }

  // set the api key for use in templates
  global._googleAPIKey = process.env.GOOGLE_MAP_KEY;

  // add in the global object models for reference
  global._objectModels = require('./server/lib/object-models');
  global._objectModels.company = require('./server/lib/model-company');
  global._objectModels.proposal = require('./server/lib/model-proposal');

  // set a reference to the root directory
  global._rootPath = __dirname;

  // set a global reference to the requested url
  global._url = req.url.replace(/(\?.*)$/, '');

  // and wrap it up
  next();
});

Open in new window


What is this:  if (!/^\/login/i.test(req.url)) {

...and again, if I'm looking at "let user = req.session.user," why is there a concern that this hasn't been validated?
WebDev
CERTIFIED EXPERT
Most Valuable Expert 2018
Distinguished Expert 2019
Commented:
Hey Bruce,

Your first point:

let passedInRedirectUrl= (req.body.redirect != '') ? req.body.redirect : '/';
res.redirect(passedInRedirectUrl);

Open in new window

[Changed the variable name to help clarify]. Basically once the email and password have been validated, the ternary statement above checks to see if the request also has a 'redirect' property passed in. If it did, then the user is redirected to that address. If it didn't, then the user is redirected to your home page (the slash just represents the homepage). The whole idea of a redirect is that if a user tries to access a page URL of 'somesecret', it's going to send them to the login page. By also passing the 'somesecret' value to the login script, the system knows where to send the user to once they've logged in.

I can only guess what your colleague means by not validating the redirect value. They probably want you to check that the request.body.redirect property is valid in some way (did the user really come from there / is it part of your own site / does it contain characters that it shouldn't). As it stands an authenticated user will be sent to the address that is provided in the redirect property, so it makes sense that it's checked for validity.

Your second point:

if (typeof req.session != 'undefined' && typeof req.session.user != 'undefined') {
    ... 
} else {
    if (!/^\/login/i.test(req.url)) {
        return res.redirect('/login?redirect=' + encodeURIComponent(req.url));
}

Open in new window

Your first if statement checks to see if you have a valid session and a valid user. If you don't, the else part runs. The [ if (!/^\/login/i.test(req.url)) { ] is a Javascript Regular Expression (RegEx) which basically checks to make sure the url beings with /login. If it doesn't, then it redirects the user to /login?redirect=whateverUrlTheyWereTryingToAccess (that redirect=xxxx is what I was discussing in the first part of my comment - it will form the req.body.redirect and get used in the ternary statement to redirect the user to after logging in)
Bruce GustPHP Developer

Author

Commented:
Hey, Chris!

I got it figured out. I went back to my coworker after I was convinced I could sound intelligent, believing that he was concerned about the way in which the current syntax did not evaluate the URL the user was coming from and, sure enough, that was it.

I put the expected URL in the .env file and then build a simple IF statement to see if that's what where the user was coming from and, if so, BOOM!

Thanks so much!