launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-repository-macaroon-secret-key into lp:launchpad.

Commit message:
Use a different secret key for Git access tokens.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1824399 in Launchpad itself: "Add Git HTTPS push tokens for snapcraft experiment"
  https://bugs.launchpad.net/launchpad/+bug/1824399

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-repository-macaroon-secret-key/+merge/366918

When I set up launchpad.internal_macaroon_secret_key I'd intended that it only be used for macaroons passed around between Launchpad components for their own purposes, and not for ones issued to users since we might need to rotate that key on a different schedule.  I forgot about that when writing the GitRepository macaroon issuer.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-repository-macaroon-secret-key into lp:launchpad.

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2019-05-01 16:31:20 +0000
+++ lib/lp/code/model/gitrepository.py	2019-05-03 13:26:10 +0000
@@ -1787,6 +1787,14 @@
             "lp.expires": self.verifyExpires,
             }
 
+    @property
+    def _root_secret(self):
+        secret = config.codehosting.git_macaroon_secret_key
+        if not secret:
+            raise RuntimeError(
+                "codehosting.git_macaroon_secret_key not configured.")
+        return secret
+
     def checkIssuingContext(self, context, user=None, **kwargs):
         """See `MacaroonIssuerBase`.
 

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2019-05-01 15:59:02 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2019-05-03 13:26:10 +0000
@@ -3917,8 +3917,7 @@
 
     def setUp(self):
         super(TestGitRepositoryMacaroonIssuer, self).setUp()
-        self.pushConfig(
-            "launchpad", internal_macaroon_secret_key="some-secret")
+        self.pushConfig("codehosting", git_macaroon_secret_key="some-secret")
 
     def test_issueMacaroon_refuses_branch(self):
         branch = self.factory.makeAnyBranch()

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2019-04-27 08:35:51 +0000
+++ lib/lp/services/config/schema-lazr.conf	2019-05-03 13:26:10 +0000
@@ -386,6 +386,9 @@
 # Git commit metadata.
 related_bugs_from_source_limit: 1000
 
+# Secret key for Git access tokens issued to Launchpad users.
+git_macaroon_secret_key: none
+
 
 [codeimport]
 # Where the Bazaar imports are stored.