Skip to content

Conversation

@AussieScorcher
Copy link
Member

Summary

Implements role-based access control (RBAC) for division management, restricting member management and airport operations based on user roles (lead_developer, nav_head, nav_member).

Changes Made

  • Add permission checks to AirportPointEditor to verify the user is a lead developer or division member before allowing access
  • Implement role detection (isLeadDev, isNavHead, isNavMember) in the DivisionManagement component
  • Restrict member management (add/remove) to lead developers and nav_head users only
  • Restrict airport request/delete actions to division members and lead developers
  • Add unauthorised user redirect to /account page
  • Display loading state while checking permissions
  • Apply disabled states to buttons when the user lacks required permissions

Additional Information

Permission hierarchy:

  • Lead Developer: Full access to all division management features
  • Nav Head: Can manage members and airports within their division
  • Nav Member: Can request and manage airports, but cannot manage members

Author Information

Discord Username: aussiescorcher
VATSIM CID: 1658308


Checklist:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

@greptile-apps
Copy link

greptile-apps bot commented Jan 29, 2026

Greptile Overview

Greptile Summary

This PR implements role-based access control (RBAC) for division management with a three-tier permission hierarchy (lead_developer, nav_head, nav_member). The implementation adds proper permission checks both at the UI level (button disabling) and at the action level (early returns in handlers).

Key Changes:

  • Permission detection in both DivisionManagement and AirportPointEditor components using API calls to check staff status and division membership
  • Role-based restrictions: member management limited to lead developers and nav heads, airport operations available to all division members
  • Proper loading states before permission checks complete
  • Unauthorized user redirects to /account page
  • UI elements appropriately disabled when users lack permissions

Issues Found:

  • Invalid Tailwind CSS class h-145 in map preview (line 1508) - should be h-[580px]

The RBAC implementation is thorough and follows good security practices by checking permissions at multiple levels. The loading states prevent premature redirects, and the permission hierarchy is clearly implemented with appropriate restrictions for each role.

Confidence Score: 4/5

  • safe to merge after fixing the invalid Tailwind class
  • the implementation is solid with proper permission checks at both UI and handler levels, loading states prevent race conditions, and the permission hierarchy is clearly defined; only one minor syntax error needs correction
  • src/components/divisions/DivisionManagement.jsx line 1508 needs the Tailwind class fixed

Important Files Changed

Filename Overview
src/components/divisions/AirportPointEditor.jsx added permission checks for lead developers and division members with proper loading states and redirects
src/components/divisions/DivisionManagement.jsx implemented comprehensive RBAC with role detection, permission checks, and UI restrictions; contains one syntax error in map preview height class

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link

greptile-apps bot commented Jan 29, 2026

Additional Comments (3)

src/components/divisions/DivisionManagement.jsx
Missing permission check at handler start. While UI buttons are disabled, this function can still be called via dev tools or state manipulation.

  const handleAddMember = async (e) => {
    e.preventDefault();
    if (!canManageMembers) return;
    setAddingMember(true);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/divisions/DivisionManagement.jsx
Line: 839:842

Comment:
Missing permission check at handler start. While UI buttons are disabled, this function can still be called via dev tools or state manipulation.

```suggestion
  const handleAddMember = async (e) => {
    e.preventDefault();
    if (!canManageMembers) return;
    setAddingMember(true);
```

How can I resolve this? If you propose a fix, please make it concise.

src/components/divisions/DivisionManagement.jsx
Missing permission check. Should validate canManageMembers and prevent nav_head removal by non-lead_developers.

  const handleRemoveMember = async (memberId) => {
    if (!canManageMembers) return;
    if (!memberToRemove) return;
    const isSelf = String(currentUserId) === String(memberId);
    if (isSelf) return;
    if (!isLeadDev && memberToRemove.role === 'nav_head') return;
    setRemovingMember(true);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/divisions/DivisionManagement.jsx
Line: 884:885

Comment:
Missing permission check. Should validate `canManageMembers` and prevent nav_head removal by non-lead_developers.

```suggestion
  const handleRemoveMember = async (memberId) => {
    if (!canManageMembers) return;
    if (!memberToRemove) return;
    const isSelf = String(currentUserId) === String(memberId);
    if (isSelf) return;
    if (!isLeadDev && memberToRemove.role === 'nav_head') return;
    setRemovingMember(true);
```

How can I resolve this? If you propose a fix, please make it concise.

src/components/divisions/DivisionManagement.jsx
Missing permission check. Should validate user is a division member or lead developer.

  const handleDeleteAirport = async (airportId) => {
    if (!isDivisionMember && !isLeadDev) return;
    setDeletingAirport(true);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/divisions/DivisionManagement.jsx
Line: 948:949

Comment:
Missing permission check. Should validate user is a division member or lead developer.

```suggestion
  const handleDeleteAirport = async (airportId) => {
    if (!isDivisionMember && !isLeadDev) return;
    setDeletingAirport(true);
```

How can I resolve this? If you propose a fix, please make it concise.

@stopbars stopbars deleted a comment from greptile-apps bot Jan 29, 2026
@AussieScorcher
Copy link
Member Author

@greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@stopbars stopbars deleted a comment from greptile-apps bot Jan 29, 2026
Copy link
Contributor

@llamavert llamavert left a comment

Choose a reason for hiding this comment

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

LGTM

@AussieScorcher AussieScorcher merged commit 11e4e6a into main Jan 29, 2026
6 checks passed
@AussieScorcher AussieScorcher deleted the feature/division-rbac-permissions branch January 29, 2026 10:58
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.

3 participants