Skip to content

Commit c474b0c

Browse files
committed
fix(token): validate SA names
Service account names derived from usernames could silently fall back to a generic `"user"` name, leading to conflicts and unclear ownership. Additionally, there was a bug when passing a tierName that was always set as "" (PEBCAK, but probably refactoring/func extraction). Validation was introduced to ensure service account names are always valid and unique, with clear errors on invalid input. Token management logic was adjusted to correctly associate service accounts with their tier. Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com>
1 parent d7a4d66 commit c474b0c

File tree

1 file changed

+12
-9
lines changed

1 file changed

+12
-9
lines changed

maas-api/internal/token/manager.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (m *Manager) GenerateToken(ctx context.Context, user *UserContext, expirati
5858
return nil, fmt.Errorf("failed to ensure tier namespace for user %s: %w", userTier, err)
5959
}
6060

61-
saName, errSA := m.ensureServiceAccount(ctx, namespace, user.Username, "")
61+
saName, errSA := m.ensureServiceAccount(ctx, namespace, user.Username, userTier)
6262
if errSA != nil {
6363
log.Printf("Failed to ensure service account for user %s in namespace %s: %v", user.Username, namespace, err)
6464
return nil, fmt.Errorf("failed to ensure service account for user %s in namespace %s: %w", user.Username, namespace, err)
@@ -86,7 +86,10 @@ func (m *Manager) RevokeTokens(ctx context.Context, user *UserContext) error {
8686

8787
namespace := m.tierMapper.Namespaces(ctx)[userTier]
8888

89-
saName := m.sanitizeServiceAccountName(user.Username)
89+
saName, errName := m.sanitizeServiceAccountName(user.Username)
90+
if errName != nil {
91+
return fmt.Errorf("failed to sanitize service account name for user %s: %w", user.Username, err)
92+
}
9093

9194
_, err = m.serviceAccountLister.ServiceAccounts(namespace).Get(saName)
9295
if errors.IsNotFound(err) {
@@ -150,7 +153,10 @@ func (m *Manager) ensureTierNamespace(ctx context.Context, tier string) (string,
150153
// ensureServiceAccount creates a service account if it doesn't exist.
151154
// It takes a raw username, sanitizes it for Kubernetes naming, and returns the sanitized name.
152155
func (m *Manager) ensureServiceAccount(ctx context.Context, namespace, username, userTier string) (string, error) {
153-
saName := m.sanitizeServiceAccountName(username)
156+
saName, errName := m.sanitizeServiceAccountName(username)
157+
if errName != nil {
158+
return "", fmt.Errorf("failed to sanitize service account name for user %s: %w", username, errName)
159+
}
154160

155161
_, err := m.serviceAccountLister.ServiceAccounts(namespace).Get(saName)
156162
if err == nil {
@@ -166,9 +172,6 @@ func (m *Manager) ensureServiceAccount(ctx context.Context, namespace, username,
166172
Name: saName,
167173
Namespace: namespace,
168174
Labels: commonLabels(m.tenantName, userTier),
169-
Annotations: map[string]string{
170-
"maas.opendatahub.io/username": username,
171-
},
172175
},
173176
}
174177

@@ -221,7 +224,7 @@ func (m *Manager) deleteServiceAccount(ctx context.Context, namespace, saName st
221224
// sanitizeServiceAccountName ensures the service account name follows Kubernetes naming conventions.
222225
// While ideally usernames should be pre-validated, Kubernetes TokenReview can return usernames
223226
// in various formats (OIDC emails, LDAP DNs, etc.) that need sanitization for use as SA names.
224-
func (m *Manager) sanitizeServiceAccountName(username string) string {
227+
func (m *Manager) sanitizeServiceAccountName(username string) (string, error) {
225228
// Kubernetes ServiceAccount names must be valid DNS-1123 labels:
226229
// [a-z0-9-], 1-63 chars, start/end alphanumeric.
227230
name := strings.ToLower(username)
@@ -235,7 +238,7 @@ func (m *Manager) sanitizeServiceAccountName(username string) string {
235238
name = reDash.ReplaceAllString(name, "-")
236239
name = strings.Trim(name, "-")
237240
if name == "" {
238-
name = "user"
241+
return "", fmt.Errorf("invalid username %q", username)
239242
}
240243

241244
// Append a stable short hash to reduce collisions
@@ -250,7 +253,7 @@ func (m *Manager) sanitizeServiceAccountName(username string) string {
250253
name = strings.Trim(name, "-")
251254
}
252255

253-
return name + "-" + suffix
256+
return name + "-" + suffix, nil
254257
}
255258

256259
func commonLabels(name string, t string) map[string]string {

0 commit comments

Comments
 (0)