Also, if you all would like to do a quick call or IRC chat to get additional clarifications, please let me know!

@kostajh - Yes, maybe it would be good to have a quick chat. I'm specifically curious if you'd like us to review everything committed since T207798 or if there are a more specific set of commits we should focus on for the features described above.

I'm specifically curious if you'd like us to review everything committed since T207798 or if there are a more specific set of commits we should focus on for the features described above.

There's no code specific to the view-Special:Homepage-as-some-other-user feature to review just yet. If the "Proposed approach" section seems like it would be OK, we can start coding and then tag you for review after.

FWIW, given how much press https://arstechnica.com/information-technology/2018/09/50-million-facebook-accounts-breached-by-an-access-token-harvesting-attack/ has been getting, view-as-other-people features make me nervous.

The possibility here that most immediately comes to mind would be if the wiki has a special page with getCacheTTL set to 0 (e.g. Like with Extension:Petition or any special page when $wgMiserMode is set to false. There are no such special pages in core when miser mode is enabled), and that the homepage will render some some stuff where $parser->getUser() gets the other user, thus leaking CSRF tokens. In practise, this probably currently can't happen (Primarily because, wfMessage() wouldn't use your custom context at all, if you go via $this->msg [where this is the SpecialPage], it uses the DerivitiveContext, but not the User part, and the user is still coming from $wgUser [in MessageCache]. If you're using $this->getOutput->addWikiTextAsInterface [or a different OutputPage parsing method. Which the current extension does not use, but it is a common method people call in SpecialPages], this does use the context from RequestContext, and could output csrf tokens in the special page transclusion case. However, DerivativeContext will generally use the OutputPage from the parent context unless explicitly overridden. This means that OutputPage will still have the parent context instead of the DerivativeContext we got the OutputPage from. This borderline seems like a bug to me, but does mean that $myDerrivContext->getOutput()->addWikiTextAsInterface(...) will be using the User and other Context stuff from the parent context and not $myDerrivContext. So it can't leak tokens that way, but that's mostly relying on a lot of implementation details. Of course there's also the possibility of something else using the DerivativeContext in an unsafe way I didn't think of

thanks for your review!

FWIW, given how much press https://arstechnica.com/information-technology/2018/09/50-million-facebook-accounts-breached-by-an-access-token-harvesting-attack/ has been getting, view-as-other-people features make me nervous.

yes, acknowledged.

this does use the context from RequestContext, and could output csrf tokens in the special page transclusion case.

We are not proposing to allow transclusion of this page although we have talked about transclusion for one of its modules, Special:Impact

I'm not sure I fully understood everything in your comment but my takeaway is that using DerivativeContext is not necessarily safe to prevent leaking sensitive information for the "other" when in view-as-other mode

Note that we use DerivateContext to render Special:Impact for another user (see the code here), but from what you're saying it sounds like for view-as-other mode, we should *not* inject a IContextSource to the Impact class (and presumably also the other modules, if we implemented view-as-other for them), but instead provide an ability to set or inject User, MessageLocalizer, Language, and Config classes.

That said, could you please look at the Special:Impact code and the Impact class to see if anything there strikes you as unsafe?

Hey @kostajh - just to follow up on this a bit more, I'd like to propose this to the Security-Team and provide a concept review. This will probably involve some measurement of risk for the "view another user's homepage" feature (based upon @Bawolff's concerns and any other potential issues) and the ownership of said risk by the growth team.

@sbassett -- in a conversation with @JBennett the other week, we talked about separating out the ability to view any user's Impact module via Special:Impact from the ability to see their whole homepage. Seeing their Impact module is our higher priority, and the thing we want to make sure is okay soonest. @kostajh can fill in details about the distinction if needed.

@MMiller_WMF, @JTannerWMF - the Security-Team discussed this today. We had a couple of initial questions for you:

1. For how long is this feature intended to exist (specifically the read-only view of the Special:Impact page)? A month? A year? Indefinitely?
2. Do you (and any additional privileged users) have 2FA enabled on all wikis where this feature would be enabled?

Finally, we have a few suggestions for some additional technical controls for the Growth Team to implement regarding this feature, which are currently under WMF-Legal review. Once that review is complete (and they potentially prescribe additional controls) we can provide them here for you to review, along with our analysis of the overall risk. I'd hope we could get this turned around in a week or less for you.

Myself, @sbassett @Catrope @kaldari and @kostajh had a follow up meeting about this yesterday. Scott is planning to talk to John to get an okay and risk assessment on this project.

The following things were discussed:

• The Task recommendation module is being put on hold and should be considered for this release
• We are looking for feedback on Special:Homepage and the Impact Module (mentioned in the description of this ticket)
• We made clear the Special:Homepage will only show already public information and a minimal amount of it

When asked for examples of the Impact Module the following were provided:
https://en.wikipedia.beta.wmflabs.org/wiki/Special:Impact/Krenair for example, or https://en.wikipedia.beta.wmflabs.org/wiki/Special:Impact

Additionally, this one is about the view-homepage-as-other-user task, which we just put in the fridge. https://phabricator.wikimedia.org/T217281 But in there, we split off the Special:Impact part of it, and that's discussed at https://phabricator.wikimedia.org/T217281#5041219 and is implemented in this patch https://gerrit.wikimedia.org/r/#/c/497841/

@JTannerWMF - just to clarify your first bullet point - "The Task recommendation module is being put on hold and should NOT be considered for this release", correct?

A few small clarifications:

• The "view Special:Homepage of another user" feature is on hold and should not be considered by Security team for now
• We are looking for feedback on viewing and interacting with Special:Homepage and viewing Special:Impact for yourself or for another user (Special:Impact/{Username}). Both can be tested on beta labs.
• Special:Homepage mostly shows publicly accessible information to the user who is viewing it, but the user can also see:
  • their email confirmation status (no email, or confirmation pending)
  • their mentor (saved as a user ID in preferences)
  • Whether the user has visited the tutorial page defined for the wiki

And yes @sbassett the task recommendation module hasn't been developed or scoped so there's nothing to review there yet.

@JTannerWMF, @kostajh, @Catrope - the Security-Team just had a chat about this. We're fine with the Special:Impact administrative view by itself. We will classify that as an "informational" risk, which wouldn't require an entry within our risk register. If/when the Growth Team is ready to proceed with the administrative view of the Special:Homepage feature (as initially proposed) we can pick up where we left off on that specific feature. If this sounds good and there are no further questions from the Growth Team on this, I can go ahead and resolve this task.

(note: the Special:Impact view does not require any additional assessment from the Security-Team or WMF-Legal for now)