Merge pull request #92 from jrchamp/fix/wpcs-input-sanitization

Fix/WPCS input sanitization and WCAG accessibility

.github/workflows/wp-plugin-ci-full.yml

@@ -27,4 +27,4 @@ jobs:
 
     - name: Run PHPCS on all files
       run: |
-        vendor/bin/phpcs -q -n --ignore=vendor --runtime-set installed_paths vendor/wp-coding-standards/wpcs --standard=WordPress-Core --report=checkstyle $GITHUB_WORKSPACE | cs2pr
+        vendor/bin/phpcs -q -n --ignore=vendor --runtime-set installed_paths vendor/wp-coding-standards/wpcs --standard=WordPress --report=checkstyle $GITHUB_WORKSPACE | cs2pr

.github/workflows/wp-plugin-ci.yml

@@ -32,4 +32,4 @@ jobs:
       run: |
         touch $GITHUB_WORKSPACE/tmp.php
         export CHANGED_FILES=$(git diff --name-only --diff-filter=AM remotes/origin/${{ github.base_ref }} | tr '\n' ' ')
-        vendor/bin/phpcs -q -n --ignore=vendor --runtime-set installed_paths vendor/wp-coding-standards/wpcs --standard=WordPress-Core --report=checkstyle $GITHUB_WORKSPACE/tmp.php $(echo $CHANGED_FILES) | cs2pr
+        vendor/bin/phpcs -q -n --ignore=vendor --runtime-set installed_paths vendor/wp-coding-standards/wpcs --standard=WordPress --report=checkstyle $GITHUB_WORKSPACE/tmp.php $(echo $CHANGED_FILES) | cs2pr

assets/js/shibboleth_login_form.js

