Skip to content

Commit

Permalink
Fix several issues from PR #16257: Poperly hide audit until docs are …
Browse files Browse the repository at this point in the history
…published, some text phrasings and the smoke test assertions.
  • Loading branch information
sebastian9er committed Nov 22, 2024
1 parent ae699f6 commit 46c9aa1
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 39 deletions.
14 changes: 14 additions & 0 deletions cli/test/smokehouse/test-definitions/hsts-missing-directives.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@ const expectations = {
audits: {
'has-hsts': {
score: 1,
details: {
items: [
{
directive: 'includeSubDomains',
description: 'No includeSubDomains directive found',
severity: 'Medium',
},
{
directive: 'preload',
description: 'No preload directive found',
severity: 'Medium',
}
]
}
},
},
},
Expand Down
11 changes: 4 additions & 7 deletions core/audits/has-hsts.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const UIStrings = {
/** Summary text for the results of a Lighthouse audit that evaluates the HSTS header. This is displayed if the max-age directive is missing. "HSTS" stands for "HTTP Strict Transport Security". */
noMaxAge: 'No max-age directive',
/** Summary text for the results of a Lighthouse audit that evaluates the HSTS header. This is displayed if the provided duration for the max-age directive is too low. "HSTS" stands for "HTTP Strict Transport Security". */
lowMaxAge: 'Max-age too low',
lowMaxAge: 'max-age is too low',
/** Table item value calling out the presence of a syntax error. */
invalidSyntax: 'Invalid syntax',
/** Label for a column in a data table; entries will be a directive of the HSTS header. "HSTS" stands for "HTTP Strict Transport Security". */
Expand Down Expand Up @@ -149,11 +149,8 @@ class HasHsts extends Audit {
}

// If there is a directive that's not an official HSTS directive.
if (!allowedDirectives.includes(actualDirective)) {
// max-age=<number> would always trigger.
if (actualDirective.includes('max-age')) {
continue;
}
if (!allowedDirectives.includes(actualDirective) &&
!actualDirective.includes('max-age')) {
syntax.push({
severity: str_(i18n.UIStrings.itemSeverityLow),
description: str_(UIStrings.invalidSyntax),
Expand All @@ -176,7 +173,7 @@ class HasHsts extends Audit {
f.directive, f.description,
str_(i18n.UIStrings.itemSeverityLow))),
];
return {score: violations.length || syntax.length > 1 ? 0 : 1, results};
return {score: violations.length || syntax.length ? 0 : 1, results};
}

/**
Expand Down
2 changes: 1 addition & 1 deletion core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ const defaultConfig = {
{id: 'geolocation-on-start', weight: 1, group: 'best-practices-trust-safety'},
{id: 'notification-on-start', weight: 1, group: 'best-practices-trust-safety'},
{id: 'csp-xss', weight: 0, group: 'best-practices-trust-safety'},
{id: 'has-hsts', weight: 0},
{id: 'has-hsts', weight: 0, group: 'hidden'},
// User Experience
{id: 'paste-preventing-inputs', weight: 3, group: 'best-practices-ux'},
{id: 'image-aspect-ratio', weight: 1, group: 'best-practices-ux'},
Expand Down
28 changes: 7 additions & 21 deletions core/test/audits/has-hsts-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,11 @@ it('max-age missing, but other directives present', async () => {

const results = await HasHsts.audit(artifacts, {computedCache: new Map()});
expect(results.notApplicable).toBeFalsy();
expect(results.details.items[0].severity).toBeDisplayString('High');
expect(results.details.items[0].description)
.toBeDisplayString('No max-age directive');
expect(results.details.items).toMatchObject([
{
severity: {
i18nId: 'core/lib/i18n/i18n.js | itemSeverityHigh',
values: undefined,
formattedDefault: 'High',
},
description: {
i18nId: 'core/audits/has-hsts.js | noMaxAge',
values: undefined,
formattedDefault: 'No max-age directive',
},
directive: 'max-age',
},
]);
Expand Down Expand Up @@ -98,18 +91,11 @@ it('max-age too low, but other directives present', async () => {

const results = await HasHsts.audit(artifacts, {computedCache: new Map()});
expect(results.notApplicable).toBeFalsy();
expect(results.details.items[0].severity).toBeDisplayString('High');
expect(results.details.items[0].description)
.toBeDisplayString('max-age is too low');
expect(results.details.items).toMatchObject([
{
severity: {
i18nId: 'core/lib/i18n/i18n.js | itemSeverityHigh',
values: undefined,
formattedDefault: 'High',
},
description: {
i18nId: 'core/audits/has-hsts.js | lowMaxAge',
values: undefined,
formattedDefault: 'Max-age too low',
},
directive: 'max-age',
},
]);
Expand Down Expand Up @@ -391,7 +377,7 @@ describe('constructResults', () => {
it('constructs result based on misconfigured HSTS header', () => {
const {score, results} = HasHsts.constructResults(
['max-age=31536000', 'foo-directive', 'includesubdomains', 'preload']);
expect(score).toEqual(1);
expect(score).toEqual(0);
expect(results[0].severity).toBeDisplayString('Low');
expect(results[0].description).toBeDisplayString('Invalid syntax');
expect(results).toMatchObject([
Expand Down
9 changes: 3 additions & 6 deletions core/test/fixtures/user-flows/reports/sample-flow-result.json
Original file line number Diff line number Diff line change
Expand Up @@ -4623,8 +4623,7 @@
},
{
"id": "has-hsts",
"weight": 0,
"group": "best-practices-trust-safety"
"weight": 0
},
{
"id": "paste-preventing-inputs",
Expand Down Expand Up @@ -10907,8 +10906,7 @@
},
{
"id": "has-hsts",
"weight": 0,
"group": "best-practices-trust-safety"
"weight": 0
},
{
"id": "image-aspect-ratio",
Expand Down Expand Up @@ -22596,8 +22594,7 @@
},
{
"id": "has-hsts",
"weight": 0,
"group": "best-practices-trust-safety"
"weight": 0
},
{
"id": "paste-preventing-inputs",
Expand Down
3 changes: 1 addition & 2 deletions core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -6753,8 +6753,7 @@
},
{
"id": "has-hsts",
"weight": 0,
"group": "best-practices-trust-safety"
"weight": 0
},
{
"id": "paste-preventing-inputs",
Expand Down
2 changes: 1 addition & 1 deletion shared/localization/locales/en-US.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion shared/localization/locales/en-XL.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 46c9aa1

Please sign in to comment.