silverstripe/silverstripe-session-manager

Render list of Sessions

Closed this issue · 8 comments

Overview

With the scaffolding work complete in #11, now the field needs to be filled with Session data.

Acceptance Criteria

  • Pass session list for given Member to SessionManagerField through schema
  • Render sessions based on designs
  • Last active hours converts to ‘days’ after 24 hours
  • For the current session, display the current IP address (not currently in the designs)
  • The current browser is being rendered with the ‘Current’ label
  • Common user agent definitions (browsers/ devices) are tested and the label reviewed
  • Readme is updated along with the image of the UI
  • The old Sessions tab is removed
  • The "Log Out" links prompt a confirmation warning before removing the session
  • The ‘Devices’ label has been reviewed for usefulness
  • It's been reviewed if there's usefulness in including a timestamp related to the current session data.
  • The order of this component in the Member UI is verified with PUX

Notes

  • The MFA component has a fairly similar layout so you may see some hints for building this out there.
  • Consider the future need of being able to add a geo-location in replacement of the IP address. Can this be built in an extensible way now?

PR

It's been reviewed if there's usefulness in including a timestamp related to the current session data.

@clarkepaul, FYI the module is already storing the 'signed in' timestamp as well as the 'last accessed' timestamp. Maybe that would be useful here? See: screenshot of current behaviour

Just noting that the "Signed in" information will not be visible with the new designs:
image

May be worth adding this again.

Thoughts @clarkepaul ?


It's also worth noting that these changes may cause more of a maintenance burden in the long run as we are essentially replicating what the existing grid field does, but as a one-off solution not used anywhere else.
Some people may also prefer having data in a separate tab, instead of appending it to the main area.
In addition to that, the Learn More link in the designs are not a part of our current API.

All valid points. The designs were done to align with the managing of MFA methods and were done prior to me seeing a screenshot of this module (I thought it was needing to be a new UI). I haven't personally stepped through that experience e.g. what happens with revoking sessions and notifications but I think its worth another conversation around whether its worth creating a new UI when this might be sufficient as an MVP.

Just noting that the "Signed in" information will not be visible with the new designs

I recall there being a bit of back and forth on whether that was something the team saw value in. It was purposefully omitted from the designs. We were not sure at the time of grooming so added the following AC to cover discussion on the topic:

  • It's been reviewed if there's usefulness in including a timestamp related to the current session data.

I think its worth another conversation around whether its worth creating a new UI when this might be sufficient as an MVP.

In regards to an MVP, the scaffolding and UI is in a final stage of peer review and is pretty much done. So there's not really 'additional' effort in shipping this.

I've also noted this in the issue on 2 Feb

I've just seen the UI in a live demo, I think we can follow the provided designs with some minor tweaks to allow the full date to show on hover if its an easy thing to do. If that is difficult then I think its fine to leave to leave the Signed in date out. I did this quick screen shot to show how it could be done but will update the design accordingly. I also mentioned BS tooltips are nicer than title text but see how we go.
image

... and on the current view swap the order of "Current" and 'IP" so it matches the other sessions. Just going to add dates to design for signed in on the current session to see if it's good or not.
image

Just posted an update of the designs @bergice, basically a catchup of designs already discussed hopefully. Some new stuff with the section label and logout action.

For the section label, I've taken a look around to see what would make the most sense to people and think retaining "Device" should make sense. That seems to fit closer to the majority scenario of a different physical device. I think we should make it slightly clearer with the addition of "Authenticated devices" so that people have more context about it as login/session type information. In saying that, session/device both seem to work well, so happy to hear if there is any objection to this.

For the logout transition (although not part of this issue), this was designed without physically testing it out so keen to work with whoever implements it to adjust as needed if not quite right.