remove assign_user_role's silent defaulting to admin role; remediate …

…some code quality issues flagged by SonarQube
pAI-OS · Jan 16, 2025 · 740996f · 740996f
1 parent 7f397ee
commit 740996f
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 57 deletions.
diff --git a/backend/api/AuthView.py b/backend/api/AuthView.py
@@ -7,6 +7,9 @@
 from backend.models import Session
 from sqlalchemy import delete
 
+GENERIC_AUTH_ERROR = "Something went wrong"
+
+
 class AuthView:
     def __init__(self):
         self.am = AuthManager()
@@ -16,7 +19,7 @@ async def auth_options(self, body: AuthOptionsRequest):
         challenge, options, type = await self.am.auth_options(body["email"])
 
         if not options:
-            return JSONResponse({"error": "Something went wrong"}, status_code=500)
+            return JSONResponse({"error": GENERIC_AUTH_ERROR}, status_code=500)
 
         response = JSONResponse({"options": options, "flow": type}, status_code=200)
         response.set_cookie(key="challenge",value=challenge, secure=True, httponly=True, samesite='strict')
@@ -26,7 +29,7 @@ async def webauthn_register_options(self, body: RegistrationOptions):
         challenge, options = await self.am.webauthn_register_options(body["email"])
 
         if not options:
-            return JSONResponse({"error": "Something went wrong"}, status_code=500)
+            return JSONResponse({"error": GENERIC_AUTH_ERROR}, status_code=500)
 
         response = JSONResponse({"options": options}, status_code=200)
         response.set_cookie(key="challenge",value=challenge, secure=True, httponly=True, samesite='strict')
@@ -36,7 +39,7 @@ async def webauthn_register(self, body: VerifyRegistration):
         challenge = request.cookies.get("challenge")
         res = await self.am.webauthn_register(challenge, body["email"], body["user_id"], body["att_resp"])
         if not res:
-            return JSONResponse({"message": "Something went wrong"}, status_code=401)
+            return JSONResponse({"message": GENERIC_AUTH_ERROR}, status_code=401)
 
         response = JSONResponse({"message": "Success"}, status_code=200)
         response.set_cookie(key="challenge",value="", expires=0,secure=True, httponly=True, samesite='strict')
@@ -47,7 +50,7 @@ async def webauthn_login_options(self, body: AuthenticationOptions):
         challenge, options = await self.am.webauthn_login_options(body["email"])
 
         if not options:
-            return JSONResponse({"error": "Something went wrong"}, status_code=500)
+            return JSONResponse({"error": GENERIC_AUTH_ERROR}, status_code=500)
 
         response = JSONResponse({"options": options}, status_code=200)
         response.set_cookie(key="challenge", value=challenge, secure=True, httponly=True, samesite='strict')
@@ -59,7 +62,7 @@ async def webauthn_login(self, body: VerifyAuthentication):
         if not token:
             return JSONResponse({"message": "Failed"}, status_code=401)
 
-        permissions = self.cb.create_resource_access(role,"ADMIN_PORTAL")
+        permissions = self.cb.get_resource_access(role,"ADMIN_PORTAL")
         response = JSONResponse({"message": "Success", "token": token, "permissions": permissions}, status_code=200)
         response.set_cookie(key="challenge",value="", expires=0,secure=True, httponly=True, samesite='strict')
 

diff --git a/backend/managers/AuthManager.py b/backend/managers/AuthManager.py
@@ -111,6 +111,8 @@ def decode_jwt(token):
 
 
 class AuthManager:
+    # AuthManager: manages authentication processes: registration, login
+
     _instance = None
     _lock = Lock()
 
@@ -303,8 +305,8 @@ async def webauthn_login(self, challenge: str, email_id:str, response):
                 "exp": datetime.utcnow() + timedelta(days=1)
             }
             token = generate_jwt(payload)
-
             return token, role
+
     async def verify_email(self, token: str):
         async with db_session_context() as session:
             user_id = verify_email_token(token)
@@ -320,9 +322,9 @@ async def verify_email(self, token: str):
             user.emailVerified = True
 
             cb = CasbinRoleManager()
-            admin_user = cb.get_admin_users("ADMIN_PORTAL")
-            if not admin_user:
-                cb.assign_user_role(user.id, "ADMIN_PORTAL")
+            admin_users = cb.get_admin_users("ADMIN_PORTAL")
+            if not admin_users:
+                cb.assign_user_role(user.id, "ADMIN_PORTAL", "admin") # assign admin role to first user created
             else:
                 cb.assign_user_role(user.id, "ADMIN_PORTAL", "user")
 

diff --git a/backend/managers/CasbinRoleManager.py b/backend/managers/CasbinRoleManager.py
@@ -46,7 +46,7 @@ def check_permissions(self, user_id, res_act, res_id, domain):
     def get_permissions(self, role, domain):
         return self.enforcer.get_permissions_for_user_in_domain(role, domain)
 
