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

Migrate header check to setupcheck API #44067

Merged
merged 7 commits into from
Mar 14, 2024

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Mar 7, 2024

See #41364

Summary

Security related header check migrated to new API and done from backend now.
TODO:

  • Migrate the tests
  • Merge the HSTS check in there

Checklist

@come-nc come-nc added the 2. developing Work in progress label Mar 7, 2024
@come-nc come-nc self-assigned this Mar 7, 2024
@come-nc come-nc force-pushed the fix/migrate-header-check-to-setupcheck branch 4 times, most recently from 27eee79 to 225cf90 Compare March 12, 2024 15:39
@come-nc come-nc added 3. to review Waiting for reviews feature: settings and removed 2. developing Work in progress labels Mar 12, 2024
@come-nc come-nc added this to the Nextcloud 30 milestone Mar 12, 2024
@come-nc
Copy link
Contributor Author

come-nc commented Mar 12, 2024

/compile /

@come-nc come-nc requested review from susnux, a team, ArtificialOwl, icewind1991 and Altahrim and removed request for a team March 12, 2024 16:52
@@ -68,7 +68,7 @@ public function run(): SetupResult {
];

foreach ($providers as $provider) {
foreach ($this->runHEAD($this->urlGenerator->getWebroot() . $provider) as $response) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that is removed, then drop also runHEAD function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still used in other checks I think but yeah we could move all of them.

@come-nc come-nc modified the milestones: Nextcloud 30, Nextcloud 29 Mar 12, 2024
@come-nc come-nc force-pushed the fix/migrate-header-check-to-setupcheck branch from 1f7d36d to dd211d0 Compare March 12, 2024 16:59
@come-nc
Copy link
Contributor Author

come-nc commented Mar 12, 2024

/compile /

@Altahrim Altahrim mentioned this pull request Mar 12, 2024
Copy link
Collaborator

@Altahrim Altahrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If domain contains only one dot (xxxxx.yyy), we could also advise to use HSTS includeSubDomains and preload (https://hstspreload.org/)

apps/settings/lib/SetupChecks/SecurityHeaders.php Outdated Show resolved Hide resolved
@susnux susnux force-pushed the fix/migrate-header-check-to-setupcheck branch from 7b00b81 to 99e967a Compare March 13, 2024 11:49
@susnux
Copy link
Contributor

susnux commented Mar 13, 2024

/compile amend

Signed-off-by: nextcloud-command <[email protected]>
@nextcloud-command nextcloud-command force-pushed the fix/migrate-header-check-to-setupcheck branch from 99e967a to 58ae7e4 Compare March 13, 2024 12:07
@Altahrim Altahrim mentioned this pull request Mar 14, 2024
@come-nc
Copy link
Contributor Author

come-nc commented Mar 14, 2024

If domain contains only one dot (xxxxx.yyy), we could also advise to use HSTS includeSubDomains and preload (https://hstspreload.org/)

I do not have any knowledge about this, I’d say that would be a followup, let’s migrate the feature first.

@come-nc come-nc merged commit d435f0c into master Mar 14, 2024
160 checks passed
@come-nc come-nc deleted the fix/migrate-header-check-to-setupcheck branch March 14, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants