From c22419dd265c0c7185bf4197a1cb286eb3d8ebc0 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Mon, 17 Dec 2018 13:38:01 +0100 Subject: [PATCH 01/10] Fix adding captions to a video --- .../+video-edit/shared/video-caption-add-modal.component.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/client/src/app/videos/+video-edit/shared/video-caption-add-modal.component.ts b/client/src/app/videos/+video-edit/shared/video-caption-add-modal.component.ts index eaf819726..1413e7262 100644 --- a/client/src/app/videos/+video-edit/shared/video-caption-add-modal.component.ts +++ b/client/src/app/videos/+video-edit/shared/video-caption-add-modal.component.ts @@ -72,8 +72,6 @@ export class VideoCaptionAddModalComponent extends FormReactive implements OnIni } async addCaption () { - this.hide() - const languageId = this.form.value[ 'language' ] const languageObject = this.videoCaptionLanguages.find(l => l.id === languageId) @@ -82,6 +80,6 @@ export class VideoCaptionAddModalComponent extends FormReactive implements OnIni captionfile: this.form.value[ 'captionfile' ] }) - this.form.reset() + this.hide() } } From e1fa0266753446b79c76b584321fec2e722b80c2 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 14 Feb 2019 09:44:33 +0100 Subject: [PATCH 02/10] Error on invalid password in reset password script --- scripts/reset-password.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/reset-password.ts b/scripts/reset-password.ts index 6516edc28..4a9037280 100755 --- a/scripts/reset-password.ts +++ b/scripts/reset-password.ts @@ -1,6 +1,7 @@ import * as program from 'commander' import { initDatabaseModels } from '../server/initializers' import { UserModel } from '../server/models/account/user' +import { isUserPasswordValid } from '../server/helpers/custom-validators/users' program .option('-u, --user [user]', 'User') @@ -36,6 +37,11 @@ initDatabaseModels(true) console.log('New password?') rl.on('line', function (password) { + if (!isUserPasswordValid(password)) { + console.error('New password is invalid.') + process.exit(-1) + } + user.password = password user.save() From 9b69bfc076a5d867fbf5142f31840407d5a7aeae Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 14 Feb 2019 10:07:25 +0100 Subject: [PATCH 03/10] Don't crash on error in notification popup --- .../shared/users/user-notification.model.ts | 106 +++++++++--------- 1 file changed, 56 insertions(+), 50 deletions(-) diff --git a/client/src/app/shared/users/user-notification.model.ts b/client/src/app/shared/users/user-notification.model.ts index 125d2120c..5d0dc19ae 100644 --- a/client/src/app/shared/users/user-notification.model.ts +++ b/client/src/app/shared/users/user-notification.model.ts @@ -63,73 +63,79 @@ export class UserNotification implements UserNotificationServer { this.type = hash.type this.read = hash.read - this.video = hash.video - if (this.video) this.setAvatarUrl(this.video.channel) + // We assume that some fields exist + // To prevent a notification popup crash in case of bug, wrap it inside a try/catch + try { + this.video = hash.video + if (this.video) this.setAvatarUrl(this.video.channel) - this.videoImport = hash.videoImport + this.videoImport = hash.videoImport - this.comment = hash.comment - if (this.comment) this.setAvatarUrl(this.comment.account) + this.comment = hash.comment + if (this.comment) this.setAvatarUrl(this.comment.account) - this.videoAbuse = hash.videoAbuse + this.videoAbuse = hash.videoAbuse - this.videoBlacklist = hash.videoBlacklist + this.videoBlacklist = hash.videoBlacklist - this.account = hash.account - if (this.account) this.setAvatarUrl(this.account) + this.account = hash.account + if (this.account) this.setAvatarUrl(this.account) - this.actorFollow = hash.actorFollow - if (this.actorFollow) this.setAvatarUrl(this.actorFollow.follower) + this.actorFollow = hash.actorFollow + if (this.actorFollow) this.setAvatarUrl(this.actorFollow.follower) - this.createdAt = hash.createdAt - this.updatedAt = hash.updatedAt + this.createdAt = hash.createdAt + this.updatedAt = hash.updatedAt - switch (this.type) { - case UserNotificationType.NEW_VIDEO_FROM_SUBSCRIPTION: - this.videoUrl = this.buildVideoUrl(this.video) - break + switch (this.type) { + case UserNotificationType.NEW_VIDEO_FROM_SUBSCRIPTION: + this.videoUrl = this.buildVideoUrl(this.video) + break - case UserNotificationType.UNBLACKLIST_ON_MY_VIDEO: - this.videoUrl = this.buildVideoUrl(this.video) - break + case UserNotificationType.UNBLACKLIST_ON_MY_VIDEO: + this.videoUrl = this.buildVideoUrl(this.video) + break - case UserNotificationType.NEW_COMMENT_ON_MY_VIDEO: - case UserNotificationType.COMMENT_MENTION: - this.accountUrl = this.buildAccountUrl(this.comment.account) - this.commentUrl = [ this.buildVideoUrl(this.comment.video), { threadId: this.comment.threadId } ] - break + case UserNotificationType.NEW_COMMENT_ON_MY_VIDEO: + case UserNotificationType.COMMENT_MENTION: + this.accountUrl = this.buildAccountUrl(this.comment.account) + this.commentUrl = [ this.buildVideoUrl(this.comment.video), { threadId: this.comment.threadId } ] + break - case UserNotificationType.NEW_VIDEO_ABUSE_FOR_MODERATORS: - this.videoAbuseUrl = '/admin/moderation/video-abuses/list' - this.videoUrl = this.buildVideoUrl(this.videoAbuse.video) - break + case UserNotificationType.NEW_VIDEO_ABUSE_FOR_MODERATORS: + this.videoAbuseUrl = '/admin/moderation/video-abuses/list' + this.videoUrl = this.buildVideoUrl(this.videoAbuse.video) + break - case UserNotificationType.BLACKLIST_ON_MY_VIDEO: - this.videoUrl = this.buildVideoUrl(this.videoBlacklist.video) - break + case UserNotificationType.BLACKLIST_ON_MY_VIDEO: + this.videoUrl = this.buildVideoUrl(this.videoBlacklist.video) + break - case UserNotificationType.MY_VIDEO_PUBLISHED: - this.videoUrl = this.buildVideoUrl(this.video) - break + case UserNotificationType.MY_VIDEO_PUBLISHED: + this.videoUrl = this.buildVideoUrl(this.video) + break - case UserNotificationType.MY_VIDEO_IMPORT_SUCCESS: - this.videoImportUrl = this.buildVideoImportUrl() - this.videoImportIdentifier = this.buildVideoImportIdentifier(this.videoImport) - this.videoUrl = this.buildVideoUrl(this.videoImport.video) - break + case UserNotificationType.MY_VIDEO_IMPORT_SUCCESS: + this.videoImportUrl = this.buildVideoImportUrl() + this.videoImportIdentifier = this.buildVideoImportIdentifier(this.videoImport) + this.videoUrl = this.buildVideoUrl(this.videoImport.video) + break - case UserNotificationType.MY_VIDEO_IMPORT_ERROR: - this.videoImportUrl = this.buildVideoImportUrl() - this.videoImportIdentifier = this.buildVideoImportIdentifier(this.videoImport) - break + case UserNotificationType.MY_VIDEO_IMPORT_ERROR: + this.videoImportUrl = this.buildVideoImportUrl() + this.videoImportIdentifier = this.buildVideoImportIdentifier(this.videoImport) + break - case UserNotificationType.NEW_USER_REGISTRATION: - this.accountUrl = this.buildAccountUrl(this.account) - break + case UserNotificationType.NEW_USER_REGISTRATION: + this.accountUrl = this.buildAccountUrl(this.account) + break - case UserNotificationType.NEW_FOLLOW: - this.accountUrl = this.buildAccountUrl(this.actorFollow.follower) - break + case UserNotificationType.NEW_FOLLOW: + this.accountUrl = this.buildAccountUrl(this.actorFollow.follower) + break + } + } catch (err) { + console.error(err) } } From bf12db2497689cb3fcf648ef6fc07a699d1b20d8 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 14 Feb 2019 10:22:10 +0100 Subject: [PATCH 04/10] Fix moderators that cannot access the muted servers table --- client/src/app/+admin/moderation/moderation.routes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/app/+admin/moderation/moderation.routes.ts b/client/src/app/+admin/moderation/moderation.routes.ts index bc6dd49d5..6f6dde290 100644 --- a/client/src/app/+admin/moderation/moderation.routes.ts +++ b/client/src/app/+admin/moderation/moderation.routes.ts @@ -64,7 +64,7 @@ export const ModerationRoutes: Routes = [ component: InstanceServerBlocklistComponent, canActivate: [ UserRightGuard ], data: { - userRight: UserRight.MANAGE_SERVER_REDUNDANCY, + userRight: UserRight.MANAGE_SERVERS_BLOCKLIST, meta: { title: 'Muted instances' } From 6e8cf67a6f7c20f9e43dd7bfc111f6b494e11c50 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 14 Feb 2019 11:04:50 +0100 Subject: [PATCH 05/10] Fix mention notification with a remote account --- server/lib/notifier.ts | 2 + server/models/video/video-comment.ts | 42 ++++++++++++-------- server/tests/api/users/user-notifications.ts | 14 +++++++ 3 files changed, 42 insertions(+), 16 deletions(-) diff --git a/server/lib/notifier.ts b/server/lib/notifier.ts index d1b331346..2fa320cd7 100644 --- a/server/lib/notifier.ts +++ b/server/lib/notifier.ts @@ -148,6 +148,8 @@ class Notifier { private async notifyOfCommentMention (comment: VideoCommentModel) { const usernames = comment.extractMentions() + logger.debug('Extracted %d username from comment %s.', usernames.length, comment.url, { usernames, text: comment.text }) + let users = await UserModel.listByUsernames(usernames) if (comment.Video.isOwned()) { diff --git a/server/models/video/video-comment.ts b/server/models/video/video-comment.ts index cf6278da7..1163f9a0e 100644 --- a/server/models/video/video-comment.ts +++ b/server/models/video/video-comment.ts @@ -466,31 +466,41 @@ export class VideoCommentModel extends Model { } extractMentions () { - if (!this.text) return [] + let result: string[] = [] const localMention = `@(${actorNameAlphabet}+)` const remoteMention = `${localMention}@${CONFIG.WEBSERVER.HOST}` + const mentionRegex = this.isOwned() + ? '(?:(?:' + remoteMention + ')|(?:' + localMention + '))' // Include local mentions? + : '(?:' + remoteMention + ')' + + const firstMentionRegex = new RegExp(`^${mentionRegex} `, 'g') + const endMentionRegex = new RegExp(` ${mentionRegex}$`, 'g') const remoteMentionsRegex = new RegExp(' ' + remoteMention + ' ', 'g') - const localMentionsRegex = new RegExp(' ' + localMention + ' ', 'g') - const firstMentionRegex = new RegExp('^(?:(?:' + remoteMention + ')|(?:' + localMention + ')) ', 'g') - const endMentionRegex = new RegExp(' (?:(?:' + remoteMention + ')|(?:' + localMention + '))$', 'g') - return uniq( - [].concat( - regexpCapture(this.text, remoteMentionsRegex) - .map(([ , username ]) => username), + result = result.concat( + regexpCapture(this.text, firstMentionRegex) + .map(([ , username1, username2 ]) => username1 || username2), - regexpCapture(this.text, localMentionsRegex) - .map(([ , username ]) => username), + regexpCapture(this.text, endMentionRegex) + .map(([ , username1, username2 ]) => username1 || username2), - regexpCapture(this.text, firstMentionRegex) - .map(([ , username1, username2 ]) => username1 || username2), - - regexpCapture(this.text, endMentionRegex) - .map(([ , username1, username2 ]) => username1 || username2) - ) + regexpCapture(this.text, remoteMentionsRegex) + .map(([ , username ]) => username) ) + + // Include local mentions + if (this.isOwned()) { + const localMentionsRegex = new RegExp(' ' + localMention + ' ', 'g') + + result = result.concat( + regexpCapture(this.text, localMentionsRegex) + .map(([ , username ]) => username) + ) + } + + return uniq(result) } toFormattedJSON () { diff --git a/server/tests/api/users/user-notifications.ts b/server/tests/api/users/user-notifications.ts index 5260d64cc..72b6a0aa2 100644 --- a/server/tests/api/users/user-notifications.ts +++ b/server/tests/api/users/user-notifications.ts @@ -506,6 +506,20 @@ describe('Test users notifications', function () { await removeAccountFromAccountBlocklist(servers[ 0 ].url, userAccessToken, 'root') }) + it('Should not send a new mention notification if the remote account mention a local account', async function () { + this.timeout(20000) + + const resVideo = await uploadVideo(servers[0].url, servers[0].accessToken, { name: 'super video' }) + const uuid = resVideo.body.video.uuid + + await waitJobs(servers) + const resThread = await addVideoCommentThread(servers[1].url, servers[1].accessToken, uuid, '@user_1 hello') + const threadId = resThread.body.comment.id + + await waitJobs(servers) + await checkCommentMention(baseParams, uuid, threadId, threadId, 'super root 2 name', 'absence') + }) + it('Should send a new mention notification after local comments', async function () { this.timeout(10000) From d4804eead75bfaf41c3150d19e380ff4229d6226 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 14 Feb 2019 11:32:18 +0100 Subject: [PATCH 06/10] Fix too long name in menu --- client/src/app/menu/menu.component.html | 4 ++-- client/src/app/menu/menu.component.scss | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/client/src/app/menu/menu.component.html b/client/src/app/menu/menu.component.html index aa5bfa9c9..1e532ec13 100644 --- a/client/src/app/menu/menu.component.html +++ b/client/src/app/menu/menu.component.html @@ -5,8 +5,8 @@
- {{ user.account?.displayName }} -
{{ user.username }}
+ {{ user.account?.displayName }} +
{{ user.username }}
diff --git a/client/src/app/menu/menu.component.scss b/client/src/app/menu/menu.component.scss index f30b89413..69704674a 100644 --- a/client/src/app/menu/menu.component.scss +++ b/client/src/app/menu/menu.component.scss @@ -41,8 +41,11 @@ menu { .logged-in-info { flex-grow: 1; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; - .logged-in-username { + .logged-in-display-name { font-size: 16px; font-weight: $font-semibold; color: var(--menuForegroundColor); @@ -51,7 +54,7 @@ menu { @include disable-default-a-behaviour; } - .logged-in-email { + .logged-in-username { font-size: 13px; color: #C6C6C6; white-space: nowrap; From 7a338188f3ce6c3aa53054afaa936aa807d65fd8 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 14 Feb 2019 11:56:23 +0100 Subject: [PATCH 07/10] Fix from header in contact form --- server/lib/emailer.ts | 18 ++++++++++++------ server/lib/job-queue/handlers/email.ts | 6 ++++-- server/tests/api/server/contact-form.ts | 3 ++- server/tests/api/server/email.ts | 7 +++++++ 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/server/lib/emailer.ts b/server/lib/emailer.ts index f384a254e..99010f473 100644 --- a/server/lib/emailer.ts +++ b/server/lib/emailer.ts @@ -361,7 +361,8 @@ class Emailer { 'PeerTube.' const emailPayload: EmailPayload = { - from: fromEmail, + fromDisplayName: fromEmail, + replyTo: fromEmail, to: [ CONFIG.ADMIN.EMAIL ], subject: '[PeerTube] Contact form submitted', text @@ -370,16 +371,21 @@ class Emailer { return JobQueue.Instance.createJob({ type: 'email', payload: emailPayload }) } - sendMail (to: string[], subject: string, text: string, from?: string) { + sendMail (options: EmailPayload) { if (!Emailer.isEnabled()) { throw new Error('Cannot send mail because SMTP is not configured.') } + const fromDisplayName = options.fromDisplayName + ? options.fromDisplayName + : CONFIG.WEBSERVER.HOST + return this.transporter.sendMail({ - from: from || CONFIG.SMTP.FROM_ADDRESS, - to: to.join(','), - subject, - text + from: `"${fromDisplayName}" <${CONFIG.SMTP.FROM_ADDRESS}>`, + replyTo: options.replyTo, + to: options.to.join(','), + subject: options.subject, + text: options.text }) } diff --git a/server/lib/job-queue/handlers/email.ts b/server/lib/job-queue/handlers/email.ts index 220d0af32..2ba39a156 100644 --- a/server/lib/job-queue/handlers/email.ts +++ b/server/lib/job-queue/handlers/email.ts @@ -6,14 +6,16 @@ export type EmailPayload = { to: string[] subject: string text: string - from?: string + + fromDisplayName?: string + replyTo?: string } async function processEmail (job: Bull.Job) { const payload = job.data as EmailPayload logger.info('Processing email in job %d.', job.id) - return Emailer.Instance.sendMail(payload.to, payload.subject, payload.text, payload.from) + return Emailer.Instance.sendMail(payload) } // --------------------------------------------------------------------------- diff --git a/server/tests/api/server/contact-form.ts b/server/tests/api/server/contact-form.ts index 93221d0a3..06a2f89b0 100644 --- a/server/tests/api/server/contact-form.ts +++ b/server/tests/api/server/contact-form.ts @@ -45,7 +45,8 @@ describe('Test contact form', function () { const email = emails[0] - expect(email['from'][0]['address']).equal('toto@example.com') + expect(email['from'][0]['address']).equal('test-admin@localhost') + expect(email['from'][0]['name']).equal('toto@example.com') expect(email['to'][0]['address']).equal('admin1@example.com') expect(email['subject']).contains('Contact form') expect(email['text']).contains('my super message') diff --git a/server/tests/api/server/email.ts b/server/tests/api/server/email.ts index f96c57b66..f8f16f54f 100644 --- a/server/tests/api/server/email.ts +++ b/server/tests/api/server/email.ts @@ -89,6 +89,7 @@ describe('Test emails', function () { const email = emails[0] + expect(email['from'][0]['name']).equal('localhost:9001') expect(email['from'][0]['address']).equal('test-admin@localhost') expect(email['to'][0]['address']).equal('user_1@example.com') expect(email['subject']).contains('password') @@ -133,6 +134,7 @@ describe('Test emails', function () { const email = emails[1] + expect(email['from'][0]['name']).equal('localhost:9001') expect(email['from'][0]['address']).equal('test-admin@localhost') expect(email['to'][0]['address']).equal('admin1@example.com') expect(email['subject']).contains('abuse') @@ -152,6 +154,7 @@ describe('Test emails', function () { const email = emails[2] + expect(email['from'][0]['name']).equal('localhost:9001') expect(email['from'][0]['address']).equal('test-admin@localhost') expect(email['to'][0]['address']).equal('user_1@example.com') expect(email['subject']).contains(' blocked') @@ -169,6 +172,7 @@ describe('Test emails', function () { const email = emails[3] + expect(email['from'][0]['name']).equal('localhost:9001') expect(email['from'][0]['address']).equal('test-admin@localhost') expect(email['to'][0]['address']).equal('user_1@example.com') expect(email['subject']).contains(' unblocked') @@ -188,6 +192,7 @@ describe('Test emails', function () { const email = emails[4] + expect(email['from'][0]['name']).equal('localhost:9001') expect(email['from'][0]['address']).equal('test-admin@localhost') expect(email['to'][0]['address']).equal('user_1@example.com') expect(email['subject']).contains(' blacklisted') @@ -205,6 +210,7 @@ describe('Test emails', function () { const email = emails[5] + expect(email['from'][0]['name']).equal('localhost:9001') expect(email['from'][0]['address']).equal('test-admin@localhost') expect(email['to'][0]['address']).equal('user_1@example.com') expect(email['subject']).contains(' unblacklisted') @@ -224,6 +230,7 @@ describe('Test emails', function () { const email = emails[6] + expect(email['from'][0]['name']).equal('localhost:9001') expect(email['from'][0]['address']).equal('test-admin@localhost') expect(email['to'][0]['address']).equal('user_1@example.com') expect(email['subject']).contains('Verify') From 816970ff7a3978ee3e82dbf5dfda51c93724f299 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 14 Feb 2019 11:58:08 +0100 Subject: [PATCH 08/10] Fix mention helper test --- server/tests/helpers/comment-model.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/tests/helpers/comment-model.ts b/server/tests/helpers/comment-model.ts index 76bb0f212..ebfd779e1 100644 --- a/server/tests/helpers/comment-model.ts +++ b/server/tests/helpers/comment-model.ts @@ -10,6 +10,8 @@ class CommentMock { text: string extractMentions = VideoCommentModel.prototype.extractMentions + + isOwned = () => true } describe('Comment model', function () { From a0917aedaeaa63c01c675feac70777facf391f15 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 14 Feb 2019 12:07:02 +0100 Subject: [PATCH 09/10] Update changelog --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13bec7535..911ec0fec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## v1.2.1 + +## Bug fixes + + * **Important** Fix invalid `From` email header in contact form that could lead to the blacklisting of your SMTP server + * Fix too long display name overflow in menu + * Fix mention notification when a remote account mention a local account that has the same username than yours + * Fix access to muted servers table for moderators + * Don't crash notification popup on bug + * Fix reset password script that leaks password on invalid value + + ## v1.2.0 ### BREAKING CHANGES From 340f31ce96c456942fb01800f31d795e0f355d74 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 14 Feb 2019 12:41:55 +0100 Subject: [PATCH 10/10] Bumped to version v1.2.1 --- client/package.json | 2 +- package.json | 2 +- support/doc/api/openapi.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/client/package.json b/client/package.json index 31fc77887..bc06fbd1c 100644 --- a/client/package.json +++ b/client/package.json @@ -1,6 +1,6 @@ { "name": "peertube-client", - "version": "1.2.0", + "version": "1.2.1", "private": true, "licence": "GPLv3", "author": { diff --git a/package.json b/package.json index 0cf39c7ee..ed99157c4 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "peertube", "description": "Federated (ActivityPub) video streaming platform using P2P (BitTorrent) directly in the web browser with WebTorrent and Angular.", - "version": "1.2.0", + "version": "1.2.1", "private": true, "licence": "AGPLv3", "engines": { diff --git a/support/doc/api/openapi.yaml b/support/doc/api/openapi.yaml index f2bb945f9..ea419029c 100644 --- a/support/doc/api/openapi.yaml +++ b/support/doc/api/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.0.0 info: title: PeerTube - version: 1.2.0 + version: 1.2.1 contact: name: PeerTube Community url: 'https://joinpeertube.org'