From 73e9bc3a22df3d2739ec312d8107cde82b2257ec Mon Sep 17 00:00:00 2001 From: Raphael Date: Sat, 27 Jun 2026 10:45:11 +0200 Subject: [PATCH 1/2] feat: added user restrictions --- .../types/user_deletion_restriction_enum.rb | 14 +++++++ app/graphql/types/user_type.rb | 7 ++++ app/services/error_code.rb | 1 + app/services/users/delete_service.rb | 20 +++++++++ docs/graphql/enum/errorcodeenum.md | 2 + docs/graphql/enum/userdeletionrestriction.md | 10 +++++ docs/graphql/object/user.md | 1 + .../cloud/app/services/cloud/error_code.rb | 22 ++++++++++ .../services/cloud/users/delete_service.rb | 33 +++++++++++++++ .../cloud/users/delete_service_spec.rb | 42 +++++++++++++++++++ .../graphql/query/users_query_spec.rb | 7 ++++ spec/services/users/delete_service_spec.rb | 20 +++++++++ 12 files changed, 179 insertions(+) create mode 100644 app/graphql/types/user_deletion_restriction_enum.rb create mode 100644 docs/graphql/enum/userdeletionrestriction.md create mode 100644 extensions/cloud/app/services/cloud/error_code.rb create mode 100644 extensions/cloud/app/services/cloud/users/delete_service.rb create mode 100644 extensions/cloud/spec/services/cloud/users/delete_service_spec.rb diff --git a/app/graphql/types/user_deletion_restriction_enum.rb b/app/graphql/types/user_deletion_restriction_enum.rb new file mode 100644 index 00000000..637f0ac1 --- /dev/null +++ b/app/graphql/types/user_deletion_restriction_enum.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Types + class UserDeletionRestrictionEnum < Types::BaseEnum + description 'The reason why a user cannot be deleted.' + + value 'LAST_ADMINISTRATOR', + 'The user is the last administrator of the instance.', + value: :last_administrator + value 'ACTIVE_SUBSCRIPTION', + 'The current user has an active subscription.', + value: :active_subscription + end +end diff --git a/app/graphql/types/user_type.rb b/app/graphql/types/user_type.rb index dce975f2..525d9b57 100644 --- a/app/graphql/types/user_type.rb +++ b/app/graphql/types/user_type.rb @@ -12,6 +12,9 @@ class UserType < Types::BaseObject null: true, description: 'Global admin status of the user', authorize: :read_admin_status + field :deletion_restriction, Types::UserDeletionRestrictionEnum, + null: true, + description: 'The reason why this user cannot be deleted' field :email, String, null: true, description: 'Email of the user', authorize: :read_email field :email_verified_at, Types::TimeType, null: true, @@ -72,5 +75,9 @@ def mfa_status backup_codes_count: object.backup_codes.size, } end + + def deletion_restriction + Users::DeleteService.new(current_authentication, object).deletion_restriction + end end end diff --git a/app/services/error_code.rb b/app/services/error_code.rb index 06711c73..1a405156 100644 --- a/app/services/error_code.rb +++ b/app/services/error_code.rb @@ -17,6 +17,7 @@ def self.error_codes missing_permission: { description: 'The user is not permitted to perform this operation' }, missing_parameter: { description: 'Not all required parameters are present' }, cannot_remove_last_administrator: { description: 'This action would remove the last administrator' }, + cannot_delete_last_administrator: { description: 'The last instance administrator cannot be deleted' }, cannot_remove_last_admin_ability: { description: 'This action would remove the last administrative ability' }, cannot_delete_last_admin_role: { description: 'This action would remove the last administrative role' }, inconsistent_namespace: { description: 'Resources are from different namespaces' }, diff --git a/app/services/users/delete_service.rb b/app/services/users/delete_service.rb index 98ea6725..40d645e7 100644 --- a/app/services/users/delete_service.rb +++ b/app/services/users/delete_service.rb @@ -16,6 +16,9 @@ def execute return ServiceResponse.error(message: 'Missing permission', error_code: :missing_permission) end + deletion_error = validate_deletion + return deletion_error if deletion_error + transactional do |t| user.destroy @@ -38,5 +41,22 @@ def execute ServiceResponse.success(message: 'Deleted user', payload: user) end end + + def deletion_restriction + :last_administrator if user.admin? && !User.where.not(id: user.id).exists?(admin: true) + end + + private + + def validate_deletion + return unless deletion_restriction == :last_administrator + + ServiceResponse.error( + message: 'The last instance administrator cannot be deleted', + error_code: :cannot_delete_last_administrator + ) + end end end + +Users::DeleteService.prepend_extensions diff --git a/docs/graphql/enum/errorcodeenum.md b/docs/graphql/enum/errorcodeenum.md index 65bd1d66..45fb15d2 100644 --- a/docs/graphql/enum/errorcodeenum.md +++ b/docs/graphql/enum/errorcodeenum.md @@ -6,7 +6,9 @@ Represents the available error responses | Value | Description | |-------|-------------| +| `CANNOT_DELETE_LAST_ADMINISTRATOR` | The last instance administrator cannot be deleted | | `CANNOT_DELETE_LAST_ADMIN_ROLE` | This action would remove the last administrative role | +| `CANNOT_DELETE_USER_WITH_ACTIVE_SUBSCRIPTION` | A user with an active subscription cannot delete itself | | `CANNOT_MODIFY_ADMIN` | Only administrators can modify admin status of users | | `CANNOT_MODIFY_OWN_ADMIN` | Users cannot modify their own admin status | | `CANNOT_REMOVE_LAST_ADMINISTRATOR` | This action would remove the last administrator | diff --git a/docs/graphql/enum/userdeletionrestriction.md b/docs/graphql/enum/userdeletionrestriction.md new file mode 100644 index 00000000..0234f987 --- /dev/null +++ b/docs/graphql/enum/userdeletionrestriction.md @@ -0,0 +1,10 @@ +--- +title: UserDeletionRestriction +--- + +The reason why a user cannot be deleted. + +| Value | Description | +|-------|-------------| +| `ACTIVE_SUBSCRIPTION` | The current user has an active subscription. | +| `LAST_ADMINISTRATOR` | The user is the last administrator of the instance. | diff --git a/docs/graphql/object/user.md b/docs/graphql/object/user.md index 0b8f27af..84564d2c 100644 --- a/docs/graphql/object/user.md +++ b/docs/graphql/object/user.md @@ -11,6 +11,7 @@ Represents a user | `admin` | [`Boolean`](../scalar/boolean.md) | Global admin status of the user | | `avatarPath` | [`String`](../scalar/string.md) | The avatar if present of the user | | `createdAt` | [`Time!`](../scalar/time.md) | Time when this User was created | +| `deletionRestriction` | [`UserDeletionRestriction`](../enum/userdeletionrestriction.md) | The reason why this user cannot be deleted | | `email` | [`String`](../scalar/string.md) | Email of the user | | `emailVerifiedAt` | [`Time`](../scalar/time.md) | Email verification date of the user if present | | `firstname` | [`String`](../scalar/string.md) | Firstname of the user | diff --git a/extensions/cloud/app/services/cloud/error_code.rb b/extensions/cloud/app/services/cloud/error_code.rb new file mode 100644 index 00000000..2d00d8a9 --- /dev/null +++ b/extensions/cloud/app/services/cloud/error_code.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module CLOUD + module ErrorCode + extend ActiveSupport::Concern + + class_methods do + include Sagittarius::Override + + override :error_codes + def error_codes + super.merge( + { + cannot_delete_user_with_active_subscription: { + description: 'A user with an active subscription cannot delete itself', + }, + } + ) + end + end + end +end diff --git a/extensions/cloud/app/services/cloud/users/delete_service.rb b/extensions/cloud/app/services/cloud/users/delete_service.rb new file mode 100644 index 00000000..fbc767c0 --- /dev/null +++ b/extensions/cloud/app/services/cloud/users/delete_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module CLOUD + module Users + module DeleteService + include Sagittarius::Override + + override :deletion_restriction + def deletion_restriction + restriction = super + return restriction if restriction + return unless user == current_authentication.user + + current_license = user.namespace&.current_license + license_data = current_license&.license + :active_subscription if license_data&.options&.[](:subscription) + end + + private + + override :validate_deletion + def validate_deletion + restriction = deletion_restriction + return super unless restriction == :active_subscription + + ServiceResponse.error( + message: 'A user with an active subscription cannot delete itself', + error_code: :cannot_delete_user_with_active_subscription + ) + end + end + end +end diff --git a/extensions/cloud/spec/services/cloud/users/delete_service_spec.rb b/extensions/cloud/spec/services/cloud/users/delete_service_spec.rb new file mode 100644 index 00000000..b9317b80 --- /dev/null +++ b/extensions/cloud/spec/services/cloud/users/delete_service_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Users::DeleteService do + subject(:service_response) do + described_class.new(create_authentication(current_user), user).execute + end + + let(:current_user) { create(:user) } + let(:user) { current_user } + let(:namespace) { current_user.ensure_namespace } + + before do + current_user.update!(admin: true) + create(:user, :admin) + end + + context 'when the current user has an active subscription' do + before { create(:license, namespace: namespace, options: { subscription: true }) } + + it 'does not delete the user' do + expect(described_class.new(create_authentication(current_user), user).deletion_restriction) + .to eq(:active_subscription) + expect(service_response).not_to be_success + expect(service_response.payload[:error_code]).to eq(:cannot_delete_user_with_active_subscription) + expect(User.exists?(user.id)).to be true + is_expected.not_to create_audit_event(:user_deleted) + end + end + + context 'when another user has an active subscription' do + let(:user) { create(:user) } + + before { create(:license, namespace: user.ensure_namespace, options: { subscription: true }) } + + it 'allows an administrator to delete the user' do + expect { service_response }.to change { User.exists?(user.id) }.from(true).to(false) + expect(service_response).to be_success + end + end +end diff --git a/spec/requests/graphql/query/users_query_spec.rb b/spec/requests/graphql/query/users_query_spec.rb index 0db57b59..a90cad65 100644 --- a/spec/requests/graphql/query/users_query_spec.rb +++ b/spec/requests/graphql/query/users_query_spec.rb @@ -12,6 +12,7 @@ nodes { id username + deletionRestriction } } } @@ -48,5 +49,11 @@ it 'returns all users' do expect(graphql_data_at(:users, :nodes)).to have_attributes(length: 4) end + + it 'returns the deletion restriction for the last administrator' do + admin = graphql_data_at(:users, :nodes).find { |user| user['id'] == current_user.to_global_id.to_s } + + expect(admin['deletionRestriction']).to eq('LAST_ADMINISTRATOR') + end end end diff --git a/spec/services/users/delete_service_spec.rb b/spec/services/users/delete_service_spec.rb index b2a989b7..4397719c 100644 --- a/spec/services/users/delete_service_spec.rb +++ b/spec/services/users/delete_service_spec.rb @@ -26,6 +26,26 @@ ) end + context 'when the user is the last administrator' do + let(:user) { current_user } + + it 'does not delete the user' do + expect(service_response).not_to be_success + expect(service_response.payload[:error_code]).to eq(:cannot_delete_last_administrator) + expect(User.exists?(user.id)).to be true + is_expected.not_to create_audit_event(:user_deleted) + end + end + + context 'when the user is an administrator and another administrator exists' do + let(:user) { create(:user, :admin) } + + it 'deletes the user successfully' do + expect { service_response }.to change { User.exists?(user.id) }.from(true).to(false) + expect(service_response).to be_success + end + end + context 'when current user lacks permission' do let(:current_user) { create(:user) } From d81957267d1599e5e463f1ef776f3dd226a05cae Mon Sep 17 00:00:00 2001 From: Raphael Date: Sat, 27 Jun 2026 11:00:42 +0200 Subject: [PATCH 2/2] fix: forgot to add restrictions to user tests --- app/services/users/delete_service.rb | 2 +- extensions/cloud/app/services/cloud/users/delete_service.rb | 2 +- spec/graphql/types/user_type_spec.rb | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/services/users/delete_service.rb b/app/services/users/delete_service.rb index 40d645e7..bb423c6f 100644 --- a/app/services/users/delete_service.rb +++ b/app/services/users/delete_service.rb @@ -46,7 +46,7 @@ def deletion_restriction :last_administrator if user.admin? && !User.where.not(id: user.id).exists?(admin: true) end - private + protected def validate_deletion return unless deletion_restriction == :last_administrator diff --git a/extensions/cloud/app/services/cloud/users/delete_service.rb b/extensions/cloud/app/services/cloud/users/delete_service.rb index fbc767c0..7e267074 100644 --- a/extensions/cloud/app/services/cloud/users/delete_service.rb +++ b/extensions/cloud/app/services/cloud/users/delete_service.rb @@ -16,7 +16,7 @@ def deletion_restriction :active_subscription if license_data&.options&.[](:subscription) end - private + protected override :validate_deletion def validate_deletion diff --git a/spec/graphql/types/user_type_spec.rb b/spec/graphql/types/user_type_spec.rb index e27bba55..df3f5e78 100644 --- a/spec/graphql/types/user_type_spec.rb +++ b/spec/graphql/types/user_type_spec.rb @@ -20,6 +20,7 @@ identities mfaStatus userAbilities + deletionRestriction createdAt updatedAt ]