Skip to content
This repository has been archived by the owner on Jun 19, 2019. It is now read-only.

Improve token timeout behaviour #153

Merged
merged 9 commits into from
Apr 26, 2018

Conversation

rjw57
Copy link
Member

@rjw57 rjw57 commented Apr 13, 2018

This PR addresses #98 and has the following dependent PRs:

It is quite a large PR and is best reviewed commit-wise since most commits have somewhat involved comments.

The main body of the PR is composed of two commits, 6c6e3af and 5dc698e.

6c6e3af replaces our existing use of redux-implicit-oauth2 with our own version of implicit OAuth2 flow (implemented in 86ecd15) which distinguishes between normal and "promptless" logins. The latter logins do not require a popup window but may fail.

5dc698e makes use of the new OAuth2 login implemented by 6c6e3af in order to provide a nicer UX on token timeout. See that commit's comment for more information. The commit itself includes notes on how it may be tested.

Since token timeout behaviour rarely appears in development, 87deb90 takes the opportunity to configure the access token lifetime in development to be 5 minutes. This is done in the hope that such a short lifetime will quickly surface token timeout problems.

78b2b6e fixes a small visual bug with snackbars discovered during the implementation of 5dc698e.

Some aspects of this PR are a little under-tested. 5dc698e and 86ecd15 are particular examples of this. The problem is that both essentially rely on the behaviour of the browser. The former performs tricks with timeouts and the latter performs tricks with the window object. They would be better tested with a full browser environment but our test suite does not currently provide that. In lieu of automated testing, 5dc698e contains a description of a procedure which can be used to test both token timeout failure modes.

Closes #98

This commit addresses a small bug found when implementing token timeout
notifications. These notifications are slightly longer than others we've
been using.

Previously the snackbar message was reset when reducing the
SNACKBAR_CLOSE action. This caused a small visual bug because the
snackbar animates shut when closing and the message just blanks as soon
as the animation starts.

This is less visible for short messages, but for long messages the
snackbar visibly "jumps" in size as it closes.

Fix the problem by simply keeping the message as is when reducing the
SNACKBAR_CLOSE action.
When running in development, configure hydra to have an access token
lifetime of 5 minutes. This will help surface any token refresh issues
in development.
We need to extend the existing OAuth2 login flow so as a first step we
need to install a lower-level OAuth2 client library.
In order to implement interactive and non-interactive login, we need a
slightly lower-level access to the OAuth2 flow. Move one step down the
chain and use Mulesoft's OAuth2 client to implement implicit flow
ourselves.

This commit contains the implementation of the OAuth2 implicit flow but
does not replace the existing uses of redux-implicit-oauth2.

The new implicit flow implementation also fixes a long-standing niggle
with the existing implementation which would repeatedly poll the window
to see when the OAuth2 callback had been called spamming the error log
as it did so. This new implementation replaces the polling with an
event-driven model using postMessage.

Unfortunately, our existing callback URL was missing the .html extension
which just makes it a little non-trivial to make a like-for-like
replacement in a clean manner. Add a simple static file to
public/oauth2-callback.html which provides the other end of the
postMessage implementation. Update create-client.sh to add this as a
redirect URL.

**IMPORTANT:** this will necessitate changes to the IAR deployment.

A benefit from this is we no-longer need to special case oauth2-callback
in our routing when we replace the existing implementation.
Replace the existing redux-implicit-oauth2 authorisation with a
work-alike which uses the same format for the redux state.

This is a little bit of a halfway-horse; ideally we'd store the full
client-oauth2 token object in the state but a lot of the rest of the app
depends on the state having the same form and this commit is intended to
be a small surgical change.

Since we already hid the redux-implicit-oauth2 actions inside the
src/redux/actions/auth.js module, replacing them is relatively
straightforward. We add the ability to do prompt-less login via the
login action. This is currently unused but is in place for the token
timeout handling behaviour.

We keep the existing behaviour of keeping the auth token in localStorage
so that re-visits to the app can have speedy login. We load it as a
by-product of initialising the local state which means we can do away
with the existing auth middleware.

Since we no-longer use the redux-implicit-oauth2 implementation, we can
remove the special-case route in AppRoutes for it.
We no longer use this module so remove it as a dependency.
Add a new component, TokenTimeout, which maintains an internal timer
which fires just before the access token times out. On time out, attempt
a prompt-less login to refresh the token. If this login fails, log the
user out with an informative message.

There's no link between the consent app's idea of which user is
currently signed in and the IAR's so on a successful prompt-less login,
re-fetch the user's lookup identity and compare it against the identity
of the user who first logged in. If they do not match, log the current
user out with an informative message.

This latter check is only likely to be triggered if another user had
logged into the IAR (or another OAuth2 protected site) within the same
browser context in the interim.
@codecov-io
Copy link

codecov-io commented Apr 13, 2018

Codecov Report

Merging #153 into master will decrease coverage by 4.76%.
The diff coverage is 43.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #153      +/-   ##
==========================================
- Coverage   81.95%   77.18%   -4.77%     
==========================================
  Files         107      110       +3     
  Lines         992     1131     +139     
  Branches      167      194      +27     
==========================================
+ Hits          813      873      +60     
- Misses        137      200      +63     
- Partials       42       58      +16
Impacted Files Coverage Δ
src/containers/AppRoutes.js 60% <ø> (+10%) ⬆️
src/containers/App.js 100% <ø> (ø) ⬆️
src/test/mock-localstorage.js 100% <ø> (ø) ⬆️
src/redux/reducers/index.js 100% <ø> (ø) ⬆️
src/redux/reducers/snackbar.js 60% <0%> (ø) ⬆️
src/redux/enhancer.js 75% <100%> (ø) ⬆️
src/config.js 100% <100%> (ø) ⬆️
src/redux/reducers/auth.js 100% <100%> (ø)
src/auth.js 12.82% <12.82%> (ø)
src/containers/TokenTimeout.js 30.3% <30.3%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b9b220...e0fbed8. Read the comment docs.

*
* An example test protocol to check token timeout behaviour in development would be as follows:
*
* 1. Edit TIMEOUT_HEAD_ROOM to be 280e3. In development the access token lifetime if 5 minutes and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is

@rjw57 rjw57 requested a review from a team April 23, 2018 15:23
@msb msb merged commit 2571291 into uisautomation:master Apr 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Token timeout behaviour is annoying to users
4 participants