Conversation
…as_more functionality
- Add GET /projects/organization/{org_id} endpoint with org validation
- Add has_more pagination to organizations list endpoint
- Add Swagger docs for list_by_org
…organizations; update read_projects_by_organization response type
…om/ProjectTech4DevAI/kaapi-backend into feat/google-integration-auth-flow
…kend into feat/google-integration-auth-flow
…m/ProjectTech4DevAI/kaapi-backend into feat/add-user-project
…m/ProjectTech4DevAI/kaapi-backend into feat/add-user-project
…m/ProjectTech4DevAI/kaapi-backend into feat/add-user-project
…m/ProjectTech4DevAI/kaapi-backend into feat/add-user-project
…h4DevAI/kaapi-backend into feat/add-user-project
…m/ProjectTech4DevAI/kaapi-backend into feat/invitation-flow
…kend into feat/invitation-flow
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/tests/api/test_auth.py (1)
89-108:⚠️ Potential issue | 🟡 MinorTest name contradicts both the docstring and actual behavior.
The test is named
test_google_auth_inactive_user_rejectedbut:
- The docstring on line 92 states it tests that "inactive user is activated on first Google login"
- The test expects HTTP 200 (success), not a rejection
- Per
backend/app/api/routes/auth.pylines 78-84, inactive users ARE activated on first Google loginThe original name
test_google_auth_activates_inactive_userwas accurate. Consider reverting the rename or updating the test to match the new name if the intended behavior has changed.Proposed fix: Revert to accurate test name
- def test_google_auth_inactive_user_rejected( + def test_google_auth_activates_inactive_user( self, mock_settings, mock_verify, db: Session, client: TestClient ): """Test that inactive user is activated on first Google login."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/api/test_auth.py` around lines 89 - 108, The test name is misleading—rename the test function back to test_google_auth_activates_inactive_user and ensure its docstring and assertions match that behavior: update the function name (currently test_google_auth_inactive_user_rejected) in backend/app/tests/api/test_auth.py to test_google_auth_activates_inactive_user and keep the existing docstring and the assert expecting HTTP 200 so it accurately reflects that inactive users are activated on first Google login (referencing the Google auth flow used in the test and mock_verify/_mock_idinfo).
🧹 Nitpick comments (4)
backend/app/api/routes/user_project.py (1)
99-124: Synchronous email sending may slow response time.Sending emails synchronously in the request loop could significantly delay the API response when adding multiple users, especially if the SMTP server is slow or unreachable. Consider offloading email delivery to a background task (e.g., Celery) for better user experience.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/user_project.py` around lines 99 - 124, The loop currently sends emails synchronously using generate_invite_token, generate_invite_email and send_email which can block the request; change this to enqueue an asynchronous background job instead (e.g., push a task to Celery/RQ/your background worker) that receives the email params (email, project_name, organization_name, invite_token) and performs generate_invite_email/send_email there, while the API handler immediately returns after scheduling the tasks and logs task IDs via logger; ensure exceptions in the worker are handled and retried rather than blocking the request flow.backend/app/email-templates/build/invite_user.html (1)
15-17: Hardcoded app name inconsistent with template variable usage.Line 16 hardcodes "Kaapi Konsole" while line 27 uses
{{ app_name }}from the template context. For consistency and maintainability, use the template variable throughout.Proposed fix
<h1 style="margin: 0 0 4px; font-size: 22px; font-weight: 600; color: `#18181b`;"> - Kaapi Konsole + {{ app_name }} </h1>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/email-templates/build/invite_user.html` around lines 15 - 17, Replace the hardcoded "Kaapi Konsole" text inside the h1 element with the template variable {{ app_name }} so the template uses the same context value used elsewhere (line that currently uses {{ app_name }}); keep the existing h1 styling/attributes unchanged and ensure the variable is rendered/escaped consistently with the other occurrences of {{ app_name }} in the template.backend/app/services/auth.py (1)
190-192: Inconsistent timestamp format betweenexpandnbfclaims.
expis encoded as a Unix timestamp (float) via.timestamp(), whilenbfis passed as a datetime object. While PyJWT accepts both, this inconsistency can be confusing. Consider using the same format for both.Proposed fix for consistency
to_encode = { "exp": expires.timestamp(), - "nbf": now, + "nbf": now.timestamp(), "sub": email, "org_id": organization_id, "project_id": project_id, "type": "invite", }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/auth.py` around lines 190 - 192, The JWT claims in to_encode use inconsistent formats: "exp" uses expires.timestamp() while "nbf" uses the datetime now object; normalize both to the same type (e.g., numeric Unix timestamps) to avoid confusion. Locate the to_encode construction in auth.py (the to_encode dict, the "exp" and "nbf" keys, and the expires and now variables) and change "nbf" to use now.timestamp() (or convert both to datetime if you prefer consistency), ensuring the token encoding call uses the updated values.backend/app/utils.py (1)
204-204:valid_dayscould display as 0 for short token expiry.If
INVITE_TOKEN_EXPIRE_HOURSis less than 24, integer division results invalid_days = 0. Consider usingmax(1, ...)or displaying hours when the expiry is less than a day.Proposed fix
- "valid_days": settings.INVITE_TOKEN_EXPIRE_HOURS // 24, + "valid_days": max(1, settings.INVITE_TOKEN_EXPIRE_HOURS // 24),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/utils.py` at line 204, The current calculation for "valid_days" in utils.py uses integer division on settings.INVITE_TOKEN_EXPIRE_HOURS which yields 0 when the expiry is under 24 hours; update the logic that sets "valid_days" (the dict key "valid_days") to avoid 0 by converting hours to days using a safe calculation (e.g., compute days = ceil(INVITE_TOKEN_EXPIRE_HOURS / 24) or use max(1, ...)); alternatively, if you want to preserve precision, return a human-friendly representation that shows hours when INVITE_TOKEN_EXPIRE_HOURS < 24 (e.g., use hours string), but ensure the code around the "valid_days" key (where settings.INVITE_TOKEN_EXPIRE_HOURS is referenced) is updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/api/routes/auth.py`:
- Around line 204-245: The current verify_invitation handler (verify_invitation)
performs side effects (activates user, commits, issues tokens via
build_token_response) while being a GET; change behavior so GET is read-only:
verify the token using verify_invite_token and return a non-destructive status
(e.g., token valid/expired and minimal invite metadata) without activating the
user or issuing tokens and do not perform DB commits. Add a new POST endpoint
(e.g., accept_invitation) that accepts the invite token in the request body (not
URL), calls verify_invite_token, activates the user (set user.is_active,
session.add/commit/refresh), calls build_token_response to issue tokens/cookies,
and returns the auth response; ensure verify_invitation no longer calls
session.commit, session.refresh, or build_token_response and that secrets are
not passed in URLs.
- Around line 219-240: The token is minted directly from invite_data which can
be stale; before calling build_token_response in the verify-invite flow,
re-resolve current access from the DB (using the same logic as select_project on
lines ~142-156) — e.g., query the user's current organization/project membership
with the session (or call select_project(user.id,
invite_data["organization_id"], invite_data["project_id"], session)) and if the
project/organization access no longer exists or the mapping changed, raise an
HTTPException (403 or 404) rejecting the stale invite; only call
build_token_response when the DB-confirmed project access is present.
In `@backend/app/api/routes/user_project.py`:
- Around line 118-123: Log messages in add_project_users currently print raw
emails; update the logger calls (logger.info and logger.error in the
add_project_users block) to pass the email through mask_string before
interpolation so sensitive addresses are masked. Ensure you call
mask_string(entry.email) in both the success and exception log lines and keep
the existing format that prefixes the function name in square brackets; also add
an import for mask_string if it's not already imported.
In `@backend/app/utils.py`:
- Around line 187-207: The generate_invite_email function declares email_to but
doesn't pass it to the template; update generate_invite_email to include the
email address in the render_email_template context (add "email": email_to or
same key used by generate_reset_password_email/generate_new_account_email) so
the invite template can render the recipient, or if the template intentionally
doesn't need it, remove the unused email_to parameter from the function
signature and all call sites; locate the function by name
(generate_invite_email) and adjust either the context dict or the signature
accordingly.
---
Outside diff comments:
In `@backend/app/tests/api/test_auth.py`:
- Around line 89-108: The test name is misleading—rename the test function back
to test_google_auth_activates_inactive_user and ensure its docstring and
assertions match that behavior: update the function name (currently
test_google_auth_inactive_user_rejected) in backend/app/tests/api/test_auth.py
to test_google_auth_activates_inactive_user and keep the existing docstring and
the assert expecting HTTP 200 so it accurately reflects that inactive users are
activated on first Google login (referencing the Google auth flow used in the
test and mock_verify/_mock_idinfo).
---
Nitpick comments:
In `@backend/app/api/routes/user_project.py`:
- Around line 99-124: The loop currently sends emails synchronously using
generate_invite_token, generate_invite_email and send_email which can block the
request; change this to enqueue an asynchronous background job instead (e.g.,
push a task to Celery/RQ/your background worker) that receives the email params
(email, project_name, organization_name, invite_token) and performs
generate_invite_email/send_email there, while the API handler immediately
returns after scheduling the tasks and logs task IDs via logger; ensure
exceptions in the worker are handled and retried rather than blocking the
request flow.
In `@backend/app/email-templates/build/invite_user.html`:
- Around line 15-17: Replace the hardcoded "Kaapi Konsole" text inside the h1
element with the template variable {{ app_name }} so the template uses the same
context value used elsewhere (line that currently uses {{ app_name }}); keep the
existing h1 styling/attributes unchanged and ensure the variable is
rendered/escaped consistently with the other occurrences of {{ app_name }} in
the template.
In `@backend/app/services/auth.py`:
- Around line 190-192: The JWT claims in to_encode use inconsistent formats:
"exp" uses expires.timestamp() while "nbf" uses the datetime now object;
normalize both to the same type (e.g., numeric Unix timestamps) to avoid
confusion. Locate the to_encode construction in auth.py (the to_encode dict, the
"exp" and "nbf" keys, and the expires and now variables) and change "nbf" to use
now.timestamp() (or convert both to datetime if you prefer consistency),
ensuring the token encoding call uses the updated values.
In `@backend/app/utils.py`:
- Line 204: The current calculation for "valid_days" in utils.py uses integer
division on settings.INVITE_TOKEN_EXPIRE_HOURS which yields 0 when the expiry is
under 24 hours; update the logic that sets "valid_days" (the dict key
"valid_days") to avoid 0 by converting hours to days using a safe calculation
(e.g., compute days = ceil(INVITE_TOKEN_EXPIRE_HOURS / 24) or use max(1, ...));
alternatively, if you want to preserve precision, return a human-friendly
representation that shows hours when INVITE_TOKEN_EXPIRE_HOURS < 24 (e.g., use
hours string), but ensure the code around the "valid_days" key (where
settings.INVITE_TOKEN_EXPIRE_HOURS is referenced) is updated accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b8bd898d-60f0-440e-ad07-982c03efc909
📒 Files selected for processing (9)
.env.examplebackend/app/api/docs/auth/invite_verify.mdbackend/app/api/routes/auth.pybackend/app/api/routes/user_project.pybackend/app/core/config.pybackend/app/email-templates/build/invite_user.htmlbackend/app/services/auth.pybackend/app/tests/api/test_auth.pybackend/app/utils.py
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/app/tests/api/test_user_project.py (1)
136-136: Assertsend_emailcall arguments, not only call count.
assert_called_once()can pass even if the email was sent to the wrong recipient. Assert key kwargs to make this test resilient.Proposed diff
-from unittest.mock import patch +from unittest.mock import ANY, patch @@ - mock_send_email.assert_called_once() + mock_send_email.assert_called_once_with( + email_to=email, + subject=ANY, + html_content=ANY, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/api/test_user_project.py` at line 136, Test currently only checks mock_send_email.assert_called_once(), which misses verifying recipients/contents; update the assertion in test_user_project (where mock_send_email is used) to assert the actual call arguments: either replace assert_called_once() with assert_called_once_with(...) supplying the expected positional/keyword params (e.g., recipient/to, subject, template, context) or retrieve mock_send_email.call_args and assert call_args.kwargs['to'] (and other key kwargs) equal the expected values so the test verifies the correct recipient and email payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/tests/api/test_auth.py`:
- Around line 305-374: All six test methods in TestInviteVerify and
TestTokenGeneration are missing explicit return type annotations; add "-> None"
to the signatures of test_verify_invalid_token, test_verify_user_not_found,
test_verify_activates_inactive_user, test_verify_success_active_user,
test_generate_and_verify_invite_token, and test_verify_invite_token_invalid so
each method signature declares a None return type to satisfy the repository
typing standard.
In `@backend/app/tests/api/test_user_project.py`:
- Around line 108-115: Update the test signature for
test_add_user_sends_invite_email to add proper type hints: annotate the
mock_settings and mock_send_email parameters as MagicMock and add an explicit
return type -> None; locate the function definition named
test_add_user_sends_invite_email and change its signature to include
mock_settings: MagicMock, mock_send_email: MagicMock, and the trailing -> None
while leaving other parameters unchanged so type checkers accept the test.
---
Nitpick comments:
In `@backend/app/tests/api/test_user_project.py`:
- Line 136: Test currently only checks mock_send_email.assert_called_once(),
which misses verifying recipients/contents; update the assertion in
test_user_project (where mock_send_email is used) to assert the actual call
arguments: either replace assert_called_once() with assert_called_once_with(...)
supplying the expected positional/keyword params (e.g., recipient/to, subject,
template, context) or retrieve mock_send_email.call_args and assert
call_args.kwargs['to'] (and other key kwargs) equal the expected values so the
test verifies the correct recipient and email payload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac513b9f-03b5-4368-8dff-2871fd057a9c
📒 Files selected for processing (2)
backend/app/tests/api/test_auth.pybackend/app/tests/api/test_user_project.py
vprashrex
approved these changes
Apr 14, 2026
AkhileshNegi
requested changes
Apr 15, 2026
| return pyjwt.encode(to_encode, settings.SECRET_KEY, algorithm=security.ALGORITHM) | ||
|
|
||
|
|
||
| def verify_invite_token(token: str) -> dict | None: |
Collaborator
There was a problem hiding this comment.
instead of dict can we have some pydantic model instead
| return user, token_data | ||
|
|
||
|
|
||
| def generate_invite_token( |
Collaborator
There was a problem hiding this comment.
This JWT generate/verify pair is now duplicated three times
- (access/refresh in core/security.py
- password-reset in utils.py:210-230
- invite in services/auth.py:181-219)
can you check refactor opportunity, ideally put these things in Utils
Collaborator
Author
There was a problem hiding this comment.
Now, so for this I have created the one function that name encode_jwt_token in the security.py then used this function everywhere as per needed.
Here it's: https://github.com/ProjectTech4DevAI/kaapi-backend/pull/739/changes#diff-3b18f773289cc1dbcdde7e163fa058a7d1fead67073a81c5409f89646bfbd503R71
| project_id: int, | ||
| ) -> str: | ||
| """Generate a JWT invitation token for a user.""" | ||
| delta = timedelta(hours=settings.INVITE_TOKEN_EXPIRE_HOURS) |
Collaborator
There was a problem hiding this comment.
we should verify organization_id and project_id also
| # Frontend URL for magic links | ||
| FRONTEND_HOST: str = "http://localhost:3000" | ||
|
|
||
| # Invitation token expiry (default 7 days) |
Collaborator
There was a problem hiding this comment.
this is long time for expiry, for first time login atmost it should be 24 hrs
| GOOGLE_CLIENT_ID: str = "" | ||
|
|
||
| # Frontend URL for magic links | ||
| FRONTEND_HOST: str = "http://localhost:3000" |
Collaborator
There was a problem hiding this comment.
this is hardcoded here, how this is taken care of in staging/production
Collaborator
Author
There was a problem hiding this comment.
So the http://localhost:3000 is just a default for local development. Pydantic Settings automatically reads FRONTEND_HOST from the environment (or .env file) and overrides the default, so as long as staging/production sets FRONTEND_HOST in their respective .env, it will use the correct value.
| def verify_invitation(session: SessionDep, token: str) -> JSONResponse: | ||
| """Verify an invitation token, activate the user, and log them in.""" | ||
|
|
||
| invite_data = verify_invite_token(token) |
Collaborator
There was a problem hiding this comment.
rename to invite_payload for better readability
| def test_verify_user_not_found(self, client: TestClient): | ||
| """Test returns 404 when invited user doesn't exist.""" | ||
| token = generate_invite_token( | ||
| email="ghost@example.com", organization_id=1, project_id=1 |
Collaborator
There was a problem hiding this comment.
can we do better her to not use hardcoded organization_id=1, project_id=1.
AkhileshNegi
approved these changes
Apr 16, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
backend/app/api/routes/user_project.py (1)
122-128:⚠️ Potential issue | 🟡 MinorMask email in log messages.
Raw recipient emails are still interpolated into both the success and error logs. Pass through
mask_stringto match the project's logging convention for sensitive values.Suggested diff
from app.utils import ( APIResponse, generate_invite_email, load_description, + mask_string, send_email, ) @@ - logger.info( - f"[add_project_users] Invitation email sent | email: {entry.email}" - ) + logger.info( + f"[add_project_users] Invitation email sent | email: {mask_string(str(entry.email))}" + ) except Exception as e: - logger.error( - f"[add_project_users] Failed to send invitation email | email: {entry.email}, error: {e}" - ) + logger.error( + f"[add_project_users] Failed to send invitation email | email: {mask_string(str(entry.email))}, error: {e}" + )As per coding guidelines: "Prefix all log messages with the function name in square brackets:
logger.info(f\"[function_name] Message {mask_string(sensitive_value)}\")".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/user_project.py` around lines 122 - 128, The logs in add_project_users are printing raw recipient emails; wrap the sensitive value with mask_string when logging. Update both logger.info and logger.error to use mask_string(entry.email) instead of entry.email, keeping the existing prefix "[add_project_users]" and message text otherwise so logs follow the project's masking convention.backend/app/api/routes/auth.py (2)
223-247:⚠️ Potential issue | 🔴 CriticalToken is minted from invite claims without re-checking current user-project mapping.
build_token_responseusesinvite_payload.organization_id/project_iddirectly. If the user-project assignment was revoked between invite issuance and acceptance, this still mints a scoped JWT for the now-stale project. Re-resolve access from the DB (asselect_projectdoes on Lines 144-151 viaget_user_accessible_projects) and reject when the mapping no longer exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/auth.py` around lines 223 - 247, When minting the token in verify_invitation, do not trust invite_payload.project_id/org_id directly; re-resolve the user's current access via the same DB checks used elsewhere (e.g., call get_user_accessible_projects or the logic in select_project) after fetching user with get_user_by_email and before calling build_token_response, ensure invite_payload.project_id is present in the returned accessible projects for that user (and that organization matches); if the project mapping no longer exists, raise an HTTPException (403 or 404 as appropriate) instead of building the token.
206-252:⚠️ Potential issue | 🟠 MajorGET endpoint performs state mutation and issues auth tokens.
This handler activates the user, commits to the DB, and mints access/refresh tokens on a
GET. Mail scanners, link preview bots, and browser prefetch can all follow invite URLs and silently consume/activate the invite before the invitee intentionally accepts. KeepGET /invite/verifyas a read-only token inspection (return validity + minimal invite metadata) and move activation +build_token_responseinto an explicitPOST /invite/acceptthat takes the token in the request body.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/auth.py` around lines 206 - 252, The GET handler verify_invitation currently mutates state and issues tokens; change its behavior to be read-only: have verify_invitation call verify_invite_token, validate_organization, validate_project and get_user_by_email (if needed) but do NOT activate the user, commit, or call build_token_response — instead return a minimal validity response and invite metadata (organization_id, project_id, email, expires_at). Add a new POST endpoint (e.g., /invite/accept) that accepts the token in the request body, calls verify_invite_token, validate_organization, validate_project, then performs the activation flow (set user.is_active, session.add, session.commit, session.refresh) and finally calls build_token_response to return tokens; reference the existing functions verify_invite_token, validate_organization, validate_project, get_user_by_email, and build_token_response when implementing these changes.backend/app/utils.py (1)
185-205:⚠️ Potential issue | 🟡 MinorUnused
email_toparameter (still not in template context).
email_tois accepted but never passed torender_email_template, unlike sibling helpers (generate_reset_password_email,generate_new_account_email). Either drop it from the signature or add it to the context so the recipient can be referenced ininvite_user.html. Ruff still flags this (ARG001).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/utils.py` around lines 185 - 205, The generate_invite_email function currently accepts email_to but never passes it to render_email_template, causing an unused-argument lint (ARG001); update the function so that the "email_to" value is included in the context passed to render_email_template (same place where app_name, project_name, organization_name, link, and valid_days are set) so invite_user.html can reference the recipient, or alternatively remove the email_to parameter from the generate_invite_email signature if the template does not need it; ensure the change is made in the generate_invite_email function and that render_email_template and invite_user.html remain consistent with the chosen approach.
🧹 Nitpick comments (5)
backend/app/core/security.py (2)
94-107:decode_jwt_tokensilently swallows all token failures.Returning
Nonefor everyInvalidTokenError(expired, bad signature, wrong issuer, malformed, type mismatch) is fine for the happy-path callers, but it leaves no signal to distinguish "expired invite" from "tampered token" in logs. Sinceverify_invitationalready maps both to a generic 400, consider at least a debug-level log inside theexceptbranch so operators can diagnose token rejections without changing the public contract.Suggested
try: payload = jwt.decode(token, settings.SECRET_KEY, algorithms=[ALGORITHM]) - except InvalidTokenError: + except InvalidTokenError as e: + logger.debug(f"[decode_jwt_token] Token rejected: {e}") return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/security.py` around lines 94 - 107, The decode_jwt_token function currently swallows InvalidTokenError without logging; update its except InvalidTokenError block to emit a debug-level log (via the module logger) including the caught exception and the token's intended expected_type, and also add a debug log when the payload type mismatches expected_type (include payload.get("type") and expected_type) so operators can diagnose expired/tampered/malformed tokens while the function still returns None.
82-82: Optional: usedatetime.UTCalias (Python 3.11+).Ruff flags
timezone.utchere asUP017. Since the project targets Python 3.11+,datetime.UTCis the idiomatic shorthand.Proposed change
-from datetime import datetime, timedelta, timezone +from datetime import UTC, datetime, timedelta @@ - now = datetime.now(timezone.utc) + now = datetime.now(UTC)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/core/security.py` at line 82, Replace use of timezone.utc with the datetime.UTC alias: change the call that sets now (datetime.now(timezone.utc)) to datetime.now(datetime.UTC) and adjust imports accordingly (remove or stop using timezone if present) so the code uses the Python 3.11+ datetime.UTC idiom; update the occurrence around the now assignment and any related references to timezone.utc in the same module (e.g., in core/security.py where now is computed).backend/app/api/routes/auth.py (1)
211-211: Consider a Pydantic body/model for the token input.The token is accepted as a bare query string on a
GET. Even once this is migrated toPOSTper the earlier comment, prefer a request model (InviteVerifyRequestwithtoken: str) so the payload is typed, documented in OpenAPI, and validated consistently with the rest of the auth routes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/auth.py` at line 211, The verify_invitation function currently accepts a bare token string; instead create a Pydantic request model (e.g., InviteVerifyRequest with token: str) and change verify_invitation to accept that model (and switch the route to POST per earlier comment) so FastAPI/OpenAPI will validate and document the payload; update any references/calls to verify_invitation to pass InviteVerifyRequest.token and adjust tests or client code accordingly.backend/app/api/routes/user_project.py (2)
99-128: Synchronous SMTP send blocks the request thread.Each invited user triggers a blocking
send_emailover SMTP inside the HTTP handler. For a bulk add (N users) with slow/unreachable SMTP, the request worker is held for N × timeout seconds and the caller waits for the full fan-out. Consider offloading to FastAPI'sBackgroundTasks(or a proper task queue) so the response returns promptly and a single flaky recipient can't stall others.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/user_project.py` around lines 99 - 128, The route currently sends SMTP emails synchronously in the request handler (loop using generate_invite_token/generate_invite_email/send_email), which blocks the worker for each recipient; change the handler to accept FastAPI's BackgroundTasks and offload sending: inside the loop, generate the invite_token and email_data synchronously but instead of calling send_email directly schedule a background task (background_tasks.add_task) that calls a new helper (or the existing send_email wrapped with logging/error handling) for each entry so responses return immediately; ensure you reference the same symbols (generate_invite_token, generate_invite_email, send_email, logger, body.users) and preserve per-recipient logging and exception handling inside the background task.
61-101: Eliminate redundant database queries by reusing validate results.
validate_organization()andvalidate_project()both return the loaded entities; capture these instead of discarding them and fetching again withget_organization_by_id()andget_project_by_id(). This saves two queries and eliminates the unnecessaryand organization and projectdefensive check.Suggested diff
- # Validate org and project exist and are active before issuing any invites. - validate_organization(session=session, org_id=body.organization_id) - validate_project(session=session, project_id=body.project_id) + # Validate org and project exist and are active before issuing any invites. + organization = validate_organization(session=session, org_id=body.organization_id) + project = validate_project(session=session, project_id=body.project_id) @@ - # Send invitation emails - organization = get_organization_by_id(session=session, org_id=body.organization_id) - project = get_project_by_id(session=session, project_id=body.project_id) - - if settings.emails_enabled and organization and project: + # Send invitation emails + if settings.emails_enabled:Remove unused imports of
get_organization_by_idandget_project_by_idfrom lines 9-10.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes/user_project.py` around lines 61 - 101, The validate_organization(...) and validate_project(...) calls already load the Organization and Project; change those calls to capture and reuse their return values (e.g., organization = validate_organization(...), project = validate_project(...)) and then remove the subsequent get_organization_by_id(...) and get_project_by_id(...) calls and any now-unused imports; also remove the redundant defensive check that tested "and organization and project" since the validators return the entities or raise on error. Ensure add_user_to_project(...) usage remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/utils.py`:
- Line 202: The current assignment to valid_days uses integer division
(valid_days = settings.INVITE_TOKEN_EXPIRE_HOURS // 24) which yields 0 for TTLs
under 24 hours; replace that expression with max(1,
settings.INVITE_TOKEN_EXPIRE_HOURS // 24) to ensure the email never shows "0
days" (or alternatively compute hours and switch the template when
INVITE_TOKEN_EXPIRE_HOURS < 24); update the assignment of valid_days in the same
place where settings.INVITE_TOKEN_EXPIRE_HOURS is referenced.
---
Duplicate comments:
In `@backend/app/api/routes/auth.py`:
- Around line 223-247: When minting the token in verify_invitation, do not trust
invite_payload.project_id/org_id directly; re-resolve the user's current access
via the same DB checks used elsewhere (e.g., call get_user_accessible_projects
or the logic in select_project) after fetching user with get_user_by_email and
before calling build_token_response, ensure invite_payload.project_id is present
in the returned accessible projects for that user (and that organization
matches); if the project mapping no longer exists, raise an HTTPException (403
or 404 as appropriate) instead of building the token.
- Around line 206-252: The GET handler verify_invitation currently mutates state
and issues tokens; change its behavior to be read-only: have verify_invitation
call verify_invite_token, validate_organization, validate_project and
get_user_by_email (if needed) but do NOT activate the user, commit, or call
build_token_response — instead return a minimal validity response and invite
metadata (organization_id, project_id, email, expires_at). Add a new POST
endpoint (e.g., /invite/accept) that accepts the token in the request body,
calls verify_invite_token, validate_organization, validate_project, then
performs the activation flow (set user.is_active, session.add, session.commit,
session.refresh) and finally calls build_token_response to return tokens;
reference the existing functions verify_invite_token, validate_organization,
validate_project, get_user_by_email, and build_token_response when implementing
these changes.
In `@backend/app/api/routes/user_project.py`:
- Around line 122-128: The logs in add_project_users are printing raw recipient
emails; wrap the sensitive value with mask_string when logging. Update both
logger.info and logger.error to use mask_string(entry.email) instead of
entry.email, keeping the existing prefix "[add_project_users]" and message text
otherwise so logs follow the project's masking convention.
In `@backend/app/utils.py`:
- Around line 185-205: The generate_invite_email function currently accepts
email_to but never passes it to render_email_template, causing an
unused-argument lint (ARG001); update the function so that the "email_to" value
is included in the context passed to render_email_template (same place where
app_name, project_name, organization_name, link, and valid_days are set) so
invite_user.html can reference the recipient, or alternatively remove the
email_to parameter from the generate_invite_email signature if the template does
not need it; ensure the change is made in the generate_invite_email function and
that render_email_template and invite_user.html remain consistent with the
chosen approach.
---
Nitpick comments:
In `@backend/app/api/routes/auth.py`:
- Line 211: The verify_invitation function currently accepts a bare token
string; instead create a Pydantic request model (e.g., InviteVerifyRequest with
token: str) and change verify_invitation to accept that model (and switch the
route to POST per earlier comment) so FastAPI/OpenAPI will validate and document
the payload; update any references/calls to verify_invitation to pass
InviteVerifyRequest.token and adjust tests or client code accordingly.
In `@backend/app/api/routes/user_project.py`:
- Around line 99-128: The route currently sends SMTP emails synchronously in the
request handler (loop using
generate_invite_token/generate_invite_email/send_email), which blocks the worker
for each recipient; change the handler to accept FastAPI's BackgroundTasks and
offload sending: inside the loop, generate the invite_token and email_data
synchronously but instead of calling send_email directly schedule a background
task (background_tasks.add_task) that calls a new helper (or the existing
send_email wrapped with logging/error handling) for each entry so responses
return immediately; ensure you reference the same symbols
(generate_invite_token, generate_invite_email, send_email, logger, body.users)
and preserve per-recipient logging and exception handling inside the background
task.
- Around line 61-101: The validate_organization(...) and validate_project(...)
calls already load the Organization and Project; change those calls to capture
and reuse their return values (e.g., organization = validate_organization(...),
project = validate_project(...)) and then remove the subsequent
get_organization_by_id(...) and get_project_by_id(...) calls and any now-unused
imports; also remove the redundant defensive check that tested "and organization
and project" since the validators return the entities or raise on error. Ensure
add_user_to_project(...) usage remains unchanged.
In `@backend/app/core/security.py`:
- Around line 94-107: The decode_jwt_token function currently swallows
InvalidTokenError without logging; update its except InvalidTokenError block to
emit a debug-level log (via the module logger) including the caught exception
and the token's intended expected_type, and also add a debug log when the
payload type mismatches expected_type (include payload.get("type") and
expected_type) so operators can diagnose expired/tampered/malformed tokens while
the function still returns None.
- Line 82: Replace use of timezone.utc with the datetime.UTC alias: change the
call that sets now (datetime.now(timezone.utc)) to datetime.now(datetime.UTC)
and adjust imports accordingly (remove or stop using timezone if present) so the
code uses the Python 3.11+ datetime.UTC idiom; update the occurrence around the
now assignment and any related references to timezone.utc in the same module
(e.g., in core/security.py where now is computed).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5317f17b-6b15-48cb-b4bd-694a02d50fea
📒 Files selected for processing (9)
backend/app/api/routes/auth.pybackend/app/api/routes/user_project.pybackend/app/core/config.pybackend/app/core/security.pybackend/app/models/__init__.pybackend/app/models/auth.pybackend/app/services/auth.pybackend/app/tests/api/test_auth.pybackend/app/utils.py
✅ Files skipped from review due to trivial changes (2)
- backend/app/models/init.py
- backend/app/models/auth.py
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/app/services/auth.py
- backend/app/core/config.py
- backend/app/tests/api/test_auth.py
| "project_name": project_name, | ||
| "organization_name": organization_name, | ||
| "link": link, | ||
| "valid_days": settings.INVITE_TOKEN_EXPIRE_HOURS // 24, |
There was a problem hiding this comment.
valid_days truncates to 0 for sub‑24h TTLs.
settings.INVITE_TOKEN_EXPIRE_HOURS // 24 renders as 0 if the token TTL is configured below 24 hours, which would read as "valid for 0 days" in the email. Consider either guarding with max(1, ...) or switching the template copy to hours when appropriate.
Suggested guard
- "valid_days": settings.INVITE_TOKEN_EXPIRE_HOURS // 24,
+ "valid_days": max(1, settings.INVITE_TOKEN_EXPIRE_HOURS // 24),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "valid_days": settings.INVITE_TOKEN_EXPIRE_HOURS // 24, | |
| html_content = render_email_template( | |
| template_name="invite_user.html", | |
| context={ | |
| "app_name": app_name, | |
| "project_name": project_name, | |
| "organization_name": organization_name, | |
| "link": link, | |
| "valid_days": max(1, settings.INVITE_TOKEN_EXPIRE_HOURS // 24), | |
| }, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/utils.py` at line 202, The current assignment to valid_days uses
integer division (valid_days = settings.INVITE_TOKEN_EXPIRE_HOURS // 24) which
yields 0 for TTLs under 24 hours; replace that expression with max(1,
settings.INVITE_TOKEN_EXPIRE_HOURS // 24) to ensure the email never shows "0
days" (or alternatively compute hours and switch the template when
INVITE_TOKEN_EXPIRE_HOURS < 24); update the assignment of valid_days in the same
place where settings.INVITE_TOKEN_EXPIRE_HOURS is referenced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue: ProjectTech4DevAI/kaapi-frontend#112
Summary:
Notes:
.envwith this values:Summary by CodeRabbit