fix: Change STIGMAN_JWT_SERVICENAME_CLAIM to conform w/ standard. Don…

…'t create null user if no username claim found. (#1163) * servicename fallback, reject request if no username can be found * Add userObject to request object first * made changes as suggested by reviewer
NUWCDIVNPT · Dec 13, 2023 · b14d838 · b14d838
1 parent abd4e65
commit b14d838
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 15 deletions.
diff --git a/api/source/utils/auth.js b/api/source/utils/auth.js
@@ -7,6 +7,7 @@ const _ = require('lodash')
 const {promisify} = require('util')
 const User = require(`../service/UserService`)
 const axios = require('axios')
+const SmError = require('./error')
 
 let jwksUri
 let client
@@ -17,7 +18,7 @@ const verifyRequest = async function (req, requiredScopes, securityDefinition) {
 
         const token = getBearerToken(req)
         if (!token) {
-            throw({status: 401, message: 'OIDC bearer token must be provided'})
+            throw(new SmError.AuthorizeError("OIDC bearer token must be provided"))
         }
         const options = {
             algorithms: ['RS256']
@@ -26,11 +27,18 @@ const verifyRequest = async function (req, requiredScopes, securityDefinition) {
         req.access_token = decoded
         req.bearer = token
         req.userObject = {
-            username: decoded[config.oauth.claims.username] || decoded[config.oauth.claims.servicename] || 'null',
-            displayName: decoded[config.oauth.claims.name] || decoded[config.oauth.claims.username] || decoded[config.oauth.claims.servicename] || 'USER',
             email: decoded[config.oauth.claims.email] ||  'None Provided'
-        }
-
+        }        
+        // Get username from configured claims in token, or fall back through precedence list. 
+        const usernamePrecedence = [config.oauth.claims.username, "preferred_username", config.oauth.claims.servicename, "azp", "client_id", "clientId"]
+        req.userObject.username = decoded[usernamePrecedence.find(element => !!decoded[element])]
+        // If no username found, throw Privilege error
+        if (req.userObject.username === undefined) {
+            throw(new SmError.PrivilegeError("No token claim mappable to username found"))
+        }    
+        // Get display name from configured claim in token, or use username
+        req.userObject.displayName = decoded[config.oauth.claims.name] || req.userObject.username
+        // Check scopes
         const grantedScopes = typeof decoded[config.oauth.claims.scope] === 'string' ? 
             decoded[config.oauth.claims.scope].split(' ') : 
             decoded[config.oauth.claims.scope]
@@ -46,7 +54,7 @@ const verifyRequest = async function (req, requiredScopes, securityDefinition) {
             }
         })
         if (commonScopes.length == 0) {
-            throw({status: 403, message: 'Not in scope'})
+            throw(new SmError.PrivilegeError("Not in scope"))
         }
         else {      
             // Get privileges      
@@ -78,7 +86,7 @@ const verifyRequest = async function (req, requiredScopes, securityDefinition) {
                 }
             }
             if ('elevate' in req.query && (req.query.elevate === 'true' && !req.userObject.privileges.canAdmin)) {
-                throw({status: 403, message: 'User has insufficient privilege to complete this request.'})
+                throw(new SmError.PrivilegeError("User has insufficient privilege to complete this request."))
             }
             return true;
         }

diff --git a/api/source/utils/config.js b/api/source/utils/config.js
@@ -74,8 +74,8 @@ let config = {
         authority: process.env.STIGMAN_OIDC_PROVIDER || process.env.STIGMAN_API_AUTHORITY || "http://localhost:8080/auth/realms/stigman",
         claims: {
             scope: process.env.STIGMAN_JWT_SCOPE_CLAIM || "scope",
-            username: process.env.STIGMAN_JWT_USERNAME_CLAIM || "preferred_username",
-            servicename: process.env.STIGMAN_JWT_SERVICENAME_CLAIM || "clientId",
+            username: process.env.STIGMAN_JWT_USERNAME_CLAIM,
+            servicename: process.env.STIGMAN_JWT_SERVICENAME_CLAIM,
             name: process.env.STIGMAN_JWT_NAME_CLAIM || process.env.STIGMAN_JWT_USERNAME_CLAIM || "name",
             privileges: formatChain(process.env.STIGMAN_JWT_PRIVILEGES_CLAIM || "realm_access.roles"),
             email: process.env.STIGMAN_JWT_EMAIL_CLAIM || "email"

diff --git a/api/source/utils/error.js b/api/source/utils/error.js
@@ -11,22 +11,30 @@ class SmError extends Error {
   }
 }
 
-class PrivilegeError extends SmError {
+
+class ClientError extends SmError {
   constructor(detail) {
-    super('User has insufficient privilege to complete this request.')
-    this.status = 403
+    super('Incorrect request.')
+    this.status = 400
     this.detail = detail
   }
 }
 
-class ClientError extends SmError {
+class AuthorizeError extends SmError {
   constructor(detail) {
-    super('Incorrect request.')
-    this.status = 400
+    super('Request not authorized.')
+    this.status = 401
     this.detail = detail
   }
 }
 
+class PrivilegeError extends SmError {
+  constructor(detail) {
+    super('User has insufficient privilege to complete this request.')
+    this.status = 403
+    this.detail = detail
+  }
+}
 class NotFoundError extends SmError {
   constructor(detail) {
     super('Resource not found.')
@@ -52,6 +60,7 @@ class InternalError extends SmError {
 
 module.exports = {
   SmError,
+  AuthorizeError,  
   PrivilegeError,
   NotFoundError,
   ClientError,