Merge lp:~sinzui/launchpad/override-blacklist-0 into lp:launchpad/db-devel

[Stuart Bishop]

Add a column to nameblacklist to specify the team that may override the
restriction.

    Launchpad bug: https://bugs.launchpad.net/bugs/276488
    Pre-implementation: jml
    Test command: ./bin/test -vv \
      -t nameblacklist -t test_fields -t test_person

Admins need to use SQL to change the name of pillar and persons because
the UI does not permit them to override the nameblacklist for approved
exceptions.

There are some names that really cannot be overridden. There are some that
admins do need to change. There are other that commericial or registy
admins could be trusted to use. We need a way to define who can override
blacklisted name.

jml and sinzui discussed the issue and decided that the blacklist admin
views should permit admins to specify the team that can override the
namebacklist. When the team is None, the nameblacklist cannot be overridden.

--------------------------------------------------------------------

RULES

    * Add a column to the nameblacklist to reference a team or is null.
      Update the functions to accept a user id to build a list of teams
      the user is in, then skip regexps when the user is in the admin team.
    * Update the NameBlacklistField to pass the current user when available.
    * Add the team field to the +nameblacklist list and the add/edit forms.

QA

    * Visit https://staging.launchpad.net/+nameblacklist
    * Verify it displays a column for admin
    * Choose to add a new expression and add ~launchpad as the admin
    * Verify you can register a project that matchs the expression
    * Update the ^launchpad expressions; add ~launchpad as the admin
    * Verify you can register a project that starts with launchpad.

LINT

    database/schema/comments.sql
    database/schema/patch-2208-97-0.sql
    database/schema/security.cfg
    database/schema/trusted.sql
    lib/lp/registry/browser/nameblacklist.py
    lib/lp/registry/browser/tests/nameblacklist-views.txt
    lib/lp/registry/interfaces/nameblacklist.py
    lib/lp/registry/interfaces/person.py
    lib/lp/registry/model/nameblacklist.py
    lib/lp/registry/model/person.py
    lib/lp/registry/templates/nameblacklists-index.pt
    lib/lp/registry/tests/test_nameblacklist.py
    lib/lp/registry/tests/test_person.py
    lib/lp/services/fields/__init__.py
    lib/lp/services/fields/tests/test_fields.py

IMPLEMENTATION

Added the nameblacklist.admin column and updated the functions to use it.
Updated the interface and model too.
    database/schema/comments.sql
    database/schema/patch-2208-97-0.sql
    database/schema/security.cfg
    database/schema/trusted.sql
    lib/lp/registry/interfaces/nameblacklist.py
    lib/lp/registry/model/nameblacklist.py
    lib/lp/registry/tests/test_nameblacklist.py

Updated the code to pass the current user to when available when working
the the nameblacklist.
    lib/lp/registry/interfaces/person.py
    lib/lp/registry/model/person.py
    lib/lp/registry/tests/test_person.py
    lib/lp/services/fields/__init__.py
    lib/lp/services/fields/tests/test_fields.py

Added nameblacklist.admin to the UI
    lib/lp/registry/browser/nameblacklist.py
    lib/lp/registry/browser/tests/nameblacklist-views.txt
    lib/lp/registry/templates/nameblacklists-index.pt

[Stuart Bishop (stub)]

Database patch needs an index:

CREATE INDEX nameblacklist__admin__idx ON NameBlackList(admin);

I don't really like that there is no warning when you create something with a blacklisted name that you are allowed to override. I'm not sure if a warning is desirable enough to complicate the UI.

Should the admins team in Launchpad be allowed ignore the blacklist entirely? Membership in this team needs to be checked for explicitly.

Here is a much more efficient version of the stored procedure that also performs the admin check:

CREATE OR REPLACE FUNCTION name_blacklist_match(text, integer) RETURNS int4
LANGUAGE plpythonu STABLE RETURNS NULL ON NULL INPUT
EXTERNAL SECURITY DEFINER SET search_path TO public AS
$$
    import re
    name = args[0].decode("UTF-8")
    user_id = args[1]

    # Initialize shared storage, shared between invocations.
    if not SD.has_key("regexp_select_plan"):

        # All the blacklist regexps except the ones we are an admin
        # for. These we do not check since they are not blacklisted to us.
        SD["regexp_select_plan"] = plpy.prepare("""
            SELECT id, regexp FROM NameBlacklist
            WHERE admin IS NULL OR admin NOT IN (
                SELECT team FROM TeamParticipation
                WHERE person = $1)
            ORDER BY id
            """, ["integer"])

        # Storage for compiled regexps
        SD["compiled"] = {}

        # admins is a celebrity and its id is immutable.
        admins_id = plpy.execute(
            "SELECT id FROM Person WHERE name='admins'")[0]["id"]

        SD["admin_select_plan"] = plpy.prepare("""
            SELECT TRUE FROM TeamParticipation
            WHERE
                TeamParticipation.team = %d
                AND TeamParticipation.person = $1
            LIMIT 1
            """ % admins_id, ["integer"])

    # Names are never blacklisted for admins.
    if user_id is not None and plpy.execute(
        SD["admin_select_plan"], [user_id]).nrows() > 0:
        return None

    compiled = SD["compiled"]

    for row in plpy.execute(SD["regexp_select_plan"], [user_id]):
        regexp_id = row["id"]
        regexp_txt = row["regexp"]
        if (compiled.get(regexp_id) is None
            or compiled[regexp_id][0] != regexp_txt):
            regexp = re.compile(
                regexp_txt, re.IGNORECASE | re.UNICODE | re.VERBOSE
                )
            compiled[regexp_id] = (regexp_txt, regexp)
        else:
            regexp = compiled[regexp_id][1]
        if regexp.search(name) is not None:
            return regexp_id
    return None
$$;

review: Approve (db)

[Stuart Bishop (stub)]

patch-2208-36-0.sql

[Abel Deuring (adeuring)]

Hi Sinzui,

the code changes looks good to me.

But I share Stuart's concern that persons registered as a nameblacklist.admin do not get the slightest hint that a new project name is blackisted. My concern i not that much about the LOSAs If they are asked to set a project name to a blacklisted value, we can be sure that the request explains the reason why the blacklist should be overridden. And the LOSAs did prove that they work very carefully, so I am confident that they check if the request makes sense. (But they might appreciate display of the affected blackist record's regex value and comment so they can easily see why a given name is blacklisted.)

But if we use the option to appoint arbitrary persons as nameblacklist.admin, things might become messy. Let's assume that somebody is the admin for two nameblacklist records, where the two records blacklist names related to two different project groups. If this person now creates a new product for group A, but the name is blacklisted to prevent "stealing" names reserved for group B, they do not get the slightest hint that they are making an error. (I would probably be the first one to walk into this trap ;)

review: Approve (code)

[Curtis Hovey (sinzui)]

Hi Abel, Stuart.

Thank you for the review. I am going to update the UI of the forms to clarify when a team can be set as the admin of an regexp. I think I can update the name change forms to add a notice when the name was changed and it matches a blacklisted expression (call the isBlacklistedName() method without the user).

There are two kinds of blacklisted names: Those for because the application reserves the name, and those that are reserved by organisations. No one can ever be permitted to override an application restriction. The LOSA want this feature because they fear they will do that using SQL. They want a way to override organisational names...

...Stuart, I cannot accept your rewrite of name_blacklist_match because it breaks tests. It allows admins to override application blacklisted names. The SQL is hardcoded to give ~admins the power, but the intent is to assign a team to do the act. A rule without an admin team can never be overridden. Very few rules will permit someone to override them, I know of four rules:
  private: ~commercial-admins
  canonical: ~admin
  launchpad: ~admin
  ubuntuone: ~admin

We may ~registry instead of ~admin, or assign the names to their respective teams (~launchpad and ~canonical). This approach allows organisations to buy namespaces and create names without complaining to Launchpad staff.

I think the admin_select_plan wrongly uses ~admin instead of the team in the nameblacklisttable. I will attempt to revise the SQL to do the right thing.

...
        # admins is a celebrity and its id is immutable.
        admins_id = plpy.execute(
            "SELECT id FROM Person WHERE name='admins'")[0]["id"]

        SD["admin_select_plan"] = plpy.prepare("""
            SELECT TRUE FROM TeamParticipation
            WHERE
                TeamParticipation.team = %d
                AND TeamParticipation.person = $1
            LIMIT 1
            """ % admins_id, ["integer"])
...

    # Names are never blacklisted for admins.
    if user_id is not None and plpy.execute(
        SD["admin_select_plan"], [user_id]).nrows() > 0:
        return None

[Curtis Hovey (sinzui)]

Hi Stuart.

I propose small modification to your script. I remove the admin_select_plan since it gave unchecked power to admins. I added a guard for user_id to set the value to zero if it is NULL so that your revised regexp_select_plan always works.

CREATE OR REPLACE FUNCTION name_blacklist_match(text, integer) RETURNS int4
LANGUAGE plpythonu STABLE RETURNS NULL ON NULL INPUT
EXTERNAL SECURITY DEFINER SET search_path TO public AS
$$
    import re
    name = args[0].decode("UTF-8")
    user_id = args[1]
    if user_id is None:
        # Ids cannot be NULL. Zero will force every repexp rule to apply.
        user_id = 0

    # Initialize shared storage, shared between invocations.
    if not SD.has_key("regexp_select_plan"):

        # All the blacklist regexps except the ones we are an admin
        # for. These we do not check since they are not blacklisted to us.
        SD["regexp_select_plan"] = plpy.prepare("""
            SELECT id, regexp FROM NameBlacklist
            WHERE admin IS NULL OR admin NOT IN (
                SELECT team FROM TeamParticipation
                WHERE person = $1)
            ORDER BY id
            """, ["integer"])

        # Storage for compiled regexps
        SD["compiled"] = {}

    compiled = SD["compiled"]

    for row in plpy.execute(SD["regexp_select_plan"], [user_id]):
        regexp_id = row["id"]
        regexp_txt = row["regexp"]
        if (compiled.get(regexp_id) is None
            or compiled[regexp_id][0] != regexp_txt):
            regexp = re.compile(
                regexp_txt, re.IGNORECASE | re.UNICODE | re.VERBOSE
                )
            compiled[regexp_id] = (regexp_txt, regexp)
        else:
            regexp = compiled[regexp_id][1]
        if regexp.search(name) is not None:
            return regexp_id
    return None
