Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add session timeout support #8250

Merged
merged 6 commits into from
Dec 15, 2020
Merged

Add session timeout support #8250

merged 6 commits into from
Dec 15, 2020

Conversation

jakemcdermott
Copy link
Contributor

@jakemcdermott jakemcdermott commented Sep 28, 2020 •
edited
Loading

SUMMARY

Intercept HTTP(S) responses and store expiration time from headers in web storage. Drive expiration timers in app container across all tabs with browser storage events and accompanying react hooks integration. Show a warning with logout countdown and continue button when session is nearly expired.

Local Storage Integration

The useStorage hook integrates with the browser's localStorage api. It accepts a localStorage key as its only argument and returns a state variable and setter function for that state variable. When local storage is shared, the hook enables bidirectional data transfer between tabs via an event listener that is registered with the Web Storage api. This means that updates to the state variable using the setter function on one tab will also update the state variable on any other tab using this hook with the same key and vice-versa.

function useStorage(key) {
  const [storageVal, setStorageVal] = useState(
    window.localStorage.getItem(key)
  );
  window.addEventListener('storage', () => {
    const newVal = window.localStorage.getItem(key);
    if (newVal !== storageVal) {
      setStorageVal(newVal);
    }
  });
  const setValue = val => {
    window.localStorage.setItem(key, val);
    setStorageVal(val);
  };
  return [storageVal, setValue];
}

Sequence Diagram for useStorage

The useStorage hook currently lives in the AppContainer component. It can be relocated to a more general location should and if the need ever arise

Session Expiration

Session timeout state is communicated to the client in the HTTP(S) response headers. Every HTTP(S) response is intercepted to read the session expiration time before being passed into the rest of the application. A timeout date is computed from the intercepted HTTP(S) headers and pushed into local storage, where it can be read using standard Web Storage apis or other utilities, such as useStorage.

ADDITIONAL INFORMATION

session

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@keithjgrant
Copy link
Member

addresses #4205

@nixocio
Copy link
Contributor

nixocio commented Sep 29, 2020 •
edited
Loading

Neat Diagram.

Edit: Helpful as well!

@jakemcdermott jakemcdermott force-pushed the session-timeout branch 7 times, most recently from 97ab1c1 to bbb9897 Compare September 29, 2020 18:37
marshmalien
marshmalien approved these changes Sep 29, 2020
View reviewed changes
Copy link
Member

@marshmalien marshmalien left a comment

Choose a reason for hiding this comment

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

Thank you for writing detailed and thorough documentation! Found a couple issues and discussed them with @jakemcdermott. One issue was due to the API allowing a delay of ~950 years, but JavaScript timers (setInterval and setTimeout) allow a max delay of 2147483647 seconds.

The fixes look good to me! I only have one nitpick but otherwise 👍

awx/ui_next/src/components/AppContainer/AppContainer.jsx Outdated Show resolved Hide resolved
@jakemcdermott jakemcdermott force-pushed the session-timeout branch 2 times, most recently from bdeb6f1 to 39a532f Compare September 29, 2020 19:34
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@unlikelyzero unlikelyzero added the type:feature prioritized on a feature board label Sep 29, 2020
@unlikelyzero
Copy link

@jakemcdermott we won't be adding automated coverage for session timeouts. I would like to put this into needs_test to 🔨 on it for an hour. Let me know when it's in a state that's appropriate -- even it if is just a needs_test ticket post merge

@jakemcdermott
Copy link
Contributor Author

@unlikelyzero Sounds good. I intend to move this over to needs test in the next day or so. I'll add the tag and ping.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@unlikelyzero
Copy link

May need to add cy.reload and network captures to AfterEach

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@tiagodread
Copy link
Contributor

Added stubs for session time out in 5743

@jakemcdermott jakemcdermott mentioned this pull request Dec 11, 2020
Closed
@unlikelyzero
Copy link

unlikelyzero commented Dec 13, 2020 •
edited
Loading

note: retest this #4745 and this #7417

@unlikelyzero unlikelyzero mentioned this pull request Dec 13, 2020
Closed
@jakemcdermott
Add session timeout support
5ccee4a 
Intercept all http(s) responses and store expiration time from headers
in local storage. Drive expiration timers in app container across all
tabs with browser storage events and accompanying react hooks
integration. Show a warning with logout countdown and continue button
when session is nearly expired.
@jakemcdermott
Debounce storage calls on http intercept
838a382
@jakemcdermott
Clamp timeout to maximum allowed by js
d7949e3
@jakemcdermott
Add logout button, remove close button
aa04804
@jakemcdermott
Unchunkify session warning string
997200d
@jakemcdermott
Don't display a negative time
487bf50 
If the remaining session time dips below 0 imediately before auto-
logout, ceil the display value to 0 to avoid showing negative
seconds left.
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit de0967a into ansible:devel Dec 15, 2020
@jakemcdermott jakemcdermott mentioned this pull request Dec 15, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nixocio nixocio nixocio left review comments

@marshmalien marshmalien marshmalien approved these changes

Assignees

@unlikelyzero unlikelyzero

Labels
type:feature prioritized on a feature board
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jakemcdermott @keithjgrant @nixocio @unlikelyzero @tiagodread @marshmalien