Skip to content

fix: swagger #2909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 23 additions & 24 deletions apps/common/auth/authenticate.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from django.core import cache
from django.core import signing
from django.utils.translation import gettext_lazy as _
from drf_spectacular.extensions import OpenApiAuthenticationExtension
from rest_framework.authentication import TokenAuthentication

from common.exception.app_exception import AppAuthenticationFailed, AppEmbedIdentityFailed, AppChatNumOutOfBoundsFailed, \
Expand All @@ -26,6 +27,20 @@ def authenticate(self, request):
return None, None


class AnonymousAuthenticationScheme(OpenApiAuthenticationExtension):
target_class = AnonymousAuthentication # 绑定到你的自定义认证类
name = "AnonymousAuth" # 自定义认证名称(显示在 Swagger UI 中)

def get_security_definition(self, auto_schema):
# 定义认证方式,这里假设匿名认证不需要凭证
return {
}

def get_security_requirement(self, auto_schema):
# 返回安全要求(空字典表示无需认证)
return {}


def new_instance_by_class_path(class_path: str):
parts = class_path.rpartition('.')
package_path = parts[0]
Expand Down Expand Up @@ -54,39 +69,23 @@ def get_token_details(self):
return self.token_details


class OpenAIKeyAuth(TokenAuthentication):
def authenticate(self, request):
auth = request.META.get('HTTP_AUTHORIZATION')
auth = auth.replace('Bearer ', '')
# 未认证
if auth is None:
raise AppAuthenticationFailed(1003, _('Not logged in, please log in first'))
try:
token_details = TokenDetails(auth)
for handle in handles:
if handle.support(request, auth, token_details.get_token_details):
return handle.handle(request, auth, token_details.get_token_details)
raise AppAuthenticationFailed(1002, _('Authentication information is incorrect! illegal user'))
except Exception as e:
traceback.format_exc()
if isinstance(e, AppEmbedIdentityFailed) or isinstance(e, AppChatNumOutOfBoundsFailed) or isinstance(e,
AppApiException):
raise e
raise AppAuthenticationFailed(1002, _('Authentication information is incorrect! illegal user'))


class TokenAuth(TokenAuthentication):
keyword = "Bearer"

# 重新 authenticate 方法,自定义认证规则
def authenticate(self, request):
auth = request.META.get('HTTP_AUTHORIZATION')
# 未认证
if auth is None:
raise AppAuthenticationFailed(1003, _('Not logged in, please log in first'))
if not auth.startswith("Bearer "):
raise AppAuthenticationFailed(1002, _('Authentication information is incorrect! illegal user'))
try:
token_details = TokenDetails(auth)
token = auth[7:]
token_details = TokenDetails(token)
for handle in handles:
if handle.support(request, auth, token_details.get_token_details):
return handle.handle(request, auth, token_details.get_token_details)
if handle.support(request, token, token_details.get_token_details):
return handle.handle(request, token, token_details.get_token_details)
raise AppAuthenticationFailed(1002, _('Authentication information is incorrect! illegal user'))
except Exception as e:
traceback.format_exc()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided Django code has several improvements and optimizations:

Improvements/Suggestions

  1. Remove Deprecated core.cache: The line from django.core import cache can be removed because it is already imported in other parts of the file.

  2. Update Authentication Classes:

    • TokenAuthentication Class: Simplified the authentication logic to require a specific Bearer keyword at the beginning of the authorization header.
    • AnonymousAuth Scheme: Added necessary methods to define authentication scheme details for OpenAPI specification.
  3. Enhanced Error Handling: Improved error handling by catching more specific exceptions and providing clear messages.

  4. Comments: Minor improvements to comments for better readability.

  5. Removed Unused Functionality: Ensure that all unused functions (like new_instance_by_class_path) are deleted to keep the code concise.

Optimizations

  1. Use List Comprehension: Replace nested loops with list comprehensions where applicable to improve performance.

  2. Consistency in Exceptions: Ensure consistent error handling across different parts of the codebase.

  3. Documentation: Add comments, type hints, and docstrings to clarify the purpose and functionality of each method.

Here's an updated version of the code incorporating these changes:

import traceback
from typing import Any, Dict

try:
    from rest_framework.authentication import TokenAuthentication
    from rest_framework.exceptions import APIException

except ImportError as e:
    print("Error importing REST Framework modules:", e)

# Assuming 'handles' is defined elsewhere
class AnonymousAuthentication(TokenAuthentication):
    def authenticate(self, request):
        return None, None


class OpenAIKeyAuth(TokenAuthentication):
    def authenticate(self, request):
        auth = request.META.get('HTTP_AUTHORIZATION')
        if not auth.startswith("Bearer "):
            raise AppAuthenticationFailed(1002, _('Authentication information is incorrect! Illegal user'))

        try:
            token = auth[7:]  # Remove the "Bearer " prefix
            token_details = TokenDetails(token)

            for handle in handles:
                if handle.support(request, token, token_details.get_token_details):
                    return handle.handle(request, token, token_details.get_token_details)
            
            raise AppAuthenticationFailed(1002, _('Authentication information is incorrect! Invalid user'))
        
        except Exception as e:
            traceback.print_exc()
            
            if isinstance(e, AppEmbedIdentityFailed) or \
               isinstance(e, AppChatNumOutOfBoundsFailed) or \
               isinstance(e, AppApiException):
                raise
            
            raise AppAuthenticationFailed(1002, _('Authentication information is incorrect! Invalid user'))


def new_instance_by_class_path(class_path: str):
    parts = class_path.rsplit('.', 1)
    package_path = parts[0]
    class_name = parts[-1]

    module = __import__(package_path.replace('.', '.')).__getattr__(class_name.strip())
    return module()


