api.main: GET users and user profiles #279

JenySadadia · 2023-06-21T06:25:56Z

Fixes #300

Implement an endpoint to get users.
It also accepts request query parameters to get matching user.
e.g. http://API_URL/users?username=test

gctucker · 2023-06-23T08:55:47Z

This is returning the full User documents including the hashed passwords, so adding skip label for now. I think we might need to define a PublicUser model just as a return type from the API for this kind of use-case. It could contain things like the username, real name of the user, list of groups, created time etc. like say on a GitHub user profile.

JenySadadia · 2023-06-23T10:15:10Z

This is returning the full User documents including the hashed passwords, so adding skip label for now. I think we might need to define a PublicUser model just as a return type from the API for this kind of use-case. It could contain things like the username, real name of the user, list of groups, created time etc. like say on a GitHub user profile.

Yes, this needs to be tweaked once we add user profile model.

gctucker · 2023-06-23T11:06:00Z

This is returning the full User documents including the hashed passwords, so adding skip label for now. I think we might need to define a PublicUser model just as a return type from the API for this kind of use-case. It could contain things like the username, real name of the user, list of groups, created time etc. like say on a GitHub user profile.

Yes, this needs to be tweaked once we add user profile model.

Even then, I'm not sure all the user profile should be public. So we could have this in the database:

User:
  groups: []
  created:
  other_admin_field:
  profile: UserProfile:
    username:
    full_name
    email:
    preferences:
    other_private_attributes:

as a way to allow POST queries to let the user update the profile but only admins can update the main User model.

Then we would also have this model, not stored in the database but only part of the user API:

UserPublicProfile:
  username:
  full_name:
  groups: []
  other_public_fields:

gctucker · 2023-06-23T12:11:37Z

Another way to look at this, along the lines of the discussion around updating models for node ownership, is to just have a User model for the database and a UserProfile model shared with the public. Then the application would encode rules about what fields can be read or written depending on the user initiating the query, rather than at the API validation level. I think that's actually almost inevitable as things like username and groups would need to be accessible publicly or at least by the user but also need to be in the main User model and not a profile sub-model.

What is going to be more important is that we use the same UserProfile model for PUT and GET requests, so we get the same data in and out even if that's not directly an object stored in the database. Then admins should be able to POST, PUT and GET the full User objects which would be directly the same model as in the database. The only potential exception to this that I can see is for the password field, maybe it should never be returned at all and therefore not included in the data models used by the API but handled with a dedicated endpoint instead.

JenySadadia · 2023-07-06T07:42:03Z

Another way to look at this, along the lines of the discussion around updating models for node ownership, is to just have a User model for the database and a UserProfile model shared with the public. Then the application would encode rules about what fields can be read or written depending on the user initiating the query, rather than at the API validation level. I think that's actually almost inevitable as things like username and groups would need to be accessible publicly or at least by the user but also need to be in the main User model and not a profile sub-model.

What is going to be more important is that we use the same UserProfile model for PUT and GET requests, so we get the same data in and out even if that's not directly an object stored in the database. Then admins should be able to POST, PUT and GET the full User objects which would be directly the same model as in the database. The only potential exception to this that I can see is for the password field, maybe it should never be returned at all and therefore not included in the data models used by the API but handled with a dedicated endpoint instead.

Please see #292

JenySadadia · 2023-07-06T09:47:29Z

Updated the PR to get user public profiles.

gctucker · 2023-07-06T13:32:34Z

Well, #292 is not what I was suggesting with a different model for the API and for the database but I'll take a look next week. I think we basically need to get the simplest implementation merged to get the feature enabled and then we can extend it as second priority.

JenySadadia · 2023-07-07T07:36:44Z

Well, #292 is not what I was suggesting with a different model for the API and for the database but I'll take a look next week. I think we basically need to get the simplest implementation merged to get the feature enabled and then we can extend it as second priority.

I have implemented UserProfile model. That will be accessible by all users with GET /users request. Maybe we should have /users/public for getting user profiles and /users for getting user models by admin. POST /user will create UserProfile and User models.

JenySadadia · 2023-07-07T17:22:15Z

Commands will look like:

$ curl http://localhost:8001/latest/users/profile
{"items":[{"id":"64a7d92d6f9a9950b9507485","username":"admin","groups":[{"id":"64a2bcff4ba7b7c80ee3c570","name":"admin"}]},{"id":"64a84222c061af66e4d0071a","username":"jeny","groups":[]}],"total":2,"limit":50,"offset":0}

$ curl http://localhost:8001/latest/users/profile?username=jeny
{"items":[{"id":"64a84222c061af66e4d0071a","username":"jeny","groups":[]}],"total":1,"limit":50,"offset":0}

