Skip to content

Commit 04e0fa1

Browse files
committed
Strengthen generateRandomString
Replace hex encoding with an alphanumeric charset (a-z, A-Z, 0-9), enforce a minimum length of 16 characters, and guarantee that every generated password contains at least one lowercase letter, one uppercase letter, and one digit. Update tests to cover the minimum-length error path and the new character-class requirements.
1 parent db6df22 commit 04e0fa1

2 files changed

Lines changed: 68 additions & 44 deletions

File tree

internal/controller/common.go

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ import (
2020
"context"
2121
"crypto/rand"
2222
_ "embed"
23-
"encoding/hex"
2423
"encoding/json"
2524
"errors"
2625
"fmt"
26+
"math/big"
2727
"strings"
2828

2929
common_helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper"
@@ -187,16 +187,45 @@ func getDeployment(ctx context.Context, h *common_helper.Helper, name string, na
187187
return deployment, nil
188188
}
189189

190-
// generateRandomString generates a random hex string of the given length.
190+
// generateRandomString generates a random alphanumeric string of the given length containing at
191+
// least one lowercase letter, one uppercase letter, and one digit. Minimum secureLength is 16.
191192
func generateRandomString(secretLength int) (string, error) {
192-
// Ceiling division: hex.EncodeToString doubles the byte count, so we need ceil(secretLength/2) bytes.
193-
randDataLen := (secretLength + 1) / 2
193+
const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
194+
const charsetLen = len(charset)
195+
const minLength = 16
196+
const maxIterations = 1000
194197

195-
b := make([]byte, randDataLen)
196-
if _, err := rand.Read(b); err != nil {
197-
return "", fmt.Errorf("failed to generate secret: %w", err)
198+
if secretLength < minLength {
199+
return "", fmt.Errorf("secret length must be at least %d", minLength)
198200
}
199201

200-
randString := hex.EncodeToString(b)
201-
return randString[:secretLength], nil
202+
for range maxIterations {
203+
204+
randPassword := make([]byte, secretLength)
205+
var hasLowerCase, hasUpperCase, hasNumber bool
206+
207+
for passwdCharIdx := range secretLength {
208+
idx, err := rand.Int(rand.Reader, big.NewInt(int64(charsetLen)))
209+
if err != nil {
210+
return "", fmt.Errorf("failed to generate secret: %w", err)
211+
}
212+
213+
passwdChar := charset[idx.Int64()]
214+
if passwdChar >= 'a' && passwdChar <= 'z' {
215+
hasLowerCase = true
216+
} else if passwdChar >= 'A' && passwdChar <= 'Z' {
217+
hasUpperCase = true
218+
} else if passwdChar >= '0' && passwdChar <= '9' {
219+
hasNumber = true
220+
}
221+
222+
randPassword[passwdCharIdx] = passwdChar
223+
}
224+
225+
if hasLowerCase && hasUpperCase && hasNumber {
226+
return string(randPassword), nil
227+
}
228+
}
229+
230+
return "", fmt.Errorf("failed to generate secret: exceeded iterations - %d", maxIterations)
202231
}

internal/controller/common_test.go

Lines changed: 30 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,42 +21,19 @@ import (
2121
)
2222

2323
func TestGenerateRandomStringLength(t *testing.T) {
24+
t.Run("Below minimum length returns error", func(t *testing.T) {
25+
_, err := generateRandomString(15)
26+
if err == nil {
27+
t.Error("generateRandomString(15) expected error, got nil")
28+
}
29+
})
30+
2431
tests := []struct {
2532
name string
2633
length int
2734
}{
28-
{
29-
name: "Zero length",
30-
length: 0,
31-
},
32-
{
33-
name: "Odd length 1",
34-
length: 1,
35-
},
36-
{
37-
name: "Even length 2",
38-
length: 2,
39-
},
40-
{
41-
name: "Odd length 7",
42-
length: 7,
43-
},
44-
{
45-
name: "Even length 8",
46-
length: 8,
47-
},
48-
{
49-
name: "Odd length 15",
50-
length: 15,
51-
},
52-
{
53-
name: "Even length 16",
54-
length: 16,
55-
},
56-
{
57-
name: "Even length 32",
58-
length: 32,
59-
},
35+
{name: "Min length 16", length: 16},
36+
{name: "Even length 32", length: 32},
6037
}
6138

6239
for _, tt := range tests {
@@ -72,16 +49,34 @@ func TestGenerateRandomStringLength(t *testing.T) {
7249
}
7350
}
7451

75-
func TestGenerateRandomStringHexCharacters(t *testing.T) {
52+
func TestGenerateRandomStringCharacters(t *testing.T) {
7653
result, err := generateRandomString(32)
7754
if err != nil {
7855
t.Fatalf("generateRandomString(32) unexpected error: %v", err)
7956
}
57+
58+
var hasLower, hasUpper, hasDigit bool
8059
for i, c := range result {
81-
if (c < '0' || c > '9') && (c < 'a' || c > 'f') {
82-
t.Errorf("generateRandomString(32) character at index %d is %q, not a lowercase hex character", i, c)
60+
switch {
61+
case c >= 'a' && c <= 'z':
62+
hasLower = true
63+
case c >= 'A' && c <= 'Z':
64+
hasUpper = true
65+
case c >= '0' && c <= '9':
66+
hasDigit = true
67+
default:
68+
t.Errorf("generateRandomString(32) character at index %d is %q, not alphanumeric", i, c)
8369
}
8470
}
71+
if !hasLower {
72+
t.Error("generateRandomString(32) result has no lowercase letter")
73+
}
74+
if !hasUpper {
75+
t.Error("generateRandomString(32) result has no uppercase letter")
76+
}
77+
if !hasDigit {
78+
t.Error("generateRandomString(32) result has no digit")
79+
}
8580
}
8681

8782
func TestGenerateRandomStringUniqueness(t *testing.T) {

0 commit comments

Comments
 (0)