Skip to content

feat: allow chatbot to access user data on server by user token#3152

Open
akiva10b wants to merge 1 commit intomasterfrom
auth-from-chatbot-to-ser-erv
Open

feat: allow chatbot to access user data on server by user token#3152
akiva10b wants to merge 1 commit intomasterfrom
auth-from-chatbot-to-ser-erv

Conversation

@akiva10b
Copy link
Contributor

This pull request introduces a new authentication mechanism that allows requests to be authenticated via an encrypted session ID header, supplementing Django's standard session authentication. The main change is the addition of a middleware that handles this authentication, along with corresponding updates to settings, utility functions, and tests.

Authentication enhancements:

  • Added SessionIDAuthMiddleware to authenticate requests with an X-Session-ID header, using a secure token to identify users, and integrated it into the Django middleware stack (sefaria/system/middleware.py, sefaria/settings.py). [1] [2]
  • Introduced SESSION_ID_AUTH_HEADER setting to specify the header name for session ID authentication (sefaria/settings.py).

Token handling improvements:

  • Implemented get_user_id_from_chatbot_user_token and improved token decryption logic, including expiration checks and error handling, to securely extract user IDs from session tokens (sefaria/utils/chatbot.py). [1] [2]

Testing:

  • Added comprehensive tests for SessionIDAuthMiddleware, covering valid, invalid, expired, and precedence scenarios for session ID authentication (sefaria/system/tests/test_middleware.py).
  • Updated test imports to support new middleware and token utilities (sefaria/system/tests/test_middleware.py).

Dependency and utility updates:

  • Updated imports to include required cryptography and Django utilities for token handling and authentication (sefaria/system/middleware.py, sefaria/utils/chatbot.py). [1] [2]This pull request introduces a new authentication mechanism that allows anonymous requests to be authenticated via an encrypted session ID header. The main change is the addition of the SessionIDAuthMiddleware, which supplements Django's session authentication by checking for a valid X-Session-ID header and associating the request with the corresponding user if present. The pull request also includes comprehensive tests to ensure the correctness of this middleware, and utility functions for securely encoding and decoding user tokens.

New authentication mechanism:

  • Added SessionIDAuthMiddleware to authenticate anonymous requests using an encrypted X-Session-ID header, supplementing Django's default session authentication. (sefaria/system/middleware.py)
  • Registered the new middleware in the Django middleware stack and defined the header name in settings. (sefaria/settings.py) [1] [2]

Token encoding/decoding utilities:

  • Implemented _decrypt_chatbot_user_token and get_user_id_from_chatbot_user_token to securely decode and validate encrypted tokens, checking expiration and extracting user ID. (sefaria/utils/chatbot.py) [1] [2]
  • Updated build_chatbot_user_token to include user_id in the payload for easier extraction during authentication. (sefaria/utils/chatbot.py)

Testing:

  • Added a test suite for SessionIDAuthMiddleware covering valid, invalid, expired, and precedence scenarios to ensure robust behavior. (sefaria/system/tests/test_middleware.py)
  • Imported necessary utilities and models for testing and middleware implementation. (sefaria/system/tests/test_middleware.py, sefaria/system/middleware.py) [1] [2]

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a header-based authentication path intended for chatbot/server-to-server calls by decrypting an encrypted session/user token and attaching the resolved Django user to otherwise-anonymous requests.

Changes:

  • Added SessionIDAuthMiddleware to authenticate anonymous requests using an encrypted X-Session-ID-style header.
  • Added token decryption + user-id extraction utilities in sefaria/utils/chatbot.py and updated token payload to include user_id.
  • Added tests for the new middleware and wired the middleware + header setting into global Django settings.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
sefaria/utils/chatbot.py Adds decryption + expiration validation and get_user_id_from_chatbot_user_token; updates token payload to include user_id.
sefaria/system/middleware.py Introduces SessionIDAuthMiddleware that reads a header token, resolves a user, and sets request.user.
sefaria/settings.py Adds SESSION_ID_AUTH_HEADER setting and registers the new middleware in MIDDLEWARE.
sefaria/system/tests/test_middleware.py Adds a new test class covering basic valid/invalid/expired and precedence scenarios for the middleware.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +314 to +320
authenticated user from the session, this middleware leaves it untouched.
"""

def process_request(self, request):
if getattr(request, "user", None) and request.user.is_authenticated:
return

Comment on lines 121 to +125
'django.middleware.csrf.CsrfViewMiddleware',
'django.middleware.locale.LocaleMiddleware',
'django.middleware.common.CommonMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'sefaria.system.middleware.SessionIDAuthMiddleware',
# Make this unique, and don't share it with anybody.
SECRET_KEY = ''
CHATBOT_USER_ID_SECRET = 'secret'
SESSION_ID_AUTH_HEADER = 'HTTP_X_SESSION_ID'
Comment on lines +537 to +541
def test_invalid_header_leaves_request_anonymous(self):
request = self.factory.get("/", HTTP_X_SESSION_ID="not-a-valid-token")
request.user = AnonymousUser()

with override_settings(CHATBOT_USER_ID_SECRET=self.secret):
@mergify
Copy link

mergify bot commented Mar 17, 2026

🧪 CI Insights

Here's what we observed from your CI run for 86fd18d.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries 🔍 CI Insights 📄 Logs
Continuous Continuous Testing: PyTest Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants