Page MenuHomePhabricator

RfC: Session storage service interface
Closed, ResolvedPublic

Description

Develop a Request for Comment detailing the API of a new multi-master session storage service.

Let's use the RfC's talk page for discussion regarding the text of the RfC.

Event Timeline

Eevans renamed this task from RfC: Multi-DC Session Storage Service to RfC: Session storage service interface.Oct 2 2018, 6:53 PM
Eevans claimed this task.
Eevans added a project: Platform Engineering.
Eevans updated the task description. (Show Details)
Eevans triaged this task as Medium priority.Oct 2 2018, 7:31 PM
Eevans updated the task description. (Show Details)
Eevans updated the task description. (Show Details)
Eevans added a subscriber: CCicalese_WMF.
Eevans updated the task description. (Show Details)
Eevans updated the task description. (Show Details)

The TechCom IRC meeting for this is scheduled for Wednesday 14 November 2pm PST(22:00 UTC, 23:00 CET) in #wikimedia-office.

22:27:36 <bawolff> So one feature that is requested sometimes is having the ability for users to view/terminate other sessions (like i think gmail lets you). I guess this interface would preclude that

Not really. We can currently allow a user to blindly terminate all sessions by updating user.user_token (or globaluser.gu_auth_token for CentralAuth). An ability to view and selectively terminate the other sessions would IMO better be done by adding another table to store multiple such tokens per user (plus UA or whatever other identifying data) rather than by trying to enumerate the session IDs themselves from the storage backend.

22:29:16 <TimStarling> or you know, extend BagOStuff, then you have an obvious way to provide CAS etc.

FWIW, having a new store implement a BagOStuff was the plan when SessionManager was written.

22:30:58 <legoktm> duesen: right now when you log out, it kills all of your sessions, which is a pain for people who log in on their phones and potentially public computers

I believe that's only with CentralAuth. Stock MediaWiki doesn't change the token on logout.

22:38:38 <urandom> as for TTL, I'd add that as an attribute to the JSON payload of the PUT

Side note: to someday implement BagOStuff::changeTTL() directly in the service (versus generic read-and-rewrite), that could be done as a PUT that specifies the TTL without specifying a new value.

22:39:48 <duesen> should we go back to discussing whether we need a standalone service at all, or just want CassandraBagOStuff, and then think about a "full" session/login/auth services?

Keep in mind that MediaWiki session/login/auth needs to still work for non-WMF installations that don't have the fancy service.

The plan when AuthManager and SessionManager were originally written was that a login service could interface with MediaWiki as a PrimaryAuthenticationProvider, while a different storage backend for session data could be a BagOStuff. The two aren't intended to know anything about each other.

From what various people seem to think an "authn" service should do, it seems it would have to be something different from both the login service and the session-storage service. It would basically be something that replaces user.user_token/globaluser.gu_auth_token and methods like User::getToken() and User::setToken(), more or less along the lines of the "table to store multiple such tokens per user" I mentioned above (in fact, the stock implementation in core would use that table). On a successful login AuthManager would acquire a new token from the authn service and inject it into the Session. Loading of the Session (whether from the session store or a remember-me Token cookie) would check the session's token against the authn service. And logout would invalidate the current session's token in the authn service.

22:48:30 <bawolff> Are these dumb blobs going to be json (as in please dont make it php serialized stuff)

Note that existing code may be putting PHP objects into the session data, expecting them to be serialized. PHP's built-in session handling specifically allows that sort of thing, and SessionManager is currently compatible with that, as is BagOStuff.

Some changes to the RFC document have been made since the IRC meeting and ensuing discussions.

  1. The data representation has been changed from application/json to application/octet-stream to better reflect that this is an opaque key-value store; Something more sophisticated may be needed in the future (key-map, for example), but any such API can be defined as a different endpoint.
  2. The set operation (which remains an upsert) has been changed from a PUT to POST. If (when?) set-if-not-exists semantics are needed, that can (will?) be implemented as PUT (which fails with a 4xx status if the key already exists).
  3. The document continues to state that TTL values are a configuration of the service (that the same TTL applies to all writes). However, a note has been added to state that if (when?) we commit to client-supplied TTLs, they will be supplied by a Cache-Control header on the request, and will override the aforementioned default. It has been suggested that client overrides of the TTL only allow a reduction in the default, this specific behavior remains an open question.
  4. A note has been added indicating that an open question pertaining to the URI namespace of RFC7807 errors remains, and needs to be addressed.

@Eevans do you think that this can go on last call, with the adjustments mentioned above? If you think it's ready, TechCom couöd put it on last call next week (if all agree).

In TechCom there was some discussion about whether this should be a standalone service at all, though, and how that question fits into the RFC discussion. Please sync with @tstarling and @Joe on this.

@Eevans do you think that this can go on last call, with the adjustments mentioned above? If you think it's ready, TechCom couöd put it on last call next week (if all agree).

I think it can go on last call, yes.

In TechCom there was some discussion about whether this should be a standalone service at all, though, and how that question fits into the RFC discussion. Please sync with @tstarling and @Joe on this.

Will do.

TechCom is placing this on Last Call ending Wednesday December 12th 1pm PST(21:00 UTC, 22:00 CET)

@Eevans still coordinate with @tstarling and @Joe regarding if this should be a service or not. TechCom determined there was no issue putting this RFC on Last Call while that discussion still happens.

TechCom has approved this, noting performance discussions are outside of the scope of this RFC.

TechCom has approved this, noting performance discussions are outside of the scope of this RFC.

Neat; AFAICT this is done