Fix crash when OAuth2 config parameters are missing

If the optional config options `config.oauth2.userProfileIdAttr` or `config.oauth2.rolesClaim` were not set, `String.split` was called on `undefined`, triggering a crash.

This commit adds handling of these cases and improves error logging in `checkAuthorization`.

Fixes #608

Signed-off-by: David Mehren <git@herrmehren.de>

1 files changed, 19 insertions, 12 deletions

diff --git a/lib/web/auth/oauth2/index.js b/lib/web/auth/oauth2/index.js
index b8e62dda..9cb17f26 100644
--- a/lib/web/auth/oauth2/index.js
+++ b/lib/web/auth/oauth2/index.js
@@ -52,7 +52,8 @@ function extractProfileAttribute (data, path) {
 }
 
 function parseProfile (data) {
-  const id = extractProfileAttribute(data, config.oauth2.userProfileIdAttr)
+  // only try to parse the id if a claim is configured
+  const id = config.oauth2.userProfileIdAttr ? extractProfileAttribute(data, config.oauth2.userProfileIdAttr) : undefined
   const username = extractProfileAttribute(data, config.oauth2.userProfileUsernameAttr)
   const displayName = extractProfileAttribute(data, config.oauth2.userProfileDisplayNameAttr)
   const email = extractProfileAttribute(data, config.oauth2.userProfileEmailAttr)
@@ -66,18 +67,24 @@ function parseProfile (data) {
 }
 
 function checkAuthorization (data, done) {
-  const roles = extractProfileAttribute(data, config.oauth2.rolesClaim)
-  const username = extractProfileAttribute(data, config.oauth2.userProfileUsernameAttr)
-
+  // a role the user must have is set in the config
   if (config.oauth2.accessRole) {
-    if (!roles) {
-      logger.error('oauth2: "accessRole" configured, but user profile doesn\'t contain roles attribute. Permission denied')
-      return done('Permission denied', null)
-    }
-
-    if (!roles.includes(config.oauth2.accessRole)) {
-      logger.debug(`oauth2: user "${username}" doesn't have the required role. Permission denied`)
-      return done('Permission denied', null)
+    // check if we know which claim contains the list of groups a user is in
+    if (!config.oauth2.rolesClaim) {
+      // log error, but accept all logins
+      logger.error('oauth2: "accessRole" is configured, but "rolesClaim" is missing from the config. Can\'t check group membership!')
+    } else {
+      // parse and check role data
+      const roles = extractProfileAttribute(data, config.oauth2.rolesClaim)
+      if (!roles) {
+        logger.error('oauth2: "accessRole" is configured, but user profile doesn\'t contain roles attribute. Permission denied')
+        return done('Permission denied', null)
+      }
+      if (!roles.includes(config.oauth2.accessRole)) {
+        const username = extractProfileAttribute(data, config.oauth2.userProfileUsernameAttr)
+        logger.debug(`oauth2: user "${username}" doesn't have the required role. Permission denied`)
+        return done('Permission denied', null)
+      }
     }
   }
 }