Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sign add-ons with recommendation signer based on DiscoveryItem. #11627

Merged
merged 4 commits into from
Jun 12, 2019

Conversation

EnTeQuAk
Copy link
Contributor

This also prepares for the possibility that we may have to use different
credentials for the recommendations signer. In case we don't, ops only
has to set the creds twice which seems fair.

Fixes mozilla/addons#6522

This also prepares for the possibility that we may have to use different
credentials for the recommendations signer. In case we don't, ops only
has to set the creds twice which seems fair.

Fixes #11062
@@ -245,4 +245,7 @@ authorizations:
key: fs5wgcer9qj819kfptdlp8gm227ewxnzvsuj9ztycsx08hfhzu
signers:
- webextensions-rsa
- id: bob
key: 9vh6bhlc10y63ow2k4zke7k0c3l9hpr8mo96p92jmbfqngs9e7d
signers:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These credentials have to match https://github.com/mozilla-services/autograph/blob/master/autograph.yaml#L1053 (and respectively for alice)

Copy link
Member

@diox diox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. Do we have any end-to-end test in reviewer tools that include signing ? Maybe it's worth adding one w/ a recommendable add-on...

@EnTeQuAk
Copy link
Contributor Author

This makes sense. Do we have any end-to-end test in reviewer tools that include signing ? Maybe it's worth adding one w/ a recommendable add-on...

We don't unfortunately,

def test_nominated_to_approved_recommended(self):
DiscoveryItem.objects.create(
addon=self.addon, recommendable=True)
assert not self.addon.is_recommended
self.test_nomination_to_public()
del self.addon.is_recommended
assert self.addon.current_version.recommendation_approved is True
assert self.addon.is_recommended
def test_approved_update_recommended(self):
DiscoveryItem.objects.create(
addon=self.addon, recommendable=True)
assert not self.addon.is_recommended
self.test_public_addon_with_version_awaiting_review_to_public()
del self.addon.is_recommended
assert self.addon.current_version.recommendation_approved is True
assert self.addon.is_recommended is True
def test_autoapprove_fails_for_recommended(self):
DiscoveryItem.objects.create(
addon=self.addon, recommendable=True)
assert not self.addon.is_recommended
self.request.user = UserProfile.objects.get(id=settings.TASK_USER_ID)
with self.assertRaises(AssertionError):
self.test_nomination_to_public()
assert self.addon.current_version.recommendation_approved is False
assert not self.addon.is_recommended
tests this mostly but doesn't test the whole flow. Guess it's worth adding a test for that that doesn't rely on mocking.

@EnTeQuAk
Copy link
Contributor Author

Maybe it's worth adding one w/ a recommendable add-on...

Added d99ab37 regarding end-to-end tests.

@EnTeQuAk EnTeQuAk merged commit 5423c64 into master Jun 12, 2019
@EnTeQuAk EnTeQuAk deleted the 11062-fix-autograph-recommendation-real-world branch June 12, 2019 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update autograph calls to send the right parameters for recommended extensions
2 participants