Improve auth-setup instructions #304

bajtos · 2017-03-03T09:56:48Z

While finalizing strongloop/loopback#3231, I realised the instructions added by #297 are not entirely correct. See also strongloop/loopback#3215 (comment) and next few comments following after that.

This pull request fixes the instructions to what I believe is correct.

cc @beeman

Connect to strongloop/loopback#3215

crandmck

I made some wording edits in your branch. I had one specific comment. Otherwise LGTM.

crandmck · 2017-03-03T22:53:46Z

pages/en/lb3/Authentication-authorization-and-permissions.md

+    "relations": {
+      "user": {
+        "type": "belongsTo",
+        "model": "user",


Would it be better to use a model named "customer" or "myUser" or anything other than "user" which is confusingly close to "User", the name of the built-in model....?

I agree that user is confusingly close to User. On the other hand, using user model is a common workaround for the fact that LoopBack does not provide any easy way how to customise the built-in User model.

@ebarault Unless you object, I will change "user" to "Customer".

i'm good with user for @bajtos 's reasons

BTW : while being a good practice in any case: is this absolutely required for single user model scenario since you've fixed the possibility of the missing relation with user here ?

is this absolutely required for single user model scenario

Well, it's not absolutely required, but only as long as we don't introduce another place that relies of this relation being correctly set up.

In strongloop/loopback#3231, I added a big warning that's printed when this relation is not configured, to prevent situations like that to reappear in the future.

ebarault · 2017-03-06T14:09:53Z

pages/en/lb3/Authentication-authorization-and-permissions.md

 " %}

-#### Access control with a single user model
+#### Access control with a single user model and built-in AccessToken model


I would then suppress this sub-section at the root of the

Preparing access control models section.

And i'd isolate the following text

In the most common scenario, an application uses only one model that extends the built-in User model and uses the built-in AccessToken model

in a

Access control with a single user model

section, adding a note like : "you're all set-up!'

This way we preserve 2 formal sections for single user model / multiple user models, which IMO gives more confidence to people on whether they've done right or have missed something

I'll change this L4 heading to Access control with a single user model and then change the heading Customizing the AccessToken model to L5.

ebarault · 2017-03-06T14:13:41Z

pages/en/lb3/Authentication-authorization-and-permissions.md


-A more complex situation is when the different types of users differ not just in their properties, but also by their access rights and relations with other models. For example, applications where the concept of an _organization_ is involved, creating intertwined layers of relationships and thus of access control.
-Such circumstances require the application to manage the different user types in separate models.
+If however the different types of users have different access rights and relations with other models,


'+ or if they basically different too greatly by their properties

ebarault · 2017-03-06T14:14:33Z

pages/en/lb3/Authentication-authorization-and-permissions.md

+    "relations": {
+      "user": {
+        "type": "belongsTo",
+        "model": "user",


i'm good with user for @bajtos 's reasons

bajtos · 2017-03-07T14:56:30Z

Updated, PTAL.

beeman · 2017-03-07T16:20:05Z

I will go over the setup once again based on these instructions later today

ebarault · 2017-03-08T10:53:42Z

just pushed one commit to add more clarity (hopefully) and align code syntax in json model excerpts.

ebarault

i'm good with your changes @bajtos , please see my additional proposals in the next commit

ebarault · 2017-03-08T10:56:55Z

pages/en/lb3/Authentication-authorization-and-permissions.md


-Normally you should have already implemented at least one custom user model, extending the built-in `User` model, as described in [Using built-in models](Using-built-in-models.html#user-model).
+The best practice is to implement at least one custom user model extending the built-in `User` model instead of using the built-in `User` model as-is, as described in [Using built-in models](Using-built-in-models.html#user-model).



i just wanted here to avoid the repetition

normally you ..

usually you ..

and tried to be a bit more directive so do get developers' confidence

ebarault · 2017-03-08T10:57:57Z

pages/en/lb3/Authentication-authorization-and-permissions.md


-This is achieved by changing the **hasMany** relation from `User` to `AccessToken` and the **belongsTo** relation from `AccessToken` to `User` by their [polymorphic](Polymorphic-relations.html) equivalents, in which the `principalType` property is used as a _discriminator_ to resolve which of the potential `user` model instance an 'accessToken' instance belongs to.
+This is achieved by changing the **hasMany** relation from `User` to `AccessToken` and the **belongsTo** relation from `AccessToken` to `User` by their [polymorphic](Polymorphic-relations.html) equivalents, in which the `principalType` property is used as a _discriminator_ to resolve which of the potential `user` model instance an 'accessToken' instance belongs to. In addition to having custom user models this requires you also define a **custom AccessToken** model extending the built-in `AccessToken` model.



i added this part for the sake a clarity
"In addition to having custom user models this requires you also define a custom AccessToken model extending the built-in AccessToken model"

crandmck · 2017-03-08T19:12:27Z

LGTM

bajtos · 2017-03-09T07:49:28Z

@ebarault thanks, your proposed changes looks good to me too.
@crandmck thank you for the review.

I have squashed the commits to a single one and land this patch once CI passes.

bajtos requested review from crandmck and ebarault March 3, 2017 09:56

bajtos added the review label Mar 3, 2017

crandmck reviewed Mar 3, 2017

View reviewed changes

ebarault reviewed Mar 6, 2017

View reviewed changes

bajtos force-pushed the fix/auth-setup branch from 86e2c56 to da00b56 Compare March 7, 2017 14:56

bajtos requested review from crandmck and ebarault March 7, 2017 14:56

ebarault approved these changes Mar 8, 2017

View reviewed changes

ebarault reviewed Mar 8, 2017

View reviewed changes

Improve auth-setup instructions

e3e3d63

bajtos force-pushed the fix/auth-setup branch from 18879b6 to e3e3d63 Compare March 9, 2017 07:48

bajtos merged commit 4d53ecb into gh-pages Mar 9, 2017

bajtos deleted the fix/auth-setup branch March 9, 2017 07:51

bajtos removed the review label Mar 9, 2017

bajtos mentioned this pull request Mar 9, 2017

Add docs for creating custom AccessToken model #200

Closed


		Normally you should have already implemented at least one custom user model, extending the built-in `User` model, as described in [Using built-in models](Using-built-in-models.html#user-model).
		The best practice is to implement at least one custom user model extending the built-in `User` model instead of using the built-in `User` model as-is, as described in [Using built-in models](Using-built-in-models.html#user-model).


		This is achieved by changing the hasMany relation from `User` to `AccessToken` and the belongsTo relation from `AccessToken` to `User` by their [polymorphic](Polymorphic-relations.html) equivalents, in which the `principalType` property is used as a _discriminator_ to resolve which of the potential `user` model instance an 'accessToken' instance belongs to.
		This is achieved by changing the hasMany relation from `User` to `AccessToken` and the belongsTo relation from `AccessToken` to `User` by their [polymorphic](Polymorphic-relations.html) equivalents, in which the `principalType` property is used as a _discriminator_ to resolve which of the potential `user` model instance an 'accessToken' instance belongs to. In addition to having custom user models this requires you also define a custom AccessToken model extending the built-in `AccessToken` model.

Improve auth-setup instructions #304

Improve auth-setup instructions #304

Uh oh!

Conversation

bajtos commented Mar 3, 2017

Uh oh!

crandmck left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Preparing access control models section.

Access control with a single user model

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bajtos commented Mar 7, 2017

Uh oh!

beeman commented Mar 7, 2017

Uh oh!

ebarault commented Mar 8, 2017

Uh oh!

ebarault left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crandmck commented Mar 8, 2017

Uh oh!

bajtos commented Mar 9, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants