launchpad-reviewers team mailing list archive

[Thomi Richards <thomi.richards@xxxxxxxxxxxxx>]

Thomi Richards has proposed merging lp:~thomir/launchpad/devel-gpogservice-integration into lp:launchpad.

Commit message:
Add a read-only feature flag for the gpg service.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~thomir/launchpad/devel-gpogservice-integration/+merge/285427

Add a read-only feature flag for the gpg service.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~thomir/launchpad/devel-gpogservice-integration into lp:launchpad.

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2016-01-26 15:47:37 +0000
+++ lib/lp/registry/browser/person.py	2016-02-09 01:44:38 +0000
@@ -205,10 +205,13 @@
 from lp.registry.model.person import get_recipients
 from lp.services.config import config
 from lp.services.database.sqlbase import flush_database_updates
+from lp.services.features import getFeatureFlag
 from lp.services.feeds.browser import FeedsMixin
 from lp.services.geoip.interfaces import IRequestPreferredLanguages
 from lp.services.gpg.interfaces import (
+    GPG_SERVICE_READONLY_FEATURE_FLAG,
     GPGKeyNotFoundError,
+    GPGReadOnly,
     IGPGHandler,
     )
 from lp.services.identity.interfaces.account import (
@@ -2531,14 +2534,16 @@
             IGPGHandler).getURLForKeyInServer(self.fingerprint, public=True)
 
     def form_action(self):
+        if self.request.method != "POST":
+            return ''
+        if getFeatureFlag(GPG_SERVICE_READONLY_FEATURE_FLAG):
+            raise GPGReadOnly()
         permitted_actions = [
             'claim_gpg',
             'deactivate_gpg',
             'remove_gpgtoken',
             'reactivate_gpg',
             ]
-        if self.request.method != "POST":
-            return ''
         action = self.request.form.get('action')
         if action not in permitted_actions:
             raise UnexpectedFormData("Action not permitted: %s" % action)

=== modified file 'lib/lp/registry/browser/tests/test_gpgkey.py'
--- lib/lp/registry/browser/tests/test_gpgkey.py	2012-08-20 14:33:30 +0000
+++ lib/lp/registry/browser/tests/test_gpgkey.py	2016-02-09 01:44:38 +0000
@@ -5,6 +5,16 @@
 
 __metaclass__ = type
 
+from testtools.matchers import (
+    Not,
+    Raises,
+    )
+
+from lp.services.features.testing import FeatureFixture
+from lp.services.gpg.interfaces import (
+    GPGReadOnly,
+    GPG_SERVICE_READONLY_FEATURE_FLAG,
+    )
 from lp.services.webapp import canonical_url
 from lp.testing import (
     login_person,
@@ -42,3 +52,23 @@
         expected_url = (
             '%s/+editpgpkeys/+login?reauth=1' % canonical_url(person))
         self.assertEqual(expected_url, response.getHeader('location'))
+
+    def test_gpgkeys_POST_readonly_with_feature_flag_set(self):
+        self.useFixture(FeatureFixture({
+            GPG_SERVICE_READONLY_FEATURE_FLAG: True,
+        }))
+        person = self.factory.makePerson()
+        login_person(person)
+        view = create_initialized_view(person, "+editpgpkeys", principal=person,
+                                       method='POST', have_fresh_login=True)
+        self.assertRaises(GPGReadOnly, view.render)
+
+    def test_gpgkeys_GET_readonly_with_feature_flag_set(self):
+        self.useFixture(FeatureFixture({
+            GPG_SERVICE_READONLY_FEATURE_FLAG: True,
+        }))
+        person = self.factory.makePerson()
+        login_person(person)
+        view = create_initialized_view(person, "+editpgpkeys", principal=person,
+                                       method='GET', have_fresh_login=True)
+        self.assertThat(view.render, Not(Raises()))

=== modified file 'lib/lp/services/gpg/interfaces.py'
--- lib/lp/services/gpg/interfaces.py	2016-01-26 15:47:37 +0000
+++ lib/lp/services/gpg/interfaces.py	2016-02-09 01:44:38 +0000
@@ -2,35 +2,54 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __all__ = [
+    'GPG_SERVICE_READONLY_FEATURE_FLAG',
     'GPGKeyAlgorithm',
     'GPGKeyDoesNotExistOnServer',
     'GPGKeyExpired',
+    'GPGKeyNotFoundError',
     'GPGKeyRevoked',
-    'GPGKeyNotFoundError',
     'GPGKeyTemporarilyNotFoundError',
+    'GPGReadOnly',
     'GPGUploadFailure',
     'GPGVerificationError',
     'IGPGHandler',
+    'IPymeKey',
     'IPymeSignature',
-    'IPymeKey',
     'IPymeUserId',
     'MoreThanOneGPGKeyFound',
     'SecretGPGKeyImportDetected',
+    'valid_fingerprint',
     'valid_keyid',
-    'valid_fingerprint',
     ]
 
-
+import httplib
 import re
 
 from lazr.enum import (
     DBEnumeratedType,
     DBItem,
     )
+from lazr.restful.declarations import error_status
 from zope.interface import (
     Attribute,
     Interface,
     )
+from zope.security.interfaces import (
+    Forbidden,
+    )
+
+
+@error_status(httplib.FORBIDDEN)
+class GPGReadOnly(Forbidden):
+    """GPG Service is in read-only mode."""
+
+    def __init__(self):
+        super(GPGReadOnly, self).__init__(
+            "The GPG key storage facilities of Launchpad are currently "
+            "read-only. Please try again later.")
+
+
+GPG_SERVICE_READONLY_FEATURE_FLAG = u"gpgservice.read_only"
 
 
 def valid_fingerprint(fingerprint):

=== modified file 'lib/lp/services/webapp/login.py'
--- lib/lp/services/webapp/login.py	2014-01-14 06:29:21 +0000
+++ lib/lp/services/webapp/login.py	2016-02-09 01:44:38 +0000
@@ -465,6 +465,8 @@
 
 def isFreshLogin(request):
     """Return True if the principal login happened in the last 120 seconds."""
+    if getattr(request, 'have_fresh_login', False):
+        return True
     session = ISession(request)
     authdata = session['launchpad.authenticateduser']
     logintime = authdata.get('logintime', None)

=== modified file 'lib/lp/services/webapp/servers.py'
--- lib/lp/services/webapp/servers.py	2016-01-26 15:14:01 +0000
+++ lib/lp/services/webapp/servers.py	2016-02-09 01:44:38 +0000
@@ -904,6 +904,11 @@
     >>> request.charsets = ['utf-8']
     >>> request.query_string_params == {'a': ['1'], 'b': ['2'], 'c': ['3']}
     True
+
+    If have_fresh_login is set to True, views such as PersonGPGView that insist
+    on the user being recently logged in will render their contents rather than
+    redirecting to the login page.
+    
     """
 
     # These two attributes satisfy IParticipation.
@@ -911,7 +916,8 @@
     interaction = None
 
     def __init__(self, body_instream=None, environ=None, form=None,
-                 skin=None, outstream=None, method='GET', **kw):
+                 skin=None, outstream=None, method='GET', have_fresh_login=False,
+                 **kw):
         super(LaunchpadTestRequest, self).__init__(
             body_instream=body_instream, environ=environ, form=form,
             skin=skin, outstream=outstream, REQUEST_METHOD=method, **kw)
@@ -921,6 +927,7 @@
         self.features = get_relevant_feature_controller()
         if self.features is None:
             self.features = NullFeatureController()
+        self.have_fresh_login = have_fresh_login
 
     @property
     def uuid(self):