Entire Code Base: Methods are not checking for valid/existing Roles and Permissions

[jburns131]

It seems that most methods (mainly assignment methods) are not checking to see if we are trying to assign valid/existing Roles and Permissions.

Example:

• The only Role/Permission in the database is the 'root' Role and 'root' Permission when we execute this,

  $rbac->Assign(5, 5);
  

  After execution the rolepermission table contains this

  RoleID = 5, PermissionID = 5
  

Result:

• We now have a role->permission relation to non-existent roles and permissions.

Should we be checking for the existence of roles/permissions before trying to manipulate them?

[abiusx]

hmm
maybe put a wrapper there for it? cuz it will make it slower
let think about performance and then decide, I have no idae right now

---

Notice: This message is digitally signed, its source and integrity are verifiable.
If you mail client does not support S/MIME verification, it will display a file (smime.p7s), which includes the X.509 certificate and the signature body. Read more at Certified E-Mail with Comodo and Thunderbird in AbiusX.com

On Shahrivar 1, 1392, at 9:27 AM, Jesse Burns notifications@github.com wrote:

It seems that most methods (mainly assignment methods) are not checking to see if we are trying to assign valid/existing Roles and Permissions.

Example:

The only Role/Permission in the database is the 'root' Role and 'root' Permission when we execute this,

$rbac->Assign(5, 5);
After execution the rolepermission table contains this

RoleID = 5, PermissionID = 5
Result:

We now have a role->permission relation to non-existent roles and permissions.
Should we be checking for the existence of roles/permissions before trying to manipulate them?

—
Reply to this email directly or view it on GitHub.

[jburns131]

I was thinking about performance too.

I'll also have to think more about this before I can offer what I think is the best solution.

One thought off of the top of my head would be to create something similar to $rbac->Reset that will clean the database and remove orphan entries.

I see two problems with that:

1. It's an after thought action that users will have to perform manually
2. It will report inaccurate data for users that are generating reports or creating a GUI that lists Roles, Permissions and relationships between the two in order to assign roles to users.

Like I said, I'll think more on this and try to offer my best opinion/suggestion.

[abiusx]

some of the functionality is used in the administrative interface, and the rest in the common application flow

---

Notice: This message is digitally signed, its source and integrity are verifiable.
If you mail client does not support S/MIME verification, it will display a file (smime.p7s), which includes the X.509 certificate and the signature body. Read more at Certified E-Mail with Comodo and Thunderbird in AbiusX.com

On Shahrivar 1, 1392, at 1:41 PM, Jesse Burns notifications@github.com wrote:

I was thinking about performance too.

I'll also have to think more about this before I can offer what I think is the best solution.

One thought off of the top of my head would be to create something similar to $rbac->Reset that will clean the database and remove orphan entries.

I see two problems with that:

It's an after thought action that users will have to perform manually
It will report inaccurate data for users that are generating reports or creating a GUI that lists Roles, Permissions and relationships between the two in order to assign roles to users.
Like I said, I'll think more on this and try to offer my best opinion/suggestion.

—
Reply to this email directly or view it on GitHub.

[jburns131]

That's a good point.

This really is the responsibility of the main application. PhpRbac will never create an orphan on it's own. If there are orphans, it is due to faulty administration code.

A helper method to remove orphans might be nice, but I think that's something we can put a hold on until we receive requests for something like that.

If you think that it's prudent, we can close this issue.

[jburns131]

Closing issue due to the fact that this topic is outside of PhpRbac's scope.