WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 4 days ago

#42790 assigned feature request

Permit basic authentication to the REST API over SSL

Reported by: kadamwhite Owned by: andraganescu
Milestone: Future Release Priority: high
Severity: normal Version:
Component: REST API Keywords: 2nd-opinion has-patch
Focuses: rest-api Cc:

Description

The only REST API authentication scheme currently supported in core is cookie/nonce authentication. This is sufficient for front-end usage within wp-admin, themes, and plugins, but prohibits full consumption of the REST API from external applications, particularly the WordPress mobile apps.

After discussion with the WordPress mobile app team, we propose adding core support for REST API authentication via basic auth for SSL-enabled environments.

These mobile apps currently use basic authentication to connect via the XML-RPC API. The XML-RPC API is disabled in some hosting environments, but discussion with the hosting team suggests this is usually to avoid amplification attacks via pingbacks rather than anything related to basic authentication itself. Using this scheme only over secured connections mitigates the primary security criticism of basic authentication. As an example, the Github API (among many others) supports basic authentication: https://developer.github.com/v3/auth/ without any clear drawbacks. These APIs also preference basic auth because it is substantially simpler to use than OAuth schemes, even with a central broker.

From the perspective of a mobile app developer, preventing REST API access via that same authentication scheme on the grounds that we are simultaneously pursuing alternatives unfairly disenfranchises the mobile app team and blocks significant potential code improvements.

Attachments (2)

42790.diff (1.0 KB) - added by georgestephanis 3 years ago.
42790.1.diff (1.5 KB) - added by georgestephanis 22 months ago.

Download all attachments as: .zip

Change History (54)

#2 @kadamwhite
3 years ago

Fast to the punch @georgestephanis ! As you note another implementation would be the json_basic_auth_handler method from https://github.com/WP-API/Basic-Auth -- the technical approach is similar, just with additional filters and error handling. (While that plugin has never made it into the plugin directory it has been used in production in a number of sites over the past few years, in some cases by having that method in-lined into the application code.)

I'm interested in the loop-back to determine whether auth headers are forwarded; how prevalent is that issue across hosts?

Further discussion with @nacin and others at the WCUS contributor day has pointed out that Github's solution permits the use of authentication tokens, which would be preferable to the direct use of user passwords as they can be individually registered and revoked. We'd want to do some design work to find a token generation & registration flow that works for mobile app users if we go that route.

#3 @georgestephanis
3 years ago

It's prevalent enough, my personal site on DreamHost needs the workaround.

#4 @georgestephanis
3 years ago

And re: token generation flow, this isn't merged yet to application passwords, but should handle what you intend, I expect:

https://github.com/georgestephanis/application-passwords/pull/39

Vizrec on the pull.

#5 @dd32
3 years ago

  • Type changed from defect (bug) to feature request

This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.


3 years ago

#7 @rmccue
3 years ago

We discussed this a bunch in today's Slack meeting.

#8 @georgestephanis
22 months ago

New patch gets around the fastcgi mode by accepting the Basic authorization via a WP-Authorization header instead of the Authorization header that is occasionally not passed along.

#9 @pento
21 months ago

  • Version trunk deleted

This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.


20 months ago

#11 @kadamwhite
11 months ago

  • Keywords close added

This may be somewhat controversial, but based on recent discussions in Slack, on GitHub, elsewhere on Trac, and at the WordCamp US contributor day, I think that this ticket should be considered for closure in favor of using an OAuth2 authentication flow which exchanges is a JWT auth token in lieu of a bearer token. See https://GitHub.com/wp-api/authentication, which is currently bare bones but which we hope to get to merge proposal over the course of 2020.

#12 follow-up: @Otto42
11 months ago

  • Keywords close removed

@kadamwhite Disagree. For sites using SSL, we should add Basic Authentication to the login flow. Not just to the REST API, but to all flows, using the general authentication mechanisms. Essentially, add basic auth to the authenticate filter for the case when SSL is enabled.

OAuth2 is a bad mechanism for this, designed for an entirely different purpose. It's for having Facebook talk to Google and the like. Where the user has accounts on two different services and wants them to share information. WordPress is a service in this equation, but more to the point it is a piece of software, actually self-run by the user. It doesn't fit OAuth's original design. Never has. The user wants to authenticate to their own site using their own credentials, and Basic Auth fits that just fine. Yeah, okay, it's not great when you're sending passwords in the clear, but for the case where SSL is enabled, it is a far better user experience and secure enough for a first pass.

+1 to Basic Auth support.

#13 @Otto42
11 months ago

Additional: for the proposal of adding basic auth to the authenticate filter, to allow it for all login flows, I would have the ssl check be capable of being disabled via a filter, or a defined constant.

If I wanna run http on my site and allow basic auth, then that's my call. It shouldn't be the default, but it should be possible.

When credentials are already sent in the clear, then there is no harm in allowing that via add-ons. The benefit here is in bringing this job into core instead of requiring plugins to do the job, perhaps incorrectly.

#14 in reply to: ↑ 12 @georgestephanis
10 months ago

Replying to Otto42:

@kadamwhite Disagree. For sites using SSL, we should add Basic Authentication to the login flow. Not just to the REST API, but to all flows, using the general authentication mechanisms. Essentially, add basic auth to the authenticate filter for the case when SSL is enabled.

Just to confirm, you feel that it should also be expanded so that basic auth can be used in lieu of cookies for http requests to wp-admin, as well as the legacy xmlrpc api?

If it can be switched to allow non-https requests as well, we should also include a switch to disallow it to be used even for https requests -- in the case of two-factor authentication where just the username and password alone are insufficient.

#15 @Otto42
10 months ago

No, I don't think it makes sense to allow it for http requests. I'd say https only for basic auth is sufficient. And any 2 factor case should disable it, although that is still within plugin territory at present.

This ticket was mentioned in Slack in #core-restapi by kadamwhite. View the logs.


10 months ago

#17 @georgestephanis
10 months ago

sorry, i didn't mean http as in non-https for wp-admin -- i meant html api -- that is the normal wp-admin html ui.

#18 @koke
10 months ago

The mobile apps are starting to need this (or an alternative) more and more often. With our focus on Gutenberg in the apps, we are finding many scenarios where Gutenberg obtains data from the REST API that we can't access because we have no good way to authenticate.

I think in the long term we would rather use a token-based approach like the one being discussed in https://github.com/wp-api/authentication but that's still in the early stages, and it looks like it's going to take some time to get right. Besides, we are going to need a migration path for all the existing users for whom we already have usernames and passwords, and basic auth seems like a great solution for the short and medium term.

It would be very helpful for Gutenberg on mobile to have this available on core soon.

#19 @koke
10 months ago

Another thing that would be good to improve is discoverability. I was happy to see that there's built-in support in the API for exposing the available authentication methods, but I haven't seen this implemented by any plugin other than OAuth1.

This ticket was mentioned in Slack in #core-restapi by sergioestevao. View the logs.


8 months ago

#21 @georgestephanis
7 months ago

Minor correction on 42790.1.diff --

patch line 303 -- $GLOBALS['user'] should be $GLOBALS['current_user'], but it may be able to swap to wp_set_current_user() instead -- that function can take a WP_User object as easily as an id for the first parameter -- I forget if there was a reason why I hadn't done that.

This ticket was mentioned in PR #169 on WordPress/wordpress-develop by draganescu.


7 months ago

Trac ticket: https://core.trac.wordpress.org/ticket/42790

Refreshes the proposed patch to apply cleanly to current WordPress master and also fixes setting the authenticated user as the current user (props @georgestephanis).

---
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See [GitHub Pull Requests for Code Review](https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/) in the Core Handbook for more details.

#23 @andraganescu
7 months ago

  • Focuses rest-api added
  • Keywords has-patch needs-testing dev-feedback 2nd-opinion added
  • Milestone changed from Awaiting Review to 5.5
  • Priority changed from normal to high

The patch linked in te Github PR above is ready for review.

This ticket aims to add BasicAuth to the REST API on hosts that support SSL. Basic Auth, although it is not the best authentication method, having the downsides of sending passwords over the wire, the likely storing passwords on clients for re-authentication and being a probable target for brute force, it is a very convenient authentication method, especially for apps that require the admin credentials of users anyway.

Moreover, the downsides of having BasicAuth for the REST API on SSL enabled hosts are not regressions considering the fact that XML-RPC already supports it and is turned on by default. The fact that this implementation is only enabling BasicAuth when SSL communication is on is in fact a progress, which addresses very well the "password over the wire" downside.

Given the above, this Ticket is just a feature request which helps when developing with REST clients in 3rd party apps and which doesn't make WordPres any less secure, than the current status quo. Of course, when there will be a clear way forward and a better authentication will be available the opportunity of still having BasicAuth should be reassessed, provided there will also be a way to maintain users logged in status on migration.

#24 @georgestephanis
7 months ago

provided there will also be a way to maintain users logged in status on migration.

Any way forward that deprecates basic auth down the road would be quite simple:

Include a single endpoint that supports basic auth, and enables the client to swap its existing credentials for a token. Perhaps also with the site sending a notification email to the user.

This feature request has been requested heavily by the mobile apps teams, specifically those implementing Gutenberg in mobile and need a way to query against REST API endpoints for blocks.

#25 @noisysocks
7 months ago

  • Keywords needs-testing dev-feedback removed
  • Owner set to andraganescu
  • Status changed from new to assigned

Excited to get this in! Thanks @andraganescu and @georgestephanis for working on it.

I tested the linked PR and left some comments inline there. There's a bug when an incorrect username or password is specified, and I'd like a REST API component maintainer to chime in with whether or not this code should be moved out of rest_api_loaded into e.g. its own function.

This ticket was mentioned in Slack in #core-restapi by andraganescu. View the logs.


6 months ago

#27 follow-ups: @spacedmonkey
6 months ago

I foundamentally disagree with this being core. I think it is a massive security issue that allow bad actors to create, edit or delete content on a users site. Without brute force protection in core, it would also bad actors to brute force a password. I know this issue with the normal password login screen, the rest api allow for a high level of automatication when it comes to attacking sites.

How about the following.

  • Create a new API that allow for password and username to be submitted ( requiring SSL ).
  • This endpoint returns a token ( maybe a JW Token ).
  • This token would live for the same amount of time as the users cookie ( 2 days / 2 weeks ).
  • Another api could be added to get a refreshed a get a new token.
  • If a user changes passwords, all tokens will be revoked.
  • Authicated requests will require the token to sent as a header.

There is already a plugin that does this on github called Tmeister/wp-api-jwt-auth.

This ticket was mentioned in Slack in #mobile by andraganescu. View the logs.


6 months ago

#29 in reply to: ↑ 27 @georgestephanis
6 months ago

While everything you stated is a totally valid concern, I don't understand how it's any different from the status quo?

Replying to spacedmonkey:

I think it is a massive security issue that allow bad actors to create, edit or delete content on a users site.

I'd like to understand how you think adding this in would introduce a new concern on that front. Core already has basic authentication enabled via XML-RPC which means all of your concerns apply to what's been status quo for core for well over a decade.

The only relevant difference is that this requires HTTPS, whereas XML-RPC can operate over unencrypted HTTP protocols.

Without brute force protection in core, it would also bad actors to brute force a password. I know this issue with the normal password login screen, the rest api allow for a high level of automatication when it comes to attacking sites.

This is arguing for brute force protection in core. Not really relevant to this issue. If you want to argue for that, maybe reopen #24193? Folks can already brute force requests to wp-login.php and xmlrpc.php -- I'm not quite sure why you think it would be easier to brute force a rest api than the existing pathways -- but would be delighted to hear your reasoning.

How about the following.

  • Create a new API that allow for password and username to be submitted ( requiring SSL ).
  • This endpoint returns a token ( maybe a JW Token ).

