Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#51638 closed task (blessed) (fixed)

Add Site Health test for verifying the Authorization header works as expected

Reported by: timothyblynjacobs's profile TimothyBlynJacobs Owned by: timothyblynjacobs's profile TimothyBlynJacobs
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.6
Component: Site Health Keywords: has-patch commit has-screenshots
Focuses: rest-api Cc:

Description

Application Passwords utilizes the Authorization header to pass the Basic Authentication credentials. In some server configurations, the values sent in the Authorization header won't reach WordPress.

Because of this, we added the wp_populate_basic_auth_from_authorization_header() and the RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] Mod Rewrite rule. This should account for the vast majority of failures.

This patch adds a test to Site Health to verify that the Authorization header is working as expected. If it isn't, we direct the user to the Permalinks screen which will regenerate their .htaccess file in case the rule was missing.

Attachments (1)

wp-api-generated.diff (1.0 KB) - added by garrett-eclipse 4 years ago.
Updated wp-api-generated.js

Download all attachments as: .zip

Change History (12)

This ticket was mentioned in PR #665 on WordPress/wordpress-develop by TimothyBJacobs.


4 years ago
#1

  • Keywords has-patch added

#2 @TimothyBlynJacobs
4 years ago

Two changes of note.

  1. I introduced support for an async test to specifying a list of headers that get included in the request. This is so we can fill the Authorization header with a known value.
  2. I added support for a skip_cron entry that can be used to declare that an async test shouldn't be run as cron. This test really doesn't make sense in a loopback environment.

This ticket was mentioned in Slack in #core-passwords by timothybjacobs. View the logs.


4 years ago

TimothyBJacobs commented on PR #665:


4 years ago
#4

For reference:

https://i0.wp.com/user-images.githubusercontent.com/3460448/97336223-252e2500-1855-11eb-83aa-591c07d277d3.png
https://i0.wp.com/user-images.githubusercontent.com/3460448/97336316-40993000-1855-11eb-9f33-87c8cc939d26.png
https://i0.wp.com/user-images.githubusercontent.com/3460448/97336531-80f8ae00-1855-11eb-8781-90fc19b8eabb.png

#5 @Clorith
4 years ago

  • Keywords commit has-screenshots added

Looking good, and the copy update is also in place I see, let's get this in as well.

#6 @TimothyBlynJacobs
4 years ago

  • Owner set to TimothyBlynJacobs
  • Resolution set to fixed
  • Status changed from new to closed

In 49334:

Site Health, App Passwords: Test if the Authorization header is populated correctly.

App Passwords rely on the Authorization header to transport the Basic Auth credentials. For Apache web servers, WordPress automatically includes a RewriteRule to populate the value for servers running in CGI or FastCGI that wouldn't ordinarily populate the value.

This tests if the header is being filled with the expected values. For Apache users, we direct the user to visit the Permalinks settings to flush their permalinks. For all other users, we direct them to a help document on developer.wordpress.org.

Props Clorith, marybaum, TimothyBlynJacobs.
Fixes #51638.

TimothyBJacobs commented on PR #665:


4 years ago
#7

Fixed in 0187bbdd7e4554b8c95c22fc9801cb73bd9086f1.

This ticket was mentioned in Slack in #core by sergey. View the logs.


4 years ago

@garrett-eclipse
4 years ago

Updated wp-api-generated.js

#9 @garrett-eclipse
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

It appears the wp-api-generated.js was overlooked here as it's appearing in diffs off trunk after running grunt test. Uploaded wp-api-generated.diff to address.
Thread - https://wordpress.slack.com/archives/C02RQBWTW/p1603922117261300

#10 @SergeyBiryukov
4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 49368:

REST API: Regenerate test fixtures after [49334].

Props garrett-eclipse.
Fixes #51638.

#11 @SergeyBiryukov
4 years ago

In 49370:

REST API: Remove accidentally duplicated key in test fixtures.

Follow-up to [49334], [49368].

See #51638.

Note: See TracTickets for help on using tickets.