Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rule(s) to check for redirects #438

Open
1 of 4 tasks
alrra opened this issue Aug 21, 2017 · 11 comments
Open
1 of 4 tasks

Add rule(s) to check for redirects #438

alrra opened this issue Aug 21, 2017 · 11 comments

Comments

@alrra
Copy link
Contributor

alrra commented Aug 21, 2017

Possible checks:

@molant
Copy link
Member

molant commented Sep 29, 2017

Redirects from HTTPS to HTTP

I guess the opposite too? Isn't this included in the previous point?

@molant
Copy link
Member

molant commented Sep 29, 2017

Also, it will be nice to tell the user how much of an impact this has had. We could check if the Tracing domain has that information. request should expose it if we add the option time which we are already using in Requester

@alrra
Copy link
Contributor Author

alrra commented Sep 29, 2017

Redirects from HTTPS to HTTP

I guess the opposite too?

The reason why I've added HTTPS => HTTP is because that is problematic, and users should fix it. HTTP => HTTPS is "OK", and users should do that (in addition to other things such as HSTS).

Isn't this included in the previous point?

Kinda. My thinking was that maybe some user won't care that it's just 1 redirect, but they should specifically care that the redirect is from HTTPS => HTTP, and we should make sure they know that.

@molant
Copy link
Member

molant commented Sep 29, 2017

they should specifically care that the redirect is from HTTPS => HTTP

I think that should be another rule under security and have a general one to avoid redirects.

@alrra
Copy link
Contributor Author

alrra commented Sep 29, 2017

I think that should be another rule under security

Yes, we can go with that.

(I wasn't sure if it should be a separate thing or not, reason why the title of the issue has: rule(s))

@molant
Copy link
Member

molant commented Nov 8, 2017

I'm starting with this rule (at least the basic redirects). These are my initial thoughts:

  • Allow the user to configure the number of hops, by default 0
  • Try to add information about time spent on dns lookup, redirects, etc. I believe we've already added that information to request and it might be a good opportunity to get @axemclion work on New: Added Chrome Traces to cdp connector #420 merged somehow. This part of the rule's should be conditional, some connectors might not have it (like the edge15 one)

Main question: What do we do about the main page? Some websites have friendly urls. E.g.: https://edge.ms --> https://developer.Microsoft.com/en-us/microsoft-edge/
I think we should add another property for the number of hops for the sourceTarget that will be 0 by default in the CLI but maybe 1-2 in the online-scanner.

@sonarwhal/contributors what do you think?

@molant molant removed their assignment Nov 9, 2017
@Ruffio
Copy link

Ruffio commented Jul 19, 2018

Redirects from HTTP to HTTPS - From a security/privacy perspective there should be a rule to check for this.
Currently I get an error because I redirect my users from http:// to https:// and I don't agree on this. If the rest of the url is exactly the same, then the redirect to a secure connection/protocol shouldn't be flagged as an issue. I would actaully say that now a days, it is the other way around. If you type in http and are not redirected to https, that is an issue.

You can try some of these for inspiration:
http://www.google.com
http://microsoft.com
http://github.com

IMHO a rule should be made to check for http -> https redirect where the rest of the url is exactly the same, and for the existing rule, max should be equal to 1 if the rest of the url is the same:

image

@alrra
Copy link
Contributor Author

alrra commented Jul 19, 2018

because I redirect my users from http:// to https://

Make we should make no-http-redirects rule allow the target to have 1 redirect if that redirect is from HTTP to HTTPS?

@webhintio/core Thoughts?

From a security/privacy perspective there should be a rule to check for this.

There are a few rules related to that (e.g.: strict-transport-security, https-only).


@Ruffio Note: If you use the CLI you can configure the rule to not complain about that.

@molant
Copy link
Member

molant commented Jul 19, 2018

Make we should make no-http-redirects rule allow the target to have 1 redirect if that redirect is from HTTP to HTTPS?

I think that would be out of the scope of the rule, especially taking into account that:

  • As you've pointed out we have rules that check similar things
  • If you use the CLI you can allow 1 redirect to the target
  • In the online scanner you can use instead the https url or ignore that result because you know what's going on and you know what you are doing is fine

@Ruffio
Copy link

Ruffio commented Jul 19, 2018

It is just a suggestion and I can live with that one 'error' on the online version :-)

@molant
Copy link
Member

molant commented Jul 19, 2018

@Ruffio and your suggestions are really appreciated!

The project is community driven so if the majority of the core team decides it's worth adding it, we will do it regardless of what any individual thinks 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants