Page MenuHomePhabricator

[Spike] Document results of iOS exploratory testing on de.beta wiki
Closed, ResolvedPublic

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?

LGoto triaged this task as Low priority.May 9 2023, 4:24 PM
kostajh claimed this task.
kostajh subscribed.

AFAIK this can be safely resolved, as now engineering work is ongoing.