OraOpenSource / Logger

Logger is used by Oracle developers to instrument their PL/SQL code
http://www.oraopensource.com/logger/
MIT License
310 stars 119 forks source link

Seperate admin calls into seperate package #60

Open martindsouza opened 9 years ago

martindsouza commented 9 years ago

From @hkpatv on January 7, 2014 9:53

We have logger installed into dedicated schema (UTL_LOGGER), and have locked this schema for security. I believe my options for changing settings (via logger.set_level) are as follows: 1) For every change required, unlock UTL_LOGGER, login as that user, make the change, log out and then relock the user. Note, this is quite a lot of manual effort and leaves open the possibility of errors (forget to relock the user) 2) Use the PROTECT_ADMIN_PROCS options to allow all users to make changes via logger.set_level. This is not ideal, as logger mechanism is entirely unsecured, all users can make changes.

Suggest a better way would be to separate the admin functions into a seperate package (LOGGER_CONFIG/LOGGER_ADMIN/????). Execute permission could then be granted or revoked on this package using standard Oracle security. I think in that case, the PROTECT_ADMIN_PROCS mechanism could be removed, simplifying the code.

Copied from original issue: tmuth/Logger---A-PL-SQL-Logging-Utility#59

martindsouza commented 9 years ago

I understand your issue. Since you're using this in a dedicated schema for your entire database is it an issue that people are setting/unsetting it often?

If the issue is on a dev machine I'd recommend leaving it unlocked and let developers have control of it. That being said it should probably remain in debug mode for dev instances.

If the issue is on prod machines I completely agree that you don't want everyone changing the system level configurations. Perhaps instead they can turn on client specific logging levels (see http://www.talkapex.com/2013/05/logger-200-enable-session-specific.html) Though currently this still requires admin access.

I'm hesitant to create an entirely new package/procedure for admin functions but am open to removing the admin check on client specific logging. Would that work for you?

If you still need more specific fine grained security perhaps you could leave PROTECT_ADMIN_PROCS on. In UTL_LOGGER you can then create a custom package to manage all the admin calls and then grant execute access to specific users. You would only need to do this once and is fairly easy to implement.

Please let me know what you do and if you want to move the client specific logging outside of admin privileges.

martindsouza commented 9 years ago

From @hkpatv on June 16, 2014 6:22

Yes, I have set at DEBUG level on non-production databases, and INFO on production databases.

We should not change this very often (if ever), if I do, then I normally update the logger_prefs table directly. Although that is not ideal, as I don't think there is any validation of value using that method.

Yes you are correct about creating a separate package, I hadn't considered that.

Removing admin check for client logging would probably get me what I want, ie allow developers to change logging for testing without allowing change to entire database.

Could you a little explain the reasoning for not wanting an extra package. Is it because normally people will install to application schema and you want to keep namespace pollution to minimum? It's just to me that the PROTECT_ADMIN_PROCS mechanism seems (slightly) complex and unnecessary for something that can be handled with standard database security. Maybe my use case is slightly different with having a dedicated logging schema shared by multiple applications.

martindsouza commented 9 years ago

Starting in 2.2.0 I have changed it that admin_security_checks get bypassed for set_level when setting it for a client specific session. This allows developers to change 1 session but not affect the entire database. I think this is a happy medium.

Before introducing another package I really want to see why I want to do it. It's also another package to support etc. We may introduce another package in the near future but for very specific reasons.