$$;

[Stuart Bishop (stub)]

The guard for user_id shouldn't be needed; "WHERE person = 0" will give the same results as "WHERE person = NULL" (the former evaluates to FALSE for every row, the latter to NULL for every row, but both filter all rows in this context). It makes no practical difference so whatever you feel is clearer.

By removing privileges from a group of admins (registry or admin or both), will we end up with cases where something private cannot be created because there is no overlap between the set of people who can can create the object with a blacklisted name and the set of people who can create private objects?

[Curtis Hovey (sinzui)]

Hi Stuart.

I see you point. I removed the guard. I updated the function to permit Lp admins
an exemption for all regexps that have the admin column assigned. This change
restores most of your performance suggestion.

=== modified file 'database/schema/trusted.sql'
--- database/schema/trusted.sql 2011-01-10 18:27:20 +0000
+++ database/schema/trusted.sql 2011-01-10 19:24:26 +0000
@@ -765,9 +765,37 @@
         # Storage for compiled regexps
         SD["compiled"] = {}

+ # admins is a celebrity and its id is immutable.
+ admins_id = plpy.execute(
+ "SELECT id FROM Person WHERE name='admins'")[0]["id"]
+
+ SD["admin_select_plan"] = plpy.prepare("""
+ SELECT TRUE FROM TeamParticipation
+ WHERE
+ TeamParticipation.team = %d
+ AND TeamParticipation.person = $1
+ LIMIT 1
+ """ % admins_id, ["integer"])
+
+ # All the blacklist regexps except those that have an admin because
+ # members of ~admin can use any name that any other admin can use.
+ SD["admin_regexp_select_plan"] = plpy.prepare("""
+ SELECT id, regexp FROM NameBlacklist
+ WHERE admin IS NULL
+ ORDER BY id
+ """, ["integer"])
+
+
     compiled = SD["compiled"]

- for row in plpy.execute(SD["regexp_select_plan"], [user_id]):
+ # Names are never blacklisted for Lauchpad admins.
+ if user_id is not None and plpy.execute(
+ SD["admin_select_plan"], [user_id]).nrows() > 0:
+ blacklist_plan = "admin_regexp_select_plan"
+ else:
+ blacklist_plan = "regexp_select_plan"
+
+ for row in plpy.execute(SD[blacklist_plan], [user_id]):

[Stuart Bishop (stub)]

All looking good.

=== modified file 'database/schema/comments.sql'
--- database/schema/comments.sql	2010-12-21 00:32:34 +0000
+++ database/schema/comments.sql	2011-01-10 19:41:16 +0000
@@ -2182,6 +2182,7 @@
 COMMENT ON TABLE NameBlacklist IS 'A list of regular expressions used to blacklist names.';
 COMMENT ON COLUMN NameBlacklist.regexp IS 'A Python regular expression. It will be compiled with the IGNORECASE, UNICODE and VERBOSE flags. The Python search method will be used rather than match, so ^ markers should be used to indicate the start of a string.';
 COMMENT ON COLUMN NameBlacklist.comment IS 'An optional comment on why this regexp was entered. It should not be displayed to non-admins and its only purpose is documentation.';
+COMMENT ON COLUMN NameBlacklist.admin IS 'The person who can override the blacklisted name.';
 
 -- ScriptActivity
 COMMENT ON TABLE ScriptActivity IS 'Records of successful runs of scripts ';

=== added file 'database/schema/patch-2208-36-0.sql'
--- database/schema/patch-2208-36-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-36-0.sql	2011-01-10 19:41:16 +0000
@@ -0,0 +1,15 @@
+-- Copyright 2011 Canonical Ltd.  This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+SET client_min_messages=ERROR;
+
+-- Add a column that identifies the person or team that is exempt from
+-- the regexp check.
+ALTER TABLE NameBlacklist
+    ADD COLUMN admin
+        INTEGER,
+    ADD CONSTRAINT nameblacklist_admin_fk FOREIGN KEY ("admin")
+        REFERENCES person (id) MATCH SIMPLE
+        ON UPDATE NO ACTION ON DELETE NO ACTION;
+
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 36, 0);

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2010-12-02 16:15:46 +0000
+++ database/schema/security.cfg	2011-01-10 19:41:16 +0000
@@ -31,13 +31,13 @@
 public.valid_regexp(text)                  = EXECUTE
 public.sane_version(text)                  = EXECUTE
 public.sha1(text)                          = EXECUTE
