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

Resolve compatibility issue with PHP 8.1 #158

Merged
merged 9 commits into from
Sep 1, 2023
14 changes: 12 additions & 2 deletions .github/workflows/phpcs_on_pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
55 changes: 55 additions & 0 deletions phpcs.wpcs.xml
Original file line number Diff line number Diff line change
@@ -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>
62 changes: 33 additions & 29 deletions phpcs.xml
Original file line number Diff line number Diff line change
@@ -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>
10 changes: 5 additions & 5 deletions src/Modules/Login.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions src/Modules/OneTapLogin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/Utils/Helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
26 changes: 13 additions & 13 deletions tests/php/Unit/Modules/LoginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public function testAuthenticationForNoCode() {
[
INPUT_GET,
'code',
FILTER_SANITIZE_STRING
FILTER_SANITIZE_FULL_SPECIAL_CHARS
]
)->andReturn( null );

Expand All @@ -168,7 +168,7 @@ public function testAuthenticationForAlreadyAuthenticatedUser() {
[
INPUT_GET,
'code',
FILTER_SANITIZE_STRING
FILTER_SANITIZE_FULL_SPECIAL_CHARS
]
)->andReturn( null );

Expand All @@ -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 );

Expand All @@ -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' );

Expand All @@ -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=' );

Expand Down Expand Up @@ -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=' );

Expand Down Expand Up @@ -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' );

Expand All @@ -431,7 +431,7 @@ public function testStateRedirectWithoutRedirectTo() {
[
INPUT_GET,
'redirect_to',
FILTER_SANITIZE_STRING
FILTER_SANITIZE_FULL_SPECIAL_CHARS
]
)->andReturn( null );

Expand All @@ -457,7 +457,7 @@ public function testLoginRedirectWithNotStateAuthenticated() {
[
INPUT_GET,
'state',
FILTER_SANITIZE_STRING
FILTER_SANITIZE_FULL_SPECIAL_CHARS
]
)->andReturn( [] );

Expand Down