docs: add guideline doc for authorize component #2721

samarpanB · 2019-04-10T11:32:40Z

Add documentation specifying guidelines on creating an authorisation component

Related #538

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
API Documentation in code was updated
Documentation in /docs/site was updated
Affected artifact templates in packages/cli were updated
Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

bajtos

Thank you @samarpanB for the contribution!

We are using US spelling (authorization, not authorisation) in our docs, could you please update your content accordingly?

Also some of the CI builds are failing, please check the errors and fix the problem.

I'll leave the review of the actual content to somebody more familiar with our Authentication/Authorization story. /cc @raymondfeng @jannyHou

docs/site/Loopback-component.md

docs/site/sidebars/lb4_sidebar.yml

bajtos · 2019-04-12T15:02:38Z

Merge branch 'master' into authorisation-component-guide

Please don't use git merge, use git rebase master instead. We use linear git history with no merge commits. By rebasing early and often, you will avoid possibly costly merge conflicts later.

raymondfeng · 2019-04-12T16:16:56Z

@samarpanB Your contribution just came in time! We're evaluating a few options - #2718.

Please also check out #2687. For authorization decisions, interceptors provide more metadata, which may make it more suitable for deciding if a request should be allowed for a given method. Please note authorization might be also needed for non-controller methods, such as repositories or service proxies.

samarpanB · 2019-04-12T18:05:22Z

Merge branch 'master' into authorisation-component-guide

Please don't use git merge, use git rebase master instead. We use linear git history with no merge commits. By rebasing early and often, you will avoid possibly costly merge conflicts later.

I'll clean up the commit history and fix as you mentioned.

raymondfeng · 2019-04-12T18:29:48Z

@samarpanB There are instructions in https://loopback.io/doc/en/lb4/submitting_a_pr.html to help you tidy up the PR.

samarpanB · 2019-04-12T18:37:13Z

@samarpanB Your contribution just came in time! We're evaluating a few options - #2718.

Please also check out #2687. For authorization decisions, interceptors provide more metadata, which may make it more suitable for deciding if a request should be allowed for a given method. Please note authorization might be also needed for non-controller methods, such as repositories or service proxies.

@raymondfeng I think the idea for this documentation was to describe a minimalist implementation of authorization as a separate component, as I have mentioned here. It could be a starter guide for those who are already working on a production ready application and need authorisation at controller methods level to begin with.

BTW, I like the idea of interceptors.

samarpanB · 2019-04-15T06:51:34Z

Fixed all issues. Please have a look. @bajtos @raymondfeng

bajtos

LGTM as far as I am concerned. I did not review the actual content, I'll leave that to @raymondfeng and possibly other members of @strongloop/loopback-maintainers.

raymondfeng · 2019-04-16T15:25:27Z

docs/site/Loopback-component-authorization.md

+ * Authorize action method interface
+ */
+export interface AuthorizeFn {
+  (userPermissions?: string[]): Promise<boolean>;


Can we describe userPermissions? Does it represent the granted permissions or mapped roles for the user?

Description added.

docs/site/Loopback-component-authorization.md

raymondfeng · 2019-04-16T15:30:17Z

docs/site/Loopback-component-authorization.md

+decorator as below.
+
+```ts
+@authorize(['CanCreateRole'])


If it's for permissions, @authorize('canCreate') is better.

Refer the way permission keys are defined. They are specific to entities. There can be a user who is allowed to create new roles but not to create a new user. We need to have that flexibility.

docs/site/Loopback-component-authorization.md

samarpanB · 2019-04-17T12:05:40Z

@raymondfeng updated with changes as per your queries and feedback. Please have a look.

raymondfeng · 2019-04-17T16:24:33Z

docs/site/Loopback-component-authorization.md

+ */
+export interface UserPermission {
+  permission: PermissionKey;
+  is_allowed: boolean;


Let's use camel case, such as isAllowed or allowed.

Maybe it's better to use enum values, such as allow | deny.

renamed to allowed. I would prefer boolean over enum as it avoid any possibility of 3rd value which I intend here.

raymondfeng

@samarpanB Great writeup! I added a few more minor comments.

raymondfeng · 2019-04-17T20:35:35Z

@samarpanB Please squash all commits into one so that we can land it. Thanks! See https://loopback.io/doc/en/lb4/submitting_a_pr.html

samarpanB · 2019-04-18T04:00:21Z

@raymondfeng done. Thanks !

docs/site/Loopback-component-authorization.md

Add documentation specifying guidelines on creating an authorization component re loopbackio#538

samarpanB · 2019-04-18T14:51:39Z

@raymondfeng it seems CI build is failing due to reasons outside of this PR. I can see below in error log. Do I need to check and fix ?

CalculatorService
adds two numbers:
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/travis/build/strongloop/loopback-next/examples/soap-calculator/dist/tests/integration/services/calculator.service.integration.js)

CalculatorService
multiplies two numbers:
Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/travis/build/strongloop/loopback-next/examples/soap-calculator/dist/tests/integration/services/calculator.service.integration.js)

raymondfeng · 2019-04-18T15:00:40Z

it seems CI build is failing due to reasons outside of this PR. I can see below in error log. Do I need to check and fix ?

No, we host the test server in cloud and sometime it takes longer to wake up and causes timeout.

nabdelgadir · 2019-04-18T15:04:33Z

@samarpanB I restarted it so it's passing now.

raymondfeng · 2019-04-18T15:30:42Z

@samarpanB PR landed 👍 Thanks!

raymondfeng · 2019-05-15T17:09:07Z

docs/site/Loopback-component-authorization.md

+      const route = this.findRoute(request);
+      const args = await this.parseParams(request, route);
+      // Do authentication of the user and fetch user permissions below
+      const authUser: AuthResponse = await this.authenticateRequest(request);


@samarpanB Can you describe AuthResponse type? See #2896

Done. Just didn't want to add authentication stuff to authorization doc. hence, skipped this.

samarpanB requested review from bajtos and raymondfeng as code owners April 10, 2019 11:32

bajtos added community-contribution Authorization labels Apr 11, 2019

bajtos suggested changes Apr 12, 2019

View reviewed changes

docs/site/Loopback-component.md Outdated Show resolved Hide resolved

docs/site/sidebars/lb4_sidebar.yml Outdated Show resolved Hide resolved

bajtos requested a review from jannyHou April 12, 2019 15:01

samarpanB force-pushed the authorisation-component-guide branch 4 times, most recently from cd26500 to 1de37cb Compare April 15, 2019 06:36

bajtos changed the title ~~docs: add guideline doc for authorise component~~ docs: add guideline doc for authorize component Apr 16, 2019

bajtos approved these changes Apr 16, 2019

View reviewed changes

bajtos assigned raymondfeng Apr 16, 2019

raymondfeng reviewed Apr 16, 2019

View reviewed changes

docs/site/Loopback-component-authorization.md Show resolved Hide resolved

raymondfeng reviewed Apr 16, 2019

View reviewed changes

docs/site/Loopback-component-authorization.md Show resolved Hide resolved

raymondfeng reviewed Apr 16, 2019

View reviewed changes

docs/site/Loopback-component-authorization.md Show resolved Hide resolved

samarpanB force-pushed the authorisation-component-guide branch from 1f6c644 to 6cf945a Compare April 17, 2019 11:57

raymondfeng reviewed Apr 17, 2019

View reviewed changes

raymondfeng approved these changes Apr 17, 2019

View reviewed changes

samarpanB force-pushed the authorisation-component-guide branch from a8c35ec to 3eabf4d Compare April 17, 2019 18:24

samarpanB force-pushed the authorisation-component-guide branch 2 times, most recently from 4d3cd47 to cabf3f2 Compare April 18, 2019 03:59

nabdelgadir reviewed Apr 18, 2019

View reviewed changes

docs/site/Loopback-component-authorization.md Outdated Show resolved Hide resolved

nabdelgadir reviewed Apr 18, 2019

View reviewed changes

docs/site/Loopback-component-authorization.md Show resolved Hide resolved

samarpanB force-pushed the authorisation-component-guide branch from cabf3f2 to 8de0d79 Compare April 18, 2019 13:30

docs: add guideline doc for authorize component

637439d

Add documentation specifying guidelines on creating an authorization component re loopbackio#538

samarpanB force-pushed the authorisation-component-guide branch from 8de0d79 to 637439d Compare April 18, 2019 13:35

raymondfeng merged commit 03e5c3b into loopbackio:master Apr 18, 2019

samarpanB deleted the authorisation-component-guide branch April 19, 2019 14:11

raymondfeng reviewed May 15, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: add guideline doc for authorize component #2721

docs: add guideline doc for authorize component #2721

samarpanB commented Apr 10, 2019 •

edited

Loading

bajtos left a comment

bajtos commented Apr 12, 2019

raymondfeng commented Apr 12, 2019

samarpanB commented Apr 12, 2019

raymondfeng commented Apr 12, 2019

samarpanB commented Apr 12, 2019

samarpanB commented Apr 15, 2019

bajtos left a comment

raymondfeng Apr 16, 2019

samarpanB Apr 17, 2019

raymondfeng Apr 16, 2019

samarpanB Apr 17, 2019

samarpanB commented Apr 17, 2019

raymondfeng Apr 17, 2019

samarpanB Apr 17, 2019

raymondfeng left a comment

raymondfeng commented Apr 17, 2019 •

edited

Loading

samarpanB commented Apr 18, 2019

samarpanB commented Apr 18, 2019

raymondfeng commented Apr 18, 2019

nabdelgadir commented Apr 18, 2019

raymondfeng commented Apr 18, 2019

raymondfeng May 15, 2019

samarpanB May 15, 2019

docs: add guideline doc for authorize component #2721

docs: add guideline doc for authorize component #2721

Conversation

samarpanB commented Apr 10, 2019 • edited Loading

Checklist

bajtos left a comment

Choose a reason for hiding this comment

bajtos commented Apr 12, 2019

raymondfeng commented Apr 12, 2019

samarpanB commented Apr 12, 2019

raymondfeng commented Apr 12, 2019

samarpanB commented Apr 12, 2019

samarpanB commented Apr 15, 2019

bajtos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samarpanB commented Apr 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raymondfeng left a comment

Choose a reason for hiding this comment

raymondfeng commented Apr 17, 2019 • edited Loading

samarpanB commented Apr 18, 2019

samarpanB commented Apr 18, 2019

raymondfeng commented Apr 18, 2019

nabdelgadir commented Apr 18, 2019

raymondfeng commented Apr 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samarpanB commented Apr 10, 2019 •

edited

Loading

raymondfeng commented Apr 17, 2019 •

edited

Loading