Marzban
Marzban copied to clipboard
UserResponse: admin field added
Details:
- User responses ship with admin info now.
- CRUD's user queries: Admin left join.
Fixes #910
nice pr but you returning all admin data
in this case this can cause data leak , users can see there admin data with "/sub/{token}/info"
request
we just need username or id
nice pr but you returning all admin data in this case this can cause data leak , users can see there admin data with
"/sub/{token}/info"
request we just need username or id
Thanks for your time. I wasn't aware of this (I usually use separate schemas for users and admin, so I didn't expect this 😅). In my opinion, the user shouldn't even be able to get the admin's username. A separated schema like AdminUserResponse
or DetailedUserResponse
with the admin
field might be the way to go. Do you agree?
And since the non-sudo admins have no access to other admins' users, there would not be any risk of data leak cause it's the data that they should/and already have access to.
i think its better to have separated schema for /info request with less data
some data like note
sub_updated_at
sub_last_user_agent
online_at
excluded_inbounds
inbounds
are unnecessary
Since I don't like the pieces of code I wrote in the last commit, I chose to explain the changes. I know there are much better ways to handle this, but as far as I know, all of them require changes in User
and UserResponse
. Since UserResponse is highly integrated in every single piece of code (Which I don't like, but it is), any change that affects it can lead to unexpected bugs. So I chose to handle it in a way that has minimal side effects. Any suggestion is welcome as usual.
Hello, thanks for your efforts. just tested it, and it returns users list empty.
Hello, thanks for your efforts. just tested it, and it returns users list empty.
Sounds to be fixed now. The join led to the exclusion of users who didn't have a db admin.
Thank you @Alirezaja1384, changes are great. I might change the way "admin" field is returning in future.