Skip to content

User refactor + remove fastapi_users dependency + update fastapi #1290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 50 commits into from
Oct 18, 2023

Conversation

ikreymer
Copy link
Member

@ikreymer ikreymer commented Oct 16, 2023

Fixes #1050

Major refactor of the user/auth system to remove fastapi_users dependency. Refactors users.py to be standalone
and adds new auth.py module for handling auth. UserManager now works similar to other ops classes.
Also includes:

Branch based off #1272 work, and may supersede it fully.

The auth should be fully backwards compatible with fastapi_users auth, including accepting previous JWT tokens w/o having to re-login. The User data model in mongodb is also unchanged.

API changes:

  • Removing theGET, PATCH, DELETE /users/<id> endpoints, which were not in used before, as users are scoped to orgs. For deletion, probably auto-delete when user is removed from last org (to be implemented).
  • Rename /users/me-with-orgs is renamed to just /users/me/

tw4l and others added 30 commits October 11, 2023 13:47
Implements a forked custom user router to make the change, since
overriding fastapi endpoints is not straightforward.
update model to contain 'password' and 'newPassword'
frontend: match model
…tion + decoding

move auth functionality to auth.py
- move relevant functions from fastapi_user manager to users
- update modules
- remove fastapi_users, update to latest fastapi!
- still more cleanup needed, simplification where possible
- keep pydantic at 1.x for now (made some model changes that are forward compatible with 2.0)
- ensure users uses _id not id similar to all other objects (will need migration)
- remove userdb, move db functions into users
- remove generic _update, add update_verified, update_password, update_email
- webhooks: add webhooks for openapi!
- remove references to 'user_db'
- unify 'get_id' / 'get_user_by_id' to just 'get_by_id'
- fix 'id' -> '_id' reference
remove is_active from model, check for 'id' on frontend instead of unused 'is_active'
- error code pass: use more consistent lowercase errors, remove old fastapi_users error code
- use 'user_already_exists' and 'invalid_password' for dupe user and invalid password errors
- update frontend error codes checking
- rename self.collection -> self.users in UserManager
@ikreymer ikreymer marked this pull request as ready for review October 17, 2023 05:02
@ikreymer ikreymer requested a review from tw4l October 17, 2023 05:03
Copy link
Member

@tw4l tw4l left a comment

Choose a reason for hiding this comment

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

Initial comments, testing locally now

@tw4l
Copy link
Member

tw4l commented Oct 17, 2023

Manually tested:

  • Changing user password via forgot password link
  • Chaning user name, email, and password via account settings
  • Changing user roles in org via org settings
  • Log in and log out
  • Usernames set correctly on crawls and workflows after creating and deleting crawls

Aside from the comment about using auth.verify_password(), this is looking good to me!

@tw4l
Copy link
Member

tw4l commented Oct 17, 2023

I am seeing an error trying to render the API docs:

Screen Shot 2023-10-17 at 4 21 12 PM

Console error:

Screen Shot 2023-10-17 at 4 21 24 PM

Update: seeing this as an issue on Chromium Version 92.0.4515.131 (Official Build, ungoogled-chromium) (arm64) on macOS. In Chrome Version 118.0.5993.70 (Official Build) (arm64), Firefox Version 118.0.5993.70 (Official Build) (arm64), and Safari Version 16.5 (17615.2.9.11.6, 17615) it works fine.

Might just be due to the older Chromium version? Going to say this is definitely not a blocker given it's only happening in one older browser and not any of the up-to-date mainstream browsers.

- fix doc strings
- add user.update_invites() when removing an invite
- fix logic for printing when superuser password has changed
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.

Update FastAPI to latest - requires updating or removing fastapi-users dependency
3 participants