jquery / jquery-wp-content

WordPress themes and plugins for the jQuery sites
GNU General Public License v2.0
253 stars 169 forks source link

adding sri modal to codeorigin #391

Closed jmervine closed 8 years ago

jmervine commented 8 years ago

Hello!

Per the discussions on gh:jquery/codeorigin.jquery.com#20 I'm adding the SRI modal as an onclick popin for each cdn file link. This is in direct support of gh:jquery/codeorigin.jquery.com#22. I've attached a screenshot of the result here:

screen shot 2016-02-01 at 6 25 35 pm

Notes:

Thanks and cheers! J

(cc @jdorfman @scottgonzalez)

jdorfman commented 8 years ago

Looks good to me

dmethvin commented 8 years ago

LGTM but I'll defer to @scottgonzalez who has been following this more closely and knows the setup better.

mgol commented 8 years ago

Do we want to have a page with hashes for all past versions for people that can't yet download the newest one?

scottgonzalez commented 8 years ago

Do we want to have a page with hashes for all past versions for people that can't yet download the newest one?

The hashes are already computed for every version. I've also suggested that we expose https://code.jquery.com/resources/sri-directives.json in https://github.com/jquery/codeorigin.jquery.com/pull/22#issuecomment-177702136

mgol commented 8 years ago

The hashes are already computed for every version.

Where are they kept?

I've also suggested that we expose https://code.jquery.com/resources/sri-directives.json in jquery/codeorigin.jquery.com#22 (comment)

Since the goal of the SRI is to prevent injecting a rogue script in case of a CDN poisoning, putting a file listing the hashes on this very CDN doesn't sound like the best option.

scottgonzalez commented 8 years ago

Where are they kept?

Umm...they're generated on deploy for the CDN. I'm getting the feeling you haven't looked at the related PR.

Since the goal of the SRI is to prevent injecting a rogue script in case of a CDN poisoning, putting a file listing the hashes on this very CDN doesn't sound like the best option.

And the values that every user will insert into their scripts comes directly from the CDN. I'm not sure where else these would be listed.

mgol commented 8 years ago

Umm...they're generated on deploy for the CDN. I'm getting the feeling you haven't looked at the related PR.

Yeah, sorry, I see it now.

And the values that every user will insert into their scripts comes directly from the CDN. I'm not sure where else these would be listed.

Those values are needed only for developers, end users don't download them. Hence, the CDN is not necessary for that, they may as well be kept somewhere on our sites, i.e. places that are separate from CDNs. We'd have to first switch our sites to HTTPS-only, though.

scottgonzalez commented 8 years ago

Hence, the CDN is not necessary for that, they may as well be kept somewhere on our sites, i.e. places that are separate from CDNs. We'd have to first switch our sites to HTTPS-only, though.

I'm really not sure what setup you have in mind. It would certainly require whatever other random site you choose to have the CDN as a dependency for getting the hashes.

mgol commented 8 years ago

I'm really not sure what setup you have in mind. It would certainly require whatever other random site you choose to have the CDN as a dependency for getting the hashes.

Only for past releases; new ones are generated offline, on a computer of the person doing the release.

scottgonzalez commented 8 years ago

Only for past releases; new ones are generated offline, on a computer of the person doing the release.

I'm not sure how you got that impression. The hash will always be generated by the production build server during deploy. Perhaps you can explain why you think this would not be the case based on the other PR.

dmethvin commented 8 years ago

I think the scenario that the SRI integrity hash is addressing is this one:

If someone hacks jquery-1.11.1.js on the web site months after it's put down, there should be thousands if not millions of web sites with the correct hash value, they would be protected, and we'd know about the problem in short order.

That's why I'm hesitant to fix the map reference in the minified jQuery 1.9.1 file by editing it. Anyone using SRI or keeping their own hashes would notice that the file has been changed two years after it was put on the CDN.

jmervine commented 8 years ago

@scottgonzalez Thanks for the feedback, comments. I'll address / answer shortly. Dealing with some other fires elsewhere at the moment.

jmervine commented 8 years ago

@scottgonzalez I've addressed some of the low hangers. I'll get back to the rest shortly. I didn't work up the client side js/css, and have asked @jdorfman to review those parts, he or I will start addressing those parts next.

Cheers!

PS: I will most certainly squash commits once all comments have been address. :)

jdorfman commented 8 years ago

we close @scottgonzalez ?

jmervine commented 8 years ago

@scottgonzalez I believe I've addressed all open comments aside from https://github.com/jquery/jquery-wp-content/pull/391/files#r52310552, which is pending comments. Let me know if I missed anything.

Cheers! J

cc: @jdorfman

jmervine commented 8 years ago

@scottgonzalez All outstanding comments have been addressed as far as I can tell. Let me know if I missed anything or if there's anything else.

Cheers!

jdorfman commented 8 years ago

Let's :shipit:

scottgonzalez commented 8 years ago

It's finally live :-)

Thanks for all the work.

jdorfman commented 8 years ago

@scottgonzalez awesome! Thank you =)