From db7602af7c26258b7f1ff5dd7ac42c6613d146df Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Mon, 4 Jun 2012 10:51:21 +0800 Subject: [PATCH 1/3] MDL-33501 - oauth2lib: improve redirect url handling Only accept PARAM_LOCALURL for state params and enforce use of moodle_url param in oauthlib to facilitate that. --- admin/oauth2callback.php | 2 +- lib/oauthlib.php | 10 +++++----- portfolio/googledocs/lib.php | 2 +- portfolio/picasa/lib.php | 2 +- repository/googledocs/lib.php | 2 +- repository/picasa/lib.php | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/admin/oauth2callback.php b/admin/oauth2callback.php index 364c0023de0..695353cc5a0 100644 --- a/admin/oauth2callback.php +++ b/admin/oauth2callback.php @@ -33,6 +33,6 @@ require_once(dirname(dirname(__FILE__)).'/config.php'); // The authorization code generated by the authorization server. $code = required_param('code', PARAM_RAW); // The state parameter we've given (used in moodle as a redirect url). -$state = required_param('state', PARAM_URL); +$state = required_param('state', PARAM_LOCALURL); redirect(new moodle_url($state, array('code' => $code))); diff --git a/lib/oauthlib.php b/lib/oauthlib.php index b4aa15b7500..88cd53c277d 100644 --- a/lib/oauthlib.php +++ b/lib/oauthlib.php @@ -368,8 +368,8 @@ abstract class oauth2_client extends curl { private $clientid = ''; /** var string The client secret. */ private $clientsecret = ''; - /** var string URL to return to after authenticating */ - private $returnurl = ''; + /** var moodle_url URL to return to after authenticating */ + private $returnurl = null; /** var string scope of the authentication request */ private $scope = ''; /** var stdClass access token object */ @@ -392,10 +392,10 @@ abstract class oauth2_client extends curl { * * @param string $clientid * @param string $clientsecret - * @param string $returnurl + * @param moodle_url $returnurl * @param string $scope */ - public function __construct($clientid, $clientsecret, $returnurl, $scope) { + public function __construct($clientid, $clientsecret, moodle_url $returnurl, $scope) { parent::__construct(); $this->clientid = $clientid; $this->clientsecret = $clientsecret; @@ -456,7 +456,7 @@ abstract class oauth2_client extends curl { array('client_id' => $this->clientid, 'response_type' => 'code', 'redirect_uri' => $callbackurl->out(false), - 'state' => $this->returnurl, + 'state' => $this->returnurl->out_as_local_url(false), 'scope' => $this->scope, )); diff --git a/portfolio/googledocs/lib.php b/portfolio/googledocs/lib.php index 9820246f9db..02ce15910ef 100644 --- a/portfolio/googledocs/lib.php +++ b/portfolio/googledocs/lib.php @@ -124,7 +124,7 @@ class portfolio_plugin_googledocs extends portfolio_plugin_push_base { $clientid = $this->get_config('clientid'); $secret = $this->get_config('secret'); - $this->googleoauth = new google_oauth($clientid, $secret, $returnurl->out(false), google_docs::REALM); + $this->googleoauth = new google_oauth($clientid, $secret, $returnurl, google_docs::REALM); } public function instance_sanity_check() { diff --git a/portfolio/picasa/lib.php b/portfolio/picasa/lib.php index d637b54edca..70200c13af8 100644 --- a/portfolio/picasa/lib.php +++ b/portfolio/picasa/lib.php @@ -124,7 +124,7 @@ class portfolio_plugin_picasa extends portfolio_plugin_push_base { $clientid = $this->get_config('clientid'); $secret = $this->get_config('secret'); - $this->googleoauth = new google_oauth($clientid, $secret, $returnurl->out(false), google_picasa::REALM); + $this->googleoauth = new google_oauth($clientid, $secret, $returnurl, google_picasa::REALM); } public function instance_sanity_check() { diff --git a/repository/googledocs/lib.php b/repository/googledocs/lib.php index 6ccb33bc436..22605dc4ae9 100644 --- a/repository/googledocs/lib.php +++ b/repository/googledocs/lib.php @@ -44,7 +44,7 @@ class repository_googledocs extends repository { $clientid = get_config('googledocs', 'clientid'); $secret = get_config('googledocs', 'secret'); - $this->googleoauth = new google_oauth($clientid, $secret, $returnurl->out(false), google_docs::REALM); + $this->googleoauth = new google_oauth($clientid, $secret, $returnurl, google_docs::REALM); $this->check_login(); } diff --git a/repository/picasa/lib.php b/repository/picasa/lib.php index 902238025fa..fb2a83a5330 100644 --- a/repository/picasa/lib.php +++ b/repository/picasa/lib.php @@ -46,7 +46,7 @@ class repository_picasa extends repository { $clientid = get_config('picasa', 'clientid'); $secret = get_config('picasa', 'secret'); - $this->googleoauth = new google_oauth($clientid, $secret, $returnurl->out(false), google_picasa::REALM); + $this->googleoauth = new google_oauth($clientid, $secret, $returnurl, google_picasa::REALM); $this->check_login(); } From 5df1b737483c9d65e16a72e0937c1efd02edafa1 Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Mon, 4 Jun 2012 11:11:38 +0800 Subject: [PATCH 2/3] MDL-33501 - oauth2lib: enforce sesskey in oauth2callback.php The sesskey needs to be embeded in the local url returned as this is the only parameter we have control of. --- admin/oauth2callback.php | 10 +++++++++- repository/googledocs/lib.php | 6 ++++-- repository/picasa/lib.php | 6 ++++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/admin/oauth2callback.php b/admin/oauth2callback.php index 695353cc5a0..2ec8c38aead 100644 --- a/admin/oauth2callback.php +++ b/admin/oauth2callback.php @@ -35,4 +35,12 @@ $code = required_param('code', PARAM_RAW); // The state parameter we've given (used in moodle as a redirect url). $state = required_param('state', PARAM_LOCALURL); -redirect(new moodle_url($state, array('code' => $code))); +$redirecturl = new moodle_url($state); +$params = $redirecturl->params(); + +if (isset($params['sesskey']) and confirm_sesskey($params['sesskey'])) { + $redirecturl->param('code', $code); + redirect($redirecturl); +} else { + print_error('invalidsesskey'); +} diff --git a/repository/googledocs/lib.php b/repository/googledocs/lib.php index 22605dc4ae9..df0f23eb0ff 100644 --- a/repository/googledocs/lib.php +++ b/repository/googledocs/lib.php @@ -39,8 +39,10 @@ class repository_googledocs extends repository { public function __construct($repositoryid, $context = SYSCONTEXTID, $options = array()) { parent::__construct($repositoryid, $context, $options); - $returnurl = new moodle_url('/repository/repository_callback.php', - array('callback' => 'yes', 'repo_id' =>$this->id)); + $returnurl = new moodle_url('/repository/repository_callback.php'); + $returnurl->param('callback', 'yes'); + $returnurl->param('repo_id', $this->id); + $returnurl->param('sesskey', sesskey()); $clientid = get_config('googledocs', 'clientid'); $secret = get_config('googledocs', 'secret'); diff --git a/repository/picasa/lib.php b/repository/picasa/lib.php index fb2a83a5330..0d0b63c0cfa 100644 --- a/repository/picasa/lib.php +++ b/repository/picasa/lib.php @@ -41,8 +41,10 @@ class repository_picasa extends repository { public function __construct($repositoryid, $context = SYSCONTEXTID, $options = array()) { parent::__construct($repositoryid, $context, $options); - $returnurl = new moodle_url('/repository/repository_callback.php', - array('callback' => 'yes', 'repo_id' =>$this->id)); + $returnurl = new moodle_url('/repository/repository_callback.php'); + $returnurl->param('callback', 'yes'); + $returnurl->param('repo_id', $this->id); + $returnurl->param('sesskey', sesskey()); $clientid = get_config('picasa', 'clientid'); $secret = get_config('picasa', 'secret'); From 7b61ac39d2674f3ce9f7a5dadc4ea85d7ded8f50 Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Mon, 4 Jun 2012 11:25:19 +0800 Subject: [PATCH 3/3] MDL-33507 - oauth2lib: switch 'code' to oauth2code To avoid param collisions in future. --- admin/oauth2callback.php | 2 +- lib/oauthlib.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/admin/oauth2callback.php b/admin/oauth2callback.php index 2ec8c38aead..c032a9c2f16 100644 --- a/admin/oauth2callback.php +++ b/admin/oauth2callback.php @@ -39,7 +39,7 @@ $redirecturl = new moodle_url($state); $params = $redirecturl->params(); if (isset($params['sesskey']) and confirm_sesskey($params['sesskey'])) { - $redirecturl->param('code', $code); + $redirecturl->param('oauth2code', $code); redirect($redirecturl); } else { print_error('invalidsesskey'); diff --git a/lib/oauthlib.php b/lib/oauthlib.php index 88cd53c277d..587ba195b60 100644 --- a/lib/oauthlib.php +++ b/lib/oauthlib.php @@ -425,7 +425,7 @@ abstract class oauth2_client extends curl { // If we've been passed then authorization code generated by the // authorization server try and upgrade the token to an access token. - $code = optional_param('code', null, PARAM_RAW); + $code = optional_param('oauth2code', null, PARAM_RAW); if ($code && $this->upgrade_token($code)) { return true; }