Refactor token generation to use the 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 |
browniebroke changed the title secrets module 3 participants
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.