fix: add ownership check to changeAppointmentStatus to prevent unauthorized session modification

[Varadraj75]

Closes #177

📝 Description

changeAppointmentStatus was updating a session's status by id alone
with no check that the session belonged to the authenticated therapist.
Any authenticated therapist could accept, decline, or modify any other
therapist's session by supplying its UUID. RLS is also disabled on the
session table, so there was no database-level safety net either.

🔧 Changes Made

• therapist/lib/repository/supabase_therapist_repository.dart: Added
  .eq('therapist_id', _supabaseClient.auth.currentUser!.id) to the
  update query so only the session owner can modify its status.
• Added .select() and a response.isEmpty check to return a 404
  when the session doesn't exist or isn't owned by the current therapist,
  preventing a false success response.

📷 Screenshots or Visual Changes (if applicable)

N/A — authorization fix, no visual changes.

🤝 Collaboration

Collaborated with: N/A

✅ Checklist

• I have read the contributing guidelines.
• I have added tests that prove my fix is effective or that my feature works.
• I have added necessary documentation (if applicable).
• Any dependent changes have been merged and published in downstream modules.

Summary by CodeRabbit

• Bug Fixes
  • Improved authorization checks for appointment status updates so only the assigned therapist can modify a session; returns clear errors when a session is not found or access is denied.
• Documentation
  • Clarified return semantics for appointment status operations to make success and error responses more explicit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4e41f7f8-ef0f-45a8-ae68-eadd5c9aa4be

📥 Commits

Reviewing files that changed from the base of the PR and between f436144 and 9bf4a8b.

📒 Files selected for processing (1)

• therapist/lib/core/repository/therapist/therapist_repository.dart

✅ Files skipped from review due to trivial changes (1)

• therapist/lib/core/repository/therapist/therapist_repository.dart

---

📝 Walkthrough

Walkthrough

changeAppointmentStatus now verifies session ownership by selecting the session with both id and therapist_id; returns 404 if not found/unauthorized and proceeds to update status when the authenticated therapist owns the session.

Changes

Cohort / File(s)	Summary
Therapist repository (ownership & responses) therapist/lib/repository/supabase_therapist_repository.dart, therapist/lib/core/repository/therapist/therapist_repository.dart	Added a session lookup that filters by id and therapist_id before updating status; returns 404 when session missing or not owned by the authenticated therapist. Added in-code documentation describing 200/404/400 return semantics.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Therapist as Therapist (client)
    participant Repo as TherapistRepository
    participant DB as Supabase DB

    Therapist->>Repo: changeAppointmentStatus(appointmentId, status)
    Repo->>DB: SELECT * FROM session WHERE id = appointmentId AND therapist_id = currentUserId
    alt session found and owned
        DB-->>Repo: session row
        Repo->>DB: UPDATE session SET status = ... WHERE id = appointmentId
        DB-->>Repo: update success
        Repo-->>Therapist: 200 ActionResultSuccess
    else not found or not owned
        DB-->>Repo: no rows
        Repo-->>Therapist: 404 ActionResultFailure
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

• jddeep

Poem

🐰 I hopped in, checked each tiny track,
I matched the ID, then closed the back,
Now sessions rest where owners dwell,
No stray changes—all is well. 🥕

🚥 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 accurately describes the main change: adding an ownership check to changeAppointmentStatus to prevent unauthorized session modification.
Linked Issues check	✅ Passed	The pull request fully implements the required fix from issue #177: adds therapist_id equality check to prevent unauthorized access and handles non-owned/missing sessions with 404 response.
Out of Scope Changes check	✅ Passed	All changes are directly within scope: the ownership check implementation in the repository, documentation of new 404 behavior, and no unrelated modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

therapist/lib/repository/supabase_therapist_repository.dart (1)

62-64: Document the new 404 outcome in the repository contract.

This method now returns 404 for “not found or not authorized,” but the interface docs still describe failures as 400 on exception only. Please update therapist/lib/core/repository/therapist/therapist_repository.dart docs to match runtime behavior.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@therapist/lib/repository/supabase_therapist_repository.dart` around lines 62
- 64, Update the repository contract docs to reflect the new 404 outcome: in the
TherapistRepository interface's method that fetches a session (e.g.,
TherapistRepository.getSession / fetchSession) change the doc comment that
currently states failures are 400-on-exception to include that the method may
return an ActionResultFailure with statusCode 404 when the session is not found
or the caller is not authorized, and keep the existing 400/exception explanation
for other failure cases; reference ActionResultFailure and the specific method
name in the comment so callers know when to expect 404.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@therapist/lib/repository/supabase_therapist_repository.dart`:
- Around line 57-61: The repository currently relies only on a client-side
ownership check in the update flow (the call in the function that uses
_supabaseClient.from('session').update(...).eq('therapist_id',
_supabaseClient.auth.currentUser!.id)), which is bypassable while Row Level
Security (RLS) is disabled; add a DB migration to enable RLS on the session
table and create a policy (e.g., session_update_own_rows) that restricts FOR
UPDATE to authenticated users and enforces therapist_id = auth.uid() in both
USING and WITH CHECK clauses so only the owning therapist can update their rows;
keep the existing client-side guard in the repository but treat it as UX
validation, not a security boundary.

---

Nitpick comments:
In `@therapist/lib/repository/supabase_therapist_repository.dart`:
- Around line 62-64: Update the repository contract docs to reflect the new 404
outcome: in the TherapistRepository interface's method that fetches a session
(e.g., TherapistRepository.getSession / fetchSession) change the doc comment
that currently states failures are 400-on-exception to include that the method
may return an ActionResultFailure with statusCode 404 when the session is not
found or the caller is not authorized, and keep the existing 400/exception
explanation for other failure cases; reference ActionResultFailure and the
specific method name in the comment so callers know when to expect 404.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 90d75cc2-9e03-4f46-8763-24e5d4ab4ca9

📥 Commits

Reviewing files that changed from the base of the PR and between 421e7c2 and f436144.

📒 Files selected for processing (1)

• therapist/lib/repository/supabase_therapist_repository.dart

[Copilot]

Pull request overview

This PR addresses Issue #177 by preventing therapists from changing the status of sessions they don’t own when calling changeAppointmentStatus, scoping the update to the authenticated therapist.

Changes:

• Adds therapist_id = currentUser.id filtering to the session status update.
• Uses select() + empty-result handling to return a 404 when no owned session is updated.

Comments suppressed due to low confidence (1)

therapist/lib/repository/supabase_therapist_repository.dart:63

• .select() will return the full updated session row, but the code only uses it to check emptiness. To reduce bandwidth and avoid unintentionally returning extra columns, select only a minimal column set (e.g., just the id) when you only need to know whether a row was updated.

      final response = await _supabaseClient.from('session')
      .update({'status': status})
      .eq('id', appointmentId)
      .eq('therapist_id', _supabaseClient.auth.currentUser!.id)
      .select();
      if (response.isEmpty) {
        return ActionResultFailure(errorMessage: 'Session not found or not authorized', statusCode: 404);

---

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

You can also share your feedback on Copilot code review. Take the survey.