diff --git a/auth-center/pkg/model/model.go b/auth-center/pkg/model/model.go index 4ac86e2d..09e702b0 100644 --- a/auth-center/pkg/model/model.go +++ b/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{ diff --git a/auth-center/pkg/model/model_test.go b/auth-center/pkg/model/model_test.go new file mode 100644 index 00000000..2bcdd3d2 --- /dev/null +++ b/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) + } + } +} diff --git a/auth-center/pkg/service/helpers.go b/auth-center/pkg/service/helpers.go index fc8f0447..ca06ef4d 100644 --- a/auth-center/pkg/service/helpers.go +++ b/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) { diff --git a/auth-center/pkg/service/user_generic.go b/auth-center/pkg/service/user_generic.go index 69006450..d8c0c0ca 100644 --- a/auth-center/pkg/service/user_generic.go +++ b/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() } } diff --git a/auth-center/pkg/service/user_generic_test.go b/auth-center/pkg/service/user_generic_test.go new file mode 100644 index 00000000..95f620da --- /dev/null +++ b/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()) + } + }) + } +}