feat: add support for app-level oauth whitelist file#898
Conversation
📝 WalkthroughWalkthroughThis PR extends OAuth whitelist configuration to support file-based entries per app. The ChangesOAuth Whitelist File Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@internal/service/access_controls_service_test.go`:
- Line 249: Replace the OS-specific substring assertion on err with a semantic
existence check: instead of assert.ErrorContains(t, err, "no such file or
directory"), assert that the error is os.ErrNotExist (e.g. assert.ErrorIs(t,
err, os.ErrNotExist) or assert.True(t, errors.Is(err, os.ErrNotExist))). Update
imports to include "os" (and "errors" if using errors.Is) and keep the same err
variable and test context in access_controls_service_test.go.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 761cac64-5163-4b56-94a5-9163c814830a
📒 Files selected for processing (5)
.env.exampleinternal/model/config.gointernal/service/access_controls_service.gointernal/service/access_controls_service_test.gointernal/utils/decoders/label_decoder_test.go
| got, err := svc.GetAccessControls("foo.example.com") | ||
|
|
||
| assert.Nil(t, got) | ||
| assert.ErrorContains(t, err, "no such file or directory") |
There was a problem hiding this comment.
Avoid OS-specific error-string assertions.
Line 249 matches "no such file or directory", which is platform-dependent. Prefer semantic error checks.
Suggested fix
- assert.ErrorContains(t, err, "no such file or directory")
+ assert.ErrorIs(t, err, os.ErrNotExist)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assert.ErrorContains(t, err, "no such file or directory") | |
| assert.ErrorIs(t, err, os.ErrNotExist) |
🤖 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/service/access_controls_service_test.go` at line 249, Replace the
OS-specific substring assertion on err with a semantic existence check: instead
of assert.ErrorContains(t, err, "no such file or directory"), assert that the
error is os.ErrNotExist (e.g. assert.ErrorIs(t, err, os.ErrNotExist) or
assert.True(t, errors.Is(err, os.ErrNotExist))). Update imports to include "os"
(and "errors" if using errors.Is) and keep the same err variable and test
context in access_controls_service_test.go.
Follow-up to #817.
Added
TINYAUTH_APPS_name_OAUTH_WHITELISTFILEsupport for loading app-specific OAuth whitelist entries from a file, merged with the existing inlineTINYAUTH_APPS_name_OAUTH_WHITELISTconfig.(Also corrected the app OAuth whitelist description from "groups" to "emails")
Summary by CodeRabbit