Role management implementation #555

senthalan · 2025-10-20T15:24:18Z

Part of #510

This pull request introduces a new Role Management feature to the backend, including database schema changes, service initialization, error handling, data models, and supporting configuration for mocking and testing. The main focus is to enable creation, management, and assignment of roles, along with permissions and assignments to users and groups, with full API and database support.

Key changes include:

Database Schema & Queries

New role management tables and indexes: Added ROLE, ROLE_PERMISSION, and ROLE_ASSIGNMENT tables to both Postgres and SQLite schemas, including supporting indexes for efficient authorization queries. These tables support storing roles, their permissions, and assignments to users/groups. [1] [2]
Role management SQL queries: Introduced a comprehensive set of queries for CRUD operations on roles, permissions, and assignments, as well as checks for uniqueness and pagination support in storeconstants.go.

Backend Service & API

Role service initialization and route registration: Implemented the Initialize function for the role service, registering HTTP routes for role CRUD operations and assignment management, with appropriate CORS handling.
Service wiring: Integrated the new role service into the main service manager, ensuring it is initialized with dependencies on user, group, and organization unit services. [1] [2]

Data Models

Role and assignment models: Defined the core data structures for roles, assignments, and related API requests/responses, supporting role details, permissions, assignments, and pagination.

Error Handling

Role management error constants: Added detailed error types and codes for client and server errors related to role operations, including validation, not found, and conflict scenarios.

Testing & Mocking

Mockery configuration updates: Updated mockery configuration files to support mocking of new role, group, and organization unit interfaces for both private and public test scenarios. [1] [2]

These changes collectively enable robust role-based access control capabilities in the backend system.

codecov · 2025-10-20T16:54:21Z

Codecov Report

❌ Patch coverage is 82.90514% with 173 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.68%. Comparing base (93b7171) to head (edc921e).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
backend/internal/role/store.go	70.63%	41 Missing and 38 partials ⚠️
backend/internal/role/init.go	0.00%	52 Missing ⚠️
backend/internal/role/handler.go	89.53%	19 Missing and 10 partials ⚠️
backend/internal/role/service.go	97.68%	6 Missing and 2 partials ⚠️
backend/internal/group/service.go	0.00%	3 Missing ⚠️
backend/cmd/server/servicemanager.go	0.00%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (93b7171) and HEAD (edc921e). Click for more details.

HEAD has 4 uploads less than BASE

Flag BASE (93b7171) HEAD (edc921e)

backend-integration-postgres 1 0

backend-combined-postgres 1 0

backend-integration-sqlite 1 0

backend-combined-sqlite 1 0

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #555       +/-   ##
===========================================
- Coverage   65.62%   31.68%   -33.95%     
===========================================
  Files         200      205        +5     
  Lines       17780    18788     +1008     
  Branches      255      255               
===========================================
- Hits        11669     5953     -5716     
- Misses       4772    12550     +7778     
+ Partials     1339      285     -1054

