exceptions: Use HTTP 401 code for authentication errors.

templates/zerver/api/changelog.md

@@ -11,6 +11,18 @@ below features are supported.
 
 ## Changes in Zulip 5.0
 
+**Feature level 76**
+
+* [`POST /fetch_api_key`](/api/fetch_api_key), [`POST
+  /dev_fetch_api_key`](/api/dev_fetch_api_key): The HTTP status for
+  authentication errors is now 401. This was previously 403.
+* All API endpoints now use the HTTP 401 error status for API requests
+  involving a deactivated user or realm. This was previously 403.
+* Mobile push notifications now include the `mentioned_user_group_id`
+  and `mentioned_user_group_name` fields when a user group containing
+  the user is mentioned.  Previously, these were indistinguishable
+  from personal mentions (as both types have `trigger="mention"`).
+
 **Feature level 75**
 
 * [`POST /register`](/api/register-queue), `PATCH /realm`: Replaced `allow_community_topic_editing`

templates/zerver/api/rest-error-handling.md

@@ -19,6 +19,10 @@ errors common to many endpoints:
 
 {generate_code_example|/rest-error-handling:post|fixture(400)}
 
+{generate_code_example|/rest-error-handling:post|fixture(401)}
+
+{generate_code_example|/rest-error-handling:post|fixture(403)}
+
 {generate_code_example|/rest-error-handling:post|fixture(429)}
 
 The `retry-after` paremeter in the response indicates how many seconds
@@ -49,5 +53,3 @@ limit.
 
 **Changes**: The `code` field in the response is new in Zulip 4.0
 (feature level 36).
-
-{generate_code_example|/rest-error-handling:post|fixture(403)}

version.py

@@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
 # Changes should be accompanied by documentation explaining what the
 # new level means in templates/zerver/api/changelog.md, as well as
 # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`.
-API_FEATURE_LEVEL = 75
+API_FEATURE_LEVEL = 76
 
 # Bump the minor PROVISION_VERSION to indicate that folks should provision
 # only when going from an old version of the code to a newer version. Bump

zerver/lib/exceptions.py

@@ -280,8 +280,7 @@ class StreamAdministratorRequired(JsonableError):
 class AuthenticationFailedError(JsonableError):
     # Generic class for authentication failures
     code: ErrorCode = ErrorCode.AUTHENTICATION_FAILED
-    # Bug: Shouldn't this be 401?
+    http_status_code = 401
-    http_status_code = 403
 
     def __init__(self) -> None:
         pass

zerver/openapi/zulip.yaml

@@ -11151,9 +11151,9 @@ paths:
                   - $ref: "#/components/schemas/InvalidApiKeyError"
                   - $ref: "#/components/schemas/MissingArgumentError"
                   - $ref: "#/components/schemas/UserNotAuthorizedError"
-        "403":
+        "401":
           description: |
-            Forbidden.
+            Unauthorized.
           content:
             application/json:
               schema:
@@ -12855,6 +12855,9 @@ components:
           description: |
             ## User account deactivated
 
+            **Changes**: These errors used the HTTP 403 status code
+              before Zulip 5.0 (feature level 76).
+
             A typical failed json response for when user's account is deactivated
     RateLimitedError:
       allOf:
@@ -12882,6 +12885,9 @@ components:
           description: |
             ## Realm deactivated
 
+            **Changes**: These errors used the HTTP 403 status code
+              before Zulip 5.0 (feature level 76).
+
             A typical failed json response for when user's organization is deactivated
 
   ###################

zerver/tests/test_auth_backends.py

@@ -4057,7 +4057,7 @@ class FetchAPIKeyTest(ZulipTestCase):
         result = self.client_post(
             "/api/v1/fetch_api_key", dict(username=self.email, password="wrong")
         )
-        self.assert_json_error(result, "Your username or password is incorrect", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 401)
 
     def test_invalid_subdomain(self) -> None:
         with mock.patch("zerver.views.auth.get_realm_from_request", return_value=None):
