Topic on Talk:Requests for comment/SessionStorageAPI

SessionManager:

Except for the lack of a client-specified TTL, this proposal looks like it has functionality sufficient to meet the needs of SessionManager.

Lack of support for a client-specified TTL isn't critical here. The main problem would come if the service TTL is less than half of the client-specified TTL, as that would prevent SessionManager's refresh logic from working.

CentralAuth:

CentralAuth uses the session storage backend for its own session-like data. It seems to have similar requirements as core SessionManager.

OAuth:

OAuth uses the session storage backend for storing tokens for its authentication process. For the most part it looks like this proposal has functionality sufficient to meet its needs, with two caveats:

• OAuth uses a "set only if unset" call in several places. While that could be implemented as a GET to check unsetness followed by a PUT, such a mechanism is open to races.
• OAuth seems to rely on short timeouts for security-related features. The service not honoring client-specified timeouts might allow things to be used for much longer than intended (or prevent nonce reuse for much longer than intended).

BagOStuff:

All of the above interact with the storage service via the BagOStuff PHP interface, and it's reasonable that people maintaining such code would expect the features of BagOStuff to generally work and might be surprised if the BagOStuff backed by your service doesn't.

Fortunately, the base BagOStuff class seems to make very few assumptions of the backend. The main shortcomings are already noted above: client-side TTLs and a non-racey implementation of add(). Implementing those two features should make almost everything else work reasonably well:

• getWithSetCallback() doesn't seem to be documented as such, but I believe the intention is that the callback is supposed to be a lookup where any races would return the "same" data.
• getMulti() and setMulti() don't seem to offer snapshot or atomicity guarantees, they're just sugar for a loop doing get() or set().
• lock() is built on top of add() and short TTLs, and unlock() is just delete().
• merge() by default uses lock(), although if you implement CAS it could use that instead.
• incr() uses lock(), decr() is just a negated incr(), and incrWithInit() uses incr() and add().

The only other thing is changeTTL(), which by default is racey. I don't know why it doesn't use lock().

It has been suggested elsewhere that BagOStuff might be the wrong interface for sessions. To clarifiy: What is the intersection between the enumerated list above, and the features needed for session management?

SessionManager uses get(), set(), and delete(). It uses client-specified TTLs, although as long as the server's TTL isn't too short compared to $wgObjectCacheSessionExpiry it shouldn't be much of a problem.

Keep in mind that moving SessionManager, CentralAuth, and OAuth all off of BagOStuff (or changing the latter two to a variable other than $wgSessionCacheType) could well be more work than just implementing TTLs and add(). Particularly when you remember that session management has to continue to work for third-party wikis that don't install your new service.

Perhaps it would be worth to investigate whether a narrower interface is more appropriate for session management. Given that it would be narrower by definition, it could be made to have a BagOStuff fallback, which alleviates the third-party concern.

On the other hand, as I pointed out, that would require changing the exiting code that configures and uses the BagOStuff. You'd probably wind up making the configuration more complex for all users to save yourself a tiny bit of complexity in implementing the interface to your service.

OAuth uses a "set only if unset" call in several places. While that could be implemented as a GET to check unsetness followed by a PUT, such a mechanism is open to races.

This could be supported using Cassandra lightweight transactions The performance penalty will have to be payed (fairly significant penaly, since, quote "the latency of operations increases fourfold due to the due to the round-trips necessary between the CAS coordinators").

In the RESTful API this feature could be exposed with supporting If-None-Match: * header for the PUT request, since per RFC7232 "If-None-Match can also be used with a value of "*" to prevent an unsafe request method (e.g., PUT) from inadvertently modifying an existing representation of the target resource when the client believes that the resource does not have a current representation (Section 4.2.1 of ). This is a variation on the "lost update" problem that might arise if more than one client attempts to create an initial representation for the target resource."

Alternatively, we could make the distinction clearer in the API: PUT does "set if unset", i.e. "if not exists", whereas POST can be used for creating a new record or updating an existing one (with CAS semantics). We could also have PATCH that only updates the record.