595634 - Get Add-ons page does not display because of inappropriate server restrictions

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Discovery Pane, defect)

Description

[Bill Gianopoulos [:WG9s]]

Bug 584831 added the header "X-Frame-Options: DENY" to content returned form AMO.

The issue here is that pages in the path "https://services.addons.mozilla.org/%LOCALE%/%APP%/discovery/" are intended to be displayed in a frame.

Since the frameset is defined in "about:addons", even specifying SAMEORIGIN rather than DENY does not help here.

The X-Frame_Options header should not be presented for pages returned from these directories.

Comment 1

[Bill Gianopoulos [:WG9s]]

HINT: In order to implement this, it is probably easier to change the location of these pages to:

https://services.addons.mozilla.org/discovery/%LOCALE%/%APP%/

Comment 2

[Dave Townsend [:mossop]]

It's looking increasingly like the bug in Firefox here won't be fixed before b6, if we want to show off the new discovery pane then I suggest we look at removing the header from it for the time being. This can be flipped back at a later time once we're all fixed

Comment 3

[Wil Clouser [:clouserw]]

James:  Is there a way to do this in the middleware?  Something like an @allow_framing decorator?

Comment 4

[Justin Scott [:fligtar]]

After following the comments in the client side bug the last couple weeks, I'm starting to think it's not worth fixing in the client at all since this is the only page that it would help.

I think everything on services.amo should be frameable.

Comment 5

[Wil Clouser [:clouserw]]

(In reply to comment #4)
> After following the comments in the client side bug the last couple weeks, I'm
> starting to think it's not worth fixing in the client at all since this is the
> only page that it would help.
> 
> I think everything on services.amo should be frameable.

With our current set of pages I think that would be fine but as we offer other options and interactions it could become a problem later.  I suspect we'll promptly forget about it and not revisit it until someone else does first.  (note that I've been paying little attention to the client bugs)

Comment 6

[James Socol [:jsocol, :james]]

(In reply to comment #3)
> James:  Is there a way to do this in the middleware?  Something like an
> @allow_framing decorator?

Not yet. Patches welcome! (Let me know if anyone else needs commit access to commonware. Or we can fork it to mozilla and let the webdev team have at it.)

Comment 7

[Dave Townsend [:mossop]]

(In reply to comment #4)
> After following the comments in the client side bug the last couple weeks, I'm
> starting to think it's not worth fixing in the client at all since this is the
> only page that it would help.

I think it is certainly worth fixing in the client, as we move more and more of our UI to tabs this will just come back to bite us, but I'm sure it will be a quicker fix for the time being to remove the header for the time being.

Comment 8

[Wil Clouser [:clouserw]]

http://github.com/jbalogh/zamboni/commit/ef4ade5ad19be90419f1680d9f98c73db9c2385b

Comment 9

[Wil Clouser [:clouserw]]

Bug to turn this back on is bug 596052.  This should go live tomorrow.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20100917 Firefox/4.0b7pre