How would this avoid your concern about brute forcing? The instant someone can pass in a username and password, and tell whether it worked (by getting a token back) they know the credentials and can send them again to get new tokens at will.

  • This token would live for the same amount of time as the users cookie ( 2 days / 2 weeks ).
  • Another api could be added to get a refreshed a get a new token.
  • If a user changes passwords, all tokens will be revoked.
  • Authicated requests will require the token to sent as a header.

There is already a plugin that does this on github called Tmeister/wp-api-jwt-auth.

There are a number of plugins that perform similar token based authentication. I wrote the initial pass of https://github.com/wordpress/application-passwords myself, and iirc @valendesigns based some work on application-passwords when working on the https://github.com/WP-API/jwt-auth plugin.

Token based authentication has the benefit of being able to individually revoke specific tokens, but if an application is ever given your actual username and password (as you suggested above), then that makes any security benefit entirely null and void. The only way to secure against a rogue application that has your real credentials is just to change your credentials. Which, as you also stated that would revoke all tokens, what's the benefit over just passing your username and password encrypted via HTTPS?

#30 in reply to: ↑ 27 @Otto42
6 months ago

Replying to spacedmonkey:

I think it is a massive security issue that allow bad actors to create, edit or delete content on a users site. Without brute force protection in core, it would also bad actors to brute force a password.

There is no real security issue in permitting basic auth in core. You're sending a username and a password. Since you can do this with both wp-login and xml-rpc already, then allowing it in any particular other request isn't an issue. The security of allowing such over plaintext headers is mitigated by requiring HTTPS (not that we require that for the other methods, but still).

Any possible alternative means of authentication for REST requests would have identical security issues. At some point, the user has to login to the site, and that means sending it a username and a password. If these are being input into say, a phone app which makes REST requests, then the security is identical since you're giving the app the password anyway.

Currently, WordPress uses the "cookie" method. Basically, on login, your browser gets a cookie. This cookie serves as a username and password all in one. Anybody with that cookie is "logged in" until the cookie expires, or the cookie gets revoked (which can be done a number of ways, changing the password is one of those ways).

The "JWToken method" you propose is functionality identical to this cookie method. You propose an endpoint which takes username and password, and gives the browser a secret code (token) in return. The token can expire, it can be refreshed, it can be revoked, all the same things. It's just an easier way to get a cookie. However, it is no more secure as the u/pw combo must be passed to the endpoint, which means you have to give them to the app anyway. The cookie method, in this case, is functionally identical.

A basic auth method is well understood and well documented. It's just a header with the username and password in it. Not terribly secure, to be sure, but simple, easy to implement, and every platform and library in existence can easily support it. It's easy to implement from the WP side too, it can be a single function hooked to the "authenticate" filter. It is as secure as your password is, which may not be terribly secure, but that's more or less the user's problem. It can be brute forced, sure, but any method of accepting a password can be brute forced. There's no real difference in taking a password via a POST or via a header.

Other methods to provide authentication can be implemented down the road, as needed. But any approach which takes a password from the user is as equally insecure as every other approach. One could envision a method where the user logs into WordPress themselves, sets up an "application password", which they then give to the app in question. This is implemented by plugins as well, but it is sort of putting a lot of burden on the user to do that. Other approaches like OAuth and similiar will work just fine, but these tend to not fit the model precisely, as we're mainly concerned with letting users connect programs they are running with their own sites, while these models tend to be more of the "letting my Facestagram talk to my Instabook account" variety. OAuth and the like let users of services have those services talk to one another, not so much let them use a program to interact with their own website.

So, at a basic level, the simple basic authentication is easy to implement, drives the line forward, and does not introduce any new security issues. Brute force, like it or not, is not a WordPress security issue, it is a server configuration issue.

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


6 months ago

#32 @andraganescu
5 months ago

Hey! Right now it is unclear how this should move along.

I know there has been some discussion in Slack REST API channel about the ticket, but I am not sure a conclusion has been reached. I see in Slack that a current stance from the REST API team is that:

  • if by the time 5.5 is released
    • there is no authentication method other than cookie
  • then we can add Basic Auth as an option for 5.6

Is that something this ticket can consider as progress? Is it a shared stance?

From the perspective of "argumentation" both the proponents and the opponents have covered everything, so I think @TimothyBlynJacobs 's proposal above would be a good compromise for the teams bothered by cookie authentication.

This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.


4 months ago

#34 @TimothyBlynJacobs
4 months ago

From my experience the fundamental difference between Basic Auth and logging in via wp-login.php is that one mechanism can easily have additional security protections put in place, and one can't.

With wp-login.php, it can easily support Recaptcha, Two-Factor, Passwordless, SSO, network-based protection ( like Jetpack ) Cloudflare and other protections.

With XML-RPC that is a lot more difficult. Anything interactive like Recaptcha, Passwordless or SSO doesn't work because it isn't interactive auth.

Two-Factor can be done, but it is an incredibly painful user experience, and pretty much only works with a TOTP type system since you'll typically append the 2fa code to the password.

Adding IP based protection is also much more difficult to impossible if you want to integrate with non-browser based applications. Both something like Jetpack Protect that is network based, or local to the server using fail2ban, etc... Take a hypothetical Zapier app. All the login requests will be occurring from the same set of IPs. If a user types in their WordPress password to Zapier wrong one too many times, they could lock out the entirety of Zapier.

Cloudflare based protections also are much weaker since it can really only be done in API mode.

From our data, we see far more attacks via XML-RPC, and this is with a plugin that recommends disabling XML-RPC if possible. It appears that APIs are being used and more for credential stuffing attacks: https://www.csoonline.com/article/3527858/apis-are-becoming-a-major-target-for-credential-stuffing-attacks.html


If the only use case we were interested in was that of something like MarsEdit where your password would only sit in your machine, I would be less worried about all of the above. But there is also the matter of giving your password to another service.

There is the obvious concern that making this the way applications interact with your WordPress site will make users more comfortable entering their WordPress password into places other than their WordPress site. This has clear phishing implications.

What if the service gets compromised? With OAuth or Application Passwords revocation becomes much simpler. Any non-compromised integrations can continue to work, but the compromised service key can be revoked. With basic auth, you'll need to rotate your actual WP password which would involve changing it with _every_ service you've signed up for. Additionally, users reuse passwords, a lot. If the service gets compromised, we shouldn't add to the damage that compromise can do.


While everything you stated is a totally valid concern, I don't understand how it's any different from the status quo?

I agree that it isn't different than the status quo. But in my opinion, the status quo is not great. Since the REST API is how we want 3rd party developers to interact with WordPress going forward ( a future GraphQL core project could also reuse the wp-api authentication mechanism ), I think we have a duty to make the authentication alongside it much more sound.

One of the reasons so many people disable XML-RPC is because of the security ramifications, as I outlined above I don't think those concerns are unwarranted. If we shipped basic auth for the REST API, I think a large number of users would disable it, and we wouldn't have made any progress.


@andraganescu I think the WP-API team's authentication project is in great shape for a merge proposal in 5.6 with plenty of time to spare. If you are interested in getting involved: https://github.com/WP-API/authentication/projects/2 It follows fairly closely to how WooCommerce's authentication flow that seems to work quite well and is standards based.

So from my perspective, we should continue to address the open issues in the wp-api/authentication repo and finish our MVP. My goal would be to complete a merge proposal by mid-July. If that merge proposal is rejected, then taking another look at this basic auth ticket or @georgestephanis's app passwords would be what I'd endorse. Chatting in #core-restapi, my and @spacedmonkey current preference would be to take this out of the 5.5 milestone.

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


4 months ago

#36 @TimothyBlynJacobs
4 months ago

  • Milestone changed from 5.5 to Future Release

Moving out of 5.5 per my last comment.

#37 @prbot
2 months ago

draganescu commented on PR #169:

It looks like 5.6 will have better authentication so closing this.

This ticket was mentioned in PR #540 on WordPress/wordpress-develop by georgestephanis.


9 days ago

This is an alternative to Basic Auth in #169

Based off existing work in https://github.com/WordPress/application-passwords

PR in progress, collaborating with @TimothyBJacobs for the moment. Trying to make sure this is workable to provide an alternative to Basic Auth.

Trac ticket: https://core.trac.wordpress.org/ticket/42790

#39 @prbot
9 days ago

georgestephanis commented on PR #540:

Added some changes since the last comment phase -- spread the code out across the admin in more relevant places.

#40 @prbot
9 days ago

TimothyBJacobs commented on PR #540:

Instead of suggesting a user to edit their .htaccess, we should probably add it to the default config in WP_Rewrite::mod_rewrite_rules as described in https://core.trac.wordpress.org/ticket/39224. Related https://core.trac.wordpress.org/ticket/47077

#41 @prbot
9 days ago

TimothyBJacobs commented on PR #540:

We should also populate rest_authentication_errors with an error message if the app auth fails.

#43 @prbot
8 days ago

TimothyBJacobs commented on PR #540:

I think we should require the WordPress site be on SSL. And also verify that both the success and reject URLs are https urls as well.

Ideally, we'd also mandate a state parameter be included. Since the URLs are completely arbitrary, client's could include it themselves, but I think it'd be a good idea to force developers to include it to encourage them to implement CSRF protections on the client side.

#44 @prbot
7 days ago

georgestephanis commented on PR #540:

And also verify that both the success and reject URLs are https urls as well.

Possibly to manually upgrade or display a warning for http urls, but please no for mandating https --

It was explicitly written to account for custom protocols such as WordPress:// so that apps could open a browser tab for the user to authenticate, with a return url that would pass the token directly back into the app upon completion.

#45 @prbot
7 days ago

georgestephanis commented on PR #540:

Re: https://github.com/WordPress/wordpress-develop/commit/42693f0bb362712ec82fde6417b2688177f20bf8

Feels like we should still have something that maybe runs and caches an option to make sure it takes? It feels like it could create a big support burden if a misconfiguration fails silently.

#47 @prbot
6 days ago

TimothyBJacobs commented on PR #540:

Possibly to manually upgrade or display a warning for http urls, but please no for mandating https --

It was explicitly written to account for custom protocols such as WordPress:// so that apps could open a browser tab for the user to authenticate, with a return url that would pass the token directly back into the app upon completion.

That's a good point, how about erroring just on http? I think then we also need to remove the esc_url_raw in authorize-application.php since that will check the list of allowed protocols.

Feels like we should still have something that maybe runs and caches an option to make sure it takes? It feels like it could create a big support burden if a misconfiguration fails silently.

We could perhaps run the Site Health test from the profile screen? I don't think we do this for other site health tests, like checking that the REST API is accessible and displaying that on the editor screen, but perhaps that would be a good thing to add.

---

I think the tests are failing because we need to make sure the $wp_rest_application_password_status global is cleared.

#49 @prbot
5 days ago

aristath commented on PR #540:

This is more of a design remark:
The wp-admin/authorize-application.php page would look a lot better if it had a look/aesthetic similar to that of the login page... There really is no need to have it in the dashboard itself.


Also a note to anyone trying to test this on localhost: Since this requires SSL (which is obviously not the case with local sites), you'll need to add this somewhere (like a PHP file in the mu-plugins folder):
{{{php
add_filter( 'wp_is_application_passwords_available', 'return_true' );
}}}

This ticket was mentioned in Slack in #hosting-community by mike. View the logs.


4 days ago

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


4 days ago

#52 @prbot
4 days ago

georgestephanis commented on PR #540:

The wp-admin/authorize-application.php page would look a lot better if it had a look/aesthetic similar to that of the login page... There really is no need to have it in the dashboard itself.

I do kinda wanna make it obvious that the user can just walk away from this process if they choose to and not have it feel like a "squeeze page" that forces a choice. I also hesitate on taking over the entire admin ui unless it's well discussed a conscious decision.

Note: See TracTickets for help on using tickets.