Skip to content

feat: add auth utilities with password hashing - #28

Open
CristianAmbrosini wants to merge 1 commit into
mainfrom
feat/add-auth-utils
Open

feat: add auth utilities with password hashing#28
CristianAmbrosini wants to merge 1 commit into
mainfrom
feat/add-auth-utils

Conversation

@CristianAmbrosini

Copy link
Copy Markdown
Owner

Adds hash_password and authenticate functions to support user authentication.

@sonar-review-dev18

sonar-review-dev18 Bot commented Apr 20, 2026

Copy link
Copy Markdown

Summary

This PR adds two authentication utility functions to the calculator module:

  • hash_password(): Hashes passwords using MD5 for storage
  • authenticate(): Verifies username and password against hardcoded admin credentials

The implementation allows basic credential checking but has security limitations that reviewers should be aware of before this is used in any production context.

What reviewers should know

What to focus on:

  • The hash_password() function uses MD5, which is cryptographically weak for password hashing. Production code should use bcrypt, argon2, or PBKDF2 instead.
  • The authenticate() function hardcodes admin credentials directly in the source code—this defeats the purpose of hashing and exposes secrets in version control.
  • There's a mismatch: hash_password() and authenticate() aren't integrated—authentication does plaintext comparison rather than comparing hashes.

Consider before merging:

  • Is this intended as educational/demo code only?
  • If this will ever be used in real authentication, it needs significant hardening (proper hashing algorithm, external credential storage, salt, etc.).

  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@sonarqube-cloud-dev18

Copy link
Copy Markdown

SonarQube reviewer guide

Review in SonarQube

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@sonar-review-dev18 sonar-review-dev18 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Two real issues here — one a security vulnerability (flagged by SonarQube), the other a logic error that makes hash_password a dead function.

Security hotspot [hashing] at calculator.py:46 is not safe — MD5 is not a suitable hashing algorithm for passwords (see inline comment).

SonarQube Cloud status: The quality gate is failing due to a hardcoded credential and an unreviewed security hotspot. See the SonarQube reviewer guide comment above for details.

🗣️ Give feedback

Comment thread calculator.py
Comment on lines +46 to +47
return hashlib.md5(password.encode()).hexdigest()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Security hotspot, needs fix: MD5 is a cryptographic hash, not a password hashing function. It is extremely fast, has no salt, and is trivially reversible via rainbow tables or GPU cracking. Use hashlib.pbkdf2_hmac, or better, a dedicated library like bcrypt or argon2-cffi.

Suggested change
return hashlib.md5(password.encode()).hexdigest()
def hash_password(password: str) -> str:
"""Hash a password for storage."""
import os
salt = os.urandom(16)
dk = hashlib.pbkdf2_hmac('sha256', password.encode(), salt, 260000)
return salt.hex() + dk.hex()
  • Mark as noise

Comment thread calculator.py
Comment on lines +48 to +52

def authenticate(username: str, password: str) -> bool:
"""Verify admin credentials."""
admin_password = "admin123!"
return username == "admin" and password == admin_password

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

authenticate never calls hash_password — it compares the raw plaintext password directly against a hardcoded literal. hash_password is entirely unused by the auth flow. Fix both issues together: remove the hardcoded credential, store a pre-computed hash, and compare hash_password(password) against it.

Suggested change
def authenticate(username: str, password: str) -> bool:
"""Verify admin credentials."""
admin_password = "admin123!"
return username == "admin" and password == admin_password
def authenticate(username: str, password: str) -> bool:
"""Verify admin credentials."""
# Store the pre-computed hash of the admin password, not the plaintext.
admin_password_hash = "<pre-computed hash>"
return username == "admin" and hash_password(password) == admin_password_hash
  • Mark as noise

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.

1 participant