Skip to content

fix: notifications - cache token to avoid auth failure on repeated calls#273

Merged
mgoetzegb merged 6 commits intomainfrom
VTI-775-fix-notification-client
Dec 18, 2025
Merged

fix: notifications - cache token to avoid auth failure on repeated calls#273
mgoetzegb merged 6 commits intomainfrom
VTI-775-fix-notification-client

Conversation

@mgoetzegb
Copy link
Copy Markdown
Member

@mgoetzegb mgoetzegb commented Dec 11, 2025

What

fix: notifications - cache token to avoid auth failure on repeated calls

Why

Previously we were retrieving the acess token from Keycloak for each request to the notification service.

This is not efficient and furthermore keycloak deactivates the user temporarily when it detects an excessing amount of logins.

Moved the authentication to a separate client as it can be useful on its own in other scenarios. This is a breaking change in the notification client initialization.

References

VTI-775

Checklist

  • Tests

@mgoetzegb mgoetzegb requested review from a team as code owners December 11, 2025 10:07
@greenbonebot

This comment was marked as resolved.

@github-actions
Copy link
Copy Markdown

Conventional Commits Report

Type Number
Bug Fixes 1

🚀 Conventional commits found.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 11, 2025

Codecov Report

❌ Patch coverage is 82.45614% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.66%. Comparing base (236ac1c) to head (dba7b37).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/auth/auth_client.go 80.76% 5 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #273      +/-   ##
==========================================
+ Coverage   57.36%   57.66%   +0.29%     
==========================================
  Files          68       69       +1     
  Lines        3781     3812      +31     
==========================================
+ Hits         2169     2198      +29     
- Misses       1432     1433       +1     
- Partials      180      181       +1     
Flag Coverage Δ
opensearch-tests 95.66% <ø> (ø)
postgres-tests 91.96% <ø> (ø)
unit-tests 52.15% <82.45%> (+0.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mgoetzegb mgoetzegb added the minor release Set label to create a minor release label Dec 11, 2025
Comment thread pkg/auth/auth_client.go
Comment thread pkg/auth/auth_client.go
Comment thread pkg/auth/auth_client.go Outdated
Comment thread pkg/auth/auth_client.go
Comment thread pkg/auth/auth_client.go Outdated
Comment thread pkg/notifications/notification_test.go Outdated
Comment thread pkg/notifications/README.md
Copy link
Copy Markdown
Member Author

@mgoetzegb mgoetzegb left a comment

Choose a reason for hiding this comment

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

Addressed all comments.

Comment thread pkg/notifications/notification_test.go Outdated
Comment thread pkg/notifications/README.md
Comment thread pkg/auth/auth_client.go
Comment thread pkg/auth/auth_client.go Outdated
Comment thread pkg/auth/auth_client.go
Comment thread pkg/auth/auth_client.go
Comment thread pkg/auth/auth_client.go Outdated
alex-heine
alex-heine previously approved these changes Dec 16, 2025
Comment thread pkg/auth/auth_client.go
Comment thread pkg/auth/auth_client.go Outdated
Comment thread pkg/auth/auth_client.go Outdated
Comment thread pkg/auth/auth_client.go Outdated
Comment thread pkg/auth/auth_client.go
Comment thread pkg/auth/auth_client.go Outdated
Comment thread pkg/auth/auth_client.go Outdated
Comment thread pkg/auth/auth_client.go Outdated
Comment thread pkg/auth/auth_client.go Outdated
Comment thread pkg/auth/auth_client_test.go Outdated
Comment thread pkg/auth/auth_client_test.go Outdated
Comment thread pkg/notifications/notification_test.go Outdated
Previously we were retrieving the acess token from Keycloak for each request to the notification service.

This is not efficient and furthermore keycloak deactivates the user temporarily when it detects an excessing amount of logins.

Moved the authentication to a separate client as it can be useful on its own in other scenarios. This is a breaking change in the notification client initialization.
@mgoetzegb mgoetzegb force-pushed the VTI-775-fix-notification-client branch from 9e44526 to dba7b37 Compare December 18, 2025 13:19
@mgoetzegb mgoetzegb enabled auto-merge (squash) December 18, 2025 13:20
@mgoetzegb mgoetzegb merged commit 1c5385d into main Dec 18, 2025
12 checks passed
@mgoetzegb mgoetzegb deleted the VTI-775-fix-notification-client branch December 18, 2025 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor release Set label to create a minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants