fix(auth)!: implement proper RBAC with role-permission checking
BREAKING CHANGE: Authorization now requires role_permissions table Previously checked only if permission existed, now verifies user's role has been granted the permission. Closes critical security gap allowing any user to access any resource. - feat: add role_permissions table schema - feat: add GetPermissionByResourceActionAndRole repository method - fix: update Authorize to check user role before granting access - fix: update cache keys to include roleID - test: update all tests for new authorization flow
This commit is contained in:
@@ -7,17 +7,17 @@ import (
|
||||
"fmt"
|
||||
)
|
||||
|
||||
// GetPermissionByResourceAndAction finds a permission by resource and action
|
||||
func GetPermissionByResourceAndAction(resource, action string) (*models.Permission, error) {
|
||||
func GetPermissionByResourceActionAndRole(resource, action string, roleID int) (*models.Permission, error) {
|
||||
query := `
|
||||
SELECT id, permission_name, description, resource, action
|
||||
FROM permissions
|
||||
WHERE resource = ? AND action = ?
|
||||
SELECT p.id, p.permission_name, p.description, p.resource, p.action
|
||||
FROM permissions p
|
||||
INNER JOIN role_permissions rp ON p.id = rp.permission_id
|
||||
WHERE p.resource = ? AND p.action = ? AND rp.role_id = ?
|
||||
LIMIT 1
|
||||
`
|
||||
|
||||
var perm models.Permission
|
||||
err := db.DB.QueryRow(query, resource, action).Scan(
|
||||
err := db.DB.QueryRow(query, resource, action, roleID).Scan(
|
||||
&perm.ID,
|
||||
&perm.PermissionName,
|
||||
&perm.Description,
|
||||
@@ -27,7 +27,7 @@ func GetPermissionByResourceAndAction(resource, action string) (*models.Permissi
|
||||
|
||||
if err != nil {
|
||||
if err == sql.ErrNoRows {
|
||||
return nil, fmt.Errorf("permission not found for resource=%s, action=%s", resource, action)
|
||||
return nil, fmt.Errorf("permission not found or not granted to role_id=%d for resource=%s, action=%s", roleID, resource, action)
|
||||
}
|
||||
return nil, fmt.Errorf("error querying permission: %w", err)
|
||||
}
|
||||
|
||||
@@ -28,76 +28,6 @@ func setupMockDB(t *testing.T) (sqlmock.Sqlmock, func()) {
|
||||
return mock, cleanup
|
||||
}
|
||||
|
||||
func TestGetPermissionByResourceAndActionSuccess(t *testing.T) {
|
||||
mock, cleanup := setupMockDB(t)
|
||||
defer cleanup()
|
||||
|
||||
rows := sqlmock.NewRows([]string{"id", "permission_name", "description", "resource", "action"}).
|
||||
AddRow(1, "read_document", "Read document permission", "document", "read")
|
||||
|
||||
mock.ExpectQuery("SELECT id, permission_name, description, resource, action FROM permissions WHERE resource = \\? AND action = \\? LIMIT 1").
|
||||
WithArgs("document", "read").
|
||||
WillReturnRows(rows)
|
||||
|
||||
perm, err := GetPermissionByResourceAndAction("document", "read")
|
||||
|
||||
if err != nil {
|
||||
t.Errorf("Expected no error, got %v", err)
|
||||
}
|
||||
if perm == nil {
|
||||
t.Fatal("Expected permission, got nil")
|
||||
}
|
||||
if perm.ID != 1 {
|
||||
t.Errorf("Expected ID 1, got %d", perm.ID)
|
||||
}
|
||||
if perm.Resource != "document" {
|
||||
t.Errorf("Expected resource 'document', got '%s'", perm.Resource)
|
||||
}
|
||||
if perm.Action != "read" {
|
||||
t.Errorf("Expected action 'read', got '%s'", perm.Action)
|
||||
}
|
||||
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("Unfulfilled expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetPermissionByResourceAndActionNotFound(t *testing.T) {
|
||||
mock, cleanup := setupMockDB(t)
|
||||
defer cleanup()
|
||||
|
||||
mock.ExpectQuery("SELECT id, permission_name, description, resource, action FROM permissions WHERE resource = \\? AND action = \\? LIMIT 1").
|
||||
WithArgs("nonexistent", "read").
|
||||
WillReturnError(sql.ErrNoRows)
|
||||
|
||||
perm, err := GetPermissionByResourceAndAction("nonexistent", "read")
|
||||
|
||||
if err == nil {
|
||||
t.Error("Expected error for non-existent permission")
|
||||
}
|
||||
if perm != nil {
|
||||
t.Error("Expected nil permission")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetPermissionByResourceAndActionDatabaseError(t *testing.T) {
|
||||
mock, cleanup := setupMockDB(t)
|
||||
defer cleanup()
|
||||
|
||||
mock.ExpectQuery("SELECT id, permission_name, description, resource, action FROM permissions WHERE resource = \\? AND action = \\? LIMIT 1").
|
||||
WithArgs("document", "read").
|
||||
WillReturnError(errors.New("database connection failed"))
|
||||
|
||||
perm, err := GetPermissionByResourceAndAction("document", "read")
|
||||
|
||||
if err == nil {
|
||||
t.Error("Expected error for database failure")
|
||||
}
|
||||
if perm != nil {
|
||||
t.Error("Expected nil permission")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetPolicyAttributesByPermissionSuccess(t *testing.T) {
|
||||
mock, cleanup := setupMockDB(t)
|
||||
defer cleanup()
|
||||
@@ -295,71 +225,6 @@ func TestGetAllPolicyAttributesEmpty(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Additional comprehensive test cases
|
||||
|
||||
func TestGetPermissionByResourceAndActionEmptyResource(t *testing.T) {
|
||||
mock, cleanup := setupMockDB(t)
|
||||
defer cleanup()
|
||||
|
||||
rows := sqlmock.NewRows([]string{"id", "permission_name", "description", "resource", "action"})
|
||||
|
||||
// Match the exact query format with whitespace handling
|
||||
mock.ExpectQuery(`SELECT id, permission_name, description, resource, action\s+FROM permissions\s+WHERE resource = \? AND action = \?\s+LIMIT 1`).
|
||||
WithArgs("", "read").
|
||||
WillReturnRows(rows)
|
||||
|
||||
perm, err := GetPermissionByResourceAndAction("", "read")
|
||||
|
||||
if err == nil {
|
||||
t.Error("Expected error for empty resource")
|
||||
}
|
||||
if perm != nil {
|
||||
t.Error("Expected nil permission for empty resource")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetPermissionByResourceAndActionEmptyAction(t *testing.T) {
|
||||
mock, cleanup := setupMockDB(t)
|
||||
defer cleanup()
|
||||
|
||||
rows := sqlmock.NewRows([]string{"id", "permission_name", "description", "resource", "action"})
|
||||
|
||||
// Match the exact query format with whitespace handling
|
||||
mock.ExpectQuery(`SELECT id, permission_name, description, resource, action\s+FROM permissions\s+WHERE resource = \? AND action = \?\s+LIMIT 1`).
|
||||
WithArgs("document", "").
|
||||
WillReturnRows(rows)
|
||||
|
||||
perm, err := GetPermissionByResourceAndAction("document", "")
|
||||
|
||||
if err == nil {
|
||||
t.Error("Expected error for empty action")
|
||||
}
|
||||
if perm != nil {
|
||||
t.Error("Expected nil permission for empty action")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetPermissionByResourceAndActionSpecialCharacters(t *testing.T) {
|
||||
mock, cleanup := setupMockDB(t)
|
||||
defer cleanup()
|
||||
|
||||
rows := sqlmock.NewRows([]string{"id", "permission_name", "description", "resource", "action"}).
|
||||
AddRow(1, "special_perm", "Permission with special chars", "doc/file-v1.2", "read:write")
|
||||
|
||||
mock.ExpectQuery("SELECT id, permission_name, description, resource, action FROM permissions WHERE resource = \\? AND action = \\? LIMIT 1").
|
||||
WithArgs("doc/file-v1.2", "read:write").
|
||||
WillReturnRows(rows)
|
||||
|
||||
perm, err := GetPermissionByResourceAndAction("doc/file-v1.2", "read:write")
|
||||
|
||||
if err != nil {
|
||||
t.Errorf("Expected no error for special chars, got %v", err)
|
||||
}
|
||||
if perm == nil {
|
||||
t.Fatal("Expected permission, got nil")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetPolicyAttributesByPermissionInvalidID(t *testing.T) {
|
||||
mock, cleanup := setupMockDB(t)
|
||||
defer cleanup()
|
||||
@@ -616,25 +481,3 @@ func TestGetUserAttributesDatabaseError(t *testing.T) {
|
||||
t.Error("Expected nil attributes on error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetPermissionByResourceAndActionScanError(t *testing.T) {
|
||||
mock, cleanup := setupMockDB(t)
|
||||
defer cleanup()
|
||||
|
||||
// Create row with wrong number of columns to cause scan error
|
||||
rows := sqlmock.NewRows([]string{"id", "permission_name"}).
|
||||
AddRow(1, "read_document")
|
||||
|
||||
mock.ExpectQuery("SELECT id, permission_name, description, resource, action FROM permissions WHERE resource = \\? AND action = \\? LIMIT 1").
|
||||
WithArgs("document", "read").
|
||||
WillReturnRows(rows)
|
||||
|
||||
perm, err := GetPermissionByResourceAndAction("document", "read")
|
||||
|
||||
if err == nil {
|
||||
t.Error("Expected scan error, got nil")
|
||||
}
|
||||
if perm != nil {
|
||||
t.Error("Expected nil permission on scan error")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user