launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:wsgi-archiveauth-config-instance into launchpad:master.

Commit message:
Make check_password set config instance name

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400811

There are almost no viable ways to pass this in, but we can get away with setting it from the mod_wsgi application group name.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:wsgi-archiveauth-config-instance into launchpad:master.

diff --git a/lib/lp/soyuz/wsgi/archiveauth.py b/lib/lp/soyuz/wsgi/archiveauth.py
index fd34161..94f9f59 100644
--- a/lib/lp/soyuz/wsgi/archiveauth.py
+++ b/lib/lp/soyuz/wsgi/archiveauth.py
@@ -55,6 +55,18 @@ _memcache_client = memcache_client_factory(timeline=False)
 
 
 def check_password(environ, user, password):
+    # We have almost no viable ways to set the config instance name.
+    # Normally it's set in LPCONFIG in the process environment, but we can't
+    # control that for mod_wsgi, and Apache SetEnv directives are
+    # intentionally not passed through to the WSGI environment.  However, we
+    # *can* control the application group via the application-group option
+    # to WSGIAuthUserScript, and overloading that as the config instance
+    # name actually makes a certain amount of sense, so use that if it's
+    # available.
+    application_group = environ.get("mod_wsgi.application_group")
+    if application_group:
+        config.setInstance(application_group)
+
     archive_reference = _get_archive_reference(environ)
     if archive_reference is None:
         return None
diff --git a/lib/lp/soyuz/wsgi/tests/test_archiveauth.py b/lib/lp/soyuz/wsgi/tests/test_archiveauth.py
index a41f9d7..d33cb34 100644
--- a/lib/lp/soyuz/wsgi/tests/test_archiveauth.py
+++ b/lib/lp/soyuz/wsgi/tests/test_archiveauth.py
@@ -16,6 +16,7 @@ from fixtures import MonkeyPatch
 import transaction
 
 from lp.services.config import config
+from lp.services.config.fixture import ConfigFixture
 from lp.services.memcache.testing import MemcacheFixture
 from lp.soyuz.wsgi import archiveauth
 from lp.testing import TestCaseWithFactory
@@ -37,6 +38,7 @@ class TestWSGIArchiveAuth(TestCaseWithFactory):
         self.useFixture(MonkeyPatch(
             "lp.soyuz.wsgi.archiveauth._memcache_client",
             self.memcache_fixture))
+        self.addCleanup(config.setInstance, config.instance_name)
 
     def test_get_archive_reference_short_url(self):
         self.assertIsNone(archiveauth._get_archive_reference(
@@ -131,6 +133,20 @@ class TestWSGIArchiveAuth(TestCaseWithFactory):
             archiveauth.check_password(
                 {"SCRIPT_NAME": archive_path}, username, password))
 
+    def test_check_password_sets_config_instance(self):
+        test_instance_name = self.factory.getUniqueUnicode()
+        self.assertNotEqual(test_instance_name, config.instance_name)
+        self.useFixture(
+            ConfigFixture(test_instance_name, config.instance_name))
+        archive, archive_path, username, password = self.makeArchiveAndToken()
+        self.assertIs(
+            True,
+            archiveauth.check_password(
+                {"mod_wsgi.application_group": test_instance_name,
+                 "SCRIPT_NAME": archive_path},
+                username, password))
+        self.assertEqual(test_instance_name, config.instance_name)
+
     def test_script(self):
         _, archive_path, username, password = self.makeArchiveAndToken()
         script_path = os.path.join(