Skip to content

feat: add more verbose audit logging to server apps - #65542

Open
GhassenKefi wants to merge 20 commits into
masterfrom
feat/admin-audit-server-apps
Open

feat: add more verbose audit logging to server apps#65542
GhassenKefi wants to merge 20 commits into
masterfrom
feat/admin-audit-server-apps

Conversation

@GhassenKefi

@GhassenKefi GhassenKefi commented Oct 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
  • Resolves: #

Summary

Add audit log entries for actions in The following apps, either by sending CriticalActionPerformedEvent from the app or by having admin_audit listen to existing events.

App Lists :

  • files
  • files_sharing (restore, link changes, remote shares)
  • federatedfilesharing (existing entries updated, plus revoke)
  • cloud_federation_api
  • files_external
  • encryption
  • federation (trusted server added, shared secret exchanged)
  • dav (calendar export, federated calendars, direct links, birthday calendars)
  • settings
  • provisioning_api
  • oauth2
  • twofactor_backupcodes
  • user_ldap
  • workflowengine
  • webhook_listeners
  • theming
  • updatenotification
  • appstore
  • systemtags (in core SystemTagManager: who may assign a tag)

TODO

  • ...

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@GhassenKefi GhassenKefi added this to the Nextcloud 36 milestone Oct 15, 2026
@GhassenKefi GhassenKefi self-assigned this Oct 15, 2026
@GhassenKefi
GhassenKefi requested review from icewind1991, leftybournes, provokateurin, sorbaugh and susnux and removed request for a team and sorbaugh October 15, 2026 09:44
@GhassenKefi
GhassenKefi force-pushed the feat/admin-audit-server-apps branch 2 times, most recently from f685ce8 to aee3d38 Compare October 16, 2026 11:28
@SebastianKrupinski

SebastianKrupinski commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Question...

What happens when one of these apps is disabled? files_external or files_sharing for instance...

Since the classes are unavailable when the app is disabled does this not crash the system?

@susnux

susnux commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Since the classes are unavailable when the app is disabled does this not crash the system?

Listeners are registered by class string so its not a problem if the class does not exist, as the listener will just never be called

@SebastianKrupinski

SebastianKrupinski commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Listeners are registered by class string so its not a problem if the class does not exist, as the listener will just never be called

That works then