@@ -4074,7 +4074,7 @@ class FetchAPIKeyTest(ZulipTestCase):
                 dict(username=self.email, password=initial_password(self.email)),
             )
             self.assert_json_error_contains(
-                result, "Password authentication is disabled in this organization", 403
+                result, "Password authentication is disabled in this organization", 401
             )
 
     @override_settings(AUTHENTICATION_BACKENDS=("zproject.backends.ZulipLDAPAuthBackend",))
@@ -4101,14 +4101,14 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username="hamlet", password=self.ldap_password("hamlet")),
         )
-        self.assert_json_error(result, "Your username or password is incorrect", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 401)
 
         self.change_ldap_user_attr("hamlet", "department", "testWrongRealm")
         result = self.client_post(
             "/api/v1/fetch_api_key",
             dict(username="hamlet", password=self.ldap_password("hamlet")),
         )
-        self.assert_json_error(result, "Your username or password is incorrect", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 401)
 
         self.change_ldap_user_attr("hamlet", "department", "zulip")
         result = self.client_post(
@@ -4134,7 +4134,7 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username="hamlet", password=self.ldap_password("hamlet")),
         )
-        self.assert_json_error(result, "Your username or password is incorrect", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 401)
 
         self.change_ldap_user_attr("hamlet", "test2", "testing")
         # Check with only one set
@@ -4142,7 +4142,7 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username="hamlet", password=self.ldap_password("hamlet")),
         )
-        self.assert_json_error(result, "Your username or password is incorrect", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 401)
 
         self.change_ldap_user_attr("hamlet", "test1", "test")
         # Setting org_membership to not cause django_ldap_auth to warn, when synchronising
@@ -4177,7 +4177,7 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username="hamlet", password=self.ldap_password("hamlet")),
         )
-        self.assert_json_error(result, "Your username or password is incorrect", 403)
+        self.assert_json_error(result, "Your username or password is incorrect", 401)
 
         # Override access with `org_membership`
         self.change_ldap_user_attr("hamlet", "department", "zulip")
@@ -4196,7 +4196,7 @@ class FetchAPIKeyTest(ZulipTestCase):
                 "/api/v1/fetch_api_key",
                 dict(username="hamlet", password=self.ldap_password("hamlet")),
             )
-            self.assert_json_error(result, "Your username or password is incorrect", 403)
+            self.assert_json_error(result, "Your username or password is incorrect", 401)
 
     def test_inactive_user(self) -> None:
         do_deactivate_user(self.user_profile, acting_user=None)
@@ -4204,7 +4204,7 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username=self.email, password=initial_password(self.email)),
         )
-        self.assert_json_error_contains(result, "Account is deactivated", 403)
+        self.assert_json_error_contains(result, "Account is deactivated", 401)
 
     def test_deactivated_realm(self) -> None:
         do_deactivate_realm(self.user_profile.realm, acting_user=None)
@@ -4212,7 +4212,7 @@ class FetchAPIKeyTest(ZulipTestCase):
             "/api/v1/fetch_api_key",
             dict(username=self.email, password=initial_password(self.email)),
         )
-        self.assert_json_error_contains(result, "This organization has been deactivated", 403)
+        self.assert_json_error_contains(result, "This organization has been deactivated", 401)
 
     def test_old_weak_password_after_hasher_change(self) -> None:
         user_profile = self.example_user("hamlet")
@@ -4234,7 +4234,7 @@ class FetchAPIKeyTest(ZulipTestCase):
                 dict(username=self.email, password=password),
             )
             self.assert_json_error(
-                result, "Your password has been disabled and needs to be reset", 403
+                result, "Your password has been disabled and needs to be reset", 401
             )
 
 
@@ -4260,17 +4260,17 @@ class DevFetchAPIKeyTest(ZulipTestCase):
     def test_unregistered_user(self) -> None:
         email = "foo@zulip.com"
         result = self.client_post("/api/v1/dev_fetch_api_key", dict(username=email))
-        self.assert_json_error_contains(result, "Your username or password is incorrect", 403)
+        self.assert_json_error_contains(result, "Your username or password is incorrect", 401)
 
     def test_inactive_user(self) -> None:
         do_deactivate_user(self.user_profile, acting_user=None)
         result = self.client_post("/api/v1/dev_fetch_api_key", dict(username=self.email))
