fix(auth-center): restrict role assignment in user create and update

auth-center/pkg/model/model.go

@@ -74,7 +74,7 @@ var (
 			RoleName: "Security engineer",
 			RolePermissions: Permissions{
 				Users: &jwt.Permission{
-					Actions:     []jwt.Action{"read", "update", "create", "delete"},
+					Actions:     []jwt.Action{"read", "update"},
 					Description: "User management",
 				},
 				Roles: &jwt.Permission{

auth-center/pkg/model/model_test.go

@@ -0,0 +1,43 @@
+package model
+
+import (
+	"testing"
+
+	"github.com/runtime-radar/runtime-radar/lib/security/jwt"
+)
+
+// Creating and deleting users are administrator-only actions. Any other predeclared role holding
+// them would let its holders manage accounts other than their own, which is what the check in the
+// service layer exists to prevent.
+func TestOnlyAdministratorManagesUsers(t *testing.T) {
+	restricted := []jwt.Action{"create", "delete"}
+
+	for _, action := range restricted {
+		var adminHolds bool
+
+		for _, role := range PredeclaredRoles {
+			users := role.RolePermissions.Users
+			if users == nil {
+				continue
+			}
+
+			var holds bool
+			for _, granted := range users.Actions {
+				if granted == action {
+					holds = true
+				}
+			}
+
+			switch {
+			case role.ID == AdminRoleID:
+				adminHolds = holds
+			case holds:
+				t.Errorf("Role %q must not hold users:%s", role.RoleName, action)
+			}
+		}
+
+		if !adminHolds {
+			t.Errorf("Expected the administrator role to hold users:%s", action)
+		}
+	}
+}

auth-center/pkg/service/helpers.go

@@ -12,6 +12,13 @@ import (
 
 const maskedPassword = "******"
 
+// gRPC errdetails.ErrorInfo.Reason codes used in service responses.
+const (
+	RoleAssignmentRestricted = "ROLE_ASSIGNMENT_RESTRICTED"
+	UserManagementRestricted = "USER_MANAGEMENT_RESTRICTED"
+	LastAdminRemovingDenied  = "LAST_ADMIN_REMOVING_DENIED"
+)
+
 func haveUpper(s string) bool {
 	for _, r := range s {
 		if unicode.IsUpper(r) && unicode.IsLetter(r) {

auth-center/pkg/service/user_generic.go

@@ -101,6 +101,10 @@ func (ug *UserGeneric) Create(ctx context.Context, req *api.CreateUserReq) (resp
 		return nil, status.Errorf(codes.InvalidArgument, "can't parse role id: %v", err)
 	}
 
+	if err := ug.verifyUserCreation(ctx); err != nil {
+		return nil, err
+	}
+
 	_, err = mail.ParseAddress(req.Email)
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "can't parse email: %v", err)
@@ -167,13 +171,73 @@ func (ug *UserGeneric) Create(ctx context.Context, req *api.CreateUserReq) (resp
 	return resp, nil
 }
 
+// verifyUserCreation checks that the caller may add an account. Only an administrator creates users,
+// since creating one also assigns its role and would otherwise hand out administrator itself.
+func (ug *UserGeneric) verifyUserCreation(ctx context.Context) error {
+	token, err := tokens.AccessTokenFromContext(ctx, ug.TokenKey)
+	if err != nil {
+		return status.Errorf(codes.Unauthenticated, "can't get token: %v", err)
+	}
+
+	if token.Role.ID != model.AdminRoleID {
+		return errcommon.StatusWithReason(codes.PermissionDenied, UserManagementRestricted,
+			"can't create users").Err()
+	}
+
+	return nil
+}
+
+// verifyUserUpdate checks that the caller may apply the requested change to target. Only an
+// administrator edits other accounts and hands out roles, otherwise users:update escalates.
+func (ug *UserGeneric) verifyUserUpdate(ctx context.Context, target *model.User, newRoleID uuid.UUID) error {
+	token, err := tokens.AccessTokenFromContext(ctx, ug.TokenKey)
+	if err != nil {
+		return status.Errorf(codes.Unauthenticated, "can't get token: %v", err)
+	}
+
+	if token.Role.ID == model.AdminRoleID {
+		if target.RoleID == model.AdminRoleID && newRoleID != model.AdminRoleID {
+			return ug.verifyNotLastAdmin(ctx)
+		}
+
+		return nil
+	}
+
+	if token.UserID != target.ID.String() {
+		return errcommon.StatusWithReason(codes.PermissionDenied, UserManagementRestricted,
+			"can't modify another user").Err()
+	}
+
+	if newRoleID != target.RoleID {
+		return errcommon.StatusWithReason(codes.PermissionDenied, RoleAssignmentRestricted,
+			"can't change your own role").Err()
+	}
+
+	return nil
+}
+
+// verifyNotLastAdmin rejects demoting the only administrator left, the same way Delete guards the last one.
+func (ug *UserGeneric) verifyNotLastAdmin(ctx context.Context) error {
+	adminUsers, err := ug.UserRepository.GetUsersByRoleID(ctx, model.AdminRoleID)
+	if err != nil {
+		return status.Error(codes.Internal, "internal error")
+	}
+
+	if len(adminUsers) == 1 {
+		return errcommon.StatusWithReason(codes.PermissionDenied, LastAdminRemovingDenied,
+			"can't demote last administrator").Err()
+	}
+
+	return nil
+}
+
 func (ug *UserGeneric) Update(ctx context.Context, req *api.UpdateUserReq) (resp *api.UserResp, err error) {
 	id, err := uuid.Parse(req.GetId())
 	if err != nil {
 		return nil, status.Errorf(codes.InvalidArgument, "can't parse id: %v", err)
 	}
 
-	_, err = ug.UserRepository.GetByID(ctx, id)
+	target, err := ug.UserRepository.GetByID(ctx, id)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
 			return nil, status.Error(codes.NotFound, "user does not exist")
@@ -195,6 +259,10 @@ func (ug *UserGeneric) Update(ctx context.Context, req *api.UpdateUserReq) (resp
 		return nil, status.Errorf(codes.InvalidArgument, "can't parse role id")
 	}
 
+	if err := ug.verifyUserUpdate(ctx, target, roleID); err != nil {
+		return nil, err
+	}
+
 	user := &model.User{
 		Base:   model.Base{ID: id},
 		Email:  req.Email,
@@ -247,7 +315,7 @@ func (ug *UserGeneric) Delete(ctx context.Context, req *api.DeleteUserReq) (resp
 			return nil, status.Error(codes.Internal, "internal error")
 		}
 		if len(adminUsers) == 1 {
-			return nil, errcommon.StatusWithReason(codes.PermissionDenied, "LAST_ADMIN_REMOVING_DENIED", "can't delete last administrator").Err()
+			return nil, errcommon.StatusWithReason(codes.PermissionDenied, LastAdminRemovingDenied, "can't delete last administrator").Err()
 		}
 	}
 

auth-center/pkg/service/user_generic_test.go

@@ -0,0 +1,226 @@
+package service
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/google/uuid"
+	"github.com/runtime-radar/runtime-radar/auth-center/pkg/database"
+	"github.com/runtime-radar/runtime-radar/auth-center/pkg/model"
+	"github.com/runtime-radar/runtime-radar/auth-center/pkg/tokens"
+	"github.com/runtime-radar/runtime-radar/lib/errcommon"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/status"
+)
+
+var (
+	securityEngineerRoleID = uuid.MustParse("00000000-0000-0000-0000-000000000002")
+	cicdRoleID             = uuid.MustParse("00000000-0000-0000-0000-000000000003")
+)
+
+// stubUserRepository answers with a fixed number of administrators; the other methods are unused here.
+type stubUserRepository struct {
+	database.UserRepository
+
+	admins int
+}
+
+func (s *stubUserRepository) GetUsersByRoleID(_ context.Context, roleID uuid.UUID) ([]*model.User, error) {
+	users := make([]*model.User, 0, s.admins)
+	for i := 0; i < s.admins; i++ {
+		users = append(users, &model.User{RoleID: roleID, Role: model.Role{ID: roleID}})
+	}
+
+	return users, nil
+}
+
+// ctxWithCaller builds an incoming gRPC context carrying an access token issued for the given user,
+// the way the interceptor would populate it for a real call.
+func ctxWithCaller(t *testing.T, key []byte, userID, roleID uuid.UUID) context.Context {
+	t.Helper()
+
+	user := model.User{
+		Base:                  model.Base{ID: userID},
+		Username:              "caller",
+		RoleID:                roleID,
+		Role:                  model.Role{ID: roleID},
+		LastPasswordChangedAt: time.Now(),
+	}
+
+	pair, err := tokens.GenerateTokenPair(user, key, time.Minute, time.Hour)
+	if err != nil {
+		t.Fatalf("can't generate token pair: %v", err)
+	}
+
+	md := metadata.Pairs(tokens.AuthorizationKey, "Bearer "+pair.AccessTokenHash)
+
+	return metadata.NewIncomingContext(context.Background(), md)
+}
+
+// requirePermission asserts that err carries the expected PermissionDenied reason, or that there is
+// no error at all when wantReason is empty.
+func requirePermission(t *testing.T, err error, wantReason string) {
+	t.Helper()
+
+	if wantReason == "" {
+		if err != nil {
+			t.Fatalf("Expected no error, got %v", err)
+		}
+
+		return
+	}
+
+	st, ok := status.FromError(err)
+	if !ok {
+		t.Fatalf("Expected a gRPC status error, got %v", err)
+	}
+	if st.Code() != codes.PermissionDenied {
+		t.Fatalf("Expected code %v, got %v", codes.PermissionDenied, st.Code())
+	}
+	if reason, _ := errcommon.ReasonFromStatus(st); reason != wantReason {
+		t.Fatalf("Expected reason %q, got %q", wantReason, reason)
+	}
+}
+
+func TestVerifyUserCreation(t *testing.T) {
+	key := []byte("test-token-key")
+
+	tests := []struct {
+		name         string
+		callerRoleID uuid.UUID
+		wantReason   string
+	}{
+		{
+			name:         "admin creates a user",
+			callerRoleID: model.AdminRoleID,
+		},
+		{
+			name:         "non-admin creates a user",
+			callerRoleID: securityEngineerRoleID,
+			wantReason:   UserManagementRestricted,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := ctxWithCaller(t, key, uuid.New(), tt.callerRoleID)
+			ug := &UserGeneric{TokenKey: key}
+
+			requirePermission(t, ug.verifyUserCreation(ctx), tt.wantReason)
+		})
+	}
+}
+
+func TestVerifyUserUpdate(t *testing.T) {
+	key := []byte("test-token-key")
+
+	tests := []struct {
+		name         string
+		callerRoleID uuid.UUID
+		// targetIsCaller tells whether the updated account is the caller's own one.
+		targetIsCaller bool
+		targetRoleID   uuid.UUID
+		newRoleID      uuid.UUID
+		admins         int
+		wantReason     string
+	}{
+		{
+			name:         "admin promotes another user",
+			callerRoleID: model.AdminRoleID,
+			targetRoleID: securityEngineerRoleID,
+			newRoleID:    model.AdminRoleID,
+		},
+		{
+			name:         "admin demotes another admin",
+			callerRoleID: model.AdminRoleID,
+			targetRoleID: model.AdminRoleID,
+			newRoleID:    securityEngineerRoleID,
+			admins:       2,
+		},
+		{
+			name:         "admin demotes the last admin",
+			callerRoleID: model.AdminRoleID,
+			targetRoleID: model.AdminRoleID,
+			newRoleID:    securityEngineerRoleID,
+			admins:       1,
+			wantReason:   LastAdminRemovingDenied,
+		},
+		{
+			name:           "non-admin edits its own account",
+			callerRoleID:   securityEngineerRoleID,
+			targetIsCaller: true,
+			targetRoleID:   securityEngineerRoleID,
+			newRoleID:      securityEngineerRoleID,
+		},
+		{
+			name:         "non-admin edits another user",
+			callerRoleID: securityEngineerRoleID,
+			targetRoleID: cicdRoleID,
+			newRoleID:    cicdRoleID,
+			wantReason:   UserManagementRestricted,
+		},
+		{
+			name:         "non-admin edits an admin account",
+			callerRoleID: securityEngineerRoleID,
+			targetRoleID: model.AdminRoleID,
+			newRoleID:    model.AdminRoleID,
+			wantReason:   UserManagementRestricted,
+		},
+		{
+			name:           "non-admin escalates itself to admin",
+			callerRoleID:   securityEngineerRoleID,
+			targetIsCaller: true,
+			targetRoleID:   securityEngineerRoleID,
+			newRoleID:      model.AdminRoleID,
+			wantReason:     RoleAssignmentRestricted,
+		},
+		{
+			name:           "non-admin changes its own role",
+			callerRoleID:   securityEngineerRoleID,
+			targetIsCaller: true,
+			targetRoleID:   securityEngineerRoleID,
+			newRoleID:      cicdRoleID,
+			wantReason:     RoleAssignmentRestricted,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			callerID := uuid.New()
+			ctx := ctxWithCaller(t, key, callerID, tt.callerRoleID)
+			ug := &UserGeneric{TokenKey: key, UserRepository: &stubUserRepository{admins: tt.admins}}
+
+			targetID := uuid.New()
+			if tt.targetIsCaller {
+				targetID = callerID
+			}
+			target := &model.User{Base: model.Base{ID: targetID}, RoleID: tt.targetRoleID}
+
+			requirePermission(t, ug.verifyUserUpdate(ctx, target, tt.newRoleID), tt.wantReason)
+		})
+	}
+}
+
+func TestVerifyWithoutToken(t *testing.T) {
+	ug := &UserGeneric{TokenKey: []byte("test-token-key")}
+	target := &model.User{Base: model.Base{ID: uuid.New()}, RoleID: model.AdminRoleID}
+
+	tests := map[string]func() error{
+		"create": func() error { return ug.verifyUserCreation(context.Background()) },
+		"update": func() error { return ug.verifyUserUpdate(context.Background(), target, model.AdminRoleID) },
+	}
+
+	for name, verify := range tests {
+		t.Run(name, func(t *testing.T) {
+			st, ok := status.FromError(verify())
+			if !ok {
+				t.Fatalf("Expected a gRPC status error, got %v", verify())
+			}
+			if st.Code() != codes.Unauthenticated {
+				t.Fatalf("Expected code %v, got %v", codes.Unauthenticated, st.Code())
+			}
+		})
+	}
+}