Skip to content

Conversation

Ishankoradia
Copy link
Contributor

@Ishankoradia Ishankoradia commented Oct 7, 2025

Summary by CodeRabbit

  • New Features

    • Added a public API endpoint at /login_token/ that accepts a token and returns the same response format as the standard login flow for consistent client integration.
    • /login/ and /logout/ endpoints remain unchanged.
  • Chores / Configuration

    • Staging app domain added to allowed CORS origins for cross-origin access during testing.

@Ishankoradia Ishankoradia requested a review from pradnk October 7, 2025 15:21
Copy link

coderabbitai bot commented Oct 7, 2025

Walkthrough

Adds a new POST endpoint /login_token/ that returns a login-shaped response using the JWT attached to the request; the auth middleware now stores the raw JWT on request.token. Also adds a staging origin to CORS whitelist in settings.

Changes

Cohort / File(s) Summary of Changes
User org API
ddpui/api/user_org_api.py
Added POST endpoint post_login_token at /login_token/. Validates request.user and username; returns 401 on failure. Uses orguserfunctions.lookup_user(username) and sets retval["token"] = request.token to return a login-shaped payload.
Auth middleware
ddpui/auth.py
In CustomJwtAuthMiddleware.authenticate, assigns request.token = token (adds the incoming JWT to the request) before returning the authenticated request.
Settings
ddpui/settings.py
Added https://staging-app.dalgo.org to CORS_ORIGIN_WHITELIST.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant C as Client
    participant Router as API Router
    participant M as CustomJwtAuthMiddleware
    participant H as post_login_token handler
    participant U as orguserfunctions

    C->>Router: POST /login_token/ (Authorization: Bearer JWT)
    Router->>M: authenticate(request)
    M->>M: validate JWT, set request.user, permissions
    M->>M: set request.token = JWT
    M-->>Router: authenticated request
    Router->>H: call handler(request)
    H->>H: verify request.user and username
    alt invalid
        H-->>C: 401 Invalid or missing token
    else valid
        H->>U: lookup_user(username)
        U-->>H: user details
        H->>H: set retval["token"] = request.token
        H-->>C: 200 JSON (login-shaped payload)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title refers to authentication updates for the v2 launch and thus touches on the correct area, but it remains vague by using “embed auth changes” without highlighting the specific additions such as the new /login_token/ endpoint or the JWT middleware modification. It does not clearly convey the most important change in the pull request. Consider revising the title to explicitly reference the key changes, for example “Add /login_token endpoint and expose JWT token in middleware for v2 launch,” so that the primary updates are immediately clear.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch embed-auth-changes

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9ade435 and d55dc32.

📒 Files selected for processing (1)
  • ddpui/settings.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: checks (3.10, 6)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

codecov bot commented Oct 7, 2025

Codecov Report

❌ Patch coverage is 33.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.65%. Comparing base (2ac2126) to head (d55dc32).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
ddpui/api/user_org_api.py 25.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1177      +/-   ##
==========================================
- Coverage   52.66%   52.65%   -0.02%     
==========================================
  Files          97       97              
  Lines       11406    11415       +9     
==========================================
+ Hits         6007     6010       +3     
- Misses       5399     5405       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
ddpui/api/user_org_api.py (1)

164-173: Clarify lookup_user docs, error handling, and response consistency

  • In ddpui/core/orguserfunctions.py update lookup_user’s docstring to “look up user by email” (or rename its email parameter) to match its implementation.
  • In post_login_token (user_org_api.py) change the error to HttpError(401, "User not found or invalid").
  • Consider returning refresh_token alongside token for consistency with /login/.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2ac2126 and 9ade435.

📒 Files selected for processing (2)
  • ddpui/api/user_org_api.py (1 hunks)
  • ddpui/auth.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
ddpui/api/user_org_api.py (2)
ddpui/tests/helper/test_auth.py (2)
  • user (23-26)
  • token (29-31)
ddpui/core/orguserfunctions.py (1)
  • lookup_user (42-73)
ddpui/auth.py (1)
ddpui/tests/helper/test_auth.py (1)
  • token (29-31)
🔇 Additional comments (1)
ddpui/auth.py (1)

170-170: LGTM!

Storing the validated token on the request object is appropriate for downstream use by the new /login_token/ endpoint. The token has already been validated at this point, so there are no security concerns.

@Ishankoradia Ishankoradia merged commit cf26ada into main Oct 8, 2025
2 of 5 checks passed
@Ishankoradia Ishankoradia deleted the embed-auth-changes branch October 8, 2025 11:57
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.

1 participant