-public.is_blacklisted_name(text)           = EXECUTE
+public.is_blacklisted_name(text, integer)  = EXECUTE
 public.is_person(text)                     = EXECUTE
 public.is_team(integer)                    = EXECUTE
 public.is_team(text)                       = EXECUTE
 public.is_printable_ascii(text)            = EXECUTE
 public.launchpaddatabaserevision           = SELECT
-public.name_blacklist_match(text)          = EXECUTE
+public.name_blacklist_match(text, integer) = EXECUTE
 public.pillarname                          = SELECT
 public.ulower(text)                        = EXECUTE
 public.generate_openid_identifier()        = EXECUTE

=== modified file 'database/schema/trusted.sql'
--- database/schema/trusted.sql	2010-11-17 10:59:19 +0000
+++ database/schema/trusted.sql	2011-01-10 19:41:16 +0000
@@ -741,19 +741,61 @@
 COMMENT ON FUNCTION debversion_sort_key(text) IS 'Return a string suitable for sorting debian version strings on';
 
 
-CREATE OR REPLACE FUNCTION name_blacklist_match(text) RETURNS int4
+CREATE OR REPLACE FUNCTION name_blacklist_match(text, integer) RETURNS int4
 LANGUAGE plpythonu STABLE RETURNS NULL ON NULL INPUT
-EXTERNAL SECURITY DEFINER AS
+EXTERNAL SECURITY DEFINER SET search_path TO public AS
 $$
     import re
     name = args[0].decode("UTF-8")
-    if not SD.has_key("select_plan"):
-        SD["select_plan"] = plpy.prepare("""
-            SELECT id, regexp FROM NameBlacklist ORDER BY id
-            """)
+    user_id = args[1]
+
+    # Initialize shared storage, shared between invocations.
+    if not SD.has_key("regexp_select_plan"):
+
+        # All the blacklist regexps except the ones we are an admin
+        # for. These we do not check since they are not blacklisted to us.
+        SD["regexp_select_plan"] = plpy.prepare("""
+            SELECT id, regexp FROM NameBlacklist
+            WHERE admin IS NULL OR admin NOT IN (
+                SELECT team FROM TeamParticipation
+                WHERE person = $1)
+            ORDER BY id
+            """, ["integer"])
+
+        # Storage for compiled regexps
         SD["compiled"] = {}
+
+        # admins is a celebrity and its id is immutable.
+        admins_id = plpy.execute(
+            "SELECT id FROM Person WHERE name='admins'")[0]["id"]
+
+        SD["admin_select_plan"] = plpy.prepare("""
+            SELECT TRUE FROM TeamParticipation
+            WHERE
+                TeamParticipation.team = %d
+                AND TeamParticipation.person = $1
+            LIMIT 1
+            """ % admins_id, ["integer"])
+
+        # All the blacklist regexps except those that have an admin because
+        # members of ~admin can use any name that any other admin can use.
+        SD["admin_regexp_select_plan"] = plpy.prepare("""
+            SELECT id, regexp FROM NameBlacklist
+            WHERE admin IS NULL
+            ORDER BY id
+            """, ["integer"])
+
+
     compiled = SD["compiled"]
