902691 - Add a "Learn More" link to HSTS security console messages

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Ivan Alagenchev :ialagenchev]

Bug 846918 landed before we started adding the "Learn More" link to security related warning messages. We need to write a MDN page that provides more information about Strict Transport Security and link to it from the warning message. 

This should be as easy as adding a new logical branch in addMoreInfoLink in browser/devtools/webconsole/webconsole.js and reusing some of the content for these blogposts: https://blog.mozilla.org/security/2012/12/26/http-strict-transport-security-2/
https://blog.mozilla.org/security/2012/11/01/preloading-hsts/

or reusing and perhaps adding more content to this mdn page: https://developer.mozilla.org/en-US/docs/Security/HTTP_Strict_Transport_Security

Comment 1

[Ivan Alagenchev :ialagenchev]

Attachment: 902691.patch

Comment 2

[Mihai Sucan [:msucan]]

Comment on attachment 789262 [details] [diff] [review]
902691.patch

Review of attachment 789262 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you! This is looking good.

::: browser/devtools/webconsole/webconsole.js
@@ +40,5 @@
>  const MIXED_CONTENT_LEARN_MORE = "https://developer.mozilla.org/en/Security/MixedContent";
>  
>  const INSECURE_PASSWORDS_LEARN_MORE = "https://developer.mozilla.org/en-US/docs/Security/InsecurePasswords";
>  
> +const STRICT_TRANSPORT_SECURITY_LEARN_MORE = "https://developer.mozilla.org/en-US/docs/Security/HTTP_Strict_Transport_Security";

Please remove the /en-US/ part to allow MDN to redirect the user to the page that best fits user's regional settings.

@@ +1401,5 @@
>      } else if (aScriptError.category == "Mixed Content Message" ||
>                 aScriptError.category == "Mixed Content Blocker") {
>        url = MIXED_CONTENT_LEARN_MORE;
> +    } else if (aScriptError.category == "Invalid HSTS Headers") {
> +      url = STRICT_TRANSPORT_SECURITY_LEARN_MORE;

nit: this function is growing now. Wouldn't it make sense to change these ifs into a switch block with cases for aScriptError.category? should be easier to follow...

Comment 3

[Ivan Alagenchev :ialagenchev]

(In reply to Mihai Sucan [:msucan] from comment #2)
> Comment on attachment 789262 [details] [diff] [review]
> 902691.patch
> 
> Review of attachment 789262 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thank you! This is looking good.
> 
> ::: browser/devtools/webconsole/webconsole.js
> @@ +40,5 @@
> >  const MIXED_CONTENT_LEARN_MORE = "https://developer.mozilla.org/en/Security/MixedContent";
> >  
> >  const INSECURE_PASSWORDS_LEARN_MORE = "https://developer.mozilla.org/en-US/docs/Security/InsecurePasswords";
> >  
> > +const STRICT_TRANSPORT_SECURITY_LEARN_MORE = "https://developer.mozilla.org/en-US/docs/Security/HTTP_Strict_Transport_Security";
> 
> Please remove the /en-US/ part to allow MDN to redirect the user to the page
> that best fits user's regional settings.
> 
> @@ +1401,5 @@
> >      } else if (aScriptError.category == "Mixed Content Message" ||
> >                 aScriptError.category == "Mixed Content Blocker") {
> >        url = MIXED_CONTENT_LEARN_MORE;
> > +    } else if (aScriptError.category == "Invalid HSTS Headers") {
> > +      url = STRICT_TRANSPORT_SECURITY_LEARN_MORE;
> 
> nit: this function is growing now. Wouldn't it make sense to change these
> ifs into a switch block with cases for aScriptError.category? should be
> easier to follow...

Once 875456 lands, I will change the if block to switch statements and also replace all of the learn more links to use the non-region specific links. Thanks for pointing those out.

Comment 4

[Ivan Alagenchev :ialagenchev]

Attachment: 902691.patch

Comment 5

[Ivan Alagenchev :ialagenchev]

tbpl: https://tbpl.mozilla.org/?tree=Try&rev=26778402333b

Comment 6

[Ivan Alagenchev :ialagenchev]

Attachment: 902691.patch

New patch since some tests turned out to be checking for the wrong url. New tbpl run: https://tbpl.mozilla.org/?tree=Try&rev=87c8235307c7

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/a5d9b30686f9

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a5d9b30686f9

Comment 9

[Tanvi Vyas[:tanvi]]

As part of this bug, shouldn't we update https://developer.mozilla.org/en-US/docs/Security/HTTP_Strict_Transport_Security to include a section that clearly explains the webconsole error/warning and provides guidance on how to fix the issue?

Comment 10

[Ivan Alagenchev :ialagenchev]

(In reply to Tanvi Vyas [:tanvi] from comment #9)
> As part of this bug, shouldn't we update
> https://developer.mozilla.org/en-US/docs/Security/
> HTTP_Strict_Transport_Security to include a section that clearly explains
> the webconsole error/warning and provides guidance on how to fix the issue?

I think that's a good idea. We can probably add a section about web console messages and describe common causes.

Comment 11

[David Bruant]

(In reply to Ivan Alagenchev :ialagenchev from comment #10)
> (In reply to Tanvi Vyas [:tanvi] from comment #9)
> > As part of this bug, shouldn't we update
> > https://developer.mozilla.org/en-US/docs/Security/
> > HTTP_Strict_Transport_Security to include a section that clearly explains
> > the webconsole error/warning and provides guidance on how to fix the issue?
> 
> I think that's a good idea. We can probably add a section about web console
> messages and describe common causes.
Another strategy could be to have pages dedicated to being the destinations of embedded "learn more" links that would have short descriptions, but heavily link to other pages of MDN.
You may want to discuss with the MDN team for a convention that would allow tracking of when people go to MDN from the console. Something like ?from=console at the end of URLs.
Anyway, happy to discuss linking and page organisation strategy at https://lists.mozilla.org/listinfo/dev-mdc