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

Improving The OTP functionality #423

Open
2 of 5 tasks
Harshdev098 opened this issue Nov 9, 2024 · 23 comments · May be fixed by #432
Open
2 of 5 tasks

Improving The OTP functionality #423

Harshdev098 opened this issue Nov 9, 2024 · 23 comments · May be fixed by #432
Assignees

Comments

@Harshdev098
Copy link
Owner

Issue Summary

Inhancing the OTP fucntionality implemented for forgot password

Issue Description

Currently the OTP is generated by random string, replace it by using an npm package otp-generator
Also implement feature for expiration of otp currently the otp code is saved but its should be erased after 2 minutes (changes should be done in frontend + database)

Proposed Solution (Optional)

No response

Priority

High - Requires urgent attention

Category

  • Enhancement
  • Refactor
  • Security
  • Design
  • Other

Additional Context (Optional)

No response

@Harshdev098 Harshdev098 added gssoc-ext This issue is under GSSoc Ext program level3 labe3 label for gssoc labels Nov 9, 2024
@Harshdev098 Harshdev098 removed gssoc-ext This issue is under GSSoc Ext program level3 labe3 label for gssoc labels Dec 29, 2024
@prajwal2431
Copy link
Contributor

i would like to contribute on this

@Harshdev098
Copy link
Owner Author

Sure, Will add labels on 1st Jan

@prajwal2431
Copy link
Contributor

is the otp sending feature added ? i have tried to reset the password but i doesnt got any email with code

@Harshdev098
Copy link
Owner Author

It was previously working but because nodemailer module is now deprecated it's not sending mails

@prajwal2431
Copy link
Contributor

Should I make that change? Should I use EmailJS as a third-party tool, or do you have any other recommendations?

@Harshdev098
Copy link
Owner Author

Yah sure you can do that, just create an issue regarding that I will assign you

@prajwal2431
Copy link
Contributor

ohk i will create the issue

@prajwal2431
Copy link
Contributor

can you add that swoc label to the issue and also to the issue i have created

@prajwal2431
Copy link
Contributor

i will start this issue when my previous pr is merged ok

@Harshdev098
Copy link
Owner Author

@prajwal2431 No worry will merge the PR,
You can start working on new issues and open the pr with different branches, also ensure that the previous changes should not be included in the new ones

@prajwal2431
Copy link
Contributor

Should an OTP contain capital letters, small letters, and special characters?

@Harshdev098
Copy link
Owner Author

You can do that too

@prajwal2431
Copy link
Contributor

yeah sure

@prajwal2431
Copy link
Contributor

Is it acceptable to work with the existing OTP section in the database, just by adding an expiration time? as i have added in docker so its changed from my side even though i have changes the branch

@Harshdev098
Copy link
Owner Author

Harshdev098 commented Jan 1, 2025

@prajwal2431 I didn't get your points, I think you should open a draft pr for this issue.

@prajwal2431
Copy link
Contributor

In my previous pr I have updated docker image for OTP column right so now even when I am on previous forked repo (without changes) still the docker image have that OTP column I was just saying that only

@Harshdev098
Copy link
Owner Author

Harshdev098 commented Jan 1, 2025

@prajwal2431 Oh, yes it's not acceptable that the new pr should consist the previous prs commit you can resolve it by moving to the default branch and pull the changes from the origin git pull origin main and built a new branch from there. If still the changes persist you can use rebase or cherry-pick for it

@prajwal2431
Copy link
Contributor

Even when done this docker image is still be the same so what to do

@prajwal2431
Copy link
Contributor

Actual codebase is not changed but docker image have the column of OTP still maybe it will not give the conflict. Conflict will be occur if the code base is same so it's ok maybe

@Harshdev098
Copy link
Owner Author

Can you make a draft pr with the current changes, I just want to see

@prajwal2431
Copy link
Contributor

Now will pull the main repo and will enhance OTP functionality

@Harshdev098
Copy link
Owner Author

Yes

@prajwal2431
Copy link
Contributor

There is endpoint issue right as youre calling resetpassword endpoint and there is reset function

Screenshot 2025-01-03 181720
Screenshot 2025-01-03 181730

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants