Skip to content

Implement token caching with automatic renewal#289

Open
Noahnc wants to merge 11 commits into
shubhamranjan:mainfrom
Noahnc:main
Open

Implement token caching with automatic renewal#289
Noahnc wants to merge 11 commits into
shubhamranjan:mainfrom
Noahnc:main

Conversation

@Noahnc
Copy link
Copy Markdown

@Noahnc Noahnc commented Apr 20, 2026

Replace the current AuthenticationInterceptor with a wrapped HttpHandler that fetches tokens on-demand during requests.

This also fixes Issue 283 where the watch can no longer re-establish once the cached token has expired.

Please ensure your pull request adheres to the following guidelines:

  • Use the following format: * [owner/repo](link)
  • Link additions should be added to the bottom of the relevant category.
  • New categories or improvements to the existing categorization are welcome.
  • Search previous suggestions before making a new one, as yours may be a duplicate.
  • Sort by alphabetical order

Thanks for contributing!

Noahnc added 3 commits April 20, 2026 09:16
Replace the current AuthenticationInterceptor with a wrapped HttpHandler that fetches tokens on-demand during requests.
Invalidate token on credentials change
@Noahnc Noahnc marked this pull request as ready for review April 20, 2026 07:59
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

This PR reworks how EtcdClient applies authentication by moving from a gRPC Interceptor that reads a cached token to an HttpMessageHandler that fetches and caches tokens on-demand, enabling automatic renewal during reconnects (notably for watch streams).

Changes:

  • Replace AuthenticationInterceptor token injection with a new AuthenticationHttpHandler that fetches/caches tokens and bypasses the Authenticate RPC to avoid recursion.
  • Update EtcdClient authentication APIs to support token cache duration configuration and credential clearing/invalidation.
  • Replace unit tests for the removed interceptor with new unit tests for the handler; adjust integration tests and update auth docs.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
dotnet-etcd/etcdClient.cs Installs AuthenticationHttpHandler, adds credential/token-cache configuration and lazy token acquisition via RequestTokenAsync.
dotnet-etcd/AuthenticationInterceptor.cs Removes the old interceptor-based auth approach.
dotnet-etcd/AuthenticationHttpHandler.cs Introduces a delegating handler to attach, cache, and refresh auth tokens on outgoing requests.
dotnet-etcd.Tests/Unit/AuthenticationInterceptorTests.cs Removes tests for the deleted interceptor.
dotnet-etcd.Tests/Unit/AuthenticationHttpHandlerTests.cs Adds unit coverage for header injection, caching, invalidation, and concurrency behavior in the new handler.
dotnet-etcd.Tests/Integration/AuthClientIntegrationTests.cs Minor updates and ensures credentials can be cleared during auth setup/teardown flows.
docs/authentication/api-reference.md Documents the new token cache duration parameter and updated credential behavior.

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

Comment thread dotnet-etcd/etcdClient.cs Outdated
Comment thread dotnet-etcd/etcdClient.cs Outdated
Comment thread dotnet-etcd/etcdClient.cs
Comment thread dotnet-etcd/etcdClient.cs Outdated
Comment thread dotnet-etcd/AuthenticationHttpHandler.cs
Comment thread dotnet-etcd/AuthenticationHttpHandler.cs Outdated
Comment thread docs/authentication/api-reference.md Outdated
Comment thread docs/authentication/api-reference.md Outdated
Copy link
Copy Markdown
Owner

@shubhamranjan shubhamranjan left a comment

Choose a reason for hiding this comment

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

Thank you for this feature. Please look into the failing tests. (Try rebasing first)

Noahnc and others added 5 commits May 4, 2026 08:26
Store credentials in record instead of struct to prevent torn read / write race condition.
Store cached token with expiration in record to prevent race condition during read / write.
@Noahnc Noahnc requested a review from shubhamranjan May 4, 2026 06:48
Overwrite existing authorization header.
@Noahnc
Copy link
Copy Markdown
Author

Noahnc commented May 4, 2026

@shubhamranjan The failing integration test seems to be unrelated to my change and was fixed in PR-293. I just merged the fix into this branch.
I also addressed the issues found by Copilot.

@shubhamranjan
Copy link
Copy Markdown
Owner

cool, thank you. I will test this branch myself locally to see if all is fine.

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.

3 participants