From ce2938b7155c62030b3394b699df11b1934aacf1 Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Fri, 14 Oct 2022 15:49:23 +0200 Subject: [PATCH 01/14] fix(sw): return !ok navigation responses instead of hitting precache --- pwa/src/service-worker/service-worker.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pwa/src/service-worker/service-worker.js b/pwa/src/service-worker/service-worker.js index 6f934f6ba..25c8d0b6a 100644 --- a/pwa/src/service-worker/service-worker.js +++ b/pwa/src/service-worker/service-worker.js @@ -99,9 +99,10 @@ export function setUpServiceWorker() { const navigationRouteHandler = ({ request }) => { return fetch(request) .then((response) => { - if (response.type === 'opaqueredirect') { - // It's sending a redirect to the login page. Return - // that to the client + if (response.type === 'opaqueredirect' || !response.ok) { + // It's sending a redirect to the login page, + // or an 'unauthorized'/'forbidden' response. + // Return that to the client return response } @@ -109,7 +110,7 @@ export function setUpServiceWorker() { return matchPrecache(indexUrl) }) .catch(() => { - // Request failed (maybe offline). Return cached response + // Request failed (probably offline). Return cached response return matchPrecache(indexUrl) }) } From b5dcd5fea08375f2f549200fcb732fd47042062d Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Fri, 14 Oct 2022 15:49:50 +0200 Subject: [PATCH 02/14] fix(sw): don't use SWR on .json and .action requests --- pwa/src/service-worker/service-worker.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pwa/src/service-worker/service-worker.js b/pwa/src/service-worker/service-worker.js index 25c8d0b6a..7f134f773 100644 --- a/pwa/src/service-worker/service-worker.js +++ b/pwa/src/service-worker/service-worker.js @@ -144,7 +144,8 @@ export function setUpServiceWorker() { ({ url }) => PRODUCTION_ENV && urlMeetsAppShellCachingCriteria(url) && - fileExtensionRegexp.test(url.pathname), + fileExtensionRegexp.test(url.pathname) && + !/\.(json|action)/.test(url.pathname), // don't SWR these new StaleWhileRevalidate({ cacheName: 'other-assets' }) ) From 858546cd3e8a9d421d9f388b214c1c3830e18ea9 Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Fri, 14 Oct 2022 17:48:54 +0200 Subject: [PATCH 03/14] feat(adapter): check user app authorities before rendering app --- adapter/src/components/AppWrapper.js | 17 +++-- adapter/src/components/AuthBoundary.js | 95 ++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 7 deletions(-) create mode 100644 adapter/src/components/AuthBoundary.js diff --git a/adapter/src/components/AppWrapper.js b/adapter/src/components/AppWrapper.js index 341d97a44..93019101f 100644 --- a/adapter/src/components/AppWrapper.js +++ b/adapter/src/components/AppWrapper.js @@ -3,6 +3,7 @@ import React from 'react' import { useCurrentUserLocale } from '../utils/useLocale.js' import { useVerifyLatestUser } from '../utils/useVerifyLatestUser.js' import { Alerts } from './Alerts.js' +import { AuthBoundary } from './AuthBoundary.js' import { ConnectedHeaderBar } from './ConnectedHeaderBar.js' import { ErrorBoundary } from './ErrorBoundary.js' import { LoadingMask } from './LoadingMask.js' @@ -19,13 +20,15 @@ export const AppWrapper = ({ children }) => { return (
- -
- window.location.reload()}> - {children} - -
- + + +
+ window.location.reload()}> + {children} + +
+ +
) } diff --git a/adapter/src/components/AuthBoundary.js b/adapter/src/components/AuthBoundary.js new file mode 100644 index 000000000..3f9988cd7 --- /dev/null +++ b/adapter/src/components/AuthBoundary.js @@ -0,0 +1,95 @@ +import { + useConfig, + useDataQuery, + clearSensitiveCaches, +} from '@dhis2/app-runtime' +import PropTypes from 'prop-types' +import React, { useState } from 'react' +import { LoadingMask } from './LoadingMask' + +// TODO: Remove useVerifyLatestUser +// TODO: add actual appName to config; rename existing to appTitle (& refactor) +// - will also need to change app-runtime and headerbar-ui APIs + +const USER_QUERY = { + user: { + resource: 'me', + params: { fields: ['id', 'authorities'] }, + }, +} + +const LATEST_USER_KEY = 'dhis2.latestUser' + +const getRequiredAppAuthority = (appName) => { + // TODO: this only works for installed, non-core apps. Need other logic for those (dhis-web-app-name) + // Maybe add this logic to CLI add this to config, instead of needing more env vars here + // Need 'coreApp', 'name', 'title' (rename current appName to appTitle) + return ( + 'M_' + + appName + .trim() + .replaceAll(/[^a-zA-Z0-9\s]/g, '') + .replaceAll(/\s/g, '_') + ) +} + +async function clearCachesIfUserChanged({ currentUserId, pwaEnabled }) { + const latestUserId = localStorage.getItem(LATEST_USER_KEY) + if (currentUserId !== latestUserId) { + const cachesCleared = await clearSensitiveCaches() + localStorage.setItem(LATEST_USER_KEY, currentUserId) + if (cachesCleared && pwaEnabled) { + // If this is a PWA app, the app-shell cache will need to + // be restored with a page reload + return window.location.reload() + } + } +} + +/** + * This hook is used to clear sensitive caches if a user other than the one + * that cached that data logs in + * @returns {Object} - { loading: boolean } + */ +export function AuthBoundary({ children }) { + const { pwaEnabled, appName } = useConfig() + const [finished, setFinished] = useState(false) + const { loading, error, data } = useDataQuery(USER_QUERY, { + onComplete: async ({ user }) => { + console.log({ authorities: user.authorities }) + + await clearCachesIfUserChanged({ + currentUserId: user.id, + pwaEnabled, + }) + setFinished(true) + }, + }) + + if (loading || !finished) { + return + } + + if (error) { + throw new Error('Failed to fetch user ID: ' + error) + } + + if (data) { + const userHasAllAuthority = data.user.authorities.includes('ALL') + + const requiredAppAuthority = getRequiredAppAuthority(appName) + console.log({ requiredAppAuthority }) + + const userHasRequiredAppAuthority = + data.user.authorities.includes(requiredAppAuthority) + if (!userHasAllAuthority && !userHasRequiredAppAuthority) { + // TODO: better UI element than error boundary? + throw new Error('Forbidden: not authorized to view this app') + } + } + + return children +} +AuthBoundary.propTypes = { + children: PropTypes.node, +} From b03b8584489b3bbbe887917f63f6fedc9b15d47f Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Mon, 17 Oct 2022 15:43:15 +0200 Subject: [PATCH 04/14] fix(adapter): use proper app authority name --- adapter/src/components/AuthBoundary.js | 61 ++++++++++++-------------- cli/src/lib/formatAppAuthName.js | 30 +++++++++++++ cli/src/lib/formatAppAuthName.test.js | 26 +++++++++++ cli/src/lib/shell/index.js | 2 + 4 files changed, 85 insertions(+), 34 deletions(-) create mode 100644 cli/src/lib/formatAppAuthName.js create mode 100644 cli/src/lib/formatAppAuthName.test.js diff --git a/adapter/src/components/AuthBoundary.js b/adapter/src/components/AuthBoundary.js index 3f9988cd7..ba6176c56 100644 --- a/adapter/src/components/AuthBoundary.js +++ b/adapter/src/components/AuthBoundary.js @@ -7,9 +7,13 @@ import PropTypes from 'prop-types' import React, { useState } from 'react' import { LoadingMask } from './LoadingMask' -// TODO: Remove useVerifyLatestUser -// TODO: add actual appName to config; rename existing to appTitle (& refactor) -// - will also need to change app-runtime and headerbar-ui APIs +// TODO: Remove useVerifyLatestUser.js (and in app wrapper) + +const LATEST_USER_KEY = 'dhis2.latestUser' +const IS_PRODUCTION_ENV = process.env.NODE_ENV === 'production' + +const APP_MANAGER_AUTHORITY = 'M_dhis-web-maintenance-appmanager' +const REQUIRED_APP_AUTHORITY = process.env.REACT_APP_DHIS2_APP_AUTH_NAME const USER_QUERY = { user: { @@ -18,21 +22,6 @@ const USER_QUERY = { }, } -const LATEST_USER_KEY = 'dhis2.latestUser' - -const getRequiredAppAuthority = (appName) => { - // TODO: this only works for installed, non-core apps. Need other logic for those (dhis-web-app-name) - // Maybe add this logic to CLI add this to config, instead of needing more env vars here - // Need 'coreApp', 'name', 'title' (rename current appName to appTitle) - return ( - 'M_' + - appName - .trim() - .replaceAll(/[^a-zA-Z0-9\s]/g, '') - .replaceAll(/\s/g, '_') - ) -} - async function clearCachesIfUserChanged({ currentUserId, pwaEnabled }) { const latestUserId = localStorage.getItem(LATEST_USER_KEY) if (currentUserId !== latestUserId) { @@ -46,6 +35,20 @@ async function clearCachesIfUserChanged({ currentUserId, pwaEnabled }) { } } +const isAppAvailable = (authorities) => { + // Skip check on dev + // TODO: should we check on dev environments too? + if (!IS_PRODUCTION_ENV) { + return true + } + // Check for three possible authorities + return authorities.some((authority) => + ['ALL', APP_MANAGER_AUTHORITY, REQUIRED_APP_AUTHORITY].includes( + authority + ) + ) +} + /** * This hook is used to clear sensitive caches if a user other than the one * that cached that data logs in @@ -56,8 +59,6 @@ export function AuthBoundary({ children }) { const [finished, setFinished] = useState(false) const { loading, error, data } = useDataQuery(USER_QUERY, { onComplete: async ({ user }) => { - console.log({ authorities: user.authorities }) - await clearCachesIfUserChanged({ currentUserId: user.id, pwaEnabled, @@ -74,21 +75,13 @@ export function AuthBoundary({ children }) { throw new Error('Failed to fetch user ID: ' + error) } - if (data) { - const userHasAllAuthority = data.user.authorities.includes('ALL') - - const requiredAppAuthority = getRequiredAppAuthority(appName) - console.log({ requiredAppAuthority }) - - const userHasRequiredAppAuthority = - data.user.authorities.includes(requiredAppAuthority) - if (!userHasAllAuthority && !userHasRequiredAppAuthority) { - // TODO: better UI element than error boundary? - throw new Error('Forbidden: not authorized to view this app') - } + if (isAppAvailable(data.user.authorities)) { + return children + } else { + throw new Error( + `Forbidden: you don't have access to the ${appName} app` + ) } - - return children } AuthBoundary.propTypes = { children: PropTypes.node, diff --git a/cli/src/lib/formatAppAuthName.js b/cli/src/lib/formatAppAuthName.js new file mode 100644 index 000000000..89e5c50a2 --- /dev/null +++ b/cli/src/lib/formatAppAuthName.js @@ -0,0 +1,30 @@ +const APP_AUTH_PREFIX = 'M_' +const DHIS_WEB = 'dhis-web-' + +/** + * Returns the string that identifies the 'App view permission' + * required to view the app + * + * Ex: coreApp && name = 'data-visualizer': authName = 'M_dhis-web-data-visualizer' + * Ex: name = 'pwa-example': authName = 'M_pwaexample' + * Ex: name = 'BNA Action Tracker': authName = 'M_BNA_Action_Tracker' + */ +const formatAppAuthName = (config) => { + if (config.coreApp) { + // TODO: Verify this formatting - are there any transformations, + // or do we trust that it's lower-case and hyphenated? + return APP_AUTH_PREFIX + DHIS_WEB + config.name + } + + // This formatting is drawn from https://github.com/dhis2/dhis2-core/blob/master/dhis-2/dhis-api/src/main/java/org/hisp/dhis/appmanager/App.java#L494-L499 + // (replaceAll is only introduced in Node 15) + return ( + APP_AUTH_PREFIX + + config.name + .trim() + .replace(/[^a-zA-Z0-9\s]/g, '') + .replace(/\s/g, '_') + ) +} + +module.exports = formatAppAuthName diff --git a/cli/src/lib/formatAppAuthName.test.js b/cli/src/lib/formatAppAuthName.test.js new file mode 100644 index 000000000..de6e76edb --- /dev/null +++ b/cli/src/lib/formatAppAuthName.test.js @@ -0,0 +1,26 @@ +const formatAppAuthName = require('./formatAppAuthName.js') + +describe('core app handling', () => { + it('should handle core apps', () => { + const config = { coreApp: true, name: 'data-visualizer' } + const formattedAuthName = formatAppAuthName(config) + + expect(formattedAuthName).toBe('M_dhis-web-data-visualizer') + }) +}) + +describe('non-core app handling', () => { + it('should handle app names with hyphens', () => { + const config = { name: 'hyphenated-string-example' } + const formattedAuthName = formatAppAuthName(config) + + expect(formattedAuthName).toBe('M_hyphenatedstringexample') + }) + + it('should handle app names with capitals and spaces', () => { + const config = { name: 'Multi Word App Name' } + const formattedAuthName = formatAppAuthName(config) + + expect(formattedAuthName).toBe('M_Multi_Word_App_Name') + }) +}) diff --git a/cli/src/lib/shell/index.js b/cli/src/lib/shell/index.js index f26a3dd07..dbd0b5728 100644 --- a/cli/src/lib/shell/index.js +++ b/cli/src/lib/shell/index.js @@ -1,4 +1,5 @@ const { exec } = require('@dhis2/cli-helpers-engine') +const formatAppAuthName = require('../formatAppAuthName') const { getPWAEnvVars } = require('../pwa') const bootstrap = require('./bootstrap') const getEnv = require('./env') @@ -7,6 +8,7 @@ module.exports = ({ config, paths }) => { const baseEnvVars = { name: config.title, version: config.version, + auth_name: formatAppAuthName(config), } return { From b882ed0e68e274b4f62f94e4c6151890a7e06957 Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Mon, 17 Oct 2022 16:13:16 +0200 Subject: [PATCH 05/14] feat(adapter): add ui for auth boundary --- adapter/src/components/AuthBoundary.js | 44 ++++++++++++++++++++------ 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/adapter/src/components/AuthBoundary.js b/adapter/src/components/AuthBoundary.js index ba6176c56..68eea4eea 100644 --- a/adapter/src/components/AuthBoundary.js +++ b/adapter/src/components/AuthBoundary.js @@ -3,9 +3,12 @@ import { useDataQuery, clearSensitiveCaches, } from '@dhis2/app-runtime' +import { CenteredContent, NoticeBox } from '@dhis2/ui' import PropTypes from 'prop-types' import React, { useState } from 'react' +import i18n from '../locales' import { LoadingMask } from './LoadingMask' +import styles from './styles/ErrorBoundary.style.js' // TODO: Remove useVerifyLatestUser.js (and in app wrapper) @@ -39,7 +42,7 @@ const isAppAvailable = (authorities) => { // Skip check on dev // TODO: should we check on dev environments too? if (!IS_PRODUCTION_ENV) { - return true + // return true } // Check for three possible authorities return authorities.some((authority) => @@ -49,13 +52,38 @@ const isAppAvailable = (authorities) => { ) } +const ForbiddenScreen = ({ appName, baseUrl }) => { + return ( +
+ + + +
+ {i18n.t( + "You don't have access to the {{appName}} app. Contact your system administrator if this seems to be an error.", + { appName } + )} +
+ + + +
+ ) +} +ForbiddenScreen.propTypes = { + appName: PropTypes.string, + baseUrl: PropTypes.string, +} + /** * This hook is used to clear sensitive caches if a user other than the one * that cached that data logs in * @returns {Object} - { loading: boolean } */ export function AuthBoundary({ children }) { - const { pwaEnabled, appName } = useConfig() + const { pwaEnabled, appName, baseUrl } = useConfig() const [finished, setFinished] = useState(false) const { loading, error, data } = useDataQuery(USER_QUERY, { onComplete: async ({ user }) => { @@ -75,13 +103,11 @@ export function AuthBoundary({ children }) { throw new Error('Failed to fetch user ID: ' + error) } - if (isAppAvailable(data.user.authorities)) { - return children - } else { - throw new Error( - `Forbidden: you don't have access to the ${appName} app` - ) - } + return isAppAvailable(data.user.authorities) ? ( + children + ) : ( + + ) } AuthBoundary.propTypes = { children: PropTypes.node, From ea3ae21244f10b5d5110f2fc79370c52ff1c950e Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Mon, 17 Oct 2022 16:16:45 +0200 Subject: [PATCH 06/14] fix(adapter): production env check --- adapter/src/components/AuthBoundary.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adapter/src/components/AuthBoundary.js b/adapter/src/components/AuthBoundary.js index 68eea4eea..66f55bf07 100644 --- a/adapter/src/components/AuthBoundary.js +++ b/adapter/src/components/AuthBoundary.js @@ -42,7 +42,7 @@ const isAppAvailable = (authorities) => { // Skip check on dev // TODO: should we check on dev environments too? if (!IS_PRODUCTION_ENV) { - // return true + return true } // Check for three possible authorities return authorities.some((authority) => From 7f97ca5b28e5c8b8712865caa4ce655077790ac0 Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Mon, 17 Oct 2022 16:26:27 +0200 Subject: [PATCH 07/14] refactor: clean up verifyLatestUser --- adapter/src/components/AppWrapper.js | 14 +++---- adapter/src/components/AuthBoundary.js | 2 - adapter/src/utils/useVerifyLatestUser.js | 47 ------------------------ 3 files changed, 6 insertions(+), 57 deletions(-) delete mode 100644 adapter/src/utils/useVerifyLatestUser.js diff --git a/adapter/src/components/AppWrapper.js b/adapter/src/components/AppWrapper.js index 93019101f..b2ad847e1 100644 --- a/adapter/src/components/AppWrapper.js +++ b/adapter/src/components/AppWrapper.js @@ -1,7 +1,6 @@ import PropTypes from 'prop-types' import React from 'react' import { useCurrentUserLocale } from '../utils/useLocale.js' -import { useVerifyLatestUser } from '../utils/useVerifyLatestUser.js' import { Alerts } from './Alerts.js' import { AuthBoundary } from './AuthBoundary.js' import { ConnectedHeaderBar } from './ConnectedHeaderBar.js' @@ -11,16 +10,15 @@ import { styles } from './styles/AppWrapper.style.js' export const AppWrapper = ({ children }) => { const { loading: localeLoading } = useCurrentUserLocale() - const { loading: latestUserLoading } = useVerifyLatestUser() - if (localeLoading || latestUserLoading) { + if (localeLoading) { return } return ( -
- - + +
+
window.location.reload()}> @@ -28,8 +26,8 @@ export const AppWrapper = ({ children }) => {
- -
+
+ ) } diff --git a/adapter/src/components/AuthBoundary.js b/adapter/src/components/AuthBoundary.js index 66f55bf07..0df2f666d 100644 --- a/adapter/src/components/AuthBoundary.js +++ b/adapter/src/components/AuthBoundary.js @@ -10,8 +10,6 @@ import i18n from '../locales' import { LoadingMask } from './LoadingMask' import styles from './styles/ErrorBoundary.style.js' -// TODO: Remove useVerifyLatestUser.js (and in app wrapper) - const LATEST_USER_KEY = 'dhis2.latestUser' const IS_PRODUCTION_ENV = process.env.NODE_ENV === 'production' diff --git a/adapter/src/utils/useVerifyLatestUser.js b/adapter/src/utils/useVerifyLatestUser.js deleted file mode 100644 index 4207247aa..000000000 --- a/adapter/src/utils/useVerifyLatestUser.js +++ /dev/null @@ -1,47 +0,0 @@ -import { - useConfig, - useDataQuery, - clearSensitiveCaches, -} from '@dhis2/app-runtime' -import { useState } from 'react' - -const USER_QUERY = { - user: { - resource: 'me', - params: { fields: ['id'] }, - }, -} - -const LATEST_USER_KEY = 'dhis2.latestUser' - -/** - * This hook is used to clear sensitive caches if a user other than the one - * that cached that data logs in - * @returns {Object} - { loading: boolean } - */ -export function useVerifyLatestUser() { - const { pwaEnabled } = useConfig() - const [finished, setFinished] = useState(false) - const { loading, error } = useDataQuery(USER_QUERY, { - onComplete: async (data) => { - const latestUserId = localStorage.getItem(LATEST_USER_KEY) - const currentUserId = data.user.id - if (currentUserId !== latestUserId) { - const cachesCleared = await clearSensitiveCaches() - localStorage.setItem(LATEST_USER_KEY, currentUserId) - if (cachesCleared && pwaEnabled) { - // If this is a PWA app, the app-shell cache will need to - // be restored with a page reload - return window.location.reload() - } - } - setFinished(true) - }, - }) - - if (error) { - throw new Error('Failed to fetch user ID: ' + error) - } - - return { loading: loading || !finished } -} From 1fe627842356976171a47fe6b0c3239eac47bb17 Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Mon, 17 Oct 2022 16:33:17 +0200 Subject: [PATCH 08/14] chore: fix comment --- adapter/src/components/AuthBoundary.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/adapter/src/components/AuthBoundary.js b/adapter/src/components/AuthBoundary.js index 0df2f666d..5990de660 100644 --- a/adapter/src/components/AuthBoundary.js +++ b/adapter/src/components/AuthBoundary.js @@ -76,9 +76,10 @@ ForbiddenScreen.propTypes = { } /** - * This hook is used to clear sensitive caches if a user other than the one - * that cached that data logs in - * @returns {Object} - { loading: boolean } + * Block the app if the user doesn't have the correct permissions to view this + * app. + * + * Also clear cached data if the current user is different from the previous. */ export function AuthBoundary({ children }) { const { pwaEnabled, appName, baseUrl } = useConfig() From 768e48dd2e0f2482d21aad0c4ce5c1afe6a62ca8 Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Mon, 17 Oct 2022 18:08:01 +0200 Subject: [PATCH 09/14] refactor: extract useVerifyLatestUser and wrap just app with authBoundary --- adapter/src/components/AppWrapper.js | 24 +++--- adapter/src/components/AuthBoundary.js | 95 ++++-------------------- adapter/src/utils/useVerifyLatestUser.js | 46 ++++++++++++ 3 files changed, 74 insertions(+), 91 deletions(-) create mode 100644 adapter/src/utils/useVerifyLatestUser.js diff --git a/adapter/src/components/AppWrapper.js b/adapter/src/components/AppWrapper.js index b2ad847e1..0aa6ee46b 100644 --- a/adapter/src/components/AppWrapper.js +++ b/adapter/src/components/AppWrapper.js @@ -1,6 +1,7 @@ import PropTypes from 'prop-types' import React from 'react' import { useCurrentUserLocale } from '../utils/useLocale.js' +import { useVerifyLatestUser } from '../utils/useVerifyLatestUser.js' import { Alerts } from './Alerts.js' import { AuthBoundary } from './AuthBoundary.js' import { ConnectedHeaderBar } from './ConnectedHeaderBar.js' @@ -10,24 +11,23 @@ import { styles } from './styles/AppWrapper.style.js' export const AppWrapper = ({ children }) => { const { loading: localeLoading } = useCurrentUserLocale() + const { loading: latestUserLoading, user } = useVerifyLatestUser() - if (localeLoading) { + if (localeLoading || latestUserLoading) { return } return ( - -
- - -
- window.location.reload()}> - {children} - -
- +
+ + +
+ window.location.reload()}> + {children} +
- + +
) } diff --git a/adapter/src/components/AuthBoundary.js b/adapter/src/components/AuthBoundary.js index 5990de660..347f8f7ec 100644 --- a/adapter/src/components/AuthBoundary.js +++ b/adapter/src/components/AuthBoundary.js @@ -1,41 +1,13 @@ -import { - useConfig, - useDataQuery, - clearSensitiveCaches, -} from '@dhis2/app-runtime' +import { useConfig } from '@dhis2/app-runtime' import { CenteredContent, NoticeBox } from '@dhis2/ui' import PropTypes from 'prop-types' -import React, { useState } from 'react' +import React from 'react' import i18n from '../locales' -import { LoadingMask } from './LoadingMask' -import styles from './styles/ErrorBoundary.style.js' -const LATEST_USER_KEY = 'dhis2.latestUser' const IS_PRODUCTION_ENV = process.env.NODE_ENV === 'production' - const APP_MANAGER_AUTHORITY = 'M_dhis-web-maintenance-appmanager' const REQUIRED_APP_AUTHORITY = process.env.REACT_APP_DHIS2_APP_AUTH_NAME -const USER_QUERY = { - user: { - resource: 'me', - params: { fields: ['id', 'authorities'] }, - }, -} - -async function clearCachesIfUserChanged({ currentUserId, pwaEnabled }) { - const latestUserId = localStorage.getItem(LATEST_USER_KEY) - if (currentUserId !== latestUserId) { - const cachesCleared = await clearSensitiveCaches() - localStorage.setItem(LATEST_USER_KEY, currentUserId) - if (cachesCleared && pwaEnabled) { - // If this is a PWA app, the app-shell cache will need to - // be restored with a page reload - return window.location.reload() - } - } -} - const isAppAvailable = (authorities) => { // Skip check on dev // TODO: should we check on dev environments too? @@ -50,64 +22,29 @@ const isAppAvailable = (authorities) => { ) } -const ForbiddenScreen = ({ appName, baseUrl }) => { - return ( -
- - - -
- {i18n.t( - "You don't have access to the {{appName}} app. Contact your system administrator if this seems to be an error.", - { appName } - )} -
- - - -
- ) -} -ForbiddenScreen.propTypes = { - appName: PropTypes.string, - baseUrl: PropTypes.string, -} - /** * Block the app if the user doesn't have the correct permissions to view this * app. - * - * Also clear cached data if the current user is different from the previous. */ -export function AuthBoundary({ children }) { - const { pwaEnabled, appName, baseUrl } = useConfig() - const [finished, setFinished] = useState(false) - const { loading, error, data } = useDataQuery(USER_QUERY, { - onComplete: async ({ user }) => { - await clearCachesIfUserChanged({ - currentUserId: user.id, - pwaEnabled, - }) - setFinished(true) - }, - }) - - if (loading || !finished) { - return - } - - if (error) { - throw new Error('Failed to fetch user ID: ' + error) - } +export function AuthBoundary({ user, children }) { + const { appName } = useConfig() - return isAppAvailable(data.user.authorities) ? ( + return isAppAvailable(user.authorities) ? ( children ) : ( - + + + {i18n.t( + "You don't have access to the {{appName}} app. Contact your system administrator if this seems to be an error.", + { appName } + )} + + ) } AuthBoundary.propTypes = { children: PropTypes.node, + user: PropTypes.shape({ + authorities: PropTypes.arrayOf(PropTypes.string), + }), } diff --git a/adapter/src/utils/useVerifyLatestUser.js b/adapter/src/utils/useVerifyLatestUser.js new file mode 100644 index 000000000..85ed0c052 --- /dev/null +++ b/adapter/src/utils/useVerifyLatestUser.js @@ -0,0 +1,46 @@ +import { + useConfig, + useDataQuery, + clearSensitiveCaches, +} from '@dhis2/app-runtime' +import { useState } from 'react' + +const USER_QUERY = { + user: { + resource: 'me', + params: { fields: ['id', 'username', 'authorities'] }, + }, +} + +const LATEST_USER_KEY = 'dhis2.latestUser' + +/** + * This hook is used to clear sensitive caches if a user other than the one + * that cached that data logs in + * @returns {Object} - { loading: boolean } + */ +export function useVerifyLatestUser() { + const { pwaEnabled } = useConfig() + const [finished, setFinished] = useState(false) + const { loading, error, data } = useDataQuery(USER_QUERY, { + onComplete: async (data) => { + const latestUserId = localStorage.getItem(LATEST_USER_KEY) + const currentUserId = data.user.id + if (currentUserId !== latestUserId) { + const cachesCleared = await clearSensitiveCaches() + localStorage.setItem(LATEST_USER_KEY, currentUserId) + if (cachesCleared && pwaEnabled) { + // If this is a PWA app, the app-shell cache will need to + // be restored with a page reload + return window.location.reload() + } + } + setFinished(true) + }, + }) + if (error) { + throw new Error('Failed to fetch user ID: ' + error) + } + + return { loading: loading || !finished, user: data?.user } +} From 431fa72a07da20809c222c2f2ad118419018069b Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Tue, 18 Oct 2022 09:18:45 +0200 Subject: [PATCH 10/14] fix: notice box text --- adapter/src/components/AuthBoundary.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/adapter/src/components/AuthBoundary.js b/adapter/src/components/AuthBoundary.js index 347f8f7ec..ff9430212 100644 --- a/adapter/src/components/AuthBoundary.js +++ b/adapter/src/components/AuthBoundary.js @@ -33,10 +33,14 @@ export function AuthBoundary({ user, children }) { children ) : ( - + {i18n.t( - "You don't have access to the {{appName}} app. Contact your system administrator if this seems to be an error.", - { appName } + 'Contact your system administrator for assistance with app access.' )} From 7fb97ef9c6c9453f0ed378e733ccf23ffde0fb15 Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Tue, 18 Oct 2022 14:23:08 +0200 Subject: [PATCH 11/14] chore: i18n --- adapter/i18n/en.pot | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/adapter/i18n/en.pot b/adapter/i18n/en.pot index d5654aeda..e01ff78a3 100644 --- a/adapter/i18n/en.pot +++ b/adapter/i18n/en.pot @@ -5,8 +5,14 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1)\n" -"POT-Creation-Date: 2022-09-22T19:11:17.660Z\n" -"PO-Revision-Date: 2022-09-22T19:11:17.660Z\n" +"POT-Creation-Date: 2022-10-18T07:20:55.997Z\n" +"PO-Revision-Date: 2022-10-18T07:20:55.997Z\n" + +msgid "You don't have access to the {{appName}} app" +msgstr "You don't have access to the {{appName}} app" + +msgid "Contact your system administrator for assistance with app access." +msgstr "Contact your system administrator for assistance with app access." msgid "Save your data" msgstr "Save your data" From 6ddc7e11cd3f4ff0bc50aad14d113166920e1024 Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Thu, 20 Oct 2022 17:38:13 +0200 Subject: [PATCH 12/14] chore: add name and title to pwa example app --- examples/pwa-app/d2.config.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/examples/pwa-app/d2.config.js b/examples/pwa-app/d2.config.js index d038e6be6..6fd241641 100644 --- a/examples/pwa-app/d2.config.js +++ b/examples/pwa-app/d2.config.js @@ -1,5 +1,7 @@ const config = { type: 'app', + name: 'pwa-example', + title: 'PWA Example', pwa: { enabled: true, From 09b100c5b263f0d8490c63940db115da85d9677f Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Thu, 20 Oct 2022 17:43:07 +0200 Subject: [PATCH 13/14] fix: ensure core app name formatting and handle legacy versions --- adapter/src/components/AuthBoundary.js | 21 ++++++++++++--------- cli/src/lib/constructAppUrl.js | 17 +++++++++++++---- cli/src/lib/formatAppAuthName.js | 15 ++++++++++----- cli/src/lib/shell/index.js | 3 ++- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/adapter/src/components/AuthBoundary.js b/adapter/src/components/AuthBoundary.js index ff9430212..22b357c78 100644 --- a/adapter/src/components/AuthBoundary.js +++ b/adapter/src/components/AuthBoundary.js @@ -6,19 +6,22 @@ import i18n from '../locales' const IS_PRODUCTION_ENV = process.env.NODE_ENV === 'production' const APP_MANAGER_AUTHORITY = 'M_dhis-web-maintenance-appmanager' -const REQUIRED_APP_AUTHORITY = process.env.REACT_APP_DHIS2_APP_AUTH_NAME +const APP_AUTH_NAME = process.env.REACT_APP_DHIS2_APP_AUTH_NAME +const LEGACY_APP_AUTH_NAME = process.env.REACT_APP_DHIS2_APP_LEGACY_AUTH_NAME -const isAppAvailable = (authorities) => { +const isAppAvailable = ({ userAuthorities, apiVersion }) => { // Skip check on dev - // TODO: should we check on dev environments too? if (!IS_PRODUCTION_ENV) { return true } + + // On server versions < 35, auth name uses config.title instead of .name + const requiredAppAuthority = + apiVersion >= 35 ? APP_AUTH_NAME : LEGACY_APP_AUTH_NAME + // Check for three possible authorities - return authorities.some((authority) => - ['ALL', APP_MANAGER_AUTHORITY, REQUIRED_APP_AUTHORITY].includes( - authority - ) + return userAuthorities.some((authority) => + ['ALL', APP_MANAGER_AUTHORITY, requiredAppAuthority].includes(authority) ) } @@ -27,9 +30,9 @@ const isAppAvailable = (authorities) => { * app. */ export function AuthBoundary({ user, children }) { - const { appName } = useConfig() + const { appName, apiVersion } = useConfig() - return isAppAvailable(user.authorities) ? ( + return isAppAvailable({ userAuthorities: user.authorities, apiVersion }) ? ( children ) : ( diff --git a/cli/src/lib/constructAppUrl.js b/cli/src/lib/constructAppUrl.js index 17cf1641d..605a3a46f 100644 --- a/cli/src/lib/constructAppUrl.js +++ b/cli/src/lib/constructAppUrl.js @@ -1,12 +1,16 @@ -module.exports.constructAppUrl = (baseUrl, config, serverVersion) => { +const formatUrlSafeAppSlug = (appName) => { + return appName.replace(/[^A-Za-z0-9\s-]/g, '').replace(/\s+/g, '-') +} + +const constructAppUrl = (baseUrl, config, serverVersion) => { let appUrl = baseUrl const isModernServer = serverVersion.major >= 2 && serverVersion.minor >= 35 // From core version 2.35, short_name is used instead of the human-readable title to generate the url slug - const urlSafeAppSlug = (isModernServer ? config.name : config.title) - .replace(/[^A-Za-z0-9\s-]/g, '') - .replace(/\s+/g, '-') + const urlSafeAppSlug = formatUrlSafeAppSlug( + isModernServer ? config.name : config.title + ) // From core version 2.35, core apps are hosted at the server root under the /dhis-web-* namespace if (config.coreApp && isModernServer) { @@ -25,3 +29,8 @@ module.exports.constructAppUrl = (baseUrl, config, serverVersion) => { appUrl = scheme + appUrl.substr(scheme.length).replace(/\/+/g, '/') return appUrl } + +module.exports = { + constructAppUrl, + formatUrlSafeAppSlug, +} diff --git a/cli/src/lib/formatAppAuthName.js b/cli/src/lib/formatAppAuthName.js index 89e5c50a2..c90fabffc 100644 --- a/cli/src/lib/formatAppAuthName.js +++ b/cli/src/lib/formatAppAuthName.js @@ -1,3 +1,5 @@ +const { formatUrlSafeAppSlug } = require('./constructAppUrl') + const APP_AUTH_PREFIX = 'M_' const DHIS_WEB = 'dhis-web-' @@ -8,19 +10,22 @@ const DHIS_WEB = 'dhis-web-' * Ex: coreApp && name = 'data-visualizer': authName = 'M_dhis-web-data-visualizer' * Ex: name = 'pwa-example': authName = 'M_pwaexample' * Ex: name = 'BNA Action Tracker': authName = 'M_BNA_Action_Tracker' + * + * The 'legacy' parameter specifies server version < 2.35 which uses + * config.title instead of config.name */ -const formatAppAuthName = (config) => { +const formatAppAuthName = ({ config, legacy }) => { + const appName = legacy ? config.title : config.name + if (config.coreApp) { - // TODO: Verify this formatting - are there any transformations, - // or do we trust that it's lower-case and hyphenated? - return APP_AUTH_PREFIX + DHIS_WEB + config.name + return APP_AUTH_PREFIX + DHIS_WEB + formatUrlSafeAppSlug(appName) } // This formatting is drawn from https://github.com/dhis2/dhis2-core/blob/master/dhis-2/dhis-api/src/main/java/org/hisp/dhis/appmanager/App.java#L494-L499 // (replaceAll is only introduced in Node 15) return ( APP_AUTH_PREFIX + - config.name + appName .trim() .replace(/[^a-zA-Z0-9\s]/g, '') .replace(/\s/g, '_') diff --git a/cli/src/lib/shell/index.js b/cli/src/lib/shell/index.js index dbd0b5728..e1413f5d5 100644 --- a/cli/src/lib/shell/index.js +++ b/cli/src/lib/shell/index.js @@ -8,7 +8,8 @@ module.exports = ({ config, paths }) => { const baseEnvVars = { name: config.title, version: config.version, - auth_name: formatAppAuthName(config), + auth_name: formatAppAuthName({ config }), + legacy_auth_name: formatAppAuthName({ config, legacy: true }), } return { From 0668608d7517bf41a7d275717381918b81b3eb08 Mon Sep 17 00:00:00 2001 From: Kai Vandivier Date: Fri, 21 Oct 2022 14:31:04 +0200 Subject: [PATCH 14/14] test: fix tests --- cli/src/lib/formatAppAuthName.test.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/cli/src/lib/formatAppAuthName.test.js b/cli/src/lib/formatAppAuthName.test.js index de6e76edb..aa044c33e 100644 --- a/cli/src/lib/formatAppAuthName.test.js +++ b/cli/src/lib/formatAppAuthName.test.js @@ -3,7 +3,7 @@ const formatAppAuthName = require('./formatAppAuthName.js') describe('core app handling', () => { it('should handle core apps', () => { const config = { coreApp: true, name: 'data-visualizer' } - const formattedAuthName = formatAppAuthName(config) + const formattedAuthName = formatAppAuthName({ config }) expect(formattedAuthName).toBe('M_dhis-web-data-visualizer') }) @@ -12,15 +12,28 @@ describe('core app handling', () => { describe('non-core app handling', () => { it('should handle app names with hyphens', () => { const config = { name: 'hyphenated-string-example' } - const formattedAuthName = formatAppAuthName(config) + const formattedAuthName = formatAppAuthName({ config }) expect(formattedAuthName).toBe('M_hyphenatedstringexample') }) it('should handle app names with capitals and spaces', () => { const config = { name: 'Multi Word App Name' } - const formattedAuthName = formatAppAuthName(config) + const formattedAuthName = formatAppAuthName({ config }) expect(formattedAuthName).toBe('M_Multi_Word_App_Name') }) }) + +describe('legacy app handling', () => { + it('should use title instead of name if the legacy flag is added', () => { + const config = { + name: 'Multi Word App Name', + title: 'Title of the App', + } + const formattedAuthName = formatAppAuthName({ config, legacy: true }) + + expect(formattedAuthName).not.toBe('M_Multi_Word_App_Name') + expect(formattedAuthName).toBe('M_Title_of_the_App') + }) +})