fix: add unique api key prefix

[ontehfritz]

No description provided.

[Copilot]

Pull request overview

This pull request adds unique prefixes to API keys by incorporating a random 8-character string into each key's prefix. Previously, all API keys used a static "api_" prefix with a colon separator. Now, each key gets a unique prefix like "bwa_aBcDeFgH" where the random portion helps identify individual keys.

Changes:

• Modified API key generation to include an 8-character random string in the prefix (format: "bwa_randomStr")
• Changed API key format separator from colon to underscore (from "prefix:secret" to "prefix_secret")
• Added a new GenerateRandomString utility function for generating random alphanumeric strings
• Updated tests to reflect the new "bwa_" prefix format

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
internal/accounts/apikey/apikey.go	Modified Generate function to create unique prefixes using random strings; changed separator from ":" to "_"
internal/utils/random.go	New utility function to generate random alphanumeric strings of specified length
internal/utils/random_test.go	Tests for the random string generation function covering length, character validity, and uniqueness
tests/integration/accounts/account_apikey_integration_test.go	Updated test assertion to check for "bwa_" prefix instead of "api_:"
internal/accounts/apikey/mongodb_apikey_repository_test.go	Updated test data to use example prefix "bwa_aBcDeFgH" instead of "api_"

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Using math/rand instead of crypto/rand for generating API key prefixes is a security vulnerability. The math/rand package generates predictable pseudorandom numbers, which could allow attackers to predict future API key prefixes. For security-sensitive use cases like API key generation, you should use crypto/rand which provides cryptographically secure random number generation. The current implementation also uses a global seededRand variable initialized once at startup, making the random sequence even more predictable.

Suggested change

	"math/rand"
	"time"
	)
	const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
	var seededRand *rand.Rand = rand.New(rand.NewSource(time.Now().UnixNano()))
	// GenerateRandomString generates a random string of the specified length
	func GenerateRandomString(length int) string {
	b := make([]byte, length)
	for i := range b {
	b[i] = charset[seededRand.Intn(len(charset))]
	"crypto/rand"
	"math/big"
	)
	const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
	// GenerateRandomString generates a random string of the specified length
	func GenerateRandomString(length int) string {
	b := make([]byte, length)
	max := big.NewInt(int64(len(charset)))
	for i := range b {
	n, err := rand.Int(rand.Reader, max)
	if err != nil {
	// In the unlikely event of an error, fall back to a deterministic character
	b[i] = charset[0]
	continue
	}
	b[i] = charset[n.Int64()]

[Copilot]

The global variable seededRand is not safe for concurrent use. The rand.Rand type is not safe for concurrent access by multiple goroutines. In a web service environment where multiple API keys could be generated concurrently, this could lead to race conditions. Either use a mutex to protect access to seededRand, or use the top-level rand functions which use a global locked source.

Suggested change

	"time"
	)
	const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
	var seededRand *rand.Rand = rand.New(rand.NewSource(time.Now().UnixNano()))
	// GenerateRandomString generates a random string of the specified length
	func GenerateRandomString(length int) string {
	b := make([]byte, length)
	"sync"
	"time"
	)
	const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
	var (
	seededRand *rand.Rand = rand.New(rand.NewSource(time.Now().UnixNano()))
	seededRandMu sync.Mutex
	)
	// GenerateRandomString generates a random string of the specified length
	func GenerateRandomString(length int) string {
	b := make([]byte, length)
	seededRandMu.Lock()
	defer seededRandMu.Unlock()

[Copilot]

The API key format has changed from "prefix:secret" to "prefix_secret", where the prefix itself now contains an underscore. This results in API keys with the format "bwa_randomStr_uuid". This could be confusing because there are now two underscores in the final key, making it unclear how to parse the key if needed. Consider using a different separator for the prefix construction (e.g., keeping colon) or documenting the new format clearly. The change from colon to underscore should also be intentional and documented.

[Copilot]

The test uses a hard-coded example prefix "bwa_aBcDeFgH" which doesn't match the actual format that would be generated. The GenerateRandomString function only generates alphanumeric characters (both upper and lower case), so "aBcDeFgH" is a valid example, but using a realistic example that matches the actual output would make the test more representative. Additionally, consider using a constant or function to generate test API keys consistently.

[Copilot]

The uniqueness test in GenerateRandomString only runs 100 iterations, which is insufficient to meaningfully test for uniqueness with an 8-character alphanumeric string. With 62 possible characters (a-z, A-Z, 0-9) and 8 positions, there are 62^8 possible combinations. While testing all combinations is impractical, 100 iterations is too small to provide confidence. By the birthday paradox, with only 100 samples from such a large space, collisions are extremely unlikely regardless of the quality of the RNG. This test would only catch a severely broken implementation. Consider increasing the iterations to at least 10,000 or testing with shorter strings where collisions are more likely.