Conversation
Member
appleboy
commented
Mar 7, 2026
- Refactor audit logging to centralize entry construction, reuse timestamps, and remove duplicated enrichment logic across async and sync paths
- Extract common token to client enrichment into a shared helper and reuse it for paginated and non-paginated queries
- Replace hardcoded token category and status strings with typed model constants throughout the token service
- Adjust device code exchange to return access denied for invalid codes instead of treating them as expired
- Update authorization checks to use client type constants instead of string literals
- Improve sensitive field masking by defining shared full and partial mask field lists
- Add a helper for logging authentication failures and simplify repeated audit logging logic
- Fix test coverage and edge cases for device code hashing, user code normalization, and token assertions
- Improve error handling by using typed database errors instead of string matching
- Simplify unique username generation logic for OAuth users
- Refactor audit logging to centralize entry construction, reuse timestamps, and remove duplicated enrichment logic across async and sync paths - Extract common token to client enrichment into a shared helper and reuse it for paginated and non-paginated queries - Replace hardcoded token category and status strings with typed model constants throughout the token service - Adjust device code exchange to return access denied for invalid codes instead of treating them as expired - Update authorization checks to use client type constants instead of string literals - Improve sensitive field masking by defining shared full and partial mask field lists - Add a helper for logging authentication failures and simplify repeated audit logging logic - Fix test coverage and edge cases for device code hashing, user code normalization, and token assertions - Improve error handling by using typed database errors instead of string matching - Simplify unique username generation logic for OAuth users Signed-off-by: appleboy <appleboy.tw@gmail.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors authentication/audit logging and token issuance paths to reduce duplication, standardize constants, and adjust OAuth device-code error semantics.
Changes:
- Centralizes audit log record construction (shared enrichment/masking + shared timestamps) and adds a helper for auth failure audit logging.
- Reuses a shared helper to enrich token listings with client metadata (paginated and non-paginated).
- Replaces token status/category and client type string literals with typed/constants, and updates device-code exchange behavior + related tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/services/audit.go | Introduces shared audit log builder and shared masking field lists. |
| internal/services/authorization.go | Switches client type checks to shared client type constants. |
| internal/services/device_security_test.go | Tightens device-code hashing negative test to ensure length checks are exercised. |
| internal/services/device_test.go | Fixes user-code normalization test to actually test lowercase input. |
| internal/services/token.go | Adjusts device-code exchange error mapping, replaces token constants, and extracts client-enrichment helper for token listings. |
| internal/services/token_client_credentials_test.go | Updates expectations to use typed token status/category constants. |
| internal/services/token_test.go | Updates expectations for device-code exchange error and typed token constants. |
| internal/services/user.go | Adds shared auth-failure audit helper, improves username generation, and refactors duplicate-email detection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Change masking logic so partial masking takes precedence over full redaction for fields like token identifiers - Add comprehensive tests for audit detail masking, covering full redaction, partial masking, short values, plain fields, and nil input - Introduce OAuth user service tests for new user creation, existing connections, duplicate email handling, and auto-registration disabled behavior - Enable GORM error translation to surface duplicate key errors for clearer application-level error handling Signed-off-by: appleboy <appleboy.tw@gmail.com>
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
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.