home: For web public realms, skip login for spectators.

To provide a smoother experience of accessing a web public stream,
we don't ask user to login unless user directly requests a
`/login` URL.

Fixes #21690.

templates/zerver/login.html

@@ -34,7 +34,6 @@ page can be easily identified in it's respective JavaScript file. -->
                     <div class="login-social">
                         <form class="anonymous_access_form form-inline" action="/" method="post">
                             {{ csrf_input }}
-                            <input type="hidden" name="prefers_web_public_view" value="true" />
                             <input type="hidden" name="next" value="{{ next }}" />
                             <button class="full-width">
                                 {{ _('Access without an account') }}

tools/ci/success-http-headers.template.txt

@@ -1,23 +1,9 @@
-HTTP/2 302 
+HTTP/2 404 
 server: {nginx_version_string}
 content-type: text/html; charset=utf-8
-location: /login/
+vary: Accept-Encoding
 vary: Accept-Language, Cookie
 content-language: en
 strict-transport-security: max-age=15768000
 x-frame-options: DENY
-x-content-type-options: nosniff
-x-xss-protection: 1; mode=block
-
-HTTP/2 200 
-server: {nginx_version_string}
-content-type: text/html; charset=utf-8
-vary: Accept-Encoding
-cache-control: max-age=0, no-cache, no-store, must-revalidate, private
-vary: Cookie, Accept-Language
-content-language: en
-strict-transport-security: max-age=15768000
-x-frame-options: DENY
-x-content-type-options: nosniff
-x-xss-protection: 1; mode=block
 

zerver/tests/test_decorators.py

@@ -2170,7 +2170,7 @@ class TestRequestNotes(ZulipTestCase):
         # no realm can be set on the request notes.
         with mock.patch("zerver.views.home.zulip_login_required", lambda f: mock_home(None)):
             result = self.client_get("/", subdomain="")
-            self.assertEqual(result.status_code, 200)
+            self.assertEqual(result.status_code, 404)
 
         root_subdomain_realm = do_create_realm("", "Root Domain")
         # Now test that that realm does get set, if it exists, for requests

zerver/tests/test_home.py

@@ -14,7 +14,7 @@ from django.utils.timezone import now as timezone_now
 
 from corporate.models import Customer, CustomerPlan
 from zerver.actions.create_user import do_create_user
-from zerver.actions.realm_settings import do_change_realm_plan_type
+from zerver.actions.realm_settings import do_change_realm_plan_type, do_set_realm_property
 from zerver.actions.users import change_user_is_active
 from zerver.lib.compatibility import LAST_SERVER_UPGRADE_TIME, is_outdated_server
 from zerver.lib.home import (
@@ -248,7 +248,7 @@ class HomeTest(ZulipTestCase):
             set(result["Cache-Control"].split(", ")), {"must-revalidate", "no-store", "no-cache"}
         )
 
-        self.assert_length(queries, 45)
+        self.assert_length(queries, 46)
         self.assert_length(cache_mock.call_args_list, 5)
 
         html = result.content.decode()
@@ -311,46 +311,22 @@ class HomeTest(ZulipTestCase):
         self.assertEqual(set(actual_keys), set(expected_keys))
 
     def test_logged_out_home(self) -> None:
-        # Redirect to login on first request.
+        realm = get_realm("zulip")
+        do_set_realm_property(realm, "enable_spectator_access", False, acting_user=None)
+
+        # Redirect to login if spectator access is disabled.
         result = self.client_get("/")
         self.assertEqual(result.status_code, 302)
         self.assertEqual(result.url, "/login/")
 
-        # Tell server that user wants to log in anonymously
+        # Load webapp directly if spectator access is enabled.
-        # Redirects to load webapp.
+        do_set_realm_property(realm, "enable_spectator_access", True, acting_user=None)
-        realm = get_realm("zulip")
-        result = self.client_post("/", {"prefers_web_public_view": "true"})
-        self.assertEqual(self.client.session.get("prefers_web_public_view"), True)
-        self.assertEqual(realm.enable_spectator_access, True)
-        self.assertEqual(result.status_code, 302)
-        self.assertEqual(result.url, "http://zulip.testserver")
-
-        # Disable spectator login. Since Realm.enable_spectator_access
-        # is False, the login should fail.
-        realm.enable_spectator_access = False
-        realm.save()
-
-        result = self.client_post("/", {"prefers_web_public_view": "true"})
-        self.assertEqual(self.client.session.get("prefers_web_public_view"), True)
-        self.assertEqual(realm.enable_spectator_access, False)
-        self.assertEqual(result.status_code, 302)
-        self.assertEqual(result.url, "/login/")
-
-        # Enable spectator login.
-        realm.enable_spectator_access = True
-        realm.save()
-
-        result = self.client_post("/", {"prefers_web_public_view": "true"})
-        self.assertEqual(self.client.session.get("prefers_web_public_view"), True)
-        self.assertEqual(realm.enable_spectator_access, True)
-        self.assertEqual(result.status_code, 302)
-        self.assertEqual(result.url, "http://zulip.testserver")
-
-        # Always load the web app from then on directly
         result = self.client_get("/")
         self.assertEqual(result.status_code, 200)
 
+        # Check no unnecessary params are passed to spectators.
         page_params = self._get_page_params(result)
+        self.assertEqual(page_params["is_spectator"], True)
         actual_keys = sorted(str(k) for k in page_params.keys())
         removed_keys = [
             "custom_profile_field_types",
@@ -361,7 +337,6 @@ class HomeTest(ZulipTestCase):
         ]
         expected_keys = [i for i in self.expected_page_params_keys if i not in removed_keys]
         self.assertEqual(actual_keys, expected_keys)
-        self.assertEqual(self.client.session.get("prefers_web_public_view"), True)
 
         # Test information passed to client about users.
         page_params = self._get_page_params(result)
@@ -384,11 +359,6 @@ class HomeTest(ZulipTestCase):
         date_length = len("YYYY-MM-DD")
         self.assert_length(page_params["realm_users"][0]["date_joined"], date_length)
 
-        # Web-public session key should clear once user is logged in
-        self.login("hamlet")
-        self.client_get("/")
-        self.assertEqual(self.client.session.get("prefers_web_public_view"), None)
-
     def test_home_under_2fa_without_otp_device(self) -> None:
         with self.settings(TWO_FACTOR_AUTHENTICATION_ENABLED=True):
             self.login("iago")
@@ -421,7 +391,7 @@ class HomeTest(ZulipTestCase):
                 result = self._get_home_page()
                 self.check_rendered_logged_in_app(result)
                 self.assert_length(cache_mock.call_args_list, 6)
-            self.assert_length(queries, 42)
+            self.assert_length(queries, 43)
 
     def test_num_queries_with_streams(self) -> None:
         main_user = self.example_user("hamlet")
@@ -452,7 +422,7 @@ class HomeTest(ZulipTestCase):
         with queries_captured() as queries2:
             result = self._get_home_page()
 
-        self.assert_length(queries2, 40)
+        self.assert_length(queries2, 41)
 
         # Do a sanity check that our new streams were in the payload.
         html = result.content.decode()

zerver/tests/test_sessions.py

@@ -4,6 +4,7 @@ from unittest import mock
 
 from django.utils.timezone import now as timezone_now
 
+from zerver.actions.realm_settings import do_set_realm_property
 from zerver.actions.users import change_user_is_active
 from zerver.lib.sessions import (
     delete_all_deactivated_user_sessions,
@@ -28,8 +29,8 @@ class TestSessions(ZulipTestCase):
         action()
         if expected_result:
             result = self.client_get("/", subdomain=realm.subdomain)
-            self.assertEqual(302, result.status_code)
+            self.assertEqual(200, result.status_code)
-            self.assertEqual("/login/", result.url)
+            self.assertTrue('is_spectator":true' in str(result.content))
         else:
             self.assertIn("_auth_user_id", self.client.session)
 
@@ -40,8 +41,8 @@ class TestSessions(ZulipTestCase):
         for session in user_sessions(user_profile):
             delete_session(session)
         result = self.client_get("/")
-        self.assertEqual(result.status_code, 302)
+        self.assertEqual(result.status_code, 200)
-        self.assertEqual(result.url, "/login/")
+        self.assertTrue('is_spectator":true' in str(result.content))
 
     def test_delete_user_sessions(self) -> None:
         user_profile = self.example_user("hamlet")
@@ -77,8 +78,19 @@ class TestSessions(ZulipTestCase):
             get_realm("zulip"),
             True,
         )
+
+        lear_realm = get_realm("lear")
+        do_set_realm_property(lear_realm, "enable_spectator_access", True, acting_user=None)
+        self.make_stream(
+            "web_public_stream",
+            realm=lear_realm,
+            is_web_public=True,
+        )
         self.do_test_session(
-            self.mit_user("sipbtest"), lambda: delete_all_user_sessions(), get_realm("zephyr"), True
+            self.lear_user("cordelia"),
+            lambda: delete_all_user_sessions(),
+            lear_realm,
+            True,
         )
 
     def test_delete_all_deactivated_user_sessions(self) -> None:
@@ -89,8 +101,8 @@ class TestSessions(ZulipTestCase):
         self.client_post("/accounts/logout/")
         delete_all_deactivated_user_sessions()
         result = self.client_get("/")
-        self.assertEqual(result.status_code, 302)
+        self.assertEqual(result.status_code, 200)
-        self.assertEqual(result.url, "/login/")
+        self.assertTrue('is_spectator":true' in str(result.content))
 
         # Test nothing happens to an active user's session
         self.login("othello")
