From 18490b07650d77d7fe376970b749af5a8c672fd6 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 25 Nov 2020 11:04:18 +0100 Subject: [PATCH] Fix migration and test --- .../my-account-applications.component.ts | 1 - .../video-user-subscriptions.component.ts | 8 +-- server/controllers/feeds.ts | 15 ++--- server/helpers/middlewares/accounts.ts | 4 +- server/initializers/constants.ts | 2 +- ...-feed-token.ts => 0560-user-feed-token.ts} | 23 +++++-- server/middlewares/validators/feeds.ts | 66 ++++++++++++------- server/tests/api/check-params/users.ts | 32 ++++++++- server/tests/feeds/feeds.ts | 53 ++++++++++----- shared/extra-utils/feeds/feeds.ts | 4 +- shared/extra-utils/users/users.ts | 32 ++++++--- 11 files changed, 161 insertions(+), 79 deletions(-) rename server/initializers/migrations/{0530-user-feed-token.ts => 0560-user-feed-token.ts} (65%) diff --git a/client/src/app/+my-account/my-account-applications/my-account-applications.component.ts b/client/src/app/+my-account/my-account-applications/my-account-applications.component.ts index 233e42c83..756644990 100644 --- a/client/src/app/+my-account/my-account-applications/my-account-applications.component.ts +++ b/client/src/app/+my-account/my-account-applications/my-account-applications.component.ts @@ -1,4 +1,3 @@ - import { Component, OnInit } from '@angular/core' import { AuthService, Notifier, ConfirmService, ScopedTokensService } from '@app/core' import { VideoService } from '@app/shared/shared-main' diff --git a/client/src/app/+videos/video-list/video-user-subscriptions.component.ts b/client/src/app/+videos/video-list/video-user-subscriptions.component.ts index 03881c295..bd0337c1d 100644 --- a/client/src/app/+videos/video-list/video-user-subscriptions.component.ts +++ b/client/src/app/+videos/video-list/video-user-subscriptions.component.ts @@ -1,3 +1,4 @@ + import { Component, OnDestroy, OnInit } from '@angular/core' import { ActivatedRoute, Router } from '@angular/router' import { AuthService, LocalStorageService, Notifier, ScopedTokensService, ScreenService, ServerService, UserService } from '@app/core' @@ -6,10 +7,9 @@ import { immutableAssign } from '@app/helpers' import { VideoService } from '@app/shared/shared-main' import { UserSubscriptionService } from '@app/shared/shared-user-subscription' import { AbstractVideoList, OwnerDisplayType } from '@app/shared/shared-video-miniature' -import { VideoSortField, FeedFormat } from '@shared/models' -import { copyToClipboard } from '../../../root-helpers/utils' +import { FeedFormat, VideoSortField } from '@shared/models' import { environment } from '../../../environments/environment' -import { forkJoin } from 'rxjs' +import { copyToClipboard } from '../../../root-helpers/utils' @Component({ selector: 'my-videos-user-subscriptions', @@ -56,7 +56,7 @@ export class VideoUserSubscriptionsComponent extends AbstractVideoList implement this.scopedTokensService.getScopedTokens().subscribe( tokens => { const feeds = this.videoService.getVideoSubscriptionFeedUrls(user.account.id, tokens.feedToken) - feedUrl = feedUrl + feeds.find((f: any) => f.format === FeedFormat.RSS).url + feedUrl = feedUrl + feeds.find(f => f.format === FeedFormat.RSS).url }, err => { diff --git a/server/controllers/feeds.ts b/server/controllers/feeds.ts index 5c95069fc..2182b42f5 100644 --- a/server/controllers/feeds.ts +++ b/server/controllers/feeds.ts @@ -12,7 +12,7 @@ import { videoCommentsFeedsValidator, videoFeedsValidator, videosSortValidator, - videoSubscriptonFeedsValidator + videoSubscriptionFeedsValidator } from '../middlewares' import { cacheRoute } from '../middlewares/cache' import { VideoModel } from '../models/video/video' @@ -60,7 +60,7 @@ feedsRouter.get('/feeds/subscriptions.:format', ] })(ROUTE_CACHE_LIFETIME.FEEDS)), commonVideosFiltersValidator, - asyncMiddleware(videoSubscriptonFeedsValidator), + asyncMiddleware(videoSubscriptionFeedsValidator), asyncMiddleware(generateVideoFeedForSubscriptions) ) @@ -198,20 +198,17 @@ async function generateVideoFeedForSubscriptions (req: express.Request, res: exp queryString: new URL(WEBSERVER.URL + req.url).search }) - const options = { - followerActorId: res.locals.user.Account.Actor.id, - user: res.locals.user - } - const resultList = await VideoModel.listForApi({ start, count: FEEDS.COUNT, sort: req.query.sort, - includeLocalVideos: true, + includeLocalVideos: false, nsfw, filter: req.query.filter as VideoFilter, withFiles: true, - ...options + + followerActorId: res.locals.user.Account.Actor.id, + user: res.locals.user }) addVideosToFeed(feed, resultList.data) diff --git a/server/helpers/middlewares/accounts.ts b/server/helpers/middlewares/accounts.ts index fa4a51e6c..e9b981262 100644 --- a/server/helpers/middlewares/accounts.ts +++ b/server/helpers/middlewares/accounts.ts @@ -39,11 +39,11 @@ async function doesAccountExist (p: Bluebird, res: Response, se return true } -async function doesUserFeedTokenCorrespond (id: number | string, token: string, res: Response) { +async function doesUserFeedTokenCorrespond (id: number, token: string, res: Response) { const user = await UserModel.loadByIdWithChannels(parseInt(id + '', 10)) if (token !== user.feedToken) { - res.status(401) + res.status(403) .json({ error: 'User and token mismatch' }) return false diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index 945185f62..6c44d703e 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -24,7 +24,7 @@ import { CONFIG, registerConfigChangedHandler } from './config' // --------------------------------------------------------------------------- -const LAST_MIGRATION_VERSION = 555 +const LAST_MIGRATION_VERSION = 560 // --------------------------------------------------------------------------- diff --git a/server/initializers/migrations/0530-user-feed-token.ts b/server/initializers/migrations/0560-user-feed-token.ts similarity index 65% rename from server/initializers/migrations/0530-user-feed-token.ts rename to server/initializers/migrations/0560-user-feed-token.ts index 421016b11..7c61def17 100644 --- a/server/initializers/migrations/0530-user-feed-token.ts +++ b/server/initializers/migrations/0560-user-feed-token.ts @@ -9,13 +9,15 @@ async function up (utils: { }): Promise { const q = utils.queryInterface - // Create uuid column for users - const userFeedTokenUUID = { - type: Sequelize.UUID, - defaultValue: Sequelize.UUIDV4, - allowNull: true + { + // Create uuid column for users + const userFeedTokenUUID = { + type: Sequelize.UUID, + defaultValue: Sequelize.UUIDV4, + allowNull: true + } + await q.addColumn('user', 'feedToken', userFeedTokenUUID) } - await q.addColumn('user', 'feedToken', userFeedTokenUUID) // Set UUID to previous users { @@ -28,6 +30,15 @@ async function up (utils: { await utils.sequelize.query(queryUpdate) } } + + { + const userFeedTokenUUID = { + type: Sequelize.UUID, + defaultValue: Sequelize.UUIDV4, + allowNull: false + } + await q.changeColumn('user', 'feedToken', userFeedTokenUUID) + } } function down (options) { diff --git a/server/middlewares/validators/feeds.ts b/server/middlewares/validators/feeds.ts index 35080ffca..18469bad3 100644 --- a/server/middlewares/validators/feeds.ts +++ b/server/middlewares/validators/feeds.ts @@ -1,17 +1,17 @@ import * as express from 'express' import { param, query } from 'express-validator' -import { isIdOrUUIDValid, isIdValid } from '../../helpers/custom-validators/misc' -import { logger } from '../../helpers/logger' -import { areValidationErrors } from './utils' import { isValidRSSFeed } from '../../helpers/custom-validators/feeds' -import { doesVideoExist } from '../../helpers/middlewares/videos' +import { exists, isIdOrUUIDValid, isIdValid } from '../../helpers/custom-validators/misc' +import { logger } from '../../helpers/logger' import { doesAccountIdExist, doesAccountNameWithHostExist, + doesUserFeedTokenCorrespond, doesVideoChannelIdExist, - doesVideoChannelNameWithHostExist, - doesUserFeedTokenCorrespond + doesVideoChannelNameWithHostExist } from '../../helpers/middlewares' +import { doesVideoExist } from '../../helpers/middlewares/videos' +import { areValidationErrors } from './utils' const feedsFormatValidator = [ param('format').optional().custom(isValidRSSFeed).withMessage('Should have a valid format (rss, atom, json)'), @@ -35,19 +35,31 @@ function setFeedFormatContentType (req: express.Request, res: express.Response, if (req.accepts(acceptableContentTypes)) { res.set('Content-Type', req.accepts(acceptableContentTypes) as string) } else { - return res.status(406).send({ - message: `You should accept at least one of the following content-types: ${acceptableContentTypes.join(', ')}` - }).end() + return res.status(406) + .json({ + message: `You should accept at least one of the following content-types: ${acceptableContentTypes.join(', ')}` + }) } return next() } const videoFeedsValidator = [ - query('accountId').optional().custom(isIdValid), - query('accountName').optional(), - query('videoChannelId').optional().custom(isIdValid), - query('videoChannelName').optional(), + query('accountId') + .optional() + .custom(isIdValid) + .withMessage('Should have a valid account id'), + + query('accountName') + .optional(), + + query('videoChannelId') + .optional() + .custom(isIdValid) + .withMessage('Should have a valid channel id'), + + query('videoChannelName') + .optional(), async (req: express.Request, res: express.Response, next: express.NextFunction) => { logger.debug('Checking feeds parameters', { parameters: req.query }) @@ -63,19 +75,22 @@ const videoFeedsValidator = [ } ] -const videoSubscriptonFeedsValidator = [ - query('accountId').custom(isIdValid), - query('token'), +const videoSubscriptionFeedsValidator = [ + query('accountId') + .custom(isIdValid) + .withMessage('Should have a valid account id'), + + query('token') + .custom(exists) + .withMessage('Should have a token'), async (req: express.Request, res: express.Response, next: express.NextFunction) => { - logger.debug('Checking feeds parameters', { parameters: req.query }) + logger.debug('Checking subscription feeds parameters', { parameters: req.query }) if (areValidationErrors(req, res)) return - // a token alone is erroneous - if (req.query.token && !req.query.accountId) return - if (req.query.accountId && !await doesAccountIdExist(req.query.accountId, res)) return - if (req.query.token && !await doesUserFeedTokenCorrespond(res.locals.account.userId, req.query.token, res)) return + if (!await doesAccountIdExist(req.query.accountId, res)) return + if (!await doesUserFeedTokenCorrespond(res.locals.account.userId, req.query.token, res)) return return next() } @@ -90,9 +105,10 @@ const videoCommentsFeedsValidator = [ if (areValidationErrors(req, res)) return if (req.query.videoId && (req.query.videoChannelId || req.query.videoChannelName)) { - return res.status(400).send({ - message: 'videoId cannot be mixed with a channel filter' - }).end() + return res.status(400) + .json({ + message: 'videoId cannot be mixed with a channel filter' + }) } if (req.query.videoId && !await doesVideoExist(req.query.videoId, res)) return @@ -107,6 +123,6 @@ export { feedsFormatValidator, setFeedFormatContentType, videoFeedsValidator, - videoSubscriptonFeedsValidator, + videoSubscriptionFeedsValidator, videoCommentsFeedsValidator } diff --git a/server/tests/api/check-params/users.ts b/server/tests/api/check-params/users.ts index 2a220be83..da7dc9704 100644 --- a/server/tests/api/check-params/users.ts +++ b/server/tests/api/check-params/users.ts @@ -14,6 +14,7 @@ import { flushAndRunServer, getMyUserInformation, getMyUserVideoRating, + getUserScopedTokens, getUsersList, immutableAssign, killallServers, @@ -23,6 +24,7 @@ import { makeUploadRequest, registerUser, removeUser, + renewUserScopedTokens, reRunServer, ServerInfo, setAccessTokensToServers, @@ -38,7 +40,7 @@ import { checkBadStartPagination } from '../../../../shared/extra-utils/requests/check-api-params' import { waitJobs } from '../../../../shared/extra-utils/server/jobs' -import { getMagnetURI, getMyVideoImports, getGoodVideoUrl, importVideo } from '../../../../shared/extra-utils/videos/video-imports' +import { getGoodVideoUrl, getMagnetURI, getMyVideoImports, importVideo } from '../../../../shared/extra-utils/videos/video-imports' import { UserAdminFlag } from '../../../../shared/models/users/user-flag.model' import { VideoPrivacy } from '../../../../shared/models/videos' @@ -609,6 +611,34 @@ describe('Test users API validators', function () { }) }) + describe('When managing my scoped tokens', function () { + + it('Should fail to get my scoped tokens with an non authenticated user', async function () { + await getUserScopedTokens(server.url, null, 401) + }) + + it('Should fail to get my scoped tokens with a bad token', async function () { + await getUserScopedTokens(server.url, 'bad', 401) + + }) + + it('Should succeed to get my scoped tokens', async function () { + await getUserScopedTokens(server.url, server.accessToken) + }) + + it('Should fail to renew my scoped tokens with an non authenticated user', async function () { + await renewUserScopedTokens(server.url, null, 401) + }) + + it('Should fail to renew my scoped tokens with a bad token', async function () { + await renewUserScopedTokens(server.url, 'bad', 401) + }) + + it('Should succeed to renew my scoped tokens', async function () { + await renewUserScopedTokens(server.url, server.accessToken) + }) + }) + describe('When getting a user', function () { it('Should fail with an non authenticated user', async function () { diff --git a/server/tests/feeds/feeds.ts b/server/tests/feeds/feeds.ts index 175ea9102..92a468192 100644 --- a/server/tests/feeds/feeds.ts +++ b/server/tests/feeds/feeds.ts @@ -8,28 +8,29 @@ import { addAccountToServerBlocklist, removeAccountFromServerBlocklist } from '@shared/extra-utils/users/blocklist' +import { addUserSubscription, listUserSubscriptionVideos } from '@shared/extra-utils/users/user-subscriptions' import { VideoPrivacy } from '@shared/models' +import { ScopedToken } from '@shared/models/users/user-scoped-token' import { cleanupTests, createUser, doubleFollow, flushAndRunMultipleServers, + flushAndRunServer, getJSONfeed, getMyUserInformation, + getUserScopedTokens, getXMLfeed, + renewUserScopedTokens, ServerInfo, setAccessTokensToServers, uploadVideo, uploadVideoAndGetId, - userLogin, - flushAndRunServer, - getUserScopedTokens + userLogin } from '../../../shared/extra-utils' import { waitJobs } from '../../../shared/extra-utils/server/jobs' import { addVideoCommentThread } from '../../../shared/extra-utils/videos/video-comments' import { User } from '../../../shared/models/users' -import { ScopedToken } from '@shared/models/users/user-scoped-token' -import { listUserSubscriptionVideos, addUserSubscription } from '@shared/extra-utils/users/user-subscriptions' chai.use(require('chai-xml')) chai.use(require('chai-json-schema')) @@ -298,14 +299,10 @@ describe('Test syndication feeds', () => { }) describe('Video feed from my subscriptions', function () { - /** - * use the 'version' query parameter to bust cache between tests - */ + let feeduserAccountId: number + let feeduserFeedToken: string it('Should list no videos for a user with no videos and no subscriptions', async function () { - let feeduserAccountId: number - let feeduserFeedToken: string - const attr = { username: 'feeduser', password: 'password' } await createUser({ url: servers[0].url, accessToken: servers[0].accessToken, username: attr.username, password: attr.password }) const feeduserAccessToken = await userLogin(servers[0], attr) @@ -332,15 +329,21 @@ describe('Test syndication feeds', () => { } }) + it('Should fail with an invalid token', async function () { + await getJSONfeed(servers[0].url, 'subscriptions', { accountId: feeduserAccountId, token: 'toto' }, 403) + }) + + it('Should fail with a token of another user', async function () { + await getJSONfeed(servers[0].url, 'subscriptions', { accountId: feeduserAccountId, token: userFeedToken }, 403) + }) + it('Should list no videos for a user with videos but no subscriptions', async function () { - { - const res = await listUserSubscriptionVideos(servers[0].url, userAccessToken) - expect(res.body.total).to.equal(0) + const res = await listUserSubscriptionVideos(servers[0].url, userAccessToken) + expect(res.body.total).to.equal(0) - const json = await getJSONfeed(servers[0].url, 'subscriptions', { accountId: userAccountId, token: userFeedToken }) - const jsonObj = JSON.parse(json.text) - expect(jsonObj.items.length).to.be.equal(0) // no subscription, it should not list the instance's videos but list 0 videos - } + const json = await getJSONfeed(servers[0].url, 'subscriptions', { accountId: userAccountId, token: userFeedToken }) + const jsonObj = JSON.parse(json.text) + expect(jsonObj.items.length).to.be.equal(0) // no subscription, it should not list the instance's videos but list 0 videos }) it('Should list self videos for a user with a subscription to themselves', async function () { @@ -376,6 +379,20 @@ describe('Test syndication feeds', () => { } }) + it('Should renew the token, and so have an invalid old token', async function () { + await renewUserScopedTokens(servers[0].url, userAccessToken) + + await getJSONfeed(servers[0].url, 'subscriptions', { accountId: userAccountId, token: userFeedToken, version: 3 }, 403) + }) + + it('Should succeed with the new token', async function () { + const res2 = await getUserScopedTokens(servers[0].url, userAccessToken) + const token: ScopedToken = res2.body + userFeedToken = token.feedToken + + await getJSONfeed(servers[0].url, 'subscriptions', { accountId: userAccountId, token: userFeedToken, version: 4 }) + }) + }) after(async function () { diff --git a/shared/extra-utils/feeds/feeds.ts b/shared/extra-utils/feeds/feeds.ts index bafbb9f94..957d4499c 100644 --- a/shared/extra-utils/feeds/feeds.ts +++ b/shared/extra-utils/feeds/feeds.ts @@ -13,14 +13,14 @@ function getXMLfeed (url: string, feed: FeedType, format?: string) { .expect('Content-Type', /xml/) } -function getJSONfeed (url: string, feed: FeedType, query: any = {}) { +function getJSONfeed (url: string, feed: FeedType, query: any = {}, statusCodeExpected = 200) { const path = '/feeds/' + feed + '.json' return request(url) .get(path) .query(query) .set('Accept', 'application/json') - .expect(200) + .expect(statusCodeExpected) .expect('Content-Type', /json/) } diff --git a/shared/extra-utils/users/users.ts b/shared/extra-utils/users/users.ts index 4d0986ce3..ebb8bc257 100644 --- a/shared/extra-utils/users/users.ts +++ b/shared/extra-utils/users/users.ts @@ -1,12 +1,12 @@ +import { omit } from 'lodash' import * as request from 'supertest' -import { makePostBodyRequest, makePutBodyRequest, updateAvatarRequest } from '../requests/requests' +import { UserUpdateMe } from '../../models/users' import { UserAdminFlag } from '../../models/users/user-flag.model' import { UserRegister } from '../../models/users/user-register.model' import { UserRole } from '../../models/users/user-role' +import { makeGetRequest, makePostBodyRequest, makePutBodyRequest, updateAvatarRequest } from '../requests/requests' import { ServerInfo } from '../server/servers' import { userLogin } from './login' -import { UserUpdateMe } from '../../models/users' -import { omit } from 'lodash' type CreateUserArgs = { url: string @@ -109,15 +109,26 @@ function getMyUserInformation (url: string, accessToken: string, specialStatus = .expect('Content-Type', /json/) } -function getUserScopedTokens (url: string, accessToken: string, specialStatus = 200) { +function getUserScopedTokens (url: string, token: string, statusCodeExpected = 200) { const path = '/api/v1/users/scoped-tokens' - return request(url) - .get(path) - .set('Accept', 'application/json') - .set('Authorization', 'Bearer ' + accessToken) - .expect(specialStatus) - .expect('Content-Type', /json/) + return makeGetRequest({ + url, + path, + token, + statusCodeExpected + }) +} + +function renewUserScopedTokens (url: string, token: string, statusCodeExpected = 200) { + const path = '/api/v1/users/scoped-tokens' + + return makePostBodyRequest({ + url, + path, + token, + statusCodeExpected + }) } function deleteMe (url: string, accessToken: string, specialStatus = 204) { @@ -359,6 +370,7 @@ export { unblockUser, askResetPassword, resetPassword, + renewUserScopedTokens, updateMyAvatar, askSendVerifyEmail, generateUserAccessToken, -- 2.41.0