Refactor token generation to use secrets module #9760
Conversation
This change was already confirmed as beneficial by @browniebroke in the previous PR: — I just moved it here as suggested. If needed, I can also add tests for this part. |
I like the intent as well. Not sure how you're thinking of testing that? I can only think of something that would test the implementation detail, which I'm not very interested in... Open to be proven wrong though! |
For example, these tests might help verify the change: def test_generate_key_returns_valid_format(self):
"""Ensure generate_key returns a valid token format"""
key = self.model.generate_key()
# Should be exactly 40 characters (to fit in CharField max_length=40)
assert len(key) == 40
# Should contain only valid hexadecimal characters
assert all(c in '0123456789abcdef' for c in key)
def test_generate_key_produces_unique_values(self):
"""Ensure generate_key produces unique values across multiple calls"""
keys = set()
# Generate multiple keys and ensure they're all different
for _ in range(100):
key = self.model.generate_key()
assert key not in keys, f"Duplicate key generated: {key}"
keys.add(key) |
|
some tests would be nice to have ... |
Sure, I’ll add these tests then. |
- Add test for valid token format (40 hex characters) - Add collision resistance test with 500 sample size - Add basic randomness quality validation - Ensure generated keys are unique and properly formatted
|
I’ve added the tests. |
PS No need to @ me, I'm checking my notifications regularly |
Summary
Replace
binascii.hexlify(os.urandom(20)).decode()withsecrets.token_hex(20)inToken.generate_key()method.Rationale
The
secretsmodule is the recommended approach for cryptographic token generation in Python 3.6+. This change maintains the same 40-character hex output format and passes all existing tests.