Skip to content
This repository was archived by the owner on Jul 15, 2023. It is now read-only.

Commit 7c4e67d

Browse files
authored
Merge pull request #790 from jeschu1/removeunneededcall2
devops: Remove HEAD request when not required
2 parents 51b3c73 + b316ea5 commit 7c4e67d

File tree

2 files changed

+112
-112
lines changed

2 files changed

+112
-112
lines changed

AzureDevOps.Authentication/Src/Authentication.cs

+103-103
Original file line numberDiff line numberDiff line change
@@ -177,148 +177,148 @@ public override async Task<bool> DeleteCredentials(TargetUri targetUri)
177177
if (targetUri is null)
178178
throw new ArgumentNullException(nameof(targetUri));
179179

180-
// Assume Azure DevOps using Azure "common tenant" (empty GUID).
181-
var tenantId = Guid.Empty;
182-
183-
// Compose the request Uri, by default it is the target Uri.
184-
var requestUri = targetUri;
185-
186-
// Override the request Uri, when actual Uri exists, with actual Uri.
187-
if (targetUri.ActualUri != null)
180+
if (IsAzureDevOpsUrl(targetUri))
188181
{
189-
requestUri = targetUri.CreateWith(queryUri: targetUri.ActualUri);
190-
}
182+
// Assume Azure DevOps using Azure "common tenant" (empty GUID).
183+
var tenantId = Guid.Empty;
191184

