Skip to content

Handle re-authentication for both unauthorized and forbidden responses #459

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.

Sign up for GitHub

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
Nov 28, 2025
Merged

Handle re-authentication for both unauthorized and forbidden responses #459

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions scripts/python/invoke-cron.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ async def invoke_endpoint(self, client: httpx.AsyncClient) -> dict:
logger.debug(f"Response status: {response.status_code}")
logger.debug(f"Response headers: {dict(response.headers)}")

# If unauthorized, re-authenticate and retry once
if response.status_code == 401:
logger.info("Token expired, re-authenticating...")
# If unauthorized or forbidden (token expired/invalid), re-authenticate and retry once
if response.status_code in (401, 403):
logger.info("Token expired or invalid, re-authenticating...")
Comment on lines +108 to +110
Copy link

@coderabbitai coderabbitai bot Nov 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, find and read the file to understand the context
wc -l scripts/python/invoke-cron.py

Repository: ProjectTech4DevAI/ai-platform

Length of output: 106

🏁 Script executed:

#!/bin/bash
# Read the full file to understand context
cat -n scripts/python/invoke-cron.py

Repository: ProjectTech4DevAI/ai-platform

Length of output: 8083

🏁 Script executed:

#!/bin/bash
# Check git history for this file to see if there's context about the 403 change
git log --oneline -20 scripts/python/invoke-cron.py

Repository: ProjectTech4DevAI/ai-platform

Length of output: 151

🏁 Script executed:

#!/bin/bash
# Check for Python version specification in project
find . -maxdepth 2 -type f \( -name "pyproject.toml" -o -name "setup.py" -o -name ".python-version" -o -name "Makefile" \) -exec cat {} \; 2>/dev/null | head -100

Repository: ProjectTech4DevAI/ai-platform

Length of output: 2254

Treating 403 (Forbidden) the same as 401 (Unauthorized) lacks justification and may cause unnecessary retries.

HTTP 403 indicates an authenticated user lacks permission, which is fundamentally different from 401 (authentication expired/invalid). If the endpoint returns 403 due to insufficient permissions, re-authenticating with the same credentials produces another token with identical permissions, causing the retry to fail again with 403. This wastes API calls without resolving the underlying authorization issue. The commit message provides no explanation for this behavior.

Add type hints to methods per project standards (Python 3.12+, mypy strict=true). Methods authenticate (line 53), invoke_endpoint (line 86), and run (line 131) are missing return type annotations.

Consider:

  • Only trigger re-authentication on 401 (authentication problem)
  • Let 403 (authorization problem) propagate as an error
  • If the API returns 403 for expired tokens (non-standard), document this clearly

Copy link
Collaborator

@Prajna1999 Prajna1999 Nov 28, 2025

Choose a reason for hiding this comment

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

Although this coderabbit comment makes sense, but as a hotfix this should work.

Copy link

@coderabbitai coderabbitai bot Nov 28, 2025

Choose a reason for hiding this comment

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

@Prajna1999, understood! For a hotfix, getting the system working quickly is the priority.

Would you like me to create a follow-up issue to properly distinguish between 401 and 403 responses in a future PR? That way, the temporary workaround can be refined when there's more time to implement the proper authorization error handling.

await self.authenticate(client)
headers = {"Authorization": f"Bearer {self.access_token}"}
response = await client.get(
Expand Down