Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to disable audit logs #6004

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,27 @@ <h2 i18n class="inner-form-title">CUSTOMIZATIONS</h2>
</div>
</div>

<div class="row mt-4"> <!-- cache grid -->
Copy link
Owner

Choose a reason for hiding this comment

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

You can remove HTML comments

<div class="col-12 col-lg-4 col-xl-3">
<div class="anchor" id="customizations"></div> <!-- customizations anchor -->
<h2 i18n class="inner-form-title">LOGS</h2>
<div i18n class="inner-form-description">
Copy link
Owner

Choose a reason for hiding this comment

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

The description of this block is invalid

Slight modifications to your PeerTube instance for when creating a plugin or theme is overkill.
</div>
</div>

<div class="col-12 col-lg-8 col-xl-9">
<ng-container formGroupName="instance">
<ng-container formGroupName="logs">
<ng-container formGroupName="auditLogs">
<div class="form-group">
<my-peertube-checkbox inputName="auditLogsEnabled" formControlName="enabled" i18n-labelText
labelText="Enable audit logs"></my-peertube-checkbox>
</div>
</ng-container>
</ng-container>
</ng-container>
</div>
</div>

</ng-container>
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ export class EditCustomConfigComponent extends FormReactive implements OnInit {
customizations: {
javascript: null,
css: null
},
logs: {
auditLogs: {
enabled: null
}
}
},
theme: {
Expand Down
7 changes: 5 additions & 2 deletions client/src/app/+admin/system/logs/logs.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
</div>

<ng-select
*ngIf="!isAuditLog() || isAuditLogsEnabled"
[(ngModel)]="startDate"
(ngModelChange)="refresh()"
[clearable]="false"
Expand All @@ -30,14 +31,16 @@

<my-select-tags *ngIf="!isAuditLog()" i18n-placeholder placeholder="Filter logs by tags" [(ngModel)]="tagsOneOf" (ngModelChange)="refresh()"></my-select-tags>

<my-button i18n-label label="Refresh" icon="refresh" (click)="refresh()"></my-button>
<my-button *ngIf="!isAuditLog() || isAuditLogsEnabled" i18n-label label="Refresh" icon="refresh" (click)="refresh()"></my-button>
</div>

<div class="logs">
<div *ngIf="loading" i18n>Loading...</div>

<div #logsElement>
<div *ngIf="!loading && logs.length === 0" i18n>No log.</div>
<div *ngIf="isAuditLog && !isAuditLogsEnabled" i18n>Audit logs disabled.</div>
Copy link
Owner

Choose a reason for hiding this comment

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

isAuditLog is a function so isAuditLog()

Also transform isAuditLogsEnabled to a function. is.. is for functions, if you still want to have a variable then prefer auditLogsEnabled. But it's better to have a function to not mix variables/functions if we can avoid


<div *ngIf="!loading && logs.length === 0 && isAuditLogsEnabled" i18n>No log.</div>
Copy link
Owner

Choose a reason for hiding this comment

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

If audit logs is disabled but you're on the normal logs, you'll never see this message

I suggest to simplify lines 41/43 using an Angular else for example (not easy to use, but will be simpler in the future with the new Angular markup language: https://blog.angular.io/meet-angulars-new-control-flow-a02c6eee7843).


<div *ngFor="let log of logs" class="log-row" [ngClass]="{ error: log.level === 'error', warn: log.level === 'warn' }">
<span class="log-level">{{ log.level }}</span>
Expand Down
18 changes: 14 additions & 4 deletions client/src/app/+admin/system/logs/logs.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Component, ElementRef, OnInit, ViewChild } from '@angular/core'
import { LocalStorageService, Notifier } from '@app/core'
import { ServerLogLevel } from '@peertube/peertube-models'
import { LocalStorageService, Notifier, ServerService } from '@app/core'
import { HTMLServerConfig, ServerLogLevel } from '@peertube/peertube-models'
import { LogRow } from './log-row.model'
import { LogsService } from './logs.service'

Expand All @@ -25,13 +25,20 @@ export class LogsComponent implements OnInit {
logType: 'audit' | 'standard'
tagsOneOf: string[] = []

serverConfig: HTMLServerConfig
isAuditLogsEnabled: boolean

constructor (
private logsService: LogsService,
private notifier: Notifier,
private localStorage: LocalStorageService
private localStorage: LocalStorageService,
private serverService: ServerService
) { }

ngOnInit (): void {
this.serverConfig = this.serverService.getHTMLConfig()
this.isAuditLogsEnabled = this.serverConfig.instance.logs.auditLogs.enabled

this.buildTimeChoices()
this.buildLevelChoices()
this.buildLogTypeChoices()
Expand All @@ -55,7 +62,10 @@ export class LogsComponent implements OnInit {
const tagsOneOf = this.tagsOneOf.length !== 0
? this.tagsOneOf
: undefined

if (!this.isAuditLogsEnabled) {
this.loading = false
return
}
this.logsService.getLogs({
isAuditLog: this.isAuditLog(),
level: this.level,
Expand Down
3 changes: 3 additions & 0 deletions config/default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,9 @@ instance:
securitytxt: |
Contact: https://github.com/Chocobozzz/PeerTube/blob/develop/SECURITY.md
Expires: 2025-12-31T11:00:00.000Z'
logs:
Copy link
Owner

Choose a reason for hiding this comment

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

You already have a log section: https://github.com/Chocobozzz/PeerTube/blob/develop/config/default.yaml#L230

Please also add this block to production.yaml.example

audit_logs:
enabled: true

services:
# Cards configuration to format video in Twitter
Expand Down
6 changes: 6 additions & 0 deletions packages/models/src/server/custom-config.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ export interface CustomConfig {
javascript?: string
css?: string
}

logs: {
auditLogs: {
enabled: boolean
}
}
}

theme: {
Expand Down
5 changes: 5 additions & 0 deletions packages/models/src/server/server-config.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ export interface ServerConfig {
javascript: string
css: string
}
logs: {
auditLogs: {
enabled: boolean
}
}
}

search: {
Expand Down
6 changes: 6 additions & 0 deletions packages/server-commands/src/server/config-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,12 @@ export class ConfigCommand extends AbstractCommand {
customizations: {
javascript: 'alert("coucou")',
css: 'body { background-color: red; }'
},

logs: {
auditLogs: {
enabled: true
}
}
},
theme: {
Expand Down
5 changes: 5 additions & 0 deletions packages/tests/src/api/check-params/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ describe('Test config API validators', function () {
customizations: {
javascript: 'alert("coucou")',
css: 'body { background-color: red; }'
},
logs: {
auditLogs: {
enabled: true
}
}
},
theme: {
Expand Down
5 changes: 5 additions & 0 deletions packages/tests/src/api/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,11 @@ const newCustomConfig: CustomConfig = {
customizations: {
javascript: 'alert("coucou")',
css: 'body { background-color: red; }'
},
logs: {
auditLogs: {
enabled: true
}
}
},
theme: {
Expand Down
35 changes: 35 additions & 0 deletions packages/tests/src/api/server/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,41 @@ describe('Test logs', function () {
expect(logsString.includes('video 10')).to.be.true
expect(logsString.includes('video 11')).to.be.false
})

it('Should refuse to create logs if disabled', async function () {
this.timeout(100000)

await server.config.updateCustomSubConfig({
newConfig: {
instance: {
logs: {
auditLogs:{
enabled: false
}
}
}
}
})

await server.videos.upload({ attributes: { name: 'video 12' } })
await waitJobs([ server ])
Copy link
Owner

Choose a reason for hiding this comment

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

Just upload one video to test audit logs. Test 403 if disabled, but also the audit log was not filled when it was disabled (so re-enable the feature and check the audit log doesn't contain your upload)


const now1 = new Date()

await server.videos.upload({ attributes: { name: 'video 13' } })
await waitJobs([ server ])

const now2 = new Date()

await server.videos.upload({ attributes: { name: 'video 14' } })
await waitJobs([ server ])

await logsCommand.getAuditLogs({
startDate: now1,
endDate: now2,
expectedStatus: HttpStatusCode.FORBIDDEN_403
})
})
})

describe('When creating log from the client', function () {
Expand Down
6 changes: 6 additions & 0 deletions server/core/controllers/api/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,12 @@ function customConfig (): CustomConfig {
customizations: {
css: CONFIG.INSTANCE.CUSTOMIZATIONS.CSS,
javascript: CONFIG.INSTANCE.CUSTOMIZATIONS.JAVASCRIPT
},

logs: {
auditLogs: {
enabled: CONFIG.INSTANCE.LOGS.AUDIT_LOGS.ENABLED
}
}
},
theme: {
Expand Down
1 change: 1 addition & 0 deletions server/core/helpers/audit-logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ const customConfigKeysToKeep = new Set([
'instance-defaultNSFWPolicy',
'instance-customizations-javascript',
'instance-customizations-css',
'instance-logs-auditLogs-enabled',
'services-twitter-username',
'services-twitter-whitelisted',
'cache-previews-size',
Expand Down
2 changes: 1 addition & 1 deletion server/core/initializers/checker-before-init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function checkMissedConfig () {
'client.videos.miniature.prefer_author_display_name', 'client.menu.login.redirect_on_single_external_auth',
'defaults.publish.download_enabled', 'defaults.publish.comments_enabled', 'defaults.publish.privacy', 'defaults.publish.licence',
'instance.name', 'instance.short_description', 'instance.description', 'instance.terms', 'instance.default_client_route',
'instance.is_nsfw', 'instance.default_nsfw_policy', 'instance.robots', 'instance.securitytxt',
'instance.is_nsfw', 'instance.default_nsfw_policy', 'instance.robots', 'instance.securitytxt', 'instance.logs.audit_logs.enabled',
'services.twitter.username', 'services.twitter.whitelisted',
'followers.instance.enabled', 'followers.instance.manual_approval',
'tracker.enabled', 'tracker.private', 'tracker.reject_too_many_announces',
Expand Down
7 changes: 6 additions & 1 deletion server/core/initializers/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,12 @@ const CONFIG = {
get CSS () { return config.get<string>('instance.customizations.css') }
},
get ROBOTS () { return config.get<string>('instance.robots') },
get SECURITYTXT () { return config.get<string>('instance.securitytxt') }
get SECURITYTXT () { return config.get<string>('instance.securitytxt') },
LOGS: {
AUDIT_LOGS:{
get ENABLED () { return config.get<boolean>('instance.logs.audit_logs.enabled') }
}
}
},
SERVICES: {
TWITTER: {
Expand Down
5 changes: 5 additions & 0 deletions server/core/lib/server-config-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ class ServerConfigManager {
customizations: {
javascript: CONFIG.INSTANCE.CUSTOMIZATIONS.JAVASCRIPT,
css: CONFIG.INSTANCE.CUSTOMIZATIONS.CSS
},
logs: {
auditLogs: {
enabled: CONFIG.INSTANCE.LOGS.AUDIT_LOGS.ENABLED
}
}
},
search: {
Expand Down
1 change: 1 addition & 0 deletions server/core/middlewares/validators/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const customConfigUpdateValidator = [
body('instance.defaultClientRoute').exists(),
body('instance.customizations.css').exists(),
body('instance.customizations.javascript').exists(),
body('instance.logs.auditLogs.enabled').exists(),

body('services.twitter.username').exists(),
body('services.twitter.whitelisted').isBoolean(),
Expand Down
2 changes: 2 additions & 0 deletions server/core/middlewares/validators/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ const getAuditLogsValidator = [
(req: express.Request, res: express.Response, next: express.NextFunction) => {
if (areValidationErrors(req, res)) return

if (!CONFIG.INSTANCE.LOGS.AUDIT_LOGS.ENABLED) return res.sendStatus(HttpStatusCode.FORBIDDEN_403)

return next()
}
]
Expand Down
16 changes: 16 additions & 0 deletions support/doc/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7935,6 +7935,14 @@ components:
type: string
css:
type: string
logs:
type: object
properties:
auditLogs:
type: object
properties:
enabled:
type: boolean
search:
type: object
properties:
Expand Down Expand Up @@ -8252,6 +8260,14 @@ components:
type: string
css:
type: string
logs:
type: object
properties:
auditLogs:
type: object
properties:
enabled:
type: boolean
theme:
type: object
properties:
Expand Down