launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26636
[Merge] ~cjwatson/launchpad:account-status-deceased into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:account-status-deceased into launchpad:master.
Commit message:
Add a "deceased" account status
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/399600
Sometimes Launchpad users die, and we're currently limited in what we can do to record that information sensitively. We generally want to at least prevent people trying to contact them via Launchpad, and prevent people who may have obtained the deceased person's credentials from logging in. If the user's family indicate that they want to hide or remove the user's data, then existing mechanisms (deactivation or close-account) are good enough, but otherwise it would be useful to have a way to archive their account.
This adds a new `DECEASED` account status. Accounts in that status will have an appropriate message displayed at the top of their profile page, and will be considered inactive so that they will not be able to log in.
Some relevant screenshots:
* Profile page: https://people.canonical.com/~cjwatson/tmp/account-status-deceased-profile.png
* Login attempt: https://people.canonical.com/~cjwatson/tmp/account-status-deceased-login.png
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:account-status-deceased into launchpad:master.
diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index 8311bd1..b15ec69 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -1349,6 +1349,12 @@ class PersonAccountAdministerView(LaunchpadFormView):
self.request.response.addInfoNotification(
u'The account "%s" is now deactivated. The user can log in '
u'to reactivate it.' % self.context.displayname)
+ elif data['status'] == AccountStatus.DECEASED:
+ # Deliberately leave the email address in place so that it can't
+ # easily be claimed by somebody else.
+ self.request.response.addInfoNotification(
+ u'The account "%s" has been marked as having belonged to a '
+ u'deceased user.' % self.context.displayname)
self.context.setStatus(data['status'], self.user, data['comment'])
diff --git a/lib/lp/registry/browser/tests/person-admin-views.txt b/lib/lp/registry/browser/tests/person-admin-views.txt
index 329b679..9e4d33d 100644
--- a/lib/lp/registry/browser/tests/person-admin-views.txt
+++ b/lib/lp/registry/browser/tests/person-admin-views.txt
@@ -166,3 +166,21 @@ user must log in to restore the email addresses using the reactivate step.
name16: Suspended -> Deactivated: Zaphod's a hoopy frood.\n"
>>> print(user.preferredemail)
None
+
+
+An admin can mark an account as belonging to a user who has died.
+
+ >>> form = {
+ ... 'field.status': 'DECEASED',
+ ... 'field.comment': 'In memoriam.',
+ ... 'field.actions.change': 'Change',
+ ... }
+ >>> view = create_initialized_view(user, '+reviewaccount', form=form)
+ >>> print(view.errors)
+ []
+ >>> user.account_status
+ <DBItem AccountStatus.DECEASED, ...>
+ >>> six.ensure_str(user.account_status_history)
+ "... name16: Active -> Suspended: Wanted by the galactic police.\n...
+ name16: Suspended -> Deactivated: Zaphod's a hoopy frood.\n...
+ name16: Deactivated -> Deceased: In memoriam.\n"
diff --git a/lib/lp/registry/browser/tests/test_person_webservice.py b/lib/lp/registry/browser/tests/test_person_webservice.py
index a4b8150..d9633f9 100644
--- a/lib/lp/registry/browser/tests/test_person_webservice.py
+++ b/lib/lp/registry/browser/tests/test_person_webservice.py
@@ -409,6 +409,20 @@ class PersonSetWebServiceTests(TestCaseWithFactory):
response = self.getOrCreateSoftwareCenterCustomer(sca)
self.assertEqual(400, response.status)
+ def test_getOrCreateSoftwareCenterCustomer_rejects_deceased(self):
+ # Deceased accounts are not returned.
+ with admin_logged_in():
+ existing = self.factory.makePerson(
+ email='somebody@xxxxxxxxxxx',
+ account_status=AccountStatus.DECEASED)
+ oid = OpenIdIdentifier()
+ oid.account = existing.account
+ oid.identifier = u'somebody'
+ Store.of(existing).add(oid)
+ sca = getUtility(IPersonSet).getByName('software-center-agent')
+ response = self.getOrCreateSoftwareCenterCustomer(sca)
+ self.assertEqual(400, response.status)
+
def test_getUsernameForSSO(self):
# canonical-identity-provider (SSO) can get the username for an
# OpenID identifier suffix.
diff --git a/lib/lp/registry/interfaces/person.py b/lib/lp/registry/interfaces/person.py
index a956d59..971cadf 100644
--- a/lib/lp/registry/interfaces/person.py
+++ b/lib/lp/registry/interfaces/person.py
@@ -2208,6 +2208,8 @@ class IPersonSet(Interface):
database was updated.
:raises AccountSuspendedError: if the account associated with the
identifier has been suspended.
+ :raises AccountDeceasedError: if the account associated with the
+ identifier belongs to a deceased user.
"""
@call_with(user=REQUEST_USER)
diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 624fab2..69ea961 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -266,6 +266,7 @@ from lp.services.helpers import (
)
from lp.services.identity.interfaces.account import (
AccountCreationRationale,
+ AccountDeceasedError,
AccountStatus,
AccountSuspendedError,
IAccount,
@@ -3375,6 +3376,10 @@ class PersonSet:
elif status == AccountStatus.SUSPENDED:
raise AccountSuspendedError(
"The account matching the identifier is suspended.")
+ elif status == AccountStatus.DECEASED:
+ raise AccountDeceasedError(
+ "The account matching the identifier belongs to a "
+ "deceased user.")
elif not trust_email and status != AccountStatus.NOACCOUNT:
# If the email address is not completely trustworthy
# (ie. it comes from SCA) and the account has already
diff --git a/lib/lp/registry/stories/person/xx-person-home.txt b/lib/lp/registry/stories/person/xx-person-home.txt
index 52f5348..fab8a67 100644
--- a/lib/lp/registry/stories/person/xx-person-home.txt
+++ b/lib/lp/registry/stories/person/xx-person-home.txt
@@ -272,3 +272,32 @@ users cannot see this, but admins like Foo Bar can.
Change email settings
+Deceased profiles
+-----------------
+
+When we have reliable information that former users have died, it can be in
+better taste to make this clear on their profile page.
+
+ >>> from lp.registry.interfaces.person import IPersonSet
+ >>> from lp.services.identity.interfaces.account import AccountStatus
+
+ >>> anon_browser.open('https://launchpad.test/~name12')
+ >>> print(find_tag_by_id(anon_browser.contents, 'deceased-note'))
+ None
+
+ >>> login('admin@xxxxxxxxxxxxx')
+ >>> name12 = getUtility(IPersonSet).getByName('name12')
+ >>> name12.setAccountStatus(AccountStatus.DECEASED, None, 'RIP')
+ >>> transaction.commit()
+ >>> logout()
+
+ >>> anon_browser.open('https://launchpad.test/~name12')
+ >>> print(extract_text(
+ ... find_tag_by_id(anon_browser.contents, 'deceased-note')))
+ This account belonged to a deceased user and has been archived.
+
+Most of the rest of their profile page remains intact.
+
+ >>> print(extract_text(
+ ... find_tag_by_id(anon_browser.contents, 'contact-details')))
+ User information...
diff --git a/lib/lp/registry/templates/person-index.pt b/lib/lp/registry/templates/person-index.pt
index 4933c09..1261219 100644
--- a/lib/lp/registry/templates/person-index.pt
+++ b/lib/lp/registry/templates/person-index.pt
@@ -54,8 +54,14 @@
<div metal:fill-slot="main"
tal:define="overview_menu context/menu:overview">
- <tal:is-valid-person tal:condition="context/is_valid_person">
-
+ <tal:show-main-content
+ condition="python: context.is_valid_person or
+ context.account_status.name == 'DECEASED'">
+ <div id="deceased-note"
+ tal:condition="context/account_status/enumvalue:DECEASED">
+ This account belonged to a deceased user and has been
+ <strong>archived</strong>.
+ </div>
<div class="description">
<tal:widget replace="structure view/description_widget" />
</div>
@@ -105,7 +111,7 @@
</div>
</div>
- </tal:is-valid-person>
+ </tal:show-main-content>
<div id="not-lp-user-or-team"
tal:condition="not: context/is_valid_person_or_team">
diff --git a/lib/lp/registry/tests/test_personset.py b/lib/lp/registry/tests/test_personset.py
index 8fa87f0..a0fee14 100644
--- a/lib/lp/registry/tests/test_personset.py
+++ b/lib/lp/registry/tests/test_personset.py
@@ -46,6 +46,7 @@ from lp.services.database.sqlbase import (
)
from lp.services.identity.interfaces.account import (
AccountCreationRationale,
+ AccountDeceasedError,
AccountStatus,
AccountSuspendedError,
IAccountSet,
@@ -637,6 +638,16 @@ class TestPersonSetGetOrCreateByOpenIDIdentifier(TestCaseWithFactory):
self.assertRaises(
AccountSuspendedError, self.callGetOrCreate, openid_ident)
+ def test_existing_deceased_account(self):
+ # An existing account belonging to a deceased user will raise an
+ # exception.
+ person = self.factory.makePerson(account_status=AccountStatus.DECEASED)
+ openid_ident = removeSecurityProxy(
+ person.account).openid_identifiers.any().identifier
+
+ self.assertRaises(
+ AccountDeceasedError, self.callGetOrCreate, openid_ident)
+
def test_no_account_or_email(self):
# An identifier can be used to create an account (it is assumed
# to be already authenticated with SSO).
@@ -787,6 +798,18 @@ class TestPersonSetGetOrCreateSoftwareCenterCustomer(TestCaseWithFactory):
getUtility(IPersonSet).getOrCreateSoftwareCenterCustomer,
self.sca, u'somebody', 'somebody@xxxxxxxxxxx', 'Example')
+ def test_fails_if_account_is_deceased(self):
+ # Accounts belonging to deceased users cannot be returned.
+ somebody = self.factory.makePerson()
+ make_openid_identifier(somebody.account, u'somebody')
+ with admin_logged_in():
+ somebody.setAccountStatus(AccountStatus.DECEASED, None, "RIP")
+ with person_logged_in(self.sca):
+ self.assertRaises(
+ AccountDeceasedError,
+ getUtility(IPersonSet).getOrCreateSoftwareCenterCustomer,
+ self.sca, u'somebody', 'somebody@xxxxxxxxxxx', 'Example')
+
class TestPersonGetUsernameForSSO(TestCaseWithFactory):
diff --git a/lib/lp/scripts/garbo.py b/lib/lp/scripts/garbo.py
index 12fd7b4..0f4867a 100644
--- a/lib/lp/scripts/garbo.py
+++ b/lib/lp/scripts/garbo.py
@@ -1064,8 +1064,9 @@ class AnswerContactPruner(BulkPruner):
"""Remove old answer contacts which are no longer required.
Remove a person as an answer contact if:
- their account has been deactivated for more than one day, or
- suspended for more than one week.
+ their account has been deactivated for more than one day,
+ suspended for more than one week, or
+ marked as deceased for more than one week.
"""
target_table_class = AnswerContact
ids_to_prune_query = """
@@ -1083,9 +1084,10 @@ class AnswerContactPruner(BulkPruner):
(Account.date_status_set <
CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
- CAST('7 days' AS interval)
- AND Account.status = %s)
+ AND Account.status IN %s)
)
- """ % (AccountStatus.DEACTIVATED.value, AccountStatus.SUSPENDED.value)
+ """ % (AccountStatus.DEACTIVATED.value,
+ (AccountStatus.SUSPENDED.value, AccountStatus.DECEASED.value))
class BranchJobPruner(BulkPruner):
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index 03ccdd2..2f1d71e 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -877,13 +877,20 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
self._test_AnswerContactPruner(
AccountStatus.SUSPENDED, SEVEN_DAYS_AGO)
+ def test_AnswerContactPruner_deceased_accounts(self):
+ # Answer contacts with an account marked as deceased at least seven
+ # days ago should be pruned.
+ self._test_AnswerContactPruner(AccountStatus.DECEASED, SEVEN_DAYS_AGO)
+
def test_AnswerContactPruner_doesnt_prune_recently_changed_accounts(self):
- # Answer contacts which are suspended or deactivated inside the
- # minimum time interval are not pruned.
+ # Answer contacts which are deactivated, suspended, or deceased
+ # inside the minimum time interval are not pruned.
self._test_AnswerContactPruner(
AccountStatus.DEACTIVATED, None, expected_count=1)
self._test_AnswerContactPruner(
AccountStatus.SUSPENDED, ONE_DAY_AGO, expected_count=1)
+ self._test_AnswerContactPruner(
+ AccountStatus.DECEASED, ONE_DAY_AGO, expected_count=1)
def test_BranchJobPruner(self):
# Garbo should remove jobs completed over 30 days ago.
diff --git a/lib/lp/services/identity/interfaces/account.py b/lib/lp/services/identity/interfaces/account.py
index e355e43..846731e 100644
--- a/lib/lp/services/identity/interfaces/account.py
+++ b/lib/lp/services/identity/interfaces/account.py
@@ -6,6 +6,7 @@
__metaclass__ = type
__all__ = [
+ 'AccountDeceasedError',
'AccountStatus',
'AccountStatusError',
'AccountSuspendedError',
@@ -46,6 +47,11 @@ class AccountSuspendedError(Exception):
"""The account being accessed has been suspended."""
+@error_status(http_client.BAD_REQUEST)
+class AccountDeceasedError(Exception):
+ """The account being accessed belongs to a deceased user."""
+
+
class AccountStatus(DBEnumeratedType):
"""The status of an account."""
@@ -90,10 +96,17 @@ class AccountStatus(DBEnumeratedType):
information as possible.
""")
+ DECEASED = DBItem(60, """
+ Deceased
+
+ The account belonged to somebody who is now deceased, and has been
+ permanently archived.
+ """)
+
INACTIVE_ACCOUNT_STATUSES = [
AccountStatus.PLACEHOLDER, AccountStatus.DEACTIVATED,
- AccountStatus.SUSPENDED, AccountStatus.CLOSED]
+ AccountStatus.SUSPENDED, AccountStatus.CLOSED, AccountStatus.DECEASED]
class AccountCreationRationale(DBEnumeratedType):
@@ -241,16 +254,23 @@ class AccountStatusChoice(Choice):
transitions = {
AccountStatus.PLACEHOLDER: [
- AccountStatus.NOACCOUNT, AccountStatus.ACTIVE],
- AccountStatus.NOACCOUNT: [AccountStatus.ACTIVE, AccountStatus.CLOSED],
+ AccountStatus.NOACCOUNT, AccountStatus.ACTIVE,
+ AccountStatus.DECEASED],
+ AccountStatus.NOACCOUNT: [
+ AccountStatus.ACTIVE, AccountStatus.CLOSED,
+ AccountStatus.DECEASED],
AccountStatus.ACTIVE: [
AccountStatus.DEACTIVATED, AccountStatus.SUSPENDED,
- AccountStatus.CLOSED],
+ AccountStatus.CLOSED, AccountStatus.DECEASED],
AccountStatus.DEACTIVATED: [
- AccountStatus.ACTIVE, AccountStatus.CLOSED],
+ AccountStatus.ACTIVE, AccountStatus.CLOSED,
+ AccountStatus.DECEASED],
AccountStatus.SUSPENDED: [
- AccountStatus.DEACTIVATED, AccountStatus.CLOSED],
+ AccountStatus.DEACTIVATED, AccountStatus.CLOSED,
+ AccountStatus.DECEASED],
AccountStatus.CLOSED: [],
+ AccountStatus.DECEASED: [
+ AccountStatus.DEACTIVATED, AccountStatus.CLOSED],
}
def constraint(self, value):
diff --git a/lib/lp/services/identity/tests/test_account.py b/lib/lp/services/identity/tests/test_account.py
index 7bf599f..36b2df1 100644
--- a/lib/lp/services/identity/tests/test_account.py
+++ b/lib/lp/services/identity/tests/test_account.py
@@ -50,41 +50,66 @@ class TestAccount(TestCaseWithFactory):
self.assertEqual(status, account.status)
def test_status_from_noaccount(self):
- # The status may change from NOACCOUNT to ACTIVE or CLOSED.
+ # The status may change from NOACCOUNT to ACTIVE, CLOSED, or
+ # DECEASED.
account = self.factory.makeAccount(status=AccountStatus.NOACCOUNT)
login_celebrity('admin')
self.assertCannotTransition(
account, [AccountStatus.DEACTIVATED, AccountStatus.SUSPENDED])
self.assertCanTransition(
- account, [AccountStatus.ACTIVE, AccountStatus.CLOSED])
+ account,
+ [AccountStatus.ACTIVE, AccountStatus.CLOSED,
+ AccountStatus.DECEASED])
def test_status_from_active(self):
- # The status may change from ACTIVE to DEACTIVATED, SUSPENDED, or
- # CLOSED.
+ # The status may change from ACTIVE to DEACTIVATED, SUSPENDED,
+ # CLOSED, or DECEASED.
account = self.factory.makeAccount(status=AccountStatus.ACTIVE)
login_celebrity('admin')
self.assertCannotTransition(account, [AccountStatus.NOACCOUNT])
self.assertCanTransition(
account,
[AccountStatus.DEACTIVATED, AccountStatus.SUSPENDED,
- AccountStatus.CLOSED])
+ AccountStatus.CLOSED, AccountStatus.DECEASED])
def test_status_from_deactivated(self):
- # The status may change from DEACTIVATED to ACTIVATED or CLOSED.
+ # The status may change from DEACTIVATED to ACTIVATED, CLOSED, or
+ # DECEASED.
account = self.factory.makeAccount()
login_celebrity('admin')
account.setStatus(AccountStatus.DEACTIVATED, None, 'gbcw')
self.assertCannotTransition(
account, [AccountStatus.NOACCOUNT, AccountStatus.SUSPENDED])
self.assertCanTransition(
- account, [AccountStatus.ACTIVE, AccountStatus.CLOSED])
+ account,
+ [AccountStatus.ACTIVE, AccountStatus.CLOSED,
+ AccountStatus.DECEASED])
def test_status_from_suspended(self):
- # The status may change from SUSPENDED to DEACTIVATED or CLOSED.
+ # The status may change from SUSPENDED to DEACTIVATED, CLOSED, or
+ # DECEASED.
account = self.factory.makeAccount()
login_celebrity('admin')
account.setStatus(AccountStatus.SUSPENDED, None, 'spammer!')
self.assertCannotTransition(
account, [AccountStatus.NOACCOUNT, AccountStatus.ACTIVE])
self.assertCanTransition(
- account, [AccountStatus.DEACTIVATED, AccountStatus.CLOSED])
+ account,
+ [AccountStatus.DEACTIVATED, AccountStatus.CLOSED,
+ AccountStatus.DECEASED])
+
+ def test_status_from_deceased(self):
+ # The status may change from DECEASED to DEACTIVATED (perhaps we
+ # were misinformed) or CLOSED (perhaps family members don't want the
+ # account to be visible).
+ account = self.factory.makeAccount()
+ login_celebrity('admin')
+ account.setStatus(AccountStatus.DECEASED, None, 'RIP')
+ self.assertCannotTransition(
+ account,
+ [AccountStatus.NOACCOUNT, AccountStatus.ACTIVE,
+ AccountStatus.SUSPENDED])
+ self.assertCanTransition(
+ account,
+ [AccountStatus.DEACTIVATED, AccountStatus.CLOSED,
+ AccountStatus.DECEASED])
diff --git a/lib/lp/services/webapp/login.py b/lib/lp/services/webapp/login.py
index f386b75..aca7b07 100644
--- a/lib/lp/services/webapp/login.py
+++ b/lib/lp/services/webapp/login.py
@@ -52,7 +52,10 @@ from lp.registry.interfaces.person import (
)
from lp.services.config import config
from lp.services.database.policy import MasterDatabasePolicy
-from lp.services.identity.interfaces.account import AccountSuspendedError
+from lp.services.identity.interfaces.account import (
+ AccountDeceasedError,
+ AccountSuspendedError,
+ )
from lp.services.openid.extensions import macaroon
from lp.services.openid.interfaces.openidconsumer import IOpenIDConsumerStore
from lp.services.propertycache import cachedproperty
@@ -278,6 +281,9 @@ class OpenIDCallbackView(OpenIDLogin):
suspended_account_template = ViewPageTemplateFile(
'templates/login-suspended-account.pt')
+ deceased_account_template = ViewPageTemplateFile(
+ 'templates/login-deceased-account.pt')
+
team_email_address_template = ViewPageTemplateFile(
'templates/login-team-email-address.pt')
@@ -379,6 +385,8 @@ class OpenIDCallbackView(OpenIDLogin):
should_update_last_write = db_updated
except AccountSuspendedError:
return self.suspended_account_template()
+ except AccountDeceasedError:
+ return self.deceased_account_template()
except TeamEmailAddressError:
return self.team_email_address_template()
diff --git a/lib/lp/services/webapp/templates/login-deceased-account.pt b/lib/lp/services/webapp/templates/login-deceased-account.pt
new file mode 100644
index 0000000..6a6121e
--- /dev/null
+++ b/lib/lp/services/webapp/templates/login-deceased-account.pt
@@ -0,0 +1,21 @@
+<html
+ xmlns="http://www.w3.org/1999/xhtml";
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ metal:use-macro="view/macro:page/main_only"
+ i18n:domain="launchpad">
+
+ <body>
+ <div class="top-portlet" metal:fill-slot="main">
+
+ <h1>This account belongs to a deceased user and has been archived</h1>
+ <p class="error">
+ If you are the user in question and we have received incorrect
+ information, then please contact
+ <a href="mailto:feedback@xxxxxxxxxxxxx";>Launchpad staff</a>.
+ </p>
+
+ </div>
+ </body>
+</html>
diff --git a/lib/lp/services/webapp/tests/test_login.py b/lib/lp/services/webapp/tests/test_login.py
index 18dc989..732257e 100644
--- a/lib/lp/services/webapp/tests/test_login.py
+++ b/lib/lp/services/webapp/tests/test_login.py
@@ -459,6 +459,17 @@ class TestOpenIDCallbackView(TestCaseWithFactory):
main_content = extract_text(find_main_content(html))
self.assertIn('This account has been suspended', main_content)
+ def test_deceased_account(self):
+ # There's a chance that our OpenID Provider lets a deceased account
+ # login, but we must not allow that.
+ person = self.factory.makePerson(
+ account_status=AccountStatus.DECEASED)
+ with SRegResponse_fromSuccessResponse_stubbed():
+ view, html = self._createViewWithResponse(person.account)
+ self.assertFalse(view.login_called)
+ main_content = extract_text(find_main_content(html))
+ self.assertIn('This account belongs to a deceased user', main_content)
+
def test_account_with_team_email_address(self):
# If the email address from the OpenID provider is owned by a
# team, there's not much we can do. See bug #556680 for
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 034095d..d4ed1fd 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -675,7 +675,9 @@ class BareLaunchpadObjectFactory(ObjectFactory):
removeSecurityProxy(email).status = email_address_status
- once_active = (AccountStatus.DEACTIVATED, AccountStatus.SUSPENDED)
+ once_active = (
+ AccountStatus.DEACTIVATED, AccountStatus.SUSPENDED,
+ AccountStatus.DECEASED)
if account_status:
if account_status in once_active:
removeSecurityProxy(person.account).status = (
