Open kroder opened 10 years ago
It is not wise to include code for setup integrity in a library. Maybe in a third party file, but the actual library does not need to know that.
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 Jun 5, 2014, at 10:26 AM, kroder notifications@github.com wrote:
I think it would be more convenient to work with composer when I can store config file as part of my repository (outside /vendor dir, which is not included). As example: phinx library.
And another one about installation: if i already have database (without rbac tables) install script just telling me "ok, congrats...", but actually it wasn't, because mysql file was not executed.
— Reply to this email directly or view it on GitHub.
Not sure that we understand each other. I'm talking about already installed library in my project through composer 'require' and hardcoded dependence by database.config file path in Rbac() constructor. Optional db params in Rbac() constructor instead of config file (or changeable path to this file) could add some flexibility, as for me.
The part about installation was apart of this question. It's just good to describe somewhere that provided gui installer will check existence of database and will not create tables if db already exists.
oh ok! Jesse is in charge of composer and installation stuff. Thanks
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 Jun 5, 2014, at 12:38 PM, kroder notifications@github.com wrote:
Not sure that we understand each other. I'm talking about already installed library in my project through composer 'require' and hardcoded dependence by database.config file path in Rbac() constructor. Optional db params in Rbac() constructor instead of config file (or changeable path to this file) could add some flexibility, as for me.
The part about installation was apart of this question. It's just good to describe somewhere that provided gui installer will check existence of database and will not create tables if db already exists.
— Reply to this email directly or view it on GitHub.
Hi kroder.
Looks like you're trying to achieve the same thing I was looking for. See the discussion here: #40.
Yeah, thank you for reply! Now it's become more clearer and I understand that authors are offering to solve this by yourself.
@kroder - I'm working on updating the GUI installer, which include a lot more checks and balances. And regarding your composer comment, could you please explain this config file 'outside the repo' that you mention? What info would you have in this config? How would it be used?
I'm pretty new to using composer to distribute packages. Until this release I have only used composer to install dependencies, so I'll have to look into installing specific/individual files outside of the 'vendor' folder. But first I need to know what this config file would be used for.
And regarding database connections and similar issues, we will be rewriting the core library and it's public interface. I have idea's that will make the code base more decoupled allowing the use of Interfaces that can be implemented and expanded upon. This will allow more flexibility regarding database connections and library initialization.
The core functionality/implementation regarding NIST Level 2 Hierarchical RBAC, namely the database structure and the optimized db queries used to query and manipulate the data won't change much.
But the ease of use should and flexibility of the library should have a lot of improvements in PHP-RBAC v3.x.
I just can't say when we'll have that done at the moment.
Speaking for myself, due to personal life I've been pretty strapped for time this past month or so, and I need to finish up a few tasks before I really get my hands dirty with v3.x. But I'll be knocking code out when I get the chance :-)
I do want to thank everyone for the feedback, that's the best way to know and understand how users are using the library, and how we can make improvements that will make this a better library.
@jburns131 @kroder I'm guessing this is on the back burner at this point, but…
The config file 'outside the repo' issue has a couple of layers:
composer update
my project to get new versions of my dependencies, it chokes because there is a mismatch between the version of PhpRbac in vendor/
and the git repo (the difference: my edited database.config file). A hassle.vendor/
in my .gitignore file so that it doesn't get tracked in my repo. Which means that any edits I make to PhpRbac in vendor/
won't be reflected in my own project… which means that I'll have re-edit PhpRbac every time I clone my project repo. …Or I have to fork PhpRbac and keep pulling down any updates from OWASP. A hassle.Usually the RBAC hierarchies are not the only thing in my database, so I probably have another data model hanging around in my project. Rather than replicating the model independently in PhpRbac, it would be keen to pass a PDO or mysqli (or whatever) object to the Rbac()
constructor to provide it with access to the database where the RBAC hierarchies are stored.
Seems to me that the simplest solution would be to allow for a database object to to be passed to the Rbac()
constructor?
Not sure how to upvote the issue, but totally agree with @battis and @kroder. Would be great if one could pass own database configuration to the lib.
I was also having issues with the fact that there's the need to manually (and not programmatically) pass on the database configuration.
Here's my commit to solve temp the issue: https://github.com/sergiodlopes/rbac/commit/bbcc3d99be030fff78a96de30676a35502c287d6 Just pass the parameters into the constructor, falls back to default configuration variables in file database.config. Hopefully this is a temp fix. Looking forward for version 3 with this issue solved.
Any feedback is welcome.
Cool. Thanks!
+1 for @sergiodlopes, that solution works great!
Thank you again @sergiodlopes :-)
I just need to review all PR's, Issues and discussion concerning the database connection values. I'll get back ASAP regarding the best way to merge this PR into the v2.x branch.
This weekend is going to be a bit hectic for me, but I'll do my best to get in front of my keyboard as much as I can ;-)
+1 for this.
It would be great to be able to specify the location of the configuration file. This is especially useful when dealing with multiple environments (I.e dev, staging and prod).
Absolutely. Just ran into this now. The user db config should not be sitting in the vendor library.
@sergiodlopes solution is a good fix - even better would be to accept an existing PDO object. I already have a connection open - I don't want or need another one. It could then also take advantage of persistent connections (sort of like connection pooling).
Lastly, maybe this will help somebody - if you're using composer, you can use a custom fork like Sergio's in your composer.json:
"repositories": [
{
"type": "vcs",
"url": "https://github.com/sergiodlopes/rbac",
}
],
"require": {
"owasp/phprbac": "dev-master"
}
I think it would be more convenient to work with composer when I can store config file as part of my repository (outside /vendor dir, which is not included). As example: phinx library.
And another one about installation: if i already have database (without rbac tables) install script just telling me "ok, congrats...", but actually it wasn't, because mysql file was not executed.