actions/runner
1
0
Fork 0
mirror of https://github.com/actions/runner.git synced 2025-12-11 04:46:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

Reduce token service and unnecessary calls - send token to redirects (#2660)

Browse Source 
* handle redirects with token

* scope to broker

* lint
This commit is contained in:
Yashwanth Anantharaju
2023-06-16 09:23:13 -04:00
committed by GitHub
parent 1096b975e4
commit 471e3ae2d9
3 changed files with 42 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/Runner.Sdk/Util/VssUtil.cs
View File

@@ -64,7 +64,7 @@ namespace GitHub.Runner.Sdk
if (StringUtil.ConvertToBoolean(Environment.GetEnvironmentVariable("USE_BROKER_FLOW"))) if (StringUtil.ConvertToBoolean(Environment.GetEnvironmentVariable("USE_BROKER_FLOW")))
{ {
settings.AllowAutoRedirect = true; settings.AllowAutoRedirectForBroker = true;
} }
// Remove Invariant from the list of accepted languages. // Remove Invariant from the list of accepted languages.

39
src/Sdk/Common/Common/VssHttpMessageHandler.cs
View File

@@ -16,7 +16,7 @@ namespace GitHub.Services.Common
public class VssHttpMessageHandler : HttpMessageHandler public class VssHttpMessageHandler : HttpMessageHandler
{ {
/// <summary> /// <summary>
/// Initializes a new <c>VssHttpMessageHandler</c> instance with default credentials and request /// Initializes a new <c>VssHttpMessageHandler</c> instance with default credentials and request
/// settings. /// settings.
/// </summary> /// </summary>
public VssHttpMessageHandler() public VssHttpMessageHandler()
@@ -25,7 +25,7 @@ namespace GitHub.Services.Common
} }
/// <summary> /// <summary>
/// Initializes a new <c>VssHttpMessageHandler</c> instance with the specified credentials and request /// Initializes a new <c>VssHttpMessageHandler</c> instance with the specified credentials and request
/// settings. /// settings.
/// </summary> /// </summary>
/// <param name="credentials">The credentials which should be used</param> /// <param name="credentials">The credentials which should be used</param>
@@ -38,7 +38,7 @@ namespace GitHub.Services.Common
} }
/// <summary> /// <summary>
/// Initializes a new <c>VssHttpMessageHandler</c> instance with the specified credentials and request /// Initializes a new <c>VssHttpMessageHandler</c> instance with the specified credentials and request
/// settings. /// settings.
/// </summary> /// </summary>
/// <param name="credentials">The credentials which should be used</param> /// <param name="credentials">The credentials which should be used</param>
@@ -211,14 +211,33 @@ namespace GitHub.Services.Common
traceInfo?.TraceBufferedRequestTime(); traceInfo?.TraceBufferedRequestTime();
// ConfigureAwait(false) enables the continuation to be run outside any captured // ConfigureAwait(false) enables the continuation to be run outside any captured
// SyncronizationContext (such as ASP.NET's) which keeps things from deadlocking... // SyncronizationContext (such as ASP.NET's) which keeps things from deadlocking...
response = await m_messageInvoker.SendAsync(request, tokenSource.Token).ConfigureAwait(false);
var tmpResponse = await m_messageInvoker.SendAsync(request, tokenSource.Token).ConfigureAwait(false);
if (Settings.AllowAutoRedirectForBroker && tmpResponse.StatusCode == HttpStatusCode.Redirect)
{
//Dispose of the previous response
tmpResponse?.Dispose();
var location = tmpResponse.Headers.Location;
request = new HttpRequestMessage(HttpMethod.Get, location);
// Reapply the token to new redirected request
ApplyToken(request, token, applyICredentialsToWebProxy: lastResponseDemandedProxyAuth);
// Resend the request
response = await m_messageInvoker.SendAsync(request, tokenSource.Token).ConfigureAwait(false);
}
else
{
response = tmpResponse;
}
traceInfo?.TraceRequestSendTime(); traceInfo?.TraceRequestSendTime();
// Now buffer the response content if configured to do so. In general we will be buffering // Now buffer the response content if configured to do so. In general we will be buffering
// the response content in this location, except in the few cases where the caller has // the response content in this location, except in the few cases where the caller has
// specified HttpCompletionOption.ResponseHeadersRead. // specified HttpCompletionOption.ResponseHeadersRead.
// Trace content type in case of error // Trace content type in case of error
await BufferResponseContentAsync(request, response, () => $"[ContentType: {response.Content.GetType().Name}]", tokenSource.Token).ConfigureAwait(false); await BufferResponseContentAsync(request, response, () => $"[ContentType: {response.Content.GetType().Name}]", tokenSource.Token).ConfigureAwait(false);
@@ -235,7 +254,7 @@ namespace GitHub.Services.Common
provider.ValidateToken(token, responseWrapper); provider.ValidateToken(token, responseWrapper);
} }
// Make sure that once we can authenticate with the service that we turn off the // Make sure that once we can authenticate with the service that we turn off the
// Expect100Continue behavior to increase performance. // Expect100Continue behavior to increase performance.
this.ExpectContinue = false; this.ExpectContinue = false;
succeeded = true; succeeded = true;
@@ -281,8 +300,8 @@ namespace GitHub.Services.Common
} }
// If the user has already tried once but still unauthorized, stop retrying. The main scenario for this condition // If the user has already tried once but still unauthorized, stop retrying. The main scenario for this condition
// is a user typed in a valid credentials for a hosted account but the associated identity does not have // is a user typed in a valid credentials for a hosted account but the associated identity does not have
// access. We do not want to continually prompt 3 times without telling them the failure reason. In the // access. We do not want to continually prompt 3 times without telling them the failure reason. In the
// next release we should rethink about presenting user the failure and options between retries. // next release we should rethink about presenting user the failure and options between retries.
IEnumerable<String> headerValues; IEnumerable<String> headerValues;
Boolean hasAuthenticateError = Boolean hasAuthenticateError =
@@ -489,7 +508,7 @@ namespace GitHub.Services.Common
httpClientHandler.AllowAutoRedirect = settings.AllowAutoRedirect; httpClientHandler.AllowAutoRedirect = settings.AllowAutoRedirect;
httpClientHandler.ClientCertificateOptions = ClientCertificateOption.Manual; httpClientHandler.ClientCertificateOptions = ClientCertificateOption.Manual;
//Setting httpClientHandler.UseDefaultCredentials to false in .Net Core, clears httpClientHandler.Credentials if //Setting httpClientHandler.UseDefaultCredentials to false in .Net Core, clears httpClientHandler.Credentials if
//credentials is already set to defaultcredentials. Therefore httpClientHandler.Credentials must be //credentials is already set to defaultcredentials. Therefore httpClientHandler.Credentials must be
//set after httpClientHandler.UseDefaultCredentials. //set after httpClientHandler.UseDefaultCredentials.
httpClientHandler.UseDefaultCredentials = false; httpClientHandler.UseDefaultCredentials = false;
httpClientHandler.Credentials = defaultCredentials; httpClientHandler.Credentials = defaultCredentials;

14
src/Sdk/Common/Common/VssHttpRequestSettings.cs
View File

@@ -102,7 +102,7 @@ namespace GitHub.Services.Common
} }
/// <summary> /// <summary>
/// Gets or sets a value indicating whether or not HttpClientHandler should follow redirect on outgoing requests. /// Gets or sets a value indicating whether or not HttpClientHandler should follow redirect on outgoing requests.
/// </summary> /// </summary>
public Boolean AllowAutoRedirect public Boolean AllowAutoRedirect
{ {
@@ -111,7 +111,17 @@ namespace GitHub.Services.Common
} }
/// <summary> /// <summary>
/// Gets or sets a value indicating whether or not compression should be used on outgoing requests. /// Gets or sets a value indicating whether or not HttpClientHandler should follow redirect on outgoing broker requests
/// This is special since this also sends token in the request, where as default AllowAutoRedirect does not
/// </summary>
public Boolean AllowAutoRedirectForBroker
{
get;
set;
}
/// <summary>
/// Gets or sets a value indicating whether or not compression should be used on outgoing requests.
/// The default value is true. /// The default value is true.
/// </summary> /// </summary>
[DefaultValue(true)] [DefaultValue(true)]