#47822 Improve logging of ACL decisions
Closed: wontfix 3 years ago by spichugi. Opened 9 years ago by abbra.

ACL plugin is painfully to debug as it doesn't give you any reason why decision was made.

Actually, it has internal logging of these decisions but the messages are suppressed when 389-ds is compiled on platforms where TNF library is not available.

However, the code in ACL plugin doesn't really use much of TNF functionality that couldn't be emulated with plain preprocessor magic.

Enable logging using slapi_log_error() for non-TNF case.


Attached is a prototype patch.

I've uploaded my version which is very similar to what Ludwig did but avoids pulling in formatting string into formatting string, i.e. avoids having
{{{
foo = sprintf("%s", tnf_string);
bar = sprintf(foo, data);
}}}
because this is a potential issue that is now detected by Fedora scripts.

ok, but I think we should have another log level, otherwise there will be surprises if acl loggin is logging much more than before.
Another question is, could this TNF -> logging transformation be useful in other places as well ?
Or could we remove this solaris specific feature and do normal logging ?

Both approaches would work for me.

In ACL plugin I specifically interested in getting information what caused evaluation to fail or to allow. These are currently displayed in TNF logging, especially in TestRights and not available on either level via normal slapi_log_error() calls.

Looking again into the TNF calls I am no longer convince that just replacing the TNF macros by calls to slapi_log_error is useful. The original use of TNF is to log traces about execution and so there are many calls just loging enter and exit of a function, this would blow up the volumnious acl logging even more and giv eno extra benefit.
The exception id in TestRights, since TNF is called at any return, and there are many places to return from TestRights this could give more information on the reason TestRights did decide to allow or deny acces.

I suggest not to automatically convert TNF calls into logging, but add more dedicated messages in TestRights, and check if there are other places, not yet covered by TNF or normal logging, which could be helpful.

maybe we close this ticket and open a new one.

changed subject according to latest comment

Metadata Update from @abbra:
- Issue assigned to lkrispen
- Issue set to the milestone: 1.3.7 backlog

7 years ago

Metadata Update from @mreynolds:
- Custom field reviewstatus adjusted to None
- Issue close_status updated to: None
- Issue set to the milestone: 1.4 backlog (was: 1.3.7 backlog)

4 years ago

Metadata Update from @mreynolds:
- Issue set to the milestone: 1.4.5 (was: 1.4 backlog)
- Issue tagged with: Access Control, RFE

3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1153

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix
- Issue status updated to: Closed (was: Open)

3 years ago

Login to comment on this ticket.

Metadata