Postgres Patch

[dinobot71]

I'm including here the details of what I had to do to make this work with PostgreSQL 9.0.x on a reasonably modern PHP 5.5. A lot of work was needed to make this run in PostgreSQL, but it passes all 183 regression tests with flying colors, I'm confident at this point that its working well in modern PostgreSQL dbs.

I have a complete patch file for you,

diff -ruN rbac.old rbac.new/ > phprbac-pgsql.patch

but I have to figure out how to make an attachment, I don't see a button for that on the GIt issue page. If you want to email me directly I can pass it to you that way as well.

If you are already working on postgresql support in an upcoming version...nevermind :) But for those of us who need postgresql support now...this should work well.

(^_^)/ mike. ............................................. `[mgarvin@start tests]$ ./pgsql_tests.sh PHPUnit 3.7.24 by Sebastian Bergmann.

Configuration read from /localdisk/var/mgarvin/start/rbac/PhpRbac/tests/phpunit_pgsql.xml

............................................................... 63 / 183 ( 34%) ............................................................... 126 / 183 ( 68%) .........................................................

Time: 1.84 minutes, Memory: 353.25Mb

OK (183 tests, 183 assertions) Press [Enter] key to continue... [mgarvin@start tests]$` .............................................

Problems that could have been avoided:

• using mixed case for table / column names, means quoting is an issue everywhere, different dbs quote differently. If lower case had been used, all those problems would be non-existent. As it stands now, huge PITA.
• using an integer for the date stamps is simple, but not very portable and will break at some point. Using the build in data/time stamp type of the database would have been much better. Enterprise users don't use sqlite, they use actual databases.
• psr-0 autoloading is difficult to use and follow, psr-4 should have been followed.
• in the composer world kalnoy/nestedset or baum is the standard for nested trees, that should have been reused here instead of writing it from scratch it adds complexity needlessly and doesn't do it obviously better.
• sprinkling of decision code; has is() functions, and then doesn't use them, calls to pdo get_class all over the place.
• resetAssignments() (both of them) doesn't make sense. Its trying to reset the sequence for rolepermissions, and userroles which are join tables not an auto-increment table.
• enforce() doesn't know anything about content type or if its being used by REST etc. What if we're doing JSON?
• functions like permissions() make use of MySQL style quoting without checking adapter type...yay, quoting!
• shouldn't be implementing a db layer anyways, Illuminate etc should be used. ORMs are well established now, writing a half baked db layer is just bad practice and causes everyone grief.
• defines model field names like left(), right(), id(), but then doesn't use them everywhere, so you have a sprinkling of hard coded field names everywhere.
• functions like pathId() use mysql specific and non-standard features, making it very difficult to port to anything else. Things like GROUP_CONCAT have equivalents, but referencing aggregates or aliases in the HAVING or GROUP BY is not allowed in the SQL standard. Had to rework with derived tables. NOT FUN.

Dealing with quoting:

• unfortunately the mysql style quoting isn't used consistently, so you can't just replace backticks with double quotes, because in my queries the backticks are present on some fields and not on others.
• can't do automatic search and replace of quotes (not consistent).
• rewriting everything to have lower case fields and no quotes, woudl be the most consistent, clean and error free way to go, but lots of work.
• correcting the code to use backticks consistently on all identifiers would enable auto translation, still lots of work.
• adding a "quoteIdentifier()" method instead of using hard coded quotes, would be ideal, but ltos of work and you are just rewriting ORM features :(
• replacing hard coded backticks with a static member that is the current quote to use for quoting identifiers, not great, still lots of work, but keeps the code as close to how it is now as possible.
• don't use any quotes (neither backtick nor double quote)...this surprisingly might be the best way to go. PostgreSQL at least will always normalize to lower case, both when creating and when fetching, so you can write mixed case but in the db its lower case (always). As long as you don't try to have columns that differ only in case...your fine. Only hurdle is that the PHP code actually refers to stuff by case...but you could go through it and clean that up to use all lower.
• can tell mysql to do it the same as postgresql: SET sql_mode='ANSI_QUOTES', then the quoting can be the same on both.
• given all the options, and the potential of breaking one one of the databases, or some of the PHP code that expects mixed case...the best option seems to be to continue the quoting but correct the code to consistently quote identifiers with a quote character based on the correct database instead of being hard coded for mysql.

The postgresql setup code:

/*

• Create Tables */

CREATE TABLE IF NOT EXISTS "PREFIX_permissions" ( "ID" SERIAL, "Lft" integer NOT NULL, "Rght" integer NOT NULL, "Title" varchar(64) NOT NULL, "Description" text NOT NULL, PRIMARY KEY ("ID", "Title", "Lft", "Rght") );

ALTER SEQUENCE "PREFIX_permissions_ID_seq" RESTART WITH 1;

CREATE TABLE IF NOT EXISTS "PREFIX_rolepermissions" ( "RoleID" integer NOT NULL, "PermissionID" integer NOT NULL, "AssignmentDate" integer NOT NULL, PRIMARY KEY ("RoleID","PermissionID") );

CREATE TABLE IF NOT EXISTS "PREFIX_roles" ( "ID" SERIAL, "Lft" integer NOT NULL, "Rght" integer NOT NULL, "Title" varchar(128) NOT NULL, "Description" text NOT NULL, PRIMARY KEY ("ID", "Title", "Lft", "Rght") );

ALTER SEQUENCE "PREFIX_roles_ID_seq" RESTART WITH 1;

CREATE TABLE IF NOT EXISTS "PREFIX_userroles" ( "UserID" integer NOT NULL, "RoleID" integer NOT NULL, "AssignmentDate" integer NOT NULL, PRIMARY KEY ("UserID","RoleID") );

/*

• Insert Initial Table Data */

INSERT INTO "PREFIX_permissions" ("ID", "Lft", "Rght", "Title", "Description") VALUES (1, 0, 1, 'root', 'root');

INSERT INTO "PREFIX_rolepermissions" ("RoleID", "PermissionID", "AssignmentDate") VALUES (1, 1, (select extract(epoch from now())::int));

INSERT INTO "PREFIX_roles" ("ID", "Lft", "Rght", "Title", "Description") VALUES (1, 0, 1, 'root', 'root');

INSERT INTO "PREFIX_userroles" ("UserID", "RoleID", "AssignmentDate") VALUES (1, 1, (select extract(epoch from now())::int));

[dinobot71]

Here is the patch file to modify the code to work with PostgreSQL 9.0.+ and with PHP 5.5+

phprbac-pgsql.zip

[abiusx]

Thank you very much for the patch! Can you submit it as a pull request so that we can merge into the main branch? Of course if it doesn’t break other DB compatibility (SQLite and MySQL at the moment). As for the points that you mentioned:

using mixed case for table / column names, means quoting is an issue everywhere, different dbs quote differently. If lower case had been used, all those problems would be non-existent. As it stands now, huge PITA.

True. The library was field agnostic, but fields were later hardcoded just to make it faster in the benchmarks. using an integer for the date stamps is simple, but not very portable and will break at some point. Using the build in data/time stamp type of the database would have been much better. Enterprise users don't use sqlite, they use actual databases.

What possible benefit is there for using built in date/time format instead of timestamp? How can timestamp not be portable? It is a simple integer supported by all systems everywhere. psr-0 autoloading is difficult to use and follow, psr-4 should have been followed.

At the time this library was built, PSR was not yet born.

in the composer world kalnoy/nestedset or baum is the standard for nested trees, that should have been reused here instead of writing it from scratch it adds complexity needlessly and doesn't do it obviously better.

This library was created at least 5 years prior to composer, and obviously more years prior to kalnoy/nestedset. Maybe they should’ve not reinvented the wheel!

sprinkling of decision code; has is() functions, and then doesn't use them, calls to pdo get_class all over the place.

What is the problem with get_class? Is_a did not exist in PHP when this was implemented. resetAssignments() (both of them) doesn't make sense. Its trying to reset the sequence for rolepermissions, and userroles which are join tables not an auto-increment table.

That’s for testing purposes. enforce() doesn't know anything about content type or if its being used by REST etc. What if we're doing JSON?

True. functions like permissions() make use of MySQL style quoting without checking adapter type...yay, quoting!

True. Have you fixed them? We can merge into the main branch. shouldn't be implementing a db layer anyways, Illuminate etc should be used. ORMs are well established now, writing a half baked db layer is just bad practice and causes everyone grief.

Definitely not. ORM has nothing to do with a DBAL. ORM is terribly slow and half-witted for such purposes. PDO is the PHP DBAL, but at the time this library was implemented, PDO was significantly slower than native drivers. Thus the adapters. defines model field names like left(), right(), id(), but then doesn't use them everywhere, so you have a sprinkling of hard coded field names everywhere.

Yes, as mentioned, this is due to performance increases. You can safely remove them now. functions like pathId() use mysql specific and non-standard features, making it very difficult to port to anything else. Things like GROUP_CONCAT have equivalents, but referencing aggregates or aliases in the HAVING or GROUP BY is not allowed in the SQL standard. Had to rework with derived tables. NOT FUN.

PathId could not be implemented database agnostic in a fast way. Keep in mind that this RBAC solution is built to scale, not port. There are a few places along the code where database specific code is needed. As for the aggregate referencing, SQL92 and SQL99 both support that, but suggest not using it. SQLite and MySQL have it by default (although can be disabled) and MSSQL and Postgre can have it enabled. Perhaps it is a good idea to port this, if it doesn’t damage performance. You definitely wouldn’t need to use derived tables instead, fixing aggregate referencing is easy.

unfortunately the mysql style quoting isn't used consistently, so you can't just replace backticks with double quotes, because in my queries the backticks are present on some fields and not on others.

True. MySQL only requires quotes when the field name is a keyword. Apparently left and right are keywords, and I didn’t want to use “Lft” and “Rght” back then. Maybe we should change all Left and Rights to Lft and Rght, and then replace all? can't do automatic search and replace of quotes (not consistent).

rewriting everything to have lower case fields and no quotes, woudl be the most consistent, clean and error free way to go, but lots of work.

I had no idea Postgre doesn’t like non-lowercase fields. correcting the code to use backticks consistently on all identifiers would enable auto translation, still lots of work.

adding a "quoteIdentifier()" method instead of using hard coded quotes, would be ideal, but ltos of work and you are just rewriting ORM features :(

PHP is not very fast on micro-function calls. That would be a bad idea, specially since we always know where it’s needed. replacing hard coded backticks with a static member that is the current quote to use for quoting identifiers, not great, still lots of work, but keeps the code as close to how it is now as possible.

Much better in terms of performance. Or using a constant? don't use any quotes (neither backtick nor double quote)...this surprisingly might be the best way to go. PostgreSQL at least will always normalize to lower case, both when creating and when fetching, so you can write mixed case but in the db its lower case (always). As long as you don't try to have columns that differ only in case...your fine. Only hurdle is that the PHP code actually refers to stuff by case...but you could go through it and clean that up to use all lower.

can tell mysql to do it the same as postgresql: SET sql_mode='ANSI_QUOTES', then the quoting can be the same on both.

given all the options, and the potential of breaking one one of the databases, or some of the PHP code that expects mixed case...the best option seems to be to continue the quoting but correct the code to consistently quote identifiers with a quote character based on the correct database instead of being hard coded for mysql.

On Jun 15, 2016, at 4:38 PM, Michael Garvin notifications@github.com wrote:

I'm including here the details of what I had to do to make this work with PostgreSQL 9.0.x on a reasonably modern PHP 5.5. A lot of work was needed to make this run in PostgreSQL, but it passes all 183 regression tests with flying colors, I'm confident at this point that its working well in modern PostgreSQL dbs.

I have a complete patch file for you,

diff -ruN rbac.old rbac.new/ > phprbac-pgsql.patch

but I have to figure out how to make an attachment, I don't see a button for that on the GIt issue page. If you want to email me directly I can pass it to you that way as well.

If you are already working on postgresql support in an upcoming version...nevermind :) But for those of us who need postgresql support now...this should work well.

(^_^)/ mike. ............................................. `[mgarvin@start tests]$ ./pgsql_tests.sh PHPUnit 3.7.24 by Sebastian Bergmann.

Configuration read from /localdisk/var/mgarvin/start/rbac/PhpRbac/tests/phpunit_pgsql.xml

............................................................... 63 / 183 ( 34%) ............................................................... 126 / 183 ( 68%) .........................................................

Time: 1.84 minutes, Memory: 353.25Mb

OK (183 tests, 183 assertions) Press [Enter] key to continue... [mgarvin@start tests]$` .............................................

Problems that could have been avoided:

using mixed case for table / column names, means quoting is an issue everywhere, different dbs quote differently. If lower case had been used, all those problems would be non-existent. As it stands now, huge PITA.

using an integer for the date stamps is simple, but not very portable and will break at some point. Using the build in data/time stamp type of the database would have been much better. Enterprise users don't use sqlite, they use actual databases.

psr-0 autoloading is difficult to use and follow, psr-4 should have been followed.

in the composer world kalnoy/nestedset or baum is the standard for nested trees, that should have been reused here instead of writing it from scratch it adds complexity needlessly and doesn't do it obviously better.

sprinkling of decision code; has is() functions, and then doesn't use them, calls to pdo get_class all over the place.

resetAssignments() (both of them) doesn't make sense. Its trying to reset the sequence for rolepermissions, and userroles which are join tables not an auto-increment table.

enforce() doesn't know anything about content type or if its being used by REST etc. What if we're doing JSON?

functions like permissions() make use of MySQL style quoting without checking adapter type...yay, quoting!

shouldn't be implementing a db layer anyways, Illuminate etc should be used. ORMs are well established now, writing a half baked db layer is just bad practice and causes everyone grief.

defines model field names like left(), right(), id(), but then doesn't use them everywhere, so you have a sprinkling of hard coded field names everywhere.

functions like pathId() use mysql specific and non-standard features, making it very difficult to port to anything else. Things like GROUP_CONCAT have equivalents, but referencing aggregates or aliases in the HAVING or GROUP BY is not allowed in the SQL standard. Had to rework with derived tables. NOT FUN.

Dealing with quoting:

unfortunately the mysql style quoting isn't used consistently, so you can't just replace backticks with double quotes, because in my queries the backticks are present on some fields and not on others.

can't do automatic search and replace of quotes (not consistent).

rewriting everything to have lower case fields and no quotes, woudl be the most consistent, clean and error free way to go, but lots of work.

correcting the code to use backticks consistently on all identifiers would enable auto translation, still lots of work.

adding a "quoteIdentifier()" method instead of using hard coded quotes, would be ideal, but ltos of work and you are just rewriting ORM features :(

replacing hard coded backticks with a static member that is the current quote to use for quoting identifiers, not great, still lots of work, but keeps the code as close to how it is now as possible.

don't use any quotes (neither backtick nor double quote)...this surprisingly might be the best way to go. PostgreSQL at least will always normalize to lower case, both when creating and when fetching, so you can write mixed case but in the db its lower case (always). As long as you don't try to have columns that differ only in case...your fine. Only hurdle is that the PHP code actually refers to stuff by case...but you could go through it and clean that up to use all lower.

can tell mysql to do it the same as postgresql: SET sql_mode='ANSI_QUOTES', then the quoting can be the same on both.

given all the options, and the potential of breaking one one of the databases, or some of the PHP code that expects mixed case...the best option seems to be to continue the quoting but correct the code to consistently quote identifiers with a quote character based on the correct database instead of being hard coded for mysql.

The postgresql setup code:

/*

Create Tables */ CREATE TABLE IF NOT EXISTS "PREFIX_permissions" ( "ID" SERIAL, "Lft" integer NOT NULL, "Rght" integer NOT NULL, "Title" varchar(64) NOT NULL, "Description" text NOT NULL, PRIMARY KEY ("ID", "Title", "Lft", "Rght") );

ALTER SEQUENCE "PREFIX_permissions_ID_seq" RESTART WITH 1;

CREATE TABLE IF NOT EXISTS "PREFIX_rolepermissions" ( "RoleID" integer NOT NULL, "PermissionID" integer NOT NULL, "AssignmentDate" integer NOT NULL, PRIMARY KEY ("RoleID","PermissionID") );

CREATE TABLE IF NOT EXISTS "PREFIX_roles" ( "ID" SERIAL, "Lft" integer NOT NULL, "Rght" integer NOT NULL, "Title" varchar(128) NOT NULL, "Description" text NOT NULL, PRIMARY KEY ("ID", "Title", "Lft", "Rght") );

ALTER SEQUENCE "PREFIX_roles_ID_seq" RESTART WITH 1;

CREATE TABLE IF NOT EXISTS "PREFIX_userroles" ( "UserID" integer NOT NULL, "RoleID" integer NOT NULL, "AssignmentDate" integer NOT NULL, PRIMARY KEY ("UserID","RoleID") );

/*

Insert Initial Table Data */ INSERT INTO "PREFIX_permissions" ("ID", "Lft", "Rght", "Title", "Description") VALUES (1, 0, 1, 'root', 'root');

INSERT INTO "PREFIX_rolepermissions" ("RoleID", "PermissionID", "AssignmentDate") VALUES (1, 1, (select extract(epoch from now())::int));

INSERT INTO "PREFIX_roles" ("ID", "Lft", "Rght", "Title", "Description") VALUES (1, 0, 1, 'root', 'root');

INSERT INTO "PREFIX_userroles" ("UserID", "RoleID", "AssignmentDate") VALUES (1, 1, (select extract(epoch from now())::int));

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/OWASP/rbac/issues/85, or mute the thread https://github.com/notifications/unsubscribe/ABVjW6D-QxEDQzhwDcuYiECs18em0ZWWks5qMGLfgaJpZM4I2xSS.

[dinobot71]

Sure thing, I've never actually done that before...I'm new to the whole github thing...give me a couple of days to get that done. I haven't reviewed the rest of your comments yet...busy with the day job :) Will reply back soon...

[lsantaniello]

Hi guys, How Can I download postgres compatibility version?

I downloaded phprbac-pgsql.zip but I don't understand how integrate it

If I use "Apply Patch", I receive "Patch format detection failed."

Thanks

[dinobot71]

Sorry I dropped off there, got maxed at work. I will still do a proper Git branch; but it will happen after October 7 (I’m just about to head out for vacation).

Meantime I will email Luca a copy of my version of the package (no patch required).

(^_^)/

Mike.

From: lsantaniello [mailto:notifications@github.com] Sent: Wednesday, September 14, 2016 6:20 AM To: OWASP/rbac Cc: Michael Garvin; Author Subject: Re: [OWASP/rbac] Postgres Patch (#85)

Hi guys, How Can I download postgres compatibility version?

I downloaded phprbac-pgsql.zip but I don't understand how integrate it

Thanks

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/OWASP/rbac/issues/85#issuecomment-246968740 , or mute the thread https://github.com/notifications/unsubscribe-auth/ARtHPL7oMnpPWLMYCn1mS2F1aU9vQpQeks5qp8o1gaJpZM4I2xSS .Image removed by sender.

[sandrobilbeisi]

any news on the Postgres patch ?

[dinobot71]

Oh! This completely fell of my radar, I do feel bad that I didn’t help out more on this one. Pretty busy at the moment, but I’ll keep this in my inbox and come back to it as soon as can. Stay tuned.

(^_^)/

Mike.

From: Sandro Bilbeisi [mailto:notifications@github.com] Sent: Tuesday, March 27, 2018 9:29 AM To: OWASP/rbac Cc: Michael Garvin; Author Subject: Re: [OWASP/rbac] Postgres Patch (#85)

any news on the Postgres patch ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/OWASP/rbac/issues/85#issuecomment-376524950 , or mute the thread https://github.com/notifications/unsubscribe-auth/ARtHPPerpNXsemC7YjL-3uH4aXZN-7Npks5tij6jgaJpZM4I2xSS . https://github.com/notifications/beacon/ARtHPFnOiaWbLEJIljDGFexjR5TLoAVyks5tij6jgaJpZM4I2xSS.gif

[seekyong]

HI , any news on the Postgres patch ? I need it , too.

[abiusx]

Unfortunately I don't think anyone is working on this right now.

On May 15, 2019, at 5:32 AM, seekyong notifications@github.com wrote:

HI , any news on the Postgres patch ? I need it , too.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/OWASP/rbac/issues/85?email_source=notifications&email_token=AAKWGWZ77RFZCPXQUBVAWM3PVPKBXA5CNFSM4CG3CSJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVOC3QI#issuecomment-492580289, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKWGW2HNUIP4PFNCK3JUT3PVPKBXANCNFSM4CG3CSJA.

[dinobot71]

Guys, sorry been a long time, I got maxed out and was never able to get back to this. I may still have the version of the code I modded around somewhere. I will try to make some time to dig for it. As I recall though, it wasn’t anything too serious, just basic PostgreSQL vs MySQL syntax for stuff like quoting…no basic logic changes to the tree code and stuff. Just straight porting stuff.

If you send me a link where I can drop off a zip file for you, I’ll just shoot you the code, you guys can then take it from there. Sorry I was never able to get back to this. Shoot me a drop off link , and I’ll try to hand off a modded version.

(^_^)/

Mike.

From: AbiusX notifications@github.com Sent: May 15, 2019 10:17 AM To: OWASP/rbac rbac@noreply.github.com Cc: Michael Garvin mgarvin@bell.net; Author author@noreply.github.com Subject: Re: [OWASP/rbac] Postgres Patch (#85)

Unfortunately I don't think anyone is working on this right now.

On May 15, 2019, at 5:32 AM, seekyong <notifications@github.com mailto:notifications@github.com > wrote:

HI , any news on the Postgres patch ? I need it , too.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <https://github.com/OWASP/rbac/issues/85?email_source=notifications https://github.com/OWASP/rbac/issues/85?email_source=notifications&email_token=AAKWGWZ77RFZCPXQUBVAWM3PVPKBXA5CNFSM4CG3CSJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVOC3QI#issuecomment-492580289 &email_token=AAKWGWZ77RFZCPXQUBVAWM3PVPKBXA5CNFSM4CG3CSJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVOC3QI#issuecomment-492580289>, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKWGW2HNUIP4PFNCK3JUT3PVPKBXANCNFSM4CG3CSJA.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/OWASP/rbac/issues/85?email_source=notifications&email_token=AENUOPFVMT2HO3TKS6DNNWLPVQLO7A5CNFSM4CG3CSJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVOZXCQ#issuecomment-492673930 , or mute the thread https://github.com/notifications/unsubscribe-auth/AENUOPGWFSQ4FACZ4Z35MTLPVQLO7ANCNFSM4CG3CSJA . https://github.com/notifications/beacon/AENUOPCRQ2V7BXUAJDJM5TLPVQLO7A5CNFSM4CG3CSJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVOZXCQ.gif

[dinobot71]

Guys, I've posted my PostgreSQL version here: https://www.dropbox.com/s/x9pvfjdu3k3n34z/phprbac.tar.gz?dl=0 you can grab the code and do as you will :) Sorry I never back to this...just got maxed out and was never able to come back to it though. The delta is just PostgreSQL vs MySQL quoting and basic stuff like that, nothing serious. If you dig in you'll see its not that big a delta. Anyways, code is posted, grab it if you want it.