$ curl -X 'GET' 'http://localhost:8001/latest/users' -H 'accept: application/json'   -H 'Content-Type: application/json'  -H 'Authorization: Bearer TOKEN'
{"items":[{"id":"64a7d92d6f9a9950b9507486","active":true,"profile":{"id":"64a7d92d6f9a9950b9507485","username":"admin","hashed_password":"$2b$12$N.GfaUJy/a775R9PQcH6IeBM0TCCwYWA2XLKKYZG6.MiVldlWuh6O","groups":[{"id":"64a2bcff4ba7b7c80ee3c570","name":"admin"}]}},{"id":"64a84222c061af66e4d0071b","active":true,"profile":{"id":"64a84222c061af66e4d0071a","username":"jeny","hashed_password":"$2b$12$P90NaB1LzKI/dfUPhJ4jBOKFlXFbsVmQI/4p8CuL4hXUFAo1JVzh2","groups":[]}}],"total":2,"limit":50,"offset":0}

$ curl -X 'GET' 'http://localhost:8001/latest/users?profile.username=jeny' -H 'accept: application/json'   -H 'Content-Type: application/json'  -H 'Authorization: Bearer TOKEN'
{"items":[{"id":"64a84222c061af66e4d0071b","active":true,"profile":{"id":"64a84222c061af66e4d0071a","username":"jeny","hashed_password":"$2b$12$P90NaB1LzKI/dfUPhJ4jBOKFlXFbsVmQI/4p8CuL4hXUFAo1JVzh2","groups":[]}}],"total":1,"limit":50,"offset":0}

JenySadadia · 2023-07-11T07:08:34Z

Added handler to get user profile by ID:

$ curl http://localhost:8001/latest/users/profile/64a7d92d6f9a9950b9507485
{"id":"64a7d92d6f9a9950b9507485","username":"admin","groups":[{"id":"64a2bcff4ba7b7c80ee3c570","name":"admin"}]}

Added a handler to get user model by ID:

$ curl -X 'GET' 'http://localhost:8001/latest/users/64abdcc7ed84334468b3acbf' -H 'accept: application/json'   -H 'Content-Type: application/json'  -H 'Authorization: Bearer TOKEN'
{"id":"64abdcc7ed84334468b3acbf","active":true,"profile":{"id":"64abdcc7ed84334468b3acbe","username":"jeny","hashed_password":"$2b$12$QhsvplXElm34.HiF2rF6yO3WlYMLm1ZyDyo2hdtp3vDv0fdTjouqK","groups":[{"id":"64abe12e40679ecb42473768","name":"kernelci"}]}}

JenySadadia · 2023-07-13T17:35:58Z

Updated the PR to accommodate user profile changes.

JenySadadia · 2023-07-14T11:22:45Z

Tested OK on staging:

$ curl https://staging.kernelci.org:9000/latest/users/profile
{"items":[{"profile":{"username":"jeny","hashed_password":"***","groups":[]}},{"profile":{"username":"testAdmin","hashed_password":"***","groups":[{"id":"649190b01432ccfdb575a250","name":"admin"}]}},{"profile":{"username":"staging.kernelci.org","hashed_password":"***","groups":[]}},{"profile":{"username":"gtucker","hashed_password":"***","groups":[]}},{"profile":{"username":"alex","hashed_password":"***","groups":[]}},{"profile":{"username":"mg-admin","hashed_password":"***","groups":[]}},{"profile":{"username":"rcn","hashed_password":"***","groups":[]}},{"profile":{"username":"admin","hashed_password":"***","groups":[{"id":"649190b01432ccfdb575a250","name":"admin"}]}}],"total":8,"limit":50,"offset":0}

$ curl -X 'GET' 'https://staging.kernelci.org:9000/latest/users' -H 'accept: application/json'   -H 'Content-Type: application/json'  -H 'Authorization: Bearer TOKEN'
{"items":[{"id":"64705c1ae76325f50371875e","active":true,"profile":{"username":"jeny","hashed_password":"***","groups":[]}},{"id":"64a69178931a45db911eef70","active":true,"profile":{"username":"testAdmin","hashed_password":"***","groups":[{"id":"649190b01432ccfdb575a250","name":"admin"}]}},{"id":"62058f7a91aebf7ab63f1492","active":true,"profile":{"username":"staging.kernelci.org","hashed_password":"***","groups":[]}},{"id":"62554673a68f9bac63a0ead2","active":true,"profile":{"username":"gtucker","hashed_password":"***","groups":[]}},{"id":"62f3ed1b8238a289a951d1be","active":true,"profile":{"username":"alex","hashed_password":"***","groups":[]}},{"id":"6359369fa2736d6b586b7fec","active":true,"profile":{"username":"mg-admin","hashed_password":"***","groups":[]}},{"id":"63593e30d3f352d83f163878","active":true,"profile":{"username":"rcn","hashed_password":"***","groups":[]}},{"id":"62f3ec222d0a223569d55be7","active":true,"profile":{"username":"admin","hashed_password":"***","groups":[{"id":"649190b01432ccfdb575a250","name":"admin"}]}}],"total":8,"limit":50,"offset":0}

gctucker · 2023-07-17T12:21:00Z

Adding the skip label as this is returning the hashed passwords again.