-    for row in plpy.execute(SD["select_plan"]):
+
+    # Names are never blacklisted for Lauchpad admins.
+    if user_id is not None and plpy.execute(
+        SD["admin_select_plan"], [user_id]).nrows() > 0:
+        blacklist_plan = "admin_regexp_select_plan"
+    else:
+        blacklist_plan = "regexp_select_plan"
+
+    for row in plpy.execute(SD[blacklist_plan], [user_id]):
         regexp_id = row["id"]
         regexp_txt = row["regexp"]
         if (compiled.get(regexp_id) is None
@@ -769,16 +811,17 @@
     return None
 $$;
 
-COMMENT ON FUNCTION name_blacklist_match(text) IS 'Return the id of the row in the NameBlacklist table that matches the given name, or NULL if no regexps in the NameBlacklist table match.';
-
-
-CREATE OR REPLACE FUNCTION is_blacklisted_name(text) RETURNS boolean
+COMMENT ON FUNCTION name_blacklist_match(text, integer) IS 'Return the id of the row in the NameBlacklist table that matches the given name, or NULL if no regexps in the NameBlacklist table match.';
+
+
+CREATE OR REPLACE FUNCTION is_blacklisted_name(text, integer)
+RETURNS boolean
 LANGUAGE SQL STABLE RETURNS NULL ON NULL INPUT EXTERNAL SECURITY DEFINER AS
 $$
-    SELECT COALESCE(name_blacklist_match($1)::boolean, FALSE);
+    SELECT COALESCE(name_blacklist_match($1, $2)::boolean, FALSE);
 $$;
 
-COMMENT ON FUNCTION is_blacklisted_name(text) IS 'Return TRUE if any regular expressions stored in the NameBlacklist table match the givenname, otherwise return FALSE.';
+COMMENT ON FUNCTION is_blacklisted_name(text, integer) IS 'Return TRUE if any regular expressions stored in the NameBlacklist table match the givenname, otherwise return FALSE.';
 
 
 CREATE OR REPLACE FUNCTION set_shipit_normalized_address() RETURNS trigger

=== modified file 'lib/lp/registry/browser/nameblacklist.py'
--- lib/lp/registry/browser/nameblacklist.py	2010-11-30 20:34:31 +0000
+++ lib/lp/registry/browser/nameblacklist.py	2011-01-10 19:41:16 +0000
@@ -73,7 +73,7 @@
     """View for editing a blacklist expression."""
 
     schema = INameBlacklist
-    field_names = ['regexp', 'comment']
+    field_names = ['regexp', 'admin', 'comment']
     label = "Edit a blacklist expression"
     page_title = label
 
@@ -88,7 +88,7 @@
     """View for adding a blacklist expression."""
 
     schema = INameBlacklist
-    field_names = ['regexp', 'comment']
+    field_names = ['regexp', 'admin', 'comment']
     label = "Add a new blacklist expression"
     page_title = label
 
@@ -107,6 +107,7 @@
         name_blacklist_set.create(
             regexp=data['regexp'],
             comment=data['comment'],
+            admin=data['admin'],
             )
         self.request.response.addInfoNotification(
             'Regular expression "%s" has been added to the name blacklist.'

=== modified file 'lib/lp/registry/browser/tests/nameblacklist-views.txt'
--- lib/lp/registry/browser/tests/nameblacklist-views.txt	2010-11-09 05:15:03 +0000
+++ lib/lp/registry/browser/tests/nameblacklist-views.txt	2011-01-10 19:41:16 +0000
@@ -25,9 +25,9 @@
     >>> view = create_initialized_view(name_blacklist_set, '+index',
     ...                                principal=registry_expert)
     >>> print extract_text(find_tag_by_id(view.render(), 'blacklist'))
-    Regular Expression                      Comment
-    ^admin Edit blacklist expression
-    blacklist Edit blacklist expression     For testing purposes
+    Regular Expression                   Admin    Comment
+    ^admin Edit blacklist expression     —
+    blacklist Edit blacklist expression  —  For testing purposes
 
 
 Add expression to blacklist
@@ -37,6 +37,7 @@
 
     >>> form = {
     ...     'field.regexp': u'(',
+    ...     'field.admin': registry_experts.name,
     ...     'field.comment': u'old-comment',
     ...     'field.actions.add': 'Add to blacklist',
     ...     }
@@ -61,17 +62,23 @@
     ...     print notification.message
     Regular expression "foo" has been added to the name blacklist.
 
+    >>> import transaction
+    >>> transaction.commit()
+    >>> foo_exp = name_blacklist_set.getByRegExp(u'foo')
+    >>> print foo_exp.regexp
+    foo
+    >>> print foo_exp.admin.name
+    registry
+
 
 Edit expression in blacklist
 ----------------------------
 
 When a regular expression is edited, it still must be valid.
 
-    >>> import transaction
-    >>> transaction.commit()
-    >>> foo_exp = name_blacklist_set.getByRegExp(u'foo')
     >>> form = {
     ...     'field.regexp': u'(',
+    ...     'field.admin': registry_experts.name,
     ...     'field.comment': u'new-comment',
     ...     'field.actions.change': 'Change',
     ...     }

=== modified file 'lib/lp/registry/interfaces/nameblacklist.py'
--- lib/lp/registry/interfaces/nameblacklist.py	2010-11-08 21:05:19 +0000
+++ lib/lp/registry/interfaces/nameblacklist.py	2011-01-10 19:41:16 +0000
@@ -13,6 +13,7 @@
 from zope.interface import Interface
 from zope.schema import (
     Int,
+    Choice,
     Text,
     TextLine,
     )
@@ -25,7 +26,18 @@
 
     id = Int(title=_('ID'), required=True, readonly=True)
     regexp = TextLine(title=_('Regular expression'), required=True)
-    comment = Text(title=_('Comment'), required=False)
+    comment = Text(
+        title=_('Comment'),
+        description=_(
+            "Why is the name blacklisted? Does the namespace belong to an "
+            "organization or is the namespace reserved by the application?"),
+        required=False)
+    admin = Choice(
+        title=_('Admin'),
+        description=_(
+            "The team that is exempt from this restriction because it "
+            "administers this namespace for an organisation."),
+        vocabulary='ValidPersonOrTeam', required=False)
 
 
 class INameBlacklistSet(Interface):

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2011-01-03 21:14:17 +0000
+++ lib/lp/registry/interfaces/person.py	2011-01-10 19:41:16 +0000
@@ -1865,8 +1865,13 @@
     def getTopContributors(limit=50):
         """Return the top contributors in Launchpad, up to the given limit."""
 
-    def isNameBlacklisted(name):
-        """Is the given name blacklisted by Launchpad Administrators?"""
+    def isNameBlacklisted(name, user=None):
+        """Is the given name blacklisted by Launchpad Administrators?
+
+        :param name: The name to be checked.
+        :param user: The `IPerson` that wants to use the name. If the user
+            is an admin for the nameblacklist expression, he can use the name.
+        """
 
     def createPersonAndEmail(
             email, rationale, comment=None, name=None, displayname=None,

=== modified file 'lib/lp/registry/model/nameblacklist.py'
--- lib/lp/registry/model/nameblacklist.py	2010-11-11 16:06:24 +0000
+++ lib/lp/registry/model/nameblacklist.py	2011-01-10 19:41:16 +0000
@@ -13,6 +13,7 @@
 from storm.base import Storm
 from storm.locals import (
     Int,
+    Reference,
     Unicode,
     )
 from zope.interface import implements
@@ -22,6 +23,7 @@
     INameBlacklist,
     INameBlacklistSet,
     )
+from lp.registry.model.person import Person
 
 
 class NameBlacklist(Storm):
@@ -34,6 +36,8 @@
     id = Int(primary=True)
     regexp = Unicode(name='regexp', allow_none=False)
     comment = Unicode(name='comment', allow_none=True)
+    admin_id = Int(name='admin', allow_none=True)
+    admin = Reference(admin_id, Person.id)
 
 
 class NameBlacklistSet:
@@ -46,11 +50,12 @@
         store = IStore(NameBlacklist)
         return store.find(NameBlacklist).order_by(NameBlacklist.regexp)
 
-    def create(self, regexp, comment=None):
+    def create(self, regexp, comment=None, admin=None):
         """See `INameBlacklistSet`."""
         nameblacklist = NameBlacklist()
         nameblacklist.regexp = regexp
         nameblacklist.comment = comment
+        nameblacklist.admin = admin
         store = IStore(NameBlacklist)
         store.add(nameblacklist)
         return nameblacklist

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2011-01-04 23:32:39 +0000
+++ lib/lp/registry/model/person.py	2011-01-10 19:41:16 +0000
@@ -2764,11 +2764,16 @@
     def __init__(self):
         self.title = 'People registered with Launchpad'
 
-    def isNameBlacklisted(self, name):
+    def isNameBlacklisted(self, name, user=None):
         """See `IPersonSet`."""
+        if user is None:
+            user_id = 0
+        else:
+            user_id = user.id
         cur = cursor()
-        cur.execute("SELECT is_blacklisted_name(%(name)s)" % sqlvalues(
-            name=name.encode('UTF-8')))
+        cur.execute(
+            "SELECT is_blacklisted_name(%(name)s, %(user_id)s)" % sqlvalues(
+            name=name.encode('UTF-8'), user_id=user_id))
         return bool(cur.fetchone()[0])
 
     def getTopContributors(self, limit=50):

=== modified file 'lib/lp/registry/templates/nameblacklists-index.pt'
--- lib/lp/registry/templates/nameblacklists-index.pt	2010-11-11 16:18:29 +0000
+++ lib/lp/registry/templates/nameblacklists-index.pt	2011-01-10 19:41:16 +0000
@@ -16,9 +16,23 @@
     </tal:side>
 
     <div metal:fill-slot="main" class="main-portlet">
+      <p>
+        There are two kinds of blacklisted names:
+      </p>
+      <ul class="bulleted">
+        <li>
+          Organisational blackisted names have an administering team
+          that is permitted to use the name for projects and teams.
+         </li>
+        <li>
+          Application blackisted names cannot have an administering team.
+          No one can to override them.
+         </li>
+      </ul>
       <table id="blacklist" class="listing sortable">
         <thead>
           <th>Regular Expression</th>
+          <th>Admin</th>
           <th>Comment</th>
         </thead>
         <tbody>
@@ -29,6 +43,12 @@
                 structure
                 item/menu:overview/edit_blacklist_expression/fmt:icon"/>
             </td>
+            <td>
+              <a
+                tal:condition="item/admin"
+                tal:replace="structure item/admin/fmt:link" />
+              <tal:none condition="not: item/admin">&mdash;</tal:none>
+            </td>
             <td tal:content="item/comment"/>
           </tr>
         </tbody>

=== modified file 'lib/lp/registry/tests/test_nameblacklist.py'
--- lib/lp/registry/tests/test_nameblacklist.py	2010-11-13 15:49:06 +0000
+++ lib/lp/registry/tests/test_nameblacklist.py	2011-01-10 19:41:16 +0000
@@ -9,6 +9,7 @@
 from zope.component import getUtility
 from zope.interface.verify import verifyObject
 
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.launchpad.webapp.authorization import check_permission
 from canonical.testing.layers import (
@@ -36,20 +37,24 @@
         self.caret_foo_exp = self.name_blacklist_set.create(u'^foo')
         self.foo_exp = self.name_blacklist_set.create(u'foo')
         self.verbose_exp = self.name_blacklist_set.create(u'v e r b o s e')
+        team = self.factory.makeTeam()
+        self.admin_exp = self.name_blacklist_set.create(u'fnord', admin=team)
         self.store = IStore(self.foo_exp)
         self.store.flush()
 
-    def name_blacklist_match(self, name):
+    def name_blacklist_match(self, name, user_id=None):
         '''Return the result of the name_blacklist_match stored procedure.'''
+        user_id = user_id or 0
         result = self.store.execute(
-            "SELECT name_blacklist_match(%s)", (name,))
+            "SELECT name_blacklist_match(%s, %s)", (name, user_id))
         return result.get_one()[0]
 
-    def is_blacklisted_name(self, name):
+    def is_blacklisted_name(self, name, user_id=None):
         '''Call the is_blacklisted_name stored procedure and return the result
         '''
+        user_id = user_id or 0
         result = self.store.execute(
-            "SELECT is_blacklisted_name(%s)", (name,))
+            "SELECT is_blacklisted_name(%s, %s)", (name, user_id))
         blacklisted = result.get_one()[0]
         self.failIf(blacklisted is None, 'is_blacklisted_name returned NULL')
         return bool(blacklisted)
@@ -69,6 +74,30 @@
             self.name_blacklist_match(u"barfoo"),
             self.foo_exp.id)
 