@@ -110,8 +122,8 @@ class TestSessions(ZulipTestCase):
             [f"INFO:root:Deactivating session for deactivated user {user_profile_3.id}"],
         )
         result = self.client_get("/")
-        self.assertEqual(result.status_code, 302)
+        self.assertEqual(result.status_code, 200)
-        self.assertEqual(result.url, "/login/")
+        self.assertTrue('is_spectator":true' in str(result.content))
 
 
 class TestExpirableSessionVars(ZulipTestCase):

zerver/tests/test_urls.py

@@ -43,9 +43,8 @@ class PublicURLTest(ZulipTestCase):
                 "/en/accounts/login/",
                 "/ru/accounts/login/",
                 "/help/",
-            ],
+                # Since web-public streams are enabled in this `zulip`
-            302: [
+                # instance, the public access experience is loaded directly.
-                # These 302 because they redirect to the spectator experience.
                 "/",
                 "/en/",
                 "/ru/",

zerver/views/development/dev_login.py

@@ -79,7 +79,6 @@ def dev_direct_login(
     realm = get_realm(subdomain)
 
     if request.POST.get("prefers_web_public_view") == "Anonymous login":
-        request.session["prefers_web_public_view"] = True
         redirect_to = get_safe_redirect_to(next, realm.uri)
         return HttpResponseRedirect(redirect_to)
 

zerver/views/home.py

@@ -8,7 +8,7 @@ from django.shortcuts import redirect, render
 from django.utils.cache import patch_cache_control
 
 from zerver.actions.user_settings import do_change_tos_version
-from zerver.context_processors import get_valid_realm_from_request
+from zerver.context_processors import get_realm_from_request, get_valid_realm_from_request
 from zerver.decorator import web_public_view, zulip_login_required
 from zerver.forms import ToSForm
 from zerver.lib.compatibility import is_outdated_desktop_app, is_unsupported_browser
@@ -19,7 +19,6 @@ from zerver.lib.subdomains import get_subdomain
 from zerver.lib.user_counts import realm_user_count
 from zerver.lib.utils import statsd
 from zerver.models import PreregistrationUser, Realm, Stream, UserProfile
-from zerver.views.auth import get_safe_redirect_to
 from zerver.views.portico import hello_view
 
 
@@ -117,14 +116,10 @@ def home(request: HttpRequest) -> HttpResponse:
     if settings.ROOT_DOMAIN_LANDING_PAGE and subdomain == Realm.SUBDOMAIN_FOR_ROOT_DOMAIN:
         return hello_view(request)
 
-    # TODO: The following logic is a bit hard to read. We save a
+    realm = get_realm_from_request(request)
-    # database query in the common case by avoiding the call to
+    if realm is None:
-    # `get_valid_realm_from_request` if user hasn't requested
+        return render(request, "zerver/invalid_realm.html", status=404)
-    # web-public access.
+    if realm.allow_web_public_streams_access():
-    if (
-        request.POST.get("prefers_web_public_view") == "true"
-        or request.session.get("prefers_web_public_view")
-    ) and get_valid_realm_from_request(request).allow_web_public_streams_access():
         return web_public_view(home_real)(request)
     return zulip_login_required(home_real)(request)
 
@@ -161,32 +156,9 @@ def home_real(request: HttpRequest) -> HttpResponse:
     if request.user.is_authenticated:
         user_profile = request.user
         realm = user_profile.realm
-
-        # User is logged in and hence no longer `prefers_web_public_view`.
-        if "prefers_web_public_view" in request.session.keys():
-            del request.session["prefers_web_public_view"]
     else:
         realm = get_valid_realm_from_request(request)
-
+        # We load the spectator experience.  We fall through to the shared code
-        # TODO: Ideally, we'd open Zulip directly as a spectator if
-        # the URL had clicked a link to content on a web-public
-        # stream.  We could maybe do this by parsing `next`, but it's
-        # not super convenient with Zulip's hash-based URL scheme.
-
-        # The "Access without an account" button on the login page
-        # submits a POST to this page with this hidden field set.
-        if request.POST.get("prefers_web_public_view") == "true":
-            request.session["prefers_web_public_view"] = True
-            # We serve a redirect here, rather than serving a page, to
-            # avoid browser "Confirm form resubmission" prompts on reload.
-            redirect_to = get_safe_redirect_to(request.POST.get("next"), realm.uri)
-            return redirect(redirect_to)
-
-        # See the assert in `home` above for why this must be true.
-        assert request.session.get("prefers_web_public_view")
-
-        # For users who have selected public access, we load the
-        # spectator experience.  We fall through to the shared code
         # for loading the application, with user_profile=None encoding
         # that we're a spectator, not a logged-in user.
         user_profile = None