Skip to content

Commit f65236a

Browse files
tiwilliaclaude
andcommitted
Simplify HTPasswd identity provider to support only users array input
Remove complex multi-option input handling for HTPasswd IDP setup in favor of single users array format for better consistency and maintainability. Changes: - Remove username/password parameter support - Remove htpasswd_file_content parameter support - Make users array parameter required - Simplify ProcessUserInput() function signature - Remove parseHtpasswordFile() function - Always hash passwords (no pre-hashed support) - Update tests to cover only users array scenarios - Update implementation plan documentation Benefits: - Clearer API with single input method - Reduced code complexity and test surface - Better alignment with ROSA CLI patterns - Improved maintainability 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 70de753 commit f65236a

File tree

7 files changed

+143
-198
lines changed

7 files changed

+143
-198
lines changed

CLAUDE.md

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,19 +103,17 @@ The HTPasswd identity provider implementation (`setup_htpasswd_identity_provider
103103
- `UsernameValidator()` - Username format validation (no /, :, % characters)
104104
- `clusterAdminValidator()` - Reserved username check (prevents "cluster-admin")
105105
- `ValidateIdpName()` - IDP name validation with regex pattern matching
106-
- `parseHtpasswordFile()` - HTPasswd file parsing for base64 encoded content
107-
- `ProcessUserInput()` - Handles multiple input formats (users array, single user, htpasswd file)
106+
- `ProcessUserInput()` - Simplified to handle only users array format
108107

109108
**Validation Flow:**
110109
1. IDP name validation using ROSA CLI regex patterns
111110
2. Username/password validation with ROSA CLI rules
112-
3. Password hashing with `ocm-common` utilities
111+
3. Password hashing with `ocm-common` utilities (always hash passwords)
113112
4. OCM API integration following ROSA CLI error handling patterns
114113

115-
**Input Compatibility:**
116-
- Users array format: `["user1:password1", "user2:password2"]`
117-
- Single user format: `username` + `password` parameters (backward compatibility)
118-
- HTPasswd file format: Base64-encoded htpasswd file content
114+
**Input Format (Simplified):**
115+
- Users array format: `["user1:password1", "user2:password2"]` (required parameter)
116+
- Removed backward compatibility for single user and htpasswd file formats for better consistency
119117

120118
## Development Notes
121119

README.md

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -221,19 +221,8 @@ Setup an HTPasswd identity provider for a ROSA HCP cluster with username/passwor
221221
},
222222
"users": {
223223
"type": "array",
224-
"description": "List of username:password pairs [\"user1:password1\", \"user2:password2\"]"
225-
},
226-
"username": {
227-
"type": "string",
228-
"description": "Single user username (for backward compatibility)"
229-
},
230-
"password": {
231-
"type": "string",
232-
"description": "Single user password (for backward compatibility)"
233-
},
234-
"htpasswd_file_content": {
235-
"type": "string",
236-
"description": "Base64-encoded htpasswd file content"
224+
"description": "List of username:password pairs [\"user1:password1\", \"user2:password2\"]",
225+
"required": true
237226
},
238227
"overwrite_existing": {
239228
"type": "boolean",

pkg/htpasswd/validation.go

Lines changed: 16 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package htpasswd
22

33
import (
4-
"encoding/base64"
54
"fmt"
65
"regexp"
76
"strings"
@@ -70,73 +69,29 @@ func validateHtUsernameAndPassword(username, password string) error {
7069
return nil
7170
}
7271

73-
// parseHtpasswordFile - copied from rosa/cmd/create/idp/htpasswd.go:281
74-
func parseHtpasswordFile(usersList *map[string]string, fileContent string) error {
75-
lines := strings.Split(fileContent, "\n")
76-
for _, line := range lines {
77-
line = strings.TrimSpace(line)
78-
if line == "" {
79-
continue
80-
}
81-
82-
// split "user:password" at colon
83-
username, password, found := strings.Cut(line, ":")
84-
if !found || username == "" || password == "" {
85-
return fmt.Errorf("Malformed line, Expected: validUsername:validPassword, Got: %s", line)
86-
}
87-
88-
(*usersList)[username] = password
89-
}
90-
return nil
91-
}
92-
93-
// ProcessUserInput - processes MCP tool parameters using ROSA CLI patterns
94-
func ProcessUserInput(userInput map[string]interface{}) (map[string]string, bool, error) {
72+
// ProcessUserInput - processes MCP tool parameters using ROSA CLI patterns (simplified to users array only)
73+
func ProcessUserInput(userInput map[string]interface{}) (map[string]string, error) {
9574
userList := make(map[string]string)
96-
isHashedPassword := false
97-
98-
// Option 1: users array (comma-separated list like ROSA CLI)
99-
if users, ok := userInput["users"].([]interface{}); ok && len(users) > 0 {
100-
for _, user := range users {
101-
userStr, ok := user.(string)
102-
if !ok {
103-
return nil, false, fmt.Errorf("invalid user format, expected string")
104-
}
105-
username, password, found := strings.Cut(userStr, ":")
106-
if !found {
107-
return nil, false, fmt.Errorf("users should be provided in format username:password")
108-
}
109-
userList[username] = password
110-
}
111-
return userList, false, nil
112-
}
11375

114-
// Option 2: single username/password (ROSA CLI backward compatibility)
115-
if username, hasUsername := userInput["username"].(string); hasUsername {
116-
if password, hasPassword := userInput["password"].(string); hasPassword {
117-
userList[username] = password
118-
return userList, false, nil
119-
}
120-
return nil, false, fmt.Errorf("password required when username is provided")
76+
// Only support users array format
77+
users, ok := userInput["users"].([]interface{})
78+
if !ok || len(users) == 0 {
79+
return nil, fmt.Errorf("'users' parameter is required: provide list of username:password pairs")
12180
}
12281

123-
// Option 3: htpasswd file content
124-
if fileContent, ok := userInput["htpasswd_file_content"].(string); ok && fileContent != "" {
125-
// Decode base64 content
126-
decoded, err := base64.StdEncoding.DecodeString(fileContent)
127-
if err != nil {
128-
return nil, false, fmt.Errorf("failed to decode htpasswd file content: %w", err)
82+
for _, user := range users {
83+
userStr, ok := user.(string)
84+
if !ok {
85+
return nil, fmt.Errorf("invalid user format, expected string")
12986
}
130-
131-
// Use ROSA CLI parsing function
132-
if err := parseHtpasswordFile(&userList, string(decoded)); err != nil {
133-
return nil, false, fmt.Errorf("failed to parse htpasswd file: %w", err)
87+
username, password, found := strings.Cut(userStr, ":")
88+
if !found || username == "" || password == "" {
89+
return nil, fmt.Errorf("users should be provided in format username:password")
13490
}
135-
isHashedPassword = true // htpasswd files contain pre-hashed passwords
136-
return userList, isHashedPassword, nil
91+
userList[username] = password
13792
}
138-
139-
return nil, false, fmt.Errorf("no user input provided: specify 'users', 'username'+'password', or 'htpasswd_file_content'")
93+
94+
return userList, nil
14095
}
14196

14297
// ValidateUserCredentials - validates username and password using ROSA CLI validation

pkg/htpasswd/validation_test.go

Lines changed: 14 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package htpasswd
22

33
import (
4-
"encoding/base64"
54
"testing"
65

76
"github.com/stretchr/testify/assert"
@@ -120,77 +119,10 @@ func TestValidateUserCredentials(t *testing.T) {
120119
}
121120
}
122121

123-
func TestParseHtpasswordFile(t *testing.T) {
124-
testCases := []struct {
125-
content string
126-
expected map[string]string
127-
expectError bool
128-
description string
129-
}{
130-
{
131-
"user1:password1\nuser2:password2",
132-
map[string]string{"user1": "password1", "user2": "password2"},
133-
false,
134-
"valid htpasswd content",
135-
},
136-
{
137-
"user1:password1\n\nuser2:password2\n",
138-
map[string]string{"user1": "password1", "user2": "password2"},
139-
false,
140-
"htpasswd content with empty lines",
141-
},
142-
{
143-
"user1:$apr1$hRY7OJWH$km1EYH.UIRjp6CzfZQz/g1",
144-
map[string]string{"user1": "$apr1$hRY7OJWH$km1EYH.UIRjp6CzfZQz/g1"},
145-
false,
146-
"htpasswd with hashed password",
147-
},
148-
{
149-
"invalid_line_without_colon",
150-
nil,
151-
true,
152-
"malformed line without colon",
153-
},
154-
{
155-
"user1:\nuser2:password2",
156-
nil,
157-
true,
158-
"empty password",
159-
},
160-
{
161-
":password1",
162-
nil,
163-
true,
164-
"empty username",
165-
},
166-
{
167-
"",
168-
map[string]string{},
169-
false,
170-
"empty content",
171-
},
172-
}
173-
174-
for _, tc := range testCases {
175-
t.Run(tc.description, func(t *testing.T) {
176-
usersList := make(map[string]string)
177-
err := parseHtpasswordFile(&usersList, tc.content)
178-
179-
if tc.expectError {
180-
assert.Error(t, err, "Expected error for content: %s", tc.content)
181-
} else {
182-
assert.NoError(t, err, "Expected no error for content: %s", tc.content)
183-
assert.Equal(t, tc.expected, usersList, "Expected parsed users to match")
184-
}
185-
})
186-
}
187-
}
188-
189122
func TestProcessUserInput(t *testing.T) {
190123
testCases := []struct {
191124
input map[string]interface{}
192125
expectedUsers map[string]string
193-
expectedHashed bool
194126
expectError bool
195127
description string
196128
}{
@@ -200,75 +132,57 @@ func TestProcessUserInput(t *testing.T) {
200132
},
201133
map[string]string{"user1": "password1", "user2": "password2"},
202134
false,
203-
false,
204135
"users array input",
205136
},
206137
{
207138
map[string]interface{}{
208-
"username": "testuser",
209-
"password": "testpassword",
210-
},
211-
map[string]string{"testuser": "testpassword"},
212-
false,
213-
false,
214-
"single username/password input",
215-
},
216-
{
217-
map[string]interface{}{
218-
"htpasswd_file_content": base64.StdEncoding.EncodeToString([]byte("user1:$apr1$hash1\nuser2:$apr1$hash2")),
139+
"users": []interface{}{"invalid_user_without_colon"},
219140
},
220-
map[string]string{"user1": "$apr1$hash1", "user2": "$apr1$hash2"},
141+
nil,
221142
true,
222-
false,
223-
"htpasswd file content input",
143+
"invalid users array format",
224144
},
225145
{
226-
map[string]interface{}{
227-
"username": "testuser",
228-
// missing password
229-
},
146+
map[string]interface{}{},
230147
nil,
231-
false,
232148
true,
233-
"username without password",
149+
"no users parameter provided",
234150
},
235151
{
236152
map[string]interface{}{
237-
"users": []interface{}{"invalid_user_without_colon"},
153+
"users": []interface{}{},
238154
},
239155
nil,
240-
false,
241156
true,
242-
"invalid users array format",
157+
"empty users array",
243158
},
244159
{
245160
map[string]interface{}{
246-
"htpasswd_file_content": "invalid_base64",
161+
"users": []interface{}{123},
247162
},
248163
nil,
249-
false,
250164
true,
251-
"invalid base64 htpasswd content",
165+
"invalid user type in array",
252166
},
253167
{
254-
map[string]interface{}{},
168+
map[string]interface{}{
169+
"users": []interface{}{"user1:password1", "user2:"},
170+
},
255171
nil,
256-
false,
257172
true,
258-
"no user input provided",
173+
"user with empty password",
259174
},
260175
}
261176

262177
for _, tc := range testCases {
263178
t.Run(tc.description, func(t *testing.T) {
264-
users, isHashed, err := ProcessUserInput(tc.input)
179+
users, err := ProcessUserInput(tc.input)
265180

266181
if tc.expectError {
267182
assert.Error(t, err, "Expected error for input: %v", tc.input)
268183
} else {
269184
assert.NoError(t, err, "Expected no error for input: %v", tc.input)
270185
assert.Equal(t, tc.expectedUsers, users, "Expected users to match")
271-
assert.Equal(t, tc.expectedHashed, isHashed, "Expected hashed flag to match")
272186
}
273187
})
274188
}

pkg/mcp/tools.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,7 @@ HTPasswd is a common identity provider for development and testing environments.
7373
mcp.WithString("cluster_id", mcp.Description("Target cluster identifier"), mcp.Required()),
7474
mcp.WithString("name", mcp.Description("Identity provider name"), mcp.DefaultString("htpasswd")),
7575
mcp.WithString("mapping_method", mcp.Description("User mapping method - options: add, claim, generate, lookup"), mcp.DefaultString("claim")),
76-
mcp.WithArray("users", mcp.Description("List of username:password pairs [\"user1:password1\", \"user2:password2\"]")),
77-
mcp.WithString("username", mcp.Description("Single user username (for backward compatibility)")),
78-
mcp.WithString("password", mcp.Description("Single user password (for backward compatibility)")),
79-
mcp.WithString("htpasswd_file_content", mcp.Description("Base64-encoded htpasswd file content")),
76+
mcp.WithArray("users", mcp.Description("List of username:password pairs [\"user1:password1\", \"user2:password2\"]"), mcp.Required()),
8077
mcp.WithBoolean("overwrite_existing", mcp.Description("Whether to overwrite if IDP with same name exists"), mcp.DefaultBool(false)),
8178
mcp.WithReadOnlyHintAnnotation(false),
8279
mcp.WithDestructiveHintAnnotation(false),

pkg/ocm/htpasswd.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -69,31 +69,29 @@ func (c *Client) SetupHTPasswdIdentityProvider(
6969
}
7070
}
7171

72-
// Step 4: Process user input using ROSA CLI patterns
73-
userList, isHashedPassword, err := htpasswd.ProcessUserInput(userInput)
72+
// Step 4: Process user input using simplified validation
73+
userList, err := htpasswd.ProcessUserInput(userInput)
7474
if err != nil {
7575
return nil, fmt.Errorf("failed to process user input: %w", err)
7676
}
7777

78-
// Step 5: Build HTPasswd user list using ROSA CLI pattern
78+
// Step 5: Build HTPasswd user list (always hash passwords)
7979
htpasswdUsers := []*cmv1.HTPasswdUserBuilder{}
8080
for username, password := range userList {
8181
// Validate each user using ROSA CLI validation
8282
if err := htpasswd.ValidateUserCredentials(username, password); err != nil {
8383
return nil, fmt.Errorf("invalid user credentials for '%s': %w", username, err)
8484
}
8585

86-
userBuilder := cmv1.NewHTPasswdUser().Username(username)
87-
if isHashedPassword {
88-
userBuilder.HashedPassword(password)
89-
} else {
90-
// Use ROSA CLI password hashing
91-
hashedPwd, err := idputils.GenerateHTPasswdCompatibleHash(password)
92-
if err != nil {
93-
return nil, fmt.Errorf("failed to hash password for user '%s': %w", username, err)
94-
}
95-
userBuilder.HashedPassword(hashedPwd)
86+
// Always hash passwords using ROSA CLI method
87+
hashedPwd, err := idputils.GenerateHTPasswdCompatibleHash(password)
88+
if err != nil {
89+
return nil, fmt.Errorf("failed to hash password for user '%s': %w", username, err)
9690
}
91+
92+
userBuilder := cmv1.NewHTPasswdUser().
93+
Username(username).
94+
HashedPassword(hashedPwd)
9795
htpasswdUsers = append(htpasswdUsers, userBuilder)
9896
}
9997

0 commit comments

Comments
 (0)