WordPress
success_url should be forced to be https
We should only redirect back to $success_url if it is HTTPS. Otherwise you're exposing the application password over plain HTTP. We might want to add an override for that for testing purposes but then we should output a clear warning on the screen that the success URL is not secure.
That’s a great point @jdevalk! Do you suggest we always override the return URL to be HTTPS and have a filter in place to disable the override?
I'd initially thought that perhaps instead of accepting a http url, we could just display the credentials to the user, so that they can enter it into the app manually -- however that seems kinda silly as most apps using this flow wouldn't have a place for the user to enter the creds if they were expecting to get them back via a redirect.
Also, if we force it to reject http:// urls, they would never get it back in the first place, so they should have caught that before they ever launched the integration in the first place. So it's kinda silly to bend over backwards to support a pattern that we can easily rule out before we launch.
It should be a simple conditional added somewhere around here:
https://github.com/WordPress/application-passwords/blob/6a1e9efec91d88ca68091dc87f488948d3a99498/class.application-passwords.php#L441-L455
-- just worth noting that if we did, we would need to do it in a way that only rejects urls matching with /^http:/i-- rather than forcing /https:/i -- as we're explicitly allowing alternate protocols for passing data back to apps:
https://developer.android.com/training/app-links https://developer.apple.com/documentation/xcode/allowing_apps_and_websites_to_link_to_your_content/defining_a_custom_url_scheme_for_your_app
How about introducing a wp_is_allowed_application_password_redirect_url() function that would accept a URL and return a WP_Error? By default, we'd reject http, but the function would be filterable.
@georgestephanis Are you suggesting we add a warning/notice to that particular admin section if the redirect URL starts with http://? Or do we also disable the "Yes, I approve of this connection." button unless the newly added filter overrides it?
A basic wp_die form of this was implemented in https://github.com/WordPress/wordpress-develop/pull/540/commits/bd57332c6b11368b47fc12b1a8b12eb290dfe42f.