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

Validating redirect_uri according to rfc6749 4.1.3 #45

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,7 @@ _book
*.epub
*.mobi
*.pdf
.idea
.idea
.vscode
vendor
composer.lock
4 changes: 2 additions & 2 deletions inc/class-client.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,8 @@ public function check_redirect_uri( $uri ) {
*
* @return Authorization_Code|WP_Error
*/
public function generate_authorization_code( WP_User $user ) {
return Authorization_Code::create( $this, $user );
public function generate_authorization_code( WP_User $user, $data ) {
return Authorization_Code::create( $this, $user, $data );
}

/**
Expand Down
2 changes: 1 addition & 1 deletion inc/endpoints/class-token.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public function exchange_token( WP_REST_Request $request ) {
return $auth_code;
}

$is_valid = $auth_code->validate();
$is_valid = $auth_code->validate( $request );
if ( is_wp_error( $is_valid ) ) {
// Invalid request, but code itself exists, so we should delete
// (and silently ignore errors).
Expand Down
49 changes: 46 additions & 3 deletions inc/tokens/class-authorization-code.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,43 @@ public function get_expiration() {
return (int) $value['expiration'];
}

/**
* Validate whether the redirect_uri matches the one in the initial OAuth2 request
*
* @param String $redirect_uri
* @return Boolean|WP_Error
*/
protected function validate_redirect_uri( $redirect_uri ) {
$value = $this->get_value();

if ( ! empty( $value['redirect_uri'] ) && ! empty( $redirect_uri ) ) {
if ( $value['redirect_uri'] !== $redirect_uri ) {
return new WP_Error(
'oauth2.tokens.authorization_code.redirect_uri.mismatch',
__( 'redirect_uri does not match the one in the initial request.', 'oauth2' ),
[
'status' => WP_Http::BAD_REQUEST,
]
);
}
}

if ( ( empty( $value['redirect_uri'] ) && ! empty( $redirect_uri ) && ! $this->client->check_redirect_uri( $redirect_uri ) ) ||
( ! empty( $value['redirect_uri'] ) && empty( $redirect_uri ) ) ) {
return new WP_Error(
'oauth2.tokens.authorization_code.redirect_uri.mismatch',
__( 'redirect_uri does not match the one in the initial request.', 'oauth2' ),
[
'status' => WP_Http::BAD_REQUEST,
'expiration' => $expiration,
'time' => $now,
]
);
}

return true;
}

/**
* Validate the code for use.
*
Expand All @@ -129,6 +166,11 @@ public function validate( $args = [] ) {
);
}

$redirect_uri = $this->validate_redirect_uri( $args['redirect_uri'] );
if ( is_wp_error( $redirect_uri ) ) {
return $redirect_uri;
}

return true;
}

Expand Down Expand Up @@ -183,12 +225,13 @@ public static function get_by_code( Client $client, $code ) {
*
* @return Authorization_Code|WP_Error Authorization code instance, or error on failure.
*/
public static function create( Client $client, WP_User $user ) {
public static function create( Client $client, WP_User $user, $redirect_uri = '' ) {
$code = wp_generate_password( static::KEY_LENGTH, false );
$meta_key = static::KEY_PREFIX . $code;
$data = [
'user' => (int) $user->ID,
'expiration' => time() + static::MAX_AGE,
'user' => (int) $user->ID,
'expiration' => time() + static::MAX_AGE,
'redirect_uri' => $redirect_uri,
];
$result = add_post_meta( $client->get_post_id(), wp_slash( $meta_key ), wp_slash( $data ), true );
if ( ! $result ) {
Expand Down
3 changes: 2 additions & 1 deletion inc/types/class-authorization-code.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ protected function handle_authorization_submission( $submit, Client $client, $da
case 'authorize':
// Generate authorization code and redirect back.
$user = wp_get_current_user();
$code = $client->generate_authorization_code( $user );
$code = $client->generate_authorization_code( $user, $redirect_uri );

if ( is_wp_error( $code ) ) {
return $code;
}
Expand Down
35 changes: 12 additions & 23 deletions inc/types/class-base.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,12 @@ public function handle_authorisation() {
}

// Validate the redirection URI.
$redirect_uri = $this->validate_redirect_uri( $client, $redirect_uri );
if ( is_wp_error( $redirect_uri ) ) {
return $redirect_uri;
if ( ! empty( $redirect_uri ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this should always pass the $redirect_uri in?

$redirect_uri = $this->validate_redirect_uri( $client, $redirect_uri );

if ( is_wp_error( $redirect_uri ) ) {
return $redirect_uri;
}
}

// Valid parameters, ensure the user is logged in.
Expand All @@ -69,8 +72,7 @@ public function handle_authorisation() {
}

// Check nonce.
$nonce_action = $this->get_nonce_action( $client );
if ( ! wp_verify_nonce( wp_unslash( $_POST['_wpnonce'] ), $none_action ) ) {
if ( ! wp_verify_nonce( wp_unslash( $_POST['_wpnonce'] ), $this->get_nonce_action( $client ) ) ) {
return new WP_Error(
'oauth2.types.authorization_code.handle_authorisation.invalid_nonce',
__( 'Invalid nonce.', 'oauth2' )
Expand Down Expand Up @@ -105,24 +107,11 @@ public function handle_authorisation() {
* @return string|WP_Error Valid redirect URI on success, error otherwise.
*/
protected function validate_redirect_uri( Client $client, $redirect_uri = null ) {
if ( empty( $redirect_uri ) ) {
$registered = $client->get_redirect_uris();
if ( count( $registered ) !== 1 ) {
// Either none registered, or more than one, so error.
return new WP_Error(
'oauth2.types.authorization_code.handle_authorisation.missing_redirect_uri',
__( 'Redirect URI was required, but not found.', 'oauth2' )
);
}

$redirect_uri = $registered[0];
} else {
if ( ! $client->check_redirect_uri( $redirect_uri ) ) {
return new WP_Error(
'oauth2.types.authorization_code.handle_authorisation.invalid_redirect_uri',
__( 'Specified redirect URI is not valid for this client.', 'oauth2' )
);
}
if ( ! $client->check_redirect_uri( $redirect_uri ) ) {
return new WP_Error(
'oauth2.types.authorization_code.handle_authorisation.invalid_redirect_uri',
__( 'Specified redirect URI is not valid for this client.', 'oauth2' )
);
}

return $redirect_uri;
Expand Down