Skip to content

Conversation

@AditiChikkali
Copy link
Contributor

@AditiChikkali AditiChikkali commented Nov 21, 2025

added /whoami endpoint to get user info from LDAP

  • accepts upn as query parameter
  • returns user details from active directory

Removed

  • GET /v1/whoami?upn=...
    • Functionality replaced by /person/me

Added/Updated

  • GET /v1/person/me (revived and enhanced)
    • Extracts UPN from headers
    • Returns comprehensive LDAP user information

Copy link
Collaborator

@padraic-shafer padraic-shafer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than creating a new route and module, it might make more sense here to revive the existing "person/me" route and update the endpoint handler to use the user principal name and to add the LDAP info into the Person or a Person-derived schema.

# TODO: Add back into schema if we decide to use this endpoint.
@router.get("/person/me", response_model=str, include_in_schema=False)
async def get_myself(current_user: Annotated[Person, Depends(get_current_user)]):
return current_user

https://github.com/NSLS2/nsls2api/blob/main/src/nsls2api/api/models/person_model.py

conn = None
try:
server = Server(ldap_server)
conn = Connection(server, user=bind_user, password=bind_password, auto_bind=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ldap3 appears to support ASYNC connections. This would be advantageous to avoid blocking.

https://ldap3.readthedocs.io/en/latest/connection.html

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempted client_strategy=ASYNC for non-blocking LDAP, but conn.result after conn.search did not reliably wait for completion or fetch results. Would need proper asyncio integration for true async; reverted to default strategy since it works reliably for now.

@AditiChikkali
Copy link
Contributor Author

Rather than creating a new route and module, it might make more sense here to revive the existing "person/me" route and update the endpoint handler to use the user principal name and to add the LDAP info into the Person or a Person-derived schema.

# TODO: Add back into schema if we decide to use this endpoint.
@router.get("/person/me", response_model=str, include_in_schema=False)
async def get_myself(current_user: Annotated[Person, Depends(get_current_user)]):
return current_user

https://github.com/NSLS2/nsls2api/blob/main/src/nsls2api/api/models/person_model.py

removed /whoami
revived and enhanced /person/me
ldap response has more data and would have to modify person model as it was designed for BNL people, so returning entire shaped ldap repsonse right now. But if necessary, can create a new ldapuser schema.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants