refactor: simplify user parsing

[steveiliop56]

Summary by CodeRabbit

• Refactor
  • Updated internal configuration handling to use array-based format for trusted proxies, user management, and OAuth whitelist settings. Configuration parsing has been restructured to process list-based values while maintaining equivalent functionality.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR converts configuration fields from comma-delimited strings to slices of strings (TrustedProxies, Users, OAuth whitelist), updating parsing utilities and all downstream usages throughout the codebase to operate on typed arrays instead of string-splitting operations.

Changes

Cohort / File(s)	Summary
Configuration Type Updates internal/config/config.go	Updated public struct fields from string to []string: ServerConfig.TrustedProxies, AuthConfig.Users, OAuthConfig.Whitelist
Bootstrap and Service Logic internal/bootstrap/router_bootstrap.go	Removed strings.Split() call; now passes raw []string directly to SetTrustedProxies. Removed unused strings import.
Auth Service internal/service/auth_service.go	Updated AuthServiceConfig.OauthWhitelist field from string to []string. IsEmailWhitelisted now converts slice to comma-separated string before filtering.
User Utilities Refactor internal/utils/user_utils.go	Major refactor: ParseUsers now accepts []string instead of string; GetUsers signature expanded to (usersCfg []string, usersPath string) and now aggregates users from both slice and file; ParseUser refactored with SplitN for structured parsing.
Test Updates internal/utils/user_utils_test.go, internal/controller/proxy_controller_test.go, internal/controller/user_controller_test.go	Updated test calls to pass []string instead of string for GetUsers and ParseUsers; updated test fixture setup to use empty slices instead of empty strings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: unified config #533: Main PR that introduced config type changes from string to []string for TrustedProxies, Users, and OAuth whitelist fields; this PR extends and adapts downstream usages to work with the new types.

Poem

A rabbit hops with joy today,
Configs in clean slices laid,
No more splitting on the way,
Arrays bloom where strings have swayed,
Typed and tidy—hip hooray! 🐰✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor: simplify user parsing' accurately describes the main focus of the changeset, which refactors user parsing utilities and related configuration handling to operate on slices rather than comma-delimited strings.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 86.48649% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 19.12%. Comparing base (4546122) to head (c453b57).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/utils/user_utils.go	91.42%	1 Missing and 2 partials ⚠️
internal/bootstrap/router_bootstrap.go	0.00%	1 Missing ⚠️
internal/service/auth_service.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #571      +/-   ##
==========================================
- Coverage   19.23%   19.12%   -0.11%     
==========================================
  Files          39       39              
  Lines        2298     2295       -3     
==========================================
- Hits          442      439       -3     
  Misses       1828     1828              
  Partials       28       28              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

internal/config/config.go (1)

39-39: Consider updating field descriptions to reflect slice types.

The field description still mentions "Comma-separated list" while the type is now []string. If the configuration loader parses CSV input into slices automatically, this description is fine. Otherwise, consider updating it to reflect that it's a list/array type.

internal/service/auth_service.go (1)

30-30: Consider refactoring to avoid join/split cycle.

The OauthWhitelist field correctly changed to []string. However, in IsEmailWhitelisted (line 190), the slice is joined into a comma-separated string before passing to CheckFilter, which then splits it back into a slice (as seen in the CheckFilter implementation from internal/utils/security_utils.go).

While this preserves existing behavior correctly, consider refactoring CheckFilter to accept []string directly to avoid the unnecessary join/split cycle.

♻️ Potential optimization

Update CheckFilter signature to accept a slice parameter:

func CheckFilter(filters []string, str string) bool {

Then update the call site:

 func (auth *AuthService) IsEmailWhitelisted(email string) bool {
-	return utils.CheckFilter(strings.Join(auth.config.OauthWhitelist, ","), email)
+	return utils.CheckFilter(auth.config.OauthWhitelist, email)
 }

Note: This would require updating all CheckFilter call sites throughout the codebase.

Also applies to: 190-190

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4546122 and c453b57.

📒 Files selected for processing (7)

• internal/bootstrap/router_bootstrap.go
• internal/config/config.go
• internal/controller/proxy_controller_test.go
• internal/controller/user_controller_test.go
• internal/service/auth_service.go
• internal/utils/user_utils.go
• internal/utils/user_utils_test.go

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-06-25T14:57:03.955Z

Learnt from: steveiliop56
Repo: steveiliop56/tinyauth PR: 211
File: internal/utils/utils.go:373-407
Timestamp: 2025-06-25T14:57:03.955Z
Learning: In Go applications using the gin web framework, IP addresses extracted from gin.Context (like from request headers or remote address) are already validated by the framework and don't require additional validation in utility functions that process them.

Applied to files:

• internal/bootstrap/router_bootstrap.go

🧬 Code graph analysis (3)

internal/utils/user_utils_test.go (1)

internal/utils/user_utils.go (2)

• GetUsers (31-61)
• ParseUsers (10-29)

internal/service/auth_service.go (1)

internal/utils/security_utils.go (1)

• CheckFilter (77-102)

internal/utils/user_utils.go (2)

internal/config/config.go (1)

• User (119-123)

internal/utils/fs_utils.go (1)

• ReadFile (5-17)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (go)

🔇 Additional comments (9)

internal/controller/proxy_controller_test.go (1)

60-60: LGTM!

The test correctly initializes OauthWhitelist as an empty slice to match the updated type signature.

internal/controller/user_controller_test.go (1)

63-63: LGTM!

The test correctly initializes OauthWhitelist as an empty slice to match the updated type signature.

internal/bootstrap/router_bootstrap.go (1)

16-22: API compatibility verified — no issues found.

The code correctly passes app.config.Server.TrustedProxies (type []string) to engine.SetTrustedProxies(), which matches gin v1.11.0's method signature. The length check before calling SetTrustedProxies is appropriate, and error handling is in place.

internal/utils/user_utils.go (3)

10-29: LGTM!

The ParseUsers function correctly handles the slice-based input, properly trims whitespace, skips empty entries, and delegates to ParseUser for individual parsing. Error handling is appropriate.

---

63-92: Consider potential issue with $$ replacement in bcrypt hashes.

The $$ to $ replacement on lines 64-66 could unintentionally modify bcrypt password hashes (which contain $ as field separators like $2a$10$...). If a bcrypt hash happens to contain $$ (unlikely but possible in the salt portion), it would be corrupted.

This appears to be intentional escape handling (test on line 126 confirms pa$$word123 → pa$word123), but the placement before splitting means it affects the entire string including username and TOTP secret. Consider documenting this behavior or applying the replacement only to the password field after splitting.

---

31-61: No action needed. The project's go.mod explicitly specifies Go 1.24.0, and strings.SplitSeq was introduced in Go 1.24.0, so the code is fully compatible with the project's Go version requirement.

internal/utils/user_utils_test.go (3)

12-75: LGTM!

The TestGetUsers function provides comprehensive coverage for the refactored API:

• File-only input (line 25)
• Config slice-only input (line 37)
• Both combined (line 49)
• Empty inputs (line 63)
• Non-existent file error handling (line 70)

All test cases correctly use the new []string parameter type.

---

77-104: LGTM!

The TestParseUsers function correctly tests the slice-based API, including proper handling of whitespace-padded entries and TOTP secrets.

---

106-164: LGTM!

The TestParseUser function provides thorough coverage including:

• Users with and without TOTP
• The $$ escape sequence handling
• Whitespace trimming
• Various invalid format scenarios

Good edge case coverage for error conditions.