feat: unified config#533
Conversation
|
Warning Rate limit exceeded@steveiliop56 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 19 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughReplaces flat config with a nested TINYAUTH_* namespaced model, migrates CLI from Cobra to Traefik Paerser with new command packages under Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as cmd/tinyauth
participant Loaders as Config Loaders
participant Parser as Traefik Paerser
participant Bootstrap as Bootstrap Layer
participant App as Application
User->>CLI: invoke command (e.g., start, create, verify)
CLI->>Loaders: run FileLoader, FlagLoader, EnvLoader
Loaders->>Parser: decode file/flags/env (TINYAUTH_* prefixed)
Parser-->>Loaders: merged config
CLI->>Bootstrap: initialize app with merged config
Bootstrap->>Bootstrap: resolve OAuth secrets, users, LDAP
Bootstrap->>App: wire routes/services
CLI->>App: execute requested command
App-->>User: result/response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring focused review:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
.gitignore (1)
29-36: Reasonable additions that align with the unified config refactoring.These ignore rules make sense for the PR's scope: config files and generated resources shouldn't be committed, and database files are runtime data. The additions follow the existing pattern of the file.
One minor nitpick:
resources(line 36) lacks a descriptive comment, unlikeconfig.ymlandtinyauth.dbabove it. Adding a brief comment would improve consistency.Optional: Add comment for consistency
# binary out tinyauth.db + # generated resources resources.env.example (1)
59-60: Consider quoting multi-word values.Values containing spaces should be quoted to ensure consistent parsing across different environments:
🔎 Proposed fix
-TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_SCOPES=openid email profile +TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_SCOPES="openid email profile" -TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_NAME=My OAuth Provider +TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_NAME="My OAuth Provider"cmd/healthcheck.go (1)
44-44: Consider adding a timeout to the HTTP client.The HTTP client is created without a timeout, which could cause the health check to hang indefinitely on unresponsive endpoints. For a health check operation, a reasonable timeout (e.g., 10-30 seconds) would improve reliability.
🔎 Proposed fix
-client := http.Client{} +client := http.Client{ + Timeout: 30 * time.Second, +}cmd/create.go (1)
57-62: Consider masking password input in interactive mode.The password input field does not use
EchoModeto hide user input. This could expose passwords on-screen.🔎 Proposed fix to mask password input
- huh.NewInput().Title("Password").Value(&tCfg.Password).Validate((func(s string) error { + huh.NewInput().Title("Password").EchoMode(huh.EchoModePassword).Value(&tCfg.Password).Validate((func(s string) error { if s == "" { return errors.New("password cannot be empty") } return nil })),cmd/generate.go (1)
67-71: Add validation for empty User field before parsing.In non-interactive mode, if
--useris not provided,tCfg.Userwill be empty andutils.ParseUser("")will be called. WhileParseUserlikely returns an error for empty input, it would be clearer to validate upfront (similar to howcreate.govalidates username/password at line 76-78).🔎 Proposed fix
+ if tCfg.User == "" { + return errors.New("user cannot be empty") + } + user, err := utils.ParseUser(tCfg.User) if err != nil { return fmt.Errorf("failed to parse user: %w", err) }internal/config/config.go (1)
88-99: Missing YAML tags on OAuthServiceConfig fields.The
OAuthServiceConfigfields havedescriptiontags but lackyamltags, unlike the nested config structs above. If these fields should be configurable via YAML config files, they need YAML tags for consistency.Verify whether OAuth providers are intended to be configured via YAML files. If so, add YAML tags:
type OAuthServiceConfig struct { - ClientID string `description:"OAuth client ID."` - ClientSecret string `description:"OAuth client secret."` - ClientSecretFile string `description:"Path to the file containing the OAuth client secret."` - Scopes []string `description:"OAuth scopes."` - RedirectURL string `description:"OAuth redirect URL."` - AuthURL string `description:"OAuth authorization URL."` - TokenURL string `description:"OAuth token URL."` - UserinfoURL string `description:"OAuth userinfo URL."` - Insecure bool `description:"Allow insecure OAuth connections."` - Name string `description:"Provider name in UI."` + ClientID string `description:"OAuth client ID." yaml:"clientId"` + ClientSecret string `description:"OAuth client secret." yaml:"clientSecret"` + ClientSecretFile string `description:"Path to the file containing the OAuth client secret." yaml:"clientSecretFile"` + Scopes []string `description:"OAuth scopes." yaml:"scopes"` + RedirectURL string `description:"OAuth redirect URL." yaml:"redirectUrl"` + AuthURL string `description:"OAuth authorization URL." yaml:"authUrl"` + TokenURL string `description:"OAuth token URL." yaml:"tokenUrl"` + UserinfoURL string `description:"OAuth userinfo URL." yaml:"userinfoUrl"` + Insecure bool `description:"Allow insecure OAuth connections." yaml:"insecure"` + Name string `description:"Provider name in UI." yaml:"name"` }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (38)
.env.example(1 hunks).github/workflows/nightly.yml(2 hunks).github/workflows/release.yml(2 hunks).gitignore(1 hunks)Dockerfile(2 hunks)Dockerfile.dev(1 hunks)Dockerfile.distroless(2 hunks)air.toml(1 hunks)cmd/create.go(1 hunks)cmd/generate.go(1 hunks)cmd/healthcheck.go(1 hunks)cmd/root.go(0 hunks)cmd/tinyauth.go(1 hunks)cmd/verify.go(1 hunks)cmd/version.go(1 hunks)config.example.yaml(1 hunks)docker-compose.example.yml(1 hunks)frontend/src/lib/i18n/i18n.ts(0 hunks)go.mod(3 hunks)internal/bootstrap/app_bootstrap.go(3 hunks)internal/bootstrap/router_bootstrap.go(2 hunks)internal/bootstrap/service_bootstrap.go(2 hunks)internal/config/config.go(2 hunks)internal/service/docker_service.go(1 hunks)internal/service/generic_oauth_service.go(1 hunks)internal/utils/app_utils.go(0 hunks)internal/utils/app_utils_test.go(0 hunks)internal/utils/decoders/decoders.go(0 hunks)internal/utils/decoders/env_decoder.go(0 hunks)internal/utils/decoders/env_decoder_test.go(0 hunks)internal/utils/decoders/flags_decoder.go(0 hunks)internal/utils/decoders/flags_decoder_test.go(0 hunks)internal/utils/decoders/label_decoder.go(1 hunks)internal/utils/decoders/label_decoder_test.go(1 hunks)internal/utils/loaders/loader_env.go(1 hunks)internal/utils/loaders/loader_file.go(1 hunks)internal/utils/loaders/loader_flag.go(1 hunks)main.go(0 hunks)
💤 Files with no reviewable changes (10)
- internal/utils/app_utils.go
- main.go
- cmd/root.go
- internal/utils/decoders/env_decoder_test.go
- internal/utils/decoders/env_decoder.go
- internal/utils/decoders/flags_decoder_test.go
- internal/utils/decoders/decoders.go
- internal/utils/app_utils_test.go
- internal/utils/decoders/flags_decoder.go
- frontend/src/lib/i18n/i18n.ts
🧰 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 (8)
internal/service/docker_service.go (2)
internal/utils/decoders/label_decoder.go (1)
DecodeLabels(7-17)internal/config/config.go (1)
Apps(158-160)
internal/utils/decoders/label_decoder_test.go (2)
internal/utils/decoders/label_decoder.go (1)
DecodeLabels(7-17)internal/config/config.go (1)
Apps(158-160)
internal/utils/loaders/loader_env.go (1)
internal/config/config.go (1)
DefaultNamePrefix(77-77)
cmd/tinyauth.go (5)
internal/config/config.go (6)
Config(17-32)ServerConfig(34-39)AuthConfig(41-48)UIConfig(56-60)ExperimentalConfig(71-73)Version(5-5)internal/utils/loaders/loader_file.go (1)
FileLoader(10-10)internal/utils/loaders/loader_flag.go (2)
FlagLoader(10-10)FlagLoader(12-22)internal/utils/loaders/loader_env.go (1)
EnvLoader(12-12)internal/bootstrap/app_bootstrap.go (1)
NewBootstrapApp(38-42)
cmd/verify.go (2)
internal/config/config.go (1)
User(108-112)internal/utils/user_utils.go (1)
ParseUser(63-92)
internal/bootstrap/router_bootstrap.go (1)
internal/controller/oauth_controller.go (2)
NewOAuthController(36-43)OAuthControllerConfig(21-27)
cmd/version.go (1)
internal/config/config.go (3)
Version(5-5)CommitHash(6-6)BuildTimestamp(7-7)
cmd/generate.go (2)
internal/config/config.go (2)
User(108-112)Config(17-32)internal/utils/user_utils.go (1)
ParseUser(63-92)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 10-10: [UnorderedKey] The TINYAUTH_DATABASEPATH key should go before the TINYAUTH_LOGLEVEL key
(UnorderedKey)
[warning] 12-12: [UnorderedKey] The TINYAUTH_DISABLEANALYTICS key should go before the TINYAUTH_LOGLEVEL key
(UnorderedKey)
[warning] 14-14: [UnorderedKey] The TINYAUTH_DISABLERESOURCES key should go before the TINYAUTH_LOGLEVEL key
(UnorderedKey)
[warning] 16-16: [UnorderedKey] The TINYAUTH_DISABLEUIWARNINGS key should go before the TINYAUTH_LOGLEVEL key
(UnorderedKey)
[warning] 18-18: [UnorderedKey] The TINYAUTH_LOGJSON key should go before the TINYAUTH_LOGLEVEL key
(UnorderedKey)
[warning] 25-25: [UnorderedKey] The TINYAUTH_SERVER_ADDRESS key should go before the TINYAUTH_SERVER_PORT key
(UnorderedKey)
[warning] 38-38: [UnorderedKey] The TINYAUTH_SECURECOOKIE key should go before the TINYAUTH_USERSFILE key
(UnorderedKey)
[warning] 40-40: [UnorderedKey] The TINYAUTH_SESSIONEXPIRY key should go before the TINYAUTH_USERSFILE key
(UnorderedKey)
[warning] 42-42: [UnorderedKey] The TINYAUTH_LOGINTIMEOUT key should go before the TINYAUTH_SECURECOOKIE key
(UnorderedKey)
[warning] 44-44: [UnorderedKey] The TINYAUTH_LOGINMAXRETRIES key should go before the TINYAUTH_LOGINTIMEOUT key
(UnorderedKey)
[warning] 51-51: [UnorderedKey] The TINYAUTH_OAUTH_AUTOREDIRECT key should go before the TINYAUTH_OAUTH_WHITELIST key
(UnorderedKey)
[warning] 53-53: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_CLIENTID key should go before the TINYAUTH_OAUTH_WHITELIST key
(UnorderedKey)
[warning] 54-54: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_CLIENTSECRET key should go before the TINYAUTH_OAUTH_WHITELIST key
(UnorderedKey)
[warning] 55-55: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_AUTHURL key should go before the TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_CLIENTID key
(UnorderedKey)
[warning] 56-56: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_TOKENURL key should go before the TINYAUTH_OAUTH_WHITELIST key
(UnorderedKey)
[warning] 57-57: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_USERINFOURL key should go before the TINYAUTH_OAUTH_WHITELIST key
(UnorderedKey)
[warning] 58-58: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_REDIRECTURL key should go before the TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_TOKENURL key
(UnorderedKey)
[warning] 59-59: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_SCOPES key should go before the TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_TOKENURL key
(UnorderedKey)
[warning] 59-59: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 60-60: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_NAME key should go before the TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_REDIRECTURL key
(UnorderedKey)
[warning] 60-60: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 62-62: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_INSECURE key should go before the TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_NAME key
(UnorderedKey)
[warning] 69-69: [UnorderedKey] The TINYAUTH_UI_FORGOTPASSWORDMESSAGE key should go before the TINYAUTH_UI_TITLE key
(UnorderedKey)
[warning] 71-71: [UnorderedKey] The TINYAUTH_UI_BACKGROUNDIMAGE key should go before the TINYAUTH_UI_FORGOTPASSWORDMESSAGE key
(UnorderedKey)
[warning] 82-82: [UnorderedKey] The TINYAUTH_LDAP_BASEDN key should go before the TINYAUTH_LDAP_BINDDN key
(UnorderedKey)
[warning] 86-86: [UnorderedKey] The TINYAUTH_LDAP_INSECURE key should go before the TINYAUTH_LDAP_SEARCHFILTER key
(UnorderedKey)
⏰ 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 (54)
air.toml (1)
6-6: LGTM! Build target updated correctly.The change from
.tocmd/*aligns with the CLI restructuring across the codebase and is the correct Go idiom for building all packages under the cmd directory.Dockerfile.dev (1)
19-21: LGTM! Environment variables added for runtime configuration.The addition of
DATABASEPATHandRESOURCESDIRenvironment variables provides sensible defaults for development and aligns with the unified config approach across all Dockerfiles..github/workflows/nightly.yml (1)
83-83: LGTM! Build commands updated consistently.Both amd64 and arm64 build steps correctly target
cmd/*while preserving version injection via ldflags. The changes align with the CLI restructuring.Also applies to: 129-129
Dockerfile (2)
40-40: Build target updated correctly.The change to
cmd/*aligns with the CLI restructuring and is consistent across all build configurations.
55-65: Runtime configuration properly configured.The environment variables, healthcheck, and entrypoint are well-configured for the new CLI architecture:
- Default paths for database and resources
- Health monitoring via
tinyauth healthchecksubcommand- Clean entrypoint for subcommand execution
.github/workflows/release.yml (1)
61-61: LGTM! Release builds updated consistently.The build commands for both architectures correctly target
cmd/*and preserve version metadata injection, consistent with the nightly workflow.Also applies to: 104-104
Dockerfile.distroless (1)
42-42: LGTM! Distroless image properly configured.The build target and runtime configuration (environment variables, healthcheck, entrypoint) are correctly set up for the distroless variant, matching the standard Dockerfile with appropriate distroless-specific adjustments.
Also applies to: 58-68
.env.example (1)
1-86: Well-structured configuration migration.The restructuring to TINYAUTH_ prefixed environment variables with logical grouping (Base, Server, Auth, OAuth, UI, LDAP) significantly improves configuration clarity and organization. The hierarchical approach is a solid improvement over the flat structure.
go.mod (2)
24-69: Indirect dependency updates look routine.The transitive dependency version bumps (OTEL, protobuf, gRPC, etc.) appear to be routine updates that come with the framework migration. No obvious compatibility concerns.
7-22: Dependency migration to Traefik Paerser v0.2.2 completed.The removal of Cobra/Viper dependencies and addition of Traefik Paerser v0.2.2 aligns with the CLI framework migration. The dependency changes are clean and well-structured, with v0.2.2 being the current stable version integrated into recent Traefik releases.
internal/service/generic_oauth_service.go (1)
41-41: LGTM! Clean config field rename.The OAuth service now reads the TLS insecure flag from
config.Insecureinstead ofInsecureSkipVerify, aligning with the new configuration structure. The TLS verification behavior remains unchanged.docker-compose.example.yml (1)
23-24: LGTM! Environment variable naming standardized.The environment variables now use the
TINYAUTH_prefix (TINYAUTH_APPURL,TINYAUTH_AUTH_USERS), aligning with the new unified configuration structure and theDefaultNamePrefixused by the EnvLoader.internal/service/docker_service.go (1)
85-85: LGTM! Updated to generic label decoder.The call to
DecodeLabelsnow uses the generic form with an explicit type parameter[config.Apps]and root key"apps", aligning with the refactored decoder signature. The decoding logic remains functionally equivalent.internal/utils/loaders/loader_flag.go (1)
1-22: LGTM! Clean loader implementation.The
FlagLoaderfollows the new loader pattern with:
- Proper early return when no args are present
- Clear error wrapping with context
- Appropriate use of Traefik Paerser's flag decoder
This aligns well with the unified configuration approach.
internal/bootstrap/router_bootstrap.go (4)
16-17: LGTM! Config path updated to nested structure.
TrustedProxiesnow accessed viaapp.config.Server.TrustedProxies, consistent with the newServerConfignesting.
60-60: LGTM! UI configuration nested appropriately.The
Titlefield now accessed viaapp.config.UI.Title, aligning with the newUIConfigstructure.
63-65: LGTM! Multiple config paths updated to nested structure.UI and OAuth configuration fields now properly accessed through their respective nested config structs:
UI.ForgotPasswordMessageUI.BackgroundImageOAuth.AutoRedirect
73-73: LGTM! Auth configuration nested appropriately.
SecureCookienow accessed viaapp.config.Auth.SecureCookie, consistent with the newAuthConfigstructure.cmd/tinyauth.go (5)
17-40: LGTM! Sensible default configuration.The default configuration provides reasonable values:
- Log level set to "info" (appropriate default)
- Server binds to all interfaces on port 3000 (typical for containerized deployments)
- Auth timeouts and retry limits are balanced for security and usability
- UI defaults provide good out-of-box experience
Note: Relative paths for
ResourcesDirandDatabasePathassume the working directory is the project root, which is typical for CLI applications.
45-49: LGTM! Loader precedence properly ordered.The loaders are applied in sequence (File → Flag → Env), giving environment variables the highest precedence, which is appropriate for containerized deployments where env vars commonly override config files.
51-89: LGTM! Robust command setup with proper error handling.The CLI command structure is well-organized:
- Root command properly configured with configuration and loaders
- Each subcommand addition includes immediate error checking
- Fatal logging with descriptive messages is appropriate for initialization failures
91-95: LGTM! Clean CLI execution.The command execution is straightforward with appropriate error handling and fatal logging for execution failures.
98-124: LGTM! Well-structured bootstrap function.The
runCmdimplementation is clean and robust:
- Log level parsing with graceful fallback to "info"
- Conditional console/JSON logging based on configuration
- Proper error wrapping for bootstrap failures
- Startup logging includes version information
internal/utils/loaders/loader_env.go (1)
1-25: LGTM! Clean environment loader implementation.The
EnvLoaderis well-implemented:
- Uses
config.DefaultNamePrefix(TINYAUTH_) for consistent environment variable naming- Proper early return when no prefixed variables are found
- Clear error wrapping with context
- Follows the same pattern as
FlagLoaderfor consistencyinternal/utils/decoders/label_decoder_test.go (1)
65-65: LGTM! Test updated for generic decoder.The test correctly uses the new generic
DecodeLabels[config.Apps]signature with the explicit root key"apps", maintaining test coverage for the refactored decoder.config.example.yaml (6)
1-18: LGTM! Clear and well-documented top-level configuration.The top-level configuration fields are well-organized with helpful inline comments explaining each option.
20-29: LGTM! Server configuration is well-structured.The nested server configuration with optional Unix socket support is clearly documented.
31-44: LGTM! Authentication configuration is comprehensive and clear.The auth section provides good examples and helpful time conversion comments (e.g., "3600 = 1 hour").
46-64: LGTM! OAuth configuration example is thorough.The OAuth provider configuration template includes all necessary fields with clear placeholder values.
66-73: LGTM! UI customization options are straightforward.Clean and simple UI configuration with sensible defaults.
75-88: LGTM! LDAP configuration is complete with a helpful example.The LDAP section provides a practical search filter example with clear placeholder documentation.
internal/utils/loaders/loader_file.go (2)
1-8: LGTM! Package structure and imports are appropriate.The imports correctly include the necessary Traefik Paerser components for file-based configuration loading.
12-35: Load method implementation is solid.The load method correctly:
- Parses flags using Paerser
- Checks for flag presence before proceeding
- Logs an appropriate experimental feature warning
- Decodes the configuration file
- Handles errors properly
internal/utils/decoders/label_decoder.go (1)
7-16: LGTM! Generic refactor improves flexibility.The conversion to a generic function allows decoding labels into different types while maintaining type safety. The
rootparameter makes the decoding path configurable.cmd/healthcheck.go (3)
22-29: LGTM! Command structure is correctly configured.The healthcheck command is properly structured with
AllowArg: trueto support passing the app URL as an argument.
30-42: LGTM! APPURL handling is correct.The code properly prioritizes the command-line argument over the environment variable and validates that a URL is provided.
64-80: LGTM! Response handling is correct.The response parsing and error handling use proper error wrapping with
%wand informative messages.internal/bootstrap/app_bootstrap.go (4)
45-52: LGTM! User configuration updated for nested structure.The user parsing correctly uses the new nested
Auth.UsersandAuth.UsersFileconfiguration paths.
54-82: LGTM! OAuth provider configuration is well-structured.The OAuth provider setup correctly:
- Loads providers from nested config
- Handles client secret files securely (reads content, clears file path)
- Sets sensible defaults for RedirectURL and Name
- Maintains provider configuration consistency
168-183: LGTM! Unix socket handling includes proper cleanup.The code correctly removes any existing socket file before binding, preventing "address already in use" errors.
186-192: LGTM! HTTP server startup uses nested server configuration.The server binding correctly uses
Server.AddressandServer.Portfrom the new nested configuration structure.internal/bootstrap/service_bootstrap.go (2)
33-48: LGTM! LDAP service initialization updated for nested config.The LDAP configuration correctly uses the new nested
Ldap.*configuration paths with appropriate error handling (warning on failure, continues without LDAP).
70-86: LGTM! Auth service configuration migrated to nested structure.The auth service initialization correctly uses:
Auth.*fields for authentication settingsOAuth.Whitelistfor OAuth-specific configurationcmd/verify.go (3)
18-34: LGTM! VerifyUserConfig structure is well-defined.The configuration struct with description tags integrates cleanly with Traefik Paerser's CLI framework.
51-83: LGTM! Interactive form provides good user experience.The huh form correctly validates required fields while leaving TOTP optional, with proper error handling.
85-107: LGTM! User verification logic is well-structured.The verification flow correctly:
- Parses the user configuration
- Validates username and password
- Handles optional TOTP gracefully with a warning if TOTP is provided but not configured
cmd/version.go (1)
10-23: LGTM! Version command is clean and simple.The version command correctly displays build information from the config package with a straightforward implementation.
cmd/create.go (2)
1-15: LGTM on imports and package structure.The imports are well-organized and align with the migration to the Traefik Paerser CLI framework.
17-31: LGTM on configuration struct and constructor.The
CreateUserConfigstruct andNewCreateUserConfigconstructor follow the pattern established across other command modules in this PR.cmd/generate.go (2)
1-17: LGTM on imports and package structure.The imports are consistent with the CLI migration pattern.
19-29: LGTM on configuration struct and constructor.The
GenerateTotpConfigstruct follows the established pattern.internal/config/config.go (3)
17-32: LGTM on the restructured Config.The migration to a hierarchical, nested configuration model improves organization and maintainability. The YAML tags and descriptions are well-defined.
34-48: LGTM on ServerConfig and AuthConfig.Both structs have consistent YAML tags and descriptions.
56-73: LGTM on UIConfig, LdapConfig, and ExperimentalConfig.The
yaml:"-"onConfigFilecorrectly excludes it from YAML parsing since it's a CLI-only option.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
cmd/tinyauth/create.go (1)
93-93: Sensitive credentials still logged.The full
username:hashedPasswordstring is still being logged to the structured logger. While the previous review flagged this concern, it appears the fix was not applied. Logging credentials (even hashed) to stderr may expose them in log aggregation systems, CI logs, or terminal history.🔎 Proposed fix
- log.Info().Str("user", fmt.Sprintf("%s:%s", tCfg.Username, passwdStr)).Msg("User created") + log.Info().Msg("User created successfully") + fmt.Printf("User credentials: %s:%s\n", tCfg.Username, passwdStr)cmd/tinyauth/generate.go (2)
82-93: Avoid logging TOTP secrets.Line 93 logs the raw TOTP secret, which exposes sensitive authentication material to log outputs. The secret is already displayed to the user via the QR code. This issue was previously flagged and remains unaddressed in the migration to the new CLI framework.
🔎 Proposed fix
- log.Info().Str("secret", secret).Msg("Generated TOTP secret") + log.Info().Msg("Generated TOTP secret (see QR code below)")
109-117: Avoid logging full user credentials including TOTP secret.Line 114 logs the complete user credentials (username, password hash, and TOTP secret) to structured logs. This exposes sensitive authentication data to log outputs. This issue was previously flagged and remains unaddressed in the migration. Consider printing to stdout instead or removing the structured field.
🔎 Proposed fix
- log.Info().Str("user", fmt.Sprintf("%s:%s:%s", user.Username, user.Password, user.TotpSecret)).Msg("Add the totp secret to your authenticator app then use the verify command to ensure everything is working correctly.") + log.Info().Msg("Add the TOTP secret to your authenticator app, then use the verify command to ensure everything is working correctly.") + fmt.Printf("Updated user: %s:%s:%s\n", user.Username, user.Password, user.TotpSecret)
🧹 Nitpick comments (1)
cmd/tinyauth/tinyauth.go (1)
45-49: Variable shadows imported package name.The local variable
loadersshadows the imported packagetinyauth/internal/utils/loaderson line 10. While this doesn't cause a runtime issue here since the package isn't used after this point, it reduces code clarity and could cause confusion during maintenance.🔎 Suggested rename
- loaders := []cli.ResourceLoader{ + resourceLoaders := []cli.ResourceLoader{ &loaders.FileLoader{}, &loaders.FlagLoader{}, &loaders.EnvLoader{}, }And update line 55:
- Resources: loaders, + Resources: resourceLoaders,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
.env.example.github/workflows/nightly.yml.github/workflows/release.yml.gitignoreDockerfileDockerfile.distrolessair.tomlcmd/tinyauth/create.gocmd/tinyauth/generate.gocmd/tinyauth/healthcheck.gocmd/tinyauth/tinyauth.gocmd/tinyauth/verify.gocmd/tinyauth/version.gogo.modinternal/bootstrap/app_bootstrap.gointernal/config/config.gointernal/service/generic_oauth_service.gointernal/service/github_oauth_service.gointernal/service/google_oauth_service.go
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/release.yml
- .github/workflows/nightly.yml
- .gitignore
🧰 Additional context used
🧬 Code graph analysis (5)
internal/service/github_oauth_service.go (1)
internal/service/oauth_broker_service.go (1)
Init(11-19)
cmd/tinyauth/generate.go (5)
internal/config/config.go (2)
User(108-112)Config(17-32)internal/utils/user_utils.go (1)
ParseUser(63-92)cmd/generate.go (3)
c(52-120)c(32-46)c(48-50)cmd/verify.go (1)
c(55-118)cmd/create.go (2)
c(53-99)c(31-47)
cmd/tinyauth/verify.go (2)
internal/config/config.go (1)
User(108-112)internal/utils/user_utils.go (1)
ParseUser(63-92)
cmd/tinyauth/tinyauth.go (4)
internal/config/config.go (6)
Config(17-32)ServerConfig(34-39)AuthConfig(41-48)UIConfig(56-60)ExperimentalConfig(71-73)Version(5-5)internal/utils/loaders/loader_file.go (1)
FileLoader(10-10)internal/utils/loaders/loader_env.go (1)
EnvLoader(12-12)internal/bootstrap/app_bootstrap.go (1)
NewBootstrapApp(38-42)
cmd/tinyauth/version.go (1)
internal/config/config.go (3)
Version(5-5)CommitHash(6-6)BuildTimestamp(7-7)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 6-6: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 8-8: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 10-10: [UnorderedKey] The TINYAUTH_DATABASEPATH key should go before the TINYAUTH_LOGLEVEL key
(UnorderedKey)
[warning] 12-12: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 12-12: [UnorderedKey] The TINYAUTH_DISABLEANALYTICS key should go before the TINYAUTH_LOGLEVEL key
(UnorderedKey)
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 14-14: [UnorderedKey] The TINYAUTH_DISABLERESOURCES key should go before the TINYAUTH_LOGLEVEL key
(UnorderedKey)
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 16-16: [UnorderedKey] The TINYAUTH_DISABLEUIWARNINGS key should go before the TINYAUTH_LOGLEVEL key
(UnorderedKey)
[warning] 18-18: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 18-18: [UnorderedKey] The TINYAUTH_LOGJSON key should go before the TINYAUTH_LOGLEVEL key
(UnorderedKey)
[warning] 23-23: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 25-25: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 25-25: [UnorderedKey] The TINYAUTH_SERVER_ADDRESS key should go before the TINYAUTH_SERVER_PORT key
(UnorderedKey)
[warning] 27-27: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 29-29: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 36-36: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 38-38: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 38-38: [UnorderedKey] The TINYAUTH_SECURECOOKIE key should go before the TINYAUTH_USERSFILE key
(UnorderedKey)
[warning] 40-40: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 40-40: [UnorderedKey] The TINYAUTH_SESSIONEXPIRY key should go before the TINYAUTH_USERSFILE key
(UnorderedKey)
[warning] 42-42: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 42-42: [UnorderedKey] The TINYAUTH_LOGINTIMEOUT key should go before the TINYAUTH_SECURECOOKIE key
(UnorderedKey)
[warning] 44-44: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 44-44: [UnorderedKey] The TINYAUTH_LOGINMAXRETRIES key should go before the TINYAUTH_LOGINTIMEOUT key
(UnorderedKey)
[warning] 49-49: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 51-51: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 51-51: [UnorderedKey] The TINYAUTH_OAUTH_AUTOREDIRECT key should go before the TINYAUTH_OAUTH_WHITELIST key
(UnorderedKey)
[warning] 53-53: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 53-53: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_CLIENTID key should go before the TINYAUTH_OAUTH_WHITELIST key
(UnorderedKey)
[warning] 54-54: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 54-54: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_CLIENTSECRET key should go before the TINYAUTH_OAUTH_WHITELIST key
(UnorderedKey)
[warning] 55-55: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 55-55: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_AUTHURL key should go before the TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_CLIENTID key
(UnorderedKey)
[warning] 56-56: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 56-56: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_TOKENURL key should go before the TINYAUTH_OAUTH_WHITELIST key
(UnorderedKey)
[warning] 57-57: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 57-57: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_USERINFOURL key should go before the TINYAUTH_OAUTH_WHITELIST key
(UnorderedKey)
[warning] 58-58: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 58-58: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_REDIRECTURL key should go before the TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_TOKENURL key
(UnorderedKey)
[warning] 59-59: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_SCOPES key should go before the TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_TOKENURL key
(UnorderedKey)
[warning] 60-60: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_NAME key should go before the TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_REDIRECTURL key
(UnorderedKey)
[warning] 62-62: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 62-62: [UnorderedKey] The TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_INSECURE key should go before the TINYAUTH_OAUTH_PROVIDERS_MYPROVIDER_NAME key
(UnorderedKey)
[warning] 67-67: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 69-69: [UnorderedKey] The TINYAUTH_UI_FORGOTPASSWORDMESSAGE key should go before the TINYAUTH_UI_TITLE key
(UnorderedKey)
[warning] 71-71: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 71-71: [UnorderedKey] The TINYAUTH_UI_BACKGROUNDIMAGE key should go before the TINYAUTH_UI_FORGOTPASSWORDMESSAGE key
(UnorderedKey)
[warning] 76-76: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 80-80: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 82-82: [UnorderedKey] The TINYAUTH_LDAP_BASEDN key should go before the TINYAUTH_LDAP_BINDDN key
(UnorderedKey)
[warning] 86-86: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 86-86: [UnorderedKey] The TINYAUTH_LDAP_INSECURE key should go before the TINYAUTH_LDAP_SEARCHFILTER key
(UnorderedKey)
⏰ 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 (32)
go.mod (2)
7-22: Removed direct dependencies are no longer used in code.Verification confirms that
github.com/spf13/cobra,github.com/spf13/viper,github.com/go-playground/validator, andgithub.com/stoewer/go-strcasehave no direct imports in the codebase. These packages may still be present as indirect dependencies of other libraries, but there are no direct usages.
26-29: The new indirect dependencies (BurntSushi/toml, Masterminds utilities, mergo, copystructure, reflectwalk, decimal) align well with the migration to Traefik Paerser and the hierarchical config refactoring. These are expected transitive dependencies when adopting traefik/paerser for configuration loading.Note: The concern about gopkg.in/yaml.v3 version downgrade is not valid—v3.0.1 is the current stable release; no v3.0.4 release exists.
.env.example (1)
1-86: LGTM! Environment variables properly organized and quoted.The migration to
TINYAUTH_prefixed environment variables with logical sectioning (Base, Server, Authentication, OAuth, UI, LDAP) improves clarity. The quotes on all values align with past review feedback requesting consistency.Note: The static analysis warnings about quote characters and key ordering can be safely ignored. The quotes were explicitly requested in previous reviews, and the logical sectioned grouping is more maintainable than strict alphabetical ordering.
air.toml (1)
6-6: LGTM! Build target updated correctly.The build command now targets
./cmd/tinyauth, consistent with the package restructuring in this PR.internal/service/google_oauth_service.go (1)
47-55: LGTM! HTTP client timeout improves resilience.The 30-second timeout prevents indefinite hangs during OAuth flows with Google's endpoints, improving reliability.
internal/service/github_oauth_service.go (1)
52-60: LGTM! HTTP client timeout improves resilience.The 30-second timeout prevents indefinite hangs during OAuth flows with GitHub's endpoints, consistent with the timeout applied to other OAuth providers.
internal/service/generic_oauth_service.go (2)
29-45: LGTM! Field name simplified.The field name change from
config.InsecureSkipVerifytoconfig.Insecureis more concise and aligns with the config restructuring in this PR.
47-66: LGTM! HTTP client timeout improves resilience.The 30-second timeout prevents indefinite hangs during OAuth flows with generic providers, consistent with timeouts applied to GitHub and Google OAuth services.
cmd/tinyauth/version.go (1)
10-23: LGTM! Version command properly structured.The Traefik Paerser CLI-based version command cleanly prints version metadata from the config package.
Dockerfile.distroless (1)
42-42: LGTM! Build target updated correctly.The build command now targets
./cmd/tinyauth, consistent with the package restructuring.cmd/tinyauth/healthcheck.go (1)
42-83: LGTM! Healthcheck implementation is robust.The healthcheck logic properly validates the URL, includes a 30-second timeout, handles errors gracefully, and provides structured logging.
cmd/tinyauth/tinyauth.go (1)
98-124: LGTM!The
runCmdfunction correctly sets up logging with fallback behavior, initializes the bootstrap app, and propagates errors appropriately. The logging configuration with conditional JSON/console output and caller information is well implemented.Dockerfile (1)
40-40: LGTM!The build path correctly points to the new
./cmd/tinyauthpackage location, and the ldflags properly inject version metadata.internal/bootstrap/app_bootstrap.go (3)
54-62: LGTM!The OAuth provider setup correctly loads client secrets from files when specified, clears the file path after loading for security, and properly reassigns the modified provider back to the map.
64-82: LGTM!The provider configuration logic correctly handles override providers by setting default redirect URLs and provider names with appropriate fallback behavior.
168-190: LGTM!The server startup logic correctly handles both Unix socket and TCP modes, properly cleans up existing socket files, and uses the nested
Serverconfiguration consistently.cmd/tinyauth/verify.go (1)
109-117: LGTM!The TOTP validation error handling has been corrected to return a proper error message without incorrectly wrapping the stale
errvariable from the bcrypt comparison.cmd/tinyauth/create.go (1)
76-91: LGTM!The input validation, bcrypt password hashing with default cost, and Docker-specific dollar sign escaping are correctly implemented.
cmd/tinyauth/generate.go (7)
1-17: LGTM!The imports and package structure are appropriate for the TOTP generation command functionality.
19-22: LGTM!The configuration struct is well-defined with appropriate description tags for CLI integration.
24-29: LGTM!The constructor follows standard patterns and initializes with appropriate defaults.
31-44: LGTM!The command setup properly integrates with the Traefik Paerser CLI framework, and the logger configuration is appropriate for a CLI tool.
46-65: LGTM!The interactive form implementation is clean, and the error handling is improved compared to the previous Cobra-based implementation.
67-80: LGTM!The user parsing, Docker detection, and TOTP secret validation logic is correct and includes appropriate error handling.
95-107: LGTM!The QR code generation and configuration is correct, with output appropriately directed to stdout for user scanning.
internal/config/config.go (7)
17-32: LGTM!The hierarchical configuration structure is well-designed with comprehensive description and YAML tags. The nested approach improves configuration organization and maintainability.
34-39: LGTM!The ServerConfig struct is well-defined with appropriate field types and comprehensive tagging.
41-48: LGTM!The AuthConfig struct is well-defined with appropriate field types and comprehensive tagging for authentication settings.
50-54: LGTM!The OAuthConfig struct is well-defined. The Providers field now includes both description and yaml tags, addressing the previous review comment.
56-69: LGTM!The UIConfig and LdapConfig structs are well-defined with comprehensive tagging for their respective configuration domains.
71-77: LGTM!The ExperimentalConfig struct appropriately excludes ConfigFile from YAML serialization using yaml:"-", and the DefaultNamePrefix constant is well-defined for environment variable loading.
88-99: LGTM!The OAuthServiceConfig struct is well-defined with comprehensive description tags. The absence of yaml tags is appropriate since this struct is used as a map value in the Providers configuration.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #533 +/- ##
==========================================
- Coverage 23.37% 20.34% -3.03%
==========================================
Files 38 37 -1
Lines 2263 2123 -140
==========================================
- Hits 529 432 -97
+ Misses 1697 1661 -36
+ Partials 37 30 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.