From 39520a8c49037d32860d0638b7325a08d2466168 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?=
 <simen.heggestoyl@origo.oslo.kommune.no>
Date: Fri, 24 Nov 2023 11:59:34 +0100
Subject: [PATCH] Hide admin page link for non-admin users

---
 CHANGELOG.md                                  |  1 +
 .../Navigation/UserMenuDropdown.vue           |  9 ++++---
 src/router/router-guards/routerGuardUtil.js   |  6 ++---
 src/store/index.js                            | 16 ------------
 src/util/user.js                              | 10 ++++++++
 src/views/Admin/AdminWrapper.vue              |  6 ++---
 src/views/Admin/components/AdminUsers.vue     |  7 ++----
 src/views/Item/ItemIntegrations.vue           |  2 +-
 tests/unit/util/user.test.js                  | 25 +++++++++++++++++++
 9 files changed, 50 insertions(+), 32 deletions(-)
 create mode 100644 src/util/user.js
 create mode 100644 tests/unit/util/user.test.js

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 265015b5f..b7583f361 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -53,6 +53,7 @@ All notable changes to this project will be documented in this file. The format
   based on the admin level of the current user.
 - Organization admins should now be allowed to edit own organizations including
   child departments and products.
+- The link to the admin page is no longer visible to non-admin users.
 
 ### Security
 
diff --git a/src/components/Navigation/UserMenuDropdown.vue b/src/components/Navigation/UserMenuDropdown.vue
index 4e01f49c2..c62218fdc 100644
--- a/src/components/Navigation/UserMenuDropdown.vue
+++ b/src/components/Navigation/UserMenuDropdown.vue
@@ -47,7 +47,7 @@
     <div class="user-menu-dropdown__footer">
       <template v-for="link in links">
         <pkt-button
-          v-if="link.show !== undefined ? link.show : true"
+          v-if="link.show"
           :key="`link_${link.key}`"
           size="small"
           skin="tertiary"
@@ -66,8 +66,9 @@
 <script>
 import { mapActions, mapState } from 'vuex';
 import { db, auth } from '@/config/firebaseConfig';
-import User from '@/db/User';
 import { PktButton } from '@oslokommune/punkt-vue2';
+import isAdmin from '@/util/user';
+import User from '@/db/User';
 import UserProfileForm from '@/components/forms/UserProfileForm.vue';
 
 export default {
@@ -117,19 +118,21 @@ export default {
           text: this.$t('general.admin'),
           icon: 'cogwheel',
           route: { name: 'Admin' },
-          show: !!this.user?.admin,
+          show: isAdmin(this.user),
         },
         {
           key: 'api',
           text: this.$t('general.api'),
           icon: 'document-code',
           route: { name: 'Api' },
+          show: true,
         },
         {
           key: 'help',
           text: this.$t('general.help'),
           icon: 'question',
           route: { name: 'Help' },
+          show: true,
         },
       ];
     },
diff --git a/src/router/router-guards/routerGuardUtil.js b/src/router/router-guards/routerGuardUtil.js
index ab5d7c7f1..95b6aa8a2 100644
--- a/src/router/router-guards/routerGuardUtil.js
+++ b/src/router/router-guards/routerGuardUtil.js
@@ -1,5 +1,6 @@
 import { db } from '@/config/firebaseConfig';
 import { firestoreEncode } from '@/util/firebaseUtil';
+import isAdmin from '@/util/user';
 import store from '@/store';
 
 const getSlugRef = async (slug) => {
@@ -15,10 +16,7 @@ const getSlugRef = async (slug) => {
     .then((snap) => snap.data())
     .catch(() => null);
 
-  return !refData ||
-    (refData.archived && (!store.state.user.admin || !store.state.user.superAdmin))
-    ? null
-    : reference;
+  return !refData || (refData.archived && !isAdmin(store.state.user)) ? null : reference;
 };
 
 export default getSlugRef;
diff --git a/src/store/index.js b/src/store/index.js
index 5890b526f..e28f9c9fc 100644
--- a/src/store/index.js
+++ b/src/store/index.js
@@ -33,22 +33,6 @@ export const storeGetters = {
     });
   },
 
-  isAdmin: (state) => {
-    // Returns `true` if user has `admin: true` or if user is member of `activeItem`
-    const { user, activeItem } = state;
-
-    if (user && user.superAdmin) {
-      return true;
-    }
-    if (user && user.admin && user.admin.length > 0) {
-      return true;
-    }
-    if (!user || !activeItem || !activeItem.team) {
-      return false;
-    }
-    return activeItem.team.map(({ id }) => id).includes(user.id);
-  },
-
   /**
    * Returns `true` if the current user is an admin of the parent organization
    * of `activeItem`.
diff --git a/src/util/user.js b/src/util/user.js
new file mode 100644
index 000000000..1381e9e4e
--- /dev/null
+++ b/src/util/user.js
@@ -0,0 +1,10 @@
+/**
+ * Return true if `user` is an admin, otherwise return false.
+ *
+ * "Admin" meaning either a super admin, or admin of any organization.
+ */
+function isAdmin(user) {
+  return Boolean(user && (user.superAdmin || user.admin?.length > 0));
+}
+
+export default isAdmin;
diff --git a/src/views/Admin/AdminWrapper.vue b/src/views/Admin/AdminWrapper.vue
index ba838aa18..de7e537b2 100644
--- a/src/views/Admin/AdminWrapper.vue
+++ b/src/views/Admin/AdminWrapper.vue
@@ -3,18 +3,18 @@
 </template>
 
 <script>
-import { mapState, mapGetters } from 'vuex';
+import { mapState } from 'vuex';
+import isAdmin from '@/util/user';
 
 export default {
   name: 'AdminWrapper',
 
   computed: {
     ...mapState(['user']),
-    ...mapGetters(['isAdmin']),
   },
 
   created() {
-    if (!this.isAdmin && !this.user.superAdmin) {
+    if (!isAdmin(this.user)) {
       this.$router.push({ name: 'Home' });
     }
   },
diff --git a/src/views/Admin/components/AdminUsers.vue b/src/views/Admin/components/AdminUsers.vue
index 3a3ebd11b..b2490753e 100644
--- a/src/views/Admin/components/AdminUsers.vue
+++ b/src/views/Admin/components/AdminUsers.vue
@@ -62,6 +62,7 @@
 import { mapState } from 'vuex';
 import Fuse from 'fuse.js';
 import { PktButton } from '@oslokommune/punkt-vue2';
+import isAdmin from '@/util/user';
 import AddUsers from './AddUsers.vue';
 import EditUser from './EditUser.vue';
 
@@ -121,11 +122,7 @@ export default {
     },
   },
 
-  methods: {
-    isAdmin(user) {
-      return user.admin?.length > 0 || user.superAdmin;
-    },
-  },
+  methods: { isAdmin },
 };
 </script>
 
diff --git a/src/views/Item/ItemIntegrations.vue b/src/views/Item/ItemIntegrations.vue
index de55aaa19..caebbd4c5 100644
--- a/src/views/Item/ItemIntegrations.vue
+++ b/src/views/Item/ItemIntegrations.vue
@@ -111,7 +111,7 @@ export default {
 
   computed: {
     ...mapState(['activeItem', 'activeItemRef']),
-    ...mapGetters(['isAdmin', 'hasEditRights']),
+    ...mapGetters(['hasEditRights']),
   },
 
   watch: {
diff --git a/tests/unit/util/user.test.js b/tests/unit/util/user.test.js
new file mode 100644
index 000000000..7f59f2c04
--- /dev/null
+++ b/tests/unit/util/user.test.js
@@ -0,0 +1,25 @@
+import isAdmin from '../../../src/util/user';
+
+describe('Test admin check', () => {
+  test('unconfigured user is not an admin', () => {
+    expect(isAdmin({})).toBe(false);
+  });
+  test('super admin is an admin', () => {
+    expect(isAdmin({ superAdmin: true })).toBe(true);
+  });
+  test('non-super admin is not an admin', () => {
+    expect(isAdmin({ superAdmin: false })).toBe(false);
+  });
+  test('falsy admin is not an admin', () => {
+    expect(isAdmin({ admin: false })).toBe(false);
+  });
+  test('empty admin is not an admin', () => {
+    expect(isAdmin({ admin: [] })).toBe(false);
+  });
+  test('admin of something is an admin', () => {
+    expect(isAdmin({ admin: ['foo-corp'] })).toBe(true);
+  });
+  test('admin of several things is an admin', () => {
+    expect(isAdmin({ admin: ['foo-corp', 'bar-corp'] })).toBe(true);
+  });
+});