feat: add authorization component #1205

raymondfeng · 2018-03-28T18:26:17Z

Connect to #538

Checklist

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 packages/example-* were updated

virkt25 · 2018-03-28T19:31:28Z

packages/authorization/src/decorators/authorize.ts

+    authorize({allow: ['$authenticated']});
+  export const denyAll = () => authorize({deny: ['$everyone']});
+  export const denyUnauthenticated = () =>
+    authorize({deny: ['$anonymous', 'unauthenticated']});


$unauthenticated?

What is the difference between $anonymous and $unauthenticated ?

One must be careful while implementing the all roles. Very often what the developer wants is all but . A developer may be tempted to write something like :

@authorize({allow: ['MODERATOR']}) @authorize.denyAll() // maybe a helper like this may be wanted @authorize.denyAllBut(['MODERATOR']) // or of course @authorize.allowAllBut(['$anonymous']) myMethod();

This is a case to handle. The order of annotation is very important and counter intuitive so the documentation must warn the user about that. Hence the helper denyAllBut();

There should be a rule that makes the order of decorators irrelevant.

bajtos · 2018-04-17T12:17:10Z

packages/authorization/README.md

+## Basic use
+
+Start by decorating your controller methods with `@authorize` to require the
+request to be authenticated.


authenticated or authorized?

bajtos · 2018-04-17T12:17:55Z

packages/authorization/src/authorization-component.ts

+    this.providers = {
+      [AuthorizationBindings.AUTHORIZE_ACTION.key]: AuthorizationProvider,
+      [AuthorizationBindings.AUTHORIZE_METADATA
+        .key]: AuthorizationMetadataProvider,


I find this formatting rather weird, was it created by prettier?

bajtos · 2018-04-17T12:19:52Z

packages/authorization/src/providers/authorize.ts

+      securityContext: SecurityContext,
+      metadata: AuthorizationMetadata,
+    ) => {
+      return AuthorizationDecision.ALLOW;


Is this correct? Shouldn't the reference implementation actually handle $everyone, $unauthenticated and $anonymous roles/scopes as defined in packages/authorization/src/decorators/authorize.ts?

bajtos · 2018-04-17T12:20:14Z

packages/authorization/src/decorators/authorize.ts

+    authorize({allow: ['$authenticated']});
+  export const denyAll = () => authorize({deny: ['$everyone']});
+  export const denyUnauthenticated = () =>
+    authorize({deny: ['$anonymous', '$unauthenticated']});


What is the difference between anonymous and unauthenticated?

JesusTheHun · 2018-10-11T16:24:51Z

packages/authorization/test/unit/authorize-decorator.test.ts

+  describe('@authorize decorator', () => {
+    it('can add authorize metadata to target method', () => {
+      class TestClass {
+        @authorize({allow: ['ADMIN'], scopes: ['secret.read']})


I'm confused by the definition of scope and the role based definition in the same decorator.
@scope('secret.read') this is my scope definition
@role({ allow: ['ADMIN']}) this is my role based access definition

These are two completely different things.

Scenario 1 : I use role based authorization, I decorate my route to define who can access to this route. This is @role.
Scenario 2 : I use scope based authorization, my user is allowed to access a set of scopes, coming from database or token ; if the route he tries to access is within his scopes, access is granted. This is @scope
Scenario 3 : I use both, every role has a set of scope associated, hard coded or defined through user land configuration. This is @scope ; in my authentication configuration I bind each role to a set of scopes.

JesusTheHun · 2018-10-11T16:30:55Z

packages/authorization/test/unit/authorize-decorator.test.ts

+
+    it('can add denyAll to target method', () => {
+      class TestClass {
+        @authorize.denyAll()


From a security perspective, denyAll() should be the default behavior. If the user wants a whitelist approach, it should be able to set it in the component configuration

JesusTheHun · 2018-10-11T17:05:26Z

For future releases, it will be nice to bring a context based authorization. Sometimes referred as a voter approach.

@vote({ voter: 'binding.myService', identifier: 'articleId' })
// or with helper
@voteFromRepository({ voter: ArticleRepository, identifier: 'articleId' })
@scope('article.write');
myWriteMethod(articleId: string);

class ArticleRepository implements Voter {}
interface Voter {
    public vote(user: MyUserModel, articleId: string, scope: string): boolean
}

The authorization decorator retrieve the articleId at runtime, ask the ArticleRepository to tell is this user is allowed to perform action on this very article, within the provided scope. The MyUserModel instance is injected by asking the authentication component.

JesusTheHun · 2018-11-07T09:07:40Z

Is there any progress on this subject ?

We would like to launch several API using LB4 in the next months but it lacks authorization and authentication components.

edit : Is it realistic to expect these two components to be added to the December milestone ?

bajtos · 2018-11-08T12:56:27Z

Is there any progress on this subject ?

Right now, we are trying to figure out a plan how to approach the large task of figuring out authorization implementation. I summarized my current thinking in #1035 (comment), but it's not definitive yet.

We would like to launch several API using LB4 in the next months but it lacks authorization and authentication components.
edit : Is it realistic to expect these two components to be added to the December milestone ?

Considering our current velocity and also the fact that many of us will be taking time off (vacations) in December, I find it very unlikely to have production-level implementation of authentication and authorization components done by the end of 2018.

JesusTheHun · 2018-11-08T13:13:34Z

Thank you for your response.
Do you see anything I can do to help you on this path ?

bajtos · 2018-11-09T16:00:49Z

@JesusTheHun thank you for the offer! I am afraid this work is currently blocked by missing hasOne relation that @b-admike is working on right now - see #1879. Once hasOne is landed, you can help us with #1996, #1997 and #1998. But be warned, this is an exploratory work that is likely to discover missing pieces. These missing pieces may be non-trivial to add and requiring design discussions first.

raymondfeng · 2019-05-10T05:10:42Z

I rewrote the code based on interceptors.

dhmlau · 2019-07-23T14:57:22Z

@emonddr @jannyHou, per our discussion earlier, I think @jannyHou will be taking over this PR, so I'm assigning to her.

@jannyHou, please reach out to @raymondfeng when you're about to start on this. He mentioned this PR is almost ready, might need some more test coverage and other things. It would be good to connect with him on more details. Thanks.

…ss level

jannyHou · 2019-08-13T16:59:29Z

For reviewers: here is a PoC PR that demos applying the authorization module in a real user scenario:
loopbackio/loopback4-example-shopping#231

agnes512

Went through the tests and the code. Except some small questions it looks good to me( if I understand them correctly).

agnes512 · 2019-08-13T19:34:52Z

packages/authorization/README.md

+import {get} from '@loopback/rest';
+
+export class MyController {
+  @authorize({allow: ['ADMIN']})


So is this saying user profile will be available for this user as a role of ADMIN?

agnes512 · 2019-08-13T19:38:32Z

packages/authorization/src/decorators/authorize.ts

+ * @param target Target object/class
+ * @param methodName Target method
+ */
+export function getAuthorizeMetadata(


I am ok with both names. But it'd be better if we make it more consistent 😛

agnes512 · 2019-08-13T19:40:17Z

packages/authorization/src/types.ts

+export const EVERYONE = '$everyone';
+export const AUTHENTICATED = '$authenticated';
+export const UNAUTHENTICATED = '$unauthenticated';
+export const ANONYMOUS = '$anonymous';


does ANONYMOUS mean the user does not need authentication, and UNAUTHENTICATED mean the user didn't pass the authentication?
Can we also add a comment on them?

raymondfeng · 2019-08-13T20:02:25Z

See https://docs.spring.io/spring-security/site/docs/3.0.x/reference/anonymous.html

I'm fine to remove it for simplicity.

jannyHou · 2019-08-13T23:20:18Z

See https://docs.spring.io/spring-security/site/docs/3.0.x/reference/anonymous.html
I'm fine to remove it for simplicity.

Thank you 👍 I will keep it in this PR...if anyone has a good reason to remove it I can quickly make another PR to remove it lol.

raymondfeng requested a review from bajtos as a code owner March 28, 2018 18:26

raymondfeng added needs-priority and removed needs-priority labels Mar 28, 2018

virkt25 reviewed Mar 28, 2018

View reviewed changes

raymondfeng force-pushed the authorization branch 2 times, most recently from b70c54c to 23a85a2 Compare April 11, 2018 16:12

bajtos reviewed Apr 17, 2018

View reviewed changes

bajtos removed the review label Jun 25, 2018

raymondfeng mentioned this pull request Jun 25, 2018

[Spike] Allow extensions to contribute custom authorization providers and strategies #1462

Closed

raymondfeng force-pushed the authorization branch 2 times, most recently from 1f6e799 to 2b66269 Compare August 21, 2018 16:32

dhmlau mentioned this pull request Oct 11, 2018

[Roadmap] 🚀1Q2019 🚀 #1839

Closed

20 tasks

JesusTheHun reviewed Oct 11, 2018

View reviewed changes

raymondfeng force-pushed the authorization branch 3 times, most recently from 3046254 to cf5b9c9 Compare October 18, 2018 05:47

raymondfeng force-pushed the authorization branch from cf5b9c9 to 096f20e Compare October 31, 2018 20:36

bajtos added feature major Authorization labels Jan 7, 2019

raymondfeng force-pushed the authorization branch from 096f20e to f9f0d70 Compare April 22, 2019 19:49

raymondfeng force-pushed the authorization branch from f9f0d70 to a24807f Compare May 10, 2019 05:09

dhmlau mentioned this pull request Jul 9, 2019

Monthly Milestone - Jul 2019 🌴⚽️🍒 #3241

Closed

30 tasks

dhmlau assigned jannyHou Jul 23, 2019

agnes512 mentioned this pull request Jul 31, 2019

Monthly Milestone - Aug 2019 🐟🔥🍤 #3482

Closed

32 tasks

dhmlau mentioned this pull request Aug 2, 2019

Create a common layer for authentication & authorization component #2900

Closed

raymondfeng force-pushed the authorization branch from 54f8510 to 997eb86 Compare August 2, 2019 15:15

This was referenced Aug 9, 2019

feat: add authorization to user/order route loopbackio/loopback4-example-shopping#231

Merged

Refactor authorization #3529

Merged

jannyHou force-pushed the authorization branch from 997eb86 to 0ee7df4 Compare August 12, 2019 16:52

raymondfeng and others added 9 commits August 13, 2019 11:43

feat: add authorization component

d91f362

feat(authorization): add an acceptance to demonstrate casbin integration

f9b9469

chore(authorization): add more interfaces and documents

4ed12d3

chore(authorization): upgrade dependencies

b9baef4

chore(authorization): rename some types

15c64ce

feat(authorization): allows @authorize decorator to be applied at cla…

5675415

…ss level

chore(authorization): add an architecture diagram

d90bfeb

chore(authorization): more refactoring

b86072c

refactor(authorization): improve pr

3c7ae65

jannyHou force-pushed the authorization branch from 72250ed to 3c7ae65 Compare August 13, 2019 15:43

jannyHou approved these changes Aug 13, 2019

View reviewed changes

agnes512 approved these changes Aug 13, 2019

View reviewed changes

refactor: apply feedback

756bacf

jannyHou changed the title ~~[WIP] feat: add authorization component~~ feat: add authorization component Aug 13, 2019

jannyHou merged commit 594a743 into master Aug 14, 2019

delete-merged-branch bot deleted the authorization branch August 14, 2019 14:13

jannyHou added this to the Aug 2019 milestone milestone Aug 14, 2019

achrinza mentioned this pull request Nov 23, 2019

docs(authorization): legacy "extract common layer" notice #4196

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add authorization component #1205

feat: add authorization component #1205

raymondfeng commented Mar 28, 2018 •

edited

Loading

virkt25 Mar 28, 2018

JesusTheHun Oct 11, 2018

JesusTheHun Oct 11, 2018 •

edited

Loading

raymondfeng Oct 17, 2018

bajtos Apr 17, 2018

bajtos Apr 17, 2018

bajtos Apr 17, 2018

bajtos Apr 17, 2018

JesusTheHun Oct 11, 2018

JesusTheHun Oct 11, 2018

JesusTheHun commented Oct 11, 2018

JesusTheHun commented Nov 7, 2018 •

edited

Loading

bajtos commented Nov 8, 2018

JesusTheHun commented Nov 8, 2018

bajtos commented Nov 9, 2018

raymondfeng commented May 10, 2019

dhmlau commented Jul 23, 2019

jannyHou commented Aug 13, 2019

agnes512 left a comment

agnes512 Aug 13, 2019

agnes512 Aug 13, 2019

agnes512 Aug 13, 2019

raymondfeng commented Aug 13, 2019 •

edited

Loading

jannyHou commented Aug 13, 2019

feat: add authorization component #1205

feat: add authorization component #1205

Conversation

raymondfeng commented Mar 28, 2018 • edited Loading

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JesusTheHun Oct 11, 2018 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JesusTheHun commented Oct 11, 2018

JesusTheHun commented Nov 7, 2018 • edited Loading

bajtos commented Nov 8, 2018

JesusTheHun commented Nov 8, 2018

bajtos commented Nov 9, 2018

raymondfeng commented May 10, 2019

dhmlau commented Jul 23, 2019

jannyHou commented Aug 13, 2019

agnes512 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

raymondfeng commented Aug 13, 2019 • edited Loading

jannyHou commented Aug 13, 2019

raymondfeng commented Mar 28, 2018 •

edited

Loading

JesusTheHun Oct 11, 2018 •

edited

Loading

JesusTheHun commented Nov 7, 2018 •

edited

Loading

raymondfeng commented Aug 13, 2019 •

edited

Loading