Skip to content

Removed config keys for <table>Plugin Now use the standard dot notation. #20

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

Merged
merged 1 commit into from
Jan 6, 2016
Merged

Removed config keys for <table>Plugin Now use the standard dot notation. #20

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 9 additions & 28 deletions src/Auth/TinyAuthorize.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,14 @@ class TinyAuthorize extends BaseAuthorize {
protected $_acl = null;

protected $_defaultConfig = [
'roleColumn' => 'role_id', // name of column in users table holding role id (used for single role/BT only)
'userColumn' => 'user_id',
'aliasColumn' => 'alias', // name of column in roles table holding role alias/slug
'roleColumn' => 'role_id', // Foreign key for the Role ID in users table or in pivot table
'userColumn' => 'user_id', // Foreign key for the User id in pivot table. Only for multi-roles setup
'aliasColumn' => 'alias', // Name of column in roles table holding role alias/slug
'rolesTable' => 'Roles', // name of Configure key holding available roles OR class name of roles table
'usersTable' => 'Users', // name of the Users table
'pivotTablePlugin' => '', // Name of the plugin managing the users table
'rolesTablePlugin' => '', // Name of the plugin managing the roles table
'multiRole' => false, // true to enables multirole/HABTM authorization (requires a valid join table)
'pivotTable' => null, // Use instead of auto-detect for the multi-role pivot table holding the user's roles
'superAdminRole' => null, // id of super admin role granted access to ALL resources
'pivotTable' => null, // Should be used in multi-roles setups
'multiRole' => false, // true to enables multirole/HABTM authorization (requires a valid pivot table)
'superAdminRole' => null, // id of super admin role, which grants access to ALL resources
'authorizeByPrefix' => false,
'prefixes' => [], // Whitelisted prefixes (only used when allowAdmin is enabled), leave empty to use all available
'allowUser' => false, // enable to allow ALL roles access to all actions except prefixed with 'adminPrefix'
Expand Down Expand Up @@ -325,12 +323,7 @@ protected function _getAvailableRoles() {
}

// fetch roles from database
$rolesPlugin = $this->_config['rolesTablePlugin'];
$roleTable = $this->_config['rolesTable'];
if (!$rolesPlugin) {
$roleTable = $rolesPlugin . '.' . $roleTable;
}
$rolesTable = TableRegistry::get($roleTable);
$rolesTable = TableRegistry::get($this->_config['rolesTable']);

$roles = $rolesTable->find('all')->formatResults(function ($results) {
return $results->combine($this->_config['aliasColumn'], 'id');
Expand Down Expand Up @@ -362,24 +355,12 @@ protected function _getUserRoles($user) {
throw new Exception(sprintf('Missing TinyAuthorize role id (%s) in user session', $this->_config['roleColumn']));
}

// multi-role: reverse engineer name of the pivot table
$rolesTableName = $this->_config['rolesTable'];
// Multi-role case : load the pivot table
$pivotTableName = $this->_config['pivotTable'];
$usersTableName = $this->_config['usersTable'];
if (!$pivotTableName) {
$tables = [
$usersTableName,
$rolesTableName
];
asort($tables);
$pivotTableName = implode('', $tables);
throw new Exception('Missing TinyAuthorize pivotTable. Check your configuration');
Copy link
Contributor

@bravo-kernel bravo-kernel Jan 6, 2016

Choose a reason for hiding this comment

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

This message make me think the table does not exist (in the database). Maybe change it to something like below, would make it consistent with the other feedback as well.

Invalid TinyAuthorize MultiRole Setup (no pivot table found in Configure)

Copy link
Contributor Author

@mtancoigne mtancoigne Jan 6, 2016

Choose a reason for hiding this comment

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

Sorry for the paste...

Copy link
Owner

@dereuromark dereuromark Jan 6, 2016

Choose a reason for hiding this comment

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

I agree. But since we have that logic here why do we not use conventions to find it? The error will then come from the orm and will say exaxtly what it said so far.

Copy link
Contributor

@bravo-kernel bravo-kernel Jan 6, 2016

Choose a reason for hiding this comment

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

@dereuromark I guess it's a preference thing. I always appreciate being informed about the root cause as concrete as possible and this would point me straight to Tiny's config but I can live with the ORM exception as well. Your plugin... your call ☺️

Copy link
Owner

@dereuromark dereuromark Jan 6, 2016

Choose a reason for hiding this comment

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

Since its cached performance is not an issue :)
I like convention driven. But maybe later

}

// fetch roles directly from the pivot table
$pivotTablePlugin = $this->_config['pivotTablePlugin'];
if (!$pivotTablePlugin) {
$pivotTableName = $pivotTablePlugin . '.' . $pivotTableName;
}
$pivotTable = TableRegistry::get($pivotTableName);
$roleColumn = $this->_config['roleColumn'];
$roles = $pivotTable->find('all', [
Expand Down