From e2e46fcbafb6122daef0abfe39da90c8ebadc533 Mon Sep 17 00:00:00 2001
From: Jen Jones Arnesen <jennifer@dhis2.org>
Date: Thu, 5 Oct 2023 11:56:30 +0200
Subject: [PATCH] fix: handle both Set and array for currentUser.authorities

---
 .../__tests__/getInterpretationAccess.spec.js | 251 +++++++++++++++++-
 .../common/getInterpretationAccess.js         |  13 +-
 2 files changed, 259 insertions(+), 5 deletions(-)

diff --git a/src/components/Interpretations/common/__tests__/getInterpretationAccess.spec.js b/src/components/Interpretations/common/__tests__/getInterpretationAccess.spec.js
index 45f75feff..3bb11d212 100644
--- a/src/components/Interpretations/common/__tests__/getInterpretationAccess.spec.js
+++ b/src/components/Interpretations/common/__tests__/getInterpretationAccess.spec.js
@@ -3,23 +3,38 @@ import {
     getCommentAccess,
 } from '../getInterpretationAccess.js'
 
-const superuser = {
+const superuserD2CurrentUser = {
     id: 'iamsuper',
     authorities: new Set(['ALL']),
 }
 
-const userJoe = {
+const userJoeD2CurrentUser = {
     id: 'johndoe',
     authorities: new Set(['Some']),
 }
 
-const userJane = {
+const userJaneD2CurrentUser = {
     id: 'jane',
     authorities: new Set(['Some']),
 }
 
+const superuser = {
+    id: 'iamsuper',
+    authorities: ['ALL'],
+}
+
+const userJoe = {
+    id: 'johndoe',
+    authorities: ['Some'],
+}
+
+const userJane = {
+    id: 'jane',
+    authorities: ['Some'],
+}
+
 describe('interpretation and comment access', () => {
-    describe('getInterpretationAccess', () => {
+    describe('getInterpretationAccess with D2', () => {
         it('returns true for all accesses for superuser', () => {
             const interpretation = {
                 access: {
@@ -113,6 +128,234 @@ describe('interpretation and comment access', () => {
                 delete: false,
             })
         })
+
+        it('returns false for all accesses when no currentUser provided', () => {
+            const interpretation = {
+                access: {
+                    write: false,
+                    manage: false,
+                },
+                createdBy: userJane,
+            }
+
+            expect(getInterpretationAccess(interpretation)).toMatchObject({
+                share: false,
+                comment: false,
+                edit: false,
+                delete: false,
+            })
+        })
+
+        it('returns false for all accesses when currentUser missing authorities', () => {
+            const interpretation = {
+                access: {
+                    write: false,
+                    manage: false,
+                },
+                createdBy: userJane,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, {
+                    id: 'usernoauthorties',
+                })
+            ).toMatchObject({
+                share: false,
+                comment: false,
+                edit: false,
+                delete: false,
+            })
+        })
+    })
+
+    describe('getInterpretationAccess using D2.currentUser', () => {
+        it('returns true for all accesses for superuser', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                    manage: true,
+                },
+                createdBy: userJoeD2CurrentUser,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, superuserD2CurrentUser)
+            ).toMatchObject({
+                share: true,
+                comment: true,
+                edit: true,
+                delete: true,
+            })
+        })
+        it('returns true for all accesses for creator', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                    manage: true,
+                },
+                createdBy: userJoeD2CurrentUser,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, userJoeD2CurrentUser)
+            ).toMatchObject({
+                share: true,
+                comment: true,
+                edit: true,
+                delete: true,
+            })
+        })
+
+        it('returns false for edit/delete if user is not creator/superuser', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                    manage: true,
+                },
+                createdBy: userJaneD2CurrentUser,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, userJoeD2CurrentUser)
+            ).toMatchObject({
+                share: true,
+                comment: true,
+                edit: false,
+                delete: false,
+            })
+        })
+
+        it('returns false for comment/edit/delete if user is not creator/superuser and no write access', () => {
+            const interpretation = {
+                access: {
+                    write: false,
+                    manage: true,
+                },
+                createdBy: userJaneD2CurrentUser,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, userJoeD2CurrentUser)
+            ).toMatchObject({
+                share: true,
+                comment: false,
+                edit: false,
+                delete: false,
+            })
+        })
+
+        it('returns false for share/comment/edit/delete if user is not creator/superuser and no write or manage access', () => {
+            const interpretation = {
+                access: {
+                    write: false,
+                    manage: false,
+                },
+                createdBy: userJaneD2CurrentUser,
+            }
+
+            expect(
+                getInterpretationAccess(interpretation, userJoeD2CurrentUser)
+            ).toMatchObject({
+                share: false,
+                comment: false,
+                edit: false,
+                delete: false,
+            })
+        })
+    })
+
+    describe('getCommentAccess using D2.currentUser', () => {
+        it('returns true for all accesses for superuser', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                },
+            }
+
+            const comment = {
+                createdBy: userJoeD2CurrentUser,
+            }
+
+            expect(
+                getCommentAccess(
+                    comment,
+                    interpretation.access.write,
+                    superuserD2CurrentUser
+                )
+            ).toMatchObject({
+                edit: true,
+                delete: true,
+            })
+        })
+
+        it('returns true for all accesses for creator when interpretation has write access', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                },
+            }
+
+            const comment = {
+                createdBy: userJoeD2CurrentUser,
+            }
+
+            expect(
+                getCommentAccess(
+                    comment,
+                    interpretation.access.write,
+                    userJoeD2CurrentUser
+                )
+            ).toMatchObject({
+                edit: true,
+                delete: true,
+            })
+        })
+
+        it('returns true for edit and false for delete for creator when interpretation does not have write access', () => {
+            const interpretation = {
+                access: {
+                    write: false,
+                },
+            }
+
+            const comment = {
+                createdBy: userJoeD2CurrentUser,
+            }
+
+            expect(
+                getCommentAccess(
+                    comment,
+                    interpretation.access.write,
+                    userJoeD2CurrentUser
+                )
+            ).toMatchObject({
+                edit: true,
+                delete: false,
+            })
+        })
+
+        it('returns false for edit/delete for user who is not creator or superuser', () => {
+            const interpretation = {
+                access: {
+                    write: true,
+                },
+            }
+
+            const comment = {
+                createdBy: userJaneD2CurrentUser,
+            }
+
+            expect(
+                getCommentAccess(
+                    comment,
+                    interpretation.access.write,
+                    userJoeD2CurrentUser
+                )
+            ).toMatchObject({
+                edit: false,
+                delete: false,
+            })
+        })
     })
 
     describe('getCommentAccess', () => {
diff --git a/src/components/Interpretations/common/getInterpretationAccess.js b/src/components/Interpretations/common/getInterpretationAccess.js
index d95e82af3..a04d6c134 100644
--- a/src/components/Interpretations/common/getInterpretationAccess.js
+++ b/src/components/Interpretations/common/getInterpretationAccess.js
@@ -1,6 +1,17 @@
+import { isArray } from 'lodash'
+
+// For backwards compatibility
+// accept both Set (from the old d2.currentUser object) and array
+const isSuperuser = (authorities = []) => {
+    if (isArray(authorities)) {
+        return authorities.includes('ALL')
+    }
+    return authorities.has('ALL')
+}
+
 const isCreatorOrSuperuser = (object, currentUser) =>
     object?.createdBy.id === currentUser?.id ||
-    currentUser?.authorities.has('ALL')
+    isSuperuser(currentUser?.authorities)
 
 export const getInterpretationAccess = (interpretation, currentUser) => {
     const canEditDelete = isCreatorOrSuperuser(interpretation, currentUser)