Skip to content

fix: refactor authentication and audit logging internals#77

Merged
appleboy merged 2 commits intomainfrom
ra
Mar 7, 2026
Merged

fix: refactor authentication and audit logging internals#77
appleboy merged 2 commits intomainfrom
ra

Conversation

@appleboy
Copy link
Copy Markdown
Member

@appleboy appleboy commented Mar 6, 2026

  • Refactor audit logging to centralize context enrichment and record creation in a shared helper, reducing duplication between async and sync paths
  • Ensure audit log timestamps are consistent by reusing a single time value per record
  • Replace hardcoded client type strings with constants in authorization checks
  • Correct device code exchange to return access denied for invalid codes and update the related test expectation
  • Standardize token category and status values by replacing string literals with model constants throughout token handling
  • Extract shared logic for joining tokens with client data to remove duplication and reuse it for paginated and non-paginated queries
  • Centralize failed authentication audit logging into a helper method to simplify user authentication flow

- Refactor audit logging to centralize context enrichment and record creation in a shared helper, reducing duplication between async and sync paths
- Ensure audit log timestamps are consistent by reusing a single time value per record
- Replace hardcoded client type strings with constants in authorization checks
- Correct device code exchange to return access denied for invalid codes and update the related test expectation
- Standardize token category and status values by replacing string literals with model constants throughout token handling
- Extract shared logic for joining tokens with client data to remove duplication and reuse it for paginated and non-paginated queries
- Centralize failed authentication audit logging into a helper method to simplify user authentication flow

Signed-off-by: appleboy <appleboy.tw@gmail.com>
Copilot AI review requested due to automatic review settings March 6, 2026 16:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors authentication/audit/token internals to reduce duplication and standardize constants and error behavior across OAuth/device-code flows.

Changes:

  • Centralizes audit log enrichment/record creation and reuses a single timestamp per audit record.
  • Fixes device-code exchange to return ErrAccessDenied for invalid/non-existent device codes and updates the associated test.
  • Replaces token status/category string literals with models constants and extracts shared token→client enrichment logic.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/services/audit.go Introduces a shared audit-log builder to enrich entries from context and ensure consistent timestamps across fields.
internal/services/user.go Extracts failed-authentication audit logging into a helper to remove duplication in the auth flow.
internal/services/token.go Adjusts device-code invalid-code error mapping, standardizes token status/category constants, and deduplicates token+client join logic.
internal/services/token_test.go Updates device-code exchange test expectation for invalid device codes to ErrAccessDenied.
internal/services/authorization.go Replaces hardcoded client type strings with shared constants for authorization checks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 47.69231% with 34 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/services/user.go 25.00% 15 Missing ⚠️
internal/services/audit.go 0.00% 10 Missing ⚠️
internal/services/token.go 72.72% 8 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

- Reformat several functions for improved readability by splitting long signatures and method calls across multiple lines

Signed-off-by: appleboy <appleboy.tw@gmail.com>
@appleboy appleboy merged commit 6e1d1ed into main Mar 7, 2026
10 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants