← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~stevenk/launchpad/destroy-openidrp into lp:launchpad

 

Steve Kowalik has proposed merging lp:~stevenk/launchpad/destroy-openidrp into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/destroy-openidrp/+merge/74007

Burn OpenIDRP{Config,Summary} in a large fire.

Both tables have not been updated since April last year, and the only thing that makes use of them is a person merge check that tells the user which untrusted root sites they have visited. Except the information could be wrong, since the tables are no longer being updated.

The tables will be dropped in a future branch.
-- 
https://code.launchpad.net/~stevenk/launchpad/destroy-openidrp/+merge/74007
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/destroy-openidrp into lp:launchpad.
=== modified file 'database/sampledata/current-dev.sql'
--- database/sampledata/current-dev.sql	2011-07-13 06:06:53 +0000
+++ database/sampledata/current-dev.sql	2011-09-05 01:24:25 +0000
@@ -6346,25 +6346,6 @@
 ALTER TABLE openididentifier ENABLE TRIGGER ALL;
 
 
-ALTER TABLE openidrpconfig DISABLE TRIGGER ALL;
-
-
-
-ALTER TABLE openidrpconfig ENABLE TRIGGER ALL;
-
-
-ALTER TABLE openidrpsummary DISABLE TRIGGER ALL;
-
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (1, 521, 'http://openid.launchpad.dev/+id/no-priv_old_oid', 'https://shop.canonical.com', '2008-01-01 13:01:01.000001', '2008-01-01 13:01:01.000001', 1);
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (2, 701, 'http://openid.launchpad.dev/+id/former-user_oid', 'https://shop.canonical.com', '2008-01-05 14:01:01.000001', '2008-01-05 14:01:01.000001', 1);
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (3, 521, 'http://openid.launchpad.dev/+id/no-priv_oid', 'https://shop.canonical.com', '2008-02-01 13:01:01.000001', '2008-04-01 18:01:01.000001', 3);
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (4, 521, 'http://openid.launchpad.dev/+id/no-priv_oid', 'http://moodle.org', '2008-02-02 14:01:01.000001', '2008-02-02 14:01:01.000001', 1);
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (5, 121, 'http://openid.launchpad.dev/+id/name12_oid', 'https://shop.canonical.com', '2008-02-04 13:01:01.000001', '2008-02-04 13:01:01.000001', 1);
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (6, 551, 'http://openid.launchpad.dev/+id/marilize_oid', 'https://shop.canonical.com', '2008-01-12 13:01:01.000001', '2008-01-12 13:01:01.000001', 1);
-
-
-ALTER TABLE openidrpsummary ENABLE TRIGGER ALL;
-
 
 ALTER TABLE packagebugsupervisor DISABLE TRIGGER ALL;
 

=== modified file 'database/sampledata/current.sql'
--- database/sampledata/current.sql	2011-07-13 06:06:53 +0000
+++ database/sampledata/current.sql	2011-09-05 01:24:25 +0000
@@ -6278,26 +6278,6 @@
 ALTER TABLE openididentifier ENABLE TRIGGER ALL;
 
 
-ALTER TABLE openidrpconfig DISABLE TRIGGER ALL;
-
-
-
-ALTER TABLE openidrpconfig ENABLE TRIGGER ALL;
-
-
-ALTER TABLE openidrpsummary DISABLE TRIGGER ALL;
-
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (1, 521, 'http://openid.launchpad.dev/+id/no-priv_old_oid', 'https://shop.canonical.com', '2008-01-01 13:01:01.000001', '2008-01-01 13:01:01.000001', 1);
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (2, 701, 'http://openid.launchpad.dev/+id/former-user_oid', 'https://shop.canonical.com', '2008-01-05 14:01:01.000001', '2008-01-05 14:01:01.000001', 1);
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (3, 521, 'http://openid.launchpad.dev/+id/no-priv_oid', 'https://shop.canonical.com', '2008-02-01 13:01:01.000001', '2008-04-01 18:01:01.000001', 3);
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (4, 521, 'http://openid.launchpad.dev/+id/no-priv_oid', 'http://moodle.org', '2008-02-02 14:01:01.000001', '2008-02-02 14:01:01.000001', 1);
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (5, 121, 'http://openid.launchpad.dev/+id/name12_oid', 'https://shop.canonical.com', '2008-02-04 13:01:01.000001', '2008-02-04 13:01:01.000001', 1);
-INSERT INTO openidrpsummary (id, account, openid_identifier, trust_root, date_created, date_last_used, total_logins) VALUES (6, 551, 'http://openid.launchpad.dev/+id/marilize_oid', 'https://shop.canonical.com', '2008-01-12 13:01:01.000001', '2008-01-12 13:01:01.000001', 1);
-
-
-ALTER TABLE openidrpsummary ENABLE TRIGGER ALL;
-
-
 ALTER TABLE packagebugsupervisor DISABLE TRIGGER ALL;
 
 

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-08-31 16:12:38 +0000
+++ database/schema/security.cfg	2011-09-05 01:24:25 +0000
@@ -235,7 +235,6 @@
 public.openidconsumerassociation        = SELECT, INSERT, UPDATE, DELETE
 public.openidconsumernonce              = SELECT, INSERT, UPDATE
 public.openididentifier                 = SELECT, INSERT, UPDATE, DELETE
-public.openidrpconfig                   = SELECT, INSERT, UPDATE, DELETE
 public.packagebugsupervisor             = SELECT, INSERT, UPDATE, DELETE
 public.packagebuild                     = DELETE
 public.packagecopyjob                   = SELECT, INSERT, UPDATE
@@ -412,7 +411,6 @@
 public.messageapproval                  = SELECT
 public.messagechunk                     = SELECT
 public.mirrorproberecord                = SELECT
-public.openidrpconfig                   = SELECT
 public.packagebuild                     = SELECT
 public.packagediff                      = SELECT
 public.packageupload                    = SELECT
@@ -1156,7 +1154,6 @@
 public.mirror                           = SELECT, INSERT, UPDATE, DELETE
 public.mirrorcontent                    = SELECT, INSERT, UPDATE, DELETE
 public.mirrorsourcecontent              = SELECT, INSERT, UPDATE, DELETE
-public.openidrpsummary                  = SELECT, INSERT, UPDATE
 public.packagebuild                     = SELECT, INSERT, UPDATE
 public.packageselection                 = SELECT, INSERT, UPDATE
 public.packageupload                    = SELECT, INSERT, UPDATE
@@ -2236,7 +2233,6 @@
 public.databasereplicationlag           = SELECT
 public.job                              = SELECT
 public.libraryfilecontent               = SELECT
-public.openidrpconfig                   = SELECT
 public.packagebuild                     = SELECT
 type=user
 

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2011-09-02 01:19:40 +0000
+++ lib/lp/registry/browser/person.py	2011-09-05 01:24:25 +0000
@@ -309,7 +309,6 @@
     XRDSContentNegotiationMixin,
     )
 from lp.services.openid.interfaces.openid import IOpenIDPersistentIdentity
-from lp.services.openid.interfaces.openidrpsummary import IOpenIDRPSummarySet
 from lp.services.propertycache import (
     cachedproperty,
     get_property_cache,
@@ -4002,9 +4001,7 @@
         new_name = data.get('name')
         bypass_check = self.request.form_ng.getOne(
             'i_know_this_is_an_openid_security_issue', 0)
-        if (new_name and new_name != self.context.name and
-            len(self.unknown_trust_roots_user_logged_in) > 0
-            and not bypass_check):
+        if (new_name and new_name != self.context.name and not bypass_check):
             # Warn the user that they might shoot themselves in the foot.
             self.setFieldError('name', structured(dedent('''
             <div class="inline-warning">
@@ -4017,35 +4014,16 @@
                     >https://help.launchpad.net/OpenID#rename-account</a>
                   for more information.
               </p>
-              <p> You may have used your identifier on the following
-                  sites:<br> %s.
-              </p>
               <p>If you click 'Save' again, we will rename your account
                  anyway.
               </p>
-            </div>'''),
-             ", ".join(self.unknown_trust_roots_user_logged_in)))
+            </div>'''),))
             self.i_know_this_is_an_openid_security_issue_input = dedent("""\
                 <input type="hidden"
                        id="i_know_this_is_an_openid_security_issue"
                        name="i_know_this_is_an_openid_security_issue"
                        value="1">""")
 
-    @cachedproperty
-    def unknown_trust_roots_user_logged_in(self):
-        """The unknown trust roots the user has logged in using OpenID.
-
-        We assume that they logged in using their delegated profile OpenID,
-        since that's the one we advertise.
-        """
-        identifier = IOpenIDPersistentIdentity(self.context)
-        unknown_trust_root_login_records = list(
-            getUtility(IOpenIDRPSummarySet).getByIdentifier(
-                identifier.openid_identity_url, True))
-        return sorted([
-            record.trust_root
-            for record in unknown_trust_root_login_records])
-
     @action(_("Save Changes"), name="save")
     def action_save(self, action, data):
         # XXX: BradCrittenden 2010-09-10 bug=634878: Find a cleaner solution

=== modified file 'lib/lp/registry/doc/person-merge.txt'
--- lib/lp/registry/doc/person-merge.txt	2011-03-26 21:50:45 +0000
+++ lib/lp/registry/doc/person-merge.txt	2011-09-05 01:24:25 +0000
@@ -98,18 +98,6 @@
     >>> marilize in ubuntu_translators.activemembers
     True
 
-Marilize has used OpenID to Login. The OpenIDRPSummaries will be
-transferred to Sample Person.
-
-    >>> from lp.services.openid.interfaces.openidrpsummary import (
-    ...     IOpenIDRPSummarySet)
-    >>> openidrpsummaryset = getUtility(IOpenIDRPSummarySet)
-    >>> summaries = openidrpsummaryset.getByIdentifier(
-    ...     'http://openid.launchpad.dev/+id/marilize_oid')
-    >>> summaries[0].account.displayname
-    u'Marilize Coetzee'
-
-
 Do the merge!
 -------------
 

=== modified file 'lib/lp/services/openid/configure.zcml'
--- lib/lp/services/openid/configure.zcml	2010-08-26 07:58:52 +0000
+++ lib/lp/services/openid/configure.zcml	2011-09-05 01:24:25 +0000
@@ -14,18 +14,6 @@
             interface=".interfaces.openididentifier.IOpenIdIdentifier" />
     </class>
 
-    <class
-        class=".model.openidrpsummary.OpenIDRPSummary">
-        <allow interface=".interfaces.openidrpsummary.IOpenIDRPSummary" />
-    </class>
-
-    <securedutility
-        class=".model.openidrpsummary.OpenIDRPSummarySet"
-        provides=".interfaces.openidrpsummary.IOpenIDRPSummarySet">
-        <allow
-            interface=".interfaces.openidrpsummary.IOpenIDRPSummarySet" />
-    </securedutility>
-
     <class class=".adapters.openid.OpenIDPersistentIdentity">
       <allow interface=".interfaces.openid.IOpenIDPersistentIdentity" />
     </class>

=== removed file 'lib/lp/services/openid/interfaces/openidrpsummary.py'
--- lib/lp/services/openid/interfaces/openidrpsummary.py	2010-08-20 20:31:18 +0000
+++ lib/lp/services/openid/interfaces/openidrpsummary.py	1970-01-01 00:00:00 +0000
@@ -1,65 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""OpenIDRPSummary related interfaces."""
-
-__metaclass__ = type
-__all__ = [
-    'IOpenIDRPSummary',
-    'IOpenIDRPSummarySet',
-    ]
-
-from lazr.restful.fields import Reference
-from zope.interface import Interface
-from zope.schema import (
-    Datetime,
-    Int,
-    TextLine,
-    )
-
-from canonical.launchpad.interfaces.account import IAccount
-
-
-class IOpenIDRPSummary(Interface):
-    """A summary of the interaction between an `Account` and an OpenID RP."""
-    id = Int(title=u'ID', required=True)
-    account = Reference(
-        title=u'The IAccount used to login.', schema=IAccount,
-        required=True, readonly=True)
-    openid_identifier = TextLine(
-        title=u'OpenID identifier', required=True, readonly=True)
-    trust_root = TextLine(
-        title=u'OpenID trust root', required=True, readonly=True)
-    date_created = Datetime(
-        title=u'Date Created', required=True, readonly=True)
-    date_last_used = Datetime(title=u'Date last used', required=True)
-    total_logins = Int(title=u'Total logins', required=True)
-
-    def increment(date_used=None):
-        """Increment the total_logins.
-
-        :param date_used: an optional datetime the login happened. The current
-            datetime is used if date_used is None.
-        """
-
-
-class IOpenIDRPSummarySet(Interface):
-    """A set of OpenID RP Summaries."""
-
-    def getByIdentifier(identifier, only_unknown_trust_roots=False):
-        """Get all the IOpenIDRPSummary objects for an OpenID identifier.
-
-        :param identifier: A string used as an OpenID identifier.
-        :param only_unknown_trust_roots: if True, only records for trust roots
-            which there is no IOpenIDRPConfig entry will be returned.
-        :return: An iterator of IOpenIDRPSummary objects.
-        """
-
-    def record(account, trust_root):
-        """Create or update an IOpenIDRPSummary.
-
-        :param account: An `IAccount`.
-        :param trust_root: A string used as an OpenID trust root.
-        :return: An `IOpenIDRPSummary` or None.
-        """
-

=== removed file 'lib/lp/services/openid/model/openidrpsummary.py'
--- lib/lp/services/openid/model/openidrpsummary.py	2010-08-20 20:31:18 +0000
+++ lib/lp/services/openid/model/openidrpsummary.py	1970-01-01 00:00:00 +0000
@@ -1,127 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""OpenIDRPSummary database classes."""
-
-__metaclass__ = type
-__all__ = [
-    'OpenIDRPSummary',
-    'OpenIDRPSummarySet',
-    ]
-
-
-from datetime import datetime
-
-import pytz
-from sqlobject import (
-    ForeignKey,
-    IntCol,
-    StringCol,
-    )
-from zope.interface import implements
-
-from canonical.database.constants import DEFAULT
-from canonical.database.datetimecol import UtcDateTimeCol
-from canonical.database.sqlbase import (
-    SQLBase,
-    sqlvalues,
-    )
-from canonical.launchpad.interfaces.account import AccountStatus
-from canonical.launchpad.webapp.url import urlparse
-from canonical.launchpad.webapp.vhosts import allvhosts
-from lp.services.openid.interfaces.openid import IOpenIDPersistentIdentity
-from lp.services.openid.interfaces.openidrpsummary import (
-    IOpenIDRPSummary,
-    IOpenIDRPSummarySet,
-    )
-
-
-class OpenIDRPSummary(SQLBase):
-    """A summary of the interaction between a `IAccount` and an OpenID RP."""
-    implements(IOpenIDRPSummary)
-    _table = 'OpenIDRPSummary'
-
-    account = ForeignKey(dbName='account', foreignKey='Account', notNull=True)
-    openid_identifier = StringCol(notNull=True)
-    trust_root = StringCol(notNull=True)
-    date_created = UtcDateTimeCol(notNull=True, default=DEFAULT)
-    date_last_used = UtcDateTimeCol(notNull=True, default=DEFAULT)
-    total_logins = IntCol(notNull=True, default=1)
-
-    def increment(self, date_used=None):
-        """See `IOpenIDRPSummary`.
-
-        :param date_used: an optional datetime the login happened. The current
-            datetime is used if date_used is None.
-        """
-        self.total_logins = self.total_logins + 1
-        if date_used is None:
-            date_used = datetime.now(pytz.UTC)
-        self.date_last_used = date_used
-
-
-class OpenIDRPSummarySet:
-    """A set of OpenID RP Summaries."""
-    implements(IOpenIDRPSummarySet)
-
-    def getByIdentifier(self, identifier, only_unknown_trust_roots=False):
-        """See `IOpenIDRPSummarySet`."""
-        # XXX: flacoste 2008-11-17 bug=274774: Normalize the trust_root
-        # in OpenIDRPSummary.
-        if only_unknown_trust_roots:
-            result = OpenIDRPSummary.select("""
-            CASE
-                WHEN OpenIDRPSummary.trust_root LIKE '%%/'
-                THEN OpenIDRPSummary.trust_root
-                ELSE OpenIDRPSummary.trust_root || '/'
-            END NOT IN (SELECT trust_root FROM OpenIdRPConfig)
-            AND openid_identifier = %s
-                """ % sqlvalues(identifier))
-        else:
-            result = OpenIDRPSummary.selectBy(openid_identifier=identifier)
-        return result.orderBy('id')
-
-    def _assert_identifier_is_not_reused(self, account, identifier):
-        """Assert no other account in the summaries has the identifier."""
-        summaries = OpenIDRPSummary.select("""
-            account != %s
-            AND openid_identifier = %s
-            """ % sqlvalues(account, identifier))
-        if summaries.count() > 0:
-            raise AssertionError(
-                'More than 1 account has the OpenID identifier of %s.' %
-                identifier)
-
-    def record(self, account, trust_root, date_used=None):
-        """See `IOpenIDRPSummarySet`.
-
-        :param date_used: an optional datetime the login happened. The current
-            datetime is used if date_used is None.
-        :raise AssertionError: If the account is not ACTIVE.
-        :return: An `IOpenIDRPSummary` or None if the trust_root is
-            Launchpad or one of its vhosts.
-        """
-        trust_site = urlparse(trust_root)[1]
-        launchpad_site = allvhosts.configs['mainsite'].hostname
-        if trust_site.endswith(launchpad_site):
-            return None
-        if account.status != AccountStatus.ACTIVE:
-            raise AssertionError(
-                'Account %d is not ACTIVE account.' % account.id)
-        identifier = IOpenIDPersistentIdentity(account).openid_identity_url
-        self._assert_identifier_is_not_reused(account, identifier)
-        if date_used is None:
-            date_used = datetime.now(pytz.UTC)
-        summary = OpenIDRPSummary.selectOneBy(
-            account=account, openid_identifier=identifier,
-            trust_root=trust_root)
-        if summary is not None:
-            # Update the existing summary.
-            summary.increment(date_used=date_used)
-        else:
-            # create a new summary.
-            summary = OpenIDRPSummary(
-                account=account, openid_identifier=identifier,
-                trust_root=trust_root, date_created=date_used,
-                date_last_used=date_used, total_logins=1)
-        return summary