Skip to content

Commit cb9c447

Browse files
longshuicyddey2tcnichol
authored
Refactor create superadmin mode (#851)
* Basic implementation of admin in user collection in mongodb * black formatting * adding codegen files * minor fix to boolean logic * Adding get_admin dependency black formatting adding codegen file removing redundant print statement fixing pytest failure fixing pytest failure ran black * fixed pytest failure * Adding admin dependency in authorization of dataset,files, metadata, search * allowing admin to view all datasets * adding admin in get_roles functions * adding codegen files * modifying test * addressing commnets * adding the test back * Admin mode implementation * removing redundant things * small fix. the name of the method for file was off so there was an error getting file role * fix to visualization rendering * backend change to depends * codegen * remove all the adminMode parameter * fix bug * black * admin can be a part of the profile; no need for additional endpoint * fix drop/enable admin toggle * simplify admin logic * rewrite backend * codegen * add endpoint to set admin * fix bug * pytest, black format and codegen * wire in the frontend toggle action * setting is working but other bugs * fix admin toggle * add admin mode toggle to every listing page * fix typo and bug in groups * only check admin in authorization dependency * remove unnecessary admin_mode * codegen/black and remove redandunt admin_mode flag * fix bug in dataset * fix search clause * add more trigger on adminMode * fix two bugs * fix bug on error --------- Co-authored-by: Dipannita Dey <[email protected]> Co-authored-by: toddn <[email protected]>
1 parent a5bc432 commit cb9c447

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+1141
-573
lines changed

backend/app/deps/authorization_deps.py

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,29 @@
11
from beanie import PydanticObjectId
22
from beanie.operators import Or
3-
from bson import ObjectId
43
from fastapi import Depends, HTTPException
54

65
from app.keycloak_auth import get_current_username
76
from app.models.authorization import RoleType, AuthorizationDB
87
from app.models.datasets import DatasetDB, DatasetStatus
9-
from app.models.files import FileOut, FileDB
10-
from app.models.groups import GroupOut, GroupDB
8+
from app.models.files import FileDB
9+
from app.models.groups import GroupDB
1110
from app.models.metadata import MetadataDB
1211
from app.models.pyobjectid import PyObjectId
1312
from app.routers.authentication import get_admin
13+
from app.routers.authentication import get_admin_mode
1414

1515

1616
async def get_role(
1717
dataset_id: str,
1818
current_user=Depends(get_current_username),
19+
admin_mode: bool = Depends(get_admin_mode),
20+
admin=Depends(get_admin),
1921
) -> RoleType:
2022
"""Returns the role a specific user has on a dataset. If the user is a creator (owner), they are not listed in
2123
the user_ids list."""
24+
if admin and admin_mode:
25+
return RoleType.OWNER
26+
2227
authorization = await AuthorizationDB.find_one(
2328
AuthorizationDB.dataset_id == PyObjectId(dataset_id),
2429
Or(
@@ -32,7 +37,12 @@ async def get_role(
3237
async def get_role_by_file(
3338
file_id: str,
3439
current_user=Depends(get_current_username),
40+
admin_mode: bool = Depends(get_admin_mode),
41+
admin=Depends(get_admin),
3542
) -> RoleType:
43+
if admin and admin_mode:
44+
return RoleType.OWNER
45+
3646
if (file := await FileDB.get(PydanticObjectId(file_id))) is not None:
3747
authorization = await AuthorizationDB.find_one(
3848
AuthorizationDB.dataset_id == file.dataset_id,
@@ -66,7 +76,12 @@ async def get_role_by_file(
6676
async def get_role_by_metadata(
6777
metadata_id: str,
6878
current_user=Depends(get_current_username),
79+
admin_mode: bool = Depends(get_admin_mode),
80+
admin=Depends(get_admin),
6981
) -> RoleType:
82+
if admin and admin_mode:
83+
return RoleType.OWNER
84+
7085
if (md_out := await MetadataDB.get(PydanticObjectId(metadata_id))) is not None:
7186
resource_type = md_out.resource.collection
7287
resource_id = md_out.resource.resource_id
@@ -97,7 +112,12 @@ async def get_role_by_metadata(
97112
async def get_role_by_group(
98113
group_id: str,
99114
current_user=Depends(get_current_username),
115+
admin_mode: bool = Depends(get_admin_mode),
116+
admin=Depends(get_admin),
100117
) -> RoleType:
118+
if admin and admin_mode:
119+
return RoleType.OWNER
120+
101121
if (group := await GroupDB.get(group_id)) is not None:
102122
if group.creator == current_user:
103123
# Creator can do everything
@@ -148,12 +168,13 @@ async def __call__(
148168
self,
149169
dataset_id: str,
150170
current_user: str = Depends(get_current_username),
171+
admin_mode: bool = Depends(get_admin_mode),
151172
admin: bool = Depends(get_admin),
152173
):
153174
# TODO: Make sure we enforce only one role per user per dataset, or find_one could yield wrong answer here.
154175

155-
# If the current user is admin, user has access irrespective of any role assigned
156-
if admin:
176+
# If the current user is admin and has turned on admin_mode, user has access irrespective of any role assigned
177+
if admin and admin_mode:
157178
return True
158179

159180
# Else check role assigned to the user
@@ -204,10 +225,11 @@ async def __call__(
204225
self,
205226
file_id: str,
206227
current_user: str = Depends(get_current_username),
228+
admin_mode: bool = Depends(get_admin_mode),
207229
admin: bool = Depends(get_admin),
208230
):
209-
# If the current user is admin, user has access irrespective of any role assigned
210-
if admin:
231+
# If the current user is admin and has turned on admin_mode, user has access irrespective of any role assigned
232+
if admin and admin_mode:
211233
return True
212234

213235
# Else check role assigned to the user
@@ -241,10 +263,11 @@ async def __call__(
241263
self,
242264
metadata_id: str,
243265
current_user: str = Depends(get_current_username),
266+
admin_mode: bool = Depends(get_admin_mode),
244267
admin: bool = Depends(get_admin),
245268
):
246-
# If the current user is admin, user has access irrespective of any role assigned
247-
if admin:
269+
# If the current user is admin and has turned on admin_mode, user has access irrespective of any role assigned
270+
if admin and admin_mode:
248271
return True
249272

250273
# Else check role assigned to the user
@@ -307,10 +330,11 @@ async def __call__(
307330
self,
308331
group_id: str,
309332
current_user: str = Depends(get_current_username),
333+
admin_mode: bool = Depends(get_admin_mode),
310334
admin: bool = Depends(get_admin),
311335
):
312-
# If the current user is admin, user has access irrespective of any role assigned
313-
if admin:
336+
# If the current user is admin and has turned on admin_mode, user has access irrespective of any role assigned
337+
if admin and admin_mode:
314338
return True
315339

316340
# Else check role assigned to the user
@@ -377,9 +401,14 @@ async def __call__(
377401
return False
378402

379403

380-
def access(user_role: RoleType, role_required: RoleType) -> bool:
381-
"""Enforce implied role hierarchy OWNER > EDITOR > UPLOADER > VIEWER"""
382-
if user_role == RoleType.OWNER:
404+
def access(
405+
user_role: RoleType,
406+
role_required: RoleType,
407+
admin_mode: bool = Depends(get_admin_mode),
408+
admin: bool = Depends(get_admin),
409+
) -> bool:
410+
"""Enforce implied role hierarchy ADMIN = OWNER > EDITOR > UPLOADER > VIEWER"""
411+
if user_role == RoleType.OWNER or (admin and admin_mode):
383412
return True
384413
elif user_role == RoleType.EDITOR and role_required in [
385414
RoleType.EDITOR,

backend/app/models/users.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class UserLogin(BaseModel):
2525

2626
class UserDoc(Document, UserBase):
2727
admin: bool
28+
admin_mode: bool = False
2829

2930
class Settings:
3031
name = "users"

backend/app/routers/authentication.py

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ async def authenticate_user(email: str, password: str):
9292
return user
9393

9494

95-
async def get_admin(dataset_id: str = None, current_username=Depends(get_current_user)):
95+
@router.get("/users/me/is_admin", response_model=bool)
96+
async def get_admin(
97+
dataset_id: str = None, current_username=Depends(get_current_user)
98+
) -> bool:
9699
if (
97100
current_user := await UserDB.find_one(UserDB.email == current_username.email)
98101
) is not None:
@@ -103,11 +106,59 @@ async def get_admin(dataset_id: str = None, current_username=Depends(get_current
103106
and (dataset_db := await DatasetDB.get(PydanticObjectId(dataset_id)))
104107
is not None
105108
):
109+
# TODO: question regarding resource creator is considered as admin of the resource?
106110
return dataset_db.creator.email == current_username.email
107111
else:
108112
return False
109113

110114

115+
@router.get("/users/me/admin_mode")
116+
async def get_admin_mode(current_username=Depends(get_current_user)) -> bool:
117+
"""Get Admin mode from User Object."""
118+
if (
119+
current_user := await UserDB.find_one(UserDB.email == current_username.email)
120+
) is not None:
121+
if current_user.admin_mode is not None:
122+
return current_user.admin_mode
123+
else:
124+
return False
125+
else:
126+
raise HTTPException(
127+
status_code=404,
128+
detail="User doesn't exist.",
129+
headers={"WWW-Authenticate": "Bearer"},
130+
)
131+
132+
133+
@router.post("/users/me/admin_mode", response_model=bool)
134+
async def set_admin_mode(
135+
admin_mode_on: bool,
136+
admin=Depends(get_admin),
137+
current_username=Depends(get_current_user),
138+
) -> bool:
139+
"""Set Admin mode from User Object."""
140+
if (
141+
current_user := await UserDB.find_one(UserDB.email == current_username.email)
142+
) is not None:
143+
# only admin can set admin mode
144+
if admin:
145+
current_user.admin_mode = admin_mode_on
146+
await current_user.replace()
147+
return current_user.admin_mode
148+
else:
149+
raise HTTPException(
150+
status_code=403,
151+
detail="You are not admin yet. Only admin can set admin mode.",
152+
headers={"WWW-Authenticate": "Bearer"},
153+
)
154+
else:
155+
raise HTTPException(
156+
status_code=404,
157+
detail="User doesn't exist.",
158+
headers={"WWW-Authenticate": "Bearer"},
159+
)
160+
161+
111162
@router.post("/users/set_admin/{useremail}", response_model=UserOut)
112163
async def set_admin(
113164
useremail: str, current_username=Depends(get_current_user), admin=Depends(get_admin)

backend/app/routers/authorization.py

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from bson import ObjectId
44
from fastapi import APIRouter, Depends
55
from fastapi.exceptions import HTTPException
6+
67
from app.dependencies import get_elasticsearchclient
78
from app.deps.authorization_deps import (
89
Authorization,
@@ -29,7 +30,7 @@
2930
from app.models.groups import GroupDB
3031
from app.models.pyobjectid import PyObjectId
3132
from app.models.users import UserDB
32-
from app.routers.authentication import get_admin
33+
from app.routers.authentication import get_admin, get_admin_mode
3334
from app.search.index import index_dataset
3435

3536
router = APIRouter()
@@ -69,22 +70,24 @@ async def save_authorization(
6970
async def get_dataset_role(
7071
dataset_id: str,
7172
current_user=Depends(get_current_username),
73+
admin_mode: bool = Depends(get_admin_mode),
7274
admin=Depends(get_admin),
7375
):
7476
"""Retrieve role of user for a specific dataset."""
7577
# Get group id and the associated users from authorization
76-
if admin:
77-
auth_db = await AuthorizationDB.find_one(
78-
AuthorizationDB.dataset_id == PyObjectId(dataset_id)
79-
)
80-
else:
81-
auth_db = await AuthorizationDB.find_one(
82-
AuthorizationDB.dataset_id == PyObjectId(dataset_id),
78+
criteria = []
79+
if not admin or not admin_mode:
80+
criteria.append(
8381
Or(
8482
AuthorizationDB.creator == current_user,
8583
AuthorizationDB.user_ids == current_user,
86-
),
84+
)
8785
)
86+
87+
auth_db = await AuthorizationDB.find_one(
88+
AuthorizationDB.dataset_id == PyObjectId(dataset_id),
89+
*criteria,
90+
)
8891
if auth_db is None:
8992
if (
9093
current_dataset := await DatasetDB.get(PydanticObjectId(dataset_id))
@@ -107,18 +110,20 @@ async def get_dataset_role(
107110
return auth_db.dict()
108111

109112

110-
@router.get("/datasets/{dataset_id}/role/viewer")
113+
@router.get("/datasets/{dataset_id}/role/viewer}")
111114
async def get_dataset_role_viewer(
112-
dataset_id: str, allow: bool = Depends(Authorization("viewer"))
115+
dataset_id: str,
116+
allow: bool = Depends(Authorization("viewer")),
113117
):
114118
"""Used for testing only. Returns true if user has viewer permission on dataset, otherwise throws a 403 Forbidden HTTP exception.
115119
See `routers/authorization.py` for more info."""
116120
return {"dataset_id": dataset_id, "allow": allow}
117121

118122

119-
@router.get("/datasets/{dataset_id}/role/owner")
123+
@router.get("/datasets/{dataset_id}/role/owner}")
120124
async def get_dataset_role_owner(
121-
dataset_id: str, allow: bool = Depends(Authorization("owner"))
125+
dataset_id: str,
126+
allow: bool = Depends(Authorization("owner")),
122127
):
123128
"""Used for testing only. Returns true if user has owner permission on dataset, otherwise throws a 403 Forbidden HTTP exception.
124129
See `routers/authorization.py` for more info."""
@@ -130,25 +135,17 @@ async def get_file_role(
130135
file_id: str,
131136
current_user=Depends(get_current_username),
132137
role: RoleType = Depends(get_role_by_file),
133-
admin=Depends(get_admin),
134138
):
135-
# admin is a superuser and has all the privileges
136-
if admin:
137-
return RoleType.OWNER
138139
"""Retrieve role of user for an individual file. Role cannot change between file versions."""
139140
return role
140141

141142

142-
@router.get("/metadata/{metadata_id}/role", response_model=AuthorizationMetadata)
143+
@router.get("/metadata/{metadata_id}/role}", response_model=AuthorizationMetadata)
143144
async def get_metadata_role(
144145
metadata_id: str,
145146
current_user=Depends(get_current_username),
146147
role: RoleType = Depends(get_role_by_metadata),
147-
admin=Depends(get_admin),
148148
):
149-
# admin is a superuser and has all the privileges
150-
if admin:
151-
return RoleType.OWNER
152149
"""Retrieve role of user for group. Group roles can be OWNER, EDITOR, or VIEWER (for regular Members)."""
153150
return role
154151

@@ -158,11 +155,7 @@ async def get_group_role(
158155
group_id: str,
159156
current_user=Depends(get_current_username),
160157
role: RoleType = Depends(get_role_by_group),
161-
admin=Depends(get_admin),
162158
):
163-
# admin is a superuser and has all the privileges
164-
if admin:
165-
return RoleType.OWNER
166159
"""Retrieve role of user on a particular group (i.e. whether they can change group memberships)."""
167160
return role
168161

@@ -342,7 +335,7 @@ async def remove_dataset_user_role(
342335
raise HTTPException(status_code=404, detail=f"Dataset {dataset_id} not found")
343336

344337

345-
@router.get("/datasets/{dataset_id}/roles", response_model=DatasetRoles)
338+
@router.get("/datasets/{dataset_id}/roles}", response_model=DatasetRoles)
346339
async def get_dataset_roles(
347340
dataset_id: str,
348341
allow: bool = Depends(Authorization("editor")),

backend/app/routers/datasets.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@
5555
from app.models.users import UserOut
5656
from app.rabbitmq.listeners import submit_dataset_job
5757
from app.routers.authentication import get_admin
58-
from app.routers.files import add_file_entry, add_local_file_entry, remove_file_entry
58+
from app.routers.authentication import get_admin_mode
59+
from app.routers.files import add_file_entry, remove_file_entry, add_local_file_entry
5960
from app.search.connect import (
6061
delete_document_by_id,
6162
)
@@ -212,8 +213,9 @@ async def get_datasets(
212213
limit: int = 10,
213214
mine: bool = False,
214215
admin=Depends(get_admin),
216+
admin_mode: bool = Depends(get_admin_mode),
215217
):
216-
if admin:
218+
if admin and admin_mode:
217219
datasets = await DatasetDBViewList.find(
218220
sort=(-DatasetDBViewList.created),
219221
skip=skip,
@@ -257,10 +259,10 @@ async def get_dataset_files(
257259
dataset_id: str,
258260
folder_id: Optional[str] = None,
259261
authenticated: bool = Depends(CheckStatus("AUTHENTICATED")),
260-
allow: bool = Depends(Authorization("viewer")),
261262
user_id=Depends(get_user),
262263
skip: int = 0,
263264
limit: int = 10,
265+
allow: bool = Depends(Authorization("viewer")),
264266
):
265267
if authenticated:
266268
query = [
@@ -380,10 +382,10 @@ async def get_dataset_folders(
380382
dataset_id: str,
381383
parent_folder: Optional[str] = None,
382384
user_id=Depends(get_user),
383-
allow: bool = Depends(Authorization("viewer")),
384385
authenticated: bool = Depends(CheckStatus("authenticated")),
385386
skip: int = 0,
386387
limit: int = 10,
388+
allow: bool = Depends(Authorization("viewer")),
387389
):
388390
if (await DatasetDB.get(PydanticObjectId(dataset_id))) is not None:
389391
if authenticated:

0 commit comments

Comments
 (0)