Refactoring: Customer credentials

[nabdelgadir]

Description

Step 1 from #1035 (comment).

Let's keep things simple for now and use a password as the only credential. (No 2-factor-auth, no OAuth/OpenID/SAML for now.)

In LoopBack 3.x, we are storing storing user's password together with user data in the same model (table), which opens a lot of security vulnerabilities to deal with. For example:

• When returning user data in HTTP response, the password property must be filtered out.
• When searching for users, the password must be excluded from the query.
• It is not possible to do a full replace of user data from a REST client (e.g. "save()") because a) the client usually does not know the password b) password must be changed using a different flow.
• A policy preventing users from reusing the same password is difficult to implement because old password are not preserved.

For LB4, I would like us to explore the design where passwords are stored in their own table and a relation "user has one password" is configured (#1422 is tracking HasOne relation). We can also use "user has many password" and include a flag (a Password model property) to distinguish between the current active password and the passwords used in the past.

The current version of the Shopping Server is already storing the password in the User model. Let's move it out to a dedicated model as part of this preparation work.

Ideally, there should be a documentation and/or a blog-post and/or a reference implementation to make it easier for LB4 users to implement similar functionality in their project.

Next step: #1997

Acceptance criteria

Based on #1996 (comment)

• in the example-shopping repo, divide the UserRepository into user credential and user profile information - to separate sensitive information from insensitive information as best practices.
• documentation and/or a blog-post and/or a reference implementation

[David-Mulder]

Just some thoughts from an outsider:

I assume that the idea is that there will be an authorization mechanism which will create some loopback class which grants a set of privileges. It sounds very reasonable to me to leave the creation of this class instance to the implementer.

So the login controller would return some token (whatever the implementer wants). The client will take care of putting this in some header/query parameter/whatever, and the implementer will put a method into the sequence file that takes this token and turns it into some Loopback comprehensible class.

Example (simple):

return new Authorization({
    scopes: []
});

Example (complex) (per #2000 , and this comment ):

return new Authorization({
    realms: [
      { id: "...", role: "x" }
    ]
});

Of course there could be some default implementation (generation of tokens and parsing of those tokens), but building that default implementation sounds to me like something that could be easily left for the very end.

[dhmlau]

@David-Mulder, thanks for your comment. This is step 1 out of the 5 steps that @bajtos proposed what we should do for the authentication & authorization story. #1035 (comment)

Would love to hear your feedback!

[dhmlau]

Updated plan is described in here: #1035 (comment)

Acceptance Criteria

• In the shopping app example, remove password property in the User model.

[dhmlau]

From estimation meeting yesterday, it seems like we should work on #1997 first and then remove the password as part of that task. As a result, closing it now.

[bajtos]

Based on the discussion in loopbackio/loopback4-example-shopping#26, I am not very confident that password is going to be removed as part of #1997. Let's reopen this issue and keep it open until the necessary changes are landed.

[dhmlau]

@jannyHou, is the above acceptance criteria still correct? If so, pls remove the needs-grooming label, then we can estimate in the next meeting. Thanks.

[jannyHou]

@dhmlau I am afraid the solution is more complicated than just removing it from the model. This story is still under discussion and not ready to estimate.

[dhmlau]

Thanks. I'll move it back to Needs Discussion.

[dhmlau]

Discussion with @raymondfeng @jannyHou @emonddr:
This issue for the example-shopping, to divide the UserRepository into user credential and user profile information - to separate sensitive information from insensitive information as best practices.

[dhmlau]

Moving to p2 because this is related to enhancement for User model.

[dhmlau]

Moving to Q4.

[dhmlau]

From a discussion with @raymondfeng and @bajtos, I think the other task about updating the tutorial to include authorization should cover this task. But I'm not 100% sure.

@raymondfeng @bajtos, could you please confirm? Thanks.

[emonddr]

@dhmlau , I think this task should occur separately and after adding authorization to shopping example.

[bajtos]

This task is about improving the way how local user credentials are stored. As long as our Example shopping application is storing user password in its database (as opposed to using a 3rd party service to authenticate users, e.g. Google or any other OAuth2 provider), this task is ready to be worked on. @emonddr why do you think this task needs to wait until authorization is added? @dhmlau I don't think authorization-related tasks are covering the acceptance criteria of this story. Especially this one: in the example-shopping repo, divide the UserRepository into user credential and user profile information - to separate sensitive information from insensitive information as best practices

[deepakrkris]

IMO, removing password field will not be useful in anyway. We need to add some metadata specific to fields with privacy protection.

1. Provide field level security options by adding decrypt() / encrypt() functions in model class, and call these functions as needed
2. Provide field level privacy options by providing filter() / watermark() functions in model class and call these functions as needed
3. Add options in the property decorator, to mention what level of privacy and security is needed for the field. pass these configurations to the security and privacy model class functions.

[bajtos]

@deepakrkris

Removing password field will not be useful in anyway. We need to add some metadata specific to fields with privacy protection.

I agree that it would be great to support encrypted & hidden property directly by the framework. However, that's out of scope of this story.

We are discussing encrypted properties here: #2095

I am not convinced it's a good idea to support hidden (private) properties because of the complexity it involves.

Now the scope of this issue is to move user credentials to a different model (e.g. UserCredentials vs. UserProfile). This will directly solve field privacy problem and won't make field security (encryption) any worse.

I am arguing it will actually make field encryption more robust, because there will be less ways how the user can change the password - it will be no longer possible to accidentally call UserRepository.replaceById with a password property included in the payload.

To recap: In longer term, we want to implement some of the features you have mentioned. For short term, we want to improve the Shopping Example using the existing framework features, and that's the goal of this story.

[bajtos]

I opened a PR with an initial proposal: loopbackio/loopback4-example-shopping#385

[jannyHou]

Thank you @bajtos your proposal looks good to team, just a quick confirm of the status of this story: you will add documentation then merge loopbackio/loopback4-example-shopping#385 right?