Skip to content

Commit

Permalink
STCOR-671 handle access-control via cookies and RTR 👋 🔄 🔒 😅 (#1376)
Browse files Browse the repository at this point in the history
Move auth tokens into HTTP-only cookies and implement refresh token
rotation (STCOR-671) by overriding `global.fetch` and
`global.XMLHttpRequest`, disabling login when cookies are disabled
(STCOR-762). This functionality is implemented behind an opt-in
feature-flag (STCOR-763). The core RTR logic here is largely the same as
it was in PR #1346 😬 , though with several important differences:

1. No buggy service-worker
2. Handle `fetch` and `XMLHttpRequest`
3. Disable login if cookies are disabled
4. Everything is opt-in 😌

Not _everything_ in PR #1346 was awful, despite it being reverted in
#1371 😬 . The fundamental difference here is that the global `fetch`
and `XMLHttpRequest` functions have been replaced 🤢 by new
implementations that handle RTR instead of intercepting such requests
via the service-worker proxy. This is not lovely. It is not elegant. It
isn't pretty in any way, but it is extremely simple and effective.
Certainly, we want to migrate away from it, but given the options we
thought it was best choice in the short-term. The options:

1. Centralized fix within stripes-core by fixing the service worker.
   Let's be honest, I didn't get it right in #1346 and then couldn't get it
   right in #1361 or #1363 or #1366 or #1369. Why would anybody possibly
   believe that I could get it right now?
2. Decentralized fix: handle this in each UI-* repository by exporting a
   new function from stripes and refactoring each UI repo to leverage the
   new code. Probably not a big refactor, but not a small effort.
3. Centralized fix within stripes-core by overwriting `global.fetch`.
   Gross, but effective, and long term we can make this a decentralized
   approach by exporting our new `fetch` function, doing the refactor
   described in 2 (above), and removing the global-overwrite once all the
   refactoring is done.

In summary:

* Replaces #1340. It was gross and I really don't want to talk about it.
  Let us never mention it again.
* Replaces #1346. It was a terrible, horrible, no good, very bad PR.
  Alexander hated that PR more than lima beans.

Additional requirements:

* Requires folio-org/stripes-connect#223
* Requires folio-org/stripes-smart-components#1397
* Requires folio-org/stripes-webpack#125

Refs STCOR-671, FOLIO-3627
  • Loading branch information
zburke authored Jan 16, 2024
1 parent 6872a92 commit 0361353
Show file tree
Hide file tree
Showing 42 changed files with 1,785 additions and 140 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
* Fix duplicated "FOLIO" in document title in some cases. Refs STCOR-767.
* Refactor away from `color()` function. Refs STCOR-768.
* Export `getEventHandler` to be able to create events in other modules. Refs STCOR-770.
* Opt-in: handle access-control via cookies. Refs STCOR-671.
* Opt-in: disable login when cookies are disabled. Refs STCOR-762.

## [10.0.0](https://github.com/folio-org/stripes-core/tree/v10.0.0) (2023-10-11)
[Full Changelog](https://github.com/folio-org/stripes-core/compare/v9.0.0...v10.0.0)
Expand Down
3 changes: 3 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ export * from './src/consortiaServices';
export { default as queryLimit } from './src/queryLimit';
export { default as init } from './src/init';

/* localforage wrappers hide the session key */
export { getOkapiSession, getTokenExpiry, setTokenExpiry } from './src/loginServices';

export { registerServiceWorker, unregisterServiceWorker } from './src/serviceWorkerRegistration';

export { getEventHandler } from './src/handlerService';
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"@formatjs/cli": "^6.1.3",
"chai": "^4.1.2",
"eslint": "^7.32.0",
"jest-fetch-mock": "^3.0.3",
"miragejs": "^0.1.32",
"moment": "^2.29.0",
"react": "^18.2.0",
Expand Down
5 changes: 0 additions & 5 deletions src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import configureLogger from './configureLogger';
import configureStore from './configureStore';
import gatherActions from './gatherActions';
import { destroyStore } from './mainActions';
import { unregisterServiceWorker } from './serviceWorkerRegistration';

import Root from './components/Root';

Expand All @@ -31,10 +30,6 @@ export default class StripesCore extends Component {
this.epics = configureEpics(connectErrorEpic);
this.store = configureStore(initialState, this.logger, this.epics);
this.actionNames = gatherActions();

// unregister any zombie service workers left over from RTR work
// prior to disabling RTR in PR #1371
unregisterServiceWorker();
}

componentWillUnmount() {
Expand Down
5 changes: 4 additions & 1 deletion src/RootWithIntl.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@ class RootWithIntl extends React.Component {
clone: PropTypes.func.isRequired,
}).isRequired,
token: PropTypes.string,
isAuthenticated: PropTypes.bool,
disableAuth: PropTypes.bool.isRequired,
history: PropTypes.shape({}),
};

static defaultProps = {
token: '',
isAuthenticated: false,
history: {},
};

Expand All @@ -67,6 +69,7 @@ class RootWithIntl extends React.Component {
render() {
const {
token,
isAuthenticated,
disableAuth,
history,
} = this.props;
Expand All @@ -85,7 +88,7 @@ class RootWithIntl extends React.Component {
>
<Provider store={stripes.store}>
<Router history={history}>
{ token || disableAuth ?
{ isAuthenticated || token || disableAuth ?
<>
<MainContainer>
<AppCtxMenuProvider>
Expand Down
2 changes: 2 additions & 0 deletions src/Stripes.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@ export const stripesShape = PropTypes.shape({
okapiReady: PropTypes.bool,
tenant: PropTypes.string.isRequired,
token: PropTypes.string,
isAuthenticated: PropTypes.bool,
translations: PropTypes.object,
url: PropTypes.string.isRequired,
withoutOkapi: PropTypes.bool,
}).isRequired,
plugins: PropTypes.object,
setBindings: PropTypes.func.isRequired,
setCurrency: PropTypes.func.isRequired,
setIsAuthenticated: PropTypes.func.isRequired,
setLocale: PropTypes.func.isRequired,
setSinglePlugin: PropTypes.func.isRequired,
setTimezone: PropTypes.func.isRequired,
Expand Down
17 changes: 16 additions & 1 deletion src/components/Login/Login.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,28 @@ class Login extends Component {
onSubmit,
} = this.props;

const cookieMessage = navigator.cookieEnabled ?
'' :
(
<Row center="xs">
<Col xs={6}>
<Headline
size="large"
tag="h3"
>
<FormattedMessage id="stripes-core.title.cookieEnabled" />
</Headline>
</Col>
</Row>);

return (
<Form
onSubmit={onSubmit}
subscription={{ values: true }}
render={({ form, submitting, handleSubmit, submitSucceeded, values }) => {
const { username } = values;
const submissionStatus = submitting || submitSucceeded;
const buttonDisabled = submissionStatus || !(username);
const buttonDisabled = submissionStatus || !(username) || !(navigator.cookieEnabled);
const buttonLabel = submissionStatus ? 'loggingIn' : 'login';
return (
<main>
Expand Down Expand Up @@ -82,6 +96,7 @@ class Login extends Component {
</Headline>
</Col>
</Row>
{ cookieMessage }
<div data-test-new-username-field>
<Row center="xs">
<Col xs={6}>
Expand Down
13 changes: 3 additions & 10 deletions src/components/MainNav/MainNav.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,14 @@ import { isEqual, find } from 'lodash';
import { compose } from 'redux';
import { injectIntl } from 'react-intl';
import { withRouter } from 'react-router';
import localforage from 'localforage';

import { branding, config } from 'stripes-config';

import { Icon } from '@folio/stripes-components';

import { withModules } from '../Modules';
import { LastVisitedContext } from '../LastVisited';
import { clearOkapiToken, clearCurrentUser } from '../../okapiActions';
import { resetStore } from '../../mainActions';
import { getLocale } from '../../loginServices';
import { getLocale, logout as sessionLogout } from '../../loginServices';
import {
updateQueryResource,
getLocationQuery,
Expand Down Expand Up @@ -123,12 +120,8 @@ class MainNav extends Component {
returnToLogin() {
const { okapi } = this.store.getState();

return getLocale(okapi.url, this.store, okapi.tenant).then(() => {
this.store.dispatch(clearOkapiToken());
this.store.dispatch(clearCurrentUser());
this.store.dispatch(resetStore());
localforage.removeItem('okapiSess');
});
return getLocale(okapi.url, this.store, okapi.tenant)
.then(sessionLogout(okapi.url, this.store));
}

// return the user to the login screen, but after logging in they will be brought to the default screen.
Expand Down
27 changes: 27 additions & 0 deletions src/components/Root/Errors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/* eslint-disable import/prefer-default-export */
/* eslint-disable max-classes-per-file */

/**
* RTRError
* Error occured during rotation
*/
export class RTRError extends Error {
constructor(message) {
super(message ?? 'Unknown Refresh Token Error');

this.name = 'RTRError';
}
}

/**
* UnexpectedResourceError
* Thrown when
*/
export class UnexpectedResourceError extends Error {
constructor(resource) {
super('Expected a string, URL, or Request but did not receive one.');

this.name = 'UnexpectedResourceError';
this.resource = resource;
}
}
5 changes: 5 additions & 0 deletions src/components/Root/Events.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/** dispatched during RTR when it is successful */
export const RTR_SUCCESS_EVENT = '@folio/stripes/core::RTRSuccess';

/** dispatched during RTR if RTR itself fails */
export const RTR_ERROR_EVENT = '@folio/stripes/core::RTRError';
Loading

0 comments on commit 0361353

Please sign in to comment.