From f547fdf83296c1d57fc0a9f99ead249f28c60074 Mon Sep 17 00:00:00 2001 From: Erik Ziegler Date: Sun, 21 Jul 2019 16:43:42 +0200 Subject: [PATCH] fix(OpenIDConnect): Stop storing tokens in sessionStorage. Prefix OIDC routes automatically. Add logout button (#698) --- package.json | 2 +- public/config/google.js | 2 +- public/logout-redirect.html | 25 ---------- rollup.config.js | 5 ++ src/App.js | 50 ++++++++++++++++--- src/OHIFStandaloneViewer.js | 20 +++++++- src/UserManagerContext.js | 5 ++ src/components/Header/Header.js | 11 ++++ src/connectedComponents/ConnectedHeader.js | 1 + src/connectedComponents/Viewer.js | 12 +++-- src/googleCloud/ConnectedDicomStorePicker.js | 4 +- src/googleCloud/DatasetPicker.js | 4 +- src/googleCloud/DatasetSelector.js | 12 +++-- src/googleCloud/DicomStorePicker.js | 6 +-- src/googleCloud/DicomStorePickerModal.js | 4 +- src/googleCloud/LocationPicker.js | 4 +- src/googleCloud/ProjectPicker.js | 4 +- src/googleCloud/api/GoogleCloudApi.js | 14 ++---- src/studylist/StudyListWithData.js | 15 +++++- .../getUserManagerForOpenIdConnectClient.js | 7 ++- yarn.lock | 10 ++-- 21 files changed, 143 insertions(+), 74 deletions(-) delete mode 100644 public/logout-redirect.html create mode 100644 src/UserManagerContext.js diff --git a/package.json b/package.json index 8dd572302..4e26bc010 100644 --- a/package.json +++ b/package.json @@ -99,7 +99,7 @@ "lodash.isequal": "4.5.0", "moment": "^2.24.0", "ohif-core": "0.10.2", - "oidc-client": "1.7.x", + "oidc-client": "1.8.x", "prop-types": "^15.7.2", "react-dropzone": "^10.1.5", "react-i18next": "^10.11.0", diff --git a/public/config/google.js b/public/config/google.js index f5673b83d..48d64d3c9 100644 --- a/public/config/google.js +++ b/public/config/google.js @@ -13,7 +13,7 @@ window.config = { // Authorization Server URL authority: 'https://accounts.google.com', client_id: 'YOURCLIENTID.apps.googleusercontent.com', - redirect_uri: `${window.location}callback`, // `OHIFStandaloneViewer.js` + redirect_uri: '/callback', // `OHIFStandaloneViewer.js` response_type: 'id_token token', scope: 'email profile openid https://www.googleapis.com/auth/cloudplatformprojects.readonly https://www.googleapis.com/auth/cloud-healthcare', // email profile openid // ~ OPTIONAL diff --git a/public/logout-redirect.html b/public/logout-redirect.html deleted file mode 100644 index 263fcb8d3..000000000 --- a/public/logout-redirect.html +++ /dev/null @@ -1,25 +0,0 @@ - - - - - OpenID Connect Logout Redirect Page - - - - - - - - - - diff --git a/rollup.config.js b/rollup.config.js index 2c25e0afa..29140ed19 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -61,10 +61,15 @@ export default { 'node_modules/redux-oidc/dist/redux-oidc.js': [ 'reducer', 'CallbackComponent', + 'SignoutCallbackComponent', 'loadUser', 'OidcProvider', 'createUserManager', ], + 'node_modules/oidc-client/lib/oidc-client.min.js': [ + 'WebStorageStateStore', + 'InMemoryWebStorage', + ], 'node_modules/cornerstoneTools/dist/cornerstoneTools.min.js': [ 'cornerstoneTools', ], diff --git a/src/App.js b/src/App.js index b7374d804..fe51866cc 100644 --- a/src/App.js +++ b/src/App.js @@ -37,6 +37,7 @@ import { getActiveContexts } from './store/layout/selectors.js'; import i18n from '@ohif/i18n'; import setupTools from './setupTools.js'; import store from './store'; +import UserManagerContext from './UserManagerContext'; // ~~~~ APP SETUP initCornerstoneTools({ @@ -84,6 +85,23 @@ function handleServers(servers) { } } +function isAbsoluteUrl(url) { + return url.includes('http://') || url.includes('https://'); +} + +function makeAbsoluteIfNecessary(url, base_url) { + if (isAbsoluteUrl(url)) { + return url; + } + + // Make sure base_url and url are not duplicating slashes + if (base_url[base_url.length - 1] === "/") { + base_url = base_url.slice(0, base_url.length - 1); + } + + return base_url + url; +} + class App extends Component { static propTypes = { routerBasename: PropTypes.string.isRequired, @@ -104,9 +122,23 @@ class App extends Component { if (this.props.oidc.length) { const firstOpenIdClient = this.props.oidc[0]; + const { protocol, host } = window.location; + const { routerBasename } = this.props; + const baseUri = `${protocol}//${host}${routerBasename}`; + + const redirect_uri = firstOpenIdClient.redirect_uri || '/callback'; + const silent_redirect_uri = firstOpenIdClient.silent_redirect_uri || '/silent-refresh.html'; + const post_logout_redirect_uri = firstOpenIdClient.post_logout_redirect_uri || '/'; + + const openIdConnectConfiguration = Object.assign({}, firstOpenIdClient, { + redirect_uri: makeAbsoluteIfNecessary(redirect_uri, baseUri), + silent_redirect_uri: makeAbsoluteIfNecessary(silent_redirect_uri, baseUri), + post_logout_redirect_uri: makeAbsoluteIfNecessary(post_logout_redirect_uri, baseUri), + }); + this.userManager = getUserManagerForOpenIdConnectClient( store, - firstOpenIdClient + openIdConnectConfiguration, ); } handleServers(this.props.servers); @@ -124,13 +156,15 @@ class App extends Component { - - - - - + + + + + + + diff --git a/src/OHIFStandaloneViewer.js b/src/OHIFStandaloneViewer.js index c5f8bc716..ccd74a16b 100644 --- a/src/OHIFStandaloneViewer.js +++ b/src/OHIFStandaloneViewer.js @@ -5,6 +5,7 @@ import { Route, Switch } from 'react-router-dom'; import { NProgress } from '@tanem/react-nprogress'; import { CSSTransition } from 'react-transition-group'; import { connect } from 'react-redux'; +import { SignoutCallbackComponent } from 'redux-oidc'; import { ViewerbaseDragDropContext } from 'react-viewerbase'; // import asyncComponent from './components/AsyncComponent.js' import IHEInvokeImageDisplay from './routes/IHEInvokeImageDisplay.js'; @@ -74,14 +75,29 @@ class OHIFStandaloneViewer extends Component { return ( - + + console.log('Signout successful')} + errorCallback={(error) => { + console.warn(error); + console.warn('Signout failed'); + }} + /> + }/> } /> { - userManager.signinRedirect(); + userManager.getUser().then(user => { + if (user) { + userManager.signinSilent(); + } else { + userManager.signinRedirect(); + } + }); return null; }} diff --git a/src/UserManagerContext.js b/src/UserManagerContext.js new file mode 100644 index 000000000..f566aed9a --- /dev/null +++ b/src/UserManagerContext.js @@ -0,0 +1,5 @@ +import React from 'react'; + +const UserManagerContext = React.createContext(); + +export default UserManagerContext; diff --git a/src/components/Header/Header.js b/src/components/Header/Header.js index 2cd0c04d6..f8d42a7e6 100644 --- a/src/components/Header/Header.js +++ b/src/components/Header/Header.js @@ -17,6 +17,7 @@ class Header extends Component { location: PropTypes.object.isRequired, children: PropTypes.node, t: PropTypes.func.isRequired, + userManager: PropTypes.object }; static defaultProps = { @@ -59,6 +60,16 @@ class Header extends Component { }, ]; + if (this.props.user && this.props.userManager) { + this.options.push({ + title: t('Logout'), + icon: { name: 'power-off' }, + onClick: () => { + this.props.userManager.signoutRedirect(); + }, + }); + } + this.hotKeysData = hotkeysManager.hotkeyDefinitions; } diff --git a/src/connectedComponents/ConnectedHeader.js b/src/connectedComponents/ConnectedHeader.js index 1f39315b6..74425cca1 100644 --- a/src/connectedComponents/ConnectedHeader.js +++ b/src/connectedComponents/ConnectedHeader.js @@ -3,6 +3,7 @@ import { connect } from 'react-redux'; const mapStateToProps = state => { return { + user: state.oidc && state.oidc.user, isOpen: state.ui.userPreferencesModalOpen, }; }; diff --git a/src/connectedComponents/Viewer.js b/src/connectedComponents/Viewer.js index fafd1e206..d47081839 100644 --- a/src/connectedComponents/Viewer.js +++ b/src/connectedComponents/Viewer.js @@ -13,6 +13,7 @@ import ConnectedStudyBrowser from './ConnectedStudyBrowser.js'; import ConnectedViewerMain from './ConnectedViewerMain.js'; import SidePanel from './../components/SidePanel.js'; import { extensionManager } from './../App.js'; +import UserManagerContext from '../UserManagerContext'; import './Viewer.css'; /** * Inits OHIF Hanging Protocol's onReady. @@ -230,9 +231,14 @@ class Viewer extends Component { {/* HEADER */} {whiteLabelling => ( - - {whiteLabelling.logoComponent} - + + { userManager => ( + + {whiteLabelling.logoComponent} + + ) + } + )} diff --git a/src/googleCloud/ConnectedDicomStorePicker.js b/src/googleCloud/ConnectedDicomStorePicker.js index 4572af90f..957ce89db 100644 --- a/src/googleCloud/ConnectedDicomStorePicker.js +++ b/src/googleCloud/ConnectedDicomStorePicker.js @@ -5,11 +5,9 @@ const isActive = a => a.active === true; const mapStateToProps = state => { const activeServer = state.servers.servers.find(isActive); - const { authority, client_id } = window.config.oidc[0]; - const oidcStorageKey = `oidc.user:${authority}:${client_id}`; return { - oidcStorageKey, + user: state.oidc && state.oidc.user, url: activeServer && activeServer.qidoRoot, }; }; diff --git a/src/googleCloud/DatasetPicker.js b/src/googleCloud/DatasetPicker.js index 581dd2be0..711c216d3 100644 --- a/src/googleCloud/DatasetPicker.js +++ b/src/googleCloud/DatasetPicker.js @@ -15,11 +15,11 @@ export default class DatasetPicker extends Component { project: PropTypes.object, location: PropTypes.object, onSelect: PropTypes.func, - oidcKey: PropTypes.string, + accessToken: PropTypes.string, }; async componentDidMount() { - api.setOidcStorageKey(this.props.oidcKey); + api.setAccessToken(this.props.accessToken); const response = await api.loadDatasets( this.props.project.projectId, diff --git a/src/googleCloud/DatasetSelector.js b/src/googleCloud/DatasetSelector.js index f347226f1..724a7a33f 100644 --- a/src/googleCloud/DatasetSelector.js +++ b/src/googleCloud/DatasetSelector.js @@ -19,7 +19,7 @@ class DatasetSelector extends Component { static propTypes = { id: PropTypes.string, event: PropTypes.string, - oidcKey: PropTypes.string, + user: PropTypes.object, canClose: PropTypes.string, setServers: PropTypes.func.isRequired, }; @@ -79,6 +79,8 @@ class DatasetSelector extends Component { }; render() { + const accessToken = this.props.user.access_token; + const { project, location, dataset } = this.state; const { onProjectClick, @@ -121,21 +123,21 @@ class DatasetSelector extends Component { {projectBreadcrumbs} {!project && ( )} {project && !location && ( )} {project && location && !dataset && ( diff --git a/src/googleCloud/DicomStorePicker.js b/src/googleCloud/DicomStorePicker.js index 7437bb9a2..a1cbcc12f 100644 --- a/src/googleCloud/DicomStorePicker.js +++ b/src/googleCloud/DicomStorePicker.js @@ -15,12 +15,12 @@ export default class DicomStorePicker extends Component { static propTypes = { dataset: PropTypes.object, onSelect: PropTypes.func, + accessToken: PropTypes.string.isRequired }; async componentDidMount() { - const { authority, client_id } = window.config.oidc[0]; - const oidcStorageKey = `oidc.user:${authority}:${client_id}`; - api.setOidcStorageKey(oidcStorageKey); + api.setAccessToken(this.props.accessToken); + const response = await api.loadDicomStores(this.props.dataset.name); if (response.isError) { diff --git a/src/googleCloud/DicomStorePickerModal.js b/src/googleCloud/DicomStorePickerModal.js index 5de69f531..c5b7e35e3 100644 --- a/src/googleCloud/DicomStorePickerModal.js +++ b/src/googleCloud/DicomStorePickerModal.js @@ -8,7 +8,7 @@ import { withTranslation } from 'react-i18next'; class DicomStorePickerModal extends Component { static propTypes = { url: PropTypes.string, - oidcStorageKey: PropTypes.string.isRequired, + user: PropTypes.object.isRequired, setServers: PropTypes.func.isRequired, isOpen: PropTypes.bool.isRequired, onClose: PropTypes.func, @@ -55,7 +55,7 @@ class DicomStorePickerModal extends Component { diff --git a/src/googleCloud/LocationPicker.js b/src/googleCloud/LocationPicker.js index 423d194c5..8b687c9a2 100644 --- a/src/googleCloud/LocationPicker.js +++ b/src/googleCloud/LocationPicker.js @@ -14,11 +14,11 @@ export default class LocationPicker extends Component { static propTypes = { project: PropTypes.object, onSelect: PropTypes.func, - oidcKey: PropTypes.string, + accessToken: PropTypes.string, }; async componentDidMount() { - api.setOidcStorageKey(this.props.oidcKey); + api.setAccessToken(this.props.accessToken); const response = await api.loadLocations(this.props.project.projectId); diff --git a/src/googleCloud/ProjectPicker.js b/src/googleCloud/ProjectPicker.js index 4455f211b..106dcd9a0 100644 --- a/src/googleCloud/ProjectPicker.js +++ b/src/googleCloud/ProjectPicker.js @@ -13,11 +13,11 @@ export default class ProjectPicker extends Component { static propTypes = { onSelect: PropTypes.func, - oidcKey: PropTypes.string, + accessToken: PropTypes.string, }; async componentDidMount() { - api.setOidcStorageKey(this.props.oidcKey); + api.setAccessToken(this.props.accessToken); const response = await api.loadProjects(); if (response.isError) { diff --git a/src/googleCloud/api/GoogleCloudApi.js b/src/googleCloud/api/GoogleCloudApi.js index d3a4cf5d2..08db55d5f 100644 --- a/src/googleCloud/api/GoogleCloudApi.js +++ b/src/googleCloud/api/GoogleCloudApi.js @@ -1,19 +1,15 @@ -import { getOidcToken } from '../utils/helpers'; - class GoogleCloudApi { - setOidcStorageKey(oidcStorageKey) { - if (!oidcStorageKey) console.error('OIDC storage key is empty'); - this.oidcStorageKey = oidcStorageKey; + setAccessToken(accessToken) { + if (!accessToken) console.error('Access token is empty'); + this.accessToken = accessToken; } get fetchConfig() { - if (!this.oidcStorageKey) throw new Error('OIDC storage key is not set'); - const accessToken = getOidcToken(this.oidcStorageKey); - if (!accessToken) throw new Error('OIDC access_token is not set'); + if (!this.accessToken) throw new Error('OIDC access_token is not set'); return { method: 'GET', headers: { - Authorization: 'Bearer ' + accessToken, + Authorization: 'Bearer ' + this.accessToken, }, }; } diff --git a/src/studylist/StudyListWithData.js b/src/studylist/StudyListWithData.js index fa406a18a..5749baf58 100644 --- a/src/studylist/StudyListWithData.js +++ b/src/studylist/StudyListWithData.js @@ -10,6 +10,8 @@ import moment from 'moment'; import ConnectedDicomFilesUploader from '../googleCloud/ConnectedDicomFilesUploader'; import ConnectedDicomStorePicker from '../googleCloud/ConnectedDicomStorePicker'; import filesToStudies from '../lib/filesToStudies.js'; +import UserManagerContext from '../UserManagerContext'; +import WhiteLabellingContext from '../WhiteLabellingContext'; class StudyListWithData extends Component { state = { @@ -272,7 +274,18 @@ class StudyListWithData extends Component { ); return ( <> - + + {whiteLabelling => ( + + { userManager => ( + + {whiteLabelling.logoComponent} + + ) + } + + )} + {studyList} ); diff --git a/src/utils/getUserManagerForOpenIdConnectClient.js b/src/utils/getUserManagerForOpenIdConnectClient.js index bd3b3688d..38447ffa2 100644 --- a/src/utils/getUserManagerForOpenIdConnectClient.js +++ b/src/utils/getUserManagerForOpenIdConnectClient.js @@ -1,5 +1,6 @@ // https://github.com/maxmantz/redux-oidc/blob/master/docs/API.md import { loadUser, createUserManager } from 'redux-oidc'; +import { WebStorageStateStore, InMemoryWebStorage } from 'oidc-client'; /** * Creates a userManager from oidcSettings; @@ -20,13 +21,17 @@ export default function(store, oidcSettings) { return; } + // Do not store tokens in localStorage or sessionStorage + // https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/HTML5_Security_Cheat_Sheet.md#local-storage + const userStore = new WebStorageStateStore({ store: new InMemoryWebStorage() }); + const settings = { ...oidcSettings, - silent_redirect_uri: '/silent-refresh.html', automaticSilentRenew: true, revokeAccessTokenOnSignout: true, filterProtocolClaims: true, loadUserInfo: true, + userStore, }; const userManager = createUserManager(settings); diff --git a/yarn.lock b/yarn.lock index 15eb21abc..c730b03de 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10515,10 +10515,12 @@ ohif-core@0.10.2: mousetrap "^1.6.3" validate.js "^0.12.0" -oidc-client@1.7.x: - version "1.7.1" - resolved "https://registry.yarnpkg.com/oidc-client/-/oidc-client-1.7.1.tgz#8b9d8d50fd7f878968b1cda17712c1747eef9a54" - integrity sha512-qsPBQVa/BY6AmdY89erANJbfDXrX1dqu9lKgvYZzkVDzIj5mmw6wGjFeQuV2HDm4TiJA0VT5HSTWOWnXZUYu0g== +oidc-client@1.8.x: + version "1.8.2" + resolved "https://registry.yarnpkg.com/oidc-client/-/oidc-client-1.8.2.tgz#5a73c33858fe0e25489fdc6de31c8ce3075f6e0b" + integrity sha512-WwoSY8S6QyNN3qpne88YurjNqjTf6z1Xr0y+OrFVvdnVPYcefkTtXlZ5iOwR2JrmP4vBuq2j8eTjUJyDZFrFNQ== + dependencies: + uuid "^3.3.2" ol@^5.3.0: version "5.3.3"