From 83a47791a56e1c5c5712daef634f56a17278ba69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 25 Nov 2024 14:37:09 +0100 Subject: [PATCH 1/6] Disable seeding of admin user --- app/seeders/admin_user_seeder.rb | 22 +++++++++++++------ config/constants/settings/definition.rb | 6 +++++ .../configuration/README.md | 10 +++++++++ spec/seeders/admin_user_seeder_spec.rb | 12 ++++++++++ 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/app/seeders/admin_user_seeder.rb b/app/seeders/admin_user_seeder.rb index 861e5e3fa7e9..f0ff1d4514fa 100644 --- a/app/seeders/admin_user_seeder.rb +++ b/app/seeders/admin_user_seeder.rb @@ -27,14 +27,10 @@ #++ class AdminUserSeeder < Seeder def seed_data! - user = new_admin - if user.save!(validate: false) - seed_data.store_reference(:openproject_admin, user) + if Setting.seed_admin_user_enabled? + seed_admin! else - print_error "Seeding admin failed:" - user.errors.full_messages.each do |msg| - print_error " #{msg}" - end + Seeder.logger.debug { " *** skipped as explicity disabled with OPENPROJECT_SEED_ADMIN_USER_ENABLED" } end end @@ -50,6 +46,18 @@ 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/config/constants/settings/definition.rb b/config/constants/settings/definition.rb index 16fe9e0705f2..c3f71f87ed2f 100644 --- a/config/constants/settings/definition.rb +++ b/config/constants/settings/definition.rb @@ -923,6 +923,12 @@ 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, + writable: false + }, seed_admin_user_password: { description: 'Password to set for the initially created admin user (Login remains "admin").', default: "admin", diff --git a/docs/installation-and-operations/configuration/README.md b/docs/installation-and-operations/configuration/README.md index 839ec412458f..920cc26e29ae 100644 --- a/docs/installation-and-operations/configuration/README.md +++ b/docs/installation-and-operations/configuration/README.md @@ -173,6 +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. + +> [!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" +``` + ### Seeding LDAP connections OpenProject allows you to create and maintain an LDAP connection with optional synchronized group filters. This is relevant for e.g., automated deployments, where you want to trigger the synchronization right at the start. diff --git a/spec/seeders/admin_user_seeder_spec.rb b/spec/seeders/admin_user_seeder_spec.rb index 0dcb79608102..2f567b4216ac 100644 --- a/spec/seeders/admin_user_seeder_spec.rb +++ b/spec/seeders/admin_user_seeder_spec.rb @@ -39,6 +39,18 @@ 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: { From e9cfa0b6e40c0cab659297afab9f3b4c6521c6dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 26 Nov 2024 10:00:16 +0100 Subject: [PATCH 2/6] Use the system user when no admin is present --- app/seeders/seeder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/seeders/seeder.rb b/app/seeders/seeder.rb index a9438416a45b..ddda5866f47a 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 + @admin_user ||= (User.not_builtin.admin.first || User.system) end protected From ff8ad6dc36a09ea8384028a6e5a07a61e9043660 Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Tue, 26 Nov 2024 10:23:06 +0100 Subject: [PATCH 3/6] Add failing specs for seed_admin_user_enabled=false --- .../seeders/root_seeder_bim_edition_spec.rb | 27 +++++++++++++++++++ .../root_seeder_standard_edition_spec.rb | 27 +++++++++++++++++++ 2 files changed, 54 insertions(+) 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 1e4870b4a904..9d5328d47262 100644 --- a/modules/bim/spec/seeders/root_seeder_bim_edition_spec.rb +++ b/modules/bim/spec/seeders/root_seeder_bim_edition_spec.rb @@ -249,4 +249,31 @@ def group_name(reference) include_examples "no email deliveries" end + + context "when admin user creation is disabled with OPENPROJECT_SEED_ADMIN_USER_ENABLED=false", + :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) + root_seeder.seed_data! + end + end + 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 + expect(Project.count).to eq 4 + expect(WorkPackage.count).to eq 76 + end + end end diff --git a/spec/seeders/root_seeder_standard_edition_spec.rb b/spec/seeders/root_seeder_standard_edition_spec.rb index af0561b82d96..ce2f6d450fcf 100644 --- a/spec/seeders/root_seeder_standard_edition_spec.rb +++ b/spec/seeders/root_seeder_standard_edition_spec.rb @@ -286,4 +286,31 @@ include_examples "no email deliveries" end + + context "when admin user creation is disabled with OPENPROJECT_SEED_ADMIN_USER_ENABLED=false", + :settings_reset do + shared_let(:root_seeder) { described_class.new } + + before_all do + with_env("OPENPROJECT_SEED_ADMIN_USER_ENABLED" => "false") do + with_edition("standard") do + reset(:seed_admin_user_enabled) + root_seeder.seed_data! + end + end + 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 + expect(Project.count).to eq 2 + expect(WorkPackage.count).to eq 36 + end + end end From 04bd5847724712aaeb89d187720a6bb4455183e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 26 Nov 2024 10:00:16 +0100 Subject: [PATCH 4/6] Use the system user when no admin is present --- spec/seeders/seeder_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/seeders/seeder_spec.rb b/spec/seeders/seeder_spec.rb index 2c9f0236abb1..bd7a5003827a 100644 --- a/spec/seeders/seeder_spec.rb +++ b/spec/seeders/seeder_spec.rb @@ -37,14 +37,16 @@ describe "#admin_user" do it "returns the admin created from the seeding" do - expect(seeder.admin_user).to be_nil - AdminUserSeeder.new(seed_data).seed! - expect(seeder.admin_user).to be_a(User) + 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) end it "does not return the system user" do expect { User.system }.to change { User.admin.count }.by(1) - expect(seeder.admin_user).to be_nil + expect(seeder.admin_user).to eq(User.system) end end end From 743f62d74b558fad97a41ffd029762247a7b3bc6 Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Tue, 26 Nov 2024 12:04:49 +0100 Subject: [PATCH 5/6] Fix test leaving state after being run --- modules/bim/spec/seeders/root_seeder_bim_edition_spec.rb | 3 +++ spec/seeders/root_seeder_standard_edition_spec.rb | 3 +++ 2 files changed, 6 insertions(+) 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 9d5328d47262..d54b919a6832 100644 --- a/modules/bim/spec/seeders/root_seeder_bim_edition_spec.rb +++ b/modules/bim/spec/seeders/root_seeder_bim_edition_spec.rb @@ -261,6 +261,9 @@ def group_name(reference) root_seeder.seed_data! end end + ensure + reset(:seed_admin_user_enabled) + RequestStore.clear! # resets `User.current` cached result end it "creates the system user" do diff --git a/spec/seeders/root_seeder_standard_edition_spec.rb b/spec/seeders/root_seeder_standard_edition_spec.rb index ce2f6d450fcf..b34d5ed7c88e 100644 --- a/spec/seeders/root_seeder_standard_edition_spec.rb +++ b/spec/seeders/root_seeder_standard_edition_spec.rb @@ -298,6 +298,9 @@ root_seeder.seed_data! end end + ensure + reset(:seed_admin_user_enabled) + RequestStore.clear! # resets `User.current` cached result end it "creates the system user" do From 834d8c51de6492c3ffe0343a9573c44c45f03725 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 6/6] 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 | 19 +++++----------- spec/seeders/admin_user_seeder_spec.rb | 13 +---------- .../root_seeder_standard_edition_spec.rb | 19 +++++----------- spec/seeders/seeder_spec.rb | 10 ++++----- 9 files changed, 38 insertions(+), 66 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..892c1ce4ebce 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_env("OPENPROJECT_SEED_ADMIN_USER_LOCKED" => "true") do with_edition("bim") 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 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