diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 69911e1..21fca49 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -46,6 +46,12 @@ func LoadPolicy() error { return err } + // Validate username uniqueness — reject configs where two pubkeys + // share the same username (ambiguous audit logs). + if err := rbac.ValidateUsernameUniqueness(); err != nil { + return err + } + var err error roleStore, err = rbac.LoadRolesFromConfig() if err != nil { diff --git a/internal/auth/jety_test.go b/internal/auth/jety_test.go index fd8068b..c9eadff 100644 --- a/internal/auth/jety_test.go +++ b/internal/auth/jety_test.go @@ -3,6 +3,7 @@ package auth import ( "os" "path/filepath" + "strings" "testing" "github.com/nats-io/nkeys" @@ -590,6 +591,49 @@ func TestLoadPolicyWithCohorts(t *testing.T) { } } +func TestLoadPolicyRejectsDuplicateUsernames(t *testing.T) { + setupJetyForTest(t) + defer clearJetyKeys(t) + defer SetPolicy(nil, nil, nil) + + jety.Set("users", map[string]interface{}{ + "admin": []interface{}{ + map[string]interface{}{"pubkey": "KEY_A", "username": "alice"}, + }, + "viewer": []interface{}{ + map[string]interface{}{"pubkey": "KEY_B", "username": "alice"}, + }, + }) + + err := LoadPolicy() + if err == nil { + t.Fatal("expected LoadPolicy to reject config with duplicate usernames") + } + if !strings.Contains(err.Error(), "duplicate username") { + t.Errorf("error should mention duplicate username, got: %v", err) + } +} + +func TestLoadPolicyAcceptsUniqueUsernames(t *testing.T) { + setupJetyForTest(t) + defer clearJetyKeys(t) + defer SetPolicy(nil, nil, nil) + + jety.Set("users", map[string]interface{}{ + "admin": []interface{}{ + map[string]interface{}{"pubkey": "KEY_A", "username": "alice"}, + }, + "viewer": []interface{}{ + map[string]interface{}{"pubkey": "KEY_B", "username": "bob"}, + }, + }) + + err := LoadPolicy() + if err != nil { + t.Fatalf("LoadPolicy should accept unique usernames, got: %v", err) + } +} + // --- DangerouslyAllowRoot bypass with jety --- func TestDangerouslyAllowRootEnabled(t *testing.T) { diff --git a/internal/rbac/config.go b/internal/rbac/config.go index c3e2893..cfe780c 100644 --- a/internal/rbac/config.go +++ b/internal/rbac/config.go @@ -13,6 +13,10 @@ import ( // multiple roles in the users/pubkeys config sections. var ErrDuplicatePubkey = errors.New("duplicate pubkey assignment") +// ErrDuplicateUsername is returned when two or more pubkeys share the +// same username in the users config section. +var ErrDuplicateUsername = errors.New("duplicate username") + // RoleStore holds all custom role definitions loaded from config. type RoleStore struct { roles map[string]*Role @@ -385,6 +389,50 @@ func ValidateUserUniqueness() error { ErrDuplicatePubkey, strings.Join(dupes, "\n")) } +// ValidateUsernameUniqueness checks the "users" config section for +// entries where two or more pubkeys share the same username. Duplicate +// usernames would make audit logs ambiguous and prevent reliable +// identification. Returns an error listing every conflicting username and +// the pubkeys that claim it. Empty/missing usernames are ignored. +func ValidateUsernameUniqueness() error { + seen := make(map[string][]string) // username → list of pubkeys + + usersMap := jety.GetStringMap("users") + for _, v := range usersMap { + entries := parseUserEntries(v) + for _, e := range entries { + if e.Username == "" { + continue + } + seen[e.Username] = append(seen[e.Username], e.Pubkey) + } + } + + var dupes []string + for username, pubkeys := range seen { + if len(pubkeys) > 1 { + sort.Strings(pubkeys) + // Truncate pubkeys for readability. + truncated := make([]string, len(pubkeys)) + for i, pk := range pubkeys { + if len(pk) > 16 { + truncated[i] = pk[:16] + "..." + } else { + truncated[i] = pk + } + } + dupes = append(dupes, fmt.Sprintf(" %q → [%s]", username, strings.Join(truncated, ", "))) + } + } + if len(dupes) == 0 { + return nil + } + + sort.Strings(dupes) + return fmt.Errorf("%w: the following usernames are assigned to multiple pubkeys:\n%s", + ErrDuplicateUsername, strings.Join(dupes, "\n")) +} + // LoadCohortsFromConfig reads the "cohorts" section from the farmer config // (via jety) and returns a populated Registry. It does not fail on an // empty or missing cohorts section — it simply returns an empty registry. diff --git a/internal/rbac/config_test.go b/internal/rbac/config_test.go index e3c6a92..0e62625 100644 --- a/internal/rbac/config_test.go +++ b/internal/rbac/config_test.go @@ -1,8 +1,10 @@ package rbac import ( + "errors" "os" "path/filepath" + "strings" "testing" "github.com/taigrr/jety" @@ -678,6 +680,114 @@ func TestValidateUserUniqueness_CrossSectionJety(t *testing.T) { } } +func TestValidateUsernameUniqueness_NoDuplicates(t *testing.T) { + setupJetyForRBACTest(t) + defer clearJetyRBACKeys(t) + + jety.Set("users", map[string]any{ + "admin": []any{ + map[string]any{"pubkey": "KEY_A", "username": "alice"}, + map[string]any{"pubkey": "KEY_B", "username": "bob"}, + }, + }) + + err := ValidateUsernameUniqueness() + if err != nil { + t.Errorf("expected no error for unique usernames, got: %v", err) + } +} + +func TestValidateUsernameUniqueness_DetectsDuplicate(t *testing.T) { + setupJetyForRBACTest(t) + defer clearJetyRBACKeys(t) + + jety.Set("users", map[string]any{ + "admin": []any{ + map[string]any{"pubkey": "KEY_A", "username": "alice"}, + }, + "viewer": []any{ + map[string]any{"pubkey": "KEY_B", "username": "alice"}, + }, + }) + + err := ValidateUsernameUniqueness() + if err == nil { + t.Fatal("expected error for duplicate username 'alice' across roles") + } + if !errors.Is(err, ErrDuplicateUsername) { + t.Errorf("expected ErrDuplicateUsername, got: %v", err) + } + if !strings.Contains(err.Error(), "alice") { + t.Errorf("error should mention 'alice', got: %v", err) + } +} + +func TestValidateUsernameUniqueness_SameRoleDuplicate(t *testing.T) { + setupJetyForRBACTest(t) + defer clearJetyRBACKeys(t) + + jety.Set("users", map[string]any{ + "admin": []any{ + map[string]any{"pubkey": "KEY_A", "username": "bob"}, + map[string]any{"pubkey": "KEY_C", "username": "bob"}, + }, + }) + + err := ValidateUsernameUniqueness() + if err == nil { + t.Fatal("expected error for duplicate username within same role") + } + if !errors.Is(err, ErrDuplicateUsername) { + t.Errorf("expected ErrDuplicateUsername, got: %v", err) + } +} + +func TestValidateUsernameUniqueness_EmptyUsernamesIgnored(t *testing.T) { + setupJetyForRBACTest(t) + defer clearJetyRBACKeys(t) + + // Two pubkeys without usernames should not conflict. + jety.Set("users", map[string]any{ + "admin": []any{"KEY_A"}, + "viewer": []any{"KEY_B"}, + }) + + err := ValidateUsernameUniqueness() + if err != nil { + t.Errorf("expected no error when usernames are empty, got: %v", err) + } +} + +func TestValidateUsernameUniqueness_MixedWithAndWithoutUsernames(t *testing.T) { + setupJetyForRBACTest(t) + defer clearJetyRBACKeys(t) + + jety.Set("users", map[string]any{ + "admin": []any{ + map[string]any{"pubkey": "KEY_A", "username": "alice"}, + "KEY_B", // No username — should not conflict. + }, + "viewer": []any{ + map[string]any{"pubkey": "KEY_C", "username": "bob"}, + }, + }) + + err := ValidateUsernameUniqueness() + if err != nil { + t.Errorf("expected no error for unique usernames with some empty, got: %v", err) + } +} + +func TestValidateUsernameUniqueness_EmptyConfig(t *testing.T) { + setupJetyForRBACTest(t) + defer clearJetyRBACKeys(t) + + err := ValidateUsernameUniqueness() + if err != nil { + t.Errorf("expected no error for empty config, got: %v", err) + } +} + func TestLoadCohortsFromConfig_Empty(t *testing.T) { setupJetyForRBACTest(t) defer clearJetyRBACKeys(t)