Don't allow connecting to ancient servers. #5102

chrisbobbe · 2021-11-04T18:06:57Z

We currently have a ServerCompatBanner to warn the user if they're connected to a Zulip server that's on a version we don't support anymore, and ask them to nag their admin to upgrade, with a link to instructions. The threshold for showing that nag banner is currently ~~2.1.0~~ 3.0, since #5334; that version number is meant to be updated along with https://zulip.readthedocs.io/en/stable/overview/release-lifecycle.html#compatibility-and-upgrading.

We should have a different threshold that's lower than the nag-banner threshold, so that for very old servers we just refuse to connect to the server at all, since we will have cleared out relevant backward-compatibility code.

Since our backward-compatibility code includes code for handling obsolete shapes of push-notification payloads, we should also make sure we're not trying to give users push notifications. This may happen naturally, if "refuse to connect" means logging the user out and deregistering them for push notifications. Since the deregister request can fail, we may also want to make the native code aware of logged-in state and filter out notifications for accounts that aren't logged in.

The text was updated successfully, but these errors were encountered:

gnprice · 2022-06-17T23:26:58Z

(I just tweaked the description to get the word "old" in there, so that this issue comes up on a wider range of search queries one might try when looking for it.)

The introduction of this property isn't mentioned in the API docs (it's from just before the time when we started being systematic about such things), which is why we'd had it unconditionally here in the type. But from Sentry we know that ee74a94, which started consuming this data (and went to beta with v27.187), introduced an error for a small number of users. Looking at the server's commit history, the property was introduced at this point. Presumably any user seeing this error is therefore on a pre-2.1 server. (I believe the symptom they see is that fresh server data never loads.) So, mark this property as optional in the types, and add the tiny bit of fallback code required to handle its absence. At some point in the future we'll start refusing to handle data from very old servers (that's zulip#5102), and giving people a reasonable error message in that case. When we do (and when that covers all servers before 2.1), we can stop having this fallback logic. Until then, we need it so we don't just break with no explanation.

The introduction of this property isn't mentioned in the API docs (it's from just before the time when we started being systematic about such things), which is why we'd had it unconditionally here in the type. But from Sentry we know that ee74a94, which started consuming this data (and went to beta with v27.187), introduced an error for a small number of users. Looking at the server's commit history, the property was introduced at this point. Presumably any user seeing this error is therefore on a pre-2.1 server. (I believe the symptom they see is that fresh server data never loads.) So, mark this property as optional in the types, and add the tiny bit of fallback code required to handle its absence. At some point in the future we'll start refusing to handle data from very old servers (that's #5102), and giving people a reasonable error message in that case. When we do (and when that covers all servers before 2.1), we can stop having this fallback logic. Until then, we need it so we don't just break with no explanation.

gnprice · 2022-08-31T21:28:49Z

Cross-linking some work so far on this issue: #5100, #5192, #5218. Copying from the description of that last PR:

This will let us get a better idea of how many (or few) users will be affected as we drop support for an old version, or as we remove compatibility hacks for old versions we already say we don't support.

In particular, I have a draft branch to start relying on server 2.0+, following up on #5192. One of the main things it does is clean up how we send messages, to use stream IDs and user IDs instead of emails and stream names. That'll be a much-appreciated improvement… but it will also mean we break some pretty core functionality for any users interacting with a server that's still on a pre-2.0 version.

We announced rather a while ago, the middle of last year, that we were no longer supporting such old versions. We even started showing a warning banner for any users on such a server, and haven't heard a peep from users seeing it. So we aren't going to shy away from making that change. But it might be the cue for us to start outright refusing to operate on those servers, as discussed at 401b3a2. Knowing how many users are still in this situation will help us decide whether to build that now.

This will also help us decide when it's a good time to drop support for server 2.1 and start requiring server 3.x+, and so on in the future.

gnprice · 2022-08-31T21:37:58Z

I think the next step here is to develop a PR that has us refuse to connect to a server older than 2.0.

That's quite a bit looser than the range we actually say we support, and will affect a very small fraction of users, keeping the risk quite small. It will also already let us start making many of the cleanups mentioned above.

After that's been out in a release and nothing's gone wrong, we can promptly bump the threshold to Zulip Server 2.1 -- still older than we actually support. Then consider bumping it to Zulip Server 3.0 (which is now over two years old), reflecting our actual stated support.

For implementation, note the bit about push notifications in the issue description above.