-    def create_resource_access(self, role, domain):
+    def get_resource_access(self, role, domain):
         permissions = self.get_permissions(role, domain)
         output = {}
         for _,_, resource, action in permissions:
@@ -58,7 +58,7 @@ def create_resource_access(self, role, domain):
 
         return output
 
-    def assign_user_role(self, user_id, domain, role="admin"):
+    def assign_user_role(self, user_id, domain, role):
         self.enforcer.add_role_for_user_in_domain(user_id, role, domain)
 
     def get_admin_users(self, domain):

diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx
@@ -21,58 +21,78 @@ import { VerifyEmail } from './VerifyEmail';
 import { hasAccess, ResourcePermissions } from './utils/authUtils';
 
 
+// Resource configuration
+const resourceConfig = [
+  {
+    name: "assets",
+    list: AssetList,
+    create: AssetCreate,
+    edit: AssetEdit,
+    show: AssetShow,
+    icon: DocIcon,
+    recordRepresentation: 'name',
+  },
+  {
+    name: "users",
+    list: UserList,
+    create: UserCreate,
+    edit: UserEdit,
+    show: UserShow,
+    icon: UserIcon,
+    recordRepresentation: 'name',
+  },
+  {
+    name: "abilities",
+    list: AbilityList,
+    show: AbilityShow,
+    icon: ExtensionIcon,
+    recordRepresentation: 'id',
+  },
+  {
+    name: "resources",
+    list: ChannelList,
+    show: ChannelShow,
+    icon: SyncAltIcon,
+    recordRepresentation: 'id',
+  },
+  {
+    name: "downloads",
+    list: DownloadsList,
+  },
+  {
+    name: "shares",
+    list: ShareList,
+    create: ShareCreate,
+    edit: ShareEdit,
+    show: ShareShow,
+    icon: LinkIcon,
+    recordRepresentation: 'id',
+  },
+];
+
 const renderResources: RenderResourcesFunction = (permissions: ResourcePermissions) => (
   <>
-    {hasAccess("assets", "list", permissions) ?
-      <Resource
-        name="assets"
-        list={AssetList}
-        create={hasAccess("assets", "create", permissions) ? AssetCreate : undefined}
-        edit={hasAccess("assets", "edit", permissions) ? AssetEdit : undefined}
-        show={hasAccess("assets", "show", permissions) ? AssetShow : undefined}
-        recordRepresentation='name'
-        icon={DocIcon} /> : null}
-    {hasAccess("users", "list", permissions) ?
-      <Resource
-        name="users"
-        list={UserList}
-        create={hasAccess("users", "create", permissions) ? UserCreate : undefined}
-        edit={hasAccess("users", "edit", permissions) ? UserEdit : undefined}
-        show={hasAccess("users", "show", permissions) ? UserShow : undefined}
-        recordRepresentation='name'
-        icon={UserIcon} /> : null}
-    {hasAccess("abilities", "list", permissions) ?
-      <Resource
-        name="abilities"
-        list={AbilityList}
-        show={hasAccess("abilities", "show", permissions) ? AbilityShow : undefined}
-        recordRepresentation='id'
-        icon={ExtensionIcon} /> : null}
-    {hasAccess("resources", "list", permissions) ?
-      <Resource
-        name="resources"
-        list={ChannelList}
-        show={hasAccess("abilities", "show", permissions) ? ChannelShow : undefined}
-        recordRepresentation='id'
-        icon={SyncAltIcon} /> : null}
-    {hasAccess("downloads", "list", permissions) ?
-      <Resource
-        name="downloads"
-        list={DownloadsList} /> : null}
-    {hasAccess("shares", "list", permissions) ?
-      <Resource
-        name="shares"
-        list={ShareList}
-        create={hasAccess("shares", "create", permissions) ? ShareCreate : undefined}
-        edit={hasAccess("shares", "edit", permissions) ? ShareEdit : undefined}
-        show={hasAccess("shares", "show", permissions) ? ShareShow : undefined}
-        recordRepresentation='id'
-        icon={LinkIcon} /> : null}
+    {resourceConfig.map(resource => {
+      if (!hasAccess(resource.name, "list", permissions)) return null;
+
+      return (
+        <Resource
+          key={resource.name}
+          name={resource.name}
+          list={resource.list}
+          create={hasAccess(resource.name, "create", permissions) ? resource.create : undefined}
+          edit={hasAccess(resource.name, "edit", permissions) ? resource.edit : undefined}
+          show={hasAccess(resource.name, "show", permissions) ? resource.show : undefined}
+          icon={resource.icon}
+          recordRepresentation={resource.recordRepresentation}
+        />
+      );
+    })}
     <CustomRoutes noLayout>
       <Route path='/verify-email/:token' element={<VerifyEmail />} />
     </CustomRoutes>
   </>
-)
+);
 
 export const App = () => (
   <Admin