878105 - Ensure origin for packaged app is unique

Status: VERIFIED FIXED

(Marketplace Graveyard :: Developer Pages, defect, P1)

Description

[Andy McKay]

In bug 852720 packaged apps were allowed to have an origin in the manifest. This is for the developer tools to ensure that if an origin is present on a packaged app, it is unique within the marketplace.

Comment 1

[Wil Clouser [:clouserw]]

Why do we need them to be unique?

Comment 2

[Andy McKay]

I'm trying to remember the security meeting, I was wonder the impression we didn't want colliding origins. But I can't remember exactly, Raymond can you remember.

If they aren't unique, I can't use them for receipts and would have to use something else.

Comment 3

[Wil Clouser [:clouserw]]

Nevermind, https://bugzilla.mozilla.org/show_bug.cgi?id=879437#c13 says they are unique

Comment 4

[Rob Hudson [:robhudson]]

When the switch 'webapps-unique-by-domain' is enabled this already happens. Should we separate packaged app origins from this switch so they are always unique? Seems like a good idea to me.

Comment 5

[Rob Hudson [:robhudson]]

https://github.com/mozilla/zamboni/commit/19e7156

Comment 6

[Iulian Timis]

Please add STR here or mark it with [qa-] if no QA is needed.

Comment 7

[Rob Hudson [:robhudson]]

To verify:
Submit a packaged app with an origin. Try to submit a 2nd packaged app with that same origin. You should get an error at submission time.

Comment 8

[Iulian Timis]

I tried to submit a 2nd packaged app with the same origin, and after I clicked contine, it just refreshed the submission page and no error message was displayed. I got the same behavior after submitting a packaged app with an invalid origin. Verified in https://marketplace-dev.allizom.org/developers/submit/ on FF25 (Win 7)
Please see screencast http://screencast.com/t/8LJxXDTdXQwQ

Comment 10

[Rob Hudson [:robhudson]]

I started working on this and it's more complicated than it seems. (Likewise for bug 892694 when we get to it).

The reason is we're validating a zip file before we actually save it. If the zip file is bad we want to toss it away and only create the FileUpload record that has the validation results with it. If the file size is < 2.5MB Django keeps it in memory but parse_addon requires a file on disk.

The form is going to have to check if it's an in-memory file and if so, create the file on disk to pass it to parse_addon in order to get the origin out of it.

The most straightforward approach is to do the above in the form -- make a tempfile, parse it, then throw it away when done.

But I'm also wondering if it might be nice to update our SafeUnzip library to take a file-like object and not assume a file on disk so we can pull this file out and handle it all in-memory.

Comment 11

[Rob Hudson [:robhudson]]

I went with the in-memory approach:
https://github.com/mozilla/zamboni/commit/29960747

Comment 12

[Iulian Timis]

Verified as fixed in https://marketplace-dev.allizom.org/developers/submit/ on FF25 (Win 7).
Postfix screencast http://screencast.com/t/yn2fSEQD
Closing bug.