Open Bug 1416215 Opened 8 years ago Updated 5 years ago

Review Validation Page for Reviewers

Categories

(Cloud Services :: Operations: AMO, task)

Product:
Cloud Services
Component:
Operations: AMO
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

People

(Reporter: erosman, Assigned: wezhou)

Details

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0 Build ID: 20170921100141 Steps to reproduce: ref: bug 1386984 Validation is still on AMO domain which wont allow script injection. https://addons.mozilla.org/en-US/developers/addon/*/file/*/validation It should be made available on 'reviewers.addons.mozilla.org' as well https://reviewers.addons.mozilla.org/en-US/developers/addon/*/file/*/validation
Note: The validation would need to be available on both domains, as this URL is also public facing for developers.
In fact, just creating a pseudo sub-domain for the validation just for the reviewers is fine. eg: https://reviewers.addons.mozilla.org/en-US/developers/addon/*/file/*/validation would actually load https://addons.mozilla.org/en-US/developers/addon/*/file/*/validation
There doesn't seem to be any activity on this issue filed 4 months ago. There was a hack (privacy.resistFingerprinting.block_mozAddonManager) that enabled running add-ons on the validation page. After spending an hour trying to find out why the add-on stopped working on validation page, it seems that the config hack has been removed in Firefox 60.0a1 (2018-03-11) (64-bit) If the removal is intentional and permanent, then the issue of availability of the validation page becomes imperative. I use the validation as part of the review process and generate data from the result. Example of generated data: libs/redactor.min.js - line: 2458, 2568, 2668, 4041 js/content_script.js - line: 280, 396, 592, 876, 901, 950, 960, 1029, 2422, 2628, 2828, 3715, 3727, 4417, 4654, 4895, 5203, 5205, 5232, 5276, 5314, 5366, 5398, 5528, 5532, 5538, 5615, 5635, 5639, 5645, 5841, 5904, 5952, 6058, 6111, 6212 Unless and until the issue is resolved, generating data such as aforementioned is impractical and not feasible manually and thus the options would be: - Not making a listing at all - Not reviewing add-ons that require making a listing
Flags: needinfo?(philipp)
Update: there is an additional hack for now (not tested yet) ref: bug 1310082
Hi Wezhou, can we make this happen so that reviewers don't have to rely on hacks?
Flags: needinfo?(philipp) → needinfo?(wezhou)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Is the requirement that 1. for reviewers, they can access https://reviewers.addons.mozilla.org/en-US/developers/addon/*/file/*/validation 2. for addon developers, they can access https://addons.mozilla.org/en-US/developers/addon/*/file/*/validation And correct me if I'm wrong, I assume 2 is already true at present, since we don't redirect '/en-US/developers/addon/*/file/*/validation' to the reviewers.a.m.o at the moment.
Flags: needinfo?(wezhou)
Not sure if comment #7 was meant to answer my questions in comment #6, but I'm still a little confused about what needs to be implemented here. Maybe I can re-phrase my questions, (a) as a reviewer, when I access [1], I'd like to ... (b) as a reviewer, when I access [2], I'd like to ... (c) as an add-on developer, when I access [1], I'd like to ... (d) as an add-on developer, when I access [2], I'd like to ... [1] https://addons.mozilla.org/en-US/developers/addon/chnprice-search-for-aliexpress/file/895021/validation [2] https://reviewers.addons.mozilla.org/en-US/developers/addon/chnprice-search-for-aliexpress/file/895021/validation
Developers should not have access to reviewers.addons.mozilla.org (a) as a reviewer, when I access [1], I'd like to get [1] (although not an issue) (b) as a reviewer, when I access [2], I'd like to get [2] <-- main issue (c) as an add-on developer, when I access [1], I'd like to get [1] (normal behaviour) (d) as an add-on developer, when I access [2], I'd like to ... (developers should not have access to [2]) I hope that helps.
The changes are on -dev. Please test the above 4 cases work as expected. We'll then schedule a time for -stage and -prod.
Assignee: nobody → wezhou
(In reply to erosman from comment #9) > Developers should not have access to reviewers.addons.mozilla.org > > *** Using my reviewer account on addons-dev: > (a) as a reviewer, when I access [1], I'd like to get [1] (although not an > issue) Works for https://addons-dev.allizom.org/en-US/developers/addon/provider-for-google-calendar/file/227444/validation > (b) as a reviewer, when I access [2], I'd like to get [2] <-- main issue Works for https://reviewers.addons-dev.allizom.org/en-US/developers/addon/provider-for-google-calendar/file/227444/validation *** Using my non-reviewer account on addons-dev (I've confirmed I don't have access to https://reviewers.addons-dev.allizom.org/en-US/reviewers/ using this account): > (c) as an add-on developer, when I access [1], I'd like to get [1] (normal > behaviour) Works for https://addons-dev.allizom.org/en-US/developers/addon/lightning/file/236477/validation > (d) as an add-on developer, when I access [2], I'd like to ... (developers > should not have access to [2]) This shows me the validation report as well, so going by the above premise this is incorrect. I don't think this is an issue though since the same route is available via addons-dev.allizom.org. Then again I don't know enough about how the routes work to reliably confirm this is not a problem.
In (c) and (d) cases, i.e. for a developer but a non-reviewer case, my assumption is that you should only be able to access your own add-on. If you're able to see other people's addon validation page, then that is an application issue, not a Nginx routing issue. Are you, as a developer, but a non-reviewer, able to see other people's addon validation report?
(In reply to :wezhou from comment #12) > Are you, as a developer, but a non-reviewer, able to see other people's > addon validation report? No, I am not. I tried accessing https://addons-dev.allizom.org/en-US/developers/addon/provider-for-google-calendar/file/227444/validation as a developer (but not of that add-on) and got the pretty "Oops! Not allowed" page. So it sounds like from an nginx perspective, everything is correct.
So if I summarize correctly, (a), (b), and (c) work as expected. For (d), the same developer (non-reviewer) can access the same page on both a.m.o and reviewers.a.m.o. Who can tell us that we can or cannot push the same change on -stage to -prod? I'm OK with it from ops perspective.
Mathieu maybe?
Flags: needinfo?(mpillard)
That looks ok to me. QA should verify it though.
Flags: needinfo?(mpillard)

Lexa, would you have time to verify this on stage (see comment 8 and onward)? No rush, it's not super urgent and not bound to a particular push. Thank you!

Flags: needinfo?(amoga)

Here are my test results for stage:

  1. with a reviewer account
  1. with a developer account

So, unless I misunderstood the problem statement, these seem to be the expected results.

Flags: needinfo?(amoga) → needinfo?(awagner)

Thank you, Lexa!

(In reply to :wezhou from comment #14)

Who can tell us that we can or cannot push the same change on -stage to prod?

This sentence made me think this has landed on stage as well, which doesn't seem to be the case, according to the test results.

Would you mind trying this on -dev as well? Sorry for the extra work.

Flags: needinfo?(awagner) → needinfo?(amoga)

I've run the same tests detailed in comment #18 on -dev. The results were the same as those reported for stage.

A few examples:

Flags: needinfo?(amoga) → needinfo?(awagner)

Thank you. It seems this is not fixed on -dev either then.

According to comment 9, a reviewer visiting https://reviewers.addons-dev.allizom.org/en-US/developers/addon/search-by-image526/file/412013/validation should not be redirected to https://addons-dev.allizom.org/...

Flags: needinfo?(awagner)

Oh, I see. It seems I've been misled by comment #2 and believed that the redirect was expected.

You need to log in before you can comment on or make changes to this bug.