-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add game pin generation functionality #12
Conversation
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.
Hey @mmpotulo28 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
function generatePIN() { | ||
let digits = Math.floor(1000 + Math.random() * 9000); | ||
|
||
let letters = ''; | ||
const alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; | ||
for (let i = 0; i < 3; i++) { | ||
letters += alphabet.charAt(Math.floor(Math.random() * alphabet.length)); | ||
} | ||
|
||
let pin = digits + letters; | ||
|
||
document.getElementById('pinDisplay').value = pin; | ||
sessionStorage.setItem('gamePin', pin); | ||
} |
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.
🚨 suggestion (security): Consider using a more secure random generator for PINs.
The current method using Math.random
is not cryptographically secure, which might be a concern for the application's security requirements.
} | ||
|
||
window.onload = function () { | ||
let storedPin = sessionStorage.getItem('gamePIN'); |
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.
issue (bug_risk): Mismatch in case sensitivity for sessionStorage key.
The key 'gamePIN' used in getItem
does not match the case of 'gamePin' used in setItem
, which could lead to unexpected behavior.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Deploying devpost-hackathon with Cloudflare Pages
|
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.
@samkeleN address the changes suggested by the AI
issue (bug_risk): Mismatch in case sensitivity for sessionStorage key.
The key 'gamePIN' used in getItem does not match the case of 'gamePin' used in setItem, which could lead to unexpected behavior.
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.
Utilize the window.crypto.getRandomValues() method to generate cryptographically secure random bytes if necessary.
reopen this PR after you made the requested changes |
No description provided.