Conversation
- closes #1001
|
We will need to add the new scopes to Auth0. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1010 +/- ##
==========================================
+ Coverage 71.62% 71.86% +0.24%
==========================================
Files 370 382 +12
Lines 20287 20869 +582
Branches 2686 2777 +91
==========================================
+ Hits 14530 14997 +467
- Misses 4692 4765 +73
- Partials 1065 1107 +42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
pmachapman
left a comment
There was a problem hiding this comment.
@pmachapman reviewed 31 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on ddaspit and Enkidu93).
src/Serval/src/Serval.Client/Client.g.cs line 492 at r1 (raw file):
[System.CodeDom.Compiler.GeneratedCode("NSwag", "14.7.1.0 (NJsonSchema v11.6.1.0 (Newtonsoft.Json v13.0.0.0))")] public partial interface IApiKeysClient
Do we want to expose the API keys API in the Serval Client given regular users of Serval will not be utilizing this API?
Code quote:
public partial interface IApiKeysClientsrc/Serval/src/Serval.ApiKeys/Features/ApiKeys/DeleteApiKey.cs line 9 at r1 (raw file):
public async Task HandleAsync(DeleteApiKey request, CancellationToken cancellationToken = default) { ApiKey? apiKey = await apiKeys.DeleteAsync(request.ApiKeyId, cancellationToken);
Instead of deleting outright, you could add a RevokedAt field, similar to ExpiresAt. This means that we can log use of revoked keys, as opposed to a client just using an incorrect key. Up to you though if you want to go down this path.
Code quote:
ApiKey? apiKey = await apiKeys.DeleteAsync(request.ApiKeyId, cancellationToken);src/Serval/src/Serval.ApiKeys/Serval.ApiKeys.csproj line 16 at r1 (raw file):
<ItemGroup> <PackageReference Include="Asp.Versioning.Abstractions" Version="8.1.0" />
This should be version 10.0.0 (the version we use elsewhere). I think also the versions in the test project are out of date compared to the versions used in other projects.
Code quote:
<PackageReference Include="Asp.Versioning.Abstractions" Version="8.1.0" />src/Serval/src/Serval.ApiKeys/Services/ApiKeyService.cs line 26 at r1 (raw file):
Owner = owner, Name = name, HashedKey = HashKey(key),
Do we want to salt the keys? We don't have to but it is easier to do now than later.
Code quote:
HashedKey = HashKey(key),
This change is