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

Make federation of unlisted videos an instance-level server preference #2802

Merged
merged 5 commits into from
Jun 3, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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 @@ -137,6 +137,11 @@ export class EditCustomConfigComponent extends FormReactive implements OnInit, A
css: null
}
},
federation: {
videos: {
federateUnlisted: true
Copy link
Owner

Choose a reason for hiding this comment

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

Do you want admins to update this setting on the web interface? If no, you can remove this

Copy link
Contributor Author

@Tak Tak May 29, 2020

Choose a reason for hiding this comment

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

Is there any reason not to allow this? I added it to production.yaml as well, based on your earlier comment.

Copy link
Owner

Choose a reason for hiding this comment

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

Is there any reason not to allow this?

It subjective, but it's not a setting admins will update frequently. So I don't think it's very useful to put this in the admin interface. But you're the one who does, so you're the one who decides :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I guess the main benefit would be discoverability.
I'm open to whatever you think - if you feel like it doesn't belong in the web interface, I'm willing to do the rework.

Copy link
Owner

Choose a reason for hiding this comment

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

Then let's move this out of the web interface. Admins should read the config' file when they install peertube, and we'll mention this in the changelog. You could also mention this setting in https://docs.joinpeertube.org/#/admin-following-instances?id=being-followed-by-an-instance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, done

}
},
theme: {
default: null
},
Expand Down
5 changes: 5 additions & 0 deletions client/src/app/core/server/server.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ export class ServerService {
registeredExternalAuths: [],
registeredIdAndPassAuths: []
},
federation: {
videos: {
federateUnlisted: true
Tak marked this conversation as resolved.
Show resolved Hide resolved
}
},
theme: {
registered: [],
default: 'default'
Expand Down
4 changes: 4 additions & 0 deletions config/default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ plugins:
check_latest_versions_interval: '12 hours' # How often you want to check new plugins/themes versions
url: 'https://packages.joinpeertube.org'

federation:
videos:
federate_unlisted: true

cache:
previews:
size: 500 # Max number of previews you want to cache
Expand Down
4 changes: 4 additions & 0 deletions config/production.yaml.example
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ plugins:
check_latest_versions_interval: '12 hours' # How often you want to check new plugins/themes versions
url: 'https://packages.joinpeertube.org'

federation:
videos:
federate_unlisted: true


###############################################################################
#
Expand Down
10 changes: 10 additions & 0 deletions server/controllers/api/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ async function getConfig (req: express.Request, res: express.Response) {
registeredExternalAuths: getExternalAuthsPlugins(),
registeredIdAndPassAuths: getIdAndPassAuthPlugins()
},
federation: {
videos: {
federateUnlisted: CONFIG.FEDERATION.VIDEOS.FEDERATE_UNLISTED
}
},
theme: {
registered: getRegisteredThemes(),
default: defaultTheme
Expand Down Expand Up @@ -348,6 +353,11 @@ function customConfig (): CustomConfig {
theme: {
default: CONFIG.THEME.DEFAULT
},
federation: {
videos: {
federateUnlisted: CONFIG.FEDERATION.VIDEOS.FEDERATE_UNLISTED
Tak marked this conversation as resolved.
Show resolved Hide resolved
}
},
services: {
twitter: {
username: CONFIG.SERVICES.TWITTER.USERNAME,
Expand Down
20 changes: 18 additions & 2 deletions server/helpers/video.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {
import { Response } from 'express'
import { DEFAULT_AUDIO_RESOLUTION } from '@server/initializers/constants'
import { JobQueue } from '@server/lib/job-queue'
import { VideoTranscodingPayload } from '@shared/models'
import { VideoPrivacy, VideoTranscodingPayload } from '@shared/models'
import { CONFIG } from "@server/initializers/config"

type VideoFetchType = 'all' | 'only-video' | 'only-video-with-rights' | 'id' | 'none' | 'only-immutable-attributes'

Expand Down Expand Up @@ -96,12 +97,27 @@ function extractVideo (videoOrPlaylist: MVideo | MStreamingPlaylistVideo) {
: videoOrPlaylist
}

function isPrivacyForFederation (privacy: VideoPrivacy) {
const castedPrivacy = parseInt(privacy + '', 10)

return castedPrivacy === VideoPrivacy.PUBLIC ||
(CONFIG.FEDERATION.VIDEOS.FEDERATE_UNLISTED === true && castedPrivacy === VideoPrivacy.UNLISTED)
}

function getPrivaciesForFederation () {
return (CONFIG.FEDERATION.VIDEOS.FEDERATE_UNLISTED === true)
? [ { privacy: VideoPrivacy.PUBLIC }, { privacy: VideoPrivacy.UNLISTED } ]
: [ { privacy: VideoPrivacy.PUBLIC } ]
}

export {
VideoFetchType,
VideoFetchByUrlType,
fetchVideo,
getVideoWithAttributes,
fetchVideoByUrl,
addOptimizeOrMergeAudioJob,
extractVideo
extractVideo,
isPrivacyForFederation,
getPrivaciesForFederation
}
3 changes: 2 additions & 1 deletion server/initializers/checker-before-init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ function checkMissedConfig () {
'history.videos.max_age', 'views.videos.remote.max_age',
'rates_limit.login.window', 'rates_limit.login.max', 'rates_limit.ask_send_email.window', 'rates_limit.ask_send_email.max',
'theme.default',
'remote_redundancy.videos.accept_from'
'remote_redundancy.videos.accept_from',
'federation.videos.federate_unlisted'
]
const requiredAlternatives = [
[ // set
Expand Down
5 changes: 5 additions & 0 deletions server/initializers/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ const CONFIG = {
URL: config.get<string>('plugins.index.url')
}
},
FEDERATION: {
VIDEOS: {
get FEDERATE_UNLISTED () { return config.get<boolean>('federation.videos.federate_unlisted') }
Tak marked this conversation as resolved.
Show resolved Hide resolved
}
},
ADMIN: {
get EMAIL () { return config.get<string>('admin.email') }
},
Expand Down
16 changes: 4 additions & 12 deletions server/models/video/video.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ import { ModelCache } from '@server/models/model-cache'
import { buildListQuery, BuildVideosQueryOptions, wrapForAPIResults } from './video-query-builder'
import { buildNSFWFilter } from '@server/helpers/express-utils'
import { getServerActor } from '@server/models/application/application'
import { getPrivaciesForFederation, isPrivacyForFederation } from "@server/helpers/video"

export enum ScopeNames {
AVAILABLE_FOR_LIST_IDS = 'AVAILABLE_FOR_LIST_IDS',
Expand Down Expand Up @@ -864,10 +865,7 @@ export class VideoModel extends Model<VideoModel> {
id: {
[Op.in]: Sequelize.literal('(' + rawQuery + ')')
},
[Op.or]: [
{ privacy: VideoPrivacy.PUBLIC },
{ privacy: VideoPrivacy.UNLISTED }
]
[Op.or]: getPrivaciesForFederation()
},
include: [
{
Expand Down Expand Up @@ -1582,12 +1580,6 @@ export class VideoModel extends Model<VideoModel> {
return videos
}

private static isPrivacyForFederation (privacy: VideoPrivacy) {
const castedPrivacy = parseInt(privacy + '', 10)

return castedPrivacy === VideoPrivacy.PUBLIC || castedPrivacy === VideoPrivacy.UNLISTED
}

static getCategoryLabel (id: number) {
return VIDEO_CATEGORIES[id] || 'Misc'
}
Expand Down Expand Up @@ -1813,11 +1805,11 @@ export class VideoModel extends Model<VideoModel> {
}

hasPrivacyForFederation () {
return VideoModel.isPrivacyForFederation(this.privacy)
return isPrivacyForFederation(this.privacy)
}

isNewVideo (newPrivacy: VideoPrivacy) {
return this.hasPrivacyForFederation() === false && VideoModel.isPrivacyForFederation(newPrivacy) === true
return this.hasPrivacyForFederation() === false && isPrivacyForFederation(newPrivacy) === true
}

setAsRefreshed () {
Expand Down
5 changes: 5 additions & 0 deletions server/tests/api/check-params/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ describe('Test config API validators', function () {
css: 'body { background-color: red; }'
}
},
federation: {
videos: {
federateUnlisted: false
Tak marked this conversation as resolved.
Show resolved Hide resolved
}
},
theme: {
default: 'default'
},
Expand Down
9 changes: 9 additions & 0 deletions server/tests/api/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ function checkInitialConfig (server: ServerInfo, data: CustomConfig) {
expect(data.instance.customizations.css).to.be.empty
expect(data.instance.customizations.javascript).to.be.empty

expect(data.federation.videos.federateUnlisted).to.be.true

expect(data.services.twitter.username).to.equal('@Chocobozzz')
expect(data.services.twitter.whitelisted).to.be.false

Expand Down Expand Up @@ -112,6 +114,8 @@ function checkUpdatedConfig (data: CustomConfig) {
expect(data.instance.customizations.javascript).to.equal('alert("coucou")')
expect(data.instance.customizations.css).to.equal('body { background-color: red; }')

expect(data.federation.videos.federateUnlisted).to.be.false

expect(data.services.twitter.username).to.equal('@Kuja')
expect(data.services.twitter.whitelisted).to.be.true

Expand Down Expand Up @@ -238,6 +242,11 @@ describe('Test config', function () {
css: 'body { background-color: red; }'
}
},
federation: {
videos: {
federateUnlisted: false
}
},
theme: {
default: 'default'
},
Expand Down
52 changes: 51 additions & 1 deletion server/tests/api/videos/video-privacy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,29 @@ import { VideoPrivacy } from '../../../../shared/models/videos/video-privacy.enu
import {
cleanupTests,
flushAndRunMultipleServers,
getConfig,
getCustomConfig,
getVideosList,
getVideosListWithToken,
ServerInfo,
setAccessTokensToServers,
updateCustomConfig,
uploadVideo
} from '../../../../shared/extra-utils/index'
import { doubleFollow } from '../../../../shared/extra-utils/server/follows'
import { userLogin } from '../../../../shared/extra-utils/users/login'
import { createUser } from '../../../../shared/extra-utils/users/users'
import { getMyVideos, getVideo, getVideoWithToken, updateVideo } from '../../../../shared/extra-utils/videos/videos'
import { waitJobs } from '../../../../shared/extra-utils/server/jobs'
import { Video } from '@shared/models'
import { ServerConfig, Video } from '../../../../shared/models'
import { CustomConfig } from "../../../../shared/models/server"

const expect = chai.expect

describe('Test video privacy', function () {
let servers: ServerInfo[] = []
let anotherUserToken: string
let customConfig: CustomConfig

let privateVideoId: number
let privateVideoUUID: string
Expand All @@ -32,6 +37,7 @@ describe('Test video privacy', function () {
let internalVideoUUID: string

let unlistedVideoUUID: string
let nonFederatedUnlistedVideoUUID: string

let now: number

Expand All @@ -46,6 +52,11 @@ describe('Test video privacy', function () {

// Server 1 and server 2 follow each other
await doubleFollow(servers[0], servers[1])

{
const res = await getCustomConfig(servers[1].url, servers[1].accessToken)
customConfig = res.body
}
})

it('Should upload a private and internal videos on server 1', async function () {
Expand Down Expand Up @@ -164,6 +175,45 @@ describe('Test video privacy', function () {
}
})

it('Should upload a non-federating unlisted video to server 2', async function () {
this.timeout(30000)

customConfig.federation.videos.federateUnlisted = false
await updateCustomConfig(servers[1].url, servers[1].accessToken, customConfig)
Tak marked this conversation as resolved.
Show resolved Hide resolved
const resConfig = await getConfig(servers[1].url)
const serverConfig: ServerConfig = resConfig.body

expect(serverConfig.federation.videos.federateUnlisted).to.be.false

const attributes = {
name: 'unlisted video',
privacy: VideoPrivacy.UNLISTED
}
await uploadVideo(servers[1].url, servers[1].accessToken, attributes)

// Server 2 has transcoding enabled
await waitJobs(servers)
})

it('Should list my new unlisted video', async function () {
const res = await getMyVideos(servers[1].url, servers[1].accessToken, 0, 2)

expect(res.body.total).to.equal(2)
expect(res.body.data).to.have.lengthOf(2)

nonFederatedUnlistedVideoUUID = res.body.data[0].uuid
})

it('Should be able to get non-federated unlisted video from origin', async function () {
const res = await getVideo(servers[1].url, nonFederatedUnlistedVideoUUID)

expect(res.body.name).to.equal('unlisted video')
})

it('Should not be able to get non-federated unlisted video from federated server', async function () {
await getVideo(servers[0].url, nonFederatedUnlistedVideoUUID, 404)
})

it('Should update the private and internal videos to public on server 1', async function () {
this.timeout(10000)

Expand Down
5 changes: 5 additions & 0 deletions shared/extra-utils/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ function updateCustomSubConfig (url: string, token: string, newConfig: DeepParti
css: 'body { background-color: red; }'
}
},
federation: {
videos: {
federateUnlisted: false
}
},
theme: {
default: 'default'
},
Expand Down
6 changes: 6 additions & 0 deletions shared/models/server/custom-config.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ export interface CustomConfig {
}
}

federation: {
videos: {
federateUnlisted: boolean
}
}

theme: {
default: string
}
Expand Down
6 changes: 6 additions & 0 deletions shared/models/server/server-config.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ export interface ServerConfig {
registeredIdAndPassAuths: RegisteredIdAndPassAuthConfig[]
}

federation: {
videos: {
federateUnlisted: boolean
}
}

theme: {
registered: ServerConfigTheme[]
default: string
Expand Down
4 changes: 4 additions & 0 deletions support/docker/production/config/production.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,7 @@ tracker:

admin:
email: null

federation:
videos:
federate_unlisted: true