Merge pull request #18243 from davelopez/24.0_fix_userOwnsHistory_con…

…ditions

[24.0] Fix userOwnsHistory conditions

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: "[email protected]",
+    tags_used: [],
+    isAnonymous: false,
+    total_disk_usage: 0,
+};
+
+const ANONYMOUS_USER: AnonymousUser = {
+    isAnonymous: true,
+};
+
+const SESSIONLESS_USER = null;
+
+function createFakeHistory<T>(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<HistorySummaryExtended>("1234", REGISTERED_USER_ID);
+const HISTORY_OWNED_BY_ANOTHER_USER = createFakeHistory<HistorySummaryExtended>("5678", ANOTHER_USER_ID);
+const HISTORY_OWNED_BY_ANONYMOUS_USER = createFakeHistory<HistorySummaryExtended>("1234", ANONYMOUS_USER_ID);
+const HISTORY_SUMMARY_WITHOUT_USER_ID = createFakeHistory<HistorySummary>("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);
+        });
+    });
+});

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;
+}

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: "<div></div>" },
             },
         });
+
+        const userStore = useUserStore();
+        userStore.setCurrentUser(CURRENT_USER);
+
         historyStore = useHistoryStore();
         axiosMock = new MockAdapter(axios);
         getUpdatedAxiosMock();