HuskieRobotics / SPOT

Apache License 2.0
25 stars 21 forks source link

refactor authentication and demo-mode to use middleware #120

Open gcschmit opened 2 weeks ago

gcschmit commented 2 weeks ago

Currently, every admin and setup endpoint checks for authentication. It would be cleaner if authentication was checked once with a middleware approach and all endpoints wouldn't have to worry about it. Similarly, demo-mode is checked throughout admin and scouting endpoints and that could be done in a single place using middleware.

Allybe commented 2 weeks ago

When you say "authentication" you mean having it check for the admin password once and that'd generate a token to be used across other endpoints, right? Could also mean using something like Google OAuth.

Allybe commented 2 weeks ago

And in that case we could use something like Passport.js which has token based authentication.

gcschmit commented 2 weeks ago

Good question; I didn't make this issue very clear. I think the access code-based authentication is fine, at least for now. While it isn't very secure, it guards against non-malicious mistakes. What I was trying to capture is that most of the routes in the admin and scouting endpoints check for authentication. For example:

router.get("/scouters", (req, res) => {
  if (!DEMO) {
    if (req.headers.authorization === config.secrets.ACCESS_CODE) {
      res.json(ScoutingSync.getScouters());
    } else {
      res.json({ error: "Not Authorized" });
    }
  } else {
    res.json(ScoutingSync.getScouters());
  }
});

Rather than duplicating the code to perform this check in each route, a middleware approach can be used instead. For example, something like this:

router.use((req, res, next) => {
  if (!ScoutingSync.initialized) {
    res.status(503).send("Scouting Sync not initialized yet!");
  }  else if (req.headers.authorization !== config.secrets.ACCESS_CODE && !req.path.startsWith("/auth")) {
   res.json({ error: "Not Authorized" });
}
else {
    next();
  }
});

Demo-mode could be handled in a similar fashion.