disable users in ldap when deleting them

broadinstitute · Mar 31, 2020 · 2c4ba97 · 2c4ba97
1 parent cf89ce0
commit 2c4ba97
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 2 deletions.
diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/LdapRegistrationDAO.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/directory/LdapRegistrationDAO.scala
@@ -63,7 +63,10 @@ class LdapRegistrationDAO(
     }
 
   override def deleteUser(userId: WorkbenchUserId): IO[Unit] =
-    executeLdap(IO(ldapConnectionPool.delete(userDn(userId))))
+    executeLdap(for {
+      _ <- disableIdentity(userId)
+      _ <- IO(ldapConnectionPool.delete(userDn(userId)))
+    } yield ())
 
   override def enableIdentity(subject: WorkbenchSubject): IO[Unit] =
     retryLdapBusyWithBackoff(100.millisecond, 4) {
@@ -85,7 +88,8 @@ class LdapRegistrationDAO(
     executeLdap(
       IO(ldapConnectionPool.modify(directoryConfig.enabledUsersGroupDn, new Modification(ModificationType.DELETE, Attr.member, subjectDn(subject)))).void
     ).recover {
-      case ldape: LDAPException if ldape.getResultCode == ResultCode.NO_SUCH_ATTRIBUTE =>
+      case ldape: LDAPException if ldape.getResultCode == ResultCode.NO_SUCH_ATTRIBUTE
+        || ldape.getResultCode == ResultCode.NO_SUCH_OBJECT =>
     }
   }
 

diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/directory/LdapRegistrationDAOSpec.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/directory/LdapRegistrationDAOSpec.scala
@@ -105,6 +105,30 @@ class LdapRegistrationDAOSpec extends FlatSpec with Matchers with TestSupport wi
       dao.loadUser(user.id).unsafeRunSync()
     }
   }
+
+  it should "disable users when deleting them" in {
+    val user = WorkbenchUser(WorkbenchUserId(UUID.randomUUID().toString), None, WorkbenchEmail("foo@bar.com"), None)
+
+    assertResult(user) {
+      dao.createUser(user).unsafeRunSync()
+    }
+
+    dao.enableIdentity(user.id).unsafeRunSync()
+
+    assertResult(true) {
+      dao.isEnabled(user.id).unsafeRunSync()
+    }
+
+    dao.deleteUser(user.id).unsafeRunSync()
+
+    assertResult(None) {
+      dao.loadUser(user.id).unsafeRunSync()
+    }
+
+    assertResult(false) {
+      dao.isEnabled(user.id).unsafeRunSync()
+    }
+  }
 }