refactor(api): Replaced OTP code from alphanumeric to numeric

[Ritika1705]

User description

Description

Replaced RandomUUid with Math.Random

Fixes #211

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

Developer's checklist

• My PR follows the style guidelines of this project
• I have performed a self-check on my work

If changes are made in the code:

• I have followed the coding guidelines
• My changes in code generate no new warnings
• My changes are breaking another fix/feature of the project
• I have added test cases to show that my feature works
• I have added relevant screenshots in my PR
• There are no UI/UX issues

Documentation Update

• This PR requires an update to the documentation at docs.keyshade.xyz
• I have made the necessary updates to the documentation, or no documentation changes are required.

---

PR Type

enhancement

---

Description

• Updated OTP generation method in auth.service.ts to generate numeric codes using Math.random() instead of alphanumeric codes with randomUUID().
• This change affects both the creation and updating of OTP codes.

---

Changes walkthrough 📝

	Relevant files
Enhancement	auth.service.ts Change OTP Generation to Use Numeric Codes                              apps/api/src/auth/service/auth.service.ts Changed OTP generation from using randomUUID() to Math.random(). OTP codes are now purely numeric instead of alphanumeric. +2/-2

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-free]

PR Description updated to latest commit (2eefd19)

[rajdip-b]

Hey @Ritika1705, thanks for making this PR! I will check this ASAP and try to merge it.

[codiumai-pr-agent-free]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	2, because the PR involves a straightforward change in the OTP generation logic, replacing alphanumeric codes with numeric codes. The change is localized to a single file and method, making it relatively easy to review.
🧪 Relevant tests	No
⚡ Possible issues	Predictability of OTP: The use of Math.random() for OTP generation may not provide sufficient randomness and security for OTP codes, which are critical for authentication purposes.
🔒 Security concerns	No

[codiumai-pr-agent-free]

PR Code Suggestions ✨

Category	Suggestion	Score
Security	Improve the security of OTP generation Replace Math.random().toString().substring(2, 8) with a more secure method of generating OTP codes. Using Math.random() for OTP can be predictable and insecure. Consider using a cryptographic library to generate a secure random number. apps/api/src/auth/service/auth.service.ts [48] -code: Math.random().toString().substring(2, 8), +code: crypto.randomBytes(3).toString('hex'), Suggestion importance[1-10]: 10 Why: The suggestion correctly identifies a significant security flaw in the use of Math.random() for OTP generation and proposes a more secure alternative using cryptographic methods.	10
Maintainability	Refactor OTP code generation to a separate method to reduce duplication Refactor the repeated code for generating the OTP code into a separate method. This will improve maintainability and reduce code duplication. apps/api/src/auth/service/auth.service.ts [48-52] -code: Math.random().toString().substring(2, 8), +code: this.generateOtpCode(), Suggestion importance[1-10]: 8 Why: The suggestion to refactor repeated code into a separate method is valid and would enhance maintainability and readability of the code.	8
Possible bug	Ensure consistent length for the OTP code Ensure that the generated OTP code has a consistent length. The current implementation using Math.random().toString().substring(2, 8) might not always return a string of length 6, as the number could start with zeros. apps/api/src/auth/service/auth.service.ts [48] -code: Math.random().toString().substring(2, 8), +code: Math.random().toString().slice(-6).padStart(6, '0'), Suggestion importance[1-10]: 7 Why: This suggestion addresses a potential bug where the OTP code might not always be of consistent length, which is crucial for OTP functionality.	7
Best practice	Add error handling for OTP generation Consider adding error handling for the OTP generation process. This is important to ensure that the service can gracefully handle any failures during the OTP generation. apps/api/src/auth/service/auth.service.ts [48] -code: Math.random().toString().substring(2, 8), +try { + code: Math.random().toString().substring(2, 8), +} catch (error) { + // handle error appropriately +} Suggestion importance[1-10]: 5 Why: While adding error handling is generally a good practice, the specific method of OTP generation used (Math.random().toString().substring) does not typically throw errors that need to be caught, making this suggestion less relevant.	5

[kriptonian1]

Hey @Ritika1705 and @rajdip-b using Math.random is not the most secure way to generate opt you can read this amazing discussion in stackexchange crypto channel to know more on why link. I will rather recommend to use TOTP for that you can use otplib. Now, using math.random is not secure because it's a pseudo random number generator, which is not cryptographically secure.

[rajdip-b]

@Ritika1705 hey! did you encounter any issue in the implementation? We are eager to merge your PR!

[Ritika1705]

Hi, Got caught up with some work. Will try to finish it by eod. Thankyou

…

On Mon, 20 May 2024 at 11:26 AM, Rajdip Bhattacharya < ***@***.***> wrote: @Ritika1705 <https://github.com/Ritika1705> hey! did you encounter any issue in the implementation? We are eager to merge your PR! — Reply to this email directly, view it on GitHub <#230 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANAPAFWD2HHJCB2ZI5M5EV3ZDGGCJAVCNFSM6AAAAABH6FJGQCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJZG4ZDEMBZGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[rajdip-b]

Oh! Alright no issues! Whenever you are free.

[Ritika1705]

For better secure code generation, we can follow this article: https://medium.com/getpowerplay/a-one-time-password-otp-generator-npm-library-based-on-nanoid-79821a173798

[rajdip-b]

Hey @Ritika1705, I went through some articles, and I feel that we can use the already existing crypto library. Maybe you can try out this suggestion?

crypto.randomBytes(3).toString('hex'),

I'm not sure how it will perform, but let's keep things simple - and use crypto package.

[Ritika1705]

Hey @Ritika1705 , I went through some articles, and I feel that we can use the already existing crypto library. Maybe you can try out this suggestion?

crypto.randomBytes(3).toString('hex'),

I'm not sure how it will perform, but let's keep things simple - and use crypto package.

Hi @rajdip-b , I have upadated the code to use crypto package.

The code BigInt(\0x${crypto.randomUUID().replace(/-/g, '')}).toString().substring(0, 6) generates a 6-character string by following these steps:

• Generate a UUID: crypto.randomUUID() generates a version 4 UUID, which looks something like 550e8400-e29b-41d4-a716-446655440000.
• Remove Hyphens: .replace(/-/g, '') removes all the hyphens from the UUID, resulting in a string like 550e8400e29b41d4a716446655440000.
• Convert to BigInt: BigInt(\0x${uuid})converts this hexadecimal string to a BigInt. The0x prefix indicates that the string is in hexadecimal format.
• Convert to String: .toString() converts the BigInt to its decimal string representation.
• Extract 6 Digits: .substring(0, 6) takes the first 6 characters from the decimal string

[rajdip-b]

LGTM!

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[rajdip-b]

🎉 This PR is included in version 1.4.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