Skip to content

Conversation

@dhruvi-16-me
Copy link
Contributor

@dhruvi-16-me dhruvi-16-me commented Jan 4, 2026

Closes #114

📝 Description

This pull request adds basic role management for team members.
Team admins can now promote members to admin or demote admins back to member from the team members screen.
This makes team management more practical after a team is created, without changing the existing team join flow.

🔧 Changes Made

  1. Added promote/demote actions for admins in the team members list
  2. Added a confirmation dialog before changing a member’s role
  3. Updated Supabase service to confirm role updates using database response
  4. Updated RLS policies so only admins can manage roles within their team
  5. Prevented admins from modifying their own role

📷 Screenshots or Visual Changes (if applicable)

WhatsApp.Video.2026-01-04.at.7.02.13.PM.mp4

🤝 Collaboration

Collaborated with: @username (optional)

✅ 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

  • New Features
    • Added the ability for team admins to change team member roles between admin and member roles. Changes include proper security validation to ensure only admins can modify roles within their team.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 4, 2026

📝 Walkthrough

Walkthrough

Implements team member role management by adding a Dart service method with client-side validation and corresponding database-level Row-Level Security (RLS) policies to enforce role changes at the database layer. Includes two SQL helper functions to check admin status and current user role.

Changes

Cohort / File(s) Summary
Dart Service Method
lib/services/supabase_service.dart
Added changeTeamMemberRole() method with initialization and authentication verification, admin-privilege validation, same-team member verification, role validation (admin/member), and structured error handling with logging.
Database RLS Policies & Functions
sqls/02_user_auth_policies.sql
Added is_admin_in_same_team() and get_current_user_role() helper functions to bypass RLS for policy checks. Replaced existing user-update policy with more restrictive version preventing self-role changes. Added new admin policy enabling role updates within team, restricted to valid roles.

Sequence Diagram

sequenceDiagram
    actor Client
    participant Dart Service
    participant SupabaseAuth
    participant Database
    participant RLS Policies

    Client->>Dart Service: changeTeamMemberRole(userId, newRole)
    
    rect rgb(220, 240, 255)
    Note over Dart Service: Client-side Validation Phase
    Dart Service->>Dart Service: Check initialization
    Dart Service->>SupabaseAuth: Verify current user authenticated
    Dart Service->>Dart Service: Validate newRole ∈ {admin, member}
    end
    
    rect rgb(240, 220, 255)
    Note over Dart Service,Database: Server-side Authorization Phase
    Dart Service->>Database: Query target user & verify same team
    Database->>Dart Service: User data (team validation)
    Dart Service->>Database: UPDATE users SET role = newRole WHERE id = userId
    end
    
    rect rgb(255, 235, 220)
    Note over Database,RLS Policies: RLS Enforcement Phase
    Database->>RLS Policies: Trigger is_admin_in_same_team(target_team_id)
    RLS Policies->>RLS Policies: Check if current_user is admin in target team
    RLS Policies->>Database: Allow/Deny update
    Database->>Dart Service: Result
    end
    
    alt Success
        Dart Service->>Client: {success: true, message: "Role updated"}
    else Failure
        Dart Service->>Client: {success: false, error: "...", details: "..."}
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • SharkyBytes

Poem

🐰 A role swap here, a privilege there,
Admins now manage with data-layer care!
RLS guards the gate with functions so keen,
Team members rise—the fairest system seen! 🌟

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change—adding a role management feature for team members.
Linked Issues check ✅ Passed All coding requirements from #114 are met: backend methods for role changes, database-level RLS policies enforcing admin-only access, immediate effect, and role validation.
Out of Scope Changes check ✅ Passed All changes are directly scoped to role management: new service method, RLS helper functions, and policies. No extraneous modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings

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
Contributor

@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: 2

🧹 Nitpick comments (1)
lib/services/supabase_service.dart (1)

557-561: Consider refreshing the team members cache after role update.

The _teamMembersCache is not invalidated after a successful role change. This may cause the UI to display stale role information until the next manual refresh.

🔎 Proposed fix
       debugPrint('Successfully updated role to $newRole for user $userId');
+      
+      // Refresh team members cache to reflect the role change
+      if (_currentTeamId != null) {
+        _teamMembersCache = [];
+        await loadTeamMembers(_currentTeamId!);
+      }
+
       return {
         'success': true,
         'message': 'Role updated successfully',
       };
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 235a88b and 0bb8352.

📒 Files selected for processing (2)
  • lib/services/supabase_service.dart
  • sqls/02_user_auth_policies.sql
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-06T10:06:27.181Z
Learnt from: SharkyBytes
Repo: AOSSIE-Org/Ell-ena PR: 11
File: sqls/03_task_schema.sql:57-78
Timestamp: 2025-07-06T10:06:27.181Z
Learning: SharkyBytes prefers to keep separate database RLS policies during the development phase in the Ell-ena project for clarity and debugging purposes, even when they might be combined for better security. They handle complex authorization logic on the application side during development, with plans to optimize database-level security later when focusing on production readiness.

Applied to files:

  • sqls/02_user_auth_policies.sql
🔇 Additional comments (4)
lib/services/supabase_service.dart (1)

487-497: LGTM on the overall structure and error handling.

The method follows the established patterns in this service: initialization check, authentication verification, validation, database operation with result verification, and structured error responses with stack trace logging.

Also applies to: 499-505, 543-555, 562-569

sqls/02_user_auth_policies.sql (3)

29-50: LGTM on helper functions.

