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

Update handling of App Pwds #730

Closed
jeffpaul opened this issue Mar 3, 2021 · 5 comments · Fixed by #934
Closed

Update handling of App Pwds #730

jeffpaul opened this issue Mar 3, 2021 · 5 comments · Fixed by #934
Assignees
Labels
help wanted type:enhancement New feature or request.
Milestone

Comments

@jeffpaul
Copy link
Member

jeffpaul commented Mar 3, 2021

Is your enhancement related to a problem? Please describe.
Following on from #682, we should look to update how Distributor leverages Application Passwords based on feedback from @TimothyBJacobs:

Just wanted to mention that you can check whether the target site supports app passwords by making a request to the REST API Index ( /wp-json ) and looking for application-passwords in the authentication section. See rest_add_application_passwords_to_index for details. That should also be the preferred way to get the URL to redirect the user to instead of hardcoding authorize-application.php.

Describe the solution you'd like

Designs

Describe alternatives you've considered

Additional context

@jeffpaul jeffpaul added type:enhancement New feature or request. help wanted labels Mar 3, 2021
@jeffpaul jeffpaul added this to the 1.7.0 milestone Mar 3, 2021
@dkotter dkotter self-assigned this Mar 19, 2021
@jeffpaul jeffpaul modified the milestones: 1.7.0, 1.6.5 Apr 27, 2021
@jeffpaul jeffpaul assigned dinhtungdu and unassigned dkotter Aug 27, 2021
@helen helen modified the milestones: 1.6.5, 1.7.0 Aug 31, 2021
@jeffpaul jeffpaul modified the milestones: 1.7.0, 1.6.6 Sep 3, 2021
@jeffpaul jeffpaul modified the milestones: 1.6.6, 1.7.0 Sep 27, 2021
@dinhtungdu
Copy link
Contributor

Note that with the approach above, we can avoid hard-coding application-password.php, but we still need to hard-code admin.php?page=auth_app for bundled Application Passwords.

@jeffpaul
Copy link
Member Author

Also noting that we should look to update our directions on setting up external connections where folks on WordPress X.Y and above do not need the Application Passwords plugin as it's part of WP core: https://github.com/10up/distributor#set-up-external-connections-using-application-passwords

@skorasaurus
Copy link
Contributor

@jeffpaul

Also noting that we should look to update our directions on setting up external connections where folks on WordPress X.Y and above do not need the Application Passwords plugin as it's part of WP core: https://github.com/10up/distributor#set-up-external-connections-using-application-passwords

I made a pull request to address this - #838

@Ritesh-patel Ritesh-patel self-assigned this May 25, 2022
@peterwilsoncc
Copy link
Collaborator

As work in beginning on Distributor 2.0.0, I've moved this enhancement off the 1.7.0 milestone to allow the point release to focus on bug fixes.

@peterwilsoncc peterwilsoncc modified the milestones: 1.7.0, 2.0.0 Jul 11, 2022
@jeffpaul jeffpaul moved this from To Do to Backlog in Open Source Practice Jul 12, 2022
@peterwilsoncc
Copy link
Collaborator

In 7f53170 I added the admin_url( 'authorize-application.php' ) as a hardcoded value in the Distributor endpoint. I did this to allow sites to get all the information required by distributor in a single request.

function distributor_meta() {
return array(
'version' => DT_VERSION,
'core_has_application_passwords' => true,
'core_application_passwords_available' => ! wp_is_application_passwords_available() ? false : true,
'core_application_passwords_endpoint' => admin_url( 'authorize-application.php' ),
);
}

This change will account for sites that install WP in a sub-directory but I am wondering if a second HTTP request (or internal request to the REST index) is preferable to account for custom authentication.

To fix this, for now, I'll work of the data added while removing the legacy plugin.

@jeffpaul jeffpaul moved this from Backlog to In Progress in Open Source Practice Aug 29, 2022
@jeffpaul jeffpaul moved this from In Progress to In Review in Open Source Practice Sep 12, 2022
Repository owner moved this from In Review to Merged in Open Source Practice Sep 13, 2022
@jeffpaul jeffpaul moved this from 🆕 New to ✅ Done in Distributor 2.0.0 Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted type:enhancement New feature or request.
Projects
Archived in project
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants