Merge pull request #158 from rtCamp/fix/php-depreciation-error

Resolve compatibility issue with PHP 8.1
rtCamp · Sep 1, 2023 · 13b48a8 · 13b48a8
2 parents 0c4ee9c + eaf0d9c
commit 13b48a8
Show file tree

Hide file tree

Showing 7 changed files with 122 additions and 53 deletions.
diff --git a/.github/workflows/phpcs_on_pull_request.yml b/.github/workflows/phpcs_on_pull_request.yml
@@ -5,11 +5,21 @@ jobs:
     name: Run PHPCS inspection
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
       with:
         ref: ${{ github.event.pull_request.head.sha }}
-    - name: Run PHPCS inspection
+
+    - name: Run PHPCS inspection with PHP 8.1
       uses: rtCamp/action-phpcs-code-review@master
       env:
         SKIP_FOLDERS: "tests,.github"
         GH_BOT_TOKEN: ${{ secrets.RTBOT_TOKEN }}
+
+    # Remove this step once WordPress Coding Standards supports PHP 8.0+.
+    - name: Run PHPCS inspection with PHP 7.4
+      uses: rtCamp/action-phpcs-code-review@master
+      env:
+        SKIP_FOLDERS: "tests,.github"
+        PHPCS_PHP_VERSION: "7.4"
+        PHPCS_STANDARD_FILE_NAME: "phpcs.wpcs.xml"
+        GH_BOT_TOKEN: ${{ secrets.RTBOT_TOKEN }}
diff --git a/phpcs.wpcs.xml b/phpcs.wpcs.xml
@@ -0,0 +1,55 @@
+<?xml version="1.0" ?>
+<!-- Remove this file once WordPress Coding Standards is updated to support PHP 8.0. -->
+<ruleset name="Login with github">
+	<config name="minimum_supported_wp_version" value="5.4.2" />
+
+	<file>src</file>
+	<file>login-with-google.php</file>
+
+	<rule ref="WordPress-Core">
+		<exclude name="WordPress.PHP.DisallowShortTernary" />
+		<exclude name="Generic.Arrays.DisallowShortArraySyntax" />
+	</rule>
+
+	<rule ref="WordPress-Docs">
+		<exclude-pattern>tests/*</exclude-pattern>
+	</rule>
+
+	<!-- For PSR-4 autoloading. -->
+	<rule ref="WordPress-Extra">
+		<exclude name="WordPress.Files.FileName" />
+		<exclude name="WordPress.Files.FileName.NotHyphenatedLowercase" />
+	</rule>
+
+	<rule ref="WordPress.WP.I18n">
+		<properties>
+			<property name="text_domain" type="array">
+				<element value="login-with-google" />
+			</property>
+		</properties>
+	</rule>
+
+	<!-- Allow namespaced hook names in dot notation. -->
+	<rule ref="WordPress.NamingConventions.ValidHookName">
+		<properties>
+			<property name="additionalWordDelimiters" value="."/>
+		</properties>
+	</rule>
+
+	<arg value="s"/>
+	<arg name="extensions" value="php"/>
+	<file>.</file>
+
+	<!-- Strip the filepaths down to the relevant bit. -->
+	<arg name="basepath" value="./"/>
+
+	<!-- Check up to 20 files simultaneously. -->
+	<arg name="parallel" value="20"/>
+
+	<exclude-pattern>*/vendor/*</exclude-pattern>
+	<exclude-pattern>*/node_modules/*</exclude-pattern>
+	<exclude-pattern>/lib/*</exclude-pattern>
+	<exclude-pattern>*/tests/*</exclude-pattern>
+	<exclude-pattern>*/.github/*</exclude-pattern>
+	<exclude-pattern>*/.scripts/*</exclude-pattern>
+</ruleset>
diff --git a/phpcs.xml b/phpcs.xml
@@ -1,60 +1,64 @@
 <?xml version="1.0" ?>
 <ruleset name="Login with github">
-    <config name="minimum_supported_wp_version" value="5.4.2" />
+	<config name="minimum_supported_wp_version" value="5.4.2" />
 	<!-- Check for PHP cross-version compatibility. -->
 	<config name="testVersion" value="7.1-"/>
 	<rule ref="PHPCompatibilityWP"/>
 
-    <file>src</file>
-    <file>login-with-google.php</file>
+	<file>src</file>
+	<file>login-with-google.php</file>
 
-    <!-- Show progress and sniff codes in all reports. -->
-    <arg value="ps"/>
-
-    <!-- A path to strip from the front of file paths inside reports. -->
-    <arg name="colors"/>
-    <arg name="extensions" value="php"/>
-
-    <rule ref="WordPress-Core">
+    <!--Uncomment WordPress rules once WordPress Coding Standards is updated to support PHP 8.0. -->
+    <!-- <rule ref="WordPress-Core">
         <exclude name="WordPress.PHP.DisallowShortTernary" />
         <exclude name="Generic.Arrays.DisallowShortArraySyntax" />
     </rule>
 
     <rule ref="WordPress-Docs">
         <exclude-pattern>tests/*</exclude-pattern>
-    </rule>
+    </rule> -->
 
     <!-- For PSR-4 autoloading. -->
-	<rule ref="WordPress-Extra">
+	<!-- <rule ref="WordPress-Extra">
 		<exclude name="WordPress.Files.FileName" />
 		<exclude name="WordPress.Files.FileName.NotHyphenatedLowercase" />
-	</rule>
-
-    <rule ref="WordPress-VIP-Go">
-        <exclude-pattern>tests/*</exclude-pattern>
-    </rule>
+	</rule> -->
 
-    <rule ref="WordPress.WP.I18n">
+    <!-- <rule ref="WordPress.WP.I18n">
         <properties>
             <property name="text_domain" type="array">
                 <element value="login-with-google" />
             </property>
         </properties>
-    </rule>
+    </rule> -->
 
 	<!-- Allow namespaced hook names in dot notation. -->
-	<rule ref="WordPress.NamingConventions.ValidHookName">
+	<!-- <rule ref="WordPress.NamingConventions.ValidHookName">
 		<properties>
 			<property name="additionalWordDelimiters" value="."/>
 		</properties>
-	</rule>
+	</rule> -->
+
+    <rule ref="WordPress-VIP-Go">
+        <exclude-pattern>tests/*</exclude-pattern>
+    </rule>
+
+	<rule ref="VariableAnalysis.CodeAnalysis.VariableAnalysis" />
+
+	<arg value="s"/>
+	<arg name="extensions" value="php"/>
+	<file>.</file>
+
+	<!-- Strip the filepaths down to the relevant bit. -->
+	<arg name="basepath" value="./"/>
 
-    <rule ref="VariableAnalysis.CodeAnalysis.VariableAnalysis" />
+	<!-- Check up to 20 files simultaneously. -->
+	<arg name="parallel" value="20"/>
 
-    <exclude-pattern>*/vendor/*</exclude-pattern>
-    <exclude-pattern>*/node_modules/*</exclude-pattern>
-    <exclude-pattern>/lib/*</exclude-pattern>
-    <exclude-pattern>*/tests/*</exclude-pattern>
-    <exclude-pattern>*/.github/*</exclude-pattern>
-    <exclude-pattern>*/.scripts/*</exclude-pattern>
+	<exclude-pattern>*/vendor/*</exclude-pattern>
+	<exclude-pattern>*/node_modules/*</exclude-pattern>
+	<exclude-pattern>/lib/*</exclude-pattern>
+	<exclude-pattern>*/tests/*</exclude-pattern>
+	<exclude-pattern>*/.github/*</exclude-pattern>
+	<exclude-pattern>*/.scripts/*</exclude-pattern>
 </ruleset>
diff --git a/src/Modules/Login.php b/src/Modules/Login.php
@@ -119,14 +119,14 @@ public function authenticate( $user = null ) {
 			return $user;
 		}
 
-		$code = Helper::filter_input( INPUT_GET, 'code', FILTER_SANITIZE_STRING );
+		$code = Helper::filter_input( INPUT_GET, 'code', FILTER_SANITIZE_FULL_SPECIAL_CHARS );
 
 		if ( ! $code ) {
 			return $user;
 		}
 
-		$state         = Helper::filter_input( INPUT_GET, 'state', FILTER_SANITIZE_STRING );
-		$decoded_state = $state ? (array) ( json_decode( base64_decode( $state ) ) ) : null;
+		$state         = Helper::filter_input( INPUT_GET, 'state', FILTER_SANITIZE_FULL_SPECIAL_CHARS );
+		$decoded_state = $state ? (array) ( json_decode( base64_decode( $state ) ) ) : null;    // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_decode
 
 		if ( ! is_array( $decoded_state ) || empty( $decoded_state['provider'] ) || 'google' !== $decoded_state['provider'] ) {
 			return $user;
@@ -198,7 +198,7 @@ public function redirect_url( string $url ): string {
 	 * @return array
 	 */
 	public function state_redirect( array $state ): array {
-		$redirect_to = Helper::filter_input( INPUT_GET, 'redirect_to', FILTER_SANITIZE_STRING );
+		$redirect_to = Helper::filter_input( INPUT_GET, 'redirect_to', FILTER_SANITIZE_FULL_SPECIAL_CHARS );
 		/**
 		 * Filter the default redirect URL in case redirect_to param is not available.
 		 * Default to admin URL.
@@ -216,7 +216,7 @@ public function state_redirect( array $state ): array {
 	 * @return void
 	 */
 	public function login_redirect(): void {
-		$state = Helper::filter_input( INPUT_GET, 'state', FILTER_SANITIZE_STRING );
+		$state = Helper::filter_input( INPUT_GET, 'state', FILTER_SANITIZE_FULL_SPECIAL_CHARS );
 
 		if ( ! $state || ! $this->authenticated ) {
 			return;

diff --git a/src/Modules/OneTapLogin.php b/src/Modules/OneTapLogin.php
@@ -162,7 +162,7 @@ public function one_tap_scripts(): void {
 	 */
 	public function validate_token(): void {
 		try {
-			$token    = Helper::filter_input( INPUT_POST, 'token', FILTER_SANITIZE_STRING );
+			$token    = Helper::filter_input( INPUT_POST, 'token', FILTER_SANITIZE_FULL_SPECIAL_CHARS );
 			$verified = $this->token_verifier->verify_token( $token );
 
 			if ( ! $verified ) {
@@ -179,8 +179,8 @@ public function validate_token(): void {
 			do_action( 'rtcamp.id_token_verified' );
 
 			$redirect_to   = apply_filters( 'rtcamp.google_default_redirect', admin_url() );
-			$state         = Helper::filter_input( INPUT_POST, 'state', FILTER_SANITIZE_STRING );
-			$decoded_state = $state ? (array) ( json_decode( base64_decode( $state ) ) ) : null;
+			$state         = Helper::filter_input( INPUT_POST, 'state', FILTER_SANITIZE_FULL_SPECIAL_CHARS );
+			$decoded_state = $state ? (array) ( json_decode( base64_decode( $state ) ) ) : null;    // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_decode
 
 			if ( is_array( $decoded_state ) && ! empty( $decoded_state['provider'] ) && 'google' === $decoded_state['provider'] ) {
 				$redirect_to = $decoded_state['redirect_to'] ?? $redirect_to;

diff --git a/src/Utils/Helper.php b/src/Utils/Helper.php
@@ -87,7 +87,7 @@ public static function filter_input( $type, $variable_name, $filter = FILTER_DEF
 			 * Use the PHP method and bail out.
 			 */
 			switch ( $filter ) {
-				case FILTER_SANITIZE_STRING:
+				case FILTER_SANITIZE_FULL_SPECIAL_CHARS:
 					$sanitized_variable = filter_input( $type, $variable_name, $filter );
 					break;
 				default:

diff --git a/tests/php/Unit/Modules/LoginTest.php b/tests/php/Unit/Modules/LoginTest.php
@@ -146,7 +146,7 @@ public function testAuthenticationForNoCode() {
 			[
 				INPUT_GET,
 				'code',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( null );
 
@@ -168,7 +168,7 @@ public function testAuthenticationForAlreadyAuthenticatedUser() {
 			[
 				INPUT_GET,
 				'code',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( null );
 
@@ -194,15 +194,15 @@ public function testAuthenticationForDifferentProvider() {
 			[
 				INPUT_GET,
 				'code',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( 'test_code' );
 
 		$helperMock->expects( 'filter_input' )->once()->withArgs(
 			[
 				INPUT_GET,
 				'state',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( $state );
 
@@ -224,15 +224,15 @@ public function testAuthenticationWithForgedState() {
 			[
 				INPUT_GET,
 				'code',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( 'abc' );
 
 		$helperMock->expects( 'filter_input' )->once()->withArgs(
 			[
 				INPUT_GET,
 				'state',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( 'eyJwcm92aWRlciI6ImdpdGh1YiJ9' );
 
@@ -251,15 +251,15 @@ public function testAuthenticationWhenUserExists() {
 			[
 				INPUT_GET,
 				'code',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( 'abc' );
 
 		$helperMock->expects( 'filter_input' )->once()->withArgs(
 			[
 				INPUT_GET,
 				'state',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( 'eyJwcm92aWRlciI6Imdvb2dsZSIsIm5vbmNlIjoidGVzdG5vbmNlIn0=' );
 
@@ -310,15 +310,15 @@ public function testAuthenticationCapturesExceptions() {
 			[
 				INPUT_GET,
 				'code',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( 'abc' );
 
 		$helperMock->expects( 'filter_input' )->once()->withArgs(
 			[
 				INPUT_GET,
 				'state',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( 'eyJwcm92aWRlciI6Imdvb2dsZSIsIm5vbmNlIjoidGVzdG5vbmNlIn0=' );
 
@@ -412,7 +412,7 @@ public function testStateRedirectWithRedirectTo() {
 			[
 				INPUT_GET,
 				'redirect_to',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( 'https://example.com/state-page' );
 
@@ -431,7 +431,7 @@ public function testStateRedirectWithoutRedirectTo() {
 			[
 				INPUT_GET,
 				'redirect_to',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( null );
 
@@ -457,7 +457,7 @@ public function testLoginRedirectWithNotStateAuthenticated() {
 			[
 				INPUT_GET,
 				'state',
-				FILTER_SANITIZE_STRING
+				FILTER_SANITIZE_FULL_SPECIAL_CHARS
 			]
 		)->andReturn( [] );