Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Restricted Task | |||||
Resolved | kostajh | T294511 2021 Security Team wikireplicas audit | |||
Declined | None | T284948 Raw IPs of logged-out users disclosed in wiki-replicas | |||
In Progress | Niharika | T324492 Temporary accounts - MVP | |||
Open | None | T326816 [Epic] Update features for temporary accounts | |||
Open | None | T329913 [Epic] Update Mobile Apps Team-owned products that may be affected by Temporary Accounts | |||
Resolved | kostajh | T335425 [Spike] Document results of iOS exploratory testing on de.beta wiki |
Event Timeline
Findings so far
The major bug I see right away is that a new account is created each time I edit anonymously. After doing a bit of digging, the reason is that our ReadingListSyncOperation, which is constantly running, assumes valid auth tokens = logged in user, and begins attempting to sync to the reading lists endpoint located at https://en.wikipedia.org/api/rest_v1/data/lists/changes/since/2023-04-27T21:22:13Z.
This in turn is returning a 401 error, and the app is automatically logging out the user before I attempt to edit again. I think this is likely just a problem with our inconsistent pointing to the beta cluster, as you may have noticed its trying to sync reading lists to EN Wiki. My hope is that once I fix that, a call to https://de.wikipedia.beta.wmflabs.org/api/rest_v1/data/lists/changes/since/2023-04-27T21:22:13Z with the temporary account cookies will not return a 401, which would stop this automatic logging out between edits.
That being said, it highlights how the app assumes valid cookies = logged in user throughout the app. We shouldn't even be attempting to hit that reading lists sync endpoint if the user has a temporary account, in my opinion, because I think that should be only a feature for permanent account users.
Problematic methods:
These will or may be true or populated for temporary accounts, which may not be what we want depending on the context we're using it in.
Session > hasValidCentralAuthCookies: https://github.com/wikimedia/wikipedia-ios/blob/main/Wikipedia/Code/Session.swift#L167
Session > isAuthenticated: https://github.com/wikimedia/wikipedia-ios/blob/main/Wikipedia/Code/Session.swift#L186 (calls hasValidCentralAuthCookies)
WMFAuthenticationManager > isLoggedIn: https://github.com/wikimedia/wikipedia-ios/blob/main/Wikipedia/Code/WMFAuthenticationManager.swift#L101
WMFAuthenticationManager > loggedInUsername: https://github.com/wikimedia/wikipedia-ios/blob/main/Wikipedia/Code/WMFAuthenticationManager.swift#L101
This is called explicitly when a user logs in to copy cookies over to wikidata & commons endpoints. We may want to be sure it gets called when a temporary account status appears as well.
Session > cloneCentralAuthCookies: https://github.com/wikimedia/wikipedia-ios/blob/main/Wikipedia/Code/Session.swift#L93
Update
After fixing the EN prod > DE beta labs mixup for syncing saved reading lists, it's acting better. User account # is retained between edits and app sessions. I do not see a logged in username under app settings, it still appears as if the user is logged out. I also don't see the app attempting to download their notifications, but I do see it attempting to sync their saved articles, which now returns with this error:
{ "type": "https://mediawiki.org/wiki/HyperSwitch/errors/bad_request", "title": "readinglists-db-error-not-set-up", "method": "get", "detail": "Reading lists have not been set up for this user.", "uri": "/de.wikipedia.beta.wmflabs.org/v1/data/lists/changes/since/2023-05-01T14%3A40%3A57Z" }
So we should probably prevent that syncing in this state.
If I log into an account, that account is attributed to the next edit. If I then log out and edit again, a new temporary account number is attributed to it.
I was unable to test cross-language or cross-site editing (for example, are we able to edit a Wikidata article description with these temporary accounts?). I expect it won't work currently - we'll need to add a cross-site cookie copying call whenever these temporary accounts are spun up after an edit, but I'm not sure. I also am unable to test cookie expiration - the app won't launch when I change the device date to next year. Maybe we could temporarily decrease it server-side to 1 week to test this scenario?