Warn on misconfigured access token user #3231
Conversation
Fix the code loading builtin models to always clone the JSON object used as model settings/definition. This is needed to allow applications to modify model settings while not affecting settings of new models created in the local registry of another app.
| // Because we are cloning objects created by JSON.parse, | ||
| // the cloning algorithm can stay much simpler than a general-purpose | ||
| // "cloneDeep" e.g. from lodash. | ||
| function cloneDeepJson(obj) { |
There was a problem hiding this comment.
I think it should be fine to use lodash as it's one-time charge.
There was a problem hiding this comment.
We are trying to avoid using lodash in our runtime code in order to keep the size of browserified bundle small (see #989). lodash contributes about 350kb unminified and increases the bundle size by approx 20% - see loopbackio/loopback-datasource-juggler#538.
raymondfeng
left a comment
raymondfeng
left a comment
There was a problem hiding this comment.
Good work! I like the defensive approach.
|
Nice work on this, @bajtos! I've tested this branch in the sample repo I was working with for #3215. I do get the warning for 2 different scenarios I tried: In both cases the The unusual thing though is in my other personal project I've been testing against, I don't get the warning message when using the same What I suspect is somewhere in my customization of my |
|
@raymondfeng @superkhau @alshamiri1 thank you for the review!
AFAIK, we don't have any code in LoopBack that is depending on
That's suspicious, I agree. |
Modify `app.enableAuth()` to verify that (custom) User and AccessToken models have correctly configured their hasMany/belongsTo relations and print a warning otherwise.
bajtos
force-pushed
the
warn-on-misconfigured-accessToken-user
branch
from
f0d6416 to
79f441b
Compare
| 'This typically happens when the application has a custom ' + | ||
| 'custom User subclass, but does not fix AccessToken relations ' + | ||
| 'to use this new model.\n' + | ||
| 'Learn more at http://ibm.biz/setup-loopback-auth', |
There was a problem hiding this comment.
@crandmck I setup http://ibm.biz/setup-loopback-auth shortcut to point to http://loopback.io/doc/en/lb3/Authentication-authorization-and-permissions.html#preparing-access-control-models. If/when you move the setup instructions to a different page, then we need to change the target url of the shortcut http://ibm.biz/setup-loopback-auth points to.
Description
Verify User and AccessToken relations at startup Modify
app.enableAuth()to verify that (custom) User and AccessToken models have correctly configured their hasMany/belongsTo relations and print a warning otherwise.As I was fixing the tests reporting this new warning, I discovered few other issues that I needed to fix first - see the first three commits abf8246, c45954c and f0c9700.
The warning and the tests are fixed by the last commit f0d6416.
I think we should wait with merging this pull request until there is a doc page written where we explain users how to fix this problem, so that we can add URL of this doc page to the new warnings.
cc @ebarault @greaterweb
Related issues
Checklist
guide