Delete empty directories after deleting an account, including symlinks up and down the chain, as appropriate (#6176)

certbot/account.py

@@ -250,12 +250,50 @@ class AccountFileStorage(interfaces.AccountStorage):
         :param account_id: id of account which should be deleted
 
         """
+        # Step 1: remove the account itself
         account_dir_path = self._account_dir_path(account_id)
         if not os.path.isdir(account_dir_path):
             raise errors.AccountNotFound(
                 "Account at %s does not exist" % account_dir_path)
         shutil.rmtree(account_dir_path)
 
+        # Step 2: remove the directory if it's empty, and linked directories
+        if not os.listdir(self.config.accounts_dir):
+            self._delete_accounts_dir_for_server_path(self.config.server_path)
+
+    def _delete_accounts_dir_for_server_path(self, server_path):
+        accounts_dir_path = self.config.accounts_dir_for_server_path(server_path)
+
+        # does an appropriate directory link to me? if so, make sure that's gone
+        reused_servers = {}
+        for k in constants.LE_REUSE_SERVERS:
+            reused_servers[constants.LE_REUSE_SERVERS[k]] = k
+
+        # is there a next one up? call that and be done
+        if server_path in reused_servers:
+            next_server_path = reused_servers[server_path]
+            next_accounts_dir_path = self.config.accounts_dir_for_server_path(next_server_path)
+            if os.path.islink(next_accounts_dir_path) \
+                and os.readlink(next_accounts_dir_path) == accounts_dir_path:
+                self._delete_accounts_dir_for_server_path(next_server_path)
+                return
+
+        # if there's not a next one up to delete, then delete me
+        # and whatever I link to if applicable
+        if os.path.islink(accounts_dir_path):
+            # save my info then delete me
+            target = os.readlink(accounts_dir_path)
+            os.unlink(accounts_dir_path)
+            # then delete whatever I linked to, if appropriate
+            if server_path in constants.LE_REUSE_SERVERS:
+                prev_server_path = constants.LE_REUSE_SERVERS[server_path]
+                prev_accounts_dir_path = self.config.accounts_dir_for_server_path(prev_server_path)
+                if target == prev_accounts_dir_path:
+                    self._delete_accounts_dir_for_server_path(prev_server_path)
+        else:
+            # just delete me
+            os.rmdir(accounts_dir_path)
+
     def _save(self, account, acme, regr_only):
         account_dir_path = self._account_dir_path(account.id)
         util.make_or_verify_dir(account_dir_path, 0o700, os.geteuid(),

certbot/tests/account_test.py

@@ -273,6 +273,40 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
     def test_delete_no_account(self):
         self.assertRaises(errors.AccountNotFound, self.storage.delete, self.acc.id)
 
+    def _assert_symlinked_account_removed(self):
+        # create v1 account
+        self._set_server('https://acme-staging.api.letsencrypt.org/directory')
+        self.storage.save(self.acc, self.mock_client)
+        # ensure v2 isn't already linked to it
+        with mock.patch('certbot.constants.LE_REUSE_SERVERS', {}):
+            self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
+            self.assertRaises(errors.AccountNotFound, self.storage.load, self.acc.id)
+
+    def _test_delete_folders(self, server_url):
+        # create symlinked servers
+        self._set_server('https://acme-staging.api.letsencrypt.org/directory')
+        self.storage.save(self.acc, self.mock_client)
+        self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
+        self.storage.find_all()
+
+        # delete starting at given server_url
+        self._set_server(server_url)
+        self.storage.delete(self.acc.id)
+
+        # make sure we're gone from both urls
+        self._set_server('https://acme-staging.api.letsencrypt.org/directory')
+        self.assertRaises(errors.AccountNotFound, self.storage.load, self.acc.id)
+        self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
+        self.assertRaises(errors.AccountNotFound, self.storage.load, self.acc.id)
+
+    def test_delete_folders_up(self):
+        self._test_delete_folders('https://acme-staging.api.letsencrypt.org/directory')
+        self._assert_symlinked_account_removed()
+
+    def test_delete_folders_down(self):
+        self._test_delete_folders('https://acme-staging-v02.api.letsencrypt.org/directory')
+        self._assert_symlinked_account_removed()
+
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover