feat: add support for oauth whitelist file (#817)#826
Merged
steveiliop56 merged 5 commits intotinyauthapp:mainfrom May 7, 2026
Merged
feat: add support for oauth whitelist file (#817)#826steveiliop56 merged 5 commits intotinyauthapp:mainfrom
steveiliop56 merged 5 commits intotinyauthapp:mainfrom
Conversation
dosubot
Bot
added
the
size:M
Contributor
📝 WalkthroughWalkthroughThis PR adds file-based OAuth whitelist support via a new ChangesOAuth Whitelist File Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/utils/string_utils_test.go (1)
4-4: Harden temp-file test setup to avoid collisions and OS-specific assertions.Line 69 and Line 87 use fixed
/tmp/...paths, and Line 88 asserts an OS-specific error string. Prefert.TempDir()+errors.Is(err, os.ErrNotExist)for stable tests.Proposed test hardening diff
import ( + "errors" "os" + "path/filepath" "testing" @@ func TestGetStringList(t *testing.T) { - file, err := os.Create("/tmp/tinyauth_list_test_file") - assert.NilError(t, err) - - _, err = file.WriteString(" third@example.com \n\n fourth@example.com \n") - assert.NilError(t, err) - - err = file.Close() - assert.NilError(t, err) - defer os.Remove("/tmp/tinyauth_list_test_file") + tmpDir := t.TempDir() + filePath := filepath.Join(tmpDir, "tinyauth_list_test_file") + err := os.WriteFile(filePath, []byte(" third@example.com \n\n fourth@example.com \n"), 0o600) + assert.NilError(t, err) - values, err := utils.GetStringList([]string{" first@example.com ", "", "second@example.com"}, "/tmp/tinyauth_list_test_file") + values, err := utils.GetStringList([]string{" first@example.com ", "", "second@example.com"}, filePath) assert.NilError(t, err) assert.DeepEqual(t, []string{"first@example.com", "second@example.com", "third@example.com", "fourth@example.com"}, values) @@ - values, err = utils.GetStringList(nil, "/tmp/non_existing_list_file") - assert.ErrorContains(t, err, "no such file or directory") + values, err = utils.GetStringList(nil, filepath.Join(tmpDir, "non_existing_list_file")) + assert.Assert(t, errors.Is(err, os.ErrNotExist)) assert.DeepEqual(t, []string{}, values) }Also applies to: 69-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/utils/string_utils_test.go` at line 4, Tests in internal/utils/string_utils_test.go use hard-coded /tmp/... paths and assert OS-specific error text; update the failing test setup to use t.TempDir() to create unique temp directories and files, construct expected paths relative to that temp dir instead of using fixed /tmp paths, and replace string equality on the error with errors.Is(err, os.ErrNotExist) (importing "errors" and "os" as needed). Locate the test function(s) that reference the fixed paths and the error assertion (search for the hard-coded "/tmp/" literals and the assertion comparing err.Error()) and modify them to use t.TempDir(), build paths with filepath.Join, and use errors.Is for the non-existence check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/utils/string_utils_test.go`:
- Line 4: Tests in internal/utils/string_utils_test.go use hard-coded /tmp/...
paths and assert OS-specific error text; update the failing test setup to use
t.TempDir() to create unique temp directories and files, construct expected
paths relative to that temp dir instead of using fixed /tmp paths, and replace
string equality on the error with errors.Is(err, os.ErrNotExist) (importing
"errors" and "os" as needed). Locate the test function(s) that reference the
fixed paths and the error assertion (search for the hard-coded "/tmp/" literals
and the assertion comparing err.Error()) and modify them to use t.TempDir(),
build paths with filepath.Join, and use errors.Is for the non-existence check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b5b80f72-6e46-43a7-abcc-42c066793a67
📒 Files selected for processing (7)
.env.exampleinternal/bootstrap/app_bootstrap.gointernal/bootstrap/service_bootstrap.gointernal/config/config.gointernal/utils/string_utils.gointernal/utils/string_utils_test.gointernal/utils/user_utils.go
dosubot
Bot
added
size:XL
Member
|
Fixed it. |
dosubot
Bot
added
size:M
steveiliop56
previously approved these changes
May 7, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/model/config.go (1)
249-252: ⚡ Quick win
AppOAuthhas noWhitelistFilecounterpart — consider whether per-app file-based whitelists are in scope.Issue
#817explicitly calls outTINYAUTH_APPS_name_OAUTH_WHITELISTFILEas desired behavior. If per-app file support is intended for this PR,AppOAuthneeds the same treatment:💡 Suggested addition to
AppOAuthtype AppOAuth struct { Whitelist string `description:"Comma-separated list of allowed OAuth groups." yaml:"whitelist"` + WhitelistFile string `description:"Path to file containing allowed OAuth groups/emails." yaml:"whitelistFile"` Groups string `description:"Comma-separated list of required OAuth groups." yaml:"groups"` }If per-app file support is intentionally deferred, a follow-up issue or TODO comment would make that explicit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/model/config.go` around lines 249 - 252, AppOAuth currently defines Whitelist and Groups but lacks a WhitelistFile field requested by Issue `#817`; add a WhitelistFile string field to the AppOAuth struct (with description and yaml tag consistent with Whitelist) to support per-app file-based whitelists, or if you intentionally defer this, add a clear TODO comment on the AppOAuth definition noting that TINYAUTH_APPS_name_OAUTH_WHITELISTFILE support is pending and reference Issue `#817` so the missing per-app file support is explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/model/config.go`:
- Around line 249-252: AppOAuth currently defines Whitelist and Groups but lacks
a WhitelistFile field requested by Issue `#817`; add a WhitelistFile string field
to the AppOAuth struct (with description and yaml tag consistent with Whitelist)
to support per-app file-based whitelists, or if you intentionally defer this,
add a clear TODO comment on the AppOAuth definition noting that
TINYAUTH_APPS_name_OAUTH_WHITELISTFILE support is pending and reference Issue
`#817` so the missing per-app file support is explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c0e1c84-1805-4fd8-9b50-038ea294b901
📒 Files selected for processing (5)
internal/bootstrap/app_bootstrap.gointernal/bootstrap/service_bootstrap.gointernal/model/config.gointernal/utils/string_utils_test.gointernal/utils/user_utils.go
✅ Files skipped from review due to trivial changes (1)
- internal/bootstrap/app_bootstrap.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/bootstrap/service_bootstrap.go
- internal/utils/string_utils_test.go
- internal/utils/user_utils.go
steveiliop56
approved these changes
May 7, 2026
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Rycochet
approved these changes
May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #817.
Added
TINYAUTH_OAUTH_WHITELISTFILEsupport for loading whitelist entries from a file, merged with the existing inline whitelist config. This follows the same pattern as auth users/usersfile. Extracted shared inline/file parsing into reusable string utils.Summary by CodeRabbit
New Features
TINYAUTH_OAUTH_WHITELISTFILEenvironment variable, providing an alternative to comma-separated values.Tests