Add user profile model

[JenySadadia]

Create a separate model to store user information that will be accessible publicly and can be updated by user. Store private information along with profile to User model and only admins will be allowed to access/update it.

[gctucker]

Let's get the basic user functionality merged and available first, then we can see how to deal with this as a follow-up.

[gctucker]

@JenySadadia I think we can have the functionality in place to create user accounts and have node owners without an implementation for a user profile, which can be added later. So I would suggest to draw the line there, to have all the basic things implemented and completely done with just User and a user name, groups, password field and the corresponding API and CLI and documentation bits all aligned first. Then once we have this stable base, users can already start using it, and we can add support for profiles as an incremental step. How does that sound?

[JenySadadia]

@JenySadadia I think we can have the functionality in place to create user accounts and have node owners without an implementation for a user profile, which can be added later. So I would suggest to draw the line there, to have all the basic things implemented and completely done with just User and a user name, groups, password field and the corresponding API and CLI and documentation bits all aligned first. Then once we have this stable base, users can already start using it, and we can add support for profiles as an incremental step. How does that sound?

Yes, we can do that. However, since we already have a few PRs for user profile support, it won't be that much time-consuming to tweak them and merge them. If we add the profile later after the early access, we would need to migrate all the existing users and also revisit the changes from these PRs.

[gctucker]

My fear is that there might still be a couple of weeks of tweaks before this gets merged, which would delay the early access rollout. Avoiding this seems to me like a smaller impact than a migration later on.

[JenySadadia]

My fear is that there might still be a couple of weeks of tweaks before this gets merged, which would delay the early access rollout. Avoiding this seems to me like a smaller impact than a migration later on.

How about I tweak all the related PRs today itself or latest by tomorrow? I don't think it would take up much time.

[gctucker]

Alright let's try that.

[JenySadadia]

Updated all existing users on staging instance to have profile field.

Tested OK on staging:

$ curl -X 'POST' 'https://staging.kernelci.org:9000/latest/user/test' -H 'accept: application/json'   -H 'Content-Type: application/json'  -H 'Authorization: Bearer TOKEN' -d '{"password": "****"}'
{"id":"64b15c1ad089ca2290f442c1","active":true,"profile":{"username":"test","hashed_password":"$2b$12$JnnfhTn1gn8SUaKV0f2H8.Ov6ZlFWuvgs5yyfMIYCfF3TVAe97.YK","groups":[]}}

[gctucker]

I can see a number of issues with user profiles which haven't been solved or even properly identified yet, and we don't really need user profiles for early access. So I still think we should put this aside for now and keep it as a follow-up development activity. We will need a way for users to change their passwords for early access and having it blocked because the user profile implementation is not ready is going to cause some problems.

[gctucker]

I wonder if these 2 fields should rather be in the User model rather than User Profile. I think the profile part hasn't been well defined yet. The password will most likely require a dedicated API endpoint to update it, and maybe the username too. In fact, even groups might be something to keep in User as it's not something the users should be able to update themselves.

[JenySadadia]

I thought we agreed to move username, password, and groups to user profile. Only passwords should be hidden from the response when we get all user profiles.

[gctucker]

Right, I guess that's not very clearly defined then. One thing is about information users can read publicly, then what users can read from their own profile, then what can users update themselves. Users shouldn't be able to escalate their privileges, and passwords shouldn't be readable at all (even encrypted as that facilitates brute-force attacks).

[JenySadadia]

I can see a number of issues with user profiles which haven't been solved or even properly identified yet, and we don't really need user profiles for early access. So I still think we should put this aside for now and keep it as a follow-up development activity. We will need a way for users to change their passwords for early access and having it blocked because the user profile implementation is not ready is going to cause some problems.

The profiles can be updated using PUT /user/profile/{username} requests. Please see the PR #293 for this.
This provides the facility to update password as well as user groups. username once set can not be updated by this request. This is due to the fact that we bind tokens with usernames. If the user changes the username, the already existing token won't work.
What other issues do you feel I need to fix for user profiles?
Also, I agree that we do not need user profiles for early access. However, I have already created all the related PRs for it and also tested all the changes on staging. I feel that we are just a few fixes away to get them merged.

[gctucker]

A few things from the top of my head:

• users should not be able to change their groups themselves
• updating the password should be possible without an API token
• we might want to make username fixed initially, so it's unclear whether it should be a user profile field
• we might not need a model for storing the user profile in the database, only at the API level

[gctucker]

We would need to go through this in more detail, these are just a few things I thought about while reviewing the code.

[JenySadadia]

A few things from the top of my head:

• users should not be able to change their groups themselves

Okay. And what about Node.user_groups? Should we allow users to set it while creating nodes?

• updating the password should be possible without an API token

Yes, that's true. The PUT /user/profile is used to update a whole user profile. Thus, it needs a token for authentication.

• we might want to make username fixed initially, so it's unclear whether it should be a user profile field

We can add another name field as you suggested earlier and make it a variable. Or we can allow username to get updated and notify the user to regenerate tokens.

• we might not need a model for storing the user profile in the database, only at the API level

We are not storing UserProfile separately in the DB. It is being used in User model.

[gctucker]

I'll reply to this in a more structured way on the GitHub issue. I think we now need to look at a minimal implementation with a public profile set of attributes and then split out things we can leave for later. Also it seems worth investigating if there's a library we can use directly with FastAPI to manage all the user / group side of things.

[JenySadadia]

I'll reply to this in a more structured way on the GitHub issue. I think we now need to look at a minimal implementation with a public profile set of attributes and then split out things we can leave for later. Also it seems worth investigating if there's a library we can use directly with FastAPI to manage all the user / group side of things.

This package provides a base user model, user manager, and basic endpoints. https://github.com/fastapi-users/fastapi-users

[gctucker]

As discussed, I think we can have this merged as-is and then rework things slightly to make it safe even if it means some features will be missing. Then we can see if there's anything else that needs to be done for "early access" and update the backlog with things to do for a full production implementation (probably using an existing package, see #304).

[gctucker]

Right, I guess that's not very clearly defined then. One thing is about information users can read publicly, then what users can read from their own profile, then what can users update themselves. Users shouldn't be able to escalate their privileges, and passwords shouldn't be readable at all (even encrypted as that facilitates brute-force attacks).