From Bugzilla Helper: User-Agent: Mozilla/5.0 Galeon/1.2.9 (X11; Linux i686; U;) Gecko/20030314 Description of problem: There is quiet alot of code in before/afterSave methods that accesses the Kernel.getContext().getParty() methods, to either record auditing information (eg creation user) or to validate actions (DML permissions checking). No problem there you may think. However, with the new behaviour of persistence wrt to flushing changes to the DB we can no longer guarentee the point in time at which the before/after save methods will be invoked. Specifically, if we are running as a particular 'effective party' there is no guarentee that the session will be flushed before we switch back to the original effective party. This results in both privilege escalation & de-escalation on permissions checks & wildly inaccurate auditing data. The following example JSP illustrates a few of the problems with auditing: <jsp:root xmlns:jsp="http://java.sun.com/JSP/Page" xmlns:define="/WEB-INF/bebop-define.tld" xmlns:show="/WEB-INF/bebop-show.tld" version="1.2"> <jsp:directive.page import="com.arsdigita.kernel.*"/> <jsp:directive.page import="com.arsdigita.auditing.*"/> <jsp:directive.page import="com.arsdigita.persistence.*"/> <jsp:declaration> AuditedACSObject bar; AuditedACSObject foo; AuditedACSObject eek; AuditedACSObject wizz; </jsp:declaration> <jsp:scriptlet> KernelExcursion test1 = new KernelExcursion() { public void excurse() { foo = new AuditedACSObject("com.arsdigita.kernel.ACSObject") {}; setParty(Kernel.getSystemParty()); bar = new AuditedACSObject("com.arsdigita.kernel.ACSObject") {}; } }; test1.run(); System.err.println("Foo: " + (foo.getCreationUser() == null ? "none" : foo.getCreationUser().getDisplayName())); System.err.println("Bar: " + (bar.getCreationUser() == null ? "none" : bar.getCreationUser().getDisplayName())); KernelExcursion test2 = new KernelExcursion() { public void excurse() { eek = new AuditedACSObject("com.arsdigita.kernel.ACSObject") {}; setParty(Kernel.getSystemParty()); wizz = new AuditedACSObject("com.arsdigita.kernel.ACSObject") {}; SessionManager.getSession().flushAll(); } }; test2.run(); System.err.println("Eek: " + (eek.getCreationUser() == null ? "none" : eek.getCreationUser().getDisplayName())); System.err.println("Wizz: " + (wizz.getCreationUser() == null ? "none" : wizz.getCreationUser().getDisplayName())); </jsp:scriptlet> </jsp:root> The only solution I can think of is to ensure that Session#flushAll() is called every time KernelContext#party is changed. Version-Release number of selected component (if applicable): How reproducible: Always Steps to Reproduce: 1. Save the above JSP 2. Visit it without first logging in 3. Watch stderr.log Actual Results: Foo: none Bar: none Eek: Administrator Account Wizz: Administrator Account Expected Results: Foo: none Bar: Administrator Account Eek: none Wizz: Administrator Account Additional info:
In principal, this change is correct, however, existing code has problems, especially in handling of new objects. I don't think we can effectively hunt down every case. The best plan I can come up with: - there is a distinction between getEffectiveParty and getParty. For auditing purposes, getParty should be used. Only login/request processing code should be calling setParty. This should mitigate some of the problem. - the safe thing to do is to force your changes to be saved with appropriate calls to save() - I will add appropriate calls to flushAll for the request handling code that does call getParty
my plan was implemented on 6.0.x (38819) and dev (38822)
Shouldn't we rather add the flushAll() to the KernelExcursion class itself, otherwise anyone else using KernelExcursion potentially has the same problems found in BaseServlet & SiteNodeDispatcher.
We would rather do that, but it breaks things (some tests at the very least). See my first comment. This bug should stay alive so we can look for better solutions.