+    def test_name_blacklist_match_admin_does_not_match(self):
+        # A user in the expresssion's admin team is exempt from the
+        # backlisted name restriction.
+        user = self.admin_exp.admin.teamowner
+        self.assertEqual(
+            None, self.name_blacklist_match(u"fnord", user.id))
+
+    def test_name_blacklist_match_launchpad_admin_can_change(self):
+        # A Launchpad admin is exempt from any backlisted name restriction
+        # that has an admin.
+        user = self.factory.makePerson()
+        admins = getUtility(ILaunchpadCelebrities).admin
+        admins.addMember(user, user)
+        self.assertEqual(
+            None, self.name_blacklist_match(u"fnord", user.id))
+
+    def test_name_blacklist_match_launchpad_admin_cannot_change(self):
+        # A Launchpad admin cannot override backlisted names without admins.
+        user = self.factory.makePerson()
+        admins = getUtility(ILaunchpadCelebrities).admin
+        admins.addMember(user, user)
+        self.assertEqual(
+            self.foo_exp.id, self.name_blacklist_match(u"barfoo", user.id))
+
     def test_name_blacklist_match_cache(self):
         # If the blacklist is changed in the DB, these changes are noticed.
         # This test is needed because the stored procedure keeps a cache
@@ -92,6 +121,11 @@
         self.foo_exp.regexp = u'bar2'
         self.failUnless(self.is_blacklisted_name(u"foo") is False)
 
+    def test_is_blacklisted_name_admin_false(self):
+        # Users in the expression's admin team are will return False.
+        user = self.admin_exp.admin.teamowner
+        self.assertFalse(self.is_blacklisted_name(u"fnord", user.id))
+
     def test_case_insensitive(self):
         self.failUnless(self.is_blacklisted_name(u"Foo") is True)
 
@@ -123,6 +157,15 @@
         self.assertEqual(u'foo', name_blacklist.regexp)
         self.assertEqual(u'bar', name_blacklist.comment)
 
+    def test_create_with_three_args(self):
+        # Test NameBlacklistSet.create(regexp, comment, admin).
+        team = self.factory.makeTeam()
+        name_blacklist = self.name_blacklist_set.create(u'foo', u'bar', team)
+        self.assertTrue(verifyObject(INameBlacklist, name_blacklist))
+        self.assertEqual(u'foo', name_blacklist.regexp)
+        self.assertEqual(u'bar', name_blacklist.comment)
+        self.assertEqual(team, name_blacklist.admin)
+
     def test_get_int(self):
         # Test NameBlacklistSet.get() with int id.
         name_blacklist = self.name_blacklist_set.create(u'foo', u'bar')

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2010-12-10 15:06:12 +0000
+++ lib/lp/registry/tests/test_person.py	2011-01-10 19:41:16 +0000
@@ -31,7 +31,10 @@
     InvalidEmailAddress,
     )
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
-from canonical.launchpad.interfaces.lpstorm import IMasterStore
+from canonical.launchpad.interfaces.lpstorm import (
+    IMasterStore,
+    IStore,
+    )
 from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller
 from canonical.testing.layers import (
     DatabaseFunctionalLayer,
@@ -47,6 +50,7 @@
     PrivatePersonLinkageError,
     )
 from lp.registry.interfaces.karma import IKarmaCacheManager
+from lp.registry.interfaces.nameblacklist import INameBlacklistSet
 from lp.registry.interfaces.person import (
     ImmutableVisibilityError,
     InvalidName,
@@ -445,6 +449,15 @@
         self.failUnless(self.person_set.isNameBlacklisted('foo'))
         self.failIf(self.person_set.isNameBlacklisted('bar'))
 
+    def test_isNameBlacklisted_user_is_admin(self):
+        team = self.factory.makeTeam()
+        name_blacklist_set = getUtility(INameBlacklistSet)
+        self.admin_exp = name_blacklist_set.create(u'fnord', admin=team)
+        self.store = IStore(self.admin_exp)
+        self.store.flush()
+        user = team.teamowner
+        self.assertFalse(self.person_set.isNameBlacklisted('fnord', user))
+
     def test_getByEmail_ignores_case_and_whitespace(self):
         person1_email = '[e-mail]'
         person1 = self.person_set.getByEmail(person1_email)

=== modified file 'lib/lp/services/fields/__init__.py'
--- lib/lp/services/fields/__init__.py	2010-12-20 17:42:47 +0000
+++ lib/lp/services/fields/__init__.py	2011-01-10 19:41:16 +0000
@@ -104,8 +104,10 @@
     name_validator,
     valid_name,
     )
+from canonical.launchpad.webapp.interfaces import ILaunchBag
 from lp.registry.interfaces.pillar import IPillarNameSet
 
+
 # Marker object to tell BaseImageUpload to keep the existing image.
 KEEP_SAME_IMAGE = object()
 
@@ -502,7 +504,8 @@
 
         # Need a local import because of circular dependencies.
         from lp.registry.interfaces.person import IPersonSet
-        if getUtility(IPersonSet).isNameBlacklisted(input):
+        user = getUtility(ILaunchBag).user
+        if getUtility(IPersonSet).isNameBlacklisted(input, user):
             raise LaunchpadValidationError(
                 "The name '%s' has been blocked by the Launchpad "
                 "administrators." % input)

=== modified file 'lib/lp/services/fields/tests/test_fields.py'
--- lib/lp/services/fields/tests/test_fields.py	2010-08-20 20:31:18 +0000
+++ lib/lp/services/fields/tests/test_fields.py	2011-01-10 19:41:16 +0000
@@ -7,20 +7,32 @@
 
 import datetime
 import time
-import unittest
-
+
+from zope.interface import Interface
+from zope.component import getUtility
+
+from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.launchpad.validators import LaunchpadValidationError
+from canonical.testing.layers import DatabaseFunctionalLayer
 from lp.services.fields import (
+    BlacklistableContentNameField,
     FormattableDate,
     StrippableText,
     )
-from lp.testing import TestCase
+from lp.registry.interfaces.nameblacklist import INameBlacklistSet
+from lp.testing import (
+    login_person,
+    TestCase,
+    TestCaseWithFactory,
+    )
 
 
 def make_target():
     """Make a trivial object to be a target of the field setting."""
+
     class Simple:
         """A simple class to test setting fields on."""
+
     return Simple()
 
 
@@ -67,5 +79,52 @@
         self.assertIs(None, target.test)
 
 
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)
+class TestBlacklistableContentNameField(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestBlacklistableContentNameField, self).setUp()
+        name_blacklist_set = getUtility(INameBlacklistSet)
+        self.team = self.factory.makeTeam()
+        admin_exp = name_blacklist_set.create(u'fnord', admin=self.team)
+        IStore(admin_exp).flush()
+
+    def makeTestField(self):
+        """Return testible subclass."""
+
+        class ITestInterface(Interface):
+            pass
+
+        class TestField(BlacklistableContentNameField):
+            _content_iface = ITestInterface
+
+            def _getByName(self, name):
+                return None
+
+        return TestField(__name__='test')
+
+    def test_validate_fails_with_blacklisted_name_anonymous(self):
+        # Anonymous users, processes, cannot create a name that matches
+        # a blacklisted name.
+        field = self.makeTestField()
+        date_value = u'fnord'
+        self.assertRaises(
+            LaunchpadValidationError, field.validate, date_value)
+
+    def test_validate_fails_with_blacklisted_name_not_admin(self):
+        # Users who do not adminster a blacklisted name cannot create
+        # a matching name.
+        field = self.makeTestField()
+        date_value = u'fnord'
+        login_person(self.factory.makePerson())
+        self.assertRaises(
+            LaunchpadValidationError, field.validate, date_value)
+
+    def test_validate_passes_for_admin(self):
+        # Users in the team that adminsters a blacklisted name may create
+        # matching names.
+        field = self.makeTestField()
+        date_value = u'fnord'
+        login_person(self.team.teamowner)
+        self.assertEqual(None, field.validate(date_value))