fix(OpenIDConnect): Stop storing tokens in sessionStorage. Prefix OIDC routes automatically. Add logout button (#698)

2019-07-21 16:43:42 +02:00 · 2019-07-21 16:43:42 +02:00 · f547fdf832
commit f547fdf832
parent ac347849d0
21 changed files with 143 additions and 74 deletions
--- 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",
--- 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
--- a/public/logout-redirect.html
+++ b/public/logout-redirect.html
@ -1,25 +0,0 @@
-<!DOCTYPE html>
-<html lang="en">
-<head>
-    <meta charset="UTF-8">
-    <title>OpenID Connect Logout Redirect Page</title>
-</head>
-<body>
-    <script src="https://cdnjs.cloudflare.com/ajax/libs/es5-shim/4.5.7/es5-shim.min.js"></script>
-    <script type="text/javascript" src='es6-shim.min.js'></script>
-    <script type="text/javascript" src='oidc-client.min.js'></script>
-    <script type="text/javascript" src='polyfill.min.js'></script>
-    <script type="text/javascript">
-      // TODO: I think this is actually not necessary anymore
-      new UserManager().signoutRedirectCallback().then(function() {
-        // TODO: Keycloak + Auth0 is not properly logging out both sessions?
-        // This is a hacky hardcoded workaround. This could also be added as a component to the React application rather than a static page.
-        var path = 'https://www.crowds-cure.org';
-        var client_id = 'z5cXMPTxeFOdB3i4xRA8JyhTonQmqMKM';
-        var url = 'https://auth.crowds-cure.org/v2/logout?client_id=' + client_id + '&returnTo=' + path;
-        window.location.href = url;
-      });
-    </script>
-</body>
-
-</html>
--- 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',
        ],
--- 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 {
        <Provider store={store}>
          <I18nextProvider i18n={i18n}>
            <OidcProvider store={store} userManager={userManager}>
-              <Router basename={this.props.routerBasename}>
-                <WhiteLabellingContext.Provider
-                  value={this.props.whiteLabelling}
-                >
-                  <OHIFStandaloneViewer userManager={userManager} />
-                </WhiteLabellingContext.Provider>
-              </Router>
+              <UserManagerContext.Provider value={userManager}>
+                <Router basename={this.props.routerBasename}>
+                  <WhiteLabellingContext.Provider
+                    value={this.props.whiteLabelling}
+                  >
+                    <OHIFStandaloneViewer userManager={userManager} />
+                  </WhiteLabellingContext.Provider>
+                </Router>
+              </UserManagerContext.Provider>
            </OidcProvider>
          </I18nextProvider>
        </Provider>
--- 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 (
        <Switch>
          <Route exact path="/silent-refresh.html" onEnter={reload} />
-          <Route exact path="/logout-redirect.html" onEnter={reload} />
+          <Route exact path="/logout-redirect" render={() =>
+            <SignoutCallbackComponent
+              userManager={userManager}
+              successCallback={() => console.log('Signout successful')}
+              errorCallback={(error) => {
+                console.warn(error);
+                console.warn('Signout failed');
+              }}
+            />
+          }/>
          <Route
            path="/callback"
            render={() => <CallbackPage userManager={userManager} />}
          />
          <Route
            component={() => {
-              userManager.signinRedirect();
+              userManager.getUser().then(user => {
+                if (user) {
+                  userManager.signinSilent();
+                } else {
+                  userManager.signinRedirect();
+                }
+              });

              return null;
            }}
--- a/src/UserManagerContext.js
+++ b/src/UserManagerContext.js
@ -0,0 +1,5 @@
+import React from 'react';
+
+const UserManagerContext = React.createContext();
+
+export default UserManagerContext;
--- 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;
  }

--- 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,
  };
 };
--- 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 */}
        <WhiteLabellingContext.Consumer>
          {whiteLabelling => (
-            <ConnectedHeader home={false}>
-              {whiteLabelling.logoComponent}
-            </ConnectedHeader>
+            <UserManagerContext.Consumer>
+              { userManager => (
+                  <ConnectedHeader home={false} userManager={userManager}>
+                    {whiteLabelling.logoComponent}
+                  </ConnectedHeader>
+                )
+              }
+            </UserManagerContext.Consumer>
          )}
        </WhiteLabellingContext.Consumer>

--- 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,
  };
 };
--- 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,
--- 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 && (
          <ProjectPicker
-            oidcKey={this.props.oidcKey}
+            accessToken={accessToken}
            onSelect={onProjectSelect}
          />
        )}

        {project && !location && (
          <LocationPicker
-            oidcKey={this.props.oidcKey}
+            accessToken={accessToken}
            project={project}
            onSelect={onLocationSelect}
          />
        )}
        {project && location && !dataset && (
          <DatasetPicker
-            oidcKey={this.props.oidcKey}
+            accessToken={accessToken}
            project={project}
            location={location}
            onSelect={onDatasetSelect}
@ -143,7 +145,7 @@ class DatasetSelector extends Component {
        )}
        {project && location && dataset && (
          <DicomStorePicker
-            oidcKey={this.props.oidcKey}
+            accessToken={accessToken}
            dataset={dataset}
            onSelect={onDicomStoreSelect}
          />
--- 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) {
--- 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 {
        <Modal.Body>
          <DatasetSelector
            setServers={this.handleEvent}
-            oidcKey={this.props.oidcStorageKey}
+            user={this.props.user}
            url={this.props.url}
          />
        </Modal.Body>
--- 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);

--- 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) {
--- 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,
      },
    };
  }
--- 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 (
      <>
-        <ConnectedHeader home={true} user={this.props.user} />
+        <WhiteLabellingContext.Consumer>
+          {whiteLabelling => (
+            <UserManagerContext.Consumer>
+              { userManager => (
+                <ConnectedHeader home={true} user={this.props.user} userManager={userManager}>
+                  {whiteLabelling.logoComponent}
+                </ConnectedHeader>
+              )
+              }
+            </UserManagerContext.Consumer>
+          )}
+        </WhiteLabellingContext.Consumer>
        {studyList}
      </>
    );
--- 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);
--- 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"