-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add JWT authentication example #5463
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
e2d4f73 to
07b10e1
Compare
jannyHou
left a comment
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.
@dhmlau The lint error is caused by local eslint plugin conflicts with the global one.
I solved it on local by:
- git clone a fresh lb-next repo
- remove the 4 dev dependencies I marked in the package.json file
npm ito reinstall the dependencies, the package-lock.json file will also update- then
npm run lint:fix
I tried npm run start in the example repo but seems the app doesn't start, it's caused by the index.js file. It doesn't run the main function.
I updated my changes in branch https://github.com/strongloop/loopback-next/tree/jwt-example-update, feel free to merge it or use the code there. The app runs on my local and lint passes
examples/todo-jwt/LICENSE
Outdated
| @@ -0,0 +1,25 @@ | |||
| Copyright (c) IBM Corp. 2018,2019. | |||
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.
nitpick: 2020
| type: 'string', | ||
| required: true, | ||
| }) | ||
| password: string; |
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 password should have json property minLength: 8 as well to make sure the new user has valid password
jannyHou
left a comment
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.
👍Tested on local the app works well, LGTM a few nitpicks.
While working on #5421, I think it might be better to have an example app, so that the tutorial can point to the source code.
What's left: CLI, add this example to Examples.md, and CODEOWNERS file.
Related to: #5421
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 👈