-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add simple tutorial for getting started with authentication #4223
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e833a99 to
90bc4ba
Compare
|
|
||
| // In the constructor's arguments | ||
|
|
||
| @inject(AuthenticationBindings.CURRENT_USER, { optional: true }) private user: UserProfile & User, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in https://github.com/joeytwiddle/lb-4-authentication-session-tokens-example/blob/first-steps/src/controllers/ping.controller.ts#L39, it doesn't have the & User portion. Do you mean just:
@inject(AuthenticationBindings.CURRENT_USER, { optional: true }) private user: UserProfile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I put UserProfile & User in the tutorial is that we found it pretty helpful in our applications to pass the entire User model to the controllers. So I wanted to show readers that is an option available to them.
But the demo cannot do this because there is no User model or repository! That's why I just left it out of my own PingController.
My hope was that people doing the tutorial will delete the User if they don't have a collection, or import it if they do have a collection.
Resolution: I could drop back to just UserProfile and add a comment:
// If you decided earlier to pass the full User object to controllers, put `UserProfile & User` hereDoes that sound ok?
(Done!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove this comment altogether.
We are using this trick to pass the entire User model to controllers, but that's out of the scope of this tutorial.
docs/site/tutorials/authentication/Quick-Authentication-Tutorial-Session-Tokens.md
Show resolved
Hide resolved
|
Thanks @joeytwiddle again for your PR! Your tutorial is clearly written that I can easily follow along. I think it has very good instructions in adding the basic authentication to a LB4 app. My only concern that about the "next step" section. I'm thinking that it might be a good transition to the JWT tutorial? I'd like to get some thoughts from @emonddr and @jannyHou since they've developed this package and the related docs. |
docs/site/tutorials/authentication/Quick-Authentication-Tutorial-Session-Tokens.md
Show resolved
Hide resolved
|
Thanks for trying it out @dhmlau. My goal with this tutorial was to introduce the authentication basics for someone who might not want to use JWT. (Perhaps they want to integrate with their existing authentication system.) But I can re-arrange that section to promote JWT at the top. Hopefully this will help developers make the right choice for them. (Done!) |
f33a401 to
f5802f0
Compare
f5802f0 to
0b1bb8d
Compare
|
@joeytwiddle, actually I was thinking to keep the tutorial to be only about basic authentication without JWT authentication. :) . It's because we have https://loopback.io/doc/en/lb4/Authentication-Tutorial.html for JWT already, so it would be good to keep them separate? What I'm proposing is:
What do you think? |
|
OK I guess that makes sense. Just implement But I'm not sure when I'll get around to changing it. If you don't want to wait, please feel free to modify this PR as desired. (I believe loopback organisation will have push permissions to this branch.) |
| return {email: foundUser.email, [securityId]: foundUser.id}; | ||
| // Or we can return the full User object, with the securityId added to it | ||
| // This will give the controllers more user properties to play with | ||
| foundUser[securityId] = foundUser.id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now that this upsets TypeScript. In our app we just @ts-ignore it!
Is there any nice way to do this?
(We like to pass the user directly to our controllers.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was half-way trying out your commented portion :)
I used the snippet below instead: (my User might be different than yours)
return {[securityId]: foundUser.email, id: foundUser.email}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that looks good. I should not be trying to return a User object in this tutorial.
Although for anyone interested, what we do is this:
// Add an extra field to the user object, so we can pass the full User model to controllers
// @ts-ignore
user[securityId] = user.id;
return user as (UserProfile & User);
|
@joeytwiddle , I can make my suggested changes to your branch. Will ask you to review when it's done. Thanks! |
|
@joeytwiddle @dhmlau what's the status of this pull request? |
|
@bajtos, I'm working on a rework of the authentication tutorial with the content from @joeytwiddle here. Work-in-progress version is here: https://github.com/dhmlau/loopback4-authentication-app. Will try to work on it next week. |
|
|
||
| Or if you want to complete the session token based system, read on. | ||
|
|
||
| ## Completing the session tokens system |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by session tokens , do you mean, a client web session with cookies ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deepakrkris More or less.
My personal experience has been passing the tokens through the Authorization header.
We use Authorization: Basic to log in, then Authorization: Bearer for the rest of the session.
I think using cookies would be a fairly similar approach. (Log in to create a new cookie, after that use the cookie to authenticate.)
|
@agnes512 Yes todo-jwt is the new entry point of authentication tutorial. @joeytwiddle 's tutorial about basic authentication is also good, I am thinking of landing it as an auth tutorial for people to quickly get started with basic auth. They both help. |
|
We just switch the contribution method from CLA to DCO, making your contribution easier in the future. Please sign the commits with DCO by amending your commit messages with Please refer to this docs page for details. Thanks! |
|
This pull request has been marked stale because it has not seen activity within two months. It will be closed within 14 days of being stale unless there is new activity. |
|
This pull request has been marked stale because it has not seen activity within two months. It will be closed within 14 days of being stale unless there is new activity. |
|
This pull request has been closed due to continued inactivity. If you are interested in finishing the proposed changes, then feel free to re-open this pull request or open a new one. |
This tutorial is intended to make it easy for a developer to quickly get familiar with the basic code needed for authentication. From this point they can then build any authentication system they like.
Migrated from loopbackio/loopback.io#910
Fixes loopbackio/loopback.io#911
Checklist
npm testpasses on your machineCode conforms with the style guide
Documentation in /docs/site was updated
npm run verify:docscompleted. Sorry, I didn't run this because I don't want to install Bundler.👉 Check out how to submit a PR 👈