[PoC] WP-API and Cookie Authentication#202
Conversation
|
I am trying to test this on my self-hosted site but couldn't make it work:
I can provide admin credentials from slack |
|
As you pointed out in slack redirecting to this screen was the problem:
After solving that issue the current solution started working, it has a bit of a latency compared to .com but I think it is not a big problem:
Thank you for putting effort to this! This is totally some dark magic :) This is absolutely making us support much more auth API calls on self hosted sites. But can we say that we shouldn't lean on to this solution much? First reason is its fragility(it's not working if login page redirects us to some other page as above ☝️). Secondly, it is hard to tell how this could go wrong so we might need to revert this one day? |
b36dafd to
8adc25c
Compare
8f00342 to
67e0f82
Compare
67e0f82 to
ea5d42f
Compare
|
CircleCI is failing to validate the pod spec because the required Secret wrapper is in a WordPressShared beta. I'll update that once the final release is out, but the can be reviewed in the meantime. |
pinarol
left a comment
There was a problem hiding this comment.
First of all thanks for this advanced solution, I learned a lot!
I gave another look at this today but haven't tested this version yet. Dropping some design thoughts, let me know what you think.
pinarol
left a comment
There was a problem hiding this comment.
I once again tested on a self-hosted site and it is working as described.
Leaving tiny code comments but overall this looks good.
WordPressKit/Authenticator.swift
Outdated
| } | ||
|
|
||
| public extension CookieNonceAuthenticator { | ||
| static func cookieNonce(username: String, password: String, loginURL: URL, adminURL: URL, version: String) -> Authenticator { |
There was a problem hiding this comment.
static func cookieNonce(...)
This method doesn't look necessary since CookieNonceAuthenticator is public. One could simply use the initializer instead.
But, if the intention was to keep CookieNonceAuthenticator private and provide a kind of factory method for Authenticator(which I think would be better) then it would make sense to keep it but make it an extension for Authenticator instead:
public extension Authenticator {
static func cookieNonce(username: String, password: String, loginURL: URL, adminURL: URL, version: String) -> Authenticator {
....
There was a problem hiding this comment.
Same is true for:
public extension TokenAuthenticator {
static func token(_ token: String, shouldAuthenticate: RequestAuthenticationValidator? = nil) -> Authenticator {
return TokenAuthenticator(token: token, shouldAuthenticate: shouldAuthenticate)
}
}
There was a problem hiding this comment.
I agree that they're not very useful now. Those were initially created to be able to use the authenticators more like enums, so you would be able to do something like:
let authToken = "secret"
let api = WordPressOrgRestApi(
url: baseURL,
authenticator: .token(authToken)
)However, since I ended up with a convenience initializer that takes a Blog, these aren't being used in that way and can be removed.
There was a problem hiding this comment.
For the record, I needed to change the code from being extension of Authenticator to the specific classes because of an error that the Swift compiler was returning.
I assume this must have been some recent change on the Swift compiler, and I will remove the methods in favor of the blog init helpers.
# Conflicts: # WordPressKit.xcodeproj/project.pbxproj
etoledom
left a comment
There was a problem hiding this comment.
Tested on Self-Hosted running WP v5.3 and v.5.2. Both strategies seem to work great!
I experienced the same problem mentioned on this comment. We have decided to tackle this problem on a separated thread, and go ahead with this one since it's already so much better than the current state.
Great job @koke and @SergioEstevao ! 🎉
|
I didn't test again but changes look good to me 👍 |
Description
We are currently lacking a way to perform requests to the WordPress REST API because we lack a good authentication method. The only method available today is based on cookies and nonces. There might be support for Basic Authentication and oAuth2 eventually, but there's no timeline for that.
At the same time, we are coming across more and more features in gutenberg-mobile that require getting data from the API. In wordpress-mobile/WordPress-iOS#12714 we managed to work around this issue because we could get enough information through an unauthenticated request. As part of wordpress-mobile/gutenberg-mobile#1513 we will need to get the theme's color palette that will be exposed by https://core.trac.wordpress.org/ticket/48798, but only for authenticated users.
In this scenario, I think it makes sense to make an attempt at implementing a solution for the only available auth method, as fragile as that might be, so here it is.
This new API Client supports both Token and Cookie authentication, although only the latter has been tested so far (
GutenbergNetworkRequestis already reusing the existing WordPress.com API Client for WordPress.com sites), but the basic design would allow for the other future authentication methods (detecting which one we should use might make things interesting, but that's not a problem for today).The
RequestAdapterandRequestRetrierpatterns that Alamofire offer seem pretty convenient for authentication, and if this moves forward, it would be interesting to refactor the existingWordPressComRestApiclient, since there is a lot of overlap with the .org one, and a lot of the specifics could be configurable via adapters.How it works
When an API request fails with a 401, the CookieNonce authenticator will POST to
wp-login.phpand ask to redirect to the new post page (wp-admin/post-new.php). The editor is the only page I've found where we can extract a usable nonce. Then the authenticator will look at the HTML source and extract the nonce from the Gutenberg initialization code. This also means the method won't work if they don't have Gutenberg as the default editor for new posts (pre-5.0 or classic editor plugin), but we don't have much of an alternative there.Nonces have an expiration of 1 day, so instead of complicating things too much with storage and invalidation, I'm just keeping the nonce in memory as part of the Authenticator. This is tied to the lifetime of the API Client, which is tied to the Blog. I'm not checking for the presence of an existing nonce or cookie, so if a request fails because either expired, we would try to authenticate again and resolve the issue.
This also means that we still have to find a way to flag when the authentication fails. We don't want to be retrying the login on every API request if it's not going to work, and could lead to hosting providers blocking the user 😱
Testing Details
I've tested this from WPiOS wordpress-mobile/WordPress-iOS#13020 and the steps described in wordpress-mobile/WordPress-iOS#12714:
Run metro server on develop
Add a image block to gutenberg.
Select an image from the WordPress media Library.
Select settings and choose a different image setting.
Change to HTML mode.
Check that the image URL has changes according to the selected image size.
Please check here if your pull request includes additional test coverage.