Issue 6

appinfo/database.xml

@@ -19,7 +19,7 @@
     <type>text</type>
     <notnull>true</notnull>
     <length>64</length>
-    <default></default>
+    <default/>
 <!--    <description>SHA256 hash of the user's salted PersistentID.</description>-->
    </field>
 
@@ -28,7 +28,7 @@
     <type>text</type>
     <notnull>true</notnull>
     <length>64</length>
-    <default></default>
+    <default/>
 <!--    <description>The user's email address (cropped if neccessary).</description>-->
    </field>
 
@@ -37,7 +37,32 @@
     <type>text</type>
     <notnull>true</notnull>
     <length>128</length>
-    <default></default>
+    <default/>
+   </field>
+
+   <!-- Persistent ID -->
+   <field>
+    <name>pid</name>
+    <type>text</type>
+    <notnull>true</notnull>
+    <length>128</length>
+    <default/>
+   </field>
+
+   <!-- Timestamp containing creation time -->
+   <field>
+    <name>created_on</name>
+    <type>datetime</type>
+    <notnull>true</notnull>
+    <default/>
+   </field>
+
+   <!-- Timestamp containing deletion time -->
+   <field>
+    <name>deleted_on</name>
+    <type>datetime</type>
+    <notnull>false</notnull>
+    <default/>
    </field>
 
    <index>
@@ -50,10 +75,10 @@
    </index>
 
    <index>
-    <name>shibboleth_display_name_index</name>
+    <name>shibboleth_pid_index</name>
     <unique>true</unique>
     <field>
-     <name>display_name</name>
+     <name>pid</name>
      <sorting>ascending</sorting>
     </field>
    </index>

database/db.php