-        self.assert_json_error_contains(result, "Account is deactivated", 403)
+        self.assert_json_error_contains(result, "Account is deactivated", 401)
 
     def test_deactivated_realm(self) -> None:
         do_deactivate_realm(self.user_profile.realm, acting_user=None)
         result = self.client_post("/api/v1/dev_fetch_api_key", dict(username=self.email))
-        self.assert_json_error_contains(result, "This organization has been deactivated", 403)
+        self.assert_json_error_contains(result, "This organization has been deactivated", 401)
 
     def test_dev_auth_disabled(self) -> None:
         with mock.patch("zerver.views.development.dev_login.dev_auth_enabled", return_value=False):

zerver/tests/test_decorators.py

@@ -1096,7 +1096,7 @@ class DeactivatedRealmTest(ZulipTestCase):
             },
         )
         self.assert_json_error_contains(
-            result, "This organization has been deactivated", status_code=403
+            result, "This organization has been deactivated", status_code=401
         )
 
         result = self.api_post(
@@ -1128,7 +1128,7 @@ class DeactivatedRealmTest(ZulipTestCase):
         realm.save()
         result = self.client_post("/json/fetch_api_key", {"password": test_password})
         self.assert_json_error_contains(
-            result, "This organization has been deactivated", status_code=403
+            result, "This organization has been deactivated", status_code=401
         )
 
     def test_webhook_deactivated_realm(self) -> None:
@@ -1143,7 +1143,7 @@ class DeactivatedRealmTest(ZulipTestCase):
         data = self.webhook_fixture_data("jira", "created_v2")
         result = self.client_post(url, data, content_type="application/json")
         self.assert_json_error_contains(
-            result, "This organization has been deactivated", status_code=403
+            result, "This organization has been deactivated", status_code=401
         )
 
 
@@ -1246,7 +1246,7 @@ class InactiveUserTest(ZulipTestCase):
                 "to": self.example_email("othello"),
             },
         )
-        self.assert_json_error_contains(result, "Account is deactivated", status_code=403)
+        self.assert_json_error_contains(result, "Account is deactivated", status_code=401)
 
         result = self.api_post(
             self.example_user("hamlet"),
@@ -1275,7 +1275,7 @@ class InactiveUserTest(ZulipTestCase):
         change_user_is_active(user_profile, False)
 
         result = self.client_post("/json/fetch_api_key", {"password": test_password})
-        self.assert_json_error_contains(result, "Account is deactivated", status_code=403)
+        self.assert_json_error_contains(result, "Account is deactivated", status_code=401)
 
     def test_login_deactivated_user(self) -> None:
         """
@@ -1341,7 +1341,7 @@ class InactiveUserTest(ZulipTestCase):
         url = f"/api/v1/external/jira?api_key={api_key}&stream=jira_custom"
         data = self.webhook_fixture_data("jira", "created_v2")
         result = self.client_post(url, data, content_type="application/json")
-        self.assert_json_error_contains(result, "Account is deactivated", status_code=403)
+        self.assert_json_error_contains(result, "Account is deactivated", status_code=401)
 
 
 class TestIncomingWebhookBot(ZulipTestCase):
@@ -1689,7 +1689,7 @@ class TestAuthenticatedJsonPostViewDecorator(ZulipTestCase):
         # we deactivate user manually because do_deactivate_user removes user session
         change_user_is_active(user_profile, False)
         self.assert_json_error_contains(
-            self._do_test(user_profile), "Account is deactivated", status_code=403
+            self._do_test(user_profile), "Account is deactivated", status_code=401
         )
         do_reactivate_user(user_profile, acting_user=None)
 
@@ -1702,7 +1702,7 @@ class TestAuthenticatedJsonPostViewDecorator(ZulipTestCase):
         self.assert_json_error_contains(
             self._do_test(user_profile),
             "This organization has been deactivated",
-            status_code=403,
+            status_code=401,
         )
         do_reactivate_realm(user_profile.realm)