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

Fix(auth.controller.ts): Ensure delivery of email which contains token for two factor authentication #46

Merged
merged 1 commit into from
May 16, 2024

Conversation

tuyishimekyrie
Copy link
Collaborator

@tuyishimekyrie tuyishimekyrie commented May 14, 2024

Fix: Ensure Correct Token Delivery for Two-Factor Authentication

This commit addresses an issue where the token sent via email for two-factor authentication was not reliably delivered to users, preventing them from verifying their identity for enabling two-factor authentication.

Changes Made:

  • Adjusted the email sending mechanism to ensure the reliable delivery of the verification token to users' inboxes.

Endpoints Affected:

  • /api/v1/auth/send-verification-email

These endpoints have been thoroughly tested to confirm the successful delivery of the verification token. Users can now reliably receive and use the token to enable two-factor authentication for their accounts.

This fix enhances the security and usability of the authentication system by ensuring that users can effectively receive tokens through email to verify their identity.

ℹ️ Description

Please write about the changes implemented in this PR. Also write a short description about why the change is implemented.

Fixes #(issue)

🧪 How can this be tested?

  • /api/v1/auth/send-verification-email

📍 Type of change

Please check the options that are relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or Enhancement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

✅ Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@tuyishimekyrie tuyishimekyrie added the In progress Use this when you're still working on your task label May 14, 2024
@tuyishimekyrie tuyishimekyrie self-assigned this May 14, 2024
@tuyishimekyrie tuyishimekyrie changed the title Fix: Ensure delivery of email which contains token for two factor aut… Fix(auth.controller.ts): Ensure delivery of email which contains token for two factor authentication May 14, 2024
@tuyishimekyrie tuyishimekyrie added the Needs peer reviews Use this when ready for peer reviews label May 14, 2024
@tuyishimekyrie tuyishimekyrie removed the In progress Use this when you're still working on your task label May 14, 2024
Aldot-02
Aldot-02 previously approved these changes May 14, 2024
Copy link
Collaborator

@Aldot-02 Aldot-02 left a comment

Choose a reason for hiding this comment

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

Well done!

Oliviier-dev
Oliviier-dev previously approved these changes May 15, 2024
Copy link
Collaborator

@Oliviier-dev Oliviier-dev left a comment

Choose a reason for hiding this comment

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

LGTM

shebz2023
shebz2023 previously approved these changes May 15, 2024
Copy link
Collaborator

@shebz2023 shebz2023 left a comment

Choose a reason for hiding this comment

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

LGTM

@tuyishimekyrie tuyishimekyrie added Ready Use this when ready for merging and removed Needs peer reviews Use this when ready for peer reviews labels May 15, 2024
@SergeInGithub SergeInGithub added Needs Rebase and removed Ready Use this when ready for merging labels May 15, 2024
@tuyishimekyrie tuyishimekyrie added Ready Use this when ready for merging and removed Needs Rebase labels May 15, 2024
Copy link
Collaborator

@SergeInGithub SergeInGithub left a comment

Choose a reason for hiding this comment

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

Some reviews below. Also rebase @tuyishimekyrie

@@ -1,6 +1,6 @@
import sendGrid, { MailDataRequired } from '@sendgrid/mail';
import dotenv from 'dotenv';
import crypto from 'crypto';
// import crypto from 'crypto';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// import crypto from 'crypto';

@SergeInGithub SergeInGithub added Changes requested Needs Rebase and removed Ready Use this when ready for merging labels May 15, 2024
@tuyishimekyrie tuyishimekyrie added the Needs peer reviews Use this when ready for peer reviews label May 15, 2024
Copy link
Collaborator

@Sagastein Sagastein left a comment

Choose a reason for hiding this comment

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

Well Done

@tuyishimekyrie tuyishimekyrie added Ready Use this when ready for merging and removed Needs peer reviews Use this when ready for peer reviews labels May 16, 2024
Copy link
Collaborator

@SergeInGithub SergeInGithub left a comment

Choose a reason for hiding this comment

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

LGTM

@SergeInGithub SergeInGithub merged commit d227055 into develop May 16, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready Use this when ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants