From fa73f2e9f41aaf042db52502322686d73caddca1 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 28 May 2024 13:31:19 +0200 Subject: [PATCH 1/2] Fix conditions for userOwnsHistory and isRegisteredUser --- client/src/api/index.test.ts | 113 +++++++++++++++++++++++++++++++++++ client/src/api/index.ts | 24 ++++++-- 2 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 client/src/api/index.test.ts diff --git a/client/src/api/index.test.ts b/client/src/api/index.test.ts new file mode 100644 index 000000000000..42b666b2f03f --- /dev/null +++ b/client/src/api/index.test.ts @@ -0,0 +1,113 @@ +import { + type AnonymousUser, + type AnyHistory, + type HistorySummary, + type HistorySummaryExtended, + isRegisteredUser, + type User, + userOwnsHistory, +} from "."; + +const REGISTERED_USER_ID = "fake-user-id"; +const ANOTHER_USER_ID = "another-fake-user-id"; +const ANONYMOUS_USER_ID = null; + +const REGISTERED_USER: User = { + id: REGISTERED_USER_ID, + email: "test@mail.test", + tags_used: [], + isAnonymous: false, + total_disk_usage: 0, +}; + +const ANONYMOUS_USER: AnonymousUser = { + isAnonymous: true, +}; + +const SESSIONLESS_USER = null; + +function createFakeHistory(historyId: string = "fake-id", user_id?: string | null): T { + const history: AnyHistory = { + id: historyId, + name: "test", + model_class: "History", + deleted: false, + archived: false, + purged: false, + published: false, + annotation: null, + update_time: "2021-09-01T00:00:00.000Z", + tags: [], + url: `/history/${historyId}`, + contents_active: { active: 0, deleted: 0, hidden: 0 }, + count: 0, + size: 0, + }; + if (user_id !== undefined) { + (history as HistorySummaryExtended).user_id = user_id; + } + return history as T; +} + +const HISTORY_OWNED_BY_REGISTERED_USER = createFakeHistory("1234", REGISTERED_USER_ID); +const HISTORY_OWNED_BY_ANOTHER_USER = createFakeHistory("5678", ANOTHER_USER_ID); +const HISTORY_OWNED_BY_ANONYMOUS_USER = createFakeHistory("1234", ANONYMOUS_USER_ID); +const HISTORY_SUMMARY_WITHOUT_USER_ID = createFakeHistory("1234"); + +describe("API Types Helpers", () => { + describe("isRegisteredUser", () => { + it("should return true for a registered user", () => { + expect(isRegisteredUser(REGISTERED_USER)).toBe(true); + }); + + it("should return false for an anonymous user", () => { + expect(isRegisteredUser(ANONYMOUS_USER)).toBe(false); + }); + + it("should return false for sessionless users", () => { + expect(isRegisteredUser(SESSIONLESS_USER)).toBe(false); + }); + }); + + describe("isAnonymousUser", () => { + it("should return true for an anonymous user", () => { + expect(isRegisteredUser(ANONYMOUS_USER)).toBe(false); + }); + + it("should return false for a registered user", () => { + expect(isRegisteredUser(REGISTERED_USER)).toBe(true); + }); + + it("should return false for sessionless users", () => { + expect(isRegisteredUser(SESSIONLESS_USER)).toBe(false); + }); + }); + + describe("userOwnsHistory", () => { + it("should return true for a registered user owning the history", () => { + expect(userOwnsHistory(REGISTERED_USER, HISTORY_OWNED_BY_REGISTERED_USER)).toBe(true); + }); + + it("should return false for a registered user not owning the history", () => { + expect(userOwnsHistory(REGISTERED_USER, HISTORY_OWNED_BY_ANOTHER_USER)).toBe(false); + }); + + it("should return true for a registered user owning a history without user_id", () => { + expect(userOwnsHistory(REGISTERED_USER, HISTORY_SUMMARY_WITHOUT_USER_ID)).toBe(true); + }); + + it("should return true for an anonymous user owning a history with null user_id", () => { + expect(userOwnsHistory(ANONYMOUS_USER, HISTORY_OWNED_BY_ANONYMOUS_USER)).toBe(true); + }); + + it("should return false for an anonymous user not owning a history", () => { + expect(userOwnsHistory(ANONYMOUS_USER, HISTORY_OWNED_BY_REGISTERED_USER)).toBe(false); + }); + + it("should return false for sessionless users", () => { + expect(userOwnsHistory(SESSIONLESS_USER, HISTORY_OWNED_BY_REGISTERED_USER)).toBe(false); + expect(userOwnsHistory(SESSIONLESS_USER, HISTORY_SUMMARY_WITHOUT_USER_ID)).toBe(false); + expect(userOwnsHistory(SESSIONLESS_USER, HISTORY_OWNED_BY_ANONYMOUS_USER)).toBe(false); + }); + }); +}); diff --git a/client/src/api/index.ts b/client/src/api/index.ts index 730f03f5ddec..304573a4c81c 100644 --- a/client/src/api/index.ts +++ b/client/src/api/index.ts @@ -26,7 +26,8 @@ export interface HistoryContentsStats { * Data returned by the API when requesting `?view=summary&keys=size,contents_active,user_id`. */ export interface HistorySummaryExtended extends HistorySummary, HistoryContentsStats { - user_id: string; + /** The ID of the user that owns the history. Null if the history is owned by an anonymous user. */ + user_id: string | null; } type HistoryDetailedModel = components["schemas"]["HistoryDetailed"]; @@ -207,6 +208,7 @@ export function isHistorySummaryExtended(history: AnyHistory): history is Histor type QuotaUsageResponse = components["schemas"]["UserQuotaUsage"]; +/** Represents a registered user.**/ export interface User extends QuotaUsageResponse { id: string; email: string; @@ -224,18 +226,30 @@ export interface AnonymousUser { export type GenericUser = User | AnonymousUser; -export function isRegisteredUser(user: User | AnonymousUser | null): user is User { - return !user?.isAnonymous; +/** Represents any user, including anonymous users or session-less (null) users.**/ +export type AnyUser = GenericUser | null; + +export function isRegisteredUser(user: AnyUser): user is User { + return user !== null && !user?.isAnonymous; +} + +export function isAnonymousUser(user: AnyUser): user is AnonymousUser { + return user !== null && user.isAnonymous; } -export function userOwnsHistory(user: User | AnonymousUser | null, history: AnyHistory) { +export function userOwnsHistory(user: AnyUser, history: AnyHistory) { return ( // Assuming histories without user_id are owned by the current user (isRegisteredUser(user) && !hasOwner(history)) || - (isRegisteredUser(user) && hasOwner(history) && user.id === history.user_id) + (isRegisteredUser(user) && hasOwner(history) && user.id === history.user_id) || + (isAnonymousUser(user) && hasAnonymousOwner(history)) ); } function hasOwner(history: AnyHistory): history is HistorySummaryExtended { return "user_id" in history && history.user_id !== null; } + +function hasAnonymousOwner(history: AnyHistory): history is HistorySummaryExtended { + return "user_id" in history && history.user_id === null; +} From 1637e202e5073b9a4ddd4fe54dde0b3dc2ca7fcc Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Tue, 28 May 2024 14:30:19 +0200 Subject: [PATCH 2/2] Adapt History SelectorModal test --- .../History/Modals/SelectorModal.test.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/client/src/components/History/Modals/SelectorModal.test.js b/client/src/components/History/Modals/SelectorModal.test.js index e6c4e708b1f9..a78afe094f7b 100644 --- a/client/src/components/History/Modals/SelectorModal.test.js +++ b/client/src/components/History/Modals/SelectorModal.test.js @@ -7,6 +7,8 @@ import { createPinia } from "pinia"; import { useHistoryStore } from "stores/historyStore"; import { getLocalVue } from "tests/jest/helpers"; +import { useUserStore } from "@/stores/userStore"; + import SelectorModal from "./SelectorModal"; const localVue = getLocalVue(); @@ -35,6 +37,14 @@ const PROPS_FOR_MODAL_MULTIPLE_SELECT = { const CURRENT_HISTORY_INDICATION_TEXT = "(Current)"; +const CURRENT_USER = { + email: "email", + id: "user_id", + tags_used: [], + isAnonymous: false, + total_disk_usage: 0, +}; + describe("History SelectorModal.vue", () => { let wrapper; let axiosMock; @@ -60,6 +70,10 @@ describe("History SelectorModal.vue", () => { icon: { template: "
" }, }, }); + + const userStore = useUserStore(); + userStore.setCurrentUser(CURRENT_USER); + historyStore = useHistoryStore(); axiosMock = new MockAdapter(axios); getUpdatedAxiosMock();