diff --git a/client/components/app/MediaPlayerContainer.vue b/client/components/app/MediaPlayerContainer.vue index 1a2b1d30..d0408ee9 100644 --- a/client/components/app/MediaPlayerContainer.vue +++ b/client/components/app/MediaPlayerContainer.vue @@ -55,7 +55,7 @@ @showPlayerQueueItems="showPlayerQueueItemsModal = true" /> - + @@ -116,7 +116,7 @@ export default { }, bookmarks() { if (!this.libraryItemId) return [] - return this.$store.getters['user/getUserBookmarksForItem'](this.libraryItemId) + return this.$store.getters['user/getUserBookmarksForItem'](this.libraryItemId, this.$store.state.streamEpisodeId) }, streamLibraryItem() { return this.$store.state.streamLibraryItem diff --git a/client/components/modals/BookmarksModal.vue b/client/components/modals/BookmarksModal.vue index d84a8ed8..62bb5726 100644 --- a/client/components/modals/BookmarksModal.vue +++ b/client/components/modals/BookmarksModal.vue @@ -47,6 +47,7 @@ export default { default: 0 }, libraryItemId: String, + episodeId: String, playbackRate: Number, hideCreate: Boolean }, @@ -92,8 +93,9 @@ export default { this.showBookmarkTitleInput = true }, deleteBookmark(bm) { + const deleteUrl = this.episodeId ? `/api/me/item/${this.libraryItemId}/bookmark/${bm.time}?episode=${this.episodeId}` : `/api/me/item/${this.libraryItemId}/bookmark/${bm.time}` this.$axios - .$delete(`/api/me/item/${this.libraryItemId}/bookmark/${bm.time}`) + .$delete(deleteUrl) .then(() => { this.$toast.success(this.$strings.ToastBookmarkRemoveSuccess) }) @@ -114,6 +116,7 @@ export default { title: this.newBookmarkTitle, time: Math.floor(this.currentTime) } + if (this.episodeId) bookmark.episodeId = this.episodeId this.$axios .$post(`/api/me/item/${this.libraryItemId}/bookmark`, bookmark) .then(() => { diff --git a/client/components/player/PlayerUi.vue b/client/components/player/PlayerUi.vue index f929943c..f35dbbcd 100644 --- a/client/components/player/PlayerUi.vue +++ b/client/components/player/PlayerUi.vue @@ -18,7 +18,7 @@ - + diff --git a/client/store/user.js b/client/store/user.js index 96e79d12..1f9d3810 100644 --- a/client/store/user.js +++ b/client/store/user.js @@ -37,10 +37,15 @@ export const getters = { return li.libraryItemId == libraryItemId }) }, - getUserBookmarksForItem: (state) => (libraryItemId) => { - if (!state.user?.bookmarks) return [] - return state.user.bookmarks.filter((bm) => bm.libraryItemId === libraryItemId) - }, + getUserBookmarksForItem: + (state) => + (libraryItemId, episodeId = null) => { + if (!state.user?.bookmarks) return [] + return state.user.bookmarks.filter((bm) => { + if (episodeId && bm.episodeId !== episodeId) return false + return bm.libraryItemId === libraryItemId + }) + }, getUserSetting: (state) => (key) => { return state.settings?.[key] || null }, diff --git a/server/controllers/MeController.js b/server/controllers/MeController.js index c5968f52..1ac23143 100644 --- a/server/controllers/MeController.js +++ b/server/controllers/MeController.js @@ -216,7 +216,7 @@ class MeController { return res.sendStatus(403) } - const { time, title } = req.body + const { time, title, episodeId } = req.body if (isNullOrNaN(time)) { Logger.error(`[MeController] createBookmark invalid time`, time) return res.status(400).send('Invalid time') @@ -226,7 +226,7 @@ class MeController { return res.status(400).send('Invalid title') } - const bookmark = await req.user.createBookmark(req.params.id, time, title) + const bookmark = await req.user.createBookmark(req.params.id, time, title, episodeId) SocketAuthority.clientEmitter(req.user.id, 'user_updated', req.user.toOldJSONForBrowser()) res.json(bookmark) } @@ -249,7 +249,7 @@ class MeController { return res.sendStatus(403) } - const { time, title } = req.body + const { time, title, episodeId } = req.body if (isNullOrNaN(time)) { Logger.error(`[MeController] updateBookmark invalid time`, time) return res.status(400).send('Invalid time') @@ -259,7 +259,7 @@ class MeController { return res.status(400).send('Invalid title') } - const bookmark = await req.user.updateBookmark(req.params.id, time, title) + const bookmark = await req.user.updateBookmark(req.params.id, time, title, episodeId) if (!bookmark) { Logger.error(`[MeController] updateBookmark not found for library item id "${req.params.id}" and time "${time}"`) return res.sendStatus(404) @@ -291,13 +291,14 @@ class MeController { if (isNaN(time)) { return res.status(400).send('Invalid time') } + const episodeId = req.query.episode - if (!req.user.findBookmark(req.params.id, time)) { + if (!req.user.findBookmark(req.params.id, time, episodeId)) { Logger.error(`[MeController] removeBookmark not found`) return res.sendStatus(404) } - await req.user.removeBookmark(req.params.id, time) + await req.user.removeBookmark(req.params.id, time, episodeId) SocketAuthority.clientEmitter(req.user.id, 'user_updated', req.user.toOldJSONForBrowser()) res.sendStatus(200) diff --git a/server/models/User.js b/server/models/User.js index 936efde1..d48da84e 100644 --- a/server/models/User.js +++ b/server/models/User.js @@ -833,13 +833,16 @@ class User extends Model { /** * Find bookmark - * TODO: Bookmarks should use mediaItemId instead of libraryItemId to support podcast episodes * * @param {string} libraryItemId * @param {number} time + * @param {string} [episodeId] * @returns {AudioBookmarkObject|null} */ - findBookmark(libraryItemId, time) { + findBookmark(libraryItemId, time, episodeId = null) { + if (episodeId) { + return this.bookmarks.find((bm) => bm.libraryItemId === libraryItemId && bm.episodeId === episodeId && bm.time == time) + } return this.bookmarks.find((bm) => bm.libraryItemId === libraryItemId && bm.time == time) } @@ -849,10 +852,11 @@ class User extends Model { * @param {string} libraryItemId * @param {number} time * @param {string} title + * @param {string} [episodeId] * @returns {Promise} */ - async createBookmark(libraryItemId, time, title) { - const existingBookmark = this.findBookmark(libraryItemId, time) + async createBookmark(libraryItemId, time, title, episodeId = null) { + const existingBookmark = this.findBookmark(libraryItemId, time, episodeId) if (existingBookmark) { Logger.warn('[User] Create Bookmark already exists for this time') if (existingBookmark.title !== title) { @@ -869,6 +873,7 @@ class User extends Model { title, createdAt: Date.now() } + if (episodeId) newBookmark.episodeId = episodeId this.bookmarks.push(newBookmark) this.changed('bookmarks', true) await this.save() @@ -881,10 +886,11 @@ class User extends Model { * @param {string} libraryItemId * @param {number} time * @param {string} title + * @param {string} [episodeId] * @returns {Promise} */ - async updateBookmark(libraryItemId, time, title) { - const bookmark = this.findBookmark(libraryItemId, time) + async updateBookmark(libraryItemId, time, title, episodeId = null) { + const bookmark = this.findBookmark(libraryItemId, time, episodeId) if (!bookmark) { Logger.error(`[User] updateBookmark not found`) return null @@ -900,14 +906,19 @@ class User extends Model { * * @param {string} libraryItemId * @param {number} time + * @param {string} [episodeId] * @returns {Promise} - true if bookmark was removed */ - async removeBookmark(libraryItemId, time) { - if (!this.findBookmark(libraryItemId, time)) { + async removeBookmark(libraryItemId, time, episodeId = null) { + if (!this.findBookmark(libraryItemId, time, episodeId)) { Logger.error(`[User] removeBookmark not found`) return false } - this.bookmarks = this.bookmarks.filter((bm) => bm.libraryItemId !== libraryItemId || bm.time !== time) + if (episodeId) { + this.bookmarks = this.bookmarks.filter((bm) => bm.libraryItemId !== libraryItemId || bm.episodeId !== episodeId || bm.time !== time) + } else { + this.bookmarks = this.bookmarks.filter((bm) => bm.libraryItemId !== libraryItemId || bm.time !== time) + } this.changed('bookmarks', true) await this.save() return true diff --git a/test/server/controllers/MeController.test.js b/test/server/controllers/MeController.test.js index 3cc5496d..2fa53398 100644 --- a/test/server/controllers/MeController.test.js +++ b/test/server/controllers/MeController.test.js @@ -222,7 +222,7 @@ describe('MeController - IDOR Security Tests', () => { it('should allow user to create bookmark for accessible library item', async () => { const expandedItem = await Database.libraryItemModel.getExpandedById(libraryItem1.id) - const bookmark = { libraryItemId: libraryItem1.id, time: 100, title: 'Test Bookmark', createdAt: Date.now() } + const bookmark = { libraryItemId: libraryItem1.id, time: 100, title: 'Test Bookmark', episodeId: 'test-ep-1', createdAt: Date.now() } const fakeReq = { user: { @@ -234,7 +234,7 @@ describe('MeController - IDOR Security Tests', () => { toOldJSONForBrowser: () => ({ id: user2.id, username: user2.username }) }, params: { id: libraryItem1.id }, - body: { time: 100, title: 'Test Bookmark' } + body: { time: 100, title: 'Test Bookmark', episodeId: 'test-ep-1' } } const fakeRes = { sendStatus: sinon.spy(), @@ -247,6 +247,7 @@ describe('MeController - IDOR Security Tests', () => { await MeController.createBookmark(fakeReq, fakeRes) + expect(fakeReq.user.createBookmark.calledWith(libraryItem1.id, 100, 'Test Bookmark', 'test-ep-1')).to.be.true expect(fakeRes.json.calledOnce).to.be.true expect(fakeRes.json.calledWith(bookmark)).to.be.true @@ -343,7 +344,7 @@ describe('MeController - IDOR Security Tests', () => { it('should allow user to update bookmark for accessible library item', async () => { const expandedItem = await Database.libraryItemModel.getExpandedById(libraryItem1.id) - const bookmark = { libraryItemId: libraryItem1.id, time: 100, title: 'Updated Title' } + const bookmark = { libraryItemId: libraryItem1.id, time: 100, title: 'Updated Title', episodeId: 'test-ep-1' } const fakeReq = { user: { @@ -355,7 +356,7 @@ describe('MeController - IDOR Security Tests', () => { toOldJSONForBrowser: () => ({ id: user1.id, username: user1.username }) }, params: { id: libraryItem1.id }, - body: { time: 100, title: 'Updated Title' } + body: { time: 100, title: 'Updated Title', episodeId: 'test-ep-1' } } const fakeRes = { sendStatus: sinon.spy(), @@ -368,6 +369,7 @@ describe('MeController - IDOR Security Tests', () => { await MeController.updateBookmark(fakeReq, fakeRes) + expect(fakeReq.user.updateBookmark.calledWith(libraryItem1.id, 100, 'Updated Title', 'test-ep-1')).to.be.true expect(fakeRes.json.calledOnce).to.be.true expect(fakeRes.json.calledWith(bookmark)).to.be.true @@ -415,11 +417,12 @@ describe('MeController - IDOR Security Tests', () => { id: user1.id, username: user1.username, checkCanAccessLibraryItem: () => true, - findBookmark: sinon.stub().returns({ libraryItemId: libraryItem1.id, time: 100, title: 'Test Bookmark' }), + findBookmark: sinon.stub().returns({ libraryItemId: libraryItem1.id, time: 100, title: 'Test Bookmark', episodeId: 'test-ep-1' }), removeBookmark: sinon.stub().resolves(true), toOldJSONForBrowser: () => ({ id: user1.id, username: user1.username }) }, - params: { id: libraryItem1.id, time: '100' } + params: { id: libraryItem1.id, time: '100' }, + query: { episode: 'test-ep-1' } } const fakeRes = { sendStatus: sinon.spy(), @@ -431,6 +434,8 @@ describe('MeController - IDOR Security Tests', () => { await MeController.removeBookmark(fakeReq, fakeRes) + expect(fakeReq.user.findBookmark.calledWith(libraryItem1.id, 100, 'test-ep-1')).to.be.true + expect(fakeReq.user.removeBookmark.calledWith(libraryItem1.id, 100, 'test-ep-1')).to.be.true expect(fakeRes.sendStatus.calledWith(200)).to.be.true Database.libraryItemModel.getExpandedById.restore()