Both is_admin_in_same_team and get_current_user_role are correctly implemented with SECURITY DEFINER to bypass RLS during policy evaluation and STABLE for query optimization. The null check on team_id in is_admin_in_same_team is good defensive coding.


85-93: LGTM on user self-update policy.

The WITH CHECK clause correctly prevents users from changing their own role by validating role = get_current_user_role().


1-19: LGTM on cleanup statements.

The drop statements are comprehensive, including legacy policy names, which prevents conflicts during migration.

Comment on lines +507 to +540
// Verify current user is an admin and both users are in the same team
final currentUserProfile = await _client
.from('users')
.select('role, team_id')
.eq('id', currentUser.id)
.single();

if (currentUserProfile['role'] != 'admin') {
return {
'success': false,
'error': 'Only admins can change member roles',
};
}

final targetUserProfile = await _client
.from('users')
.select('team_id, role')
.eq('id', userId)
.single();

if (currentUserProfile['team_id'] != targetUserProfile['team_id']) {
return {
'success': false,
'error': 'Users must be in the same team',
};
}

// Validate new role
if (newRole != 'admin' && newRole != 'member') {
return {
'success': false,
'error': 'Invalid role: $newRole',
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Missing check to prevent admins from modifying their own role.

Per the PR objectives, admins should not be able to modify their own role. However, there's no validation that currentUser.id != userId. This would allow an admin to demote themselves, potentially leaving a team without admins.

🔎 Proposed fix
       if (currentUserProfile['role'] != 'admin') {
         return {
           'success': false,
           'error': 'Only admins can change member roles',
         };
       }
 
+      // Prevent admins from changing their own role
+      if (currentUser.id == userId) {
+        return {
+          'success': false,
+          'error': 'You cannot change your own role',
+        };
+      }
+
       final targetUserProfile = await _client
           .from('users')
           .select('team_id, role')
           .eq('id', userId)
           .single();
📝 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
// Verify current user is an admin and both users are in the same team
final currentUserProfile = await _client
.from('users')
.select('role, team_id')
.eq('id', currentUser.id)
.single();
if (currentUserProfile['role'] != 'admin') {
return {
'success': false,
'error': 'Only admins can change member roles',
};
}
final targetUserProfile = await _client
.from('users')
.select('team_id, role')
.eq('id', userId)
.single();
if (currentUserProfile['team_id'] != targetUserProfile['team_id']) {
return {
'success': false,
'error': 'Users must be in the same team',
};
}
// Validate new role
if (newRole != 'admin' && newRole != 'member') {
return {
'success': false,
'error': 'Invalid role: $newRole',
};
}
// Verify current user is an admin and both users are in the same team
final currentUserProfile = await _client
.from('users')
.select('role, team_id')
.eq('id', currentUser.id)
.single();
if (currentUserProfile['role'] != 'admin') {
return {
'success': false,
'error': 'Only admins can change member roles',
};
}
// Prevent admins from changing their own role
if (currentUser.id == userId) {
return {
'success': false,
'error': 'You cannot change your own role',
};
}
final targetUserProfile = await _client
.from('users')
.select('team_id, role')
.eq('id', userId)
.single();
if (currentUserProfile['team_id'] != targetUserProfile['team_id']) {
return {
'success': false,
'error': 'Users must be in the same team',
};
}
// Validate new role
if (newRole != 'admin' && newRole != 'member') {
return {
'success': false,
'error': 'Invalid role: $newRole',
};
}
🤖 Prompt for AI Agents
In lib/services/supabase_service.dart around lines 507 to 540, add a guard to
prevent an admin from modifying their own role: after fetching
currentUserProfile and targetUserProfile, check if currentUser.id == userId and
if so return {'success': false, 'error': 'Admins cannot modify their own role'};
keep this check before the team membership and newRole validation so
self-modification is blocked immediately.

Comment on lines +97 to +108
CREATE POLICY "Admins can manage roles within their team"
ON users
FOR UPDATE
USING (
-- Check: current user is admin in same team (using helper function to avoid recursion)
users.team_id IS NOT NULL
AND is_admin_in_same_team(users.team_id)
)
WITH CHECK (
-- Validate the new role value
role IN ('admin', 'member')
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Admin policy allows self-role modification - add exclusion for current user.

The "Admins can manage roles within their team" policy will match for an admin's own row since is_admin_in_same_team(users.team_id) returns true for the admin themselves. This bypasses the "Users can update their own profile (except role)" policy, allowing admins to change their own role.

🔎 Proposed fix - add exclusion for current user
 CREATE POLICY "Admins can manage roles within their team"
 ON users
 FOR UPDATE
 USING (
   -- Check: current user is admin in same team (using helper function to avoid recursion)
   users.team_id IS NOT NULL
   AND is_admin_in_same_team(users.team_id)
+  -- Prevent admins from changing their own role
+  AND users.id != auth.uid()
 )
 WITH CHECK (
   -- Validate the new role value
   role IN ('admin', 'member')
 );
🤖 Prompt for AI Agents
In sqls/02_user_auth_policies.sql around lines 97 to 108, the "Admins can manage
roles within their team" policy currently allows an admin to match their own row
(because is_admin_in_same_team returns true for them) which lets admins change
their own role; update the USING clause to exclude the current user by adding a
condition like AND users.id <> auth.uid() (or the equivalent current user id
expression in your environment) so the policy cannot apply to the admin's own
row while keeping the team-admin check intact.

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.

FEATURE REQUEST: Add role management for team members

1 participant