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
refactor(server): update user vs public user vs auth endpoints #9063
base: main
Are you sure you want to change the base?
Conversation
Deploying immich with Cloudflare Pages
|
f5c02f2
to
be49280
Compare
be49280
to
e79ee7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I like the idea of differentiating between admin user endpoints and non admin ones, but I think the endpoints need to be adjusted. I was thinking all the existing user endpoints can eventually become admin only. Then we add a new endpoint for searching users that everyone else can use. We should migrate clients to this new one before changing anything about the old one.
The new one can maybe be something like GET /public-users. It would be best if it was in a separate controller.
Good idea! Where should be the |
Yes I think auth controller should make sense. Probably just /auth/user |
a3ca770
to
fbefa67
Compare
fbefa67
to
304ee53
Compare
A few notes:
|
346d8cf
to
c760f24
Compare
c760f24
to
23784ab
Compare
I would like to take this opportunity to start building up a system for managing breaking changes/backward compatibility in API for the mobile app as well, so the endpoint that got deleted and caused breaking changes should be put back and marked as deprecated and will be removed in the future date |
This is going to be a bit complicated to do because there are probably 4-5 endpoints that fall into this category. |
If we have to do this in a non-breaking manner, I think we'll need to add the new endpoints for public users without changing any of the old implementations (a bit annoying). Then, in the future, update the endpoints to be admin-only, and simplify the implementation. |
Thinking about it, I don't feel good about admins being able to have access to all users'
|
That sounds good @martabal! I was also thinking about it and we probably should add the new endpoints, then transition the web and mobile app to use them in the future and deprecate the old endpoints before finally removing the old ones. Specifically we can add public users and new profile picture routes, get my auth user, but revert any changes that deleted old routes and revert the user core changes. Once the mobile app stops using the old get my user and update user we can change those routes to return the admin user dto too |
Currently, all users have access to all information about other users such as the date they were created, quota usages, if memories is enabled...
This information should only be accessible by the administrators and the users themselves.