Skip to content

Commit

Permalink
Disable XML-RPC and REST login by default (#271)
Browse files Browse the repository at this point in the history
  • Loading branch information
kasparsd authored Mar 19, 2019
1 parent f33778a commit 6b49839
Show file tree
Hide file tree
Showing 7 changed files with 134 additions and 18 deletions.
16 changes: 10 additions & 6 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ addons:

cache:
directories:
- node_modules
- vendor
- $HOME/.composer/cache
- $HOME/.npm

branches:
only:
- master

notifications:
email: false
Expand All @@ -25,11 +29,11 @@ env:

before_script:
- npm install
- export DEV_LIB_PATH=vendor/xwp/wp-dev-lib
- source $DEV_LIB_PATH/travis.install.sh
- export DEV_LIB_PATH="vendor/xwp/wp-dev-lib/scripts"
- source "$DEV_LIB_PATH/travis.install.sh"

script:
- source $DEV_LIB_PATH/travis.script.sh
- source "$DEV_LIB_PATH/travis.script.sh"

after_script:
- source $DEV_LIB_PATH/travis.after_script.sh
- source "$DEV_LIB_PATH/travis.after_script.sh"
52 changes: 52 additions & 0 deletions class.two-factor-core.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ public static function add_hooks() {
add_filter( 'manage_users_columns', array( __CLASS__, 'filter_manage_users_columns' ) );
add_filter( 'wpmu_users_columns', array( __CLASS__, 'filter_manage_users_columns' ) );
add_filter( 'manage_users_custom_column', array( __CLASS__, 'manage_users_custom_column' ), 10, 3 );

// Run only after the core wp_authenticate_username_password() check.
add_filter( 'authenticate', array( __CLASS__, 'filter_authenticate' ), 50 );
}

/**
Expand Down Expand Up @@ -238,6 +241,55 @@ public static function wp_login( $user_login, $user ) {
exit;
}

/**
* Prevent login through XML-RPC and REST API for users with at least one
* two-factor method enabled.
*
* @param WP_User|WP_Error $user Valid WP_User only if the previous filters
* have verified and confirmed the
* authentication credentials.
*
* @return WP_User|WP_Error
*/
public static function filter_authenticate( $user ) {
if ( $user instanceof WP_User && self::is_api_request() && self::is_user_using_two_factor( $user->ID ) && ! self::is_user_api_login_enabled( $user->ID ) ) {
return new WP_Error(
'invalid_application_credentials',
__( 'Error: API login for user disabled.', 'two-factor' )
);
}

return $user;
}

/**
* If the current user can login via API requests such as XML-RPC and REST.
*
* @param integer $user_id User ID.
*
* @return boolean
*/
public static function is_user_api_login_enabled( $user_id ) {
return (bool) apply_filters( 'two_factor_user_api_login_enable', false, $user_id );
}

/**
* Is the current request an XML-RPC or REST request.
*
* @return boolean
*/
public static function is_api_request() {
if ( defined( 'XMLRPC_REQUEST' ) && XMLRPC_REQUEST ) {
return true;
}

if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) {
return true;
}

return false;
}

/**
* Display the login form.
*
Expand Down
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"description": "Two-Factor Authentication for WordPress.",
"require-dev": {
"php-coveralls/php-coveralls": "^1.0",
"wp-coding-standards/wpcs": "^1.0",
"xwp/wp-dev-lib": "dev-master"
"wp-coding-standards/wpcs": "^2.0",
"xwp/wp-dev-lib": "^1.0"
}
}
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
"scripts": {
"deploy": "grunt deploy",
"build": "npm run readme && grunt build",
"test": "DIFF_BASE=master ./vendor/xwp/wp-dev-lib/pre-commit",
"test": "DIFF_BASE=master ./vendor/xwp/wp-dev-lib/scripts/pre-commit",
"readme": "./vendor/xwp/wp-dev-lib/generate-markdown-readme",
"phpunit": "DEV_LIB_ONLY=phpunit ./vendor/xwp/wp-dev-lib/pre-commit",
"phpunit": "DEV_LIB_ONLY=phpunit ./vendor/xwp/wp-dev-lib/scripts/pre-commit",
"preinstall": "composer install",
"precommit": "DEV_LIB_SKIP=phpunit ./vendor/xwp/wp-dev-lib/pre-commit",
"precommit": "DEV_LIB_SKIP=phpunit ./vendor/xwp/wp-dev-lib/scripts/pre-commit",
"prepush": "npm test"
},
"repository": {
Expand Down
6 changes: 1 addition & 5 deletions phpcs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,15 @@
<rule ref="WordPress-Core">
<exclude name="Generic.Formatting.MultipleStatementAlignment" />
</rule>

<rule ref="WordPress-Docs">
<exclude name="Squiz.Commenting.FileComment.MissingPackageTag"/>
<exclude name="Squiz.Commenting.FileComment.Missing"/>
<exclude name="Squiz.Commenting.FileComment.WrongStyle"/>
<exclude name="Squiz.Commenting.FileComment.SpacingAfterComment"/>
<exclude name="Squiz.Commenting.ClassComment.Missing"/>
</rule>
<rule ref="WordPress-Extra">
<exclude name="WordPress.CSRF.NonceVerification" />
<exclude name="WordPress.WP.PreparedSQL.NotPrepared" />
</rule>

<exclude-pattern>*/dev-lib/*</exclude-pattern>
<exclude-pattern>*/includes/*</exclude-pattern>
<exclude-pattern>*/node_modules/*</exclude-pattern>
<exclude-pattern>*/tests/*</exclude-pattern>
Expand Down
5 changes: 3 additions & 2 deletions phpunit.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<phpunit
bootstrap="vendor/xwp/wp-dev-lib/phpunit-plugin-bootstrap.php"
bootstrap="vendor/xwp/wp-dev-lib/sample-config/phpunit-plugin-bootstrap.php"
backupGlobals="false"
colors="true"
convertErrorsToExceptions="true"
Expand All @@ -18,9 +18,10 @@
<whitelist processUncoveredFilesFromWhitelist="false">
<directory suffix=".php">.</directory>
<exclude>
<directory suffix=".php">bin</directory>
<directory suffix=".php">includes</directory>
<directory suffix=".php">tests</directory>
<directory suffix=".php">vendor</directory>
<directory suffix=".php">node_modules</directory>
</exclude>
</whitelist>
</filter>
Expand Down
63 changes: 63 additions & 0 deletions tests/class.two-factor-core.php
Original file line number Diff line number Diff line change
Expand Up @@ -215,4 +215,67 @@ public function test_login_url() {
) )
);
}

/**
* @covers Two_Factor_Core::is_user_api_login_enabled
*/
public function test_user_api_login_is_disabled_by_default() {
$user_id = $this->factory->user->create();

$this->assertFalse( Two_Factor_Core::is_user_api_login_enabled( $user_id ) );
}

/**
* @covers Two_Factor_Core::is_user_api_login_enabled
*/
public function test_user_api_login_can_be_enabled_via_filter() {
$user_id_default = $this->factory->user->create();
$user_id_enabled = $this->factory->user->create();

add_filter( 'two_factor_user_api_login_enable', function( $enabled, $user_id ) use ( $user_id_enabled ) {
return ( $user_id === $user_id_enabled );
}, 10, 2 );

$this->assertTrue(
Two_Factor_Core::is_user_api_login_enabled( $user_id_enabled ),
'Filters allows specific users to enable API login'
);

$this->assertFalse(
Two_Factor_Core::is_user_api_login_enabled( $user_id_default ),
'Filter doesnot impact other users'
);

// Undo all filters.
remove_all_filters( 'two_factor_user_api_login_enable', 10 );
}

/**
* @covers Two_Factor_Core::is_api_request
*/
public function test_is_api_request() {
$this->assertFalse( Two_Factor_Core::is_api_request() );
}

/**
* @covers Two_Factor_Core::filter_authenticate
*/
public function test_filter_authenticate() {
$user_default = new WP_User( $this->factory->user->create() );
$user_2fa_enabled = $this->get_dummy_user(); // User with a dummy two-factor method enabled.

// TODO: Get Two_Factor_Core away from static methods to allow mocking this.
define( 'XMLRPC_REQUEST', true );

$this->assertInstanceOf(
'WP_User',
Two_Factor_Core::filter_authenticate( $user_default )
);

$this->assertInstanceOf(
'WP_Error',
Two_Factor_Core::filter_authenticate( $user_2fa_enabled )
);
}

}

0 comments on commit 6b49839

Please sign in to comment.