Fix AuditCustom event not respecting audit.enabled and audit.console

[rrodrigofranco]

Fixes #1016

The RecordCustomAudit listener was not checking the audit.enabled
and audit.console config values before processing custom audit events,
causing audits to be recorded even when auditing was explicitly disabled.

Changes

• Added isAuditingEnabled() to the Contracts\Auditable interface
• Added config check in RecordCustomAudit::handle() to return early
  when auditing is disabled
• Added tests covering both audit.enabled = false and console context
  without audit.console = true

[willpower232]

This is interesting, I wonder if there is actually an issue here

laravel-auditing/src/Auditable.php

Lines 256 to 267 in c516bdf

	public function readyForAuditing(): bool
	{
	if (static::$auditingDisabled || Models\Audit::$auditingGloballyDisabled) {
	return false;
	}
	if ($this->isCustomEvent) {
	return true;
	}
	return $this->isEventAuditable($this->auditEvent);
	}

@erikn69 presumably this needs to check self::isAuditingEnabled() because the other two disabled flags aren't related to the config?

[rrodrigofranco]

@willpower232 Good point. The two flags ($auditingDisabled / $auditingGloballyDisabled) are runtime flags set programmatically (e.g. via withoutAuditing()), while isAuditingEnabled() reflects the config state.

For regular auditing, the config check happens at boot time in bootAuditable(), the observer simply isn't registered if auditing is disabled, so readyForAuditing() is never reached.

For custom events there was no equivalent gate, which is exactly the bug this PR fixes, by checking isAuditingEnabled() in the listener before calling Auditor::execute().

Adding isAuditingEnabled() inside readyForAuditing() would provide defense-in-depth (e.g. if someone calls Auditor::execute() directly), but it would also change the semantics of that method beyond its current responsibility. Happy to add it if the maintainers think that's the right place.

[erikn69]

presumably this needs to check self::isAuditingEnabled() because the other two disabled flags aren't related to the config?

As I recall, that method was only for boot; it shouldn't be used at runtime. For runtime, there are $auditingDisabled and $auditingGloballyDisabled.

I can see the problem, but it would also happen in regular audits, since the event is being triggered manually.

Event::dispatch(new \OwenIt\Auditing\Events\DispatchAudit($article));

I think it needs a different approach, this has never been tested and apparently has always been present

[rrodrigofranco]

@erikn69 you're right, the same issue exists for DispatchAudit when dispatched manually. The root cause is that readyForAuditing() doesn't check the config, it only checks the runtime flags.

Would the preferred fix be to add the config check inside readyForAuditing() itself? Something like:

public function readyForAuditing(): bool
{
    if (static::$auditingDisabled || Models\Audit::$auditingGloballyDisabled) {
        return false;
    }

    if (App::runningInConsole() && ! Config::get('audit.console', false)) {
        return false;
    }

    if (! Config::get('audit.enabled', true)) {
        return false;
    }
    // ...
}

That would fix both AuditCustom and DispatchAudit in a single place, rather than patching each listener individually.

[erikn69]

As I recall, that method was only for boot; it shouldn't be used at runtime

Try #1050

[rrodrigofranco]

@erikn69 Thanks for the guidance and for opening #1050 with an alternative approach. I'll close this PR in favor of yours.