Skip to content

Add VerifiedGuard#218

Draft
ahmedxgouda wants to merge 2 commits into
devfrom
feature/add-verified-guard
Draft

Add VerifiedGuard#218
ahmedxgouda wants to merge 2 commits into
devfrom
feature/add-verified-guard

Conversation

@ahmedxgouda

@ahmedxgouda ahmedxgouda commented Jul 7, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Added email verification checks for protected areas of the app.
  • Bug Fixes

    • Updated several secured sections to require both approval and verified email status before access is granted.
    • Requests from unauthenticated users continue to pass through as before, while unverified signed-in users now receive a clear access denied message.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 34aa01f2-9b44-4ba8-8c5e-03da30f22efb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

A new VerifiedGuard is added that blocks requests from authenticated users whose email is not verified, while allowing unauthenticated requests through. This guard is then registered alongside the existing ApprovedGuard on seven controllers.

Changes

VerifiedGuard implementation and controller wiring

Layer / File(s) Summary
VerifiedGuard implementation
backend/src/auth/guards/verified.guard.ts
New CanActivate guard reads request.user; allows when no user is present, throws ForbiddenException('Email is not verified') when emailVerified is falsy, otherwise allows.
Apply VerifiedGuard across controllers
backend/src/appointments/appointments.controller.ts, backend/src/diagnosis/diagnosis.controller.ts, backend/src/doctors/doctors.controller.ts, backend/src/patients/patients.controller.ts, backend/src/scans/scans.controller.ts, backend/src/schedules/schedules.controller.ts, backend/src/users/users.controller.ts
Each controller imports VerifiedGuard and updates the class-level @UseGuards decorator from ApprovedGuard only to ApprovedGuard, VerifiedGuard.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Controller
  participant ApprovedGuard
  participant VerifiedGuard
  Client->>Controller: HTTP request
  Controller->>ApprovedGuard: canActivate(context)
  ApprovedGuard-->>Controller: allow/deny
  Controller->>VerifiedGuard: canActivate(context)
  VerifiedGuard->>VerifiedGuard: check request.user.emailVerified
  alt not verified
    VerifiedGuard-->>Controller: throw ForbiddenException
    Controller-->>Client: 403 Forbidden
  else verified or no user
    VerifiedGuard-->>Controller: allow
    Controller-->>Client: proceed to handler
  end
Loading

Possibly related PRs

  • MedAI-MU/medai#192: Both PRs modify the same controllers' class-level @UseGuards configuration by adding/enforcing ApprovedGuard, which this PR extends with VerifiedGuard.
  • MedAI-MU/medai#200: Both PRs modify controller-level guard configuration in scans.controller.ts and schedules.controller.ts.

Suggested reviewers: MustafaAtef, markrizkalla, Abdalrahman-gits, mena-rizkalla

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding a new VerifiedGuard to the backend.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/add-verified-guard

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.

mena-rizkalla
mena-rizkalla previously approved these changes Jul 7, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/src/auth/guards/verified.guard.ts`:
- Around line 11-21: The VerifiedGuard logic is checking a property that is not
present on the authenticated user payload. Update JwtStrategy.validate() and the
TokenUser/request.user shape so `emailVerified` is included, or change
`VerifiedGuard.canActivate()` to fetch the full User entity before checking
verification status; use the `VerifiedGuard` and `JwtStrategy.validate` symbols
to keep the auth payload and guard in sync.
🪄 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: f12623af-6920-4de5-a32f-ff574ee445b6

📥 Commits

Reviewing files that changed from the base of the PR and between 8d54db3 and 9630d47.

📒 Files selected for processing (8)
  • backend/src/appointments/appointments.controller.ts
  • backend/src/auth/guards/verified.guard.ts
  • backend/src/diagnosis/diagnosis.controller.ts
  • backend/src/doctors/doctors.controller.ts
  • backend/src/patients/patients.controller.ts
  • backend/src/scans/scans.controller.ts
  • backend/src/schedules/schedules.controller.ts
  • backend/src/users/users.controller.ts

Comment on lines +11 to +21
canActivate(context: ExecutionContext): boolean {
const request = context.switchToHttp().getRequest<RequestWithUser>();
const user = request.user;
if (!user) return true;

if (!user.emailVerified) {
throw new ForbiddenException('Email is not verified');
}

return true;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# What does the JWT/auth strategy assign to request.user?
fd -e ts . backend/src/auth --exec rg -nP 'validate\s*\(|request\.user\s*=|return\s' {} \; 2>/dev/null
echo "----- TokenUser interface -----"
fd 'token-user.interface.ts' backend/src --exec cat -n {}
echo "----- CurrentUser decorator -----"
fd 'current-user.decorator.ts' backend/src --exec cat -n {}
echo "----- emailVerified occurrences -----"
rg -nP '\bemailVerified\b' backend/src

Repository: MedAI-MU/medai

Length of output: 3667


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "----- auth tree -----"
fd -t f . backend/src/auth | sort

echo "----- jwt strategy outline -----"
ast-grep outline backend/src/auth/strategies/jwt.strategy.ts --view expanded

echo "----- verified guard usages -----"
rg -n "VerifiedGuard" backend/src

echo "----- auth strategy file -----"
cat -n backend/src/auth/strategies/jwt.strategy.ts

echo "----- relevant auth service snippets -----"
sed -n '1,220p' backend/src/auth/auth.service.ts

Repository: MedAI-MU/medai

Length of output: 11370


request.user never carries emailVerified
JwtStrategy.validate() returns a TokenUser with only id, email, role, and status, so VerifiedGuard will treat every authenticated request as unverified and throw ForbiddenException. Add emailVerified to the JWT user payload, or load the full User entity before this check.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/auth/guards/verified.guard.ts` around lines 11 - 21, The
VerifiedGuard logic is checking a property that is not present on the
authenticated user payload. Update JwtStrategy.validate() and the
TokenUser/request.user shape so `emailVerified` is included, or change
`VerifiedGuard.canActivate()` to fetch the full User entity before checking
verification status; use the `VerifiedGuard` and `JwtStrategy.validate` symbols
to keep the auth payload and guard in sync.

@ahmedxgouda
ahmedxgouda marked this pull request as draft July 7, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants