adding sri modal to codeorigin #391

jmervine · 2016-02-02T02:33:01Z

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:

Notes:

I'm not 100% about the method used to include both style and script tags in the correct location. I attempted to use wp_enqueue_script and wp_enqueue_style respectively, without success. I'm new to Wordpress, so I may have been doing something incorrectly. Any direction here would be appreciated.
I did my best to follow the style guide. I apologies if I missed anything. I'm happy to make any updates necessary. That said, the versioned css and js files includes were added as is from their sources.

Thanks and cheers!
J

(cc @jdorfman @scottgonzalez)

jdorfman · 2016-02-02T07:03:05Z

Looks good to me

dmethvin · 2016-02-08T19:18:41Z

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

mgol · 2016-02-08T20:53:13Z

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

scottgonzalez · 2016-02-08T21:42:07Z

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 jquery/codeorigin.jquery.com#22 (comment)

mgol · 2016-02-08T22:40:37Z

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 · 2016-02-08T23:36:07Z

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 · 2016-02-08T23:40:55Z

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 · 2016-02-08T23:44:59Z

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 · 2016-02-08T23:52:18Z

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 · 2016-02-09T00:23:51Z

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 · 2016-02-09T00:26:55Z

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

You start development for your web site and decide to use files from the CDN.
You calculate the integrity hash and include it in the script tags referencing the CDN.
If the files are later changed on the CDN, for example by an attacker, the integrity hash will not match and the code will not run.

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.

scottgonzalez · 2016-02-09T13:53:58Z

themes/codeorigin.jquery.com/page.php

+// attach custom header and footer tags
+function header_append() {
+	echo '
+	<link rel="stylesheet" href="' . get_template_directory_uri() . '/css/font-awesome.css" />


Font Awesome is already loaded on the page.

I saw it in the themes/jquery/css directory, but not included. I'l drop this out.

jmervine · 2016-02-09T23:38:36Z

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

jmervine · 2016-02-10T03:04:38Z

@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 · 2016-02-12T23:07:04Z

we close @scottgonzalez ?

jmervine · 2016-02-13T16:55:21Z

@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

scottgonzalez · 2016-02-16T01:51:36Z

themes/codeorigin.jquery.com/page.php

+			&nbsp;&nbsp;<span>crossorigin="anonymous"&gt;&lt;/script&gt;</span>
+		</code>
+
+		<button class="sri-modal-copy-btn"


This button should start with a title and have the tooltip initialized so that the feature is discoverable.

jmervine · 2016-02-17T17:53:37Z

@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 · 2016-03-09T21:44:11Z

Let's

scottgonzalez · 2016-03-10T21:37:00Z

It's finally live :-)

Thanks for all the work.

jdorfman · 2016-03-10T21:48:50Z

@scottgonzalez awesome! Thank you =)

adding sri modal to codeorigin

2d6cd06

jquerybot added the CLA: Valid label Feb 2, 2016

dmethvin mentioned this pull request Feb 8, 2016

jQuery 1.9.1.min.js from code.jquery.com CDN points to wrong sourcemap jquery/jquery#2899

Closed

scottgonzalez reviewed Feb 9, 2016
View reviewed changes

jmervine added 4 commits February 9, 2016 18:49

removing font-awesome, changing modal title

aec1567

renaming modal.js to sri-modal.js

37a4ecd

fixing small spacing issue with script tag in sri modal

50680d7

renaming modal.css to sri-modal.css

b084449

jmervine added 2 commits February 9, 2016 18:55

removing unused code tag styling

ed6cd93

wrapping modal js in an iife

0d5db46

jmervine added 4 commits February 9, 2016 19:36

using cdn version of jquery-ui

3570ee7

fixing up modal click handler, per comments

2b99d13

fixing replace helper, per comments

e4f0003

removing blur actions from clipboard events

d30709b

jmervine added 2 commits February 13, 2016 08:53

using tooltip over hint

0a77507

removing added jquery ui

5656dc2

scottgonzalez reviewed Feb 16, 2016
View reviewed changes

jmervine added 2 commits February 16, 2016 07:38

loading style and scripts with enqueue over custom append

eb0c755

addressing tooltip handing comments

3df694f

scottgonzalez closed this in 283ed1a Mar 10, 2016

mgol mentioned this pull request Mar 14, 2016

Add support for SRI jquery/codeorigin.jquery.com#20

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding sri modal to codeorigin #391

adding sri modal to codeorigin #391

jmervine commented Feb 2, 2016

jdorfman commented Feb 2, 2016

dmethvin commented Feb 8, 2016

mgol commented Feb 8, 2016

scottgonzalez commented Feb 8, 2016

mgol commented Feb 8, 2016

scottgonzalez commented Feb 8, 2016

mgol commented Feb 8, 2016

scottgonzalez commented Feb 8, 2016

mgol commented Feb 8, 2016

scottgonzalez commented Feb 9, 2016

dmethvin commented Feb 9, 2016

scottgonzalez Feb 9, 2016

jmervine Feb 10, 2016

jmervine commented Feb 9, 2016

jmervine commented Feb 10, 2016

jdorfman commented Feb 12, 2016

jmervine commented Feb 13, 2016

scottgonzalez Feb 16, 2016

jmervine commented Feb 17, 2016

jdorfman commented Mar 9, 2016

scottgonzalez commented Mar 10, 2016

jdorfman commented Mar 10, 2016

adding sri modal to codeorigin #391

adding sri modal to codeorigin #391

Conversation

jmervine commented Feb 2, 2016

Notes:

jdorfman commented Feb 2, 2016

dmethvin commented Feb 8, 2016

mgol commented Feb 8, 2016

scottgonzalez commented Feb 8, 2016

mgol commented Feb 8, 2016

scottgonzalez commented Feb 8, 2016

mgol commented Feb 8, 2016

scottgonzalez commented Feb 8, 2016

mgol commented Feb 8, 2016

scottgonzalez commented Feb 9, 2016

dmethvin commented Feb 9, 2016

scottgonzalez Feb 9, 2016

Choose a reason for hiding this comment

jmervine Feb 10, 2016

Choose a reason for hiding this comment

jmervine commented Feb 9, 2016

jmervine commented Feb 10, 2016

jdorfman commented Feb 12, 2016

jmervine commented Feb 13, 2016

scottgonzalez Feb 16, 2016

Choose a reason for hiding this comment

jmervine commented Feb 17, 2016

jdorfman commented Mar 9, 2016

scottgonzalez commented Mar 10, 2016

jdorfman commented Mar 10, 2016