Flag	Coverage Δ
backend-combined-postgres	`?`
backend-combined-sqlite	`?`
backend-integration-postgres	`?`
backend-integration-sqlite	`?`
backend-unit	`31.68% <82.90%> (+3.04%)`	⬆️
frontend-apps-develop-unit	`31.68% <82.90%> (+3.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull Request Overview

This PR implements role management functionality, enabling permission-based access control through roles that can be assigned to users and groups. The implementation includes complete CRUD operations, assignment management, pagination support, and comprehensive test coverage.

Key Changes

New role management API with CRUD endpoints and role assignment capabilities
Database schema additions for roles, permissions, and assignments
Complete service layer with validation and error handling
Integration tests validating all API operations

Reviewed Changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
backend/internal/role/service.go	Implements role service with CRUD operations and assignment management
backend/internal/role/store.go	Database layer for role operations with transaction handling
backend/internal/role/model.go	Data models for roles, assignments, and API requests/responses
backend/internal/role/errorconstants.go	Error constants for role-related operations
backend/internal/role/storeconstants.go	Database query definitions for role operations
backend/internal/role/service_test.go	Comprehensive unit tests for service layer
backend/internal/role/init.go	Route registration and service initialization
backend/dbscripts/thunderdb/sqlite.sql	SQLite schema additions for role tables
tests/integration/role/roleapi_test.go	Integration tests for role API endpoints
tests/integration/role/model.go	Test models for integration testing
tests/integration/testutils/models.go	Added Group model for test utilities
tests/integration/testutils/apiutils.go	Helper functions for group operations in tests
backend/.mockery.public.yml	Mock configuration for group and OU services
docs/apis/role.yaml	OpenAPI specification for role management API

Copilot

Pull Request Overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

backend/internal/system/error/serviceerror/error.go

Copilot

Pull Request Overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

backend/internal/role/handler.go

Copilot

Pull Request Overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

Copilot

Pull Request Overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

thiva-k · 2025-10-29T16:46:35Z

backend/internal/role/service.go

+
+	if err := rs.roleStore.CreateRole(role); err != nil {
+		logger.Error("Failed to create role", log.Error(err))
+		return nil, &ErrorInternalServerError


Scenarios for duplicate assignments/permissions to a role should be treated as client-side errors. Currently, these seem not to be differentiated in pre-checks, so they will be treated as server errors due to DB constraints right ?

Updated the precheck to remove the duplicates

thiva-k · 2025-10-29T16:49:09Z

backend/internal/role/service.go

+
+	// Validate group IDs using group service
+	if len(groupIDs) > 0 {
+		for _, groupID := range groupIDs {


Can't we use the existing validateGroupIDs here by making it public, instead of looping?

thiva-k · 2025-10-29T17:03:57Z

backend/dbscripts/thunderdb/postgres.sql

+-- Table to store Roles
+CREATE TABLE "ROLE" (
+    ID                  INT GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
+    ROLE_ID             VARCHAR(36) UNIQUE NOT NULL,


It will be better to add a unique constraint on (OU_ID, NAME) at DB level too right ? Could reduce potential race conditions

Added unique constraint on (OU_ID, NAME)

backend/dbscripts/thunderdb/postgres.sql

ThaminduDilshan · 2025-10-30T11:51:11Z

backend/internal/role/handler.go

+		return
+	}
+
+	logger.Debug("Successfully created role", log.String("role id", createdRole.ID))


Suggested change

logger.Debug("Successfully created role", log.String("role id", createdRole.ID))

logger.Debug("Successfully created role", log.String("roleId", createdRole.ID))

Shall we use camel case for log parameters for consistency?
Check other places as well

ThaminduDilshan · 2025-10-30T11:58:43Z

backend/internal/role/handler.go

+		for i, assignment := range request.Assignments {
+			sanitized.Assignments[i] = Assignment{
+				ID:   sysutils.SanitizeString(assignment.ID),
+				Type: assignment.Type,


Don't we need to sanitize this as well?

This is an AssigneeType. So don't need sanitize

Still this will be treated as a string right? AFAIK it doesn't guarantee this will be one of the defined AssigneeType.

Since AssigneeType only has two valid values ("user" and "group"), and the service layer already validates this. Do we need to do string sanitize ?

If we want to do it, then the option is to Convert to string, sanitize, then convert back to AssigneeType

Type: AssigneeType(sysutils.SanitizeString(string(assignment.Type))),

I don't think that's good idea.

ThaminduDilshan · 2025-10-30T12:05:55Z

backend/internal/role/model.go

+
+const (
+	// AssigneeTypeUser is the type for users.
+	AssigneeTypeUser AssigneeType = "user"


What if we define these type values as all caps for consistency?

This is how it's been defined in the groups package

https://github.com/asgardeo/thunder/blob/main/backend/internal/group/model.go#L26

I'd prefer to change them too. Maybe with a separate PR.
WDYT @darshanasbg ?

ThaminduDilshan · 2025-10-30T12:10:37Z

backend/internal/role/service.go

+// RoleServiceInterface defines the interface for the role service.
+type RoleServiceInterface interface {
+	GetRoleList(limit, offset int) (*RoleListResponse, *serviceerror.ServiceError)
+	CreateRole(request CreateRoleRequest) (*Role, *serviceerror.ServiceError)


I'd prefer to define separate structs for service layer rather than using request models.
WDYT?

ThaminduDilshan · 2025-10-30T12:50:10Z

backend/internal/role/service.go

+}
+
+// getDisplayNameForAssignment retrieves the display name for a user or group assignment.
+func (rs *roleService) getDisplayNameForAssignment(assignment *Assignment) (string, error) {


Shall we handle service errors properly when calling other services from this method? Currently even client errors gets wrapped as a server error right?
Maybe we can return a service error from getDisplayNameForAssignment method too

Updated the getDisplayNameForAssignment function

ThaminduDilshan · 2025-10-30T12:59:49Z

backend/internal/role/service.go

+
+	// Populate display names if requested
+	if includeDisplay {
+		for i := range assignments {


As discussed shall we add a note/ create an issue to improve display name retrieval?

Created an issue to track this #639

ThaminduDilshan · 2025-10-30T13:03:26Z

backend/internal/role/service.go

+	// Validate user IDs using user service
+	if len(userIDs) > 0 {
+		invalidUserIDs, svcErr := rs.userService.ValidateUserIDs(userIDs)
+		if svcErr != nil {


Shall we handle service errors properly?

backend/internal/role/storeconstants.go

backend/internal/system/error/serviceerror/error.go

backend/.mockery.private.yml

tests/integration/role/roleapi_test.go

Copilot

Pull Request Overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.

Copilot · 2025-10-30T18:17:17Z

backend/internal/system/error/serviceerror/error.go

+	// EncodingError is the error returned when encoding the response.
+	ErrorEncodingError = "{Code: \"ENC-5000\",Error: \"Encoding error\"," +
+		"ErrorDescription: \"An error occurred while encoding the response\"}"
+)


This error is defined as a raw JSON string instead of following the established ServiceError struct pattern used elsewhere in the codebase. This should be refactored to use the ServiceError struct type for consistency with other error definitions like ErrorInvalidRequestFormat in the role package.

Suggested change

// EncodingError is the error returned when encoding the response.

ErrorEncodingError = "{Code: \"ENC-5000\",Error: \"Encoding error\"," +

"ErrorDescription: \"An error occurred while encoding the response\"}"

)

// ErrorEncodingError is the error returned when encoding the response.

ErrorEncodingError = &ServiceError{

Code: "ENC-5000",

Type: ServerErrorType,

Error: "Encoding error",

ErrorDescription: "An error occurred while encoding the response",

}

This is intensional to send the json as string to the client incase of a encoding issue

Copilot

Pull Request Overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated no new comments.

rajithacharith · 2025-10-31T09:04:47Z

backend/internal/role/internal/role/RoleServiceInterface_mock_test.go

Is this directory structure intended? (internal/role/internal/role)

rajithacharith · 2025-10-31T09:10:36Z

backend/internal/role/service.go

+	// Validate organization unit exists using OU service
+	_, svcErr := rs.ouService.GetOrganizationUnit(request.OrganizationUnitID)
+	if svcErr != nil {
+		if svcErr.Code == oupkg.ErrorOrganizationUnitNotFound.Code {


Can we check for the exact error using errors.Is()? WDYT about returning the OU service error without having a new error object?

We can't do errors.Is() because svcErr is not a type of go error

Copilot

Pull Request Overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.

Copilot · 2025-10-31T13:30:35Z

backend/internal/role/errorconstants.go

+	// ErrorEmptyAssignments is the error returned when assignments list is empty.
+	ErrorEmptyAssignments = serviceerror.ServiceError{
+		Type:             serviceerror.ClientErrorType,
+		Code:             "ROL-1014",


Missing error code ROL-1013 in the sequence. The error codes jump from ROL-1012 to ROL-1014. Consider adding the missing code or renumbering ErrorEmptyAssignments to ROL-1013 to maintain sequential error codes.

Suggested change

Code: "ROL-1014",

Code: "ROL-1013",

Copilot

Pull Request Overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.

backend/internal/role/storeconstants.go

Copilot

Pull Request Overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

ThaminduDilshan · 2025-11-02T11:35:20Z

backend/internal/role/errorconstants.go

+	// ErrorMissingUserOrGroups is the error returned when both user ID and groups are missing.
+	ErrorMissingUserOrGroups = serviceerror.ServiceError{
+		Type:             serviceerror.ClientErrorType,
+		Code:             "ROL-1015",


Suggested change

Code: "ROL-1015",

Code: "ROL-1011",

ThaminduDilshan · 2025-11-02T11:38:44Z

backend/internal/role/handler.go

+		for i, assignment := range request.Assignments {
+			sanitized.Assignments[i] = Assignment{
+				ID:   sysutils.SanitizeString(assignment.ID),
+				Type: assignment.Type,


Still this will be treated as a string right? AFAIK it doesn't guarantee this will be one of the defined AssigneeType.

backend/internal/role/handler.go

ThaminduDilshan · 2025-11-02T11:53:31Z

backend/internal/role/model.go

+
+const (
+	// AssigneeTypeUser is the type for users.
+	AssigneeTypeUser AssigneeType = "user"


I'd prefer to change them too. Maybe with a separate PR.
WDYT @darshanasbg ?

ThaminduDilshan · 2025-11-02T11:56:56Z

backend/internal/role/model.go

+	Links        []LinkResponse       `json:"links"`
+}
+
+// Internal service layer structs - used for business logic processing


Suggested change

// Internal service layer structs - used for business logic processing

We don't need these comments right?

I would prefer to keep it as a separator to differentiate the structs used in the handler and service layer

Copilot

Pull Request Overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

senthalan requested review from ThaminduDilshan and darshanasbg as code owners October 20, 2025 15:24

senthalan force-pushed the implement-rbac-role-mgt branch from b9f6c29 to c44f505 Compare October 20, 2025 16:52

senthalan force-pushed the implement-rbac-role-mgt branch from c44f505 to ea4c14d Compare October 20, 2025 17:07

senthalan added the Type/Improvement label Oct 20, 2025

senthalan force-pushed the implement-rbac-role-mgt branch from ea4c14d to 2f79947 Compare October 20, 2025 18:25

senthalan requested a review from Copilot October 20, 2025 18:30

Copilot AI reviewed Oct 20, 2025

View reviewed changes

senthalan force-pushed the implement-rbac-role-mgt branch from 2f79947 to 5869e15 Compare October 21, 2025 05:01

Copilot AI review requested due to automatic review settings October 29, 2025 11:07

senthalan force-pushed the implement-rbac-role-mgt branch from 5869e15 to b683c07 Compare October 29, 2025 11:07

Copilot AI reviewed Oct 29, 2025

View reviewed changes

backend/internal/system/error/serviceerror/error.go Outdated Show resolved Hide resolved

senthalan force-pushed the implement-rbac-role-mgt branch from b683c07 to cfe897d Compare October 29, 2025 11:12

senthalan requested a review from Copilot October 29, 2025 11:14

Copilot AI reviewed Oct 29, 2025

View reviewed changes

backend/internal/role/handler.go Outdated Show resolved Hide resolved

senthalan requested a review from Copilot October 29, 2025 12:09

Copilot AI reviewed Oct 29, 2025

View reviewed changes

senthalan force-pushed the implement-rbac-role-mgt branch from cfe897d to 1f0db2d Compare October 29, 2025 12:51

senthalan requested a review from Copilot October 29, 2025 12:53

Copilot AI reviewed Oct 29, 2025

View reviewed changes

thiva-k reviewed Oct 29, 2025

View reviewed changes

ThaminduDilshan added Type/New Feature and removed Type/Improvement labels Oct 30, 2025

ThaminduDilshan reviewed Oct 30, 2025

View reviewed changes

senthalan force-pushed the implement-rbac-role-mgt branch from 1f0db2d to 49b0d57 Compare October 30, 2025 18:14

Copilot AI review requested due to automatic review settings October 30, 2025 18:15

senthalan force-pushed the implement-rbac-role-mgt branch from 49b0d57 to 8307bf6 Compare October 30, 2025 18:15

Copilot AI reviewed Oct 30, 2025

View reviewed changes

senthalan force-pushed the implement-rbac-role-mgt branch from 8307bf6 to eb95510 Compare October 30, 2025 18:20

senthalan requested a review from Copilot October 30, 2025 18:25

Copilot AI reviewed Oct 30, 2025

View reviewed changes

rajithacharith reviewed Oct 31, 2025

View reviewed changes

senthalan force-pushed the implement-rbac-role-mgt branch from eb95510 to dadda65 Compare October 31, 2025 13:23

Copilot AI review requested due to automatic review settings October 31, 2025 13:28

senthalan force-pushed the implement-rbac-role-mgt branch from dadda65 to b6261e4 Compare October 31, 2025 13:28

Copilot AI reviewed Oct 31, 2025

View reviewed changes

senthalan force-pushed the implement-rbac-role-mgt branch from b6261e4 to da0dbfc Compare October 31, 2025 13:35

Copilot AI review requested due to automatic review settings October 31, 2025 13:39

senthalan force-pushed the implement-rbac-role-mgt branch from da0dbfc to d86c3b7 Compare October 31, 2025 13:39

Copilot AI reviewed Oct 31, 2025

View reviewed changes

backend/internal/role/storeconstants.go Show resolved Hide resolved

senthalan force-pushed the implement-rbac-role-mgt branch from d86c3b7 to 4d17e91 Compare November 1, 2025 13:45

Copilot AI review requested due to automatic review settings November 2, 2025 05:43

Copilot AI reviewed Nov 2, 2025

View reviewed changes

ThaminduDilshan reviewed Nov 2, 2025

View reviewed changes

senthalan force-pushed the implement-rbac-role-mgt branch from 603668a to 0d7afb2 Compare November 2, 2025 16:16

Copilot AI review requested due to automatic review settings November 2, 2025 16:31

senthalan force-pushed the implement-rbac-role-mgt branch from 0d7afb2 to 322e91d Compare November 2, 2025 16:31

Copilot AI reviewed Nov 2, 2025

View reviewed changes

senthalan enabled auto-merge November 2, 2025 19:13

Role management implementation

edc921e

senthalan force-pushed the implement-rbac-role-mgt branch from 322e91d to edc921e Compare November 3, 2025 04:41

ThaminduDilshan approved these changes Nov 3, 2025

View reviewed changes

senthalan merged commit 952f916 into asgardeo:main Nov 3, 2025
5 of 8 checks passed

senthalan mentioned this pull request Nov 3, 2025

Fix role integration test #642

Merged

	logger.Debug("Successfully created role", log.String("role id", createdRole.ID))
	logger.Debug("Successfully created role", log.String("roleId", createdRole.ID))

-	// EncodingError is the error returned when encoding the response.
-	ErrorEncodingError = "{Code: \"ENC-5000\",Error: \"Encoding error\"," +
-		"ErrorDescription: \"An error occurred while encoding the response\"}"
-)
+	// ErrorEncodingError is the error returned when encoding the response.
+	ErrorEncodingError = &ServiceError{
+		Code:             "ENC-5000",
+		Type:             ServerErrorType,
+		Error:            "Encoding error",
+		ErrorDescription: "An error occurred while encoding the response",
+	}

Role management implementation #555

Role management implementation #555

Uh oh!

Conversation

senthalan commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Database Schema & Queries

Backend Service & API

Data Models

Error Handling

Testing & Mocking

Uh oh!

codecov bot commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

senthalan commented Oct 20, 2025 •

edited

Loading

codecov bot commented Oct 20, 2025 •

edited

Loading