From da2e16e349e32a30a0943116b4d4e89d7cc63949 Mon Sep 17 00:00:00 2001 From: Jason Sherman Date: Mon, 12 Feb 2024 12:45:33 -0800 Subject: [PATCH] Initial commit for moving from Keycloak custom realm to BC Gov standard realm Signed-off-by: Jason Sherman --- app/app.js | 22 ++- app/config/custom-environment-variables.json | 9 +- app/config/default.json | 11 +- app/frontend/src/main.js | 2 +- app/frontend/src/store/auth.js | 1 + app/package-lock.json | 14 ++ app/package.json | 1 + app/src/components/jwtService.js | 89 +++++++++++ app/src/components/keycloak.js | 18 +-- app/src/forms/admin/routes.js | 5 +- app/src/forms/auth/middleware/userAccess.js | 143 +++++++++++++----- app/src/forms/auth/service.js | 16 +- app/src/forms/form/routes.js | 5 +- app/src/forms/permission/routes.js | 5 +- app/src/forms/rbac/routes.js | 9 +- app/src/forms/role/routes.js | 11 +- app/src/forms/user/routes.js | 4 +- .../forms/auth/middleware/userAccess.spec.js | 83 ++++++---- 18 files changed, 324 insertions(+), 124 deletions(-) create mode 100644 app/src/components/jwtService.js diff --git a/app/app.js b/app/app.js index f960debc5..fc6420d4c 100644 --- a/app/app.js +++ b/app/app.js @@ -5,7 +5,7 @@ const path = require('path'); const Problem = require('api-problem'); const querystring = require('querystring'); -const keycloak = require('./src/components/keycloak'); +//const keycloak = require('./src/components/keycloak'); const log = require('./src/components/log')(module.filename); const httpLogger = require('./src/components/log').httpLogger; const middleware = require('./src/forms/common/middleware'); @@ -41,7 +41,7 @@ if (process.env.NODE_ENV !== 'test') { } // Use Keycloak OIDC Middleware -app.use(keycloak.middleware()); +//app.use(keycloak.middleware()); // Block requests until service is ready app.use((_req, res, next) => { @@ -178,11 +178,16 @@ function initializeConnections() { .then((results) => { state.connections.data = results[0]; - if (state.connections.data) log.info('DataConnection Reachable', { function: 'initializeConnections' }); + if (state.connections.data) + log.info('DataConnection Reachable', { + function: 'initializeConnections', + }); }) .catch((error) => { log.error(`Initialization failed: Database OK = ${state.connections.data}`, { function: 'initializeConnections' }); - log.error('Connection initialization failure', error.message, { function: 'initializeConnections' }); + log.error('Connection initialization failure', error.message, { + function: 'initializeConnections', + }); if (!state.ready) { process.exitCode = 1; shutdown(); @@ -191,7 +196,9 @@ function initializeConnections() { .finally(() => { state.ready = Object.values(state.connections).every((x) => x); if (state.ready) { - log.info('Service ready to accept traffic', { function: 'initializeConnections' }); + log.info('Service ready to accept traffic', { + function: 'initializeConnections', + }); // Start periodic 10 second connection probe check probeId = setInterval(checkConnections, 10000); } @@ -211,7 +218,10 @@ function checkConnections() { Promise.all(tasks).then((results) => { state.connections.data = results[0]; state.ready = Object.values(state.connections).every((x) => x); - if (!wasReady && state.ready) log.info('Service ready to accept traffic', { function: 'checkConnections' }); + if (!wasReady && state.ready) + log.info('Service ready to accept traffic', { + function: 'checkConnections', + }); log.verbose(state); if (!state.ready) { process.exitCode = 1; diff --git a/app/config/custom-environment-variables.json b/app/config/custom-environment-variables.json index ca034709e..ac58b84cf 100755 --- a/app/config/custom-environment-variables.json +++ b/app/config/custom-environment-variables.json @@ -43,11 +43,12 @@ "basePath": "SERVER_BASEPATH", "bodyLimit": "SERVER_BODYLIMIT", "keycloak": { - "clientId": "SERVER_KC_CLIENTID", - "clientSecret": "SERVER_KC_CLIENTSECRET", - "publicKey": "SERVER_KC_PUBLICKEY", "realm": "SERVER_KC_REALM", - "serverUrl": "SERVER_KC_SERVERURL" + "serverUrl": "SERVER_KC_SERVERURL", + "jwksUri": "SERVER_KC_JWKSURI", + "issuer": "SERVER_KC_ISSUER", + "audience": "SERVER_KC_AUDIENCE", + "maxTokenAge": "SERVER_KC_MAXTOKENAGE" }, "logFile": "SERVER_LOGFILE", "logLevel": "SERVER_LOGLEVEL", diff --git a/app/config/default.json b/app/config/default.json index 97491f8ac..fe4303825 100644 --- a/app/config/default.json +++ b/app/config/default.json @@ -32,7 +32,7 @@ "basePath": "/app", "keycloak": { "clientId": "chefs-frontend", - "realm": "chefs", + "realm": "standard", "serverUrl": "https://dev.loginproxy.gov.bc.ca/auth" } }, @@ -41,9 +41,12 @@ "basePath": "/app", "bodyLimit": "30mb", "keycloak": { - "clientId": "chefs", - "realm": "chefs", - "serverUrl": "https://dev.loginproxy.gov.bc.ca/auth" + "realm": "standard", + "serverUrl": "https://dev.loginproxy.gov.bc.ca/auth", + "jwksUri": "https://dev.loginproxy.gov.bc.ca/auth/realms/standard/protocol/openid-connect/certs", + "issuer": "https://dev.loginproxy.gov.bc.ca/auth/realms/standard", + "audience": "chefs-frontend", + "maxTokenAge": "300" }, "logLevel": "http", "port": "8080", diff --git a/app/frontend/src/main.js b/app/frontend/src/main.js index fd75a2651..18d1ef2be 100755 --- a/app/frontend/src/main.js +++ b/app/frontend/src/main.js @@ -139,7 +139,7 @@ function loadKeycloak(config) { }; const options = Object.assign({}, defaultParams, { - init: { onLoad: 'check-sso' }, + init: { pkceMethod: 'S256', checkLoginIframe: false, onLoad: 'check-sso' }, config: { clientId: config.keycloak.clientId, realm: config.keycloak.realm, diff --git a/app/frontend/src/store/auth.js b/app/frontend/src/store/auth.js index ca2d1b25e..a234ab932 100644 --- a/app/frontend/src/store/auth.js +++ b/app/frontend/src/store/auth.js @@ -102,6 +102,7 @@ export const useAuthStore = defineStore('auth', { if (options.idpHint) { // Redirect to Keycloak if idpHint is available + //this.keycloak.login(options); window.location.replace(this.createLoginUrl(options)); } else { // Navigate to internal login page if no idpHint specified diff --git a/app/package-lock.json b/app/package-lock.json index 39e45278c..b16376c7d 100644 --- a/app/package-lock.json +++ b/app/package-lock.json @@ -30,6 +30,7 @@ "form-data": "^4.0.0", "fs-extra": "^10.1.0", "handlebars": "^4.7.8", + "jose": "^5.2.1", "js-yaml": "^4.1.0", "jsonwebtoken": "^9.0.0", "keycloak-connect": "^21.1.1", @@ -7853,6 +7854,14 @@ "node": ">= 0.6.0" } }, + "node_modules/jose": { + "version": "5.2.1", + "resolved": "https://registry.npmjs.org/jose/-/jose-5.2.1.tgz", + "integrity": "sha512-qiaQhtQRw6YrOaOj0v59h3R6hUY9NvxBmmnMfKemkqYmBB0tEc97NbLP7ix44VP5p9/0YHG8Vyhzuo5YBNwviA==", + "funding": { + "url": "https://github.com/sponsors/panva" + } + }, "node_modules/js-sdsl": { "version": "4.4.0", "resolved": "https://registry.npmjs.org/js-sdsl/-/js-sdsl-4.4.0.tgz", @@ -17109,6 +17118,11 @@ "resolved": "https://registry.npmjs.org/jmespath/-/jmespath-0.16.0.tgz", "integrity": "sha512-9FzQjJ7MATs1tSpnco1K6ayiYE3figslrXA72G2HQ/n76RzvYlofyi5QM+iX4YRs/pu3yzxlVQSST23+dMDknw==" }, + "jose": { + "version": "5.2.1", + "resolved": "https://registry.npmjs.org/jose/-/jose-5.2.1.tgz", + "integrity": "sha512-qiaQhtQRw6YrOaOj0v59h3R6hUY9NvxBmmnMfKemkqYmBB0tEc97NbLP7ix44VP5p9/0YHG8Vyhzuo5YBNwviA==" + }, "js-sdsl": { "version": "4.4.0", "resolved": "https://registry.npmjs.org/js-sdsl/-/js-sdsl-4.4.0.tgz", diff --git a/app/package.json b/app/package.json index 7bbebb39d..f87994c05 100644 --- a/app/package.json +++ b/app/package.json @@ -68,6 +68,7 @@ "form-data": "^4.0.0", "fs-extra": "^10.1.0", "handlebars": "^4.7.8", + "jose": "^5.2.1", "js-yaml": "^4.1.0", "jsonwebtoken": "^9.0.0", "keycloak-connect": "^21.1.1", diff --git a/app/src/components/jwtService.js b/app/src/components/jwtService.js new file mode 100644 index 000000000..1aec105e9 --- /dev/null +++ b/app/src/components/jwtService.js @@ -0,0 +1,89 @@ +const jose = require('jose'); +const config = require('config'); +const errorToProblem = require('./errorToProblem'); + +const SERVICE = 'JwtService'; + +const jwksUri = config.get('server.keycloak.jwksUri'); + +// Create a remote JWK set that fetches the JWK set from server with caching +const JWKS = jose.createRemoteJWKSet(new URL(jwksUri)); + +class JwtService { + constructor({ issuer, audience, maxTokenAge }) { + if (!issuer || !audience || !maxTokenAge) { + throw new Error('JwtService is not configured. Check configuration.'); + } + + this.audience = audience; + this.issuer = issuer; + this.maxTokenAge = maxTokenAge; + } + + getBearerToken(req) { + if (req.headers && req.headers.authorization && req.headers.authorization.startsWith('Bearer ')) { + return req.headers.authorization.substring(7); + } + // do we want to throw errors? + return null; + } + + async getTokenPayload(req) { + const bear = this.getBearerToken(req); + if (bear) { + return await this._verify(bear); + } + return null; + } + + async _verify(token) { + // could throw JWTClaimValidationFailed + const { payload } = await jose.jwtVerify(token, JWKS, { + issuer: this.issuer, + audience: this.audience, + maxTokenAge: parseInt(this.maxTokenAge), + }); + return payload; + } + + async validateAccessToken(token) { + try { + await this._verify(token); + // these claims passed, just return true. + return true; + } catch (e) { + if (e instanceof jose.errors.JWTClaimValidationFailed) { + return false; + } else { + errorToProblem(SERVICE, e); + } + } + } + + protect(spec) { + // actual middleware + return async (req, res, next) => { + // get token, check if valid + const token = this.getBearerToken(req); + if (token) { + const payload = await this._verify(token); + if (spec && !payload.roles.includes(spec)) { + // todo: fix logic to prevent access + next(); + } + } + next(); + }; + } +} + +const audience = config.get('server.keycloak.audience'); +const issuer = config.get('server.keycloak.issuer'); +const maxTokenAge = config.get('server.keycloak.maxTokenAge'); + +let jwtService = new JwtService({ + issuer: issuer, + audience: audience, + maxTokenAge: maxTokenAge, +}); +module.exports = jwtService; diff --git a/app/src/components/keycloak.js b/app/src/components/keycloak.js index 3781b3354..71e41f854 100755 --- a/app/src/components/keycloak.js +++ b/app/src/components/keycloak.js @@ -1,19 +1,3 @@ -const config = require('config'); const Keycloak = require('keycloak-connect'); -module.exports = new Keycloak( - {}, - { - bearerOnly: true, - 'confidential-port': 0, - clientId: config.get('server.keycloak.clientId'), - 'policy-enforcer': {}, - realm: config.get('server.keycloak.realm'), - realmPublicKey: config.has('server.keycloak.publicKey') ? config.get('server.keycloak.publicKey') : undefined, - secret: config.get('server.keycloak.clientSecret'), - serverUrl: config.get('server.keycloak.serverUrl'), - 'ssl-required': 'external', - 'use-resource-role-mappings': true, - 'verify-token-audience': false, - } -); +module.exports = new Keycloak({}, {}); diff --git a/app/src/forms/admin/routes.js b/app/src/forms/admin/routes.js index 735a7f5b8..47766a7e8 100644 --- a/app/src/forms/admin/routes.js +++ b/app/src/forms/admin/routes.js @@ -1,14 +1,13 @@ -const config = require('config'); const routes = require('express').Router(); const currentUser = require('../auth/middleware/userAccess').currentUser; const controller = require('./controller'); const userController = require('../user/controller'); -const keycloak = require('../../components/keycloak'); +const jwtService = require('../../components/jwtService'); // Always have this applied to all routes here -routes.use(keycloak.protect(`${config.get('server.keycloak.clientId')}:admin`)); +routes.use(jwtService.protect('admin')); routes.use(currentUser); // Routes under the /admin pathing will fetch data without doing Form permission checks in the database diff --git a/app/src/forms/auth/middleware/userAccess.js b/app/src/forms/auth/middleware/userAccess.js index 1f622cdbb..800bd942a 100644 --- a/app/src/forms/auth/middleware/userAccess.js +++ b/app/src/forms/auth/middleware/userAccess.js @@ -1,23 +1,15 @@ const Problem = require('api-problem'); const { validate } = require('uuid'); -const keycloak = require('../../../components/keycloak'); +const jwtService = require('../../../components/jwtService'); const Permissions = require('../../common/constants').Permissions; const Roles = require('../../common/constants').Roles; const service = require('../service'); const rbacService = require('../../rbac/service'); -const getToken = (req) => { - try { - return req.kauth.grant.access_token; - } catch (err) { - return null; - } -}; - const setUser = async (req, _res, next) => { try { - const token = getToken(req); + const token = await jwtService.getTokenPayload(req); req.currentUser = await service.login(token); next(); } catch (error) { @@ -27,12 +19,14 @@ const setUser = async (req, _res, next) => { const currentUser = async (req, res, next) => { // Check if authorization header is a bearer token - if (req.headers && req.headers.authorization && req.headers.authorization.startsWith('Bearer ')) { + const token = jwtService.getBearerToken(req); + if (token) { // need to check keycloak, ensure the bearer token is valid - const token = req.headers.authorization.substring(7); - const ok = await keycloak.grantManager.validateAccessToken(token); + const ok = await jwtService.validateAccessToken(token); if (!ok) { - return new Problem(403, { detail: 'Authorization token is invalid.' }).send(res); + return new Problem(403, { + detail: 'Authorization token is invalid.', + }).send(res); } } @@ -40,11 +34,17 @@ const currentUser = async (req, res, next) => { }; const _getForm = async (currentUser, formId) => { - const forms = await service.getUserForms(currentUser, { active: true, formId: formId }); + const forms = await service.getUserForms(currentUser, { + active: true, + formId: formId, + }); let form = forms.find((f) => f.formId === formId); if (!form) { - const deletedForms = await service.getUserForms(currentUser, { active: false, formId: formId }); + const deletedForms = await service.getUserForms(currentUser, { + active: false, + formId: formId, + }); form = deletedForms.find((f) => f.formId === formId); } @@ -60,7 +60,9 @@ const hasFormPermissions = (permissions) => { if (!req.currentUser) { // cannot find the currentUser... guess we don't have access... FAIL! - return new Problem(401, { detail: 'Current user not found on request.' }).send(res); + return new Problem(401, { + detail: 'Current user not found on request.', + }).send(res); } // If we invoke this middleware and the caller is acting on a specific formId, whether in a param or query (precedence to param) const formId = req.params.formId || req.query.formId; @@ -71,7 +73,9 @@ const hasFormPermissions = (permissions) => { let form = await _getForm(req.currentUser, formId); if (!form) { // cannot find the form... guess we don't have access... FAIL! - return new Problem(401, { detail: 'Current user has no access to form.' }).send(res); + return new Problem(401, { + detail: 'Current user has no access to form.', + }).send(res); } if (!Array.isArray(permissions)) { @@ -83,7 +87,9 @@ const hasFormPermissions = (permissions) => { }); if (intersection.length !== permissions.length) { - return new Problem(401, { detail: 'Current user does not have required permission(s) on form' }).send(res); + return new Problem(401, { + detail: 'Current user does not have required permission(s) on form', + }).send(res); } else { return next(); } @@ -114,7 +120,10 @@ const hasSubmissionPermissions = (permissions) => { // Does the user have permissions for this submission due to their FORM permissions if (req.currentUser) { - const forms = await service.getUserForms(req.currentUser, { active: true, formId: submissionForm.form.id }); + const forms = await service.getUserForms(req.currentUser, { + active: true, + formId: submissionForm.form.id, + }); let formFromCurrentUser = forms.find((f) => f.formId === submissionForm.form.id); if (formFromCurrentUser) { // Do they have the submission permissions being requested on this FORM @@ -130,7 +139,11 @@ const hasSubmissionPermissions = (permissions) => { // Deleted submissions are inaccessible if (submissionForm.submission.deleted) { - return next(new Problem(401, { detail: 'You do not have access to this submission.' })); + return next( + new Problem(401, { + detail: 'You do not have access to this submission.', + }) + ); } // TODO: consider whether DRAFT submissions are restricted as deleted above @@ -146,7 +159,11 @@ const hasSubmissionPermissions = (permissions) => { if (submissionPermission) return next(); // no access to this submission... - return next(new Problem(401, { detail: 'You do not have access to this submission.' })); + return next( + new Problem(401, { + detail: 'You do not have access to this submission.', + }) + ); } catch (error) { next(error); } @@ -180,7 +197,11 @@ const filterMultipleSubmissions = () => { //validate all submission ids const isValidSubmissionId = submissionIds.every((submissionId) => validate(submissionId)); if (!isValidSubmissionId) { - return next(new Problem(401, { detail: 'Invalid submissionId(s) in the submissionIds list.' })); + return next( + new Problem(401, { + detail: 'Invalid submissionId(s) in the submissionIds list.', + }) + ); } if (formIdWithDeletePermission === formId) { @@ -189,11 +210,19 @@ const filterMultipleSubmissions = () => { const isForeignSubmissionId = metaData.every((SubmissionMetadata) => SubmissionMetadata.formId === formId); if (!isForeignSubmissionId || metaData.length !== submissionIds.length) { - return next(new Problem(401, { detail: 'Current user does not have required permission(s) for some submissions in the submissionIds list.' })); + return next( + new Problem(401, { + detail: 'Current user does not have required permission(s) for some submissions in the submissionIds list.', + }) + ); } return next(); } - return next(new Problem(401, { detail: 'Current user does not have required permission(s) for to delete submissions' })); + return next( + new Problem(401, { + detail: 'Current user does not have required permission(s) for to delete submissions', + }) + ); } catch (error) { next(error); } @@ -203,7 +232,10 @@ const filterMultipleSubmissions = () => { const hasFormRole = async (formId, user, role) => { let hasRole = false; - const forms = await service.getUserForms(user, { active: true, formId: formId }); + const forms = await service.getUserForms(user, { + active: true, + formId: formId, + }); const form = forms.find((f) => f.formId === formId); if (form) { @@ -227,7 +259,10 @@ const hasFormRoles = (formRoles, hasAll = false) => { return new Problem(401, { detail: 'Form Id not found on request.' }).send(res); } - const forms = await service.getUserForms(req.currentUser, { active: true, formId: formId }); + const forms = await service.getUserForms(req.currentUser, { + active: true, + formId: formId, + }); const form = forms.find((f) => f.formId === formId); if (form) { for (let roleIndex = 0; roleIndex < form.roles.length; roleIndex++) { @@ -247,10 +282,19 @@ const hasFormRoles = (formRoles, hasAll = false) => { } if (hasAll) { - if (formRoles.length > 0) return next(new Problem(401, { detail: 'You do not have permission to update this role.' })); + if (formRoles.length > 0) + return next( + new Problem(401, { + detail: 'You do not have permission to update this role.', + }) + ); else return next(); } - return next(new Problem(401, { detail: 'You do not have permission to update this role.' })); + return next( + new Problem(401, { + detail: 'You do not have permission to update this role.', + }) + ); }; }; @@ -261,7 +305,9 @@ const hasRolePermissions = (removingUsers = false) => { const formId = req.params.formId || req.query.formId; if (!formId) { // No form provided to this route that secures based on form... that's a problem! - return new Problem(401, { detail: 'Form Id not found on request.' }).send(res); + return new Problem(401, { + detail: 'Form Id not found on request.', + }).send(res); } const currentUser = req.currentUser; @@ -270,7 +316,12 @@ const hasRolePermissions = (removingUsers = false) => { const isOwner = await hasFormRole(formId, currentUser, Roles.OWNER); if (removingUsers) { - if (data.includes(currentUser.id)) return next(new Problem(401, { detail: "You can't remove yourself from this form." })); + if (data.includes(currentUser.id)) + return next( + new Problem(401, { + detail: "You can't remove yourself from this form.", + }) + ); if (!isOwner) { for (let i = 0; i < data.length; i++) { @@ -280,12 +331,20 @@ const hasRolePermissions = (removingUsers = false) => { // Can't update another user's roles if they are an owner if (userRoles.some((fru) => fru.role === Roles.OWNER) && userId !== currentUser.id) { - return next(new Problem(401, { detail: "You can not update an owner's roles." })); + return next( + new Problem(401, { + detail: "You can not update an owner's roles.", + }) + ); } // If the user is trying to remove the designer role if (userRoles.some((fru) => fru.role === Roles.FORM_DESIGNER)) { - return next(new Problem(401, { detail: "You can't remove a form designer role." })); + return next( + new Problem(401, { + detail: "You can't remove a form designer role.", + }) + ); } } } @@ -300,7 +359,11 @@ const hasRolePermissions = (removingUsers = false) => { // If the user is trying to remove the team manager role for their own userid if (userRoles.some((fru) => fru.role === Roles.TEAM_MANAGER) && !data.some((role) => role.role === Roles.TEAM_MANAGER) && userId == currentUser.id) { - return next(new Problem(401, { detail: "You can't remove your own team manager role." })); + return next( + new Problem(401, { + detail: "You can't remove your own team manager role.", + }) + ); } // Can't update another user's roles if they are an owner @@ -313,10 +376,18 @@ const hasRolePermissions = (removingUsers = false) => { // If the user is trying to remove the designer role for another userid if (userRoles.some((fru) => fru.role === Roles.FORM_DESIGNER) && !data.some((role) => role.role === Roles.FORM_DESIGNER)) { - return next(new Problem(401, { detail: "You can't remove a form designer role." })); + return next( + new Problem(401, { + detail: "You can't remove a form designer role.", + }) + ); } if (!userRoles.some((fru) => fru.role === Roles.FORM_DESIGNER) && data.some((role) => role.role === Roles.FORM_DESIGNER)) { - return next(new Problem(401, { detail: "You can't add a form designer role." })); + return next( + new Problem(401, { + detail: "You can't add a form designer role.", + }) + ); } } } diff --git a/app/src/forms/auth/service.js b/app/src/forms/auth/service.js index df212e6fe..5608066ee 100644 --- a/app/src/forms/auth/service.js +++ b/app/src/forms/auth/service.js @@ -67,20 +67,20 @@ const service = { try { // identity_provider_* will be undefined if user login is to local keycloak (userid/password) const { - idp_userid: idpUserId, - idp_username: identity, + idir_user_guid: idpUserId, + idir_username: identity, identity_provider: idp, preferred_username: username, given_name: firstName, family_name: lastName, - sub: keycloakId, + idir_user_guid: keycloakId, name: fullName, email, - } = token.content; + } = token; return { idpUserId: idpUserId, - keycloakId: keycloakId, + keycloakId: keycloakId.replace(/([0-z]{8})([0-z]{4})([0-z]{4})([0-z]{4})([0-z]{12})/, '$1-$2-$3-$4-$5'), username: identity ? identity : username, firstName: firstName, lastName: lastName, @@ -124,7 +124,11 @@ const service = { } // return with the db id... - return { id: user.id, usernameIdp: user.idpCode ? `${user.username}@${user.idpCode}` : user.username, ...userInfo }; + return { + id: user.id, + usernameIdp: user.idpCode ? `${user.username}@${user.idpCode}` : user.username, + ...userInfo, + }; }, getUserForms: async (userInfo, params = {}) => { diff --git a/app/src/forms/form/routes.js b/app/src/forms/form/routes.js index af62c7589..c395a89a4 100644 --- a/app/src/forms/form/routes.js +++ b/app/src/forms/form/routes.js @@ -1,16 +1,15 @@ -const config = require('config'); const routes = require('express').Router(); const apiAccess = require('../auth/middleware/apiAccess'); const { currentUser, hasFormPermissions } = require('../auth/middleware/userAccess'); const P = require('../common/constants').Permissions; const rateLimiter = require('../common/middleware').apiKeyRateLimiter; -const keycloak = require('../../components/keycloak'); +const jwtService = require('../../components/jwtService'); const controller = require('./controller'); routes.use(currentUser); -routes.get('/', keycloak.protect(`${config.get('server.keycloak.clientId')}:admin`), async (req, res, next) => { +routes.get('/', jwtService.protect('admin'), async (req, res, next) => { await controller.listForms(req, res, next); }); diff --git a/app/src/forms/permission/routes.js b/app/src/forms/permission/routes.js index bd9ad9b13..83627e4ff 100644 --- a/app/src/forms/permission/routes.js +++ b/app/src/forms/permission/routes.js @@ -1,12 +1,11 @@ -const config = require('config'); const routes = require('express').Router(); const currentUser = require('../auth/middleware/userAccess').currentUser; const controller = require('./controller'); -const keycloak = require('../../components/keycloak'); +const jwtService = require('../../components/jwtService'); -routes.use(keycloak.protect(`${config.get('server.keycloak.clientId')}:admin`)); +routes.use(jwtService.protect('admin')); routes.use(currentUser); routes.get('/', async (req, res, next) => { diff --git a/app/src/forms/rbac/routes.js b/app/src/forms/rbac/routes.js index 35c463f3b..70002e0b4 100644 --- a/app/src/forms/rbac/routes.js +++ b/app/src/forms/rbac/routes.js @@ -1,19 +1,18 @@ -const config = require('config'); const routes = require('express').Router(); const controller = require('./controller'); -const keycloak = require('../../components/keycloak'); +const jwtService = require('../../components/jwtService'); const P = require('../common/constants').Permissions; const R = require('../common/constants').Roles; const { currentUser, hasFormPermissions, hasSubmissionPermissions, hasFormRoles, hasRolePermissions } = require('../auth/middleware/userAccess'); routes.use(currentUser); -routes.get('/current', keycloak.protect(), async (req, res, next) => { +routes.get('/current', jwtService.protect(), async (req, res, next) => { await controller.getCurrentUser(req, res, next); }); -routes.get('/current/submissions', keycloak.protect(), async (req, res, next) => { +routes.get('/current/submissions', jwtService.protect(), async (req, res, next) => { await controller.getCurrentUserSubmissions(req, res, next); }); @@ -37,7 +36,7 @@ routes.put('/submissions', hasSubmissionPermissions(P.SUBMISSION_UPDATE), async await controller.setSubmissionUserPermissions(req, res, next); }); -routes.get('/users', keycloak.protect(`${config.get('server.keycloak.clientId')}:admin`), async (req, res, next) => { +routes.get('/users', jwtService.protect('admin'), async (req, res, next) => { await controller.getUserForms(req, res, next); }); diff --git a/app/src/forms/role/routes.js b/app/src/forms/role/routes.js index 4042f8943..4bb8c83cd 100644 --- a/app/src/forms/role/routes.js +++ b/app/src/forms/role/routes.js @@ -1,26 +1,25 @@ -const config = require('config'); const routes = require('express').Router(); const currentUser = require('../auth/middleware/userAccess').currentUser; const controller = require('./controller'); -const keycloak = require('../../components/keycloak'); +const jwtService = require('../../components/jwtService'); routes.use(currentUser); -routes.get('/', keycloak.protect(), async (req, res, next) => { +routes.get('/', jwtService.protect(), async (req, res, next) => { await controller.list(req, res, next); }); -routes.post('/', keycloak.protect(`${config.get('server.keycloak.clientId')}:admin`), async (req, res, next) => { +routes.post('/', jwtService.protect('admin'), async (req, res, next) => { await controller.create(req, res, next); }); -routes.get('/:code', keycloak.protect(), async (req, res, next) => { +routes.get('/:code', jwtService.protect(), async (req, res, next) => { await controller.read(req, res, next); }); -routes.put('/:code', keycloak.protect(`${config.get('server.keycloak.clientId')}:admin`), async (req, res, next) => { +routes.put('/:code', jwtService.protect('admin'), async (req, res, next) => { await controller.update(req, res, next); }); diff --git a/app/src/forms/user/routes.js b/app/src/forms/user/routes.js index 2dd90ca4a..032cd6513 100644 --- a/app/src/forms/user/routes.js +++ b/app/src/forms/user/routes.js @@ -2,9 +2,9 @@ const routes = require('express').Router(); const controller = require('./controller'); const currentUser = require('../auth/middleware/userAccess').currentUser; -const keycloak = require('../../components/keycloak'); +const jwtService = require('../../components/jwtService'); -routes.use(keycloak.protect()); +routes.use(jwtService.protect()); routes.use(currentUser); // diff --git a/app/tests/unit/forms/auth/middleware/userAccess.spec.js b/app/tests/unit/forms/auth/middleware/userAccess.spec.js index a3bbd5f75..fd81ccc93 100644 --- a/app/tests/unit/forms/auth/middleware/userAccess.spec.js +++ b/app/tests/unit/forms/auth/middleware/userAccess.spec.js @@ -3,16 +3,10 @@ const Problem = require('api-problem'); const { currentUser, hasFormPermissions, hasSubmissionPermissions, hasFormRoles, hasRolePermissions } = require('../../../../../src/forms/auth/middleware/userAccess'); -const keycloak = require('../../../../../src/components/keycloak'); +const jwtService = require('../../../../../src/components/jwtService'); const service = require('../../../../../src/forms/auth/service'); const rbacService = require('../../../../../src/forms/rbac/service'); -const kauth = { - grant: { - access_token: 'fsdfhsd08f0283hr', - }, -}; - const userId = 'c6455376-382c-439d-a811-0381a012d695'; const userId2 = 'c6455376-382c-439d-a811-0381a012d696'; const formId = 'c6455376-382c-439d-a811-0381a012d697'; @@ -26,7 +20,9 @@ const Roles = { }; // Mock the token validation in the KC lib -keycloak.grantManager.validateAccessToken = jest.fn().mockReturnValue('yeah ok'); +jwtService.validateAccessToken = jest.fn().mockReturnValue(true); +jwtService.getBearerToken = jest.fn().mockReturnValue('bearer-token-value'); +jwtService.getTokenPayload = jest.fn().mockReturnValue({ token: 'payload' }); // Mock the service login const mockUser = { user: 'me' }; @@ -50,16 +46,16 @@ describe('currentUser', () => { headers: { authorization: 'Bearer hjvds0uds', }, - kauth: kauth, }; const nxt = jest.fn(); await currentUser(testReq, testRes, nxt); - expect(keycloak.grantManager.validateAccessToken).toHaveBeenCalledTimes(1); - expect(keycloak.grantManager.validateAccessToken).toHaveBeenCalledWith('hjvds0uds'); + expect(jwtService.validateAccessToken).toHaveBeenCalledTimes(1); + expect(jwtService.getBearerToken).toHaveBeenCalledTimes(1); + expect(jwtService.validateAccessToken).toHaveBeenCalledWith('bearer-token-value'); expect(service.login).toHaveBeenCalledTimes(1); - expect(service.login).toHaveBeenCalledWith(kauth.grant.access_token); + expect(service.login).toHaveBeenCalledWith({ token: 'payload' }); expect(testReq.currentUser).toEqual(mockUser); expect(nxt).toHaveBeenCalledTimes(1); expect(nxt).toHaveBeenCalledWith(); @@ -76,11 +72,12 @@ describe('currentUser', () => { headers: { authorization: 'Bearer hjvds0uds', }, - kauth: kauth, }; await currentUser(testReq, testRes, jest.fn()); - expect(service.login).toHaveBeenCalledWith(kauth.grant.access_token); + expect(jwtService.getBearerToken).toHaveBeenCalledTimes(1); + expect(jwtService.getTokenPayload).toHaveBeenCalledTimes(1); + expect(service.login).toHaveBeenCalledWith({ token: 'payload' }); }); it('uses the query param if both if that is whats provided', async () => { @@ -91,11 +88,12 @@ describe('currentUser', () => { headers: { authorization: 'Bearer hjvds0uds', }, - kauth: kauth, }; await currentUser(testReq, testRes, jest.fn()); - expect(service.login).toHaveBeenCalledWith(kauth.grant.access_token); + expect(jwtService.getBearerToken).toHaveBeenCalledTimes(1); + expect(jwtService.getTokenPayload).toHaveBeenCalledTimes(1); + expect(service.login).toHaveBeenCalledWith({ token: 'payload' }); }); it('403s if the token is invalid', async () => { @@ -106,27 +104,33 @@ describe('currentUser', () => { }; const nxt = jest.fn(); - keycloak.grantManager.validateAccessToken = jest.fn().mockReturnValue(undefined); + jwtService.validateAccessToken = jest.fn().mockReturnValue(false); await currentUser(testReq, testRes, nxt); - expect(keycloak.grantManager.validateAccessToken).toHaveBeenCalledTimes(1); - expect(keycloak.grantManager.validateAccessToken).toHaveBeenCalledWith('hjvds0uds'); + expect(jwtService.getBearerToken).toHaveBeenCalledTimes(1); + expect(jwtService.validateAccessToken).toHaveBeenCalledTimes(1); + expect(jwtService.validateAccessToken).toHaveBeenCalledWith('bearer-token-value'); expect(service.login).toHaveBeenCalledTimes(0); expect(testReq.currentUser).toEqual(undefined); expect(nxt).toHaveBeenCalledTimes(0); - // expect(nxt).toHaveBeenCalledWith(new Problem(403, { detail: 'Authorization token is invalid.' })); + //expect(nxt).toHaveBeenCalledWith(new Problem(403, { detail: 'Authorization token is invalid.' })); }); }); describe('getToken', () => { - it('returns a null token if no kauth in the request', async () => { + it('returns a null token if no auth bearer in the headers', async () => { const testReq = { params: { formId: 2, }, }; + jwtService.getBearerToken = jest.fn().mockReturnValue(null); + jwtService.getTokenPayload = jest.fn().mockReturnValue(null); + await currentUser(testReq, testRes, jest.fn()); + expect(jwtService.getBearerToken).toHaveBeenCalledTimes(1); + expect(jwtService.getTokenPayload).toHaveBeenCalledTimes(1); expect(service.login).toHaveBeenCalledTimes(1); expect(service.login).toHaveBeenCalledWith(null); }); @@ -513,7 +517,10 @@ describe('hasSubmissionPermissions', () => { it('falls through to the query if the current user does not have any FORM access on the current form', async () => { service.getSubmissionForm = jest.fn().mockReturnValue({ submission: { deleted: false }, - form: { id: '999', identityProviders: [{ code: 'idir' }, { code: 'bceid' }] }, + form: { + id: '999', + identityProviders: [{ code: 'idir' }, { code: 'bceid' }], + }, }); service.checkSubmissionPermission = jest.fn().mockReturnValue(undefined); @@ -688,7 +695,11 @@ describe('hasFormRoles', () => { await hfr(req, testRes, nxt); expect(nxt).toHaveBeenCalledTimes(1); - expect(nxt).toHaveBeenCalledWith(new Problem(401, { detail: 'You do not have permission to update this role.' })); + expect(nxt).toHaveBeenCalledWith( + new Problem(401, { + detail: 'You do not have permission to update this role.', + }) + ); }); it('falls through if the current user does not have all of the required form roles', async () => { @@ -706,7 +717,11 @@ describe('hasFormRoles', () => { await hfr(req, testRes, nxt); expect(nxt).toHaveBeenCalledTimes(1); - expect(nxt).toHaveBeenCalledWith(new Problem(401, { detail: 'You do not have permission to update this role.' })); + expect(nxt).toHaveBeenCalledWith( + new Problem(401, { + detail: 'You do not have permission to update this role.', + }) + ); }); it('moves on if the user has at least one of the required form roles', async () => { @@ -975,7 +990,7 @@ describe('hasRolePermissions', () => { updatedAt: '', }, { - id: '5', + id: '6', role: Roles.FORM_SUBMITTER, formId: formId, userId: userId2, @@ -1071,7 +1086,11 @@ describe('hasRolePermissions', () => { await hrp(req, testRes, nxt); expect(nxt).toHaveBeenCalledTimes(1); - expect(nxt).toHaveBeenCalledWith(new Problem(401, { detail: "You can't remove your own team manager role." })); + expect(nxt).toHaveBeenCalledWith( + new Problem(401, { + detail: "You can't remove your own team manager role.", + }) + ); }); }); @@ -1332,7 +1351,11 @@ describe('hasRolePermissions', () => { await hrp(req, testRes, nxt); expect(nxt).toHaveBeenCalledTimes(1); - expect(nxt).toHaveBeenCalledWith(new Problem(401, { detail: "You can't add a form designer role." })); + expect(nxt).toHaveBeenCalledWith( + new Problem(401, { + detail: "You can't add a form designer role.", + }) + ); }); it('falls through if trying to remove a designer role', async () => { @@ -1401,7 +1424,11 @@ describe('hasRolePermissions', () => { await hrp(req, testRes, nxt); expect(nxt).toHaveBeenCalledTimes(1); - expect(nxt).toHaveBeenCalledWith(new Problem(401, { detail: "You can't remove a form designer role." })); + expect(nxt).toHaveBeenCalledWith( + new Problem(401, { + detail: "You can't remove a form designer role.", + }) + ); }); it('should succeed when adding a manager/reviewer/submitter roles', async () => {