Fix Super Admin User Login issue

[JackHaeg]

Overview

After implementing the Super Admin User feature in #1747, we identified a bug where the Super Admin User ([email protected]) is unable to log in to VRMS. This issue tracks the fix for this bug.

Past Research

Bug summary from Jack Haeger_2024-10-17

Trillium merged the Super User PR, rebuilt Dev, and edited the DB to make [email protected] a super user.

The good news is that almost everything is working as expected (the user screen is grayed out and no other admin users can edit this user).

The bad news is that we cannot log in with this user to dev.vrms.io:

• When we go to log in with that user ([email protected]) we are unable to log in and we receive the error message “We don’t recognize your email address. Please, create an account.”
• I can confirm that this account already existed due to the fact that 1) the account shows up in dev.VRMS.io, and 2) the account’s email inbox received VRMS magic links to log in to the dev.vrms.io website on September 16, so it clearly existed and was logging in appropriately.
• Also, this email address logs in as expected on PROD.

Nikhil's research_2024-10-22

The checkAuth Method has authOrigin Set to LOG_IN

It fetches SIGN_IN

Which is exposed by this route

That router works in this way and it checks for a method called verifyUser.isAdminByEmai

And voila

if (role === 'admin' || user.managedProjects.length > 0) It is allowing either "admin" or someone who has managedProjects.length>0

Now if we look at the super user Neither are they "admin" and their "managedProjects" empty So they are not able to log in

But when you see Trillium's profile, Jack Haeger told me that when converting Trillium to super-user, he was still able to log in because the managedProjects list is not empty!

Action Items

• [ ] Implement Nikhil's proposed fix below:

change user.middleware.js to
const { User } = require('../models');
function checkDuplicateEmail(req, res, next) {
  User.findOne({ email: req.body.email }).then((user) => {
    if (user) {
      return res.sendStatus(400);
    }
    next();
  });
}
function isAdminByEmail(req, res, next) {
  User.findOne({ email: req.body.email }).then((user) => {
    if (!user) {
      return res.sendStatus(400);
    } else {
      const role = user.accessLevel;
      if (role === 'admin' || role ==='superadmin' ||  user.managedProjects.length > 0) {
        next();
      } else {
        next(res.sendStatus(401));
      }
    }
  });
}
const verifyUser = {
  checkDuplicateEmail,
  isAdminByEmail,
};
module.exports = verifyUser;

• [ ] Test if fix is successful by logging into Dev with [email protected]
  • [ ] If fix worked, the "Success" message will be displayed and a VRMS Login link will be sent to the inbox of [email protected]
  • [ ] If fix fails, the error message will be displayed under the email name: "We don't recognize your email address. Please, create an account."

"Success" Screenshot

Error message screenshot

Resources/Instructions

• This issue is part of this epic: #1737

[ntrehan]

@JackHaeg The good news is that I was indeed able to sign in with the super user with the changes So I was on the right track

But we have another big problem The entire application runs as if the super user is a "USER"

This is happening because as I checked, a lot of parts of VRMS are only visible to users that have accessLevel = "ADMIN", for example user edit screen, event creation etc.

We have two options, I would like to know your opinions @trillium @jbubar @bkmorgan3

1. Scan the entire code base and wherever anything is allowed for Admin, should also be allowed for superadmin
2. Keep accessLevel of the superuser to be "admin", and add a totally separate boolean "isSuperAdmin" in the mongoDB users collection. That way the super user is both admin and super admin. This approach is much simpler and will need less refactoring

[JackHaeg]

@trillium Please provide next steps for Nikhil as discussed on call (while considering delivery timeline of 2-3 weeks)

[trillium]

I'd prefer us to create an isAdmin() function that takes in the user's access level and returns whathe app currently expects eg:

function isAdmin(user) { 
    user.accessLevel('admin' || 'superadmin') {
        return true
    }
    return false
}

and then swap that function in where the comparisons are added for " === 'admin' " is requested in the app

[JackHaeg]

@ntrehan Just checking in on this issue:

• Does the feedback from @trillium make sense / do you have any additional questions?
• Can you please provide an update on your progress on this issue?
  1. Progress: "What is the current status of your project? What have you completed and what is left to do?"
  2. Blockers: "Difficulties or errors encountered."
  3. Availability: "How much time will you have this week to work on this issue?"
  4. ETA: "When do you expect this issue to be completed?"
  5. Pictures or links* (if necessary): "Add any pictures or links that will help illustrate what you are working on." - remember to add links to the top of the issue if they are going to be needed again.

[trillium]

FYI - tested logging in on Dev with an account that includes symbols (+ and -), and was unable to log in. However, if your build is allowing emails with symbols to log in, you can ignore the following screenshots.

[trillium]

Hey all, I'd like us to use a different boolean check for admin status, let's do:

Checking if the access level string contains the word "admin" will return true for both admin and superadmin

[trillium]

// change user.middleware.js to

const { User } = require('../models');
function checkDuplicateEmail(req, res, next) {
  User.findOne({ email: req.body.email }).then((user) => {
    if (user) {
      return res.sendStatus(400);
    }
    next();
  });
}
function isAdminByEmail(req, res, next) {
  User.findOne({ email: req.body.email }).then((user) => {
    if (!user) {
      return res.sendStatus(400);
    } else {
      const role = user.accessLevel;
      if (role.includes('admin') ||  user.managedProjects.length > 0) { // changes added here with .includes()
        next();
      } else {
        next(res.sendStatus(401));
      }
    }
  });
}
const verifyUser = {
  checkDuplicateEmail,
  isAdminByEmail,
};
module.exports = verifyUser;

[JackHaeg]

@ntrehan just checking in! Do you have any updates on this fix (ETA, blockers, etc.)?

[JackHaeg]

Hi @ntrehan! Checking in one last time to see if you are still interested / available to work on this issue. If you are no longer available, we will likely unassign you and move this issue back to the prioritized backlog for another developer to work on.

Hope all is well!

[JackHaeg]

Hi @ntrehan! When you have a moment, can you please provide an update with the following information:

1. Progress: "What is the current status of your project? What have you completed and what is left to do?"
2. Blockers: "Difficulties or errors encountered."
3. Availability: "How much time will you have this week to work on this issue?"
4. ETA: "When do you expect this issue to be completed?"
5. Pictures or links* (if necessary): "Add any pictures or links that will help illustrate what you are working on."

• remember to add links to the top of the issue if they are going to be needed again.

[ntrehan]

@JackHaeg

1. Progress: On my local I promoted myself to 'superadmin' and am able to access all features that an 'admin' can
2. No blockers however the way used to check a user's accessLevel varies drastically all across the application. Making an overhaul to that will need some time and planning. However I have tried my best to integrate a middleware solution at some places and explicit checks at others
3. I have 6 hours this week to work on the issue
4. I will have a PR ready for this before today's call (10th February 2025). And I assume the testing and PR comments will be addressed by next week. So estimated ETA is 17th February 2025
5. This is the super admin view now (Logged in and accessing all admin features)

[JackHaeg]

Thanks for this update and for all of your work, @ntrehan! I will add this to tonight's agenda to highlight the review to the team.