Page MenuHomePhabricator

Users can't mark their Notifications from loginwiki as read because they don't have the `writeapi` permission
Closed, ResolvedPublic

Description

Local notifications on loginwiki, seen as cross-wiki notifications from loginwiki on other wikis, are generated by the LoginNotify extension for things like failed logins. However, on 2020-10-06 (and presumably beforehand?), these can't be marked as read.

Locally, it fails with:

{"error":{"code":"writeapidenied","info":"You're not allowed to edit this wiki through the API.","*":"…"},"servedby":"mw2306"}

Remotely, it fails with:

{"query":{"echomarkread":{"result":"success","errors":{"loginwiki":{"code":"writeapidenied","info":"You're not allowed to edit this wiki through the API.","*":"…"}},"alert":{"rawcount":1,"count":"1"},"message":{"rawcount":0,"count":"0"},"rawcount":1,"count":"1"}}}

Users indeed do not have the writeapi permission, and though git blame isn't helpful, it looks like it's been this way for a while. However, in T179765: Can't dismiss failed login attempt notification (or other notifications from login.wikimedia.org) this was noted as working, so presumably something changed.

The "easy" fix is to grant users the writeapi permission, but this might have security implications, so opening a ticket first.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 632598 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[operations/mediawiki-config@master] [DNM] loginwiki: Allow users to mark Notifications as read

https://gerrit.wikimedia.org/r/632598

The writeapi permission doesn't really grant any rights on its own, all code paths should be checking the respective permission anyways. So removing it only protects us in the case where an API module isn't doing proper authorization checking, but the web UI is. I think that's a pretty low risk, and going down as we continue to unify backend logic to be common across the web UI and API. And removing API access just causes weird problems, which is why we got rid of $wgEnableWriteAPI and $wgEnableAPI a while back.

Adding Security-Team for signoff.

We'll plan to discuss this at our clinic meeting on 1-11-2021. Given @Legoktm's rationale above, I'd assume this would end up being rated low risk.

The Security-Team discussed this this morning and we'd rate this approach low risk for now.

The Security-Team discussed this this morning and we'd rate this approach low risk for now.

Could you explain on what this mean for non-WMFers? Particularly in terms of "may I deploy the change or should I wait for someone's approval".

"may I deploy the change or should I wait for someone's approval".

That's what low risk would imply within this context. I also +1'd the patch.

Change 632598 merged by jenkins-bot:

[operations/mediawiki-config@master] loginwiki: Allow users to mark Notifications as read

https://gerrit.wikimedia.org/r/632598

Mentioned in SAL (#wikimedia-operations) [2021-05-10T18:34:06Z] <jforrester@deploy1002> Synchronized wmf-config/CommonSettings.php: Config: [[gerrit:632598|loginwiki: Allow users to mark Notifications as read (T264834)]] (duration: 00m 57s)

Jdforrester-WMF claimed this task.

Deployed. A lo, I was able to scratch my own itch and mark said notification as read.