WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 3 months ago

#30264 closed task (blessed) (fixed)

Users should have a UI for managing sessions

Reported by: jorbin Owned by: johnbillion
Milestone: 4.1 Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch commit fixed-major i18n-change
Focuses: Cc:

Description (last modified by ocean90)

After the work to add logged-in session code under the hood in 4.0, We should aim to provide users with controls on the user profile screen and user editing screen for managing logged-in sessions.

Feature Plugin: https://github.com/johnbillion/wp-session-manager

Attachments (6)

30264.diff (5.8 KB) - added by jorbin 2 years ago.
30264.2.diff (5.8 KB) - added by jorbin 2 years ago.
30264.3.diff (5.8 KB) - added by jorbin 2 years ago.
30264.4.diff (6.4 KB) - added by ocean90 2 years ago.
30264.5.diff (6.7 KB) - added by johnbillion 2 years ago.
30264.6.diff (733 bytes) - added by DrewAPicture 2 years ago.
Periods.

Download all attachments as: .zip

Change History (31)

#1 @ocean90
2 years ago

  • Description modified (diff)

This ticket was mentioned in Slack in #core by drew. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.


2 years ago

#4 @johnbillion
2 years ago

  • Owner set to johnbillion
  • Status changed from new to assigned

@jorbin
2 years ago

#5 @jorbin
2 years ago

  • Keywords has-patch added

As we had discussed in the dev chat, I've written up a patch that adds a button to the profile page for destroying sessions and also for capturing IP address, User Agent, and session creation time.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by johnbillion. View the logs.


2 years ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.


2 years ago

@jorbin
2 years ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.


2 years ago

@jorbin
2 years ago

#10 @jorbin
2 years ago

The new version cleans things up a bit based on feedback from ocean90 and johnbillion.

@ocean90
2 years ago

#11 @ocean90
2 years ago

30264.4.diff:

  • Adds missing p tag for inline messages
  • Adjusts styling for inline messages
  • Fixes sprintf( __( 'Log $s out of all sessions' ), $profileuser->display_name );
  • Simplifies button selector to #destroy-sessions
  • Caches jQuery selector
  • Renames sessionmanager to _wpSessionMangager
  • Adds translators comments

@johnbillion
2 years ago

#12 @johnbillion
2 years ago

  • Keywords commit added

30264.5.diff is the final pass at this. Additions:

  • Generic error message to avoid introducing multiple strings
  • Only displays the button on the profile screen when there are sessions which can be cleared for the user
  • Actually store the additional session information against the session

#13 @johnbillion
2 years ago

In 30333:

Introduce a button on the user profile screen which clears all other sessions, and on the user editing screen which clears all sessions. Only appears when there are applicable sessions which can be cleared.

See #30264.
Props jorbin, ocean90, johnbillion

#14 @johnbillion
2 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

Marking as fixed for 4.1. Future enhancements can go into another ticket.

#15 @ocean90
2 years ago

In 30334:

Improvements to [30333]:

  • Move .hide-if-no-js class to table row
  • Add a wrapper class
  • Add missing translators comment

see #30264.

@DrewAPicture
2 years ago

Periods.

This ticket was mentioned in Slack in #polyglots by drew. View the logs.


2 years ago

#17 @SergeyBiryukov
2 years ago

In 30596:

Add missing periods to strings introduced in [30333].

props DrewAPicture.
see #30264.

#18 @nacin
2 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This implementation can currently leak a session token when there is no actual need to do so. While cookies are HTTP Only, the current session token is available on profile.php via JavaScript. All we need to do is check if the current user is the user being edited, and then trash all other cookies. Otherwise it's also possible to log yourself out this way when it isn't designed like that. That's not a CSRF issue as you have a nonce also blocking this, but we can't be leaking a piece of a cookie like this.

I'm making a few other code changes:

  • Re-use the update-user_{$id} nonce. No need for a separate nonce here at this time.
  • Don't localize any values, we can get them from the form (especially since we're reusing a nonce).
  • Use wp_verify_nonce() rather than check_ajax_referer() because there's no benefit to check_ajax_referer(), and the double false actually tripped me up pretty badly.
  • Use wp.ajax from wp-util.js rather than parsing the response from wp_send_json() ourselves.
  • I'm giving our error/success messages the proper padding by overriding a margin style on .form-table td p.

Finally, I'm making a last-minute UX change:

More than a few testers have wondered why there's no button on the profile page. This may be because we mention this on the about page. It also may be because someone might tell someone else "go here and do that" and the next thing they're going to say is "I don't understand, where is it?" rather than think "OK, I'm good." markjaquith and I both think there should be a disabled button normally. The language is "You are only logged in at this location."

(Note this uses count() === 1, not count() <= 1, because if sessions are for some reason disabled, we shouldn't show anything.)

#19 @nacin
2 years ago

  • Keywords fixed-major added

#20 @nacin
2 years ago

In 30888:

Updates to the 'Log out everywhere' implementation.

  • Include a message and a disabled button when you're only logged in at one location.
  • Avoid leaking the session token in HTML.
  • Simplify, simplify, simplify.

see #30264.

#21 @johnbillion
2 years ago

  • Keywords i18n-change added

Nice improvements.

#22 @johnbillion
2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 30895:

Updates to the 'Log out everywhere' implementation.

  • Include a message and a disabled button when you're only logged in at one location.
  • Avoid leaking the session token in HTML.
  • Simplify, simplify, simplify.

Merges [30888] to the 4.1 branch.

Fixes #30264.

This ticket was mentioned in Slack in #forums by jeffr0. View the logs.


2 years ago

This ticket was mentioned in Slack in #forums by jeffr0. View the logs.


2 years ago

#25 @SergeyBiryukov
3 months ago

In 39908:

I18N: Reference correct placeholder in a translator comment added in [30333].

See #30264.

Note: See TracTickets for help on using tickets.