-
Notifications
You must be signed in to change notification settings - Fork 55
Added the role management feature #118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughImplements 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
Sequence DiagramsequenceDiagram
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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. Comment |
There was a problem hiding this 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
_teamMembersCacheis 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
📒 Files selected for processing (2)
lib/services/supabase_service.dartsqls/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_teamandget_current_user_roleare correctly implemented withSECURITY DEFINERto bypass RLS during policy evaluation andSTABLEfor query optimization. The null check onteam_idinis_admin_in_same_teamis good defensive coding.
85-93: LGTM on user self-update policy.The
WITH CHECKclause correctly prevents users from changing their own role by validatingrole = get_current_user_role().
1-19: LGTM on cleanup statements.The drop statements are comprehensive, including legacy policy names, which prevents conflicts during migration.
| // 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', | ||
| }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| // 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.
| 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') | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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
📷 Screenshots or Visual Changes (if applicable)
WhatsApp.Video.2026-01-04.at.7.02.13.PM.mp4
🤝 Collaboration
Collaborated with:
@username(optional)✅ Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.