Skip to content

Add support for API key auth - #1010

Open
ddaspit wants to merge 1 commit into
mainfrom
api-key
Open

Add support for API key auth#1010
ddaspit wants to merge 1 commit into
mainfrom
api-key

Conversation

@ddaspit

@ddaspit ddaspit commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

@ddaspit

ddaspit commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

We will need to add the new scopes to Auth0.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.75891% with 131 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.86%. Comparing base (db0513a) to head (2eb481e).

Files with missing lines Patch % Lines
src/Serval/src/Serval.Client/Client.g.cs 65.65% 78 Missing and 35 partials ⚠️
...rc/Serval.ApiKeys/Features/ApiKeys/CreateApiKey.cs 81.81% 3 Missing and 5 partials ⚠️
src/Serval/src/Serval.ApiServer/Startup.cs 81.08% 7 Missing ⚠️
...al.ApiKeys/Services/ApiKeyAuthenticationHandler.cs 88.23% 1 Missing and 1 partial ⚠️
...erval/src/Serval.ApiKeys/Services/ApiKeyService.cs 98.30% 0 Missing and 1 partial ⚠️
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.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Enkidu93
Enkidu93 requested review from Enkidu93 and pmachapman July 29, 2026 19:57

@pmachapman pmachapman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@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 IApiKeysClient

src/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),

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.

Add support for authenticating by API key (or something similar)

3 participants