diff --git a/src/js/_enqueues/admin/auth-app.js b/src/js/_enqueues/admin/auth-app.js index 99478d1824a2d..5885ede6f7217 100644 --- a/src/js/_enqueues/admin/auth-app.js +++ b/src/js/_enqueues/admin/auth-app.js @@ -2,19 +2,50 @@ * @output wp-admin/js/auth-app.js */ -/* global authApp */ +/* global authApp, ClipboardJS */ ( function( $, authApp ) { var $appNameField = $( '#app_name' ), $approveBtn = $( '#approve' ), $rejectBtn = $( '#reject' ), $form = $appNameField.closest( 'form' ), + clipboard = new ClipboardJS( '.application-password-display .copy-button' ), context = { userLogin: authApp.user_login, - successUrl: authApp.success, - rejectUrl: authApp.reject + successUrl: authApp.success }; + // Bound at module scope so it covers both the no-JS server-rendered button and the one inserted after the AJAX request. + clipboard.on( 'success', function( e ) { + var $successElement = $( '.success', $( e.trigger ).parent() ); + + e.clearSelection(); + $successElement.removeClass( 'hidden' ); + + setTimeout( function() { + $successElement.addClass( 'hidden' ); + }, 3000 ); + + wp.a11y.speak( wp.i18n.__( 'Application password has been copied to your clipboard.' ) ); + } ); + + // If redirecting to an external site, gate the approve button behind the confirmation checkbox. + if ( authApp.successHost ) { + var $checkbox = $( 'input[name="confirm_external_redirect"]' ); + + // Start the approve button in a disabled state. + $approveBtn.prop( 'aria-disabled', true ).addClass( 'disabled' ); + + // Toggle the approve button when the checkbox state changes. + $checkbox.on( 'change', function() { + if ( $checkbox.prop( 'checked' ) ) { + $approveBtn.removeProp( 'aria-disabled' ).removeClass( 'disabled' ); + } else { + $approveBtn.prop( 'aria-disabled', true ).addClass( 'disabled' ); + } + } ); + } + $approveBtn.on( 'click', function( e ) { var name = $appNameField.val(), appId = $( 'input[name="app_id"]', $form ).val(); @@ -25,8 +56,8 @@ return; } - if ( 0 === name.length ) { - $appNameField.trigger( 'focus' ); + if ( ! $form[ 0 ].checkValidity() ) { + $form[ 0 ].reportValidity(); return; } @@ -44,12 +75,12 @@ * Filters the request data used to Authorize an Application Password request. * * @since 5.6.0 + * @since x.y.z A reject URL is no longer supported or used. * * @param {Object} request The request data. * @param {Object} context Context about the Application Password request. * @param {string} context.userLogin The user's login username. * @param {string} context.successUrl The URL the user will be redirected to after approving the request. - * @param {string} context.rejectUrl The URL the user will be redirected to after rejecting the request. */ request = wp.hooks.applyFilters( 'wp_application_passwords_approve_app_request', request, context ); @@ -89,17 +120,25 @@ /* translators: %s: Application name. */ '', '' - ) + ' '; + ); $notice = $( '
' ) .attr( 'role', 'alert' ) .attr( 'tabindex', -1 ) .addClass( 'notice notice-success notice-alt' ) .append( $( '

' ).addClass( 'application-password-display' ).html( message ) ) + .append( + $( '

' ) + .addClass( 'application-password-display' ) + .append( '' ) + .append( ' ' ) + .append( '' ) + ) .append( '

' + wp.i18n.__( 'Be sure to save this in a safe location. You will not be able to retrieve it.' ) + '

' ); // We're using .text() to write the variables to avoid any chance of XSS. $( 'strong', $notice ).text( response.name ); $( 'input', $notice ).val( response.password ); + $( '.copy-button', $notice ).attr( 'data-clipboard-text', response.password ); $form.replaceWith( $notice ); $notice.trigger( 'focus' ); @@ -147,16 +186,22 @@ * Fires when an Authorize Application Password request has been rejected by the user. * * @since 5.6.0 + * @since x.y.z A reject URL is no longer supported or used. * * @param {Object} context Context about the Application Password request. * @param {string} context.userLogin The user's login username. * @param {string} context.successUrl The URL the user will be redirected to after approving the request. - * @param {string} context.rejectUrl The URL the user will be redirected to after rejecting the request. */ wp.hooks.doAction( 'wp_application_passwords_reject_app', context ); - // @todo: Make a better way to do this so it feels like less of a semi-open redirect. - window.location = authApp.reject; + var $notice = $( '
' ) + .attr( 'role', 'alert' ) + .attr( 'tabindex', -1 ) + .addClass( 'notice notice-info' ) + .append( $( '

' ).text( wp.i18n.__( 'You have not approved this connection. No data has been shared with the application.' ) ) ); + + $form.replaceWith( $notice ); + $notice.trigger( 'focus' ); } ); $form.on( 'submit', function( e ) { diff --git a/src/wp-admin/authorize-application.php b/src/wp-admin/authorize-application.php index 8d931f46666a2..41d7d539e171d 100644 --- a/src/wp-admin/authorize-application.php +++ b/src/wp-admin/authorize-application.php @@ -17,17 +17,12 @@ check_admin_referer( 'authorize_application_password' ); $success_url = $_POST['success_url']; - $reject_url = $_POST['reject_url']; $app_name = $_POST['app_name']; $app_id = $_POST['app_id']; $redirect = ''; if ( isset( $_POST['reject'] ) ) { - if ( $reject_url ) { - $redirect = $reject_url; - } else { - $redirect = admin_url(); - } + $redirect = admin_url(); } elseif ( isset( $_POST['approve'] ) ) { $created = WP_Application_Passwords::create_new_application_password( get_current_user_id(), @@ -56,7 +51,7 @@ } if ( $redirect ) { - // Explicitly not using wp_safe_redirect b/c sends to arbitrary domain. + // Explicitly not using wp_safe_redirect b/c sends to arbitrary domain or custom scheme URL. wp_redirect( $redirect ); exit; } @@ -67,19 +62,11 @@ $app_name = ! empty( $_REQUEST['app_name'] ) ? $_REQUEST['app_name'] : ''; $app_id = ! empty( $_REQUEST['app_id'] ) ? $_REQUEST['app_id'] : ''; -$success_url = ! empty( $_REQUEST['success_url'] ) ? $_REQUEST['success_url'] : null; - -if ( ! empty( $_REQUEST['reject_url'] ) ) { - $reject_url = $_REQUEST['reject_url']; -} elseif ( $success_url ) { - $reject_url = add_query_arg( 'success', 'false', $success_url ); -} else { - $reject_url = null; -} +$success_url = ! empty( $_REQUEST['success_url'] ) ? $_REQUEST['success_url'] : ''; $user = wp_get_current_user(); -$request = compact( 'app_name', 'app_id', 'success_url', 'reject_url' ); +$request = compact( 'app_name', 'app_id', 'success_url' ); $is_valid = wp_is_authorize_application_password_request_valid( $request, $user ); if ( is_wp_error( $is_valid ) ) { @@ -96,7 +83,7 @@ array( 'response' => 501, 'link_text' => __( 'Go Back' ), - 'link_url' => $reject_url ? add_query_arg( 'error', 'disabled', $reject_url ) : admin_url(), + 'link_url' => admin_url(), ) ); } @@ -114,20 +101,26 @@ array( 'response' => 501, 'link_text' => __( 'Go Back' ), - 'link_url' => $reject_url ? add_query_arg( 'error', 'disabled', $reject_url ) : admin_url(), + 'link_url' => admin_url(), ) ); } wp_enqueue_script( 'auth-app' ); + +// Determine how to display the success URL target to the user. +$success_scheme = $success_url ? strtolower( (string) wp_parse_url( $success_url, PHP_URL_SCHEME ) ) : ''; +$is_custom_scheme = $success_scheme && ! in_array( $success_scheme, array( 'http', 'https' ), true ); +$success_host_display = wp_get_authorize_application_redirect_url_display( $success_url ); + wp_localize_script( 'auth-app', 'authApp', array( - 'site_url' => site_url(), - 'user_login' => $user->user_login, - 'success' => $success_url, - 'reject' => $reject_url ? $reject_url : admin_url(), + 'site_url' => site_url(), + 'user_login' => $user->user_login, + 'success' => $success_url, + 'successHost' => $success_host_display, ) ); @@ -150,56 +143,6 @@

- -

- ' . esc_html( $app_name ) . '' - ); - ?> -

- -

- - - ID, true ); - $blogs_count = count( $blogs ); - - if ( $blogs_count > 1 ) { - ?> -

- the %2$s site in this installation that you have permissions on.', - 'This will grant access to all %2$s sites in this installation that you have permissions on.', - $blogs_count - ); - - if ( is_super_admin() ) { - /* translators: 1: URL to my-sites.php, 2: Number of sites the user has. */ - $message = _n( - 'This will grant access to the %2$s site on the network as you have Super Admin rights.', - 'This will grant access to all %2$s sites on the network as you have Super Admin rights.', - $blogs_count - ); - } - - printf( - $message, - admin_url( 'my-sites.php' ), - number_format_i18n( $blogs_count ) - ); - ?> -

- ' . esc_html( $app_name ) . '' ) . ' +

+

+ +

' . __( 'Be sure to save this in a safe location. You will not be able to retrieve it.' ) . '

'; $args = array( @@ -236,18 +183,103 @@ do_action( 'wp_authorize_application_password_form_approved_no_js', $new_password, $request, $user ); else : ?> -
+ - + + +

+ ' . esc_html( $app_name ) . '' + ); + ?> +

+ +

+ + + ID, true ); + $blogs_count = count( $blogs ); + + if ( $blogs_count > 1 ) { + ?> +

+ the %2$s site in this installation that you have permissions on.', + 'This will grant access to all %2$s sites in this installation that you have permissions on.', + $blogs_count + ); + + if ( is_super_admin() ) { + /* translators: 1: URL to my-sites.php, 2: Number of sites the user has. */ + $message = _n( + 'This will grant access to the %2$s site on the network as you have Super Admin rights.', + 'This will grant access to all %2$s sites on the network as you have Super Admin rights.', + $blogs_count + ); + } + + printf( + $message, + admin_url( 'my-sites.php' ), + number_format_i18n( $blogs_count ) + ); + ?> +

+ + + +
+

+ +

+

+

+
+
+ +

+ + +

+ +
+

+
+ + 'description-approve', - ) + false ); ?> -