JenySadadia · 2023-07-17T12:22:12Z

Adding the skip label as this is returning the hashed passwords again.

Yes, I need to exclude it.

JenySadadia · 2023-07-17T13:05:46Z

Adding the skip label as this is returning the hashed passwords again.

Yes, I need to exclude it.

Excluded password from /users/profile response.

gctucker · 2023-07-17T13:07:59Z

But what about not including passwords in the profile model? I think there are a number of things to clarify before we can resolve this and complete the implementation.

JenySadadia · 2023-07-17T13:17:27Z

But what about not including passwords in the profile model? I think there are a number of things to clarify before we can resolve this and complete the implementation.

I have moved all the editable fields by the user to UserProfile and the password is one of them.

gctucker · 2023-07-17T14:16:08Z

Please see #292 (comment)

Implement an endpoint to get public profile details of all available users. It also accepts request query parameters to get matching user. e.g. `http://API_URL/users?username=test` This will exclude password field from the response for security reasons. Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>

Implement an endpoint to get all available user models. The user model will contain private information along with public user profiles. Thus, the endpoint will be accessible by admin users only. Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>

Implement an endpoint to get user model matching provided user ID. The endpoint will be accessible by admin users only and will not include password in the response. Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>

gctucker · 2023-07-20T08:38:52Z

api/main.py

+
+@app.get('/users', response_model=PageModel,
+         response_model_exclude={"items": {"__all__": {"profile": {
+                                    "hashed_password"}}}})


OK I think that should be fine for now.

JenySadadia changed the title ~~api.main: add /users endpoint~~ api.main: add GET /users endpoint Jun 21, 2023

gctucker added the staging-skip Don't test automatically on staging.kernelci.org label Jun 23, 2023

JenySadadia force-pushed the get-users-endpoint branch from 64ce045 to e932649 Compare July 6, 2023 09:45

JenySadadia force-pushed the get-users-endpoint branch from e932649 to a977ea4 Compare July 7, 2023 17:17

JenySadadia changed the title ~~api.main: add GET /users endpoint~~ api.main: GET users and user profiles Jul 11, 2023

JenySadadia force-pushed the get-users-endpoint branch 2 times, most recently from f196609 to fc9f13b Compare July 13, 2023 17:33

JenySadadia removed the staging-skip Don't test automatically on staging.kernelci.org label Jul 14, 2023

JenySadadia force-pushed the get-users-endpoint branch from fc9f13b to f1a003e Compare July 17, 2023 06:37

gctucker added the staging-skip Don't test automatically on staging.kernelci.org label Jul 17, 2023

JenySadadia force-pushed the get-users-endpoint branch from f1a003e to 44af1e9 Compare July 17, 2023 13:05

Jeny Sadadia added 2 commits July 20, 2023 11:50

api.main: add /user/<user-id> endpoint

44e6d08

Implement an endpoint to get user model matching provided user ID. The endpoint will be accessible by admin users only and will not include password in the response. Signed-off-by: Jeny Sadadia <jeny.sadadia@collabora.com>

JenySadadia force-pushed the get-users-endpoint branch from 44af1e9 to 44e6d08 Compare July 20, 2023 06:21

JenySadadia removed the staging-skip Don't test automatically on staging.kernelci.org label Jul 20, 2023

gctucker approved these changes Jul 20, 2023

View reviewed changes

gctucker added this pull request to the merge queue Jul 20, 2023

Merged via the queue into kernelci:main with commit 388f8bd Jul 20, 2023

JenySadadia deleted the get-users-endpoint branch July 20, 2023 08:40

api.main: GET users and user profiles #279

api.main: GET users and user profiles #279

Uh oh!

Conversation

JenySadadia commented Jun 21, 2023 • edited by gctucker Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gctucker commented Jun 23, 2023

Uh oh!

JenySadadia commented Jun 23, 2023

Uh oh!

gctucker commented Jun 23, 2023

Uh oh!

gctucker commented Jun 23, 2023

Uh oh!

JenySadadia commented Jul 6, 2023

Uh oh!

JenySadadia commented Jul 6, 2023

Uh oh!

gctucker commented Jul 6, 2023

Uh oh!

JenySadadia commented Jul 7, 2023

Uh oh!

JenySadadia commented Jul 7, 2023

Uh oh!

JenySadadia commented Jul 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JenySadadia commented Jul 13, 2023

Uh oh!

JenySadadia commented Jul 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gctucker commented Jul 17, 2023

Uh oh!

JenySadadia commented Jul 17, 2023

Uh oh!

JenySadadia commented Jul 17, 2023

Uh oh!

gctucker commented Jul 17, 2023

Uh oh!

JenySadadia commented Jul 17, 2023

Uh oh!

gctucker commented Jul 17, 2023

Uh oh!

gctucker Jul 20, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JenySadadia commented Jun 21, 2023 •

edited by gctucker

Loading

JenySadadia commented Jul 11, 2023 •

edited

Loading

JenySadadia commented Jul 14, 2023 •

edited

Loading