-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Show an card to prompt application password authentication #24561
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
Conversation
|
App Name | WordPress | |
Configuration | Release-Alpha | |
Build Number | 27808 | |
Version | PR #24561 | |
Bundle ID | org.wordpress.alpha | |
Commit | bcad039 | |
Installation URL | 2gkq9eugsit1o |
|
App Name | Jetpack | |
Configuration | Release-Alpha | |
Build Number | 27808 | |
Version | PR #24561 | |
Bundle ID | com.jetpack.alpha | |
Commit | bcad039 | |
Installation URL | 7ka44s0cc3jpo |
38dff55
to
8e51c99
Compare
71236d7
to
2733ea7
Compare
|
||
if let site = try? WordPressSite(blog: blog) { | ||
return MediaServiceRemoteCoreREST(client: .init(site: site)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a reminder to myself that we'll keep the original implementation for media, where we prefer the WP.com API over the REST API.
63314b5
to
aa71a8d
Compare
So that the new application password can be added to the blog
Only available when the "Application Passwords for self-hosted sites" feature flag is on. The feature flag is available as one of the "Experimental Features".
2a0dc27
to
88e4269
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM, though we should ask Jetpack to make an Application Password for us. There's an API for that, but we should only use it if the background auto discovery works.
I think we'll need to add support for that API to wordpress-rs
first in the jetpack
namespace.
return CommentServiceRemoteREST(wordPressComRestApi: api, siteID: dotComID) | ||
} | ||
|
||
// The REST API does not have information about comment "likes". We'll continue to use WordPress.com API for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkmassel FYI, I have reverted the comment API changes, due to lack of "likes" support in the REST API.
|
Note
This PR touched quite a few areas. It might be easier to review this PR commit by commit.
Description
This PR adds a card to the blog details screen to ask users to authenticate with application passwords. This card is only available when the "Application Passwords for self-hosted sites" feature flag/experimental feature is enabled.
Once an application password is authorized to the app, comments and media use it to talk directly to the site via REST API.
This card will show up on all self-hosted sites, with or without WP.com account. Here are a couple of recordings:
⬇️ Site outside of WP.com
self-hosted-sites.mp4
⬇️ Sites within WP.com
wp-com-sites.mp4
Testing instructions
Enable the "Application Passwords for self-hosted sites" feature flag and test the new card on vanilla sites and sites that are connected to WP.com.