Merge pull request #393 from sasjs/auth-diagram

docs(api): document the current authentication flow, comment non-obvi…

api/src/controllers/auth.ts

@@ -100,8 +100,13 @@ const token = async (data: any): Promise<TokenResponse> => {
 
   AuthController.deleteCode(userInfo.userId, clientId)
 
-  // get tokens from DB
+  // Re-exchanging a code for the same user/client while a still-valid token
+  // pair already exists returns that pair instead of minting a new one -
+  // keeps other tabs/sessions using the old tokens alive instead of
+  // silently invalidating them (saveTokensInDB below overwrites, it doesn't
+  // append).
   const existingTokens = await getTokensFromDB(userInfo.userId, clientId)
+
   if (existingTokens) {
     return {
       accessToken: existingTokens.accessToken,
@@ -109,7 +114,12 @@ const token = async (data: any): Promise<TokenResponse> => {
     }
   }
 
+  // Only used to look up token expirations - clientSecret is intentionally
+  // not checked here. The credential for this exchange is the auth code
+  // itself (single-use, 30s-lived, only obtainable by a caller that already
+  // held a valid session - see verifyAuthCode below and web.ts's authorize()).
   const client = await Client.findOne({ clientId })
+
   if (!client) throw new Error('Invalid clientId.')
 
   const accessToken = generateAccessToken(

api/src/middlewares/authenticateToken.ts

@@ -113,6 +113,9 @@ const authenticateToken = async (
 
     throw 'Unauthorized'
   } catch (error) {
+    // A missing/invalid/expired token doesn't necessarily mean 401 - if an
+    // admin has granted the built-in Public group access to this exact
+    // path, an unauthenticated caller still gets through as publicUser.
     if (await isPublicRoute(req)) {
       req.user = publicUser
       return next()

api/src/middlewares/desktop.ts

@@ -5,6 +5,11 @@ import { ModeType } from '../utils'
 
 const regexUser = /^\/SASjsApi\/user\/[0-9]*$/ // /SASjsApi/user/1
 
+// Desktop mode has no login/logout (see authenticateAccessToken's desktop
+// bypass) and every request runs as the single fixed desktopUser, but the
+// desktop UI still needs to read/update that user's own profile (e.g.
+// autoExec) - so these two routes stay reachable while every other
+// /SASLogon/* and user-management route is blocked below.
 const allowedInDesktopMode: { [key: string]: RegExp[] } = {
   GET: [regexUser],
   PATCH: [regexUser]

api/src/routes/api/client.ts

@@ -5,8 +5,16 @@ import { authenticateAccessToken, verifyAdmin } from '../../middlewares'
 
 const clientRouter = express.Router()
 
+// Neither this route nor the router itself declares auth middleware - both
+// POST and GET are actually gated by authenticateAccessToken/verifyAdmin
+// applied at the mount point in routes/api/index.ts (`router.use('/client',
+// ..., authenticateAccessToken, verifyAdmin, clientRouter)`), which runs for
+// every method under /client before requests reach the handlers below. GET's
+// own authenticateAccessToken/verifyAdmin a few lines down is therefore
+// redundant, not the thing actually protecting it.
 clientRouter.post('/', async (req, res) => {
   const { error, value: body } = registerClientValidation(req.body)
+
   if (error) return res.status(400).send(error.details[0].message)
 
   const controller = new ClientController()

api/src/utils/getAuthorizedRoutes.ts

@@ -2,6 +2,10 @@ import { Request } from 'express'
 
 export const TopLevelRoutes = ['/AppStream', '/SASjsApi']
 
+// Being authenticated is enough for most routes. These specifically also
+// require a granted Permission (checked by the authorize middleware) because
+// they run arbitrary submitted code or manipulate arbitrary files on disk -
+// a narrower blast radius than everything else a logged-in user can do.
 const StaticAuthorizedRoutes = [
   '/SASjsApi/code/execute',
   '/SASjsApi/stp/execute',

api/src/utils/verifyTokenInDB.ts

@@ -20,6 +20,13 @@ export const fetchLatestAutoExec = async (
   }
 }
 
+// Access/refresh tokens are JWTs, so they're self-verifying - but that
+// alone would make them impossible to revoke before expiry. Requiring the
+// exact token string to still be the one stored on the user's tokens array
+// (written by saveTokensInDB, cleared by removeTokensInDB) turns them into
+// revocable credentials: logout, or issuing a fresh pair via /auth/refresh,
+// immediately invalidates the old token even though its signature is still
+// valid.
 export const verifyTokenInDB = async (
   userId: number,
   clientId: string,