launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #20781
[Merge] lp:~maxiberta/launchpad/named-auth-tokens-htaccess into lp:launchpad
Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/named-auth-tokens-htaccess into lp:launchpad with lp:~maxiberta/launchpad/named-auth-tokens as a prerequisite.
Commit message:
Add support for named auth tokens in ppa htpasswd creation.
Requested reviews:
Maximiliano Bertacchini (maxiberta)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens-htaccess/+merge/300002
Add support for named auth tokens in ppa htpasswd creation (including the removal of revoked named tokens).
--
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/archivepublisher/htaccess.py'
--- lib/lp/archivepublisher/htaccess.py 2013-06-20 05:50:00 +0000
+++ lib/lp/archivepublisher/htaccess.py 2016-07-13 20:42:37 +0000
@@ -58,43 +58,51 @@
file = open(filename, "a")
try:
- for entry in users:
- user, password, salt = entry
+ for user, password, salt in users:
encrypted = crypt.crypt(password, salt)
file.write("%s:%s\n" % (user, encrypted))
finally:
file.close()
-def htpasswd_credentials_for_archive(archive, tokens=None):
+def htpasswd_credentials_for_archive(archive):
"""Return credentials for an archive for use with write_htpasswd.
:param archive: An `IArchive` (must be private)
- :param tokens: Optional iterable of `IArchiveAuthToken`s.
:return: Iterable of tuples with (user, password, salt) for use with
write_htpasswd.
"""
assert archive.private, "Archive %r must be private" % archive
- if tokens is None:
- tokens = IStore(ArchiveAuthToken).find(
- (ArchiveAuthToken.person_id, ArchiveAuthToken.token),
- ArchiveAuthToken.archive == archive,
- ArchiveAuthToken.date_deactivated == None)
+ tokens = IStore(ArchiveAuthToken).find(
+ (ArchiveAuthToken.person_id, ArchiveAuthToken.name,
+ ArchiveAuthToken.token),
+ ArchiveAuthToken.archive == archive,
+ ArchiveAuthToken.date_deactivated == None)
# We iterate tokens more than once - materialise it.
tokens = list(tokens)
- # The first .htpasswd entry is the buildd_secret.
- yield (BUILDD_USER_NAME, archive.buildd_secret, BUILDD_USER_NAME[:2])
-
+ # Preload map with person ID to person name.
person_ids = map(itemgetter(0), tokens)
names = dict(
IStore(Person).find(
(Person.id, Person.name), Person.id.is_in(set(person_ids))))
- # Combine the token list with the person list, sorting by person ID
- # so the file can be compared later.
- sorted_tokens = [(token[1], names[token[0]]) for token in sorted(tokens)]
- # Iterate over tokens and write the appropriate htpasswd
- # entries for them.
- for token, person_name in sorted_tokens:
- yield (person_name, token, person_name[:2])
+
+ # Format the user field by combining the token list with the person list
+ # (when token has person_id) or prepending a '+' (for named tokens).
+ output = []
+ for person_id, token_name, token in tokens:
+ if token_name:
+ # A named auth token.
+ output.append(('+' + token_name, token, token_name[:2]))
+ else:
+ # A subscription auth token.
+ output.append((names[person_id], token, names[person_id][:2]))
+
+ # The first .htpasswd entry is the buildd_secret.
+ yield (BUILDD_USER_NAME, archive.buildd_secret, BUILDD_USER_NAME[:2])
+
+ # Iterate over tokens and write the appropriate htpasswd entries for them.
+ # Sort by name/person ID so the file can be compared later.
+ for user, password, salt in sorted(output):
+ yield (user, password, salt)
=== modified file 'lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py'
--- lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py 2014-10-29 06:04:09 +0000
+++ lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py 2016-07-13 20:42:37 +0000
@@ -89,8 +89,7 @@
fd, temp_filename = tempfile.mkstemp(dir=pub_config.temproot)
os.close(fd)
- write_htpasswd(
- temp_filename, htpasswd_credentials_for_archive(ppa))
+ write_htpasswd(temp_filename, htpasswd_credentials_for_archive(ppa))
return temp_filename
@@ -176,6 +175,7 @@
store = IStore(ArchiveSubscriber)
valid_tokens = store.find(
ArchiveAuthToken,
+ ArchiveAuthToken.name == None,
ArchiveAuthToken.date_deactivated == None,
ArchiveAuthToken.archive_id == ArchiveSubscriber.archive_id,
ArchiveSubscriber.status == ArchiveSubscriberStatus.CURRENT,
@@ -186,6 +186,7 @@
# all active tokens and valid tokens.
all_active_tokens = store.find(
ArchiveAuthToken,
+ ArchiveAuthToken.name == None,
ArchiveAuthToken.date_deactivated == None)
return all_active_tokens.difference(valid_tokens)
@@ -274,6 +275,22 @@
*extra_expr)
return new_ppa_tokens
+ def getDeactivatedNamedTokens(self, since=None):
+ """Return result set of named tokens deactivated since given time."""
+ now = datetime.now(pytz.UTC)
+
+ store = IStore(ArchiveAuthToken)
+ extra_expr = []
+ if since:
+ extra_expr = [ArchiveAuthToken.date_deactivated >= since]
+ tokens = store.find(
+ ArchiveAuthToken,
+ ArchiveAuthToken.name != None,
+ ArchiveAuthToken.date_deactivated != None,
+ ArchiveAuthToken.date_deactivated <= now,
+ *extra_expr)
+ return tokens
+
def getNewPrivatePPAs(self, since=None):
"""Return the recently created private PPAs."""
store = IStore(Archive)
@@ -294,6 +311,18 @@
last_success = self.getTimeToSyncFrom()
+ # Include ppas with named tokens deactivated since last time we ran.
+ num_tokens = 0
+ for token in self.getDeactivatedNamedTokens(since=last_success):
+ affected_ppas.add(token.archive)
+ num_tokens += 1
+
+ new_ppa_count = len(affected_ppas)
+ self.logger.debug(
+ "%s deactivated named tokens since last run, %s PPAs affected"
+ % (num_tokens, new_ppa_count - current_ppa_count))
+ current_ppa_count = new_ppa_count
+
# In addition to the ppas that are affected by deactivated
# tokens, we also want to include any ppas that have tokens
# created since the last time we ran.
@@ -316,7 +345,7 @@
self.logger.debug('%s PPAs require updating' % new_ppa_count)
for ppa in affected_ppas:
- # If this PPA is blacklisted, do not touch it's htaccess/pwd
+ # If this PPA is blacklisted, do not touch its htaccess/pwd
# files.
blacklisted_ppa_names_for_owner = self.blacklist.get(
ppa.owner.name, [])
=== modified file 'lib/lp/archivepublisher/tests/test_generate_ppa_htaccess.py'
--- lib/lp/archivepublisher/tests/test_generate_ppa_htaccess.py 2016-01-26 15:47:37 +0000
+++ lib/lp/archivepublisher/tests/test_generate_ppa_htaccess.py 2016-07-13 20:42:37 +0000
@@ -32,6 +32,7 @@
from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.teammembership import TeamMembershipStatus
from lp.services.config import config
+from lp.services.features.testing import FeatureFixture
from lp.services.log.logger import BufferLogger
from lp.services.osutils import (
ensure_directory_exists,
@@ -43,6 +44,7 @@
ArchiveStatus,
ArchiveSubscriberStatus,
)
+from lp.soyuz.interfaces.archive import NAMED_AUTH_TOKEN_FEATURE_FLAG
from lp.testing import TestCaseWithFactory
from lp.testing.dbuser import (
lp_dbuser,
@@ -72,6 +74,9 @@
ubuntutest = getUtility(IDistributionSet)['ubuntutest']
self.ppa.distribution = ubuntutest
+ # Enable named auth tokens.
+ self.useFixture(FeatureFixture({NAMED_AUTH_TOKEN_FEATURE_FLAG: u"on"}))
+
def getScript(self, test_args=None):
"""Return a HtaccessTokenGenerator instance."""
if test_args is None:
@@ -260,6 +265,11 @@
all_persons, persons1, persons2, tokens) = data
team1_person = persons1[0]
+ # Named tokens should be ignored for deactivation.
+ self.ppa.newNamedAuthToken(u"tokenname1", as_dict=False)
+ named_token = self.ppa.newNamedAuthToken(u"tokenname2", as_dict=False)
+ named_token.deactivate()
+
# Initially, nothing is eligible for deactivation.
script = self.getScript()
script.deactivateInvalidTokens()
@@ -325,12 +335,9 @@
sub2 = self.ppa.newSubscription(name16, self.ppa.owner)
token1 = self.ppa.newAuthToken(name12)
token2 = self.ppa.newAuthToken(name16)
+ token3 = self.ppa.newNamedAuthToken(u"tokenname3", as_dict=False)
self.layer.txn.commit()
- subs = [sub1]
- subs.append(sub2)
- tokens = [token1]
- tokens.append(token2)
- return subs, tokens
+ return (sub1, sub2), (token1, token2, token3)
def ensureNoFiles(self):
"""Ensure the .ht* files don't already exist."""
@@ -375,6 +382,32 @@
os.remove(htaccess)
os.remove(htpasswd)
+ def testBasicOperation_with_named_tokens(self):
+ """Invoke the actual script and make sure it generates some files."""
+ token1 = self.ppa.newNamedAuthToken(u"tokenname1", as_dict=False)
+ token2 = self.ppa.newNamedAuthToken(u"tokenname2", as_dict=False)
+ token3 = self.ppa.newNamedAuthToken(u"tokenname3", as_dict=False)
+ token3.deactivate()
+
+ # Call the script and check that we have a .htaccess and a .htpasswd.
+ htaccess, htpasswd = self.ensureNoFiles()
+ script = self.getScript()
+ script.main()
+ self.assertThat([htaccess, htpasswd], AllMatch(FileExists()))
+ self.assertIn('+' + token1.name, open(htpasswd).read())
+ self.assertIn('+' + token2.name, open(htpasswd).read())
+ self.assertNotIn('+' + token3.name, open(htpasswd).read())
+
+ # Deactivate a named token and verify it is removed from .htpasswd.
+ token2.deactivate()
+ script.main()
+ self.assertThat([htaccess, htpasswd], AllMatch(FileExists()))
+ self.assertIn('+' + token1.name, open(htpasswd).read())
+ self.assertNotIn('+' + token2.name, open(htpasswd).read())
+ self.assertNotIn('+' + token3.name, open(htpasswd).read())
+ os.remove(htaccess)
+ os.remove(htpasswd)
+
def _setupOptionsData(self):
"""Setup test data for option testing."""
subs, tokens = self.setupDummyTokens()
@@ -598,14 +631,47 @@
script = self.getScript()
self.assertContentEqual(tokens[1:], script.getNewTokens())
+ def test_getDeactivatedNamedTokens_no_previous_run(self):
+ """All deactivated named tokens returned if there is no record
+ of previous run."""
+ last_start = datetime.now(pytz.UTC) - timedelta(seconds=90)
+ before_last_start = last_start - timedelta(seconds=30)
+
+ self.ppa.newNamedAuthToken(u"tokenname1", as_dict=False)
+ token2 = self.ppa.newNamedAuthToken(u"tokenname2", as_dict=False)
+ token2.deactivate()
+ token3 = self.ppa.newNamedAuthToken(u"tokenname3", as_dict=False)
+ token3.date_deactivated = before_last_start
+
+ script = self.getScript()
+ self.assertContentEqual(
+ [token2, token3], script.getDeactivatedNamedTokens())
+
+ def test_getDeactivatedNamedTokens_only_those_since_last_run(self):
+ """Only named tokens deactivated since last run are returned."""
+ last_start = datetime.now(pytz.UTC) - timedelta(seconds=90)
+ before_last_start = last_start - timedelta(seconds=30)
+ tomorrow = datetime.now(pytz.UTC) + timedelta(days=1)
+
+ self.ppa.newNamedAuthToken(u"tokenname1", as_dict=False)
+ token2 = self.ppa.newNamedAuthToken(u"tokenname2", as_dict=False)
+ token2.deactivate()
+ token3 = self.ppa.newNamedAuthToken(u"tokenname3", as_dict=False)
+ token3.date_deactivated = before_last_start
+ token4 = self.ppa.newNamedAuthToken(u"tokenname4", as_dict=False)
+ token4.date_deactivated = tomorrow
+
+ script = self.getScript()
+ self.assertContentEqual(
+ [token2], script.getDeactivatedNamedTokens(last_start))
+
def test_processes_PPAs_without_subscription(self):
# A .htaccess file is written for Private PPAs even if they don't have
# any subscriptions.
htaccess, htpasswd = self.ensureNoFiles()
transaction.commit()
- # Call the script and check that we have a .htaccess and a
- # .htpasswd.
+ # Call the script and check that we have a .htaccess and a .htpasswd.
return_code, stdout, stderr = self.runScript()
self.assertEqual(
return_code, 0, "Got a bad return code of %s\nOutput:\n%s" %
=== modified file 'lib/lp/archivepublisher/tests/test_htaccess.py'
--- lib/lp/archivepublisher/tests/test_htaccess.py 2012-05-31 01:57:07 +0000
+++ lib/lp/archivepublisher/tests/test_htaccess.py 2016-07-13 20:42:37 +0000
@@ -15,6 +15,8 @@
)
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.person import IPersonSet
+from lp.services.features.testing import FeatureFixture
+from lp.soyuz.interfaces.archive import NAMED_AUTH_TOKEN_FEATURE_FLAG
from lp.testing import TestCaseWithFactory
from lp.testing.layers import LaunchpadZopelessLayer
@@ -36,6 +38,9 @@
ubuntutest = getUtility(IDistributionSet)['ubuntutest']
self.ppa.distribution = ubuntutest
+ # Enable named auth tokens.
+ self.useFixture(FeatureFixture({NAMED_AUTH_TOKEN_FEATURE_FLAG: u"on"}))
+
def test_write_htpasswd(self):
"""Test that writing the .htpasswd file works properly."""
fd, filename = tempfile.mkstemp()
@@ -109,15 +114,20 @@
self.ppa.newSubscription(name12, self.ppa.owner)
self.ppa.newSubscription(name16, self.ppa.owner)
first_created_token = self.ppa.newAuthToken(name16)
- tokens = [
- (token.person_id, token.token)
- for token in [self.ppa.newAuthToken(name12), first_created_token]]
-
- credentials = list(htpasswd_credentials_for_archive(self.ppa, tokens))
-
- self.assertContentEqual(
+ second_created_token = self.ppa.newAuthToken(name12)
+ named_token_20 = self.ppa.newNamedAuthToken(u"name20", as_dict=False)
+ named_token_14 = self.ppa.newNamedAuthToken(u"name14", as_dict=False)
+ named_token_99 = self.ppa.newNamedAuthToken(u"name99", as_dict=False)
+ named_token_99.deactivate()
+
+ credentials = list(htpasswd_credentials_for_archive(self.ppa))
+
+ # Use assertEqual instead of assertContentEqual to verify order.
+ self.assertEqual(
credentials, [
("buildd", "geheim", "bu"),
- ("name12", tokens[0][1], "na"),
- ("name16", tokens[1][1], "na")
+ ("+name14", named_token_14.token, "na"),
+ ("+name20", named_token_20.token, "na"),
+ ("name12", second_created_token.token, "na"),
+ ("name16", first_created_token.token, "na"),
])
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2016-07-13 20:42:37 +0000
+++ lib/lp/soyuz/model/archive.py 2016-07-13 20:42:37 +0000
@@ -1952,7 +1952,7 @@
IStore(ArchiveAuthToken).add(archive_auth_token)
return archive_auth_token
- def newNamedAuthToken(self, name, token=None):
+ def newNamedAuthToken(self, name, token=None, as_dict=True):
"""See `IArchive`."""
if not getFeatureFlag(NAMED_AUTH_TOKEN_FEATURE_FLAG):
@@ -1980,7 +1980,10 @@
archive_auth_token.name = name
archive_auth_token.token = token
IStore(ArchiveAuthToken).add(archive_auth_token)
- return archive_auth_token.asDict()
+ if as_dict:
+ return archive_auth_token.asDict()
+ else:
+ return archive_auth_token
def getNamedAuthToken(self, name):
"""See `IArchive`."""
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py 2016-07-13 20:42:37 +0000
+++ lib/lp/soyuz/tests/test_archive.py 2016-07-13 20:42:37 +0000
@@ -1320,9 +1320,8 @@
self.private_ppa.newNamedAuthToken, u"tokenname")
def test_newNamedAuthToken_with_custom_secret(self):
- self.private_ppa.newNamedAuthToken(u"tokenname", u"somesecret")
- token = getUtility(IArchiveAuthTokenSet).getActiveNamedTokenForArchive(
- self.private_ppa, u"tokenname")
+ token = self.private_ppa.newNamedAuthToken(
+ u"tokenname", u"somesecret", as_dict=False)
self.assertEqual(u"somesecret", token.token)
def test_getNamedAuthToken_with_no_token(self):
@@ -1336,9 +1335,7 @@
res)
def test_revokeNamedAuthToken_with_token(self):
- self.private_ppa.newNamedAuthToken(u"tokenname")
- token = getUtility(IArchiveAuthTokenSet).getActiveNamedTokenForArchive(
- self.private_ppa, u"tokenname")
+ token = self.private_ppa.newNamedAuthToken(u"tokenname", as_dict=False)
self.private_ppa.revokeNamedAuthToken(u"tokenname")
self.assertIsNotNone(token.date_deactivated)
Follow ups
