Skip to content

AAA-251: remove app_metadata handling, query users in the database#71

Merged
marius-mather merged 26 commits into
mainfrom
remove-app-metadata
Sep 19, 2025
Merged

AAA-251: remove app_metadata handling, query users in the database#71
marius-mather merged 26 commits into
mainfrom
remove-app-metadata

Conversation

@marius-mather

@marius-mather marius-mather commented Sep 17, 2025

Copy link
Copy Markdown
Collaborator

Description

AAI-251: we now use our user database to record group/platform membership, instead of Auth0's app_metadata. This PR removes our code for recording/querying app_metadata. Because of this, we also need to move some of our user queries to the database - any user queries (for pending/approved/revoked users) should use the database, and return the data in a consistent format.

Changes

  • Remove groups and services from app_metadata
  • Drop API endpoints that are no longer relevant
  • Update user query endpoints to query the database instead of Auth0

Updated endpoints

  • /admin/users/approved, /admin/users/pending, /admin/users/revoked: all updated to query the DB instead of Auth0
  • /admin/users/unverified: Updated to use email_verified status from the DB (not always guaranteed to be up to date, but much more efficient to query in the DB)
  • /admin/users/{user_id}: updated to return user info from the DB. /admin/users/{user_id}/details still returns data from Auth0 so can be used for the detailed user info page
  • /me/services/* -> /me/platforms/*: updated user services endpoints to /platforms to match our current terminology, and to query the platform membership info in the DB instead
  • /me/groups/*: added group membership endpoints, replacing the previous resources endpoints.
  • /me/all/pending: updated to combine info about pending platforms/groups

Removed endpoints

  • /admin/users/{user_id}/services/{service_id}/approve
  • /admin/users/{user_id}/services/{service_id}/approve
  • /admin/users/{user_id}/services/{service_id}/resources/{resource_id}/approve
  • /admin/users/{user_id}/services/{service_id}/resources/{resource_id}/revoke

We don't currently use these and need to make sure any approvals/revocations can only be done by users with the right roles, so would need to update them anyway when implementing approvals under the current design.

  • /me/request/service
  • /me/request/{service_id}/{resource_id}

We don't currently use these - requests for platform/group access happen at registration time currently, and we would need to update the logic when implementing these requests anyway.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have added unit / integration tests that prove my fix is effective or that my feature works
  • I have run all tests locally and they pass
  • I have updated the documentation (if applicable)

How to Test Manually (if necessary)

Run uv run pytest

@marius-mather marius-mather requested review from amandazhuyilan, Copilot and minh-biocommons and removed request for amandazhuyilan September 18, 2025 04:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the authentication system to move from Auth0's app_metadata to a database-based approach for managing user platform and group memberships. The changes remove app_metadata handling for groups and services, while updating user query endpoints to use the database instead of Auth0.

  • Removes all app_metadata service/group management code and associated API endpoints
  • Updates user endpoints to query database for platform/group memberships instead of Auth0 metadata
  • Refactors admin endpoints to use database queries for user status filtering

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/test_user.py Updates tests to use database-based platform/group membership instead of Auth0 app_metadata
tests/test_models.py Removes all service/resource model tests that are no longer relevant
tests/test_bpa_register.py Simplifies BPA registration tests by removing service creation logic
tests/test_admin.py Updates admin endpoint tests to use database queries instead of Auth0 client calls
tests/conftest.py Adds proper session cleanup in test database fixture
schemas/service.py Completely removes file containing Service, Resource, and Group models
schemas/requests.py Completely removes file containing ServiceRequest and ResourceRequest models
schemas/biocommons.py Removes app_metadata service/group handling methods and updates registration logic
schemas/init.py Removes imports for deleted Service, Resource, and Group models
routers/user.py Replaces Auth0 metadata queries with database queries for platform/group memberships
routers/bpa_register.py Removes BPA service creation during registration
routers/admin.py Updates admin endpoints to query database for user filtering instead of Auth0
db/setup.py Adds proper session cleanup in database session dependency

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread tests/test_user.py Outdated
Comment thread tests/test_admin.py Outdated
Comment thread routers/user.py Outdated
Comment thread routers/user.py Outdated
marius-mather and others added 3 commits September 18, 2025 15:01
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
amandazhuyilan
amandazhuyilan previously approved these changes Sep 18, 2025

@amandazhuyilan amandazhuyilan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good - thanks for the change!

@amandazhuyilan amandazhuyilan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks again for doing this!

@marius-mather marius-mather merged commit 77d046c into main Sep 19, 2025
2 checks passed
@amandazhuyilan amandazhuyilan deleted the remove-app-metadata branch November 24, 2025 22:23
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.

3 participants