#540 서버 오류 응답 상태 코드 수정 (#711)

Author: Neibce

backend/account/decorators.py

@@ -18,18 +18,19 @@ def __init__(self, func):
     def __get__(self, obj, obj_type):
         return functools.partial(self.__call__, obj)
 
-    def error(self, data):
-        return JSONResponse.response({"error": "permission-denied", "data": data})
+    def error(self, data, status=403):
+        return JSONResponse.response({"error": "permission-denied", "data": data}, status=status)
 
     def __call__(self, *args, **kwargs):
         self.request = args[1]
 
         if self.check_permission():
             if self.request.user.is_disabled:
-                return self.error("Your account is disabled")
+                return self.error("Your account is disabled", status=403)
             return self.func(*args, **kwargs)
         else:
-            return self.error("Please login first")
+            status = 401 if not self.request.user.is_authenticated else 403
+            return self.error("Please login first", status=status)
 
     def check_permission(self):
         raise NotImplementedError()
@@ -212,7 +213,7 @@ def error(self, data):
         return JSONResponse.response({
             "error": "permission-denied",
             "data": "Scheduler token is invalid or missing",
-        })
+        }, status=403)
 
 
 def scheduler_only(view_func):

backend/account/middleware.py

@@ -134,7 +134,11 @@ def process_request(self, request):
         path = request.path_info
         if path.startswith("/admin/") or path.startswith("/api/admin/"):
             if not (request.user.is_authenticated and request.user.is_admin_role()):
-                return JSONResponse.response({"error": "login-required", "data": "Please login in first"})
+                status = 401 if not request.user.is_authenticated else 403
+                return JSONResponse.response(
+                    {"error": "login-required", "data": "Please login in first"},
+                    status=status,
+                )
 
 
 class LogSqlMiddleware(MiddlewareMixin):

backend/account/tests.py

@@ -17,8 +17,8 @@
 from options.options import SysOptions
 
 from .models import AdminType, ProblemPermission, User
-from .decorators import scheduler_only
-from .middleware import RequestIDMiddleware
+from .decorators import login_required, scheduler_only
+from .middleware import AdminRoleRequiredMiddleware, RequestIDMiddleware
 from .tasks import calculate_user_score_basis, calculate_user_score_fluctuation
 
 
@@ -65,6 +65,35 @@ def test_generates_request_id_when_header_has_no_safe_characters(self):
         self.assertEqual(response["X-Request-ID"], request.request_id)
 
 
+class AdminRoleRequiredMiddlewareTest(SimpleTestCase):
+
+    def setUp(self):
+        self.factory = RequestFactory()
+
+    def test_admin_api_requires_login_with_http_401(self):
+        middleware = AdminRoleRequiredMiddleware(lambda request: JsonResponse({"ok": True}))
+        request = self.factory.get("/api/admin/problem")
+        request.user = mock.MagicMock()
+        request.user.is_authenticated = False
+
+        response = middleware(request)
+
+        self.assertEqual(response.status_code, 401)
+        self.assertEqual(response.data, {"error": "login-required", "data": "Please login in first"})
+
+    def test_admin_api_requires_admin_role_with_http_403(self):
+        middleware = AdminRoleRequiredMiddleware(lambda request: JsonResponse({"ok": True}))
+        request = self.factory.get("/api/admin/problem")
+        request.user = mock.MagicMock()
+        request.user.is_authenticated = True
+        request.user.is_admin_role.return_value = False
+
+        response = middleware(request)
+
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.data, {"error": "login-required", "data": "Please login in first"})
+
+
 class PermissionDecoratorTest(APITestCase):
     """
     데코레이터 테스트
@@ -78,7 +107,18 @@ def setUp(self):
         self.request.user.is_authenticated = mock.MagicMock()
 
     def test_login_required(self):
-        self.request.user.is_authenticated.return_value = False
+        class TestAPIView(APIView):
+
+            @login_required
+            def get(self, request):
+                return self.success("Success")
+
+        self.request.user.is_authenticated = False
+
+        response = TestAPIView().get(self.request)
+
+        self.assertEqual(response.status_code, 401)
+        self.assertFailed(response, "Please login first")
 
     def test_admin_required(self):
         pass
@@ -106,13 +146,15 @@ def test_empty_environment_token(self):
         """ Test when SCHEDULER_TOKEN is not set in the environment."""
         request = self.factory.post("/", HTTP_X_SCHEDULER_TOKEN='secret')
         response = self.view.post(request)
+        self.assertEqual(response.status_code, 403)
         self.assertFailed(response)
 
     def test_missing_header_token(self):
         """ Test when SCHEDULER_TOKEN is not set in the environment and request header is missing."""
         with mock.patch('os.environ', {}):
             request = self.factory.post("/")
             response = self.view.post(request)
+        self.assertEqual(response.status_code, 403)
         self.assertFailed(response)
 
     def test_valid_token(self):
@@ -127,6 +169,7 @@ def test_invalid_token(self):
         with mock.patch('os.environ', {'SCHEDULER_TOKEN': 'secret'}):
             request = self.factory.post("/", HTTP_X_SCHEDULER_TOKEN='wrong_secret')
             response = self.view.post(request)
+        self.assertEqual(response.status_code, 403)
         self.assertFailed(response)
 
 

backend/utils/api/api.py

@@ -61,8 +61,8 @@ class JSONResponse(object):
     content_type = ContentType.json_response
 
     @classmethod
-    def response(cls, data):
-        resp = HttpResponse(json.dumps(data, indent=4), content_type=cls.content_type)
+    def response(cls, data, status=200):
+        resp = HttpResponse(json.dumps(data, indent=4), content_type=cls.content_type, status=status)
         resp.data = data
         return resp
 
@@ -116,14 +116,14 @@ def _get_request_data(self, request):
 
         return request.data
 
-    def response(self, data):
-        return self.response_class.response(data)
+    def response(self, data, status=200):
+        return self.response_class.response(data, status=status)
 
     def success(self, data=None):
         return self.response({"error": None, "data": data})
 
-    def error(self, msg="error", err="error"):
-        return self.response({"error": err, "data": msg})
+    def error(self, msg="error", err="error", status=400):
+        return self.response({"error": err, "data": msg}, status=status)
 
     def extract_errors(self, errors, key="field"):
         if isinstance(errors, dict):
@@ -145,7 +145,7 @@ def invalid_serializer(self, serializer):
         return self.error(err=f"invalid-{key}", msg=msg)
 
     def server_error(self):
-        return self.error(err="server-error", msg="server error")
+        return self.error(err="server-error", msg="server error", status=500)
 
     def paginate_data(self, request, query_set, object_serializer=None):
         """

backend/utils/tests.py

@@ -3,11 +3,12 @@
 import os
 import pickle
 from unittest.mock import patch, mock_open, MagicMock
-from django.test import SimpleTestCase, TestCase, override_settings
+from django.test import RequestFactory, SimpleTestCase, TestCase, override_settings
 from django.core.cache import cache
 from prometheus_client.core import GaugeMetricFamily
 
 from utils.json_logging import CodePlaceJsonFormatter
+from utils.api import APIView
 from utils.observability_metrics import CodePlaceCollector
 from utils import observability_tracing
 from utils.testcase_cache import TestCaseCacheManager
@@ -33,6 +34,36 @@ def test_reads_pickled_byte_values_from_django_redis_hash(self):
         self.assertEqual(bucket._last_capacity, 7.0)
 
 
+class APIViewStatusCodeTest(SimpleTestCase):
+
+    def setUp(self):
+        self.factory = RequestFactory()
+
+    def test_server_exception_returns_http_500_with_existing_json_body(self):
+
+        class BrokenAPIView(APIView):
+
+            def get(self, request):
+                raise RuntimeError("boom")
+
+        response = BrokenAPIView.as_view()(self.factory.get("/"))
+
+        self.assertEqual(response.status_code, 500)
+        self.assertEqual(response.data, {"error": "server-error", "data": "server error"})
+
+    def test_business_error_returns_http_400(self):
+
+        class BusinessErrorAPIView(APIView):
+
+            def get(self, request):
+                return self.error(err="problem-not-found", msg="Problem not exist")
+
+        response = BusinessErrorAPIView.as_view()(self.factory.get("/"))
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.data, {"error": "problem-not-found", "data": "Problem not exist"})
+
+
 class CodePlaceCollectorTest(SimpleTestCase):
 
     def _samples_by_metric(self, metrics):

frontend/src/pages/admin/api.js

@@ -467,10 +467,16 @@ function ajax(url, method, options) {
           }
         }
       },
-      (res) => {
+      (error) => {
         // API请求异常，一般为Server error 或 network error
-        reject(res)
-        Vue.prototype.$error(res.data.data)
+        const res = error.response || error
+        const data = res.data || {}
+        const message = data.data || error.message || "Server error"
+        Vue.prototype.$error(message)
+        reject(error)
+        if (typeof message === "string" && message.startsWith("Please login")) {
+          router.push({ name: "login" })
+        }
       },
     )
   })

frontend/src/pages/oj/api.js

@@ -514,10 +514,19 @@ function ajax(url, method, options) {
           // }
         }
       },
-      (res) => {
+      (error) => {
         // API请求异常，一般为Server error 或 network error
-        reject(res)
-        Vue.prototype.$error(res.data.data)
+        const res = error.response || error
+        const data = res.data || {}
+        const message = data.data || error.message || "Server error"
+        Vue.prototype.$error(message)
+        reject(error)
+        if (typeof message === "string" && message.startsWith("Please login")) {
+          store.dispatch("changeModalStatus", {
+            mode: "login",
+            visible: true,
+          })
+        }
       },
     )
   })