Merge pull request #1078 from code0-tech/#923-user-deletion-fails

raphael-goetz · web-flow · commit 315fff7e9283 · 2026-07-10T21:16:16.000+02:00
added ghost user
diff --git a/app/models/user.rb b/app/models/user.rb
@@ -3,8 +3,15 @@
 class User < ApplicationRecord
   include NamespaceParent
 
+  USER_TYPES = {
+    regular: 0,
+    ghost: 1,
+  }.freeze
+
   has_secure_password
 
+  enum :user_type, USER_TYPES
+
   validates :username, length: { maximum: 50 },
                        presence: true,
                        allow_blank: false,
@@ -31,6 +38,10 @@ class User < ApplicationRecord
 
   has_one_attached :avatar
 
+  def self.ghost
+    find_by!(user_type: :ghost)
+  end
+
   def mfa_enabled?
     totp_secret != nil
   end
diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb
@@ -3,6 +3,7 @@
 class GlobalPolicy < BasePolicy
   condition(:organization_creation_restricted) { ApplicationSetting.current[:organization_creation_restricted] }
   condition(:admin) { user&.admin }
+  condition(:user_is_regular) { user&.regular? }
 
   rule { ~anonymous }.enable :create_organization
   rule { organization_creation_restricted & ~admin }.prevent :create_organization
@@ -15,6 +16,8 @@ class GlobalPolicy < BasePolicy
     enable :read_velorum_config
   end
 
+  rule { user_is_regular }.enable :create_user_session
+
   rule { admin }.policy do
     enable :read_application_setting
     enable :update_application_setting
diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb
@@ -3,6 +3,7 @@
 class UserPolicy < BasePolicy
   condition(:user_is_self) { subject.id == user&.id }
   condition(:user_is_admin) { user&.admin? || false }
+  condition(:subject_is_regular) { subject.regular? }
   condition(:admin_status_visible) { ApplicationSetting.current[:admin_status_visible] }
 
   rule { ~anonymous }.enable :read_user
@@ -17,6 +18,8 @@ class UserPolicy < BasePolicy
     enable :read_mfa_status
   end
 
+  rule { ~subject_is_regular }.prevent :delete_user
+
   rule { admin_status_visible & ~anonymous }.enable :read_admin_status
 
   rule { user_is_self }.policy do
diff --git a/app/services/users/delete_service.rb b/app/services/users/delete_service.rb
@@ -18,7 +18,10 @@ def execute
 
       transactional do |t|
         namespace = user.namespace
+        ghost_user = User.ghost
+        audit_author_id = user == current_authentication.user ? ghost_user.id : current_authentication.user.id
 
+        user.authored_audit_events.update_all(author_id: ghost_user.id) # rubocop:disable Rails/SkipsModelValidations
         user.destroy
 
         if user.persisted?
@@ -41,7 +44,7 @@ def execute
 
         AuditService.audit(
           :user_deleted,
-          author_id: current_authentication.user.id,
+          author_id: audit_author_id,
           entity: user,
           target: AuditEvent::GLOBAL_TARGET,
           details: {}
diff --git a/app/services/users/identity/login_service.rb b/app/services/users/identity/login_service.rb
@@ -50,6 +50,16 @@ def execute
                                                          details: user_session.errors)
           end
 
+          unless Ability.allowed?(Sagittarius::Authentication.new(:session, user_session), :create_user_session)
+            logger.warn(
+              message: 'User was not allowed to create user session',
+              user_id: user.id,
+              username: user.username
+            )
+            t.rollback_and_return! ServiceResponse.error(message: 'Not allowed to create user session',
+                                                         error_code: :invalid_login_data)
+          end
+
           AuditService.audit(
             :user_logged_in,
             author_id: user.id,
diff --git a/app/services/users/login_service.rb b/app/services/users/login_service.rb
@@ -45,6 +45,12 @@ def execute
                                                        details: user_session.errors)
         end
 
+        unless Ability.allowed?(Sagittarius::Authentication.new(:session, user_session), :create_user_session)
+          logger.warn(message: 'User was not allowed to create user session', user_id: user.id, username: user.username)
+          t.rollback_and_return! ServiceResponse.error(message: 'Not allowed to create user session',
+                                                       error_code: :invalid_login_data)
+        end
+
         AuditService.audit(
           :user_logged_in,
           author_id: user.id,
diff --git a/db/fixtures/02_users.rb b/db/fixtures/02_users.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+User.seed_once :user_type do |u|
+  u.username = 'ghost'
+  u.email = 'ghost@code0.tech'
+  u.password = SecureRandom.hex
+  u.admin = false
+  u.user_type = :ghost
+  u.readme = 'This user will hold the activity of the users that have been deleted'
+end
diff --git a/db/fixtures/production/01_users.rb b/db/fixtures/production/01_users.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-return unless User.none?
+return unless User.regular.none?
 
 initial_root_email = ENV.fetch('INITIAL_ROOT_MAIL', nil)
 initial_root_password = ENV.fetch('INITIAL_ROOT_PASSWORD', SecureRandom.hex)
diff --git a/db/migrate/20260629120000_add_user_type_to_users.rb b/db/migrate/20260629120000_add_user_type_to_users.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddUserTypeToUsers < Code0::ZeroTrack::Database::Migration[1.0]
+  def change
+    add_column :users, :user_type, :integer, default: 0, null: false
+  end
+end
diff --git a/db/schema_migrations/20260629120000 b/db/schema_migrations/20260629120000
@@ -0,0 +1 @@
+68074a1fb7f8bdf3087c3b6823eaef35175c480f05d4bc73c190ef7b2811cf68
diff --git a/db/structure.sql b/db/structure.sql
@@ -1164,6 +1164,7 @@ CREATE TABLE users (
     totp_secret text,
     email_verified_at timestamp with time zone,
     readme text,
+    user_type integer DEFAULT 0 NOT NULL,
     blocked_at timestamp with time zone,
     CONSTRAINT check_11461c37fb CHECK ((char_length(readme) <= 5000)),
     CONSTRAINT check_3bedaaa612 CHECK ((char_length(email) <= 255)),
diff --git a/extensions/ee/app/services/ee/users/validate_user_limit.rb b/extensions/ee/app/services/ee/users/validate_user_limit.rb
@@ -8,7 +8,7 @@ module ValidateUserLimit
       def validate_user_limit!(t)
         license = License.current
         return if license.nil? || !license.restricted?(:user_count)
-        return if User.count <= license.restrictions[:user_count]
+        return if User.regular.count <= license.restrictions[:user_count]
 
         t.rollback_and_return! ServiceResponse.error(
           message: 'No free user seats in license',
diff --git a/extensions/ee/spec/services/ee/users/create_service_spec.rb b/extensions/ee/spec/services/ee/users/create_service_spec.rb
@@ -26,4 +26,14 @@
     it { expect { service_response }.not_to change { User.count } }
     it { expect(service_response.payload[:error_code]).to eq(:no_free_license_seats) }
   end
+
+  context 'when non-regular users exist' do
+    before do
+      current_user
+      create(:user, :ghost)
+      create(:license, restrictions: { user_count: 2 })
+    end
+
+    it { is_expected.to be_success }
+  end
 end
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
@@ -22,6 +22,10 @@
       blocked_at { Time.zone.now }
     end
 
+    trait :ghost do
+      user_type { :ghost }
+    end
+
     trait :with_namespace do
       after :build, &:ensure_namespace
     end
diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb
@@ -18,4 +18,16 @@
 
     it { is_expected.not_to be_allowed(:read_user) }
   end
+
+  context 'when the current user is an admin' do
+    let(:current_user) { create(:user, :admin) }
+
+    it { is_expected.to be_allowed(:delete_user) }
+
+    context 'when the user is internal' do
+      let(:user) { create(:user, :ghost) }
+
+      it { is_expected.not_to be_allowed(:delete_user) }
+    end
+  end
 end
diff --git a/spec/requests/graphql/mutation/users/delete_spec.rb b/spec/requests/graphql/mutation/users/delete_spec.rb
@@ -31,6 +31,7 @@
   let(:current_user) { create(:user, :admin) }
 
   before do
+    create(:user, :ghost)
     post_graphql mutation, variables: variables, current_user: current_user
   end
 
diff --git a/spec/requests/graphql/query/users_query_spec.rb b/spec/requests/graphql/query/users_query_spec.rb
@@ -19,9 +19,7 @@
   end
 
   before do
-    create(:user)
-    create(:user)
-    create(:user)
+    create_list(:user, 3)
 
     post_graphql query, current_user: current_user
   end
@@ -46,7 +44,10 @@
     let(:current_user) { create(:user, :admin) }
 
     it 'returns all users' do
-      expect(graphql_data_at(:users, :nodes)).to have_attributes(length: 4)
+      expected_ids = User.all.map { |user| user.to_gid.to_s }
+      returned_ids = graphql_data_at(:users, :nodes).pluck('id')
+
+      expect(returned_ids).to match_array(expected_ids)
     end
   end
 end
diff --git a/spec/services/users/delete_service_spec.rb b/spec/services/users/delete_service_spec.rb
@@ -9,6 +9,7 @@
 
   let(:user) { create(:user) }
   let(:current_user) { create(:user, :admin) }
+  let!(:ghost_user) { create(:user, :ghost) }
 
   it 'deletes the user successfully' do
     namespace_id = user.ensure_namespace.id
@@ -31,6 +32,65 @@
     )
   end
 
+  context 'when the user authored audit events' do
+    let!(:audit_event) do
+      create(:audit_event,
+             author: user,
+             action_type: :user_logged_in,
+             entity: user,
+             target: AuditEvent::GLOBAL_TARGET)
+    end
+
+    it 'reassigns authored audit events to the ghost user' do
+      expect { service_response }.to change { User.exists?(user.id) }.from(true).to(false)
+      expect(service_response).to be_success
+      expect(audit_event.reload.author).to eq(ghost_user)
+    end
+  end
+
+  context 'when the user owns a namespace' do
+    let!(:namespace) { user.ensure_namespace }
+    let!(:project) { create(:namespace_project, namespace: namespace) }
+    let!(:flow) { create(:flow, project: project) }
+
+    it 'deletes the owned namespace and associated projects and flows' do
+      expect { service_response }
+        .to change { Namespace.exists?(namespace.id) }.from(true).to(false)
+        .and change { NamespaceProject.exists?(project.id) }.from(true).to(false)
+        .and change { Flow.exists?(flow.id) }.from(true).to(false)
+
+      expect(service_response).to be_success
+    end
+  end
+
+  context 'when deleting the current user' do
+    let(:user) { current_user }
+
+    it 'creates the deletion audit event with the ghost user as author' do
+      expect(service_response).to be_success
+
+      is_expected.to create_audit_event(
+        :user_deleted,
+        author_id: ghost_user.id,
+        entity_type: 'User',
+        entity_id: user.id,
+        details: {},
+        target_type: 'global',
+        target_id: 0
+      )
+    end
+  end
+
+  context 'when deleting the ghost user' do
+    let(:user) { ghost_user }
+
+    it 'returns a missing permission error' do
+      expect(service_response).not_to be_success
+      expect(service_response.payload[:error_code]).to eq(:missing_permission)
+      expect(User.exists?(ghost_user.id)).to be true
+    end
+  end
+
   context 'when current user lacks permission' do
     let(:current_user) { create(:user) }
 
diff --git a/spec/services/users/identity/login_service_spec.rb b/spec/services/users/identity/login_service_spec.rb
@@ -56,6 +56,18 @@ def setup_identity_provider(identity)
         expect(service_response.payload[:error_code]).to eq(:user_blocked)
       end
     end
+
+    context 'when user is ghost' do
+      before do
+        current_user.update!(user_type: :ghost)
+      end
+
+      it do
+        is_expected.not_to create_audit_event
+        expect(service_response).to be_error
+        expect(service_response.payload[:error_code]).to eq(:invalid_login_data)
+      end
+    end
   end
 
   context 'when user identity validation fails' do
diff --git a/spec/services/users/login_service_spec.rb b/spec/services/users/login_service_spec.rb
@@ -67,6 +67,20 @@
       end
     end
 
+    context 'when user is ghost' do
+      let(:params) { { username: username, password: password } }
+
+      before do
+        current_user.update!(user_type: :ghost)
+      end
+
+      it 'returns an error response' do
+        expect(service_response).to be_error
+        expect(service_response.payload[:error_code]).to eq(:invalid_login_data)
+        is_expected.not_to create_audit_event
+      end
+    end
+
     context 'when using mfa' do
       context 'when mfa is not activated' do
         let(:params) do

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+68074a1fb7f8bdf3087c3b6823eaef35175c480f05d4bc73c190ef7b2811cf68`