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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authentication service/tests #39

Merged
merged 7 commits into from
Oct 19, 2023
Merged

Authentication service/tests #39

merged 7 commits into from
Oct 19, 2023

Conversation

elvincheng3
Copy link
Contributor

@elvincheng3 elvincheng3 commented Oct 17, 2023

Description/Problem

Closes Authentication service/tests

Add basic auth groundwork.

Solution

Add basic auth groundwork.

Dependencies

[X] This PR adds new dependencies

Testing

Please describe how you tested this PR (manually and/or automatically)
Provide instructions so we can reproduce, and include screenshots of UI changes.

  • Manual Tests?
    • Test out the /auth/login route, and try accessing the protected root route after authenticating with JWT token.
  • Unit/Integration/E2E Tests?
    • Added unit tests for auth service

@vercel
Copy link

vercel bot commented Oct 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mfa-form-automator ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 17, 2023 4:52pm

This reverts commit 11ced20.
@elvincheng3 elvincheng3 changed the title basic auth framework Authentication service/tests Oct 17, 2023
@elvincheng3 elvincheng3 marked this pull request as ready for review October 17, 2023 21:03
@elvincheng3
Copy link
Contributor Author

AuthModule,
PassportModule,
JwtModule.register({
secret: process.env.JWT_SECRET,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does secret mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's what's used to generate the jwt token and decode it

if (user?.pswdHash && !(await bcrypt.compare(pass, user.pswdHash!))) {
return null;
}
// eslint-disable-next-line @typescript-eslint/no-unused-vars
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for eslint? Do we need to keep this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're basically splitting out the password from the object and using js destructuring to do it, but that means we have an unused variable that eslint complains abt. just telling eslint to ignore this one line for that error

Copy link
Contributor

@AnshulShirude AnshulShirude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 small questions are linked, but looks good.

@elvincheng3 elvincheng3 merged commit c5d1c3f into main Oct 19, 2023
6 checks passed
@elvincheng3 elvincheng3 deleted the auth branch October 24, 2023 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants