diff options
author | David Mehren | 2020-11-30 15:04:30 +0100 |
---|---|---|
committer | David Mehren | 2020-11-30 15:04:30 +0100 |
commit | cc7fa947bfb4043bd4b97b0040e82daef892f365 (patch) | |
tree | 309bb8b2bbad207a5c763216c6c48910b3701d01 /lib/web/auth | |
parent | 116fddd584a9d0927ce1109484826f6bc4074090 (diff) |
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>
Diffstat (limited to '')
-rw-r--r-- | lib/web/auth/oauth2/index.js | 31 |
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) + } } } } |