launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14934
[Merge] lp:~wgrant/launchpad/registry-view-accounts into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/registry-view-accounts into lp:launchpad.
Commit message:
Let ~registry see OpenID identifiers, validated email addresses, and guessed email addresses on +reviewaccount.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/registry-view-accounts/+merge/143438
We sometimes need to see users' OpenID identifiers. They're shown on +reviewaccount, but only for admins. This branch lets ~registry see them too, as they're not sensitive.
I also added guessed (NEW) email addresses to the page, as they can be useful for working out account merges and strange OpenID linkage incidents.
--
https://code.launchpad.net/~wgrant/launchpad/registry-view-accounts/+merge/143438
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/registry-view-accounts into lp:launchpad.
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2013-01-11 03:41:17 +0000
+++ lib/lp/registry/browser/person.py 2013-01-16 05:13:23 +0000
@@ -1133,23 +1133,12 @@
"""Administer an `IPerson`."""
schema = IPerson
label = "Review person"
- default_field_names = [
+ field_names = [
'name', 'displayname',
'personal_standing', 'personal_standing_reason']
custom_widget(
'personal_standing_reason', TextAreaWidget, height=5, width=60)
- def setUpFields(self):
- """Setup the normal fields from the schema, as appropriate.
-
- If not an admin (e.g. registry expert), remove the displayname field.
- """
- self.field_names = self.default_field_names[:]
- admin = check_permission('launchpad.Admin', self.context)
- if not admin:
- self.field_names.remove('displayname')
- super(PersonAdministerView, self).setUpFields()
-
@property
def is_viewing_person(self):
"""Is the view showing an `IPerson`?
@@ -1179,6 +1168,7 @@
"""Administer an `IAccount` belonging to an `IPerson`."""
schema = IAccount
label = "Review person's account"
+ field_names = ['displayname', 'status', 'status_comment']
custom_widget(
'status_comment', TextAreaWidget, height=5, width=60)
@@ -1189,10 +1179,6 @@
# It also means that permissions are checked on IAccount, not IPerson.
self.person = self.context
self.context = self.person.account
- # Set fields to be displayed.
- self.field_names = ['status', 'status_comment']
- if self.viewed_by_admin:
- self.field_names = ['displayname'] + self.field_names
@property
def is_viewing_person(self):
@@ -1204,11 +1190,6 @@
return False
@property
- def viewed_by_admin(self):
- """Is the user a Launchpad admin?"""
- return check_permission('launchpad.Admin', self.context)
-
- @property
def email_addresses(self):
"""A list of the user's preferred and validated email addresses."""
emails = sorted(
@@ -1218,12 +1199,17 @@
return emails
@property
+ def guessed_email_addresses(self):
+ """A list of the user's new email addresses.
+
+ This is just EmailAddressStatus.NEW addresses, not unvalidated ones
+ created through the web UI. They only have LoginTokens.
+ """
+ return sorted(email.email for email in self.person.guessedemails)
+
+ @property
def next_url(self):
"""See `LaunchpadEditFormView`."""
- is_suspended = self.context.status == AccountStatus.SUSPENDED
- if is_suspended and not self.viewed_by_admin:
- # Non-admins cannot see suspended persons.
- return canonical_url(getUtility(IPersonSet))
return canonical_url(self.person)
@property
=== modified file 'lib/lp/registry/stories/person/xx-admin-person-review.txt'
--- lib/lp/registry/stories/person/xx-admin-person-review.txt 2012-12-10 13:43:47 +0000
+++ lib/lp/registry/stories/person/xx-admin-person-review.txt 2013-01-16 05:13:23 +0000
@@ -1,23 +1,49 @@
Review person
=============
-Launchpad admins such as Foo Bar can review users and update some of
-their information.
-
- >>> admin_browser.open('http://launchpad.dev/~salgado')
-
- >>> admin_browser.getLink('Administer').click()
- >>> print admin_browser.url
+Registry admins can review users and update some of their information.
+
+ >>> email = "expert@xxxxxxxxxxx"
+ >>> expert = factory.makeRegistryExpert(email=email)
+ >>> expert_browser = setupBrowser(auth='Basic %s:test' % email)
+ >>> logout()
+
+ >>> expert_browser.open('http://launchpad.dev/~salgado')
+ >>> expert_browser.getLink('Administer').click()
+ >>> print expert_browser.url
http://launchpad.dev/~salgado/+review
- >>> print admin_browser.title
+ >>> print expert_browser.title
Review person...
+ >>> expert_browser.getControl('Name', index=0).value = 'no-way'
+ >>> expert_browser.getControl('Personal standing').value = ['GOOD']
+ >>> expert_browser.getControl(
+ ... name='field.personal_standing_reason').value = 'good guy'
+ >>> expert_browser.getControl('Change').click()
+ >>> print expert_browser.url
+ http://launchpad.dev/~no-way
+
+Registry experts can't change the displayname.
+
+ >>> expert_browser.getLink('Administer').click()
+ >>> expert_browser.getControl(
+ ... 'Display Name', index=0).value = 'The one and only Salgado'
+ >>> expert_browser.getControl('Change').click()
+ Traceback (most recent call last):
+ ...
+ Unauthorized: ...
+
+But Launchpad admins can.
+ >>> admin_browser.open('http://launchpad.dev/~no-way/+review')
+ >>> admin_browser.getControl('Name', index=0).value = 'salgado'
>>> admin_browser.getControl(
... 'Display Name', index=0).value = 'The one and only Salgado'
>>> admin_browser.getControl('Change').click()
>>> print admin_browser.title
The one and only Salgado in Launchpad
+ >>> print admin_browser.url
+ http://launchpad.dev/~salgado
Review account
@@ -25,84 +51,25 @@
The review page has a link to the review account page.
- >>> admin_browser.open('http://launchpad.dev/~salgado')
- >>> admin_browser.getLink('Administer Account').click()
- >>> print admin_browser.title
+ >>> expert_browser.open('http://launchpad.dev/~salgado')
+ >>> expert_browser.getLink('Administer Account').click()
+ >>> print expert_browser.title
Review person's account...
The +reviewaccount page displays account information that is normally
hidden from the UI.
- >>> content = find_main_content(admin_browser.contents)
+ >>> content = find_main_content(expert_browser.contents)
>>> for tr in content.find(id='summary').findAll('tr'):
... print extract_text(tr)
Created: 2005-06-06
Creation reason: Created by the owner himself, coming from Launchpad.
- OpenID Identifiers: salgado_oid
- Email Addresses: guilherme.salgado@xxxxxxxxxxxxx
+ OpenID identifiers: salgado_oid
+ Email addresses: guilherme.salgado@xxxxxxxxxxxxx
+ Guessed email addresses: salgado@xxxxxxxxxx
The page also contains a link back to the +review page.
- >>> link = admin_browser.getLink(url='+review')
+ >>> link = expert_browser.getLink(url='+review')
>>> print link.text
edit[IMG] Review the user's Launchpad information
-
-
-Registry experts
-----------------
-
-Registry experts can see the +review page, minus the displayname.
-
- >>> email = "expert@xxxxxxxxxxx"
- >>> expert = factory.makeRegistryExpert(email=email)
- >>> expert_browser = setupBrowser(auth='Basic %s:test' % email)
- >>> logout()
- >>> expert_browser.open('http://launchpad.dev/~no-priv/+review')
- >>> print expert_browser.title
- Review person...
-
- >>> expert_browser.getControl('Name', index=0).value = "no-way"
- >>> expert_browser.getControl(
- ... 'Display Name', index=0)
- Traceback (most recent call last):
- ...
- LookupError: label 'Display Name'
-
- >>> expert_browser.getControl('Personal standing').value = ['GOOD']
- >>> expert_browser.getControl(
- ... name='field.personal_standing_reason').value = 'good guy'
- >>> expert_browser.getControl('Change').click()
- >>> print expert_browser.url
- http://launchpad.dev/~no-way
-
-However, members of the registry team only get partial access to the
-review account page to be able to suspend spam accounts.
-
- >>> expert_browser.open('http://launchpad.dev/~no-way/+reviewaccount')
- >>> print expert_browser.title
- Review person's account...
-
-The +reviewaccount page does not display account information for registry
-experts. It also has no form elements to change the display name.
-
- >>> content = find_main_content(expert_browser.contents)
- >>> print content.find(id='summary')
- None
- >>> print content.find(name='field.displayname')
- None
-
-The only option for registry experts is to change an account's status and to
-provide a reason why they did it. After suspending the account the
-page will only be visible to admins, so the registry expert will be
-redirected to the "people" main page. A feedback message informs the expert
-about the suspension.
-
- >>> control = expert_browser.getControl(name='field.status_comment')
- >>> control.value = 'This is SPAM!'
- >>> expert_browser.getControl(
- ... 'The status of this account').value = ['SUSPENDED']
- >>> expert_browser.getControl('Change').click()
- >>> print expert_browser.url
- http://launchpad.dev/people
- >>> print get_feedback_messages(expert_browser.contents)[0]
- The account "No Privileges Person" has been suspended.
=== modified file 'lib/lp/registry/templates/person-review.pt'
--- lib/lp/registry/templates/person-review.pt 2012-02-17 15:31:38 +0000
+++ lib/lp/registry/templates/person-review.pt 2013-01-16 05:13:23 +0000
@@ -27,8 +27,7 @@
</p>
</tal:review-person>
- <tal:review-account
- condition="python: not view.is_viewing_person and view.viewed_by_admin">
+ <tal:review-account condition="python: not view.is_viewing_person">
<p>
The account displayname is not always the same as the Launchpad
displayname.
@@ -49,7 +48,7 @@
<td><tal:reason replace="view/context/creation_rationale/title" /></td>
</tr>
<tr>
- <th>OpenID Identifiers:</th>
+ <th>OpenID identifiers:</th>
<td>
<ul>
<li tal:repeat="identifier view/context/openid_identifiers"
@@ -58,7 +57,7 @@
</td>
</tr>
<tr>
- <th>Email Addresses:</th>
+ <th>Email addresses:</th>
<td>
<tal:emails repeat="email view/email_addresses">
<span style="white-space: nowrap">
@@ -68,6 +67,17 @@
</tal:emails>
</td>
</tr>
+ <tr>
+ <th>Guessed email addresses:</th>
+ <td>
+ <tal:emails repeat="email view/guessed_email_addresses">
+ <span style="white-space: nowrap">
+ <tal:email replace="email">foo2@xxxxxxx</tal:email>
+ <br tal:omit-tag="repeat/email/end" />
+ </span>
+ </tal:emails>
+ </td>
+ </tr>
</table>
</tal:review-account>
</div>
=== modified file 'lib/lp/security.py'
--- lib/lp/security.py 2013-01-09 05:37:18 +0000
+++ lib/lp/security.py 2013-01-16 05:13:23 +0000
@@ -382,6 +382,20 @@
class ViewAccount(EditAccountBySelfOrAdmin):
permission = 'launchpad.View'
+ def checkAuthenticated(self, user):
+ """Extend permission to registry experts."""
+ return (
+ super(ViewAccount, self).checkAuthenticated(user)
+ or user.in_registry_experts)
+
+
+class ModerateAccountByRegistryExpert(AuthorizationBase):
+ usedfor = IAccount
+ permission = 'launchpad.Moderate'
+
+ def checkAuthenticated(self, user):
+ return user.in_admin or user.in_registry_experts
+
class ViewOpenIdIdentifierBySelfOrAdmin(AuthorizationBase):
permission = 'launchpad.View'
@@ -391,24 +405,6 @@
return user.in_admin or user.person.accountID == self.obj.accountID
-class SpecialAccount(EditAccountBySelfOrAdmin):
- permission = 'launchpad.Special'
-
- def checkAuthenticated(self, user):
- """Extend permission to registry experts."""
- return (
- super(SpecialAccount, self).checkAuthenticated(user)
- or user.in_registry_experts)
-
-
-class ModerateAccountByRegistryExpert(AuthorizationBase):
- usedfor = IAccount
- permission = 'launchpad.Moderate'
-
- def checkAuthenticated(self, user):
- return user.in_admin or user.in_registry_experts
-
-
class EditOAuthAccessToken(AuthorizationBase):
permission = 'launchpad.Edit'
usedfor = IOAuthAccessToken
=== modified file 'lib/lp/services/identity/configure.zcml'
--- lib/lp/services/identity/configure.zcml 2012-01-15 04:50:33 +0000
+++ lib/lp/services/identity/configure.zcml 2013-01-16 05:13:23 +0000
@@ -50,10 +50,7 @@
interface="lp.services.identity.interfaces.account.IAccountPublic"/>
<require
permission="launchpad.View"
- interface="lp.services.identity.interfaces.account.IAccountPrivate"/>
- <require
- permission="launchpad.Special"
- interface="lp.services.identity.interfaces.account.IAccountSpecialRestricted"/>
+ interface="lp.services.identity.interfaces.account.IAccountViewRestricted"/>
<require
permission="launchpad.Moderate"
set_attributes="status date_status_set status_comment"/>
=== modified file 'lib/lp/services/identity/interfaces/account.py'
--- lib/lp/services/identity/interfaces/account.py 2013-01-07 02:40:55 +0000
+++ lib/lp/services/identity/interfaces/account.py 2013-01-16 05:13:23 +0000
@@ -11,10 +11,10 @@
'AccountSuspendedError',
'AccountCreationRationale',
'IAccount',
- 'IAccountPrivate',
'IAccountPublic',
'IAccountSet',
'IAccountSpecialRestricted',
+ 'IAccountViewRestricted',
'INACTIVE_ACCOUNT_STATUSES',
]
@@ -244,6 +244,7 @@
class IAccountPublic(Interface):
"""Public information on an `IAccount`."""
+
id = Int(title=_('ID'), required=True, readonly=True)
displayname = StrippedTextLine(
@@ -255,8 +256,9 @@
readonly=False, vocabulary=AccountStatus)
-class IAccountPrivate(Interface):
+class IAccountViewRestricted(Interface):
"""Private information on an `IAccount`."""
+
date_created = Datetime(
title=_('Date Created'), required=True, readonly=True)
@@ -266,10 +268,6 @@
openid_identifiers = Attribute(_("Linked OpenId Identifiers"))
-
-class IAccountSpecialRestricted(Interface):
- """Attributes of `IAccount` protected with launchpad.Special."""
-
date_status_set = Datetime(
title=_('Date status last modified.'),
required=True, readonly=False)
@@ -287,7 +285,7 @@
"""
-class IAccount(IAccountPublic, IAccountPrivate, IAccountSpecialRestricted):
+class IAccount(IAccountPublic, IAccountViewRestricted):
"""Interface describing an `Account`."""
