Skip to content

Commit

Permalink
Add router to get limited user information by Orcid ID
Browse files Browse the repository at this point in the history
This router exposes limited user information (first name, last name,
and username/Orcid ID) to non-admin users. Previously, non-admin
users could not look up users at all.
Non-self user lookup by non-admin users is necessary for adding
users to a collection in the UI.
Use Starlette custom type convertor to determine whether the provided
ID in the router path is an Orcid ID. If so, limited information about
the user will be returned, whether or not the requesting user has admin
privileges. If the ID in the router path is an integer (database ID),
the previously existing router will be used, which requires admin privileges
and returns the full admin view of the user. We may choose to combine these
routers in the future, in order to only allow user lookup by Orcid ID/username,
unless there is a use case for looking up by database ID.
Note that the custom type convertor requires an actual Orcid ID, so
usernames that are not Orcid IDs cannot be used for user lookup.
  • Loading branch information
sallybg committed Nov 26, 2024
1 parent 6bcfcf1 commit 933b7d4
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 3 deletions.
10 changes: 10 additions & 0 deletions src/mavedb/lib/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@


class Action(Enum):
LOOKUP = "lookup"
READ = "read"
UPDATE = "update"
DELETE = "delete"
Expand Down Expand Up @@ -376,6 +377,15 @@ def has_permission(user_data: Optional[UserData], item: Base, action: Action) ->
raise NotImplementedError(f"has_permission(User, ScoreSet, {action}, Role)")

elif isinstance(item, User):
if action == Action.LOOKUP:
# any existing user can look up any mavedb user by Orcid ID
# lookup differs from read because lookup means getting the first name, last name, and orcid ID of the user,
# while read means getting an admin view of the user's details
if user_data is not None and user_data.user is not None:
return PermissionResponse(True)
else:
# TODO is this inappropriately acknowledging the existence of the user?
return PermissionResponse(False, 401, "Insufficient permissions for user lookup.")
if action == Action.READ:
if user_is_self:
return PermissionResponse(True)
Expand Down
44 changes: 41 additions & 3 deletions src/mavedb/routers/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from fastapi import APIRouter, Depends, HTTPException
from sqlalchemy.orm import Session
from starlette.convertors import Convertor, register_url_convertor

from mavedb import deps
from mavedb.lib.authentication import UserData
Expand All @@ -21,6 +22,21 @@
logger = logging.getLogger(__name__)


# Define custom type convertor (see https://www.starlette.io/routing/#path-parameters)
# in order to recognize user lookup id as an int or orcid id, and call the appropriate function
class OrcidIdConverter(Convertor):
regex = "\d{4}-\d{4}-\d{4}-(\d{4}|\d{3}X)"

def convert(self, value: str) -> str:
return value

def to_string(self, value: str) -> str:
return str(value)


register_url_convertor("orcid_id", OrcidIdConverter())


# Trailing slash is deliberate
@router.get("/users/", status_code=200, response_model=list[user.AdminUser], responses={404: {}})
async def list_users(
Expand All @@ -41,18 +57,40 @@ async def show_me(*, user_data: UserData = Depends(require_current_user)) -> Any
return user_data.user


@router.get("/users/{id}", status_code=200, response_model=user.AdminUser, responses={404: {}, 500: {}})
async def show_user(
@router.get("/users/{id:int}", status_code=200, response_model=user.AdminUser, responses={404: {}, 500: {}})
async def show_user_admin(
*, id: int, user_data: UserData = Depends(RoleRequirer([UserRole.admin])), db: Session = Depends(deps.get_db)
) -> Any:
"""
Fetch a single user by ID.
Fetch a single user by ID. Returns admin view of requested user.
"""
save_to_logging_context({"requested_user": id})
item = db.query(User).filter(User.id == id).one_or_none()
if not item:
logger.warning(msg="Could not show user; Requested user does not exist.", extra=logging_context())
raise HTTPException(status_code=404, detail=f"User with ID {id} not found")

# moving toward always accessing permissions module, even though this function does already require admin role to access
assert_permission(user_data, item, Action.READ)
return item


@router.get("/users/{orcid_id:orcid_id}", status_code=200, response_model=user.User, responses={404: {}, 500: {}})
async def show_user(
*, orcid_id: str, user_data: UserData = Depends(require_current_user), db: Session = Depends(deps.get_db)
) -> Any:
"""
Fetch a single user by Orcid ID. Returns limited view of user.
"""
save_to_logging_context({"requested_user": orcid_id})

item = db.query(User).filter(User.username == orcid_id).one_or_none()
if not item:
logger.warning(msg="Could not show user; Requested user does not exist.", extra=logging_context())
raise HTTPException(status_code=404, detail=f"User with ID {orcid_id} not found")

# moving toward always accessing permissions module, even though this function does already require existing user in order to access
assert_permission(user_data, item, Action.LOOKUP)
return item


Expand Down

0 comments on commit 933b7d4

Please sign in to comment.