-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[INNO] - Passkey Directory Endpoint #7317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
01ac04b
c3b1a28
0d4dfd8
1e90cc8
0595a10
b4bfc99
5cf1c66
f73e5ab
7a24848
990b9ee
6cf6ebd
d6704a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| using Bit.Core.Dirt.Reports.ReportFeatures.OrganizationReportMembers.Interfaces; | ||
| using Bit.Core.Dirt.Reports.ReportFeatures.Requests; | ||
| using Bit.Core.Exceptions; | ||
| using Bit.Core.Utilities; | ||
| using Microsoft.AspNetCore.Authorization; | ||
| using Microsoft.AspNetCore.Mvc; | ||
|
|
||
|
|
@@ -24,8 +25,7 @@ public class ReportsController : Controller | |
| private readonly IAddPasswordHealthReportApplicationCommand _addPwdHealthReportAppCommand; | ||
| private readonly IGetPasswordHealthReportApplicationQuery _getPwdHealthReportAppQuery; | ||
| private readonly IDropPasswordHealthReportApplicationCommand _dropPwdHealthReportAppCommand; | ||
| private readonly IAddOrganizationReportCommand _addOrganizationReportCommand; | ||
| private readonly IGetOrganizationReportQuery _getOrganizationReportQuery; | ||
| private readonly IGetPasskeyDirectoryQuery _getPasskeyDirectoryQuery; | ||
| private readonly ILogger<ReportsController> _logger; | ||
|
|
||
| public ReportsController( | ||
|
|
@@ -35,8 +35,7 @@ public ReportsController( | |
| IAddPasswordHealthReportApplicationCommand addPasswordHealthReportApplicationCommand, | ||
| IGetPasswordHealthReportApplicationQuery getPasswordHealthReportApplicationQuery, | ||
| IDropPasswordHealthReportApplicationCommand dropPwdHealthReportAppCommand, | ||
| IGetOrganizationReportQuery getOrganizationReportQuery, | ||
| IAddOrganizationReportCommand addOrganizationReportCommand, | ||
| IGetPasskeyDirectoryQuery getPasskeyDirectoryQuery, | ||
| ILogger<ReportsController> logger | ||
| ) | ||
| { | ||
|
|
@@ -46,8 +45,7 @@ ILogger<ReportsController> logger | |
| _addPwdHealthReportAppCommand = addPasswordHealthReportApplicationCommand; | ||
| _getPwdHealthReportAppQuery = getPasswordHealthReportApplicationQuery; | ||
| _dropPwdHealthReportAppCommand = dropPwdHealthReportAppCommand; | ||
| _getOrganizationReportQuery = getOrganizationReportQuery; | ||
| _addOrganizationReportCommand = addOrganizationReportCommand; | ||
| _getPasskeyDirectoryQuery = getPasskeyDirectoryQuery; | ||
| _logger = logger; | ||
| } | ||
|
|
||
|
|
@@ -206,4 +204,22 @@ public async Task DropPasswordHealthReportApplication( | |
|
|
||
| await _dropPwdHealthReportAppCommand.DropPasswordHealthReportApplicationAsync(request); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the list of domains that support passkeys from the 2FA Directory | ||
| /// </summary> | ||
| /// <returns>List of domains with passkey support details</returns> | ||
| [HttpGet("passkey-directory")] | ||
| [RequireFeature(FeatureFlagKeys.PasskeyDirectoryReport)] | ||
| public async Task<IEnumerable<PasskeyDirectoryResponseModel>> GetPasskeyDirectoryAsync() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. β QUESTION: No DetailsEvery other endpoint in The returned data originates from a public API, so there may be no confidentiality concern. However, the PR description states this powers a report for "organization administrators," and placing the endpoint in this controller without the same authorization pattern could be an intentional choice or an oversight. Was the decision to skip |
||
| { | ||
| var entries = await _getPasskeyDirectoryQuery.GetPasskeyDirectoryAsync(); | ||
| return entries.Select(e => new PasskeyDirectoryResponseModel | ||
| { | ||
| DomainName = e.DomainName, | ||
| Passwordless = e.Passwordless, | ||
| Mfa = e.Mfa, | ||
| Instructions = e.Instructions | ||
| }); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| ο»Ώnamespace Bit.Api.Dirt.Models.Response; | ||
|
|
||
| public class PasskeyDirectoryResponseModel | ||
| { | ||
| public string DomainName { get; set; } = string.Empty; | ||
| public bool Passwordless { get; set; } | ||
| public bool Mfa { get; set; } | ||
| public string Instructions { get; set; } = string.Empty; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| ο»Ώnamespace Bit.Core.Dirt.Reports.Models.Data; | ||
|
|
||
| public class PasskeyDirectoryEntry | ||
| { | ||
| public string DomainName { get; set; } = string.Empty; | ||
| public bool Passwordless { get; set; } | ||
| public bool Mfa { get; set; } | ||
| public string Instructions { get; set; } = string.Empty; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| ο»Ώusing System.Text.Json; | ||
| using Bit.Core.Dirt.Reports.Models.Data; | ||
| using Bit.Core.Dirt.Reports.ReportFeatures.Interfaces; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Microsoft.Extensions.Logging; | ||
| using ZiggyCreatures.Caching.Fusion; | ||
|
|
||
| namespace Bit.Core.Dirt.Reports.ReportFeatures; | ||
|
|
||
| public class GetPasskeyDirectoryQuery( | ||
| IHttpClientFactory httpClientFactory, | ||
| [FromKeyedServices(GetPasskeyDirectoryQuery.CacheName)] | ||
| IFusionCache cache, | ||
| ILogger<GetPasskeyDirectoryQuery> logger) | ||
| : IGetPasskeyDirectoryQuery | ||
| { | ||
| public const string HttpClientName = "PasskeyDirectoryHttpClient"; | ||
| public const string CacheName = "PasskeyDirectory"; | ||
|
|
||
| private static readonly TimeSpan _cacheDuration = TimeSpan.FromDays(1); | ||
| private const string _cacheKey = "passkey-directory"; | ||
| private const string _passkeyDirectoryUrl = "https://passkeys-api.2fa.directory/v1/all.json"; | ||
|
|
||
| private readonly HttpClient _httpClient = httpClientFactory.CreateClient(HttpClientName); | ||
|
|
||
| public async Task<IEnumerable<PasskeyDirectoryEntry>> GetPasskeyDirectoryAsync() | ||
| { | ||
| var entries = await cache.GetOrSetAsync( | ||
| key: _cacheKey, | ||
| factory: async _ => await FetchPasskeyDirectoryAsync(), | ||
| options: new FusionCacheEntryOptions(duration: _cacheDuration) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π¨ SUGGESTED: Passing a bare Details and fix
Constructing The repo's var entries = await cache.GetOrSetAsync(
key: _cacheKey,
factory: async _ => await FetchPasskeyDirectoryAsync(),
options => options.SetDuration(_cacheDuration)
);This keeps the 24h duration while preserving fail-safe and factory timeouts. Reference: |
||
| ); | ||
|
|
||
| return entries; | ||
| } | ||
|
|
||
| private async Task<List<PasskeyDirectoryEntry>> FetchPasskeyDirectoryAsync() | ||
| { | ||
| logger.LogInformation(Constants.BypassFiltersEventId, | ||
| "Fetching passkey directory from external API"); | ||
|
|
||
| var response = await _httpClient.GetAsync(_passkeyDirectoryUrl); | ||
| response.EnsureSuccessStatusCode(); | ||
|
|
||
| await using var stream = await response.Content.ReadAsStreamAsync(); | ||
| var directory = await JsonSerializer.DeserializeAsync<Dictionary<string, JsonElement>>(stream); | ||
|
|
||
| if (directory is null) | ||
| { | ||
| return []; | ||
| } | ||
|
|
||
| var entries = new List<PasskeyDirectoryEntry>(); | ||
|
|
||
| foreach (var (domain, serviceData) in directory) | ||
| { | ||
| var hasPasswordless = serviceData.TryGetProperty("passwordless", out var passwordlessElement) | ||
| && passwordlessElement.ValueKind == JsonValueKind.String; | ||
| var hasMfa = serviceData.TryGetProperty("mfa", out var mfaElement) | ||
| && mfaElement.ValueKind == JsonValueKind.String; | ||
|
|
||
| if (!hasPasswordless && !hasMfa) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| var instructions = serviceData.TryGetProperty("documentation", out var docElement) | ||
| && docElement.ValueKind == JsonValueKind.String | ||
| ? docElement.GetString() ?? string.Empty | ||
| : string.Empty; | ||
|
|
||
| entries.Add(new PasskeyDirectoryEntry | ||
| { | ||
| DomainName = domain, | ||
| Passwordless = hasPasswordless, | ||
| Mfa = hasMfa, | ||
| Instructions = instructions | ||
| }); | ||
| } | ||
|
|
||
| logger.LogInformation(Constants.BypassFiltersEventId, | ||
| "Fetched {Count} passkey directory entries from external API", entries.Count); | ||
|
|
||
| return entries; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| ο»Ώusing Bit.Core.Dirt.Reports.Models.Data; | ||
|
|
||
| namespace Bit.Core.Dirt.Reports.ReportFeatures.Interfaces; | ||
|
|
||
| public interface IGetPasskeyDirectoryQuery | ||
| { | ||
| /// <summary> | ||
| /// Passkey directory data from the cache or source. | ||
| /// </summary> | ||
| /// <returns> | ||
| /// Enumerable response with entries each representing | ||
| /// a passkey directory entry with domain name, passwordless and MFA support, and | ||
| /// associated instructions. These are domains that may potentially support passkeys for either | ||
| /// Login or Mutli-Factor Authentication. These are matched up with ciphers client-side with link to documentation | ||
| /// to use passkeys. | ||
| /// </returns> | ||
| Task<IEnumerable<PasskeyDirectoryEntry>> GetPasskeyDirectoryAsync(); | ||
| } |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not critical: Where should this README live/is it appropriate to include here? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| # Passkey Directory Report | ||
|
|
||
| ## Overview | ||
|
|
||
| The Passkey Directory Report provides a list of domains that support passkeys, sourced from the [2FA Directory API](https://2fa.directory). This data powers a report in the Bitwarden client that helps organization administrators understand which of their members' credentials could be upgraded to passkeys. | ||
|
|
||
| For client-side implementation details, see the [clients README](https://github.com/bitwarden/clients/blob/d866c8126444bf95f2be2ee5f59646aa1237e8a7/apps/web/src/app/dirt/reports/pages/README.md). | ||
|
|
||
| ## Feature Flag | ||
|
|
||
| This feature is gated behind the `PasskeyDirectoryReport` feature flag. | ||
|
|
||
| ## Architecture | ||
|
|
||
| ### Data Flow | ||
|
|
||
| ``` | ||
| 2FA Directory API --> GetPasskeyDirectoryQuery (cached 24h) --> ReportsController --> Client | ||
| ``` | ||
|
|
||
| 1. **External source**: The [2FA Directory v1 API](https://passkeys-api.2fa.directory/v1/all.json) provides a JSON dictionary of domains and their passkey/MFA support. | ||
| 2. **Query layer** (`GetPasskeyDirectoryQuery`): Fetches and parses the external data, caching results for 24 hours via FusionCache. | ||
| 3. **API endpoint** (`ReportsController`): Exposes `GET /reports/passkey-directory` which returns the cached directory entries. | ||
|
|
||
| ### Key Files | ||
|
|
||
| | File | Purpose | | ||
| |------|---------| | ||
| | `GetPasskeyDirectoryQuery.cs` | Core query β fetches, parses, and caches the 2FA Directory data | | ||
| | `Interfaces/IGetPasskeyDirectoryQuery.cs` | Query interface | | ||
| | `ReportingServiceCollectionExtensions.cs` | DI registration for the query, HTTP client, and cache | | ||
| | `../../Models/Data/PasskeyDirectoryEntry.cs` | Domain model for a directory entry | | ||
| | `src/Api/Dirt/Controllers/ReportsController.cs` | API controller exposing the endpoint | | ||
| | `src/Api/Dirt/Models/Response/PasskeyDirectoryResponseModel.cs` | API response model | | ||
|
|
||
| ### Caching | ||
|
|
||
| - **Provider**: FusionCache (keyed service `"PasskeyDirectory"`) | ||
| - **Duration**: 24 hours | ||
| - **Key**: `"passkey-directory"` | ||
| - Cache is registered in `ReportingServiceCollectionExtensions.AddReportingServices()`. | ||
|
|
||
| ### Response Shape | ||
|
|
||
| Each entry in the response array contains: | ||
|
|
||
| | Field | Type | Description | | ||
| |-------|------|-------------| | ||
| | `domainName` | `string` | The domain (e.g. `github.com`) | | ||
| | `passwordless` | `bool` | Whether the domain supports passwordless passkey login | | ||
| | `mfa` | `bool` | Whether the domain supports passkeys as an MFA method | | ||
| | `instructions` | `string` | URL to setup documentation (empty if unavailable) | | ||
|
|
||
| ### API Endpoint | ||
|
|
||
| ``` | ||
| GET /reports/passkey-directory | ||
| ``` | ||
|
|
||
| - **Auth**: Standard Bitwarden authentication | ||
| - **Feature flag**: `PasskeyDirectoryReport` | ||
| - **Response**: `IEnumerable<PasskeyDirectoryResponseModel>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not critical: instead of placing the endpoint under
/reports/, I'm wondering if we want to consolidate endpoints that call third party APIs elsewhere. They could share a controller with a base route like/externalor similar.Alternatively, we could follow what was done for HIBP and just introduce a new controller for 2FA directory API proxy requests, like
/2fa-directory/.It is worth noting that the Inactive 2FA report in the web client has direct calls to the 2FA directory API still, which should also be refactored to make requests through our API like this.