diff --git a/.env.example b/.env.example index ab43017eba..f7dfc780cd 100755 --- a/.env.example +++ b/.env.example @@ -237,6 +237,13 @@ MIX_PUSHER_APP_CLUSTER="${PUSHER_APP_CLUSTER}" # Whether or not a Project administrator can register a user # PROJECT_ADMIN_REGISTRATION_FORM_ENABLED=true +# The minimum permission level able to register a user. +# Leaving this value blank will disable registration forms, unless a value can be +# generated from USER_REGISTRATION_FORM_ENABLED and PROJECT_ADMIN_REGISTRATION_FORM_ENABLED + +# Options: PUBLIC, PROJECT_ADMIN, ADMIN, DISABLED +# USER_REGISTRATION_ACCESS_LEVEL_REQUIRED=PUBLIC + # Require all new projects to use authenticated submissions. # Instance administrators can override this, and this setting has no effect on # existing projects. diff --git a/app/Enums/RegistrationPermissionsLevel.php b/app/Enums/RegistrationPermissionsLevel.php new file mode 100644 index 0000000000..0a2ef605ef --- /dev/null +++ b/app/Enums/RegistrationPermissionsLevel.php @@ -0,0 +1,11 @@ +admin) { + if ($current_user->can('create', [User::class, EloquentProject::find($projectid)])) { $xml .= add_XML_value('canRegister', '1'); } $xml .= ''; @@ -426,7 +428,7 @@ private static function find_site_maintainers(int $projectid): array private function register_user($projectid, $email, $firstName, $lastName, $repositoryCredential) { - if (config('auth.project_admin_registration_form_enabled') === false) { + if (Gate::authorize('create', [User::class, EloquentProject::findOrFail($projectid)])->denied()) { return 'Users cannot be registered via this form at the current time.'; } diff --git a/app/Policies/UserPolicy.php b/app/Policies/UserPolicy.php new file mode 100644 index 0000000000..41171a5079 --- /dev/null +++ b/app/Policies/UserPolicy.php @@ -0,0 +1,35 @@ +value : ((bool) env('PROJECT_ADMIN_REGISTRATION_FORM_ENABLED', true) ? RegistrationPermissionsLevel::PROJECT_ADMIN->value : RegistrationPermissionsLevel::ADMIN->value); + } + + /** + * Determine whether the user can create models. + */ + public function create(User $user): bool + { + $user_permission_level = Project::whereRelation('administrators', 'users.id', request()->user()?->id)->exists() ? 1 : 0; + $user_permission_level = $user->admin ? 2 : $user_permission_level; + $registration_permission_level_required = match (Str::upper(config('auth.user_registration_access_level_required'))) { + 'PUBLIC' => RegistrationPermissionsLevel::PUBLIC->value, + 'PROJECT_ADMIN' => RegistrationPermissionsLevel::PROJECT_ADMIN->value, + 'ADMIN' => RegistrationPermissionsLevel::ADMIN->value, + 'DISABLED' => RegistrationPermissionsLevel::DISABLED->value, + default => $this->AttemptValueBuild(), + }; + + // Fail if the caller is requesting a value that the setting disallows + return $user_permission_level >= $registration_permission_level_required; + } +} diff --git a/app/cdash/tests/CMakeLists.txt b/app/cdash/tests/CMakeLists.txt index 19950c2fe6..a727eb1881 100644 --- a/app/cdash/tests/CMakeLists.txt +++ b/app/cdash/tests/CMakeLists.txt @@ -214,6 +214,11 @@ add_laravel_test(/Browser/Pages/ProjectSitesPageTest) set_tests_properties(/Browser/Pages/ProjectSitesPageTest PROPERTIES RUN_SERIAL TRUE) set_tests_properties(/Browser/Pages/ProjectSitesPageTest PROPERTIES DEPENDS /CDash/XmlHandler/UpdateHandler) +# Technically this test doesn't have to run serially. It just needs to have exclusive access to the .env +add_laravel_test(/Browser/Pages/RegistrationTest) +set_tests_properties(/Browser/Pages/RegistrationTest PROPERTIES RUN_SERIAL TRUE) +set_tests_properties(/Browser/Pages/RegistrationTest PROPERTIES DEPENDS /CDash/XmlHandler/UpdateHandler) + add_laravel_test(/Feature/PurgeUnusedProjectsCommand) set_tests_properties(/Feature/PurgeUnusedProjectsCommand PROPERTIES RUN_SERIAL TRUE) set_tests_properties(/Feature/PurgeUnusedProjectsCommand PROPERTIES DEPENDS /CDash/XmlHandler/UpdateHandler) diff --git a/config/auth.php b/config/auth.php index 17ebbf1a5a..0d636e0157 100755 --- a/config/auth.php +++ b/config/auth.php @@ -6,10 +6,9 @@ return [ // Whether or not "normal" username+password authentication is enabled 'username_password_authentication_enabled' => env('USERNAME_PASSWORD_AUTHENTICATION_ENABLED', true), - // Whether or not "normal" username+password authentication is enabled - 'user_registration_form_enabled' => env('USER_REGISTRATION_FORM_ENABLED', true), - // Whether or not a Project administrator can register a user - 'project_admin_registration_form_enabled' => env('PROJECT_ADMIN_REGISTRATION_FORM_ENABLED', true), + // Which level of permissions can register a user + // supported: PUBLIC,PROJECT_ADMIN,ADMIN,"" + 'user_registration_access_level_required' => env('USER_REGISTRATION_ACCESS_LEVEL_REQUIRED', (bool) env('USER_REGISTRATION_FORM_ENABLED', true) ? 'PUBLIC' : ((bool) env('PROJECT_ADMIN_REGISTRATION_FORM_ENABLED', true) ? 'PROJECT_ADMIN' : 'ADMIN')), /* |-------------------------------------------------------------------------- diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index a8799cfd1e..a03165fa4d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -3739,6 +3739,16 @@ parameters: count: 2 path: app/Policies/ProjectPolicy.php + - + message: "#^Dynamic call to static method Illuminate\\\\Database\\\\Eloquent\\\\Builder\\\\:\\:exists\\(\\)\\.$#" + count: 1 + path: app/Policies/UserPolicy.php + + - + message: "#^Parameter \\#1 \\$value of static method Illuminate\\\\Support\\\\Str\\:\\:upper\\(\\) expects string, mixed given\\.$#" + count: 1 + path: app/Policies/UserPolicy.php + - message: "#^Dynamic call to static method Illuminate\\\\Foundation\\\\Application\\:\\:isProduction\\(\\)\\.$#" count: 1 diff --git a/resources/views/admin/manage-users.blade.php b/resources/views/admin/manage-users.blade.php index 5e4bee0a6c..2ea77c84f4 100644 --- a/resources/views/admin/manage-users.blade.php +++ b/resources/views/admin/manage-users.blade.php @@ -40,93 +40,95 @@ > - - - - - - - - - - Add new user - - - - - First Name: - - - - - - - - Last Name: - - - - - - - - Email: - - - - - - - - Password: - - - - - - - - - - Confirm Password: - - - - - - - - Institution: - - - - - - - - - - (password will be displayed in clear text upon addition) - - + @can("create", App\Models\User::class) + + + + + + + + + + Add new user + + + + + First Name: + + + + + + + + Last Name: + + + + + + + + Email: + + + + + + + + Password: + + + + + + + + + + Confirm Password: + + + + + + + + Institution: + + + + + + + + + + (password will be displayed in clear text upon addition) + + + @endcan diff --git a/resources/views/components/header.blade.php b/resources/views/components/header.blade.php index 6e1ba51993..725028e3ae 100755 --- a/resources/views/components/header.blade.php +++ b/resources/views/components/header.blade.php @@ -2,9 +2,10 @@ if (isset($project)) { $logoid = $project->ImageId; } -$hideRegistration = config('auth.user_registration_form_enabled') === false; -@endphp + use Illuminate\Support\Str; + $canRegister = Str::upper(config('auth.user_registration_access_level_required')) === "PUBLIC"; +@endphp @@ -18,7 +19,7 @@ Logout @else Login - @if(!$hideRegistration) + @if($canRegister) {{ __('Register') }} @endif @endif diff --git a/tests/Browser/Pages/RegistrationTest.php b/tests/Browser/Pages/RegistrationTest.php new file mode 100644 index 0000000000..7440892b43 --- /dev/null +++ b/tests/Browser/Pages/RegistrationTest.php @@ -0,0 +1,203 @@ + + */ + private array $projects = []; + + private string|bool $original = ''; + private string $path = '../../../../.env'; + + /** + * Stolen from https://laracasts.com/discuss/channels/testing/how-to-change-env-variable-config-in-dusk-test + * + * @param array $variables + */ + protected function override(array $variables = []): void + { + if (file_exists($this->path)) { + // The environment variables to prepend + $prepend = ''; + + // Convert all new parameters to expected format + foreach ($variables as $key => $value) { + $prepend .= $key . '=' . $value . PHP_EOL; + } + + // Grab original .env file contents + $this->original = file_get_contents($this->path); + // Write all to .env file for dusk test + file_put_contents($this->path, $prepend . $this->original); + } + } + + public function tearDown(): void + { + parent::tearDown(); + + foreach ($this->projects as $project) { + $project->delete(); + } + $this->projects = []; + + // Reset the .env file + file_put_contents($this->path, $this->original); + parent::tearDown(); + } + + /** + * @return array> + */ + public static function PublicRegistrationChecks(): array + { + return [ + ['PUBLIC', true], + ['PROJECT_ADMIN', false], + ['ADMIN', false], + ['DISABLED', false], + ]; + } + + /** + * @dataProvider PublicRegistrationChecks + */ + public function testPublicRegistrationForm(string $envVariableValue, bool $shouldFind): void + { + $this->override(['USER_REGISTRATION_ACCESS_LEVEL_REQUIRED' => $envVariableValue]); + $this->browse(function (Browser $browser) use ($shouldFind) { + $browser->refresh()->visit('/projects}') + ->whenAvailable('#topmenu', function (Browser $browser) use ($shouldFind) { + $shouldFind ? $browser->assertSee('Register') : $browser->assertDontSee('Register'); + }); + }); + } + + /** + * @return array> + */ + public static function ProjectManagerChecksAsAdministrator(): array + { + return [ + ['PUBLIC', true], + ['PROJECT_ADMIN', true], + ['ADMIN', true], + ['DISABLED', false], + ]; + } + + public function visitProjectManageUsersForm(string $envVariableValue, bool $shouldFind, User $user): void + { + $this->override(['USER_REGISTRATION_ACCESS_LEVEL_REQUIRED' => $envVariableValue]); + $this->browse(function (Browser $browser) use ($shouldFind, $user) { + $browser->loginAs((string) $user->id) + ->visit("/manageProjectRoles.php?projectid={$this->projects['public1']->id}") + ->refresh() + ->whenAvailable(' #wizard', function (Browser $browser) use ($shouldFind) { + $shouldFind ? $browser->assertPresent('#fragment-3') : $browser->assertNotPresent('#fragment-3'); + }); + }); + } + + /** + * @dataProvider ProjectManagerChecksAsAdministrator + */ + public function testProjectManageUsersFormAsSiteAdministrator(string $envVariableValue, bool $shouldFind): void + { + $user = $this->makeAdminUser(); + $this->projects['public1'] = $this->makePublicProject(); + $this->projects['public1']->description = Str::uuid()->toString(); + $this->projects['public1']->save(); + + $this->visitProjectManageUsersForm($envVariableValue, $shouldFind, $user); + } + /* + public function ProjectManagerChecksAsProjectAdministrator() + { + return [ + ["PUBLIC", true], + ["PROJECT_ADMIN", true], + ["ADMIN", false], + ["DISABLED", false], + ]; + } + + /** + * @dataProvider ProjectManagerChecksAsProjectAdministrator + + + public function testProjectManageUsersFormAsProjectAdminstrator(string $envVariableValue, bool $shouldFind): void + { + $user = $this->makeNormalUser(); + $this->projects['public1'] = $this->makePublicProject(); + $this->projects['public1']->description = Str::uuid()->toString(); + $this->projects['public1']->save(); + // Add the user to project as admin. + DB::table('user2project')->insert([ + 'userid' => $user->id, + 'projectid' => $this->projects['public1']->id, + 'role' => 0, + 'emailtype' => 0, + 'emailcategory' => 0, + 'emailsuccess' => 0, + 'emailmissingsites' => 0, + ]); + print(DB::table('user2project')->find(['userid' => $user->id])->toSql()); + $this->visitProjectManageUsersForm($envVariableValue, $shouldFind, $user); + + $user2project = DB::table('user2project')->find(['userid' => $user->id])->id; + if($user2project > 0) { + DB::table('user2project')->delete($user2project); + } + } + */ + + /** + * @return array> + */ + public static function ManageUsersChecks(): array + { + return [ + ['PUBLIC', true], + ['PROJECT_ADMIN', true], + ['ADMIN', true], + ['DISABLED', false], + ]; + } + + public function visitManageUsersForm(string $envVariableValue, bool $shouldFind, User $user): void + { + $this->override(['USER_REGISTRATION_ACCESS_LEVEL_REQUIRED' => $envVariableValue]); + $this->browse(function (Browser $browser) use ($shouldFind, $user) { + $browser->loginAs((string) $user->id) + ->visit('/manageUsers.php') + ->refresh() + ->whenAvailable('', function (Browser $browser) use ($shouldFind) { + $shouldFind ? $browser->assertSee('Add new user') : $browser->assertDontSee('Add new user'); + }); + }); + } + + /** + * @dataProvider ProjectManagerChecksAsAdministrator + */ + public function testManageUsersForm(string $envVariableValue, bool $shouldFind): void + { + $user = $this->makeAdminUser(); + $this->visitManageUsersForm($envVariableValue, $shouldFind, $user); + } +}