192-
// If the protocol (aka scheme) being used isn't HTTP based, there's no point in
193-
// querying the server, so skip that work.
194-
if (OrdinalIgnoreCase.Equals(requestUri.Scheme, Uri.UriSchemeHttp)
195-
|| OrdinalIgnoreCase.Equals(requestUri.Scheme, Uri.UriSchemeHttps))
196-
{
197-
var requestUrl = GetTargetUrl(requestUri, false);
185+
// Compose the request Uri, by default it is the target Uri.
186+
var requestUri = targetUri;
198187

199-
// Read the cache from disk.
200-
var cache = await DeserializeTenantCache(context);
188+
// Override the request Uri, when actual Uri exists, with actual Uri.
189+
if (targetUri.ActualUri != null)
190+
{
191+
requestUri = targetUri.CreateWith(queryUri: targetUri.ActualUri);
192+
}
201193

202-
// Check the cache for an existing value.
203-
if (cache.TryGetValue(requestUrl, out tenantId))
194+
// If the protocol (aka scheme) being used isn't HTTP based, there's no point in
195+
// querying the server, so skip that work.
196+
if (OrdinalIgnoreCase.Equals(requestUri.Scheme, Uri.UriSchemeHttp)
197+
|| OrdinalIgnoreCase.Equals(requestUri.Scheme, Uri.UriSchemeHttps))
204198
{
205-
context.Trace.WriteLine($"'{requestUrl}' is Azure DevOps, tenant resource is {{{tenantId.ToString("N")}}}.");
199+
var requestUrl = GetTargetUrl(requestUri, false);
206200

207-
return tenantId;
208-
}
201+
// Read the cache from disk.
202+
var cache = await DeserializeTenantCache(context);
209203

210-
// Use the properly formatted URL
211-
requestUri = requestUri.CreateWith(queryUrl: requestUrl);
204+
// Check the cache for an existing value.
205+
if (cache.TryGetValue(requestUrl, out tenantId))
206+
{
207+
context.Trace.WriteLine($"'{requestUrl}' is Azure DevOps, tenant resource is {{{tenantId.ToString("N")}}}.");
212208

213-
var options = new NetworkRequestOptions(false)
214-
{
215-
Flags = NetworkRequestOptionFlags.UseProxy,
216-
Timeout = TimeSpan.FromMilliseconds(Global.RequestTimeout),
217-
};
209+
return tenantId;
210+
}
218211

219-
try
220-
{
221-
// Query the host use the response headers to determine if the host is Azure DevOps or not.
222-
using (var response = await context.Network.HttpHeadAsync(requestUri, options))
212+
// Use the properly formatted URL
213+
requestUri = requestUri.CreateWith(queryUrl: requestUrl);
214+
215+
var options = new NetworkRequestOptions(false)
223216
{
224-
if (response.Headers != null)
217+
Flags = NetworkRequestOptionFlags.UseProxy,
218+
Timeout = TimeSpan.FromMilliseconds(Global.RequestTimeout),
219+
};
220+
221+
try
222+
{
223+
// Query the host use the response headers to determine if the host is Azure DevOps or not.
224+
using (var response = await context.Network.HttpHeadAsync(requestUri, options))
225225
{
226-
// If the "X-VSS-ResourceTenant" was returned, then it is Azure DevOps and we'll need it's value.
227-
if (response.Headers.TryGetValues(XvssResourceTenantHeader, out IEnumerable<string> values))
226+
if (response.Headers != null)
228227
{
229-
context.Trace.WriteLine($"detected '{requestUrl}' as Azure DevOps from GET response.");
230-
231-
// The "Www-Authenticate" is a more reliable header, because it indicates the
232-
// authentication scheme that should be used to access the requested entity.
233-
if (response.Headers.WwwAuthenticate != null)
228+
// If the "X-VSS-ResourceTenant" was returned, then it is Azure DevOps and we'll need it's value.
229+
if (response.Headers.TryGetValues(XvssResourceTenantHeader, out IEnumerable<string> values))
234230
{
235-
foreach (var header in response.Headers.WwwAuthenticate)
236-
{
237-
const string AuthorizationUriPrefix = "authorization_uri=";
238-
239-
var value = header.Parameter;
231+
context.Trace.WriteLine($"detected '{requestUrl}' as Azure DevOps from GET response.");
240232

241-
if (value?.Length >= AuthorizationUriPrefix.Length + AuthorityHostUrlBase.Length + GuidStringLength)
233+
// The "Www-Authenticate" is a more reliable header, because it indicates the
234+
// authentication scheme that should be used to access the requested entity.
235+
if (response.Headers.WwwAuthenticate != null)
236+
{
237+
foreach (var header in response.Headers.WwwAuthenticate)
242238
{
243-
// The header parameter will look something like "authorization_uri=https://login.microsoftonline.com/72f988bf-86f1-41af-91ab-2d7cd011db47"
244-
// and all we want is the portion after the '=' and before the last '/'.
245-
int index1 = value.IndexOf('=', AuthorizationUriPrefix.Length - 1);
246-
int index2 = value.LastIndexOf('/');
239+
const string AuthorizationUriPrefix = "authorization_uri=";
247240

248-
// Parse the header value if the necessary characters exist...
249-
if (index1 > 0 && index2 > index1)
241+
var value = header.Parameter;
242+
243+
if (value?.Length >= AuthorizationUriPrefix.Length + AuthorityHostUrlBase.Length + GuidStringLength)
250244
{
251-
var authorityUrl = value.Substring(index1 + 1, index2 - index1 - 1);
252-
var guidString = value.Substring(index2 + 1, GuidStringLength);
245+
// The header parameter will look something like "authorization_uri=https://login.microsoftonline.com/72f988bf-86f1-41af-91ab-2d7cd011db47"
246+
// and all we want is the portion after the '=' and before the last '/'.
247+
int index1 = value.IndexOf('=', AuthorizationUriPrefix.Length - 1);
248+
int index2 = value.LastIndexOf('/');
253249

254-
// If the authority URL is as expected, attempt to parse the tenant resource identity.
255-
if (OrdinalIgnoreCase.Equals(authorityUrl, AuthorityHostUrlBase)
256-
&& Guid.TryParse(guidString, out tenantId))
250+
// Parse the header value if the necessary characters exist...
251+
if (index1 > 0 && index2 > index1)
257252
{
258-
// Update the cache.
259-
cache[requestUrl] = tenantId;
260-
261-
// Write the cache to disk.
262-
await SerializeTenantCache(context, cache);
263-
264-
// Since we found a value, break the loop (likely a loop of one item anyways).
265-
break;
253+
var authorityUrl = value.Substring(index1 + 1, index2 - index1 - 1);
254+
var guidString = value.Substring(index2 + 1, GuidStringLength);
255+
256+
// If the authority URL is as expected, attempt to parse the tenant resource identity.
257+
if (OrdinalIgnoreCase.Equals(authorityUrl, AuthorityHostUrlBase)
258+
&& Guid.TryParse(guidString, out tenantId))
259+
{
260+
// Update the cache.
261+
cache[requestUrl] = tenantId;
262+
263+
// Write the cache to disk.
264+
await SerializeTenantCache(context, cache);
265+
266+
// Since we found a value, break the loop (likely a loop of one item anyways).
267+
break;
268+
}
266269
}
267270
}
268271
}
269272
}
270-
}
271-
else
272-
{
273-
// Since there wasn't a "Www-Authenticate" header returned
274-
// iterate through the values, taking the first non-zero value.
275-
foreach (string value in values)
273+
else
276274
{
277-
// Try to find a value for the resource-tenant identity.
278-
// Given that some projects will return multiple tenant identities,
279-
if (!string.IsNullOrWhiteSpace(value)
280-
&& Guid.TryParse(value, out tenantId))
275+
// Since there wasn't a "Www-Authenticate" header returned
276+
// iterate through the values, taking the first non-zero value.
277+
foreach (string value in values)
281278
{
282-
// Update the cache.
283-
cache[requestUrl] = tenantId;
279+
// Try to find a value for the resource-tenant identity.
280+
// Given that some projects will return multiple tenant identities,
281+
if (!string.IsNullOrWhiteSpace(value)
282+
&& Guid.TryParse(value, out tenantId))
283+
{
284+
// Update the cache.
285+
cache[requestUrl] = tenantId;
284286

285-
// Write the cache to disk.
286-
await SerializeTenantCache(context, cache);
287+
// Write the cache to disk.
288+
await SerializeTenantCache(context, cache);
287289

288-
// Break the loop if a non-zero value has been detected.
289-
if (tenantId != Guid.Empty)
290-
{
291-
break;
290+
// Break the loop if a non-zero value has been detected.
291+
if (tenantId != Guid.Empty)
292+
{
293+
break;
294+
}
292295
}
293296
}
294297
}
295-
}
296298

297-
context.Trace.WriteLine($"tenant resource for '{requestUrl}' is {{{tenantId.ToString("N")}}}.");
299+
context.Trace.WriteLine($"tenant resource for '{requestUrl}' is {{{tenantId.ToString("N")}}}.");
298300

299-
// Return the tenant identity to the caller because this is Azure DevOps.
300-
return tenantId;
301+
// Return the tenant identity to the caller because this is Azure DevOps.
302+
return tenantId;
303+
}
304+
}
305+
else
306+
{
307+
context.Trace.WriteLine($"unable to get response from '{requestUri}' [{(int)response.StatusCode} {response.StatusCode}].");
301308
}
302309
}
303-
else
304-
{
305-
context.Trace.WriteLine($"unable to get response from '{requestUri}' [{(int)response.StatusCode} {response.StatusCode}].");
306-
}
310+
}
311+
catch (HttpRequestException exception)
312+
{
313+
context.Trace.WriteLine($"unable to get response from '{requestUri}', an error occurred before the server could respond.");
314+
context.Trace.WriteException(exception);
307315
}
308316
}
309-
catch (HttpRequestException exception)
317+
else
310318
{
311-
context.Trace.WriteLine($"unable to get response from '{requestUri}', an error occurred before the server could respond.");
312-
context.Trace.WriteException(exception);
319+
context.Trace.WriteLine($"detected non-http(s) based protocol: '{requestUri.Scheme}'.");
313320
}
314321
}
315-
else
316-
{
317-
context.Trace.WriteLine($"detected non-http(s) based protocol: '{requestUri.Scheme}'.");
318-
}
319-
320-
if (OrdinalIgnoreCase.Equals(VstsBaseUrlHost, requestUri.Host))
321-
return Guid.Empty;
322322

323323
// Fallback to basic authentication.
324324
return null;

0 commit comments

Comments
 (0)