Add resource service auth tests (Unsecured, ApiKey) #3824
Conversation
Adds tests for `Unsecured` and `ApiKey` auth modes for the resource service, from the perspective of the dashboard client in the dashboard project. Also: - Work around race condition between cancellation and connection attempts that leads to a previously-unhandled `ObjectDisposedException`. - Move resource service code into own namespace, out of model. - Use a more specific namespace for generated protobuf code.
| using Xunit; | ||
| using DashboardServiceBase = Aspire.ResourceService.Proto.V1.DashboardService.DashboardServiceBase; | ||
|
|
||
| namespace Aspire.Dashboard.Tests.Integration; |
There was a problem hiding this comment.
These tests are ok and don't need to be rewritten, but I think there is a simpler implementation. You could have added support for passing a custom HttpMessageHandler to the resource client, then capture the outgoing request when it is sent to the handler and returned a response from the handler. Then assert the outgoing request has headers expected. That saves having a mock server. The integration tests could turn into unit tests.
On the other hand, binary gRPC request/responses aren't easily approachable when working at a bytes level like a message handler does.
| private const string ApiKeyHeaderName = "x-resource-service-api-key"; | ||
|
|
||
| [Fact] | ||
| public async Task ConnectsToResourceService_Unsecured() |
There was a problem hiding this comment.
Should there be HTTPS versions of these tests?
There was a problem hiding this comment.
On that topic, I thought there wasn't much point in testing client cert auth, since it was just adding a certificate to the handler, but I looked through the resource client code and certificate loading happens inside the client.
I think we either need to test the end-to-end with cert auth or split the cert loading out into a separate type and test loading cert from configuration independently.
There was a problem hiding this comment.
Should there be HTTPS versions of these tests?
|
Presumably this would be good in the 8.0 branch too when it's ready. |
drewnoakes
changed the title
Contributes to #3200
Adds tests for
UnsecuredandApiKeyauth modes for the resource service, from the perspective of the dashboard client in the dashboard project.Also:
ObjectDisposedException.Microsoft Reviewers: Open in CodeFlow