-
Notifications
You must be signed in to change notification settings - Fork 325
44 hmute #246
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
Open
Hmute
wants to merge
460
commits into
AccountGo:main
Choose a base branch
from
medhatelmasry:44Hmute
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
44 hmute #246
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… to EditTax page when clicking edit button.
…stration Service.
… and Dto and Service.
…d child account functionality
…es do.Now everything is plug and play again.
implement parent account dropdown
93redesign-nav
…_the_page_is_not_found 89 when viewing a customer the page is not found
18 Chart of Accounts must be editable
…e-API Added rate limiting to the API. After requesting 5 times, there will …
…avecompany added username to savecompany
…abase seeding - Add user menu dropdown with logout link to layout files - Create SignedOut.cshtml view for post-logout confirmation - Fix MigrationService to migrate both ApiDbContext and ApplicationIdentityDbContext - Add EnableRetryOnFailure to DbContext configurations for transient error handling - Fix seeding order: Create SecurityRoles before admin user to avoid FK constraint violations - Add admin user seeding with detailed logging ([email protected] / P@ssword1) - Add migration completion check before seeding to prevent race conditions - Improve API error handling in AccountController SignIn endpoint - Add error logging in MVC AccountController for better debugging
- Created EF Core migrations for Identity and API database contexts - Fixed authentication flow with JWT token generation and storage - Implemented role-based access control with SystemAdministrators and GeneralUsers roles - Added JWT token injection in all MVC API calls (BaseController and GoodController) - Fixed dual security system synchronization (Identity + Application SecurityRoles) - Updated DatabaseSeeder to assign users to roles in both systems - Secured API endpoints with role-based authorization attributes - Created AccessDenied view and configured AccessDeniedPath - Added role claims to JWT tokens via UserManager.GetRolesAsync() - Fixed TaxController to use authenticated GetAsync method - Added test users: [email protected] and [email protected] (both P@ssword1) - Created RBAC documentation in docs/RBAC-Implementation.md - Fixed NullReferenceException issues in AccountController and views - Enlarged login form email input box
- Hide Organization Settings and System Administration menus for general users - Hide tax management buttons (New, Edit, Delete) for general users - General users now have read-only access to tax data - Fix GetUser endpoint authorization with AllowAnonymous + AnyUser pattern - Fix DatabaseSeeder to handle existing users and ensure role assignment in both systems - Modified SeedDefaultAdminUser and SeedDefaultGeneralUser to repair existing users - Ensures users are assigned to roles in both Identity and application domain - UI now matches API authorization restrictions for better security
- Fixed Groups.cshtml: Added null checks and user-friendly messages when no groups available - Fixed Roles.cshtml: Added null checks and count display for roles - Fixed TrialBalance.cshtml: Added null checks with permission warning message - Enhanced SecurityService.AddRole(): Automatically sets DisplayName from role Name - Maps 'SystemAdministrators' -> 'System Administrators' - Maps 'GeneralUsers' -> 'General Users' - Uses regex for other role names to add spacing - Updated DatabaseSeeder: Repair existing roles lacking DisplayName - All views now gracefully handle null Models from failed API authorization calls
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement Role-Based Access Control (RBAC) with JWT Authentication
Overview
This PR implements a comprehensive authentication and role-based access control (RBAC) system for the GoodBooks application, replacing the previous basic authentication with a dual security architecture using ASP.NET Core Identity and a custom application-level permission system.
Key Features
Authentication & Authorization:
Dual Security System: Integrated ASP.NET Core Identity (authentication) with custom application security (authorization)
JWT Token Authentication: API endpoints secured with Bearer token authentication
Cookie Authentication: MVC web application uses cookie-based sessions
Role-Based Access: Two roles implemented - SystemAdministrators and GeneralUsers
User Management:
Seeded Default Users:
Admin: [email protected] / P@ssword1 (SystemAdministrators role)
General User: [email protected] / P@ssword1 (GeneralUsers role)
Automatic Role Assignment: Users assigned to both Identity and application domain roles during seeding
Display Name Support: Security roles now include user-friendly display names (e.g., "System Administrators")
UI Improvements:
Role-Based Menu Visibility: Organization Settings and System Administration menus only visible to admins
Conditional Action Buttons: Create/Edit/Delete buttons hidden for general users on restricted pages (Taxes, etc.)
Null Model Handling: Admin pages (Roles, Groups, Trial Balance) now gracefully handle unauthorized access with user-friendly messages
User Role Display: Users page correctly shows role names instead of "null"
Technical Changes
Database Seeding (DatabaseSeeder.cs)
Create roles in both Identity and application domain
Seed admin and general users with proper role assignments
Repair existing roles missing DisplayName values
Security Service (SecurityService.cs)
AddRole() method enhanced to automatically generate display names from role names
Supports both creating new roles and updating existing ones
API Controllers
AdministrationController: Class-level [Authorize(Roles = "SystemAdministrators")]
GetUser endpoint: [AllowAnonymous] to permit any authenticated user to retrieve their info
Views
_Layout.cshtml: Role-based menu visibility
Taxes.cshtml: Conditional action buttons
Roles.cshtml, Groups.cshtml, TrialBalance.cshtml: Null model handling with informative messages
Testing
Admin user can log in and access all features
General user can log in with restricted access
Menu items properly hidden based on roles
API endpoints enforce authorization correctly
No NullReferenceException when unauthorized users access admin pages
Security Considerations
Passwords stored using Identity's secure hashing
JWT tokens include role claims for API authorization
Separation of authentication (Identity) and authorization (application domain)
All admin endpoints properly protected
Migration Notes
For existing deployments:
Database will auto-seed roles and default users on first run
Existing roles will have DisplayName populated automatically
No manual intervention required
Closes: N/A (New feature implementation)
Related Issues: Authentication, Authorization, RBAC
Future Improvements
High Priority
Password Reset: Implement "Forgot Password" with email verification
Account Lockout: Auto-lock accounts after failed login attempts
Audit Logging: Track all auth events, role changes, and sensitive operations
Fine-Grained Permissions: Move from role-based to claims-based for granular control
Email Verification: Require email confirmation for new accounts
Medium Priority
Multi-Factor Authentication: Add 2FA support (TOTP/SMS)
Token Refresh: Implement refresh tokens to extend sessions
Dynamic Role Management: UI for creating roles and assigning permissions without code
Custom 403 Page: Dedicated unauthorized access page instead of null data messages
Permission Caching: Cache user permissions to reduce DB queries
Lower Priority
API Key Auth: Support for service-to-service authentication
SSO Integration: Connect with Azure AD, Google, etc.
User Activity Tracking: Log last login, IP addresses, actions
Role Templates: Predefined roles (Accountant, Viewer, Manager)
Bulk User Operations: CSV import/export, bulk role assignments
Testing & Monitoring
Unit & Integration Tests: Comprehensive auth flow testing
Security Dashboard: Monitor failed logins and suspicious activity
API Documentation: Document role requirements in Swagger
Immediate Next Steps: Password reset, account lockout, and audit logging would provide the most value with minimal effort.