Skip to content

Merge bmilesp fork #23

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.

Sign up for GitHub

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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Merge bmilesp fork #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a9c1d64
added support for multiple logins per api domain
bmilesp Jul 19, 2013
934a720
added tolkenized path support, added Cake-ified ruturn data ( $respo…
bmilesp Jul 19, 2013
c3476d1
PathTokenSource.php as extended APisource.php for tokenized path support
bmilesp Jul 19, 2013
6623c5a
added support for multiple logins per api domain in tokenStoreDb.php
bmilesp Jul 19, 2013
9a1cec5
updated readme.md and integration.md
bmilesp Jul 19, 2013
9812229
updated readme.md added multiple domain feature
bmilesp Jul 19, 2013
f520a4f
added samdbox_domain to tokens table
bmilesp Jul 19, 2013
836c973
added a column for organization grouping, for multiple users all usin…
bmilesp Jul 22, 2013
7ce02ec
make override compatable with Cakephp 2.4, !empty catches on unrequir…
bmilesp Jul 22, 2013
69a6511
modified cakeFormat for use after decoding, added arguments to decode…
bmilesp Jul 23, 2013
c531811
modified cakeFormat for use after decoding, added arguments to decode…
bmilesp Jul 23, 2013
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
3 changes: 3 additions & 0 deletions Config/Schema/schema.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@ public function after($event = array()) {
public $tokens = array(
'id' => array('type' => 'integer', 'null' => false, 'default' => null, 'key' => 'primary'),
'user_id' => array('type' => 'integer', 'null' => false, 'default' => null, 'key' => 'index'),
'organization_id' => array('type' => 'integer', 'null' => false, 'default' => null, 'key' => 'index'),
Copy link
Member

Choose a reason for hiding this comment

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

Since this seems rather implementation-specific and does not have any other code which refers to it, I would recommend against including this line in the project.

Copy link

Choose a reason for hiding this comment

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

the idea behind this was to have multiple users be able to share apis, aka an organization's apis. Probably more implementation specific, like you've said.

'api' => array('type' => 'string', 'null' => false, 'default' => null, 'collate' => 'utf8_general_ci', 'charset' => 'utf8'),
'access_token' => array('type' => 'text', 'null' => false, 'default' => null, 'collate' => 'utf8_general_ci', 'charset' => 'utf8'),
'modified' => array('type' => 'datetime', 'null' => true, 'default' => null),
'refresh_token' => array('type' => 'text', 'null' => true, 'default' => null, 'collate' => 'utf8_general_ci', 'charset' => 'utf8'),
'token_secret' => array('type' => 'text', 'null' => true, 'default' => null, 'collate' => 'utf8_general_ci', 'charset' => 'utf8'),
'expires_in' => array('type' => 'string', 'null' => true, 'default' => null, 'collate' => 'utf8_general_ci', 'charset' => 'utf8'),
'api_domain' => array('type' => 'string', 'null' => true, 'default' => null, 'collate' => 'utf8_general_ci', 'charset' => 'utf8'),
'sandbox_domain' => array('type' => 'string', 'null' => true, 'default' => null, 'collate' => 'utf8_general_ci', 'charset' => 'utf8'),
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, except with a little more reason to include. Presumably there is code somewhere that makes this work?

Copy link

Choose a reason for hiding this comment

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

api_domain is necessary to be able to store multiple domains per api (aka if you have multiple Shopify stores).

For the sandbox_domain, this is not necessary for Copula at this point- i thought it would be more of a global attribute but i've only used this for a single api thus far.

'indexes' => array(
'PRIMARY' => array('column' => 'id', 'unique' => 1),
'user_id' => array('column' => array('user_id', 'api'), 'unique' => 1)
Expand Down
26 changes: 18 additions & 8 deletions Controller/Component/OauthComponent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ protected function _checkAuthFailure(Controller $controller) {
function getOauthUri($apiName, $path, $extra = array()) {
if (Configure::check('Copula.' . $apiName . '.Auth')) {
$config = Configure::read('Copula.' . $apiName . '.Auth');

if(!empty($config['callback']) && is_array($config['callback'])){
$config['callback'] = Router::url($config['callback']);
}

//if a domain is passed into the options, override the default
if(!empty($extra['api_domain'])){
$config['host'] = $extra['api_domain'];
}
Copy link
Member

Choose a reason for hiding this comment

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

The callback here is okay. I don't feel like this is the appropriate place to change the host, however. I think it would make more sense to do that sort of switching in e.g. a beforeRequest callback. Possibly the default behavior for that should also include a check to see if the model has a method named beforeRequest() -- which was likely the thinking behind this otherwise pointless bit of code. The idea behind extras was for it to be passed directly to Router::querystring(), which can take additional arguments. I don't entirely trust my memory, but I suspect that using $extras in this manner will append the api domain to the query string as well as change the host.


if ($config['authMethod'] == 'OAuth') {
return $config['scheme'] . '://' . $config['host'] . '/' . $config[$path];
} elseif ($config['authMethod'] == 'OAuthV2') {
Expand Down Expand Up @@ -158,7 +168,7 @@ function getOauthRequestToken($apiName, $requestOptions = array()) {
* @param string $apiName
* @throws CakeException
*/
function callback($apiName) {
function callback($apiName, $api_domain = null) {
$method = $this->getOauthMethod($apiName);
if ($method == 'OAuthV2') {
$code = $this->controller->request->query('code');
Expand All @@ -167,9 +177,9 @@ function callback($apiName) {
}
$accessToken = $this->getAccessTokenV2($apiName, $code);
if (!empty($accessToken)) {
return $this->_afterRequest($accessToken, $apiName, $method);
return $this->_afterRequest($accessToken, $apiName, $method, $api_domain);
} else {
throw new CakeException(__('Could not get OAuthV2 Access Token from %s', $apiName));
throw new CakeException(__('Could not get OAuthV2 Access Token from %s or Token already exists', $apiName));
}
Copy link
Member

Choose a reason for hiding this comment

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

This seems rather ambiguous.

Copy link

Choose a reason for hiding this comment

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

See comment about api_domain in schema.php

} elseif ($method == 'OAuth') {
$verifier = $this->controller->request->query('oauth_verifier');
Expand All @@ -190,8 +200,8 @@ function callback($apiName) {
}
}

protected function _afterRequest(array $accessToken, $apiName, $version) {
if ($this->store($accessToken, $apiName, $version)) {
protected function _afterRequest(array $accessToken, $apiName, $version, $api_domain = null) {
if ($this->store($accessToken, $apiName, $version, $api_domain)) {
if ($this->Session->check('Oauth.redirect')) {
$redirect = $this->Session->read('Oauth.redirect');
$this->Session->delete('Oauth.redirect');
Expand All @@ -200,7 +210,7 @@ protected function _afterRequest(array $accessToken, $apiName, $version) {
return $accessToken;
}
} else {
throw new CakeException(__('Could not store access token for API %s', $apiName));
throw new CakeException(__('Could not store access token for API %s Possibly already in db', $apiName));
}
}

Expand All @@ -210,11 +220,11 @@ protected function _afterRequest(array $accessToken, $apiName, $version) {
* @param string|array $accessToken
* @param string $tokenSecret
*/
public function store(array $accessToken, $apiName, $version) {
public function store(array $accessToken, $apiName, $version, $api_domain = null) {
$storageMethod = (empty($this->controller->Apis[$apiName]['store'])) ? 'Db' : ucfirst($this->controller->Apis[$apiName]['store']);
$Store = ClassRegistry::init('Copula.TokenStore' . $storageMethod);
if ($Store instanceof TokenStoreInterface) {
return $Store->saveToken($accessToken, $apiName, AuthComponent::user('id'), $version);
return $Store->saveToken($accessToken, $apiName, AuthComponent::user('id'), $version, $api_domain);
} else {
throw new CakeException(__('Storage Method: %s not supported.', $storageMethod));
}
Expand Down
4 changes: 2 additions & 2 deletions Integration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ For the purposes of this document, the hosts configuration file is assumed to be
```php

<?php
$config['Copula']['cloudprint']['Auth'] = array(
$config['Copula']['cloudprint']['path']['Auth'] = array(
Copy link
Member

Choose a reason for hiding this comment

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

One of the two hard problems in computer science. At the risk of entering into a bike shed debate, is there a rationale for this?

Copy link

Choose a reason for hiding this comment

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

i had to add ['path'] in my paths.php file to get Copula working out of the box. It took a while to realize that the docs were wrong, so i had updated them.

'authMethod' => 'OAuthV2',
'scheme' => 'https',
'authorize' => 'o/oauth2/auth',
Expand All @@ -99,7 +99,7 @@ For the purposes of this document, the hosts configuration file is assumed to be
'callback' => 'https://example.com/oauth2callback/'
);

$config['Copula']['cloudprint']['Api'] = array(
$config['Copula']['cloudprint']['path']['Api'] = array(
'host' => 'www.google.com/cloudprint',
'authMethod' => 'OAuthV2'
);
Expand Down
23 changes: 19 additions & 4 deletions Model/Behavior/OAuthConsumerBehavior.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ public function setup(\Model $model, $config = array()) {
$this->config[$model->alias] = array_merge(
$this->config[$model->alias], (array) $config);
if ($this->config[$model->alias]['autoFetch'] === true) {
$this->authorize($model, AuthComponent::user('id'));
$authId = (!empty($config['user_id']))? $config['user_id'] : AuthComponent::user('id');
$this->authorize($model, $authId);
}
}

Expand All @@ -24,14 +25,16 @@ public function setup(\Model $model, $config = array()) {
* @param TokenStoreInterface $Store
* @return boolean
*/
function authorize(\Model $model, $userId, TokenStoreInterface $Store = null, $apiName = null) {
function authorize(\Model $model, $userId, TokenStoreInterface $Store = null, $apiName = null, $apiDomain = null) {

if (empty($Store)) {
$Store = ClassRegistry::init('Copula.TokenStoreDb');
}
if (empty($apiName)) {
$apiName = $model->useDbConfig;
}
$token = $Store->getToken($userId, $apiName);

$token = $Store->getToken($userId, $apiName, $apiDomain);
if (!empty($token)) {
ConnectionManager::getDataSource($model->useDbConfig)->setConfig($token);
return TRUE;
Expand All @@ -48,7 +51,8 @@ function authorize(\Model $model, $userId, TokenStoreInterface $Store = null, $a
* @return void
* @author Ceeram
*/
public function setDbConfig(\Model $model, $source = null, $useTable = null) {
public function setDbConfig(\Model $model, $source = null, $useTable = null, $api_domain = null, $userId = null, TokenStoreInterface $Store = null) {

$datasource = $model->getDataSource();
if (method_exists($datasource, 'flushMethodCache')) {
$datasource->flushMethodCache();
Expand All @@ -59,6 +63,17 @@ public function setDbConfig(\Model $model, $source = null, $useTable = null) {
if ($useTable !== null) {
$this->setSource($useTable);
}
if($api_domain){
if (empty($Store)) {
$Store = ClassRegistry::init('Copula.TokenStoreDb');
}
$token = $Store->getToken($userId, $apiName, $api_domain);
if (!empty($token)) {
$model->getDataSource($model->useDbConfig)->setConfig($token);
} else {
throw new CakeException(__('Could not get access token for Api %s', $model->useDbConfig));
}
}
} else {
if (!empty($this->config[$model->alias]['default'])) {
$this->setDataSource($this->config[$model->alias]['default']['useDbConfig']);
Expand Down
Loading