On a side note I think this approache is unsatainable... I think each app/module should implement its own auditing... like the NC logger... (full transparancy this might already exist I just haven't looked in to it)

@susnux

susnux commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

On a side note I think this approache is unsatainable... I think each app/module should implement its own auditing... like the NC logger... (full transparancy this might already exist I just haven't looked in to it)

Yes I agree! @GhassenKefi apps should, as discussed, use the CriticalActionPerformedEvent.
The listeners in admin_audit app are only for actions that are not bound to an app like file system events or activity.
Basically everything that is done in OCP / OC / the lib (and maybe core) is fine as a listener in admin_audit.

All other app related logging should be done with CriticalActionPerformedEvent directly by that app.

@GhassenKefi
GhassenKefi force-pushed the feat/admin-audit-server-apps branch from aee3d38 to 9492e7b Compare October 19, 2026 08:57
@SebastianKrupinski

SebastianKrupinski commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

@GhassenKefi

Saw the most recent push. The CriticalActionPerformedEvent is a better way.

This also should probably be implemented for all crud and sharing operations in CalDAV and CardDAV.

  • Collection Create, Delete (Calendar/Address Book)
  • Entity Create, Update, Delete (Event/Card)
  • Delegation (Calendar Only atm)
  • Sharing (Calendar/Address Book)

@GhassenKefi
feat(admin_audit): log subadmin, two-factor provider and account changes
615cad1 
admin_audit now listens to core events that were dispatched but not
audited:

- SubAdminAddedEvent / SubAdminRemovedEvent (groups.subadmin.*)
- GroupChangedEvent for display name changes
- TwoFactorProviderForUserRegistered / Unregistered
  (auth.twofactor_provider.*)
- UserChangedEvent for display name, quota and managers

Display name values are not logged, only the fact that they changed.

Password changes are now logged for every user backend, not only the
database backend, so LDAP password renewals and admin resets of LDAP
passwords are audited too. The backend is added as a parameter.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(systemtags): log tag changes, deletions and restricted tag assig…
474dfbd 
…nments

Updating and deleting system tags is now audited through the existing
TagUpdatedEvent and TagDeletedEvent. Changing the groups that may
assign a restricted tag dispatches no event, so SystemTagManager
dispatches a CriticalActionPerformedEvent there.

Assigning and unassigning is only logged for restricted and invisible
tags, as those are the ones access control and retention rules rely
on. Ordinary user tags are not logged. Each entry carries the number
of objects and at most ten object IDs, so bulk assignments stay a
single short entry.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(files): log ownership transfers
c9a9d6c 
Requesting, accepting and rejecting an ownership transfer, and the
transfer itself, are now written to the audit log. The transfer entry
is dispatched by OwnershipTransferService, so it covers the background
job and occ files:transfer-ownership. Rejecting by dismissing the
notification is logged as well.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(files_sharing): log share accept, leave, restore and link changes
3736237 
- Accepting a share and leaving it are logged through the existing
  ShareAcceptedEvent and ShareDeletedFromSelfEvent.
- Restoring a deleted share is logged by DeletedShareAPIController.
- Share20\Manager::updateShare logs changes to the download restriction
  and share attributes, and a changed link token. The token itself is
  never logged.
- Accepting, declining and removing an incoming federated share is
  logged by the external share manager.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(federatedfilesharing): add operations and named parameters to au…
0c4b22d 
…dit entries

The existing audit entries of the legacy OCS endpoints now carry an
operation (federatedfilesharing.share.*) and named parameters, with the
messages unchanged. Revoking a reshare is logged as well.

Shares and notifications arriving through the generic OCM endpoints
are logged by cloud_federation_api, so nothing is logged twice.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(cloud_federation_api): log incoming federated shares and notific…
55c59ac 
…ations

Shares and notifications received on the OCM endpoints are now audited
for every resource type (files, calendars, Talk, ...), after the
provider has accepted them. The shared secret of a notification is not
logged.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(files_external): log storage and credential changes
c04b4e8 
Creating, updating and deleting external storages is now audited in
StoragesService, so admin and personal storages are covered for the web
UI and occ. Saving global or per-user credentials is logged by the
controllers. Backend options and credentials are never logged.

The existing Storage*Event classes are not public API and turn a mount
point change into a delete and a create, so they are not used for this.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(encryption): log recovery key and key management actions
2da2d3a 
Enabling and disabling the recovery key, changing its password,
changing a user's recovery setting, recovering a user's files,
regenerating a user's keys after an admin password reset, changing the
private key password and toggling home storage encryption are now
audited. Passwords and keys are never logged.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(federation): log trusted server changes
9f6a9a4 
Adding and removing a trusted server and exchanging a shared secret
with it are now written to the audit log by the federation app through
CriticalActionPerformedEvent. The shared secret is never logged.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
GhassenKefi force-pushed the feat/admin-audit-server-apps branch from 9492e7b to d09c987 Compare October 20, 2026 09:41
@GhassenKefi

GhassenKefi commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

@SebastianKrupinski This 2 were already implemented

  • Collection Create, Delete (Calendar/Address Book)
  • Sharing (Calendar/Address Book)

i added this in my last push

  • Delegation (Calendar Only atm)

you can find them all in this commit 52e0ad4c3db

as for

  • Entity Create, Update, Delete (Event/Card)

it was deliberately not logged, because every sync client produces them constantly. so it would flood the audit log.
maybe we can implemented for the deletes only?

@susnux susnux changed the title Feat/admin audit server apps feat: add more verbose audit logging to server apps Oct 21, 2026
susnux
susnux approved these changes Oct 21, 2026
View reviewed changes

@susnux susnux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much better 👍

@SebastianKrupinski

SebastianKrupinski commented Oct 21, 2026

Copy link
Copy Markdown
Contributor
  • Entity Create, Update, Delete (Event/Card)

it was deliberately not logged, because every sync client produces them constantly. so it would flood the audit log. maybe we can implemented for the deletes only?

A sync client should only retrieve data, it should not create, update or delete unless a user strictly preforms an action, in calendar or contacts. I don't think these actions would cause a lot of audit log spam.

At the minimum we should log deletes.

@GhassenKefi
feat(dav): log calendar and address book sharing and lifecycle
0dc4fcf 
A new dav listener writes calendar and address book creation, trashing,
deletion, restore, publishing and share changes to the audit log
through CriticalActionPerformedEvent, based on the existing dav events.
The public link of a published calendar is not logged.

Calendar export, accepting and declining federated calendars, creating
direct download links and enabling or disabling birthday calendars are
logged where they happen. Direct link tokens are never logged.

Granting and revoking calendar delegation (read or write access to all
calendars of a user, set through the calendar-proxy-read and
calendar-proxy-write principals) is logged once per delegate whose
access actually changed. Setting the same access again is not logged.

Events and contacts created, updated, deleted, moved to the trash,
restored or moved to another calendar or address book are logged with
the object URI, never their content. The birthday calendar and the
system address book are skipped, as the server generates them from
contacts and user accounts; subscribed calendars dispatch separate
events and are not logged either.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(settings): log credential, security and server settings changes
3467be8 
Audit entries are added for app password creation, deletion and scope
changes, remote wipe requests, WebAuthn devices, two-factor
enforcement, mail server settings and credentials, admin delegation,
presets, declarative settings and log file downloads.

The existing AI settings entry keeps its message and now carries an
operation (settings.ai.updated) and named parameters.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(provisioning_api): log app config changes and edits of other acc…
22bd90d 
…ounts

Setting and deleting app config values through the API is now audited,
with the app and key only, since app config can hold secrets. Remote
wipe of a user's devices, resending the welcome email and edits of
another user's account are logged too. For account edits only the
field name is logged, and fields already audited by core are skipped.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(oauth2): log OAuth2 client creation and deletion
2cfce59 
ClientService now audits adding and removing OAuth2 clients, which
covers the admin settings and occ. The client secret is never logged.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(twofactor_backupcodes): log backup code generation
128d136 
Generating backup codes, which replaces any previous codes, is now
audited. The codes are never logged.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(user_ldap): log LDAP configuration changes
43342fc 
Creating, updating, copying and deleting LDAP configurations through
the API, revealing the bind password and clearing user or group
mappings are now audited. For updates only the names of the changed
keys are logged, never their values.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(workflowengine): log flow rule changes
ecfc70b 
Creating, updating and deleting flow rules is now audited in the
manager, for both admin and user scope. Check values are not logged.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(webhook_listeners): log webhook registration changes
eb18dc4 
Creating, updating and deleting webhooks is now audited. Only the
origin of the target URI is logged, since webhook URIs often contain
secrets in their path or query. Headers and auth data are never logged.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(theming): log theming changes
08b2cb8 
Changing, resetting and uploading theming settings and images through
the web UI is now audited. Only the setting name is logged.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(updatenotification): log update channel changes and updater tokens
3a8d860 
Changing the update channel and creating a web updater token are now
audited. The token is never logged.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
feat(appstore): log app uninstallation
0d1b6e8 
Removing an app from the app store is now audited, next to the
existing enable, disable and update entries.

Assisted-by: ClaudeCode:claude-opus-5-5
Signed-off-by: Ghassen kefi <ghassen.kefi.dev@gmail.com>
@GhassenKefi
GhassenKefi force-pushed the feat/admin-audit-server-apps branch from d09c987 to 0d1b6e8 Compare October 21, 2026 16:27
@GhassenKefi

GhassenKefi commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@SebastianKrupinski event and contact create, update and delete are now logged in the same dav commit, with server-generated data (birthday calendar, system address book, subscriptions) excluded.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@susnux susnux susnux approved these changes
@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst ChristophWurst is a code owner
@SebastianKrupinski SebastianKrupinski Awaiting requested review from SebastianKrupinski SebastianKrupinski is a code owner
@tcitworld tcitworld Awaiting requested review from tcitworld tcitworld is a code owner
@miaulalala miaulalala Awaiting requested review from miaulalala miaulalala is a code owner
@icewind1991 icewind1991 Awaiting requested review from icewind1991 icewind1991 is a code owner automatically assigned from nextcloud/server-backend
@leftybournes leftybournes Awaiting requested review from leftybournes leftybournes is a code owner automatically assigned from nextcloud/server-backend
@provokateurin provokateurin Awaiting requested review from provokateurin provokateurin is a code owner automatically assigned from nextcloud/server-backend

At least 2 approving reviews are required to merge this pull request.

Assignees

@GhassenKefi GhassenKefi

Labels

3. to review Waiting for reviews AI assisted enhancement feature: admin audit

Projects

None yet

Milestone

Nextcloud 36

Development

Successfully merging this pull request may close these issues.

3 participants

@GhassenKefi @SebastianKrupinski @susnux