Skip to content

Commit ce978cf

Browse files
tcnicholddey2
andauthored
866 readonly user (#974)
* adding readonly user initial commit * adding read only boolean to profile * access checks read only user * place holder need to make sure read only users can only be viewers * can only add user as viewer but front end does not get the error * throws error but says added * adding a todo * shows success if user was added * remove console logs * dataset roles do not update after we add user, not sure what is wrong * formatting * add the read only users separately * other auth for read only users * return viewer for user in group that is readonly * routes for enable/disable read only user * ran codegen * button toggle does not curently work * fixing the logic to handle the action for enable/disable readonly users and update the users in store * backend - prevent read only user from being an admin * fix exception raise * formatting * Fixed test. Problem was that the user_id was being removed from auth.user_ids when the user_ids was empty. Added a check to only try to remove user_id if user_id was in the list. * disable read only user if user is an admin * add set read only user method to replace the enable and disable methods * formatting * adding new method * using old action * fixing using imported as name * wrong backend method was being called for disable readonly * using helper method now for chaning read only unused back end methods removed from backend and corresponding front end * fix package lock file * ran codegen * cannot make an admin a read only user * read only user cannot create new dataset read only user cannot add metadata definition or delete * read only user cannot delete metadata definition * cannot make admins read only, only need to check for admin * wrong function call in service can now make read only user regular user again * fix problem with making a user read only ,then admin and it freezes eliminate console log * formatting --------- Co-authored-by: Dipannita Dey <[email protected]>
1 parent 87d180a commit ce978cf

File tree

19 files changed

+541
-97
lines changed

19 files changed

+541
-97
lines changed

backend/app/deps/authorization_deps.py

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
from app.keycloak_auth import get_current_username
1+
from beanie import PydanticObjectId
2+
from beanie.operators import Or
3+
from fastapi import Depends, HTTPException
4+
from app.keycloak_auth import get_current_username, get_read_only_user
25
from app.models.authorization import AuthorizationDB, RoleType
36
from app.models.datasets import DatasetDB, DatasetStatus
47
from app.models.files import FileDB, FileStatus
@@ -196,6 +199,7 @@ async def __call__(
196199
current_user: str = Depends(get_current_username),
197200
admin_mode: bool = Depends(get_admin_mode),
198201
admin: bool = Depends(get_admin),
202+
readonly: bool = Depends(get_read_only_user),
199203
):
200204
# TODO: Make sure we enforce only one role per user per dataset, or find_one could yield wrong answer here.
201205

@@ -476,20 +480,34 @@ def access(
476480
role_required: RoleType,
477481
admin_mode: bool = Depends(get_admin_mode),
478482
admin: bool = Depends(get_admin),
483+
read_only_user: bool = Depends(get_read_only_user),
479484
) -> bool:
485+
# check for read only user first
486+
if read_only_user and role_required == RoleType.VIEWER:
487+
return True
480488
"""Enforce implied role hierarchy ADMIN = OWNER > EDITOR > UPLOADER > VIEWER"""
481489
if user_role == RoleType.OWNER or (admin and admin_mode):
482490
return True
483-
elif user_role == RoleType.EDITOR and role_required in [
484-
RoleType.EDITOR,
485-
RoleType.UPLOADER,
486-
RoleType.VIEWER,
487-
]:
491+
elif (
492+
user_role == RoleType.EDITOR
493+
and role_required
494+
in [
495+
RoleType.EDITOR,
496+
RoleType.UPLOADER,
497+
RoleType.VIEWER,
498+
]
499+
and not read_only_user
500+
):
488501
return True
489-
elif user_role == RoleType.UPLOADER and role_required in [
490-
RoleType.UPLOADER,
491-
RoleType.VIEWER,
492-
]:
502+
elif (
503+
user_role == RoleType.UPLOADER
504+
and role_required
505+
in [
506+
RoleType.UPLOADER,
507+
RoleType.VIEWER,
508+
]
509+
and not read_only_user
510+
):
493511
return True
494512
elif user_role == RoleType.VIEWER and role_required == RoleType.VIEWER:
495513
return True

backend/app/keycloak_auth.py

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,91 @@ async def get_current_username(
312312
)
313313

314314

315+
async def get_read_only_user(
316+
token: str = Security(oauth2_scheme),
317+
api_key: str = Security(api_key_header),
318+
token_cookie: str = Security(jwt_header),
319+
) -> bool:
320+
"""Retrieve the user object from Mongo by first getting user id from JWT and then querying Mongo.
321+
Potentially expensive. Use `get_current_username` if all you need is user name.
322+
"""
323+
324+
if token:
325+
try:
326+
userinfo = keycloak_openid.userinfo(token)
327+
user = await UserDB.find_one(UserDB.email == userinfo["email"])
328+
return user.read_only_user
329+
except KeycloakAuthenticationError as e:
330+
raise HTTPException(
331+
status_code=e.response_code,
332+
detail=json.loads(e.error_message),
333+
headers={"WWW-Authenticate": "Bearer"},
334+
)
335+
if token_cookie:
336+
try:
337+
userinfo = keycloak_openid.userinfo(token_cookie.removeprefix("Bearer%20"))
338+
user = await UserDB.find_one(UserDB.email == userinfo["email"])
339+
return user.read_only_user
340+
# expired token
341+
except KeycloakAuthenticationError as e:
342+
raise HTTPException(
343+
status_code=e.response_code,
344+
detail=json.loads(e.error_message),
345+
headers={"WWW-Authenticate": "Bearer"},
346+
)
347+
348+
if api_key:
349+
serializer = URLSafeSerializer(settings.local_auth_secret, salt="api_key")
350+
try:
351+
payload = serializer.loads(api_key)
352+
# Key is valid, check expiration date in database
353+
if (
354+
key := await ListenerAPIKeyDB.find_one(
355+
ListenerAPIKeyDB.user == payload["user"],
356+
ListenerAPIKeyDB.key == payload["key"],
357+
)
358+
) is not None:
359+
user = await UserDB.find_one(UserDB.email == key.user)
360+
return user.read_only_user
361+
elif (
362+
key := await UserAPIKeyDB.find_one(
363+
UserAPIKeyDB.user == payload["user"],
364+
UserAPIKeyDB.key == payload["key"],
365+
)
366+
) is not None:
367+
current_time = datetime.utcnow()
368+
369+
if key.expires is not None and current_time >= key.expires:
370+
# Expired key, delete it first
371+
await key.delete()
372+
raise HTTPException(
373+
status_code=401,
374+
detail={"error": "Key is expired."},
375+
headers={"WWW-Authenticate": "Bearer"},
376+
)
377+
else:
378+
user = await UserDB.find_one(UserDB.email == key.user)
379+
return user.read_only_user
380+
else:
381+
raise HTTPException(
382+
status_code=401,
383+
detail={"error": "Key is invalid."},
384+
headers={"WWW-Authenticate": "Bearer"},
385+
)
386+
except BadSignature as e:
387+
raise HTTPException(
388+
status_code=401,
389+
detail={"error": "Key is invalid."},
390+
headers={"WWW-Authenticate": "Bearer"},
391+
)
392+
393+
raise HTTPException(
394+
status_code=401,
395+
detail="Not authenticated.", # "token expired",
396+
headers={"WWW-Authenticate": "Bearer"},
397+
)
398+
399+
315400
async def get_current_user_id(identity: Json = Depends(get_token)) -> str:
316401
"""Retrieve internal Keycloak id. Does not query MongoDB."""
317402
keycloak_id = identity["sub"]

backend/app/models/users.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class UserLogin(BaseModel):
2626
class UserDoc(Document, UserBase):
2727
admin: bool = False
2828
admin_mode: bool = False
29+
read_only_user: bool = False
2930

3031
class Settings:
3132
name = "users"

backend/app/routers/authentication.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,54 @@ async def revoke_admin(
206206
)
207207

208208

209+
@router.post("/users/enable_readonly/{useremail}", response_model=UserOut)
210+
async def enable_readonly_user(
211+
useremail: str, current_username=Depends(get_current_user), admin=Depends(get_admin)
212+
):
213+
if admin:
214+
if (user := await UserDB.find_one(UserDB.email == useremail)) is not None:
215+
if not user.admin:
216+
user.read_only_user = True
217+
await user.replace()
218+
return user.dict()
219+
else:
220+
raise HTTPException(
221+
status_code=403,
222+
detail=f"User {useremail} is admin cannot be read only",
223+
)
224+
else:
225+
raise HTTPException(status_code=404, detail=f"User {useremail} not found")
226+
else:
227+
raise HTTPException(
228+
status_code=403,
229+
detail=f"User {current_username.email} is not an admin. Only admin can make others admin.",
230+
)
231+
232+
233+
@router.post("/users/disable_readonly/{useremail}", response_model=UserOut)
234+
async def disable_readonly_user(
235+
useremail: str, current_username=Depends(get_current_user), admin=Depends(get_admin)
236+
):
237+
if admin:
238+
if (user := await UserDB.find_one(UserDB.email == useremail)) is not None:
239+
if not user.admin:
240+
user.read_only_user = False
241+
await user.replace()
242+
return user.dict()
243+
else:
244+
raise HTTPException(
245+
status_code=403,
246+
detail=f"User {useremail} is admin cannot be read only",
247+
)
248+
else:
249+
raise HTTPException(status_code=404, detail=f"User {useremail} not found")
250+
else:
251+
raise HTTPException(
252+
status_code=403,
253+
detail=f"User {current_username.email} is not an admin. Only admin can make others admin.",
254+
)
255+
256+
209257
@router.post("/users/enable/{useremail}", response_model=UserOut)
210258
async def user_enable(
211259
useremail: str, current_username=Depends(get_current_user), admin=Depends(get_admin)

backend/app/routers/authorization.py

Lines changed: 50 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,12 @@ async def get_group_role(
158158
role: RoleType = Depends(get_role_by_group),
159159
):
160160
"""Retrieve role of user on a particular group (i.e. whether they can change group memberships)."""
161-
return role
161+
if (user := await UserDB.find_one(UserDB.email == current_user)) is not None:
162+
# return viewer if read only user
163+
if user.read_only_user:
164+
return RoleType.VIEWER
165+
else:
166+
return role
162167

163168

164169
@router.post(
@@ -186,25 +191,60 @@ async def set_dataset_group_role(
186191
) is not None:
187192
if group_id not in auth_db.group_ids:
188193
auth_db.group_ids.append(group_id)
194+
readonly_user_ids = []
189195
for u in group.users:
190-
auth_db.user_ids.append(u.user.email)
196+
if u.user.read_only_user:
197+
readonly_user_ids.append(u.user.email)
198+
else:
199+
auth_db.user_ids.append(u.user.email)
191200
await auth_db.replace()
192-
await index_dataset(es, DatasetOut(**dataset.dict()), auth_db.user_ids)
201+
await index_dataset(
202+
es, DatasetOut(**dataset.dict()), auth_db.user_ids
203+
)
204+
if len(readonly_user_ids) > 0:
205+
readonly_auth_db = AuthorizationDB(
206+
creator=user_id,
207+
dataset_id=PydanticObjectId(dataset_id),
208+
role=RoleType.VIEWER,
209+
group_ids=[PydanticObjectId(group_id)],
210+
user_ids=readonly_user_ids,
211+
)
212+
await readonly_auth_db.insert()
213+
await index_dataset(
214+
es, DatasetOut(**dataset.dict()), readonly_auth_db.user_ids
215+
)
193216
return auth_db.dict()
194217
else:
195218
# Create new role entry for this dataset
196219
user_ids = []
220+
readonly_user_ids = []
197221
for u in group.users:
198-
user_ids.append(u.user.email)
222+
if u.user.read_only_user:
223+
readonly_user_ids.append(u.user.email)
224+
else:
225+
user_ids.append(u.user.email)
226+
# add the users who get the role
199227
auth_db = AuthorizationDB(
200228
creator=user_id,
201229
dataset_id=PydanticObjectId(dataset_id),
202230
role=role,
203231
group_ids=[PydanticObjectId(group_id)],
204232
user_ids=user_ids,
205233
)
234+
readonly_auth_db = AuthorizationDB(
235+
creator=user_id,
236+
dataset_id=PydanticObjectId(dataset_id),
237+
role=RoleType.VIEWER,
238+
group_ids=[PydanticObjectId(group_id)],
239+
user_ids=readonly_user_ids,
240+
)
241+
# if there are read only users add them with the role of viewer
206242
await auth_db.insert()
207243
await index_dataset(es, DatasetOut(**dataset.dict()), auth_db.user_ids)
244+
await readonly_auth_db.insert()
245+
await index_dataset(
246+
es, DatasetOut(**dataset.dict()), readonly_auth_db.user_ids
247+
)
208248
return auth_db.dict()
209249
else:
210250
raise HTTPException(status_code=404, detail=f"Group {group_id} not found")
@@ -227,13 +267,18 @@ async def set_dataset_user_role(
227267
"""Assign a single user a specific role for a dataset."""
228268

229269
if (dataset := await DatasetDB.get(PydanticObjectId(dataset_id))) is not None:
230-
if (await UserDB.find_one(UserDB.email == username)) is not None:
270+
if (user := await UserDB.find_one(UserDB.email == username)) is not None:
231271
# First, remove any existing role the user has on the dataset
232272
await remove_dataset_user_role(dataset_id, username, es, user_id, allow)
233273
auth_db = await AuthorizationDB.find_one(
234274
AuthorizationDB.dataset_id == PydanticObjectId(dataset_id),
235275
AuthorizationDB.role == role,
236276
)
277+
rolename = role.name
278+
if user.read_only_user and role.name is not RoleType.VIEWER.name:
279+
raise HTTPException(
280+
status_code=405, detail=f"User {username} is read only"
281+
)
237282
if auth_db is not None and username not in auth_db.user_ids:
238283
auth_db.user_ids.append(username)
239284
await auth_db.save()

backend/app/routers/groups.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,9 @@ async def remove_member(
271271
# Remove user from all affected Authorization entries
272272
# TODO not sure if this is right
273273
async for auth in AuthorizationDB.find({"group_ids": ObjectId(group_id)}):
274-
auth.user_ids.remove(username)
275-
await auth.replace()
274+
if username in auth.user_ids:
275+
auth.user_ids.remove(username)
276+
await auth.replace()
276277

277278
# Update group itself
278279
group.users.remove(found_user)

frontend/src/actions/user.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,3 +303,40 @@ export function revokeAdmin(email) {
303303
});
304304
};
305305
}
306+
307+
308+
export const ENABLE_READONLY = "ENABLE_READONLY";
309+
310+
export function enableReadOnly(email) {
311+
return (dispatch) => {
312+
return V2.LoginService.enableReadonlyUserApiV2UsersEnableReadonlyUseremailPost(email)
313+
.then((json) => {
314+
dispatch({
315+
type: ENABLE_READONLY,
316+
profile: json,
317+
receivedAt: Date.now(),
318+
});
319+
})
320+
.catch((reason) => {
321+
dispatch(handleErrors(reason, enableReadOnly(email)));
322+
});
323+
};
324+
}
325+
326+
export const DISABLE_READONLY = "DISABLE_READONLY";
327+
328+
export function disableReadOnly(email) {
329+
return (dispatch) => {
330+
return V2.LoginService.disableReadonlyUserApiV2UsersDisableReadonlyUseremailPost(email)
331+
.then((json) => {
332+
dispatch({
333+
type: DISABLE_READONLY,
334+
profile: json,
335+
receivedAt: Date.now(),
336+
});
337+
})
338+
.catch((reason) => {
339+
dispatch(handleErrors(reason, disableReadOnly(email)));
340+
});
341+
};
342+
}

0 commit comments

Comments
 (0)