Validate endpoint by domain name #499

RichardChen820 · 2023-11-21T09:00:49Z

Validate endpoint by domain name

jimmyca15 · 2023-11-21T16:28:37Z

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/ConfigurationClientManager.cs


-            if (_validDomains == null || !_validDomains.Any())
+            foreach (string domainPrefix in TrustedDomainPrefixes)


The search needs to be from right to left.

Seems to be fine since you look for segment prefixed with label separator.

I'd still feel better if we went from right to left and found exact label match because conceptually that's what we're going for.

Perhaps .LastIndexOf($".{label}.")

Got you, I will update

@jimmyca15 But we have a domain in such a pattern right? Like azconfig-test.io , so can we make exact label match? Or we list all labels we have (or potentially have in the future) and traverse them?

We'll have to think about azconfig-test.io. Most likely, we'll have to phase it out and adopt a proper .azconfig.xyz domain in ppe.

jimmyca15 · 2023-11-21T16:39:45Z

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/ConfigurationClientManager.cs

        private DateTimeOffset _lastFallbackClientRefresh = default;
        private DateTimeOffset _lastFallbackClientRefreshAttempt = default;
        private Logger _logger = new Logger();

        private static readonly TimeSpan FallbackClientRefreshExpireInterval = TimeSpan.FromHours(1);
        private static readonly TimeSpan MinimalClientRefreshInterval = TimeSpan.FromSeconds(30);
+        private static readonly string[] TrustedDomainPrefixes = new[] { ".azconfig", ".appconfig" };


It's not a prefix. azconfig and appconfig are both "label"s in the domain name.

jimmyca15

we should not have azconfig-test, appconfig-test.

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/ConfigurationClientManager.cs

* Validate endpoint by domain name * Resolve comments * Remove *-test

* Automatic http failover base on Srv Dns * update failover condition * Logging messages for auto failover * Not use generic method * Remove useless constructor * Failover to TCP when response is truncated * Error handling of TCP failover * Resolve comments and add thrid party notice * Fix the tests * Fix the tess * Fix the test * Update to use DnsClient.NET * Remove unecessary code * Resolve conflict * Stop once it has been auto failovered * Update GetAvaliableClient method * Resolve comments * Resolve comments * Internalize getting dynamic client in ConfigurationClientManager * Fix format * Fix format * Resolve comments * Resolve comments * Resolve comments * Resolve comments * Resolve comments * Resolve comments * Resolve comments * Not await RefreshFallbackClients when startup * Revert non-await RefreshFallbackClients * Validate domain of Srv endpoints (#489) * Validate domain of Srv endpoints * Resolve comments * Resolve comments * Create object in using * Add the origin host to the srv lookup result * Validate endpoint by domain name (#499) * Validate endpoint by domain name * Resolve comments * Remove *-test * Fix formatResolve conflict * Use await using for IAsyncDisposible * Replace IAsyncEumerable with ValueTask<IEnumerable> * Resolve comments * Remove cancallation token * Resolve comments

* Automatic http failover base on Srv Dns (#442) * Automatic http failover base on Srv Dns * update failover condition * Logging messages for auto failover * Not use generic method * Remove useless constructor * Failover to TCP when response is truncated * Error handling of TCP failover * Resolve comments and add thrid party notice * Fix the tests * Fix the tess * Fix the test * Update to use DnsClient.NET * Remove unecessary code * Resolve conflict * Stop once it has been auto failovered * Update GetAvaliableClient method * Resolve comments * Resolve comments * Internalize getting dynamic client in ConfigurationClientManager * Fix format * Fix format * Resolve comments * Resolve comments * Resolve comments * Resolve comments * Resolve comments * Resolve comments * Resolve comments * Not await RefreshFallbackClients when startup * Revert non-await RefreshFallbackClients * Validate domain of Srv endpoints (#489) * Validate domain of Srv endpoints * Resolve comments * Resolve comments * Create object in using * Add the origin host to the srv lookup result * Validate endpoint by domain name (#499) * Validate endpoint by domain name * Resolve comments * Remove *-test * Fix formatResolve conflict * Use await using for IAsyncDisposible * Replace IAsyncEumerable with ValueTask<IEnumerable> * Resolve comments * Remove cancallation token * Resolve comments * Refactor and move backoff logic to AzureAppConfigurationProvider (#508) * move backoff logic to provider, can't access backoffendtime from manager for dynamic clients * add condition before calling getclients in refreshasync, add refreshclients method * fix logic to check if backoff ended for client * fix indents * check client backoff before refresh, fix tests in progress * remove tolist * comment changes, fix refresh tests * reduce minimum backoff when testing * fix tests, fix typo with failover logic * update comments in tests * resolve PR comments * resolve small comments * fix tests backoff duration * fix tests backoff duration * fix error from merging main branch in * add back fix for attempts backoff calculation --------- Co-authored-by: Richard chen <99175581+RichardChen820@users.noreply.github.com>

Validate endpoint by domain name

1def465

jimmyca15 reviewed Nov 21, 2023

View reviewed changes

Resolve comments

e770eca

jimmyca15 approved these changes Nov 22, 2023

View reviewed changes

jimmyca15 requested changes Nov 22, 2023

View reviewed changes

jimmyca15 reviewed Nov 22, 2023

View reviewed changes

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/ConfigurationClientManager.cs Outdated Show resolved Hide resolved

Remove *-test

52802d4

jimmyca15 approved these changes Nov 28, 2023

View reviewed changes

RichardChen820 merged commit ab837f3 into user/junbchen/srvDnsClient Nov 28, 2023
2 checks passed

RichardChen820 deleted the user/junbchen/domainCheck branch November 28, 2023 02:32

RichardChen820 added a commit that referenced this pull request Nov 28, 2023

Validate endpoint by domain name (#499)

a959bf3

* Validate endpoint by domain name * Resolve comments * Remove *-test

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate endpoint by domain name #499

Validate endpoint by domain name #499

RichardChen820 commented Nov 21, 2023

jimmyca15 Nov 21, 2023

jimmyca15 Nov 21, 2023

jimmyca15 Nov 21, 2023 •

edited

Loading

jimmyca15 Nov 21, 2023

RichardChen820 Nov 22, 2023

RichardChen820 Nov 22, 2023 •

edited

Loading

jimmyca15 Nov 22, 2023

RichardChen820 Nov 22, 2023

jimmyca15 Nov 21, 2023

jimmyca15 left a comment


		if (_validDomains == null \|\| !_validDomains.Any())
		foreach (string domainPrefix in TrustedDomainPrefixes)

Validate endpoint by domain name #499

Validate endpoint by domain name #499

Conversation

RichardChen820 commented Nov 21, 2023

jimmyca15 Nov 21, 2023

Choose a reason for hiding this comment

jimmyca15 Nov 21, 2023

Choose a reason for hiding this comment

jimmyca15 Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

jimmyca15 Nov 21, 2023

Choose a reason for hiding this comment

RichardChen820 Nov 22, 2023

Choose a reason for hiding this comment

RichardChen820 Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

jimmyca15 Nov 22, 2023

Choose a reason for hiding this comment

RichardChen820 Nov 22, 2023

Choose a reason for hiding this comment

jimmyca15 Nov 21, 2023

Choose a reason for hiding this comment

jimmyca15 left a comment

Choose a reason for hiding this comment

jimmyca15 Nov 21, 2023 •

edited

Loading

RichardChen820 Nov 22, 2023 •

edited

Loading