HTTPArchive / custom-metrics

Custom metrics to use with WebPageTest agents
Apache License 2.0
19 stars 21 forks source link

Add two .well-known custom metrics for the security chapter #22

Closed vikvanderlinden closed 2 years ago

vikvanderlinden commented 2 years ago

Progress on https://github.com/HTTPArchive/almanac.httparchive.org/issues/2892

The /change-password/ and /resource-that-should-not-exist-whose-status-code-should-not-be-200/ have been added to the well-known metrics for the security chapter.

Tests: https://webpagetest.org/result/220525_AiDc91_D7M/ https://webpagetest.org/result/220525_AiDcEC_D80/ https://webpagetest.org/result/220525_BiDc9Y_BXN/

vikvanderlinden commented 2 years ago

@GJFR @VictorLeP

rviscomi commented 2 years ago

Thanks @vikvanderlinden! Could you add a few tests to the PR description?

vikvanderlinden commented 2 years ago

Thanks @vikvanderlinden! Could you add a few tests to the PR description?

I've added some to the description

rviscomi commented 2 years ago

In the example.com case, should this be found: false?

{
    "found": true,
    "data": {
        "status": 404,
        "redirected": false,
        "url": "https://www.example.com/.well-known/resource-that-should-not-exist-whose-status-code-should-not-be-200/"
    }
}
SaptakS commented 2 years ago

I think the found key is a bit confusing in these cases, since we don't expect to find a file or content in these scenarios unlike for .well-known/security.txt. https://www.example.com/.well-known/resource-that-should-not-exist-whose-status-code-should-not-be-200/ is always supposed to return 404, which it does. It's more like a sanity check to see the servers are set up properly with status codes.

So I would say we can skip the found key for responses like this maybe?

https://w3c.github.io/webappsec-change-password-url/response-code-reliability.html this has more on the actual specification.

vikvanderlinden commented 2 years ago

I was basically skipping the check for status 200 and redirects in the new function parseResponseWithRedirects but I left the found key in. I removed it now because it did indeed not make much sense as @SaptakS suggested.