Page MenuHomePhabricator

Implement access restrictions in WatchedItemStore
Open, Needs TriagePublic

Description

When replacing User::addWatch and friends with calls to the corresponding methods on WatchedItemStore, the viewmywatchlist and editmywatchlist permissions have to be checked. We don't want all callers to duplicate that logic, but we don't really want WatchedItemStore to worry about user permissions too much either. We could however implement a simplified access check, given the following assumptions:

  • Users can only access their own watchlist
  • Users may want to restrict access further when acting through OAuth
  • Some operations may want to bypass this restriction (e.g. edits via oauth should still automatically add items to the watchlist if this behavior is enabled in the user's preferences)

This can be implemented by having a $readRestriction and a $writeRestriction field in WatchedItemStore. Each restriction field can have three kinds of values:

  • null: no restrictions apply
  • true: restricted, access will fail.
  • a UserIdentity: only this user's watchlist items can be accessed, attempts to access other user's watchlist items will fail.

Checked access will be done via new methods, using names like addWatchChecked or some such (maybe these should not be in WatchedItemStoreInterface).

The restriction levels can be set via the constructor, or a setter in WatchedeItemStore (not WatchedItemStoreInterface). It would have to be set by wiring, when the WatchedeItemStore is constructed, based on information from the current request's session.

NOTE: this is blocked on a decision about how service behavior can vary based on the current user's session, see T218555 and T231930.

Event Timeline

JTannerWMF moved this task from Inbox to External on the Growth-Team board.Jul 9 2019, 7:45 PM
JTannerWMF added a subscriber: JTannerWMF.

It appears Core Platform Team is taking this on so the Growth-Team is moving this to external.

daniel added a comment.Jul 9 2019, 9:29 PM

It appears Core Platform Team is taking this on so the Growth-Team is moving this to external.

It's in scope of the decoupling initiative, but not yet scheduled. If there is any urgency to having this implemented, we should coordinate on who has capacity when.

Krinkle added a subscriber: Krinkle.Sep 3 2019, 2:00 PM
daniel updated the task description. (Show Details)Jan 21 2020, 11:54 AM