Conversation
agnes512
left a comment
agnes512
left a comment
There was a problem hiding this comment.
Thanks for the writeup ❤️! I think the structure is good. Just have some nitpicks.
|
|
||
|  | ||
|
|
||
| Based on the screenshot, suppose u2(user with id 2) makes a request to |
There was a problem hiding this comment.
Maybe wrap u2, p1_team, etc. here?
jannyHou
force-pushed
the
doc/example/acl-migration
branch
2 times, most recently
from
c423a4e to
33b401b
Compare
jannyHou
requested review from
deepakrkris,
emonddr,
hacksparrow and
nabdelgadir
as code owners
jannyHou
force-pushed
the
doc/example/acl-migration
branch
from
33b401b to
7e68244
Compare
jannyHou
changed the title
|
@agnes512 Thank you for providing the feedback so fast! I added all the reference and the doc is now complete 📝 (some content is waiting for final decision/approve of the feat PR). I will address your comments asap. |
docs/site/migration/auth/example.md
Outdated
|
|
||
| # App Scenario | ||
|
|
||
| This example is a donation system. Users can view or make donations to projects |
There was a problem hiding this comment.
In the original loopback-example-access control README, it has a pretty good description of what the scenario is. Do you want to reuse that?
docs/site/migration/auth/example.md
Outdated
| (Project, Team, User). And we add one more model UserCredentials to separate the | ||
| sensitive information from the User model. | ||
|
|
||
| You can run `lb4 model` to create the 4 models, and run `lb4 repository` to |
There was a problem hiding this comment.
Instead of creating the models from scratch, is it possible to run lb4 import-lb3-models to import the LB3 models?
https://loopback.io/doc/en/lb4/Importing-LB3-models.html
There was a problem hiding this comment.
The User model is simplified in the migrated application, and the credentials are also extracted into a separate module.
Only Project and Team can be migrated.
I haven't tried that command and my example app is not created using it, can I keep the content as it is now then open a new PR to try the model migration? If I need to update the example code, will also do it together.
There was a problem hiding this comment.
open a new PR to make use the lb4 import-lb3-models sounds good to me. 👍 I'd prefer to do it right away though, because it will make the migration process cleaner. Thanks.
docs/site/migration/auth/example.md
Outdated
|
|
||
| ## Try Out | ||
|
|
||
| (a question for reviewers: do you prefer to see all the screenshots in this |
There was a problem hiding this comment.
IMHO, it would be good to have a screenshot for the first one to show where the token can be found and also the Authorize button. No need to show screenshots for all roles though.
|
|
||
| Try role 'admin': | ||
|
|
||
| - Login as admin first (user Bob) |
There was a problem hiding this comment.
The screenshot(s) i have in mind:
- under the
users/loginendpoint, it shows:- the request body json
- the token as the response
- Authorize button
- (optional) shows what it looks like after clicking the Authorize button
Suggestion: We could possibly highlight the portion of the screenshot with red rectangle, annotated with numbers that shows the sequence of actions that you're describing in text.
|
|
||
| ## References | ||
|
|
||
| ### Model Creation |
There was a problem hiding this comment.
Ideally, we can use the lb4 import-lb3-models.
docs/site/migration/auth/example.md
Outdated
|
|
||
| ## Set up authentication | ||
|
|
||
| This demo uses token based authentication, and uses the jwt authentication |
There was a problem hiding this comment.
perhaps I don't know enough of LB3 or the access-control example... Where in https://github.com/strongloop/loopback-example-access-control indicates it's using JWT authentication or is it by default that JWT authentication is used when authentication is enabled?
| example seeds data in the boot script, now they are migrated to an observer file | ||
| called `sample.observer.ts`. | ||
|
|
||
| _Since the data are already generated in `db.json`, that observer file is |
There was a problem hiding this comment.
@bajtos for #4571 (comment), see the Seed Data section here. The observer is automatically skipped.
emonddr
left a comment
emonddr
left a comment
There was a problem hiding this comment.
Great job, Janny. Very detailed migration guide.
Just several tiny typos to fix.
docs/site/migration/auth/example.md
Outdated
|
|
||
| This example is a donation system. Users can view or make donations to projects | ||
| based on their roles. A project is owned by a user, and a user can create a team | ||
| to involve other users as team members. The system has an administration. |
There was a problem hiding this comment.
has an administration.
has an administrator ?
There was a problem hiding this comment.
replaced the app description with the original one in https://github.com/strongloop/loopback-example-access-control#loopback-example-access-control
docs/site/migration/auth/example.md
Outdated
|
|
||
| The diagram for models also include the pre-created data in this application. | ||
|
|
||
| For example, u1 owns project1, it also creates a team with u1 and u2 as members. |
There was a problem hiding this comment.
For example, u1 owns project1, it also creates a team with u1 and u2 as members.
In this example, u1 owns project1, and u1 created a team with u1 and u2 as members.
docs/site/migration/auth/example.md
Outdated
| # Difference | ||
|
|
||
| LoopBack 3 has several built-in models that consists of a RBAC system. The | ||
| models are User, Role, RoleMapping, ACL, AccessToken. You can learn how they |
There was a problem hiding this comment.
ACL, AccessToken.
ACL, and AccessToken.
Last item in a list should get an 'and'.
docs/site/migration/auth/example.md
Outdated
| work together in tutorial | ||
| [Controlling data access](https://loopback.io/doc/en/lb3/Controlling-data-access.html). | ||
|
|
||
| LoopBack 4 authorization system gives developers the flexibility to implement |
There was a problem hiding this comment.
LoopBack 4 authorization system
The LoopBack 4 authorization system
docs/site/migration/auth/example.md
Outdated
|
|
||
| # Steps with Casbin | ||
|
|
||
| Next let's migrate the LoopBack 3 access control example to LoopBack 4. Here is |
There was a problem hiding this comment.
Next let's migrate
Let's migrate
There was a problem hiding this comment.
Place Here is an overview of the steps: on its own line. It seems far away from bullet list
docs/site/migration/auth/example.md
Outdated
| - add a new user to a team | ||
| - find the projects owned by the team owner, then create role inherit rules | ||
| g, u${id}, project${id}\_team | ||
| - add a new endpoint(operation) |
There was a problem hiding this comment.
add a new endpoint(operation)
adding a new endpoint(operation)
docs/site/migration/auth/example.md
Outdated
| _Since the data are already generated in `db.json`, that observer file is | ||
| skipped by default._ | ||
|
|
||
| ## Try Out |
docs/site/migration/auth/example.md
Outdated
| - Run `npm start` to start the application. | ||
| - Open the explorer | ||
|
|
||
| Try role 'admin': |
docs/site/migration/auth/example.md
Outdated
| - Try the 5 endpoints, 'show-balance' and 'withdraw' will return 401, others | ||
| succeed | ||
|
|
||
| Try role 'owner': |
docs/site/migration/auth/example.md
Outdated
| - Click authorize button and paste the token | ||
| - Try the 5 endpoints, 'view-all' will return 401, others succeed | ||
|
|
||
| Try role 'team-member': |
There was a problem hiding this comment.
Try the 'team-member' role:
jannyHou
force-pushed
the
example/acl-migration
branch
from
1cffe6d to
7ed7254
Compare
jannyHou
force-pushed
the
doc/example/acl-migration
branch
from
7e68244 to
32484ea
Compare
jannyHou
force-pushed
the
example/acl-migration
branch
2 times, most recently
from
bd5ee68 to
76bb1e6
Compare
docs/site/migration/auth/example.md
Outdated
| Which means u1 is the **owner** of project1, and u1 and u2 are the **team | ||
| members** of project1. And u3 is the **admin**. |
There was a problem hiding this comment.
Which means u1 is the **owner** of project1, and u1 and u2 are the **team members** of project1. And u3 is the **admin**. -> This means u1 is the **owner** of project1, u1 and u2 are the **team members** of project1, and u3 is the **admin**.
docs/site/migration/auth/example.md
Outdated
| work together in tutorial | ||
| [Controlling data access](https://loopback.io/doc/en/lb3/Controlling-data-access.html). |
There was a problem hiding this comment.
in tutorial [Controlling data access](https://loopback.io/doc/en/lb3/Controlling-data-access.html) -> in the [Controlling data access](https://loopback.io/doc/en/lb3/Controlling-data-access.html) tutorial
docs/site/migration/auth/example.md
Outdated
| the role mapping. | ||
|
|
||
| This guide includes a demo for using [casbin](#steps-with-casbin) and using | ||
| [oauth0](#steps-with-oauth0)(TBD) |
There was a problem hiding this comment.
What's the (TBD) here for?
There was a problem hiding this comment.
This example uses casbin as the 3rd party lib for access control, in the future we will explorer using oauth0 as the lib.
There was a problem hiding this comment.
Is #steps-with-casbin supposed to link to Migrating Example to LoopBack 4 with Casbin? And since #steps-with-oauth0 doesn't exist (in this PR), can you remove the link to it?
docs/site/migration/auth/example.md
Outdated
| and uses the authentication and authorization system in LoopBack 4 to implement | ||
| the access control. | ||
|
|
||
| # App Scenario |
There was a problem hiding this comment.
The headings should start with the second-level ones # -> ## (so shift all of them by one)
| and `teamMember` is defined and registered in | ||
| [role-resolver](https://github.com/strongloop/loopback-example-access-control/blob/master/server/boot/role-resolver.js). | ||
|
|
||
| In the migrated application, we use a 3rd-party library |
There was a problem hiding this comment.
3rd-party -> third-party
jannyHou
force-pushed
the
doc/example/acl-migration
branch
2 times, most recently
from
a76154f to
1fc0251
Compare
docs/site/migration/auth/example.md
Outdated
|
|
||
| The LoopBack 4 authorization system gives developers the flexibility to | ||
| implement the RBAC on their own. You can leverage popular third-party libraries | ||
| like [casbin](https://github.com/casbin/casbin) or [oauth0](https://auth0.com/) |
There was a problem hiding this comment.
From that web site, it refers as Auth0. Should we use that instead?
There was a problem hiding this comment.
yeah double checked, I think Auth0 is correct. oAuth is used for 2.0
docs/site/migration/auth/example.md
Outdated
| for the role mapping. | ||
|
|
||
| This guide includes a demo for using [casbin](#steps-with-casbin) and using | ||
| [oauth0](#steps-with-oauth0)(TBD) |
jannyHou
force-pushed
the
doc/example/acl-migration
branch
2 times, most recently
from
c02c77b to
674239f
Compare
nabdelgadir
left a comment
nabdelgadir
left a comment
There was a problem hiding this comment.
The rest of my comments were just stylistic so this LGTM 👍
jannyHou
force-pushed
the
doc/example/acl-migration
branch
from
674239f to
174204c
Compare
6 participants
This is the doc for #4520, it helps review feat PR #4571
The content is temporarily written in the readme file. It should be moved to https://github.com/strongloop/loopback-next/blob/auth-migration/docs/site/migration/auth/example.md
To make the tutorial more readable and consistent, I am going to put all the code snippet, cmd prompts into references. Now I only have the tutorial part, those references are in progress
Since the feat PR is still in change, I just write an outline for the authorizer and enforcers.
Checklist
👉 Read and sign the CLA (Contributor License Agreement) 👈
npm testpasses on your machinepackages/cliwere updatedexamples/*were updated👉 Check out how to submit a PR 👈