From a48cd194db893f81ff3d73ede31ccd204dc7b8d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 26 Nov 2024 12:59:06 +0100 Subject: [PATCH] Lock user instead --- app/seeders/admin_user_seeder.rb | 22 ++++++------------- app/seeders/root_seeder.rb | 5 +++++ app/seeders/seeder.rb | 2 +- config/constants/settings/definition.rb | 8 +++---- .../configuration/README.md | 6 +++-- .../seeders/root_seeder_bim_edition_spec.rb | 21 ++++++------------ spec/seeders/admin_user_seeder_spec.rb | 13 +---------- .../root_seeder_standard_edition_spec.rb | 19 +++++----------- spec/seeders/seeder_spec.rb | 10 ++++----- 9 files changed, 39 insertions(+), 67 deletions(-) diff --git a/app/seeders/admin_user_seeder.rb b/app/seeders/admin_user_seeder.rb index f0ff1d4514fa..861e5e3fa7e9 100644 --- a/app/seeders/admin_user_seeder.rb +++ b/app/seeders/admin_user_seeder.rb @@ -27,10 +27,14 @@ #++ class AdminUserSeeder < Seeder def seed_data! - if Setting.seed_admin_user_enabled? - seed_admin! + user = new_admin + if user.save!(validate: false) + seed_data.store_reference(:openproject_admin, user) else - Seeder.logger.debug { " *** skipped as explicity disabled with OPENPROJECT_SEED_ADMIN_USER_ENABLED" } + print_error "Seeding admin failed:" + user.errors.full_messages.each do |msg| + print_error " #{msg}" + end end end @@ -46,18 +50,6 @@ def not_applicable_message "No need to seed an admin as there already is one." end - def seed_admin! - user = new_admin - if user.save!(validate: false) - seed_data.store_reference(:openproject_admin, user) - else - print_error "Seeding admin failed:" - user.errors.full_messages.each do |msg| - print_error " #{msg}" - end - end - end - def new_admin # rubocop:disable Metrics/AbcSize User.new.tap do |user| user.admin = true diff --git a/app/seeders/root_seeder.rb b/app/seeders/root_seeder.rb index f1a150e4723f..0958652ca40c 100644 --- a/app/seeders/root_seeder.rb +++ b/app/seeders/root_seeder.rb @@ -75,6 +75,11 @@ def do_seed! seed_development_data if seed_development_data? seed_plugins_data seed_env_data + cleanup_seed_data + end + + def cleanup_seed_data + admin_user.lock! if Setting.seed_admin_user_locked? end def seed_development_data? diff --git a/app/seeders/seeder.rb b/app/seeders/seeder.rb index ddda5866f47a..a9438416a45b 100644 --- a/app/seeders/seeder.rb +++ b/app/seeders/seeder.rb @@ -80,7 +80,7 @@ def not_applicable_message # The user being the author of all data created during seeding. def admin_user - @admin_user ||= (User.not_builtin.admin.first || User.system) + @admin_user ||= User.not_builtin.admin.first end protected diff --git a/config/constants/settings/definition.rb b/config/constants/settings/definition.rb index c3f71f87ed2f..8c7c30a1bd65 100644 --- a/config/constants/settings/definition.rb +++ b/config/constants/settings/definition.rb @@ -923,10 +923,10 @@ class Definition default: "https://releases.openproject.com/v1/check.svg", writable: false }, - seed_admin_user_enabled: { - description: "Enable creating the admin user on first startup. " \ - "If set to false, an admin user has to be created manually.", - default: true, + seed_admin_user_locked: { + description: "Lock the created admin user after seeding, so it can not be used for logging in. " \ + "If set to true, an admin user has to be created manually or through an SSO provider.", + default: false, writable: false }, seed_admin_user_password: { diff --git a/docs/installation-and-operations/configuration/README.md b/docs/installation-and-operations/configuration/README.md index 920cc26e29ae..8ff6d3f58672 100644 --- a/docs/installation-and-operations/configuration/README.md +++ b/docs/installation-and-operations/configuration/README.md @@ -173,14 +173,16 @@ OPENPROJECT_SEED_ADMIN_USER_NAME="OpenProject Admin" # Name to assign to that us OPENPROJECT_SEED_ADMIN_USER_MAIL="admin@example.net" # Email attribute to assign to that user. Note that in packaged installations, a wizard step will assign this variable as well. ``` -Optionally, you can also completely disable the creation of such a user. +Optionally, you can also lock the admin user that gets created right away. This is useful when you have an LDAP or SSO integration set up and you want to prevent the admin user from logging in. + +```shell > [!WARNING] > With the admin user seeding disabled, you need to have an LDAP or SSO integration set up through environment variables. > Otherwise, you will not be able to retain access to the system. ```shell -OPENPROJECT_SEED_ADMIN_USER_DISABLED="true" +OPENPROJECT_SEED_ADMIN_USER_LOCKED="true" ``` ### Seeding LDAP connections diff --git a/modules/bim/spec/seeders/root_seeder_bim_edition_spec.rb b/modules/bim/spec/seeders/root_seeder_bim_edition_spec.rb index d54b919a6832..92a37c492e51 100644 --- a/modules/bim/spec/seeders/root_seeder_bim_edition_spec.rb +++ b/modules/bim/spec/seeders/root_seeder_bim_edition_spec.rb @@ -250,33 +250,26 @@ def group_name(reference) include_examples "no email deliveries" end - context "when admin user creation is disabled with OPENPROJECT_SEED_ADMIN_USER_ENABLED=false", + context "when admin user creation is locked with OPENPROJECT_SEED_ADMIN_USER_LOCKED=true", :settings_reset do shared_let(:root_seeder) { described_class.new } before_all do - with_env("OPENPROJECT_SEED_ADMIN_USER_ENABLED" => "false") do - with_edition("bim") do - reset(:seed_admin_user_enabled) + with_env("OPENPROJECT_SEED_ADMIN_USER_LOCKED" => "true") do + with_edition("standard") do + reset(:seed_admin_user_locked) root_seeder.seed_data! end end ensure - reset(:seed_admin_user_enabled) + reset(:seed_admin_user_locked) RequestStore.clear! # resets `User.current` cached result end - it "creates the system user" do - expect(SystemUser.where(admin: true).count).to eq 1 - end - - it "does not create an admin user" do - expect(User.not_builtin.where(admin: true).count).to eq 0 - end - - it "seeds without any errors" do + it "seeds without any errors, but locks the admin user", :aggregate_failures do expect(Project.count).to eq 4 expect(WorkPackage.count).to eq 76 + expect(root_seeder.admin_user).to be_locked end end end diff --git a/spec/seeders/admin_user_seeder_spec.rb b/spec/seeders/admin_user_seeder_spec.rb index 2f567b4216ac..db1f4d1de5ef 100644 --- a/spec/seeders/admin_user_seeder_spec.rb +++ b/spec/seeders/admin_user_seeder_spec.rb @@ -39,18 +39,6 @@ expect { seeder.seed! }.to change { User.admin.count }.by(1) end - context "when skipped with OPENPROJECT_SEED_ADMIN_USER_ENABLED=false", - :settings_reset, - with_env: { - OPENPROJECT_SEED_ADMIN_USER_ENABLED: "false" - } do - it "skips the creation" do - reset(:seed_admin_user_enabled) - - expect { seeder.seed! }.not_to change { User.admin.count } - end - end - context "when providing admin user seed variables", :settings_reset, with_env: { @@ -68,6 +56,7 @@ seeder.seed! admin = User.admin.last + expect(admin).to be_active expect(admin.firstname).to eq "foo" expect(admin.lastname).to eq "bar" expect(admin.mail).to eq "foobar@example.com" diff --git a/spec/seeders/root_seeder_standard_edition_spec.rb b/spec/seeders/root_seeder_standard_edition_spec.rb index b34d5ed7c88e..9f416a03afe6 100644 --- a/spec/seeders/root_seeder_standard_edition_spec.rb +++ b/spec/seeders/root_seeder_standard_edition_spec.rb @@ -287,33 +287,26 @@ include_examples "no email deliveries" end - context "when admin user creation is disabled with OPENPROJECT_SEED_ADMIN_USER_ENABLED=false", + context "when admin user creation is locked with OPENPROJECT_SEED_ADMIN_USER_LOCKED=true", :settings_reset do shared_let(:root_seeder) { described_class.new } before_all do - with_env("OPENPROJECT_SEED_ADMIN_USER_ENABLED" => "false") do + with_env("OPENPROJECT_SEED_ADMIN_USER_LOCKED" => "true") do with_edition("standard") do - reset(:seed_admin_user_enabled) + reset(:seed_admin_user_locked) root_seeder.seed_data! end end ensure - reset(:seed_admin_user_enabled) + reset(:seed_admin_user_locked) RequestStore.clear! # resets `User.current` cached result end - it "creates the system user" do - expect(SystemUser.where(admin: true).count).to eq 1 - end - - it "does not create an admin user" do - expect(User.not_builtin.where(admin: true).count).to eq 0 - end - - it "seeds without any errors" do + it "seeds without any errors, but locks the admin user", :aggregate_failures do expect(Project.count).to eq 2 expect(WorkPackage.count).to eq 36 + expect(root_seeder.admin_user).to be_locked end end end diff --git a/spec/seeders/seeder_spec.rb b/spec/seeders/seeder_spec.rb index bd7a5003827a..2c9f0236abb1 100644 --- a/spec/seeders/seeder_spec.rb +++ b/spec/seeders/seeder_spec.rb @@ -37,16 +37,14 @@ describe "#admin_user" do it "returns the admin created from the seeding" do - expect(seeder.admin_user).to eq(User.system) - expect { AdminUserSeeder.new(seed_data).seed! }.to change { User.admin.count }.by(1) - expect(seeder.admin_user).to eq(User.system) - - expect(described_class.new.admin_user).to eq(User.not_builtin.admin.first) + expect(seeder.admin_user).to be_nil + AdminUserSeeder.new(seed_data).seed! + expect(seeder.admin_user).to be_a(User) end it "does not return the system user" do expect { User.system }.to change { User.admin.count }.by(1) - expect(seeder.admin_user).to eq(User.system) + expect(seeder.admin_user).to be_nil end end end