We don't currently use these three properties of the response, but if we do, we'd want them in these more convenient forms for passing around. (We will want to use zulip_version soon, though, for zulip#5102, "Don't allow connecting to ancient servers.") Not marking as NFC just because this will cause fresh errors if zulip_version doesn't parse. There's no reason to suspect it wouldn't parse, though.

With an error alert that appears at the following points: - On submitting `RealmInputScreen` (titled "Welcome") - On selecting a logged-out account in `AccountPickScreen` (titled "Pick account") - When we connect to the active account's server (get a `/register` response; the "Connecting" banner disappears) and we learn its current server version Fixes: zulip#5102

Now that zulip#5102 is done, there are definitely no servers we expect to connect to that would send us data missing this property.

…rvers This completes all our `TODO(zulip#5102)`s.

As Greg suggested almost a year ago: zulip#5102 (comment) with the note that Server 3.0 was then over two years old. This significantly increases the amount of backwards-compatibility code we can now simplify or sweep away: search for "TODO(server-3" or "TODO(server-2". (There are even a handful of straggling `TODO(server-1.9)`s.) But unlocking those simplifications isn't the main aim here, since our efforts are focused on the new Flutter app, to replace this app: https://github.com/zulip/zulip-flutter Rather, the goal is just to continue incrementally nudging people toward current Zulip Server versions. In particular, when we release the Flutter app, we'll want to give it a floor of Server 4.0 or later, depending on when we release it.

…ion numbers. Previously I've wanted to have this page spell out the concrete version number that our clients support, rather than the policy we use for determining that version number, because that's the sort of question that I feel like as a user I'd want a straight answer to and would be annoyed if I couldn't get one. But as the text stands, it's come to look more like it's the policy (something that's heavyweight to change) than like the value that the policy currently happens to work out to. Also, because this page is kind of chaotically organized (and fixing that is a bigger yak than I want to shave right now), it repeats the 18-month rule in three separate places and the current value (version 4.0) is in a fourth separate place, so it looks internally inconsistent. Let's therefore take a different tack: like in those other three spots on this page, state just the policy instead of the value it currently works out to; but also add a link to help the reader pin down for themselves what value that does work out to. This also means we no longer need to update this page as old releases age and that value advances. Also fix a typo, and cut the reference to working degraded on older releases. Starting earlier this year we finally started hard-refusing such connections: zulip/zulip-mobile#5102 zulip/zulip-mobile#5633 (which was because there were some swathes of compatibility code that we could only cut if we completely broke the handling of ancient servers, and so we preferred to have the app communicate that break clearly up front.)

…ion numbers. Previously I've wanted to have this page spell out the concrete version number that our clients support, rather than the policy we use for determining that version number, because that's the sort of question that I feel like as a user I'd want a straight answer to and would be annoyed if I couldn't get one. But as the text stands, it's come to look more like it's the policy (something that's heavyweight to change) than like the value that the policy currently happens to work out to. Also, because this page is kind of chaotically organized (and fixing that is a bigger yak than I want to shave right now), it repeats the 18-month rule in three separate places and the current value (version 4.0) is in a fourth separate place, so it looks internally inconsistent. Let's therefore take a different tack: like in those other three spots on this page, state just the policy instead of the value it currently works out to; but also add a link to help the reader pin down for themselves what value that does work out to. This also means we no longer need to update this page as old releases age and that value advances. Also fix a typo, and cut the reference to working degraded on older releases. Starting earlier this year we finally started hard-refusing such connections: zulip/zulip-mobile#5102 zulip/zulip-mobile#5633 (which was because there were some swathes of compatibility code that we could only cut if we completely broke the handling of ancient servers, and so we preferred to have the app communicate that break clearly up front.) (cherry picked from commit bb6fe03)

chrisbobbe mentioned this issue Nov 4, 2021

api: Start relying more on non-ancient servers #5100

Merged

gnprice added the P1 high-priority label Jun 17, 2022

gnprice mentioned this issue Jun 17, 2022

realm: Handle missing realm_default_external_accounts before 2.1 #5418

Merged

chrisbobbe mentioned this issue Dec 7, 2022

[toward #5102, refuse ancient servers] notifs: Try to stop push notifications when removing account from list #5585

Merged

chrisbobbe mentioned this issue Dec 7, 2022

[toward #5102, refuse ancient servers] Parse zulip_version in api.getServerSettings and api.registerForEvents #5586

Merged

chrisbobbe mentioned this issue Dec 21, 2022

(More) consistently clear old data on all ways of leaving an account #5612

Merged

chrisbobbe mentioned this issue Jan 3, 2023

api: Don't allow connecting to servers <2.0 #5633

Merged

gnprice closed this as completed in 3b811d6 Jan 25, 2023

gnprice added a commit to gnprice/zulip-mobile that referenced this issue Mar 29, 2023

presence: Drop a fallback for ancient servers

b945442

Now that zulip#5102 is done, there are definitely no servers we expect to connect to that would send us data missing this property.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 30, 2023

model: Remove various fallbacks for ancient, sometimes prehistoric se…

52ec3e8

…rvers This completes all our `TODO(zulip#5102)`s.

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 30, 2023

model: Remove various fallbacks for ancient, sometimes prehistoric se…

09dc8d3

…rvers This completes all our `TODO(zulip#5102)`s.

chrisbobbe mentioned this issue Mar 30, 2023

model: Remove various fallbacks for ancient, sometimes prehistoric servers #5706

Merged

chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Mar 31, 2023

model: Remove various fallbacks for ancient, sometimes prehistoric se…

cbf8b3a

…rvers This completes all our `TODO(zulip#5102)`s.

chrisbobbe mentioned this issue Aug 8, 2023

api: Don't allow connecting to servers <3.0 #5747

Merged

This was referenced Aug 14, 2023

Disallow connecting to unsupported ancient servers zulip/zulip-flutter#267

Open

docs: Consistently say "18 months" on compatibility, no specific version numbers. zulip/zulip#26522

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't allow connecting to ancient servers. #5102

Don't allow connecting to ancient servers. #5102

chrisbobbe commented Nov 4, 2021 •

edited by gnprice

Loading

gnprice commented Jun 17, 2022 •

edited

Loading

gnprice commented Aug 31, 2022

gnprice commented Aug 31, 2022

Don't allow connecting to ancient servers. #5102

Don't allow connecting to ancient servers. #5102

Comments

chrisbobbe commented Nov 4, 2021 • edited by gnprice Loading

gnprice commented Jun 17, 2022 • edited Loading

gnprice commented Aug 31, 2022

gnprice commented Aug 31, 2022

chrisbobbe commented Nov 4, 2021 •

edited by gnprice

Loading

gnprice commented Jun 17, 2022 •

edited

Loading