Added the role management feature#118
Conversation
📝 WalkthroughWalkthroughAdds team role management: a new Dart service method to request role changes and new/updated database RLS policies and helper functions to enforce admin-only role changes within the same team. Changes
Sequence DiagramsequenceDiagram
actor Client
participant "Dart Service" as Dart
participant "Supabase Auth" as Auth
participant Database
participant "RLS Policies" as RLS
Client->>Dart: changeTeamMemberRole(userId, newRole)
rect rgba(220,240,255,0.5)
Dart->>Dart: check initialization
Dart->>Auth: verify current user authenticated
Dart->>Dart: validate newRole ∈ {admin, member}
end
rect rgba(240,220,255,0.5)
Dart->>Database: SELECT target user (verify same team)
Database->>Dart: return user + team info
Dart->>Database: UPDATE users SET role = newRole WHERE id = userId
end
rect rgba(255,235,220,0.5)
Database->>RLS: evaluate update (calls helpers)
RLS->>RLS: is_admin_in_same_team(...) and get_current_user_role()
RLS-->>Database: allow/deny
Database->>Dart: update result / error
end
alt success
Dart->>Client: {success: true, message}
else failure
Dart->>Client: {success: false, error, details}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
|
Hey @dhruvi-16-me, I was going through the CodeRabbit comments on your PR, and they actually pointed out a real edge case. Right now, an admin can still modify their own role because:
Probably the simplest fix is:
Just sharing since it’s one of those edge-case things. |
|
@g-k-s-03 Thanks for pointing it out. I should have added that edge case earlier. I will work on it. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sqls/02_user_auth_policies.sql (2)
64-66:⚠️ Potential issue | 🟡 MinorPolicy name doesn't match its logic.
The policy is named "Only admins can update their team" but the
USINGclause checkscreated_by = auth.uid(), which restricts updates to the team creator rather than admins. If the team creator is demoted to member, they would still be able to update the team.Consider either:
- Renaming to "Only team creators can update their team", or
- Changing the condition to use
is_admin_in_same_team(id)if admin-based access is intended.
119-120:⚠️ Potential issue | 🟠 MajorGranting ALL permissions to
anonrole is overly permissive.The
anonrole represents unauthenticated users in Supabase. While RLS policies provide row-level protection, grantingALL(SELECT, INSERT, UPDATE, DELETE) to anonymous users violates the principle of least privilege and increases attack surface if RLS policies have bugs or are temporarily disabled.Consider restricting
anonpermissions:🛡️ Proposed fix - restrict anonymous permissions
GRANT USAGE ON SCHEMA public TO anon, authenticated; GRANT SELECT ON user_teams TO anon, authenticated; -GRANT ALL ON teams TO anon, authenticated; -GRANT ALL ON users TO anon, authenticated; +GRANT SELECT ON teams TO anon; +GRANT ALL ON teams TO authenticated; +GRANT SELECT ON users TO anon; +GRANT ALL ON users TO authenticated;If anonymous access is intentionally required for specific operations (e.g., signup flow), grant only the minimum necessary permissions explicitly.
2 participants
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
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.