class TokenDetails:
    def __init__(self, encoded):
        self._encoded = encoded
        
    def get_encoded_details(self) -> str:
        return self._encoded
    
    def decode_details(self, secret_key:str) -> Dict[str, Any]:
        try:
            decoded_data = signing.loads(encoded=self._encoded, key=secret_key)
        except SigningExpired:
            raise AppAuthenticationFailed(1003, "Token expired")
        except ValidationError:
            raise AppAuthenticationFailed(1002, "Invalid signature")

        return decoded_data


class AnonymousAuthenticationScheme(OpenApiAuthenticationExtension):
    target_class = AnonymousAuthentication  # Bind to your custom authentication class
    name = "AnonymousAuth"  # Custom authentication name (displayed on Swagger UI)

    def get_security_definition(self, auto_schema):
        # Define authentication way, assuming anonymous authentication doesn't need credentials
        return {
        }

    def get_security_requirement(self, auto_schema):
        # Return security requirement (empty dictionary means no authentication required)
        return {}    

This version of the code should work well with modern Python environments and ensures best practices in terms of error handling, documentation, and overall structure.

Expand Down
10 changes: 1 addition & 9 deletions apps/maxkb/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,11 @@
2. Add a URL to urlpatterns: path('blog/', include('blog.urls'))
"""
from django.urls import path, re_path, include
from drf_spectacular.views import SpectacularAPIView, SpectacularRedocView, SpectacularSwaggerView
from rest_framework import permissions
from common.auth import AnonymousAuthentication
from django.views import static
from drf_spectacular.views import SpectacularAPIView, SpectacularRedocView, SpectacularSwaggerView

from maxkb import settings

SpectacularSwaggerView.permission_classes = [permissions.AllowAny]
SpectacularSwaggerView.authentication_classes = [AnonymousAuthentication]
SpectacularAPIView.permission_classes = [permissions.AllowAny]
SpectacularAPIView.authentication_classes = [AnonymousAuthentication]
SpectacularRedocView.permission_classes = [permissions.AllowAny]
SpectacularRedocView.authentication_classes = [AnonymousAuthentication]
urlpatterns = [
path("api/", include("users.urls")),
]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are several issues in the code snippet you provided:

  1. There are three instances of SpectacularAPIView being configured with permissions and authentication classes twice, which is redundant. This can be simplified by setting these once at the beginning.
  2. The URLs defined for /api/ and /blog/ are not explicitly specified. The line path("api/", include("users.urls")) is missing the actual URL pattern for "api/".

Here's an optimized version of the code:

# Corrected lines:
from django.urls import path, re_path, include
from drf_spectacular.views import (
    SpectacularAPIView,
    SpectacularRedocView,
    SpectacularSwaggerView,
)
from rest_framework.permissions import AllowAny

urlpatterns = [
    # Include user-related URLs using the 'include' function
    path("api/users/", include("users.urls")),
]

# Simplified configuration of views:
SpectacularAPIView.produces_formatters = ['application/vnd.openapi+json', 'application/yaml']

Changes Made:

  1. Removed Repeated Configurations: Removed the duplicated configurations for SpectacularAPIView, SpectacularSwaggerView, and SpectacularRedocView.
  2. Added Documentation Type Information: Added a more specific content type to improve API documentation quality. You might want to customize this based on your needs or preferences.

These changes make the code cleaner and maintainable while ensuring that the required functionality remains intact.

Expand Down
8 changes: 4 additions & 4 deletions apps/users/views/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
@date:2025/4/14 19:25
@desc:
"""
from drf_spectacular.utils import extend_schema
from rest_framework.views import APIView
from django.utils.translation import gettext_lazy as _
from drf_spectacular.utils import extend_schema
from rest_framework.request import Request
from rest_framework.views import APIView

from common.auth import TokenAuth
from common.auth.authenticate import TokenAuth
from common.auth.authentication import has_permissions
from common.constants.permission_constants import PermissionConstants
from common.result import result
Expand All @@ -36,7 +36,7 @@ class TestPermissionsUserView(APIView):

@extend_schema(methods=['GET'],
description=_("Get current user information"),
operation_id=_("Get current user information"),
operation_id="测试",
tags=[_("User management")],
responses=UserProfileAPI.get_response())
@has_permissions(PermissionConstants.USER_EDIT)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few issues and suggestions to make in the provided code:

Issues:

  1. Import Statement: The file authenticate.py is imported twice without specifying its exact source or path, which might lead to confusion or errors if not properly set up.
  2. Operation ID Mismatch: The operation IDs "测试" (test) may cause conflicts with existing operations since they have reserved characters like colons (:). Using non-reserved identifiers is recommended.

Suggestion:

To ensure clarity and consistency in naming conventions, you should either update the operation IDs or specify the exact path for the import statements in common/auth/authenticate.py.

Here's how you can address these issues:

# Example updates based on your suggestion

## Step 1: Update or clarify the import paths if necessary
# Uncomment the line below if it points to an incorrect location
#import common.auth.authenticate as authenticate

## Step 2: Correct the operation IDs or provide more context
@extend_schema(methods=['GET'],
            description=_("Get current user information"),
            #operation_id=_("Get current user information"),  # Remove this or change
            operation_id="GetUserInfo",  # Replace '测试' with a valid identifier
            tags=[_("User management")],
            responses=UserProfileAPI.get_response())
@has_permissions(PermissionConstants.USER_EDIT)
class TestPermissionsUserView(APIView):

This ensures that the project remains clear and avoids potential conflicts between different modules.

Expand Down