@@ -1,5 +1,9 @@
-// Originally from Automattic's Jetpack SSO module (v5.3)
-// @see https://github.com/Automattic/jetpack/blob/5.3/modules/sso/jetpack-sso-login.js
+/**
+ * Originally from Automattic's Jetpack SSO module (v5.3)
+ *
+ * @see https://github.com/Automattic/jetpack/blob/5.3/modules/sso/jetpack-sso-login.js.
+ * @package shibboleth
+ */
 
 jQuery( document ).ready(
 	function( $ ) {
@@ -15,7 +19,7 @@ jQuery( document ).ready(
 		// UI under the submit button.
 		//
 		// @TODO: Remove this approach once core ticket 28528 is in and we have more actions in wp-login.php.
-		// See - https://core.trac.wordpress.org/ticket/28528
+		// See - https://core.trac.wordpress.org/ticket/28528.
 		loginForm.append( overflow );
 		overflow.append( $( 'p.forgetmenot' ), $( 'p.submit' ) );
 

options-user.php

@@ -158,7 +158,7 @@ function shibboleth_link_accounts_button( $user ) {
 					<?php } elseif ( defined( 'IS_PROFILE_PAGE' ) && ! IS_PROFILE_PAGE ) { ?>
 						<p class="description"><?php esc_html_e( 'This user account has not been linked to Shibboleth.', 'shibboleth' ); ?></p>
 					<?php } else { ?>
-						<a href="?shibboleth=link"><button type="button" class="button"><?php esc_html_e( 'Link Shibboleth Account', 'shibboleth' ); ?></button></a>
+						<a href="<?php echo esc_url( wp_nonce_url( '?shibboleth=link', 'shibboleth-link' ) ); ?>"><button type="button" class="button"><?php esc_html_e( 'Link Shibboleth Account', 'shibboleth' ); ?></button></a>
 						<p class="description"><?php esc_html_e( 'Your account has not been linked to Shibboleth. To link your account, click the button above.', 'shibboleth' ); ?></p>
 					<?php } ?>
 				</td>
@@ -182,16 +182,18 @@ function shibboleth_link_accounts() {
 	if ( is_admin() && 'profile' === $screen->id ) {
 		$user_id = get_current_user_id();
 
-		// If profile page has ?shibboleth=link action and current user can edit their profile, proceed
+		// If profile page has ?shibboleth=link action and current user can edit their profile, proceed.
 		if ( isset( $_GET['shibboleth'] ) && 'link' === $_GET['shibboleth'] && current_user_can( 'edit_user', $user_id ) ) {
+			check_admin_referer( 'shibboleth-link' );
+
 			$shib_logging = shibboleth_getoption( 'shibboleth_logging', false, true );
 			$allowed = shibboleth_getoption( 'shibboleth_manually_combine_accounts', 'disallow' );
 
-			// If user's account is not already linked with shibboleth, proceed
+			// If user's account is not already linked with shibboleth, proceed.
 			if ( ! get_user_meta( $user_id, 'shibboleth_account' ) ) {
-				// If manual account merging is enabled, proceed
+				// If manual account merging is enabled, proceed.
 				if ( 'allow' === $allowed || 'bypass' === $allowed ) {
-					// If there is an existing shibboleth session, proceed
+					// If there is an existing shibboleth session, proceed.
 					if ( shibboleth_session_active() ) {
 						$shib_headers = shibboleth_getoption( 'shibboleth_headers', false, true );
 
@@ -200,53 +202,53 @@ function shibboleth_link_accounts() {
 
 						$user = get_user_by( 'id', $user_id );
 
-						// If username and email match, safe to merge
+						// If username and email match, safe to merge.
 						if ( $user->user_login === $username && strtolower( $user->user_email ) === strtolower( $email ) ) {
 							update_user_meta( $user->ID, 'shibboleth_account', true );
 							if ( in_array( 'account_merge', $shib_logging, true ) || defined( 'WP_DEBUG' ) && WP_DEBUG ) {
 								error_log( '[Shibboleth WordPress Plugin Logging] SUCCESS: User ' . $user->user_login . ' (ID: ' . $user->ID . ') merged accounts manually.' );
 							}
 							wp_safe_redirect( get_edit_user_link() . '?shibboleth=linked' );
 							exit;
-							// If username matches, check if there is a conflict with the email
+							// If username matches, check if there is a conflict with the email.
 						} elseif ( $user->user_login === $username ) {
 								$prevent_conflict = get_user_by( 'email', $email );
-								// If username matches and there is no existing account with the email, safe to merge
+								// If username matches and there is no existing account with the email, safe to merge.
 							if ( ! $prevent_conflict->ID ) {
 								update_user_meta( $user->ID, 'shibboleth_account', true );
 								if ( in_array( 'account_merge', $shib_logging, true ) || defined( 'WP_DEBUG' ) && WP_DEBUG ) {
 									error_log( '[Shibboleth WordPress Plugin Logging] SUCCESS: User ' . $user->user_login . ' (ID: ' . $user->ID . ') merged accounts manually.' );
 								}
 								wp_safe_redirect( get_edit_user_link() . '?shibboleth=linked' );
 								exit;
-								// If username matches and there is an existing account with the email, fail
+								// If username matches and there is an existing account with the email, fail.
 							} else {
 								if ( in_array( 'account_merge', $shib_logging, true ) || defined( 'WP_DEBUG' ) && WP_DEBUG ) {
 									error_log( '[Shibboleth WordPress Plugin Logging] ERROR: User ' . $user->user_login . ' (ID: ' . $user->ID . ') failed to manually merge accounts. Reason: An account already exists with the email: ' . $email . ' .' );
 								}
 								wp_safe_redirect( get_edit_user_link() . '?shibboleth=failed' );
 								exit;
 							}
-							// If email matches and username bypass is enabled, check if there is a conflict with the username
+							// If email matches and username bypass is enabled, check if there is a conflict with the username.
 						} elseif ( strtolower( $user->user_email ) === strtolower( $email ) && 'bypass' === $allowed ) {
 							$prevent_conflict = get_user_by( 'user_login', $username );
-							// If email matches and there is no existing account with the username, safe to merge
+							// If email matches and there is no existing account with the username, safe to merge.
 							if ( ! $prevent_conflict->ID ) {
 								update_user_meta( $user->ID, 'shibboleth_account', true );
 								if ( in_array( 'account_merge', $shib_logging, true ) || defined( 'WP_DEBUG' ) && WP_DEBUG ) {
 										error_log( '[Shibboleth WordPress Plugin Logging] SUCCESS: User ' . $user->user_login . ' (ID: ' . $user->ID . ') merged accounts manually using username bypass. Username provided by attribute is: ' . $username . '.' );
 								}
 								wp_safe_redirect( get_edit_user_link() . '?shibboleth=linked' );
 								exit;
-								// If there is an existing account with the email, fail
+								// If there is an existing account with the email, fail.
 							} else {
 								if ( in_array( 'account_merge', $shib_logging, true ) || defined( 'WP_DEBUG' ) && WP_DEBUG ) {
 									error_log( '[Shibboleth WordPress Plugin Logging] ERROR: User ' . $user->user_login . ' (ID: ' . $user->ID . ') failed to manually merge accounts using username bypass. Reason: An account already exists with the email: ' . $email . ' .' );
 								}
 								wp_safe_redirect( get_edit_user_link() . '?shibboleth=failed' );
 								exit;
 							}
-							// If no other conditions are met, fail
+							// If no other conditions are met, fail.
 						} else {
 							if ( in_array( 'account_merge', $shib_logging, true ) || defined( 'WP_DEBUG' ) && WP_DEBUG ) {
 								error_log( '[Shibboleth WordPress Plugin Logging] ERROR: User ' . $user->user_login . ' (ID: ' . $user->ID . ') failed to manually merge accounts. Reason: Username and email do not match what is provided by attributes. Username provided by attribute is: ' . $username . ' and email provided by attribute is ' . $email . '.' );
@@ -255,21 +257,21 @@ function shibboleth_link_accounts() {
 							exit;
 						}
 						// If there is no existing shibboleth session, kick to the shibboleth_session_initiator_url
-						// and redirect to this page with the ?shibboleth=link action
+						// and redirect to this page with the ?shibboleth=link action.
 					} else {
-						$initiator_url = shibboleth_session_initiator_url( get_edit_user_link() . '?shibboleth=link' );
+						$initiator_url = shibboleth_session_initiator_url( wp_nonce_url( get_edit_user_link() . '?shibboleth=link', 'shibboleth-link' ) );
 						wp_redirect( $initiator_url );
 						exit;
 					}
-					// If manual merging is disabled, fail
+					// If manual merging is disabled, fail.
 				} else {
 					if ( in_array( 'account_merge', $shib_logging, true ) || defined( 'WP_DEBUG' ) && WP_DEBUG ) {
 						error_log( '[Shibboleth WordPress Plugin Logging] ERROR: User ' . $user->user_login . ' (ID: ' . $user->ID . ') failed to manually merge accounts. Reason: Manual account merging is disabled.' );
 					}
 					wp_safe_redirect( get_edit_user_link() . '?shibboleth=failed' );
 					exit;
 				}
-				// If account is already merged, warn
+				// If account is already merged, warn.
 			} else {
 				if ( in_array( 'account_merge', $shib_logging, true ) || defined( 'WP_DEBUG' ) && WP_DEBUG ) {
 					error_log( '[Shibboleth WordPress Plugin Logging] WARN: User ' . $user->user_login . ' (ID: ' . $user->ID . ') failed to manually merge accounts. Reason: User\'s account is already merged.' );

readme.txt

@@ -2,9 +2,9 @@
 Contributors: michaelryanmcneill, willnorris, mitchoyoshitaka, jrchamp, dericcrago, bshelton229, Alhrath, dandalpiaz
 Tags: shibboleth, authentication, login, saml
 Requires at least: 4.0
-Tested up to: 6.1
+Tested up to: 6.2
 Requires PHP: 5.6
-Stable tag: 2.4.1
+Stable tag: 2.4.2
 
 Allows WordPress to externalize user authentication and account creation to a Shibboleth Service Provider.
 
@@ -197,6 +197,12 @@ This update brings with it a major change to the way Shibboleth attributes are a
 This update brings with it a major change to the way Shibboleth attributes are accessed. For most users, no additional configuration will be necessary. If you are using a specialized server configuration, such as a Shibboleth Service Provider on a reverse proxy or a server configuration that results in environment variables being sent with the prefix REDIRECT_, you should see the changelog for additional details: https://wordpress.org/plugins/shibboleth/#developers
 
 == Changelog ==
+= version 2.4.2 (2023-04-07) =
+ - Documentation: Updated Shibboleth documentation external links [#92](https://github.com/michaelryanmcneill/shibboleth/pull/92)
+ - Accessibility: Improve labels and heading structure on admin pages [#92](https://github.com/michaelryanmcneill/shibboleth/pull/92)
+ - Security: Improve input sanitization and use of nonces [#92](https://github.com/michaelryanmcneill/shibboleth/pull/92)
+ - CI: Switch GitHub Action workflows to check against WordPress coding standard [#92](https://github.com/michaelryanmcneill/shibboleth/pull/92)
+
 = version 2.4.1 (2023-03-20) =
  - Compatibility: Fix redirect_to issues on WordPress 6; thanks @masteradhoc, @caosborne89, @jakeparis [#88](https://github.com/michaelryanmcneill/shibboleth/pull/88)
  - Accessibility: Improve color contrast on login page [#89](https://github.com/michaelryanmcneill/shibboleth/pull/89)