NOJIRA sign out of all synchronized tabs

[solidgoldpig]

Sign out of all synchronised tabs

Ensure user is signed out of all synchronised tabs that are open.

Fixes #522

Checklist

• I've read the CONTRIBUTING guidance, including next steps and expected response from the owners
• I've added appropriate unit tests, and run all unit tests
• I've run the visual regression test using Backstop and updated the images if needed
• I've run npm version minor to update the version in package.json and package-lock.json
• I've updated the CHANGELOG

[hmrc-web-operations]

Thank you for submitting a pull request on this HMRC repository. A member of the HMRC team will review your changes and action accordingly.

HMRC reviewer: please reference this runbook for guidance on how to proceed.

[hmrc-web-operations]

Thank you for submitting a pull request on this HMRC repository. A member of the HMRC team will review your changes and action accordingly.

HMRC reviewer: please reference this runbook for guidance on how to proceed.

[oscarduignan]

a lot of hmrc services share a domain tax.service.gov.uk so if we implemented this change there would be some services with the new js and some on the old - I think the way that sessionActivityService is implemented at the moment - broadcasting signOut event through the same channel would look make it look like session activity to other tabs because it doesn't discriminate type of event at the moment - not sure what that would result in (though somewhat might be same as current - in that once user has signed out, and they hit another page of a tax service the session activity might be broadcast from that new page - though lack of timestamp that it expects from session activity might cause an error maybe)

some other thoughts

• I wonder if there's some risk of background tabs being paused or off loaded in someway by browsers and then when reopened old sign out messages being broadcast and it destroying a new session the user has unintentionally
• on hmrc side, I wonder if the auth service will handle several sign out requests at the same time gracefully (where I think for a lot of services the timeout and sign out urls will be the same - because all requests to the kind of hmrc services we support on tax.service.gov.uk extend session - so if you displayed a timeout error page then it could end up extending session you have said is timed out)

I think I'll need to think about it a bit more thoroughly - it makes sense to me - but might need to be extra defensive in how it's implemented / make it so you need to opt-in somehow such that we don't turn it on by default for stuff on tax.service.gov.uk so we didn't break some unknowingly delicately balanced thing

[solidgoldpig]

Fair enough. Happy to add another option

synchroniseTabsSignout
data-synchronise-tabs-signout

Would that work for you?

[oscarduignan]

yup that would make it easier to add for us I think - when we first implemented the syncing - it was just to stop a background tab being able to take out the session - and we flagged up there were still gaps / not great behaviours (like if data was entered in form on background tab, without this the data could sit in there filled in indefinitely- and as you say have it seem like session was still active) but we weren't confident we wouldn't pull wrong jenga piece and cause a live incident of some kind

[solidgoldpig]

Pushed an updated version.

[hmrc-web-operations]

Thank you for submitting a pull request on this HMRC repository. A member of the HMRC team will review your changes and action accordingly.

HMRC reviewer: please reference this runbook for guidance on how to proceed.

[oscarduignan]

apologies! Taken me longer than I thought to come back, I am still a bit unsure about scenario where someone signs out after updating to a version of hmrc-frontend with this - it seems because of the reuse of session activity channel - and because events in that channel aren't discriminated in any way - like at the moment any message to it for someone on current version would be presumed to be session activity (that should keep session alive)

so if message { signedOut: true } is sent then any services using old version will run:

const timeOfActivity = event.timestamp;
cleanup();
setupDialogTimer(timeOfActivity);

I'm not actually sure how default params work for js functions - if passing undefined here will cause the default to be used, or if it will cause it to be undefined

hmrc-frontend/src/components/timeout-dialog/timeout-dialog.js

Line 127 in 01a9ae5

const setupDialogTimer = (timeOfLastActivity = getDateNow()) => {

it might be I guess that it won't break anything but will cause background tabs timeout timers to reset - where they might get reset anyway, if the foreground tab which was explicitly signed out lands on a page with timeout dialog on it and some session activity from new session is broadcast

---

then one other thing I notice with this, background tabs will rebroadcast the sign out event - so if you have 4 tabs, and you sign out in 1, so there's the chance for some kind of sign out loop or oddness

like if someone signs out in one window, they get redirected immediately to sign out endpoint and may end up after that on a page with timeout dialog on it again - if that was case and then there is the chance that sign out events broadcast from background pages would hit pages from the new session and could theoretically create some kind of sign out loop

I guess the bits that are making stuff difficult:

• lack of some kind of discriminator of event types making it hard to extend session activity channel with support for new event types
• lack of some kind of discriminator of session within which event occurred making it hard to not discard events that maybe shouldn't be handled if they were for a different session

---

this is kind of hitting on the past stuff we had with this I think where it feels really complicated and like there are quite a lot of risks in terms of architecture of frontend services in HMRC

[oscarduignan]

timeout dialog code just seems to be quite a precarious jenga tower to make changes too - it might MIGHT be easier to fork it and modify it for your purposes - than try to change it upstream - and then for the future maybe it's the kind of thing that would be better reimplemented with with the bits making stuff difficult considered upfront as part of the design -where we kind of patched in this session awareness stuff