Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: update docs on authentication package #2977

Merged
merged 1 commit into from Jun 13, 2019

Conversation

Projects
None yet
6 participants
@emonddr
Copy link
Contributor

commented May 27, 2019

Add a new section on loopback.io about AuthenticationComponent

Associated with : #2940

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 馃憟

@emonddr

This comment has been minimized.

Copy link
Contributor Author

commented May 27, 2019

This is a work on progress and is not ready for review. Just saving my work for today.

@emonddr emonddr force-pushed the dremond_authentication_readme branch 4 times, most recently from bfda07d to 5a283fb May 29, 2019

@emonddr emonddr marked this pull request as ready for review May 29, 2019

@emonddr emonddr requested review from bajtos and raymondfeng as code owners May 29, 2019

@emonddr

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

I've taken a first pass at explaining the authentication component's main pieces, and what a developer has to do for authentication. I haven't yet used any charts from our design document : https://github.com/strongloop/loopback-next/blob/master/packages/authentication/docs/authentication-system.md.

@dhmlau
Copy link
Contributor

left a comment

Looking good!
My review comments are more from the new user's perspective because I'm not familiar with what had been implemented. :)
Besides the review comments, I think it might be useful to have a diagram to show the relationship among the authentication component/strategy(and interface)/provider.

constructor(options?: ApplicationConfig) {
super(options);
...

This comment has been minimized.

Copy link
@dhmlau

dhmlau May 31, 2019

Contributor

@bajtos suggested, at some point, that we should make the ts code snippet a valid one. so, my suggested change would be:

// ... 

instead of simply
...

## Authentication Component

To utilize `authentication` in an application `application.ts`, you must load
the authentication component named `AuthenticationComponent`.

This comment has been minimized.

Copy link
@dhmlau

dhmlau May 31, 2019

Contributor

Apologize that I'm not familiar with this feature. So the name has to be AuthenticationComponent? That means we cannot have more than one Authentication component?

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 4, 2019

Author Contributor

@dhmlau

Apologize that I'm not familiar with this feature. So the name has to be AuthenticationComponent? That means we cannot have more than one Authentication component?

So most of the logic of @loopback/authentication ( the authentication metadata provider, the authentication action provider, and the authentication strategy provider ) is available in a component named AuthenticationComponent, and must be loaded by an application.

The user needs to create and register their own custom authentication strategies (user can have many of these), and implement their own token service or user service etc, and create a custom sequence that places the authentication action in a specific place.

But the core authentication framework logic is stored in 1 component.

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 7, 2019

Author Contributor

@dhmlau ^^ :)


As you can see in the example, an authentication strategy can inject custom
services to help it accomplish certain tasks. A complete example of a **basic**
authentication strategy can be found

This comment has been minimized.

Copy link
@dhmlau

dhmlau May 31, 2019

Contributor

Suggested change (see if it makes sense to you):
See the complete example for basic authentication strategy and jwt authentication strategy

[Authentication Decorator](https://loopback.io/doc/en/lb4/Decorators_authenticate.html)
on your controller endpoints.

They decorator's syntax is :

This comment has been minimized.

Copy link
@dhmlau

dhmlau May 31, 2019

Contributor

typo: They -> The

@authenticate(strategyName: string, options?: Object)
```

The strategy `name` is **mandatory** and the `options` object is **optional**.

This comment has been minimized.

Copy link
@dhmlau

dhmlau May 31, 2019

Contributor

I think it's kind of understood and no need for this paragraph?

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 4, 2019

Author Contributor

@dhmlau

I think it's kind of understood and no need for this paragraph?

You are right. Sometimes I like writing for the lowest common denominator (somebody new to TypeScript). I will remove it.

An example of the decorator when options **are** specified looks like this:

```ts
@authenticate('basic', { propertyOne : "foo", propertyTwo:"bar"})

This comment has been minimized.

Copy link
@dhmlau

dhmlau May 31, 2019

Contributor

nitpick: do you have some more realistic values that our users might want to use the options for?

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 4, 2019

Author Contributor

@dhmlau

nitpick: do you have some more realistic values that our users might want to use the options for?

Not really because we don't ship any authentication strategies at all ( just the interface). I could put a made up options object { useCookie: true} or {session:true} or { enforceLongPassword: true} but this would give them the wrong impression that our interface supports this kind of option. @jannyHou @raymondfeng , what are your thoughts on this?

This comment has been minimized.

Copy link
@jannyHou

jannyHou Jun 7, 2019

Contributor

I was also searching for some meaningful options for the basic auth in passport-http, but unfortunately didn't find any...like what Dom said, they are all related to the passport middleware instead of the strategy itself.

This comment has been minimized.

Copy link
@dhmlau

dhmlau Jun 7, 2019

Contributor

got it. maybe at least mention those options are specific to the strategy the user select?

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 10, 2019

Author Contributor

@jannyHou

I was also searching for some meaningful options for the basic auth in passport-http, but unfortunately didn't find any...like what Dom said, they are all related to the passport middleware instead of the strategy itself.

Actually, the focus of the discussion is not on passport stuff in this discussion. The @authenticate(strategyName, options?) decorator allows for passing in options. This gives the developer of custom authentication strategies a way to pass options in...if needed.

(Buy Janny is correct that the passport-based api have options they can pass in too...)

This comment has been minimized.

Copy link
@raymondfeng

raymondfeng Jun 10, 2019

Member

We can use something as follows:

@authenticate('basic', { /* some options for the strategy */})

## Adding an Authentication Action to a Sequence

In a LoopBack 4 REST application, each request passes through a stateless

This comment has been minimized.

Copy link
@dhmlau

dhmlau May 31, 2019

Contributor

does it only applicable to LB4 application with REST server? I find it a bit strange to refer it as "LoopBack 4 REST application".

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 4, 2019

Author Contributor

@dhmlau , I see what you mean. A user can create a gRPC server I guess, and not a typical REST API server which uses HTTP requests. Are people using LoopBack for more than REST servers at the moment?

I will remove the word 'REST' in this instance.

This comment has been minimized.

Copy link
@dhmlau

dhmlau Jun 7, 2019

Contributor

i think they can, right? @raymondfeng

This comment has been minimized.

Copy link
@jannyHou

jannyHou Jun 7, 2019

Contributor

@dhmlau The default sequence mentioned here is for REST server only.

And please note that in the AuthenticationStrategy interface, the Request type is imported from @loopback/rest, see https://github.com/strongloop/loopback-next/blob/master/packages/authentication/src/types.ts#L7

So...just speak for myself :-p mentioning REST application is ok to me here.

Those services like UserService and TokenService are reusable for other clients, but the authentication component does target on the REST requests.

This comment has been minimized.

Copy link
@raymondfeng

raymondfeng Jun 10, 2019

Member

The sequence so far only applies to RestServer. Maybe we can say In a LoopBack 4 application with REST API endpoints.

@emonddr emonddr force-pushed the dremond_authentication_readme branch from 2e31957 to 60e06bb Jun 4, 2019

@jannyHou
Copy link
Contributor

left a comment

The documentation part looks perfect to me 馃檪 I don't have anything else to add, just 2 tiny comments.

And for the diagram, the content looks good, a few suggestion:

  • A more accurate type would be Promise<UserProfile>:

Screen Shot 2019-06-07 at 2 37 14 PM

  • nitpick: 'some' --> 'amodel' or 'somemodel'. Just a suggestion based on personally preferred naming conversion.

Screen Shot 2019-06-07 at 2 34 36 PM

  • Maybe re-organize the rectangles in a more sorted way?
The decorator's syntax is :

```ts
@authenticate(strategyName: string, options?: Object)

This comment has been minimized.

Copy link
@jannyHou

jannyHou Jun 7, 2019

Contributor

Object --> object :)

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 7, 2019

Author Contributor

@jannyHou thanks for your feedback.

A more accurate type would be Promise

When creating this diagram, space is very limited, so I chose to avoid using stuff like:
this. and Promise< > to save space, and give a general idea of the logic. The documentation text in the sections below the diagram will show the correct code.

nitpick: 'some' --> 'amodel' or 'somemodel'. Just a suggestion based on personally preferred naming conversion.

The lb4 controller CLI creates controller files is this format: someName.controller.ts . And all the artifacts follow the same pattern. I think my filename is ok.

Maybe re-organize the rectangles in a more sorted way?

It took a long time to create the diagram and all its pieces and labels and move them into place, so I am reluctant to iteratively move all the PowerPoint shapes/labels/arrows around unless someone sends me a pencil and paper picture of a re-design.

This comment has been minimized.

Copy link
@jannyHou

jannyHou Jun 9, 2019

Contributor

When creating this diagram, space is very limited, so I chose to avoid using stuff like:
this. and Promise< > to save space

馃憤 Fair enough.

And the naming and diagram comments are just nitpicks 馃檪 feel free to ignore.

```

{% include tip.html content="
To avoid repeating the same options in the <b>@authenticate</b> decorator for many endpoints in a controller, you can instead define global options which can be injected into an authentication strategy thereby allowing you to avoid specifying the options object in the decorator itself. For controller endpoints that need to override a global option, you can specify it in an options object passed into the decorator. Your authentication strategy would need to handle the option overrides.

This comment has been minimized.

Copy link
@jannyHou

jannyHou Jun 7, 2019

Contributor

It would be good if we can show a code example here. While I also realized we don't have an auth option sample yet, maybe use the greeter extension https://github.com/strongloop/loopback-next/tree/master/examples/greeter-extension#implement-an-extension ?

This comment has been minimized.

This comment has been minimized.

Copy link
@jannyHou

jannyHou Jun 7, 2019

Contributor

@emonddr Cool the unit test is much more relevant. Looks good to me.

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 11, 2019

Author Contributor

@jannyHou , I've changed my mind about pointing to the unit test. It doesn't really show global options being overridden by request options from the decorator. Perhaps after this PR lands, we can have a follow up PR to add a section about injecting global options into the strategy, and then the strategy checking for overriding options from the decorator and then overriding the global options. I have an example of this I can dig up from one of my workspaces. But I won't be able to prepare it nicely today for @raymondfeng to review.

This comment has been minimized.

Copy link
@jannyHou

jannyHou Jun 11, 2019

Contributor

@emonddr Sure it would be good to have an example and point to it. +1 for improving it in another PR.

@nabdelgadir
Copy link
Contributor

left a comment

Just some nitpicks but 馃憤. When you're linking to another docs on the website, if you use {file-name}.md, then, in vscode, it goes to the file instead of the website so that's why I recommend changing those (there are more of them in the file that I didn't comment on).

Show resolved Hide resolved docs/site/Loopback-component-authentication.md Outdated
Show resolved Hide resolved docs/site/Loopback-component-authentication.md Outdated
This document describes the details of the LoopBack 4 `Authentication`
component.

![authentication_overview](./imgs/authentication_overview.png) Basically, to

This comment has been minimized.

Copy link
@nabdelgadir

nabdelgadir Jun 7, 2019

Contributor

Nitpick: can you add a new line between the image and paragraph?

This comment has been minimized.

Copy link
@dhmlau

dhmlau Jun 7, 2019

Contributor

Thanks for adding the diagram. I was thinking of a more higher-level diagram to help users to understand the big picture first. :)
I was thinking something along the line of the diagram below (it's probably not reflecting what's actually happening, but just an illustration). Does it make sense?
Screen Shot 2019-06-07 at 5 36 07 PM

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 10, 2019

Author Contributor

Perhaps we should have a whiteboard session and see how we can move between highlevel and lowlevel diagrams and when...

This comment has been minimized.

Copy link
@dhmlau

dhmlau Jun 10, 2019

Contributor

It's just a suggestion. Since I don't have much background on the detailed design, so a higher level diagram helps me better to understand. Maybe it's just personal preference.

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 11, 2019

Author Contributor

@dhmlau

The action provider isn't a composite part of the strategy provider, and the metadata provider can be used from anywhere (actually all of them are set up like this), so it is more accurate to have them separate from each other than contained in each other as in my diagram.

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 11, 2019

Author Contributor

@dhmlau @jannyHou how about this for a high level?
image

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 11, 2019

Author Contributor
  1. wondering if I should show custom authentication strategies at all in this diagram and just leave it as extensions
  2. wondering if I should specify the parameters of the decorator: strategyName:string, options?:object .

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 11, 2019

Author Contributor

actually this one is better

image

Show resolved Hide resolved docs/site/Loopback-component-authentication.md Outdated
Show resolved Hide resolved docs/site/Loopback-component-authentication.md Outdated
Show resolved Hide resolved docs/site/Loopback-component-authentication.md Outdated
Show resolved Hide resolved docs/site/Loopback-component-authentication.md Outdated
secure your API endpoints, you need to: create/register a custom authentication
strategy with a unique **name**, insert the authentication action in a custom
sequence, and then decorate the endpoints of a controller with the
`@authenticate(strategyName, options?)` authentication decorator. The

This comment has been minimized.

Copy link
@raymondfeng

raymondfeng Jun 7, 2019

Member

Let's use a bullet list.

* If the user credentials are missing, this method should throw an error, or return 'undefined'
* and let the authentication action deal with it.
*
* @param request

This comment has been minimized.

Copy link
@raymondfeng

raymondfeng Jun 7, 2019

Member

Add some description for request, such as:

@param request - Express request object

@emonddr emonddr force-pushed the dremond_authentication_readme branch 4 times, most recently from 975741c to 93496eb Jun 11, 2019

- Authorization

`Authentication` is a process of verifying someone's identity before a protected
endpoint is accessed.

This comment has been minimized.

Copy link
@raymondfeng

raymondfeng Jun 11, 2019

Member

endpoint -> resource?

`Authentication` is a process of verifying someone's identity before a protected
endpoint is accessed.

`Authorization` is a process of verifying the actions that an authenticated user

This comment has been minimized.

Copy link
@raymondfeng

raymondfeng Jun 11, 2019

Member

verifying ... -> deciding if a user can perform an action on a protected resource?

This comment has been minimized.


Here is a **high level** overview of the authentication component.

![authentication_overview_highlevel](./imgs/authentication_overview_highlevel.png)

This comment has been minimized.

Copy link
@raymondfeng

raymondfeng Jun 11, 2019

Member

Let's crop the diagram so that there is no blank area to the left:

image

![authentication_overview_highlevel](./imgs/authentication_overview_highlevel.png)

It is made up of a decorator, a few providers, and an extension point to which
authentication strategies must be registered.

This comment has been minimized.

Copy link
@raymondfeng

raymondfeng Jun 11, 2019

Member

Terms like decorator/provider/extension point seem to be too low level here. We can say:

  • Decorators to express authentication requirement on controller methods
  • A provider to access method-level authentication metadata
  • An action in the REST sequence to enforce authentication
  • An extension point to discover all authentication strategies and handle the delegation

This comment has been minimized.

Copy link
@emonddr

emonddr Jun 11, 2019

Author Contributor

Decorators

A decorator


Here is a **detailed** overview of the authentication component.

![authentication_overview_detailed](./imgs/authentication_overview_detailed.png)

This comment has been minimized.

Copy link
@raymondfeng
- insert the authentication action in a custom sequence
- decorate the endpoints of a controller with the
`@authenticate(strategyName, options?)` authentication decorator

This comment has been minimized.

Copy link
@raymondfeng

raymondfeng Jun 11, 2019

Member

Maybe change the order as follows:

  1. decorate (app developer)
  2. add authentication action (app developer/cli)
  3. create custom authentication strategy (extension developer)
  4. register a custom authentication strategy (app developer)
}
```

Authentication is **not** part of the default sequence of actions, so you must

This comment has been minimized.

Copy link
@raymondfeng

raymondfeng Jun 11, 2019

Member

By default, ...

@emonddr emonddr force-pushed the dremond_authentication_readme branch from b5aa8d6 to 8204304 Jun 12, 2019

}
```
You can find a working **example** of a LoopBack 4 shopping cart application

This comment has been minimized.

Copy link
@dhmlau

dhmlau Jun 12, 2019

Contributor

nitpick: I'm not quite sure if we need to bold the "example" word.. similar to a few incidents towards the end of the last section. It could be just personal preference.

@dhmlau

dhmlau approved these changes Jun 12, 2019

Copy link
Contributor

left a comment

LGTM. Thanks for addressing my comments. Please at least get approvals from @raymondfeng and ideally @jannyHou too on this. Thanks.

@raymondfeng
Copy link
Member

left a comment

Great details!

@emonddr emonddr force-pushed the dremond_authentication_readme branch from 8204304 to 93f27bf Jun 12, 2019

@b-admike
Copy link
Member

left a comment

Overall LGTM; I like how detailed the documentation is!

point in an application `application.ts` is as simple as:

```ts
registerAuthenticationStrategy(this, BasicAuthenticationStrategy);

This comment has been minimized.

Copy link
@b-admike

b-admike Jun 12, 2019

Member

can we show the import for registerAuthenticationStrategy here?

@nabdelgadir
Copy link
Contributor

left a comment

馃憤

@jannyHou
Copy link
Contributor

left a comment

馃挭 LGTM

@emonddr emonddr force-pushed the dremond_authentication_readme branch 4 times, most recently from 5e1bde4 to c681657 Jun 12, 2019

docs: add section about using authentication component
add new section in documentation about using the authentication component

@emonddr emonddr force-pushed the dremond_authentication_readme branch from c681657 to c4b6559 Jun 13, 2019

@emonddr emonddr removed the request for review from bajtos Jun 13, 2019

@emonddr emonddr merged commit 956300a into master Jun 13, 2019

4 checks passed

clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 91.714%
Details

@delete-merged-branch delete-merged-branch bot deleted the dremond_authentication_readme branch Jun 13, 2019

@jannyHou jannyHou referenced this pull request Jun 13, 2019

Merged

[LABS] feat: adapter that wraps strategy #3056

3 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.