- ' . esc_html( - add_query_arg( - array( - 'site_url' => site_url(), - 'user_login' => $user->user_login, - 'password' => '[------]', - ), - $success_url - ) - ) . '' - ); - } else { - _e( 'You will be given a password to manually enter into the application in question.' ); - } - ?> -

'description-reject', + // Allow rejection when JavaScript is disabled. + 'formnovalidate' => 'formnovalidate', ) ); ?> -

- ' . esc_html( $reject_url ) . '' - ); - } else { - _e( 'You will be returned to the WordPress Dashboard, and no changes will be made.' ); - } - ?> -

diff --git a/src/wp-admin/css/forms.css b/src/wp-admin/css/forms.css index 2ace16f733004..b11f503c6fe62 100644 --- a/src/wp-admin/css/forms.css +++ b/src/wp-admin/css/forms.css @@ -1068,6 +1068,32 @@ table.form-table td .updated p { max-width: 768px; } +.auth-app-warning { + margin: 16px 0; + padding: 14px 16px; + border-left: 4px solid #dba617; + background: #fcf9e8; +} + +.auth-app-warning p { + margin: 0; +} + +.auth-app-warning .auth-app-host { + font-size: 18px; + font-family: monospace; + font-weight: bold; + color: #3c2b00; +} + +.auth-app-warning .auth-app-caution { + margin-top: 8px; +} + +.auth-app-card .auth-app-checkbox { + margin-top: 16px; +} + .authorize-application-php .form-wrap p { display: block; } diff --git a/src/wp-admin/includes/user.php b/src/wp-admin/includes/user.php index c6c08c4eaf76c..401c2d9f007c3 100644 --- a/src/wp-admin/includes/user.php +++ b/src/wp-admin/includes/user.php @@ -624,7 +624,8 @@ function admin_created_user_email( $text ) { * * @since 5.6.0 * @since 6.2.0 Allow insecure HTTP connections for the local environment. - * @since 6.3.2 Validates the success and reject URLs to prevent `javascript` pseudo protocol from being executed. + * @since 6.3.2 Validates the success URL to prevent `javascript` pseudo protocol from being executed. + * @since x.y.z A reject URL is no longer supported or used. * * @param array $request { * The array of request data. All arguments are optional and may be empty. @@ -632,7 +633,6 @@ function admin_created_user_email( $text ) { * @type string $app_name The suggested name of the application. * @type string $app_id A UUID provided by the application to uniquely identify it. * @type string $success_url The URL the user will be redirected to after approving the application. - * @type string $reject_url The URL the user will be redirected to after rejecting the application. * } * @param WP_User $user The user authorizing the application. * @return true|WP_Error True if the request is valid, a WP_Error object contains errors if not. @@ -650,16 +650,6 @@ function wp_is_authorize_application_password_request_valid( $request, $user ) { } } - if ( isset( $request['reject_url'] ) ) { - $validated_reject_url = wp_is_authorize_application_redirect_url_valid( $request['reject_url'] ); - if ( is_wp_error( $validated_reject_url ) ) { - $error->add( - $validated_reject_url->get_error_code(), - $validated_reject_url->get_error_message() - ); - } - } - if ( ! empty( $request['app_id'] ) && ! wp_is_uuid( $request['app_id'] ) ) { $error->add( 'invalid_app_id', @@ -734,6 +724,13 @@ function wp_is_authorize_application_redirect_url_valid( $url ) { ); } + if ( null !== wp_parse_url( $url, PHP_URL_USER ) ) { + return new WP_Error( + 'invalid_redirect_url_format', + __( 'Credentials are not allowed in the URL.' ) + ); + } + // Allow insecure HTTP connections to locally hosted applications. $is_loopback = in_array( strtolower( $host ), @@ -750,3 +747,33 @@ function wp_is_authorize_application_redirect_url_valid( $url ) { return true; } + +/** + * Returns a human-readable representation of the destination of an application password redirect URL. + * + * For `http` and `https` URLs this is the host name. For any other scheme the scheme is + * included so the user can see that they will be sent to an app rather than a website. + * + * @since x.y.z + * + * @param string $url The redirect URL. + * @return string The destination to display, or an empty string if the URL has no host. + */ +function wp_get_authorize_application_redirect_url_display( string $url ): string { + if ( empty( $url ) ) { + return ''; + } + + $scheme = strtolower( (string) wp_parse_url( $url, PHP_URL_SCHEME ) ); + $host = strtolower( (string) wp_parse_url( $url, PHP_URL_HOST ) ); + + if ( '' === $host ) { + return ''; + } + + if ( in_array( $scheme, array( 'http', 'https' ), true ) ) { + return $host; + } + + return $scheme . '://' . $host; +} diff --git a/src/wp-includes/script-loader.php b/src/wp-includes/script-loader.php index 2d839af4a1a82..c6d0f4afd3bea 100644 --- a/src/wp-includes/script-loader.php +++ b/src/wp-includes/script-loader.php @@ -1245,7 +1245,7 @@ function wp_default_scripts( $scripts ) { $scripts->add( 'application-passwords', "/wp-admin/js/application-passwords$suffix.js", array( 'jquery', 'wp-util', 'wp-api-request', 'wp-date', 'wp-i18n', 'wp-hooks' ), false, 1 ); $scripts->set_translations( 'application-passwords' ); - $scripts->add( 'auth-app', "/wp-admin/js/auth-app$suffix.js", array( 'jquery', 'wp-api-request', 'wp-i18n', 'wp-hooks' ), false, 1 ); + $scripts->add( 'auth-app', "/wp-admin/js/auth-app$suffix.js", array( 'clipboard', 'jquery', 'wp-api-request', 'wp-i18n', 'wp-a11y', 'wp-hooks' ), false, 1 ); $scripts->set_translations( 'auth-app' ); $scripts->add( 'user-profile', "/wp-admin/js/user-profile$suffix.js", array( 'clipboard', 'jquery', 'password-strength-meter', 'wp-util', 'wp-a11y' ), false, 1 ); diff --git a/tests/phpunit/tests/admin/includes/user/WpGetAuthorizeApplicationRedirectUrlDisplay_Test.php b/tests/phpunit/tests/admin/includes/user/WpGetAuthorizeApplicationRedirectUrlDisplay_Test.php new file mode 100644 index 0000000000000..e01b98d0cda8a --- /dev/null +++ b/tests/phpunit/tests/admin/includes/user/WpGetAuthorizeApplicationRedirectUrlDisplay_Test.php @@ -0,0 +1,78 @@ +assertSame( $expected, $actual ); + } + + /** + * Data provider for test_wp_get_authorize_application_redirect_url_display. + * + * @return list + */ + public function data_wp_get_authorize_application_redirect_url_display() { + return array( + 'an "https" URL' => array( + 'url' => 'https://example.org/callback?foo=bar#baz', + 'expected' => 'example.org', + ), + 'an "http" URL' => array( + 'url' => 'http://example.org/callback', + 'expected' => 'example.org', + ), + 'a URL with a port' => array( + 'url' => 'https://example.org:8443/callback', + 'expected' => 'example.org', + ), + 'a subdomain' => array( + 'url' => 'https://app.example.org/callback', + 'expected' => 'app.example.org', + ), + 'a loopback IPv4 address' => array( + 'url' => 'http://127.0.0.1:8080/callback', + 'expected' => '127.0.0.1', + ), + 'a loopback IPv6 address' => array( + 'url' => 'http://[::1]:8080/callback', + 'expected' => '[::1]', + ), + 'a mixed case scheme and host' => array( + 'url' => 'HTTPS://Example.ORG/Callback', + 'expected' => 'example.org', + ), + 'a custom scheme' => array( + 'url' => 'wordpress://example/callback', + 'expected' => 'wordpress://example', + ), + 'a custom scheme with a dot host' => array( + 'url' => 'com.example.app://auth/callback', + 'expected' => 'com.example.app://auth', + ), + 'a mixed case custom scheme' => array( + 'url' => 'WordPress://Example/callback', + 'expected' => 'wordpress://example', + ), + ); + } +} diff --git a/tests/phpunit/tests/admin/includes/user/WpIsAuthorizeApplicationPasswordRequestValid_Test.php b/tests/phpunit/tests/admin/includes/user/WpIsAuthorizeApplicationPasswordRequestValid_Test.php index a2e1320b2a150..ae88a4b719233 100644 --- a/tests/phpunit/tests/admin/includes/user/WpIsAuthorizeApplicationPasswordRequestValid_Test.php +++ b/tests/phpunit/tests/admin/includes/user/WpIsAuthorizeApplicationPasswordRequestValid_Test.php @@ -52,44 +52,38 @@ public function data_is_authorize_application_password_request_valid() { 'env' => $environment_type, ); - $datasets[ $environment_type . ' and a "https" scheme "reject_url"' ] = array( - 'request' => array( 'reject_url' => 'https://example.org' ), - 'expected_error_code' => '', - 'env' => $environment_type, - ); - $datasets[ $environment_type . ' and an app scheme "success_url"' ] = array( 'request' => array( 'success_url' => 'wordpress://example' ), 'expected_error_code' => '', 'env' => $environment_type, ); - $datasets[ $environment_type . ' and an app scheme "reject_url"' ] = array( - 'request' => array( 'reject_url' => 'wordpress://example' ), - 'expected_error_code' => '', - 'env' => $environment_type, - ); - $datasets[ $environment_type . ' and a "http" scheme "success_url"' ] = array( 'request' => array( 'success_url' => 'http://example.org' ), 'expected_error_code' => 'local' === $environment_type ? '' : 'invalid_redirect_scheme', 'env' => $environment_type, ); - $datasets[ $environment_type . ' and a "http" scheme "reject_url"' ] = array( - 'request' => array( 'reject_url' => 'http://example.org' ), - 'expected_error_code' => 'local' === $environment_type ? '' : 'invalid_redirect_scheme', + $datasets[ $environment_type . ' and a backslash before the "@" in the "success_url"' ] = array( + 'request' => array( 'success_url' => 'http://collector.example.test\\@127.0.0.1/callback' ), + 'expected_error_code' => 'invalid_redirect_url_format', 'env' => $environment_type, ); - $datasets[ $environment_type . ' and a "http" loopback "success_url"' ] = array( - 'request' => array( 'success_url' => 'http://127.0.0.1:8080/callback' ), - 'expected_error_code' => '', + $datasets[ $environment_type . ' and a userinfo "success_url"' ] = array( + 'request' => array( 'success_url' => 'https://user:pass@evil.com/capture' ), + 'expected_error_code' => 'invalid_redirect_url_format', + 'env' => $environment_type, + ); + + $datasets[ $environment_type . ' and a userinfo-only "success_url"' ] = array( + 'request' => array( 'success_url' => 'https://google.com@evil.com/capture' ), + 'expected_error_code' => 'invalid_redirect_url_format', 'env' => $environment_type, ); - $datasets[ $environment_type . ' and a "http" loopback "reject_url"' ] = array( - 'request' => array( 'reject_url' => 'http://127.0.0.1/callback' ), + $datasets[ $environment_type . ' and a "http" loopback "success_url"' ] = array( + 'request' => array( 'success_url' => 'http://127.0.0.1:8080/callback' ), 'expected_error_code' => '', 'env' => $environment_type, ); diff --git a/tests/phpunit/tests/admin/includes/user/WpIsAuthorizeApplicationRedirectUrlValid_Test.php b/tests/phpunit/tests/admin/includes/user/WpIsAuthorizeApplicationRedirectUrlValid_Test.php index 3ba7b9045ac28..37b344411ea20 100644 --- a/tests/phpunit/tests/admin/includes/user/WpIsAuthorizeApplicationRedirectUrlValid_Test.php +++ b/tests/phpunit/tests/admin/includes/user/WpIsAuthorizeApplicationRedirectUrlValid_Test.php @@ -103,6 +103,38 @@ public function data_wp_is_authorize_application_redirect_url_valid() { 'env' => $environment_type, ); + // Credentials in the URL should always be rejected (authority confusion attack). + $datasets[ $environment_type . ' and a URL with username and password' ] = array( + 'url' => 'https://user:pass@evil.com/capture', + 'expected_error_code' => 'invalid_redirect_url_format', + 'env' => $environment_type, + ); + + $datasets[ $environment_type . ' and a URL with username only' ] = array( + 'url' => 'https://google.com@evil.com/capture', + 'expected_error_code' => 'invalid_redirect_url_format', + 'env' => $environment_type, + ); + + $datasets[ $environment_type . ' and a URL with username and empty password' ] = array( + 'url' => 'https://user:@evil.com/capture', + 'expected_error_code' => 'invalid_redirect_url_format', + 'env' => $environment_type, + ); + + // PHP treats everything before the "@" as userinfo, browsers treat the backslash as the end of the host. + $datasets[ $environment_type . ' and a URL with a backslash before the "@"' ] = array( + 'url' => 'http://collector.example.test\\@127.0.0.1/callback', + 'expected_error_code' => 'invalid_redirect_url_format', + 'env' => $environment_type, + ); + + $datasets[ $environment_type . ' and a "https" URL with a backslash before the "@"' ] = array( + 'url' => 'https://collector.example.test\\@example.org/callback', + 'expected_error_code' => 'invalid_redirect_url_format', + 'env' => $environment_type, + ); + // HTTP + loopback IP addresses should be valid in all environments. $datasets[ $environment_type . ' and a "http" scheme URL with 127.0.0.1' ] = array( 'url' => 'http://127.0.0.1/callback',