From 51c14227b25340388db1f922156a2879910f8066 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Mon, 25 Jan 2010 05:18:17 +0000 Subject: [PATCH] API CHANGE Security::setDefaultAdmin() no longer writes credentials to any Member database records (created through Security::findAnAdministrator(). This prevents outdated credentials when setDefaultAdmin() code changes after creating the database record (see #4271) API CHANGE Security::findAnAdministrator() no longer sets 'Email' and 'Password' properties on newly created members. Removed the $username and $password argments from the method. ENHANCEMENT Member->requireDefaultRecords() no longer creates a default administrator based on $_REQUEST data. Moved functionality into Installer->install() MINOR Security::findAnAdministrator() names any default administrators 'Default Admin' instead of 'Admin' git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.4@97478 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- security/Member.php | 9 ---- security/Security.php | 29 ++++++++----- tests/security/SecurityDefaultAdminTest.php | 48 +++++++++++++++++++++ 3 files changed, 66 insertions(+), 20 deletions(-) create mode 100644 tests/security/SecurityDefaultAdminTest.php diff --git a/security/Member.php b/security/Member.php index 1b16707e7..a2718a24e 100755 --- a/security/Member.php +++ b/security/Member.php @@ -975,15 +975,6 @@ class Member extends DataObject { } return $labels; } - - function requireDefaultRecords() { - parent::requireDefaultRecords(); - - if(!DB::query("SELECT * FROM \"Member\"")->value() && isset($_REQUEST['username']) && isset($_REQUEST['password'])) { - Security::findAnAdministrator($_REQUEST['username'], $_REQUEST['password']); - DB::alteration_message("Added admin account","created"); - } - } /** * Users can view their own record. diff --git a/security/Security.php b/security/Security.php index e5c06f02f..2f2e5b660 100644 --- a/security/Security.php +++ b/security/Security.php @@ -603,12 +603,19 @@ class Security extends Controller { /** - * Return a member with administrator privileges - * - * @return Member Returns a member object that has administrator - * privileges. + * Return an existing member with administrator privileges, or create one of necessary. + * + * Will create a default 'Administrators' group if no group is found + * with an ADMIN permission. Will create a new 'Admin' member with administrative permissions + * if no existing Member with these permissions is found. + * + * Important: Any newly created administrator accounts will NOT have valid + * login credentials (Email/Password properties), which means they can't be used for login + * purposes outside of any default credentials set through {@link Security::setDefaultAdmin()}. + * + * @return Member */ - static function findAnAdministrator($username = 'admin', $password = 'password') { + static function findAnAdministrator() { // coupling to subsites module $subsiteCheck = class_exists('GroupSubsites') ? ' AND "Group"."SubsiteID" = 0' : ''; @@ -635,10 +642,10 @@ class Security extends Controller { } if(!isset($member)) { + // Leave 'Email' and 'Password' are not set to avoid creating + // persistent logins in the database. See Security::setDefaultAdmin(). $member = Object::create('Member'); - $member->FirstName = $member->Surname = 'Admin'; - $member->Email = $username; - $member->Password = $password; + $member->FirstName = 'Default Admin'; $member->write(); $member->Groups()->add($adminGroup); } @@ -650,13 +657,13 @@ class Security extends Controller { /** * Set a default admin in dev-mode * - * This will set a static default-admin (e.g. "td") which is not existing + * This will set a static default-admin which is not existing * as a database-record. By this workaround we can test pages in dev-mode * with a unified login. Submitted login-credentials are first checked - * against this static information in {@authenticate()}. + * against this static information in {@link Security::authenticate()}. * * @param string $username The user name - * @param string $password The password in cleartext + * @param string $password The password (in cleartext) */ public static function setDefaultAdmin($username, $password) { // don't overwrite if already set diff --git a/tests/security/SecurityDefaultAdminTest.php b/tests/security/SecurityDefaultAdminTest.php new file mode 100644 index 000000000..f8015d9f4 --- /dev/null +++ b/tests/security/SecurityDefaultAdminTest.php @@ -0,0 +1,48 @@ +assertTrue(Security::has_default_admin()); + $this->assertTrue( + Security::check_default_admin('admin', 'password'), + 'Succeeds with correct username and password' + ); + $this->assertFalse( + Security::check_default_admin('wronguser', 'password'), + 'Fails with incorrect username' + ); + $this->assertFalse( + Security::check_default_admin('admin', 'wrongpassword'), + 'Fails with incorrect password' + ); + + Security::setDefaultAdmin(null, null); + } + + function testFindAnAdministratorCreatesNewUser() { + $adminMembers = Permission::get_members_by_permission('ADMIN'); + $this->assertFalse($adminMembers); + + $admin = Security::findAnAdministrator(); + + $this->assertType('Member', $admin); + $this->assertTrue(Permission::checkMember($admin, 'ADMIN')); + $this->assertNull($admin->Email); + $this->assertNull($admin->Password); + } + +} \ No newline at end of file