@@ -22,20 +22,23 @@
 
 class DB {
 
-	public static function loginNameExists($loginName) {
-		$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT COUNT(*) FROM *PREFIX*shibboleth_user WHERE login_name = ?');
+	public static function loginNameExists($loginName, $showDeleted = false) {
+		$sql = 'SELECT COUNT(*) FROM *PREFIX*shibboleth_user WHERE login_name = ?';
+		if(!$showDeleted) {
+			$sql .= ' AND deleted_on IS NULL';
+		}
+		$stmt = \OC::$server->getDatabaseConnection()->prepare($sql);
 		$result = $stmt->execute(array($loginName));
 
-		if ($result !== false) {
+		if (!($result === false)) {
 			$count = $stmt->fetchAll(\PDO::FETCH_COLUMN);
 			if(is_array($count)) {
-				return intval($count[0]) === 1;//not all PHP/DBS combinations return result of type integer
+				return intval($count[0]) === 1;
 			}
 		}
 		return false;
 	}
 
-
 	public static function getHomeDir($loginName) {
 		$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT home_dir FROM *PREFIX*shibboleth_user WHERE login_name = ?');
 		$result = $stmt->execute(array($loginName));
@@ -48,8 +51,7 @@ public static function getHomeDir($loginName) {
 		return false;
 	}
 
-
-	public static function getLoginNames($partialLoginName, $limit, $offset) {//was getUsers
+	public static function getLoginNames($partialLoginName, $limit, $offset) {
 
 		if ($limit === 0) {
 			$limit = '0';
@@ -59,11 +61,11 @@ public static function getLoginNames($partialLoginName, $limit, $offset) {//was
 		}
 
 		if (strlen($partialLoginName) === 0) {
-			$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT login_name FROM *PREFIX*shibboleth_user', $limit, $offset); // LIMIT ? OFFSET ?');
+			$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT login_name FROM *PREFIX*shibboleth_user WHERE deleted_on IS NULL', $limit, $offset); // LIMIT ? OFFSET ?');
 			$result = $stmt->execute();
 		} else {
 			$partialLoginName = '%'.$partialLoginName.'%';
-			$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT login_name FROM *PREFIX*shibboleth_user WHERE login_name LIKE ?',$limit, $offset); // LIMIT ? OFFSET ?');
+			$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT login_name FROM *PREFIX*shibboleth_user WHERE login_name LIKE ? AND deleted_on IS NULL',$limit, $offset); // LIMIT ? OFFSET ?');
 			$result = $stmt->execute(array($partialLoginName));
 		}
 
@@ -74,7 +76,7 @@ public static function getLoginNames($partialLoginName, $limit, $offset) {//was
 	}
 
 	public static function getDisplayName($loginName) {
-		$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT display_name FROM *PREFIX*shibboleth_user WHERE login_name = ?');
+		$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT display_name FROM *PREFIX*shibboleth_user WHERE login_name = ? AND deleted_on IS NULL');
 		$result = $stmt->execute(array($loginName));
 
 		if ($result !== false) {
@@ -96,12 +98,12 @@ public static function getDisplayNames($partialDisplayName, $limit, $offset=0) {
 		}
 
 		if (strlen($partialDisplayName) === 0) {
-			$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT `login_name`, `display_name` FROM *PREFIX*shibboleth_user', $limit, $offset);
+			$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT `login_name`, `display_name` FROM *PREFIX*shibboleth_user WHERE deleted_on IS NULL', $limit, $offset);
 			$result = $stmt->execute();
 		}
 		 else {
 			$partialDisplayName = '%'.$partialDisplayName.'%';
-			$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT login_name, display_name FROM *PREFIX*shibboleth_user WHERE display_name LIKE ?',$limit, $offset);
+			$stmt = \OC::$server->getDatabaseConnection()->prepare('SELECT login_name, display_name FROM *PREFIX*shibboleth_user WHERE display_name LIKE ? AND deleted_on IS NULL',$limit, $offset);
 			$result = $stmt->execute(array($partialDisplayName));
 		}
 
@@ -118,21 +120,30 @@ public static function getDisplayNames($partialDisplayName, $limit, $offset=0) {
 		return false;
 	}
 
-	public static function addUser($loginName, $displayName, $homeDir) {
-		$stmt = \OC::$server->getDatabaseConnection()->prepare('INSERT INTO *PREFIX*shibboleth_user values(?, ?, ?)');
-		$result = $stmt->execute(array($loginName, $displayName, $homeDir));
-		return $result !== false;
+	public static function addUser($loginName, $displayName, $homeDir, $pid) {
+		if (self::loginNameExists($loginName, true)) {
+			\OCP\Util::writeLog(APP_NAME, "re-adding user: $loginName", \OCP\Util::INFO);
+			$stmt = \OC::$server->getDatabaseConnection()->prepare('UPDATE *PREFIX*shibboleth_user SET deleted_on = NULL WHERE login_name = ?');
+			$result = $stmt->execute(array($loginName));
+		} else {
+			\OCP\Util::writeLog(APP_NAME, "adding user: $loginName", \OCP\Util::INFO);
+			$stmt = \OC::$server->getDatabaseConnection()->prepare('INSERT INTO *PREFIX*shibboleth_user values(?, ?, ?, ?, datetime("now"), NULL)');
+			$result = $stmt->execute(array($loginName, $displayName, $homeDir, $pid));
+		}
+		return !($result === false);
 	}
 
 	public static function updateDisplayName($loginName, $displayName) {
-		$stmt = \OC::$server->getDatabaseConnection()->prepare('UPDATE *PREFIX*shibboleth_user SET display_name = ? WHERE login_name = ?');
+		\OCP\Util::writeLog(APP_NAME, "renaming user: $loginName -> $displayName", \OCP\Util::INFO);
+		$stmt = \OC::$server->getDatabaseConnection()->prepare('UPDATE *PREFIX*shibboleth_user SET display_name = ? WHERE login_name = ? AND deleted_on IS NULL');
 		$result = $stmt->execute(array($displayName, $loginName));
-		return $result !== false;
+		return !($result === false);
 	}
 
 	public static function deleteUser($loginName) {
-		$stmt = \OC::$server->getDatabaseConnection()->prepare('DELETE FROM *PREFIX*shibboleth_user WHERE login_name = ?');
+		\OCP\Util::writeLog(APP_NAME, "deleting user: $loginName", \OCP\Util::INFO);
+		$stmt = \OC::$server->getDatabaseConnection()->prepare('UPDATE *PREFIX*shibboleth_user SET deleted_on = datetime("now") WHERE login_name = ?');
 		$result = $stmt->execute(array($loginName));
-		return $result !== false;
+		return !self::loginNameExists($loginName);
 	}
 }

lib/auth.php

@@ -1,7 +1,7 @@
 <?php
 /**
  * ownCloud - user_shibboleth
- * 
+ *
  * Copyright (C) 2013 Andreas Ergenzinger [email protected]
  *
  * This library is free software: you can redistribute it and/or modify
@@ -17,9 +17,7 @@
  * You should have received a copy of the GNU General Public License
  * along with this library. If not, see <http://www.gnu.org/licenses/>.
  */
-
 namespace OCA\user_shibboleth;
-
 class Auth {
 	private static function getAttribute($name) {
 		$attributeName = \OC::$server->getConfig()->getAppValue('user_shibboleth', $name, '');

login.php

@@ -79,7 +79,7 @@
 			} else {
 				// Create a new user account
 				$homeDir = \OCA\user_shibboleth\LoginLib::getHomeDirPath($loginName);
-				\OCA\user_shibboleth\DB::addUser($loginName, $displayName, $homeDir);
+				\OCA\user_shibboleth\DB::addUser($loginName, $displayName, $homeDir, OCA\user_shibboleth\Auth::getPersistentId());
 				// Set email
 				\OC::$server->getConfig()->setUserValue($loginName, 'settings', 'email', $mail);
 			}

tests/DBTest.php

@@ -11,79 +11,116 @@ class DBTest extends PHPUnit_Framework_TestCase {
 	private static $userZ;
 
 	private static function cleanUpDatabase() {
-		DB::deleteUser(self::$userX['LoginName']);
-		DB::deleteUser(self::$userY['LoginName']);
-		DB::deleteUser(self::$userZ['LoginName']);
+		$query = \OC::$server->getDatabaseConnection()->prepare('DELETE FROM *PREFIX*shibboleth_user');
+		$query->execute();
 	}
 
 	public static function setUpBeforeClass() {
-		self::$userX = array('LoginName' => 'MisterX',
-			'DisplayName' => '[email protected]',
+		self::$userX = array(
+			'LoginName' => 'MisterX',
+			'DisplayName' => 'Mister X',
+			'PID' => '[email protected]',
 			'HomeDir' => '/dev/null/shibboleth/MisterX');
-		self::$userY = array('LoginName' => 'MisterY',
-			'DisplayName' => '[email protected]',
+		self::$userY = array(
+			'LoginName' => 'MisterY',
+			'DisplayName' => 'Mister Y',
+			'PID' => '[email protected]',
 			'HomeDir' => '/dev/null/shibboleth/MisterY');
-		self::$userZ = array('LoginName' => 'MisterZ',
-			'DisplayName' => '[email protected]',
+		self::$userZ = array(
+			'LoginName' => 'MisterZ',
+			'DisplayName' => 'Mister Z',
+			'PID' => '[email protected]',
 			'HomeDir' => '/dev/null/shibboleth/MisterZ');
-		self::cleanUpDatabase();//in case tearDownAfter was not called due to error
-		DB::addUser(self::$userX['LoginName'], self::$userX['DisplayName'], self::$userX['HomeDir']);
+
+		// In case tearDownAfter was not called due to error
+		self::cleanUpDatabase();
+
+		DB::addUser(self::$userX['LoginName'], self::$userX['DisplayName'], self::$userX['HomeDir'], self::$userX['PID']);
 	}
 
 	public static function tearDownAfterClass() {
 		self::cleanUpDatabase();
 	}
 
 	public function testAddUser() {
-		$outcome = DB::addUser(self::$userY['LoginName'], self::$userY['DisplayName'], self::$userY['HomeDir']);
+		$outcome = DB::addUser(self::$userY['LoginName'], self::$userY['DisplayName'], self::$userY['HomeDir'], self::$userY['PID']);
 		$this->assertTrue($outcome);
-		$outcome = DB::addUser(self::$userZ['LoginName'], self::$userZ['DisplayName'], self::$userZ['HomeDir']);
+
+		$outcome = DB::addUser(self::$userZ['LoginName'], self::$userZ['DisplayName'], self::$userZ['HomeDir'], self::$userZ['PID']);
 		$this->assertTrue($outcome);
 	}
 
-	public function testDeleteUser() {//run after testAddUser()
+	// Run after testAddUser()
+	public function testDeleteUser() {
+		// Existing user
 		$outcome = DB::deleteUser(self::$userZ['LoginName']);
 		$this->assertTrue($outcome);
 	}
 
-	public function testLoginNameExists() {
-		$outcome = DB::loginNameExists(self::$userX['LoginName']);
+	public function testReAddingUser() {
+		$outcome = DB::addUser(self::$userZ['LoginName'], self::$userZ['DisplayName'], self::$userZ['HomeDir'], self::$userZ['PID']);
 		$this->assertTrue($outcome);
-		$outcome = DB::loginNameExists(self::$userZ['LoginName']);
-		$this->assertFalse($outcome);
+	}
+
+	public function testLoginNameExists() {
+		// Existing user
+		$this->assertTrue(DB::loginNameExists(self::$userX['LoginName']));
+
+		// Deleted user
+		$this->assertTrue(DB::loginNameExists(self::$userZ['LoginName']));
+
+		// Non existing user
+		$this->assertFalse(DB::loginNameExists('NonExisting'));
 	}
 
 	public function testGetDisplayName() {
+		// Existing user
 		$displayName = DB::getDisplayName(self::$userX['LoginName']);
 		$this->assertEquals($displayName, self::$userX['DisplayName']);
+
+		// Deleted user
 		$displayName = DB::getDisplayName(self::$userZ['LoginName']);
+		$this->assertEquals($displayName, self::$userZ['DisplayName']);
+
+		// Non existing user
+		$displayName = DB::getDisplayName('NonExisting');
 		$this->assertFalse($displayName);
 	}
 
 	public function testGetHomeDir() {
+		// Existing user
 		$homeDir = DB::getHomeDir(self::$userX['LoginName']);
 		$this->assertEquals($homeDir, self::$userX['HomeDir']);
-		$homeDir = DB::getDisplayName(self::$userZ['LoginName']);
+
+		// Deleted user
+		$homeDir = DB::getHomeDir(self::$userZ['LoginName']);
+		$this->assertEquals($homeDir, self::$userZ['HomeDir']);
+
+		// Non existing user
+		$homeDir = DB::getHomeDir('NonExisting');
 		$this->assertFalse($homeDir);
 	}
 
 	public function testUpdateDisplayName() {
+		// Existing user
 		DB::updateDisplayName(self::$userY['LoginName'], self::$userZ['DisplayName']);
 		$displayName = DB::getDisplayName(self::$userY['LoginName']);
 		$this->assertEquals($displayName, self::$userZ['DisplayName']);
-		$outcome = DB::updateDisplayName(self::$userY['LoginName'], self::$userY['DisplayName']);//undo change
+
+		// Undo change
+		$outcome = DB::updateDisplayName(self::$userY['LoginName'], self::$userY['DisplayName']);
 		$this->assertTrue($outcome);
 	}
 
 	public function testGetLoginNames() {
-		//test based on login name
+		// Test based on login name
 		$loginNames = DB::getLoginNames('Mister', 10, 0);
 		$success = (in_array(self::$userX['LoginName'], $loginNames) && in_array(self::$userY['LoginName'], $loginNames));
 		$this->assertTrue($success);
 	}
 
 	public function testGetDisplayNames() {
-		$result = DB::getDisplayNames('mister', 10, 0);
+		$result = DB::getDisplayNames('Mister', 10, 0);
 		$this->assertEquals($result[self::$userX['LoginName']], self::$userX['DisplayName']);
 		$this->assertEquals($result[self::$userY['LoginName']], self::$userY['DisplayName']);
 	}

user_shibboleth.php

@@ -23,9 +23,10 @@
 class UserShibboleth extends \OC_User_Backend {
 
 	public function getSupportedActions() {
-		return OC_USER_BACKEND_CHECK_PASSWORD |
-			OC_USER_BACKEND_GET_HOME |
-			OC_USER_BACKEND_GET_DISPLAYNAME;
+		return 
+			\OC_User_Backend::CHECK_PASSWORD |
+			\OC_User_Backend::GET_HOME |
+			\OC_User_Backend::GET_DISPLAYNAME;
 	}
 
 	/**
@@ -106,7 +107,6 @@ public function getDisplayNames($search = '', $limit = null, $offset = null) {
 	}
 
 	public function deleteUser($uid) {
-		// TODO: Delete files as well
 		return DB::deleteUser($uid);
 	}