Skip to content
This repository was archived by the owner on Mar 20, 2019. It is now read-only.

Commit 9dbf195

Browse files
AArnottAArnott
committed
Fixes RequireSsl in OpenID identifier discovery.
Makes UntrustedWebRequestHandler a DelegatingHandler. Now 21 failures.
1 parent 982dc39 commit 9dbf195

File tree

5 files changed

+117
-98
lines changed

5 files changed

+117
-98
lines changed

‎src/DotNetOpenAuth.OpenId/DefaultOpenIdHostFactories.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public class DefaultOpenIdHostFactories : IHostFactories {
2828
/// </remarks>
2929
public virtual HttpMessageHandler CreateHttpMessageHandler() {
3030
var handler = new UntrustedWebRequestHandler();
31-
handler.CachePolicy = new RequestCachePolicy(RequestCacheLevel.NoCacheNoStore);
31+
((WebRequestHandler)handler.InnerHandler).CachePolicy = new RequestCachePolicy(RequestCacheLevel.NoCacheNoStore);
3232
return handler;
3333
}
3434

‎src/DotNetOpenAuth.OpenId/OpenId/OpenIdUtilities.cs‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -217,32 +217,36 @@ internal static HttpClient CreateHttpClient(this IHostFactories hostFactories, b
217217

218218
var rootHandler = hostFactories.CreateHttpMessageHandler();
219219
var handler = rootHandler;
220+
bool sslRequiredSet = false, cachePolicySet = false;
220221
do {
221222
var webRequestHandler = handler as WebRequestHandler;
222223
var untrustedHandler = handler as UntrustedWebRequestHandler;
223224
var delegatingHandler = handler as DelegatingHandler;
224225
if (webRequestHandler != null) {
225226
if (cachePolicy != null) {
226227
webRequestHandler.CachePolicy = cachePolicy;
228+
cachePolicySet = true;
227229
}
228-
229-
break;
230230
} else if (untrustedHandler != null) {
231-
if (cachePolicy != null) {
232-
untrustedHandler.CachePolicy = cachePolicy;
233-
}
234-
235231
untrustedHandler.IsSslRequired = requireSsl;
236-
break;
237-
} else if (delegatingHandler != null) {
232+
sslRequiredSet = true;
233+
}
234+
235+
if (delegatingHandler != null) {
238236
handler = delegatingHandler.InnerHandler;
239237
} else {
240-
Logger.Http.DebugFormat("Unable to set cache policy on unsupported {0}.", handler.GetType().FullName);
241238
break;
242239
}
243240
}
244241
while (true);
245242

243+
if (cachePolicy != null && !cachePolicySet) {
244+
Logger.OpenId.Warn(
245+
"Unable to set cache policy due to HttpMessageHandler instances not being of type WebRequestHandler.");
246+
}
247+
248+
ErrorUtilities.VerifyProtocol(!requireSsl || sslRequiredSet, "Unable to set RequireSsl on message handler because no HttpMessageHandler was of type {0}.", typeof(UntrustedWebRequestHandler).FullName);
249+
246250
return hostFactories.CreateHttpClient(rootHandler);
247251
}
248252

‎src/DotNetOpenAuth.OpenId/OpenId/UntrustedWebRequestHandler.cs‎

Lines changed: 64 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,7 @@ namespace DotNetOpenAuth.OpenId {
3636
/// If a particular host would be permitted but is in the blacklist, it is not allowed.
3737
/// If a particular host would not be permitted but is in the whitelist, it is allowed.
3838
/// </remarks>
39-
public class UntrustedWebRequestHandler : HttpMessageHandler {
40-
/// <summary>
41-
/// The inner handler.
42-
/// </summary>
43-
private readonly InternalWebRequestHandler innerHandler;
44-
39+
public class UntrustedWebRequestHandler : DelegatingHandler {
4540
/// <summary>
4641
/// The set of URI schemes allowed in untrusted web requests.
4742
/// </summary>
@@ -71,7 +66,13 @@ public class UntrustedWebRequestHandler : HttpMessageHandler {
7166
/// The maximum redirections to follow in the course of a single request.
7267
/// </summary>
7368
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
74-
private int maximumRedirections = Configuration.MaximumRedirections;
69+
private int maxAutomaticRedirections = Configuration.MaximumRedirections;
70+
71+
/// <summary>
72+
/// A value indicating whether to automatically follow redirects.
73+
/// </summary>
74+
[DebuggerBrowsable(DebuggerBrowsableState.Never)]
75+
private bool allowAutoRedirect = true;
7576

7677
/// <summary>
7778
/// Initializes a new instance of the <see cref="UntrustedWebRequestHandler" /> class.
@@ -80,9 +81,8 @@ public class UntrustedWebRequestHandler : HttpMessageHandler {
8081
/// The inner handler. This handler will be modified to suit the purposes of this wrapping handler,
8182
/// and should not be used independently of this wrapper after construction of this object.
8283
/// </param>
83-
public UntrustedWebRequestHandler(WebRequestHandler innerHandler = null) {
84-
this.innerHandler = new InternalWebRequestHandler(innerHandler ?? new WebRequestHandler());
85-
84+
public UntrustedWebRequestHandler(WebRequestHandler innerHandler = null)
85+
: base(innerHandler ?? new WebRequestHandler()) {
8686
// If SSL is required throughout, we cannot allow auto redirects because
8787
// it may include a pass through an unprotected HTTP request.
8888
// We have to follow redirects manually.
@@ -100,6 +100,18 @@ public UntrustedWebRequestHandler(WebRequestHandler innerHandler = null) {
100100
}
101101
}
102102

103+
/// <summary>
104+
/// Initializes a new instance of the <see cref="UntrustedWebRequestHandler"/> class
105+
/// for use in unit testing.
106+
/// </summary>
107+
/// <param name="innerHandler">
108+
/// The inner handler which is responsible for processing the HTTP response messages.
109+
/// This handler should NOT automatically follow redirects.
110+
/// </param>
111+
internal UntrustedWebRequestHandler(HttpMessageHandler innerHandler)
112+
: base(innerHandler) {
113+
}
114+
103115
/// <summary>
104116
/// Gets or sets a value indicating whether all requests must use SSL.
105117
/// </summary>
@@ -108,26 +120,37 @@ public UntrustedWebRequestHandler(WebRequestHandler innerHandler = null) {
108120
/// </value>
109121
public bool IsSslRequired { get; set; }
110122

111-
/// <summary>
112-
/// Gets or sets the cache policy.
113-
/// </summary>
114-
public RequestCachePolicy CachePolicy {
115-
get { return this.InnerWebRequestHandler.CachePolicy; }
116-
set { this.InnerWebRequestHandler.CachePolicy = value; }
117-
}
118-
119123
/// <summary>
120124
/// Gets or sets the total number of redirections to allow on any one request.
121125
/// Default is 10.
122126
/// </summary>
123127
public int MaxAutomaticRedirections {
124128
get {
125-
return this.maximumRedirections;
129+
return base.InnerHandler is WebRequestHandler ? this.InnerWebRequestHandler.MaxAutomaticRedirections : this.maxAutomaticRedirections;
126130
}
127131

128132
set {
129133
Requires.Range(value >= 0, "value");
130-
this.maximumRedirections = value;
134+
this.maxAutomaticRedirections = value;
135+
if (base.InnerHandler is WebRequestHandler) {
136+
this.InnerWebRequestHandler.MaxAutomaticRedirections = value;
137+
}
138+
}
139+
}
140+
141+
/// <summary>
142+
/// Gets or sets a value indicating whether to automatically follow redirects.
143+
/// </summary>
144+
public bool AllowAutoRedirect {
145+
get {
146+
return base.InnerHandler is WebRequestHandler ? this.InnerWebRequestHandler.AllowAutoRedirect : this.allowAutoRedirect;
147+
}
148+
149+
set {
150+
this.allowAutoRedirect = value;
151+
if (base.InnerHandler is WebRequestHandler) {
152+
this.InnerWebRequestHandler.AllowAutoRedirect = value;
153+
}
131154
}
132155
}
133156

@@ -190,8 +213,8 @@ public ICollection<Regex> BlacklistHostsRegex {
190213
/// <value>
191214
/// The inner web request handler.
192215
/// </value>
193-
protected WebRequestHandler InnerWebRequestHandler {
194-
get { return (WebRequestHandler)this.innerHandler.InnerHandler; }
216+
public WebRequestHandler InnerWebRequestHandler {
217+
get { return (WebRequestHandler)this.InnerHandler; }
195218
}
196219

197220
/// <summary>
@@ -253,7 +276,8 @@ internal static bool IsExceptionFrom417ExpectationFailed(Exception ex) {
253276
/// <returns>
254277
/// Returns <see cref="T:System.Threading.Tasks.Task`1" />.The task object representing the asynchronous operation.
255278
/// </returns>
256-
protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) {
279+
protected override async Task<HttpResponseMessage> SendAsync(
280+
HttpRequestMessage request, CancellationToken cancellationToken) {
257281
this.EnsureAllowableRequestUri(request.RequestUri);
258282

259283
// Since we may require SSL for every redirect, we handle each redirect manually
@@ -264,16 +288,20 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
264288
int i;
265289
for (i = 0; i < this.MaxAutomaticRedirections; i++) {
266290
this.EnsureAllowableRequestUri(request.RequestUri);
267-
var response = await this.innerHandler.SendAsync(request, cancellationToken);
268-
if (response.StatusCode == HttpStatusCode.MovedPermanently || response.StatusCode == HttpStatusCode.Redirect
269-
|| response.StatusCode == HttpStatusCode.RedirectMethod || response.StatusCode == HttpStatusCode.RedirectKeepVerb) {
270-
// We have no copy of the post entity stream to repeat on our manually
271-
// cloned HttpWebRequest, so we have to bail.
272-
ErrorUtilities.VerifyProtocol(request.Method != HttpMethod.Post, MessagingStrings.UntrustedRedirectsOnPOSTNotSupported);
273-
Uri redirectUri = new Uri(request.RequestUri, response.Headers.Location);
274-
request = request.Clone();
275-
request.RequestUri = redirectUri;
276-
continue;
291+
var response = await base.SendAsync(request, cancellationToken);
292+
if (this.AllowAutoRedirect) {
293+
if (response.StatusCode == HttpStatusCode.MovedPermanently || response.StatusCode == HttpStatusCode.Redirect
294+
|| response.StatusCode == HttpStatusCode.RedirectMethod
295+
|| response.StatusCode == HttpStatusCode.RedirectKeepVerb) {
296+
// We have no copy of the post entity stream to repeat on our manually
297+
// cloned HttpWebRequest, so we have to bail.
298+
ErrorUtilities.VerifyProtocol(
299+
request.Method != HttpMethod.Post, MessagingStrings.UntrustedRedirectsOnPOSTNotSupported);
300+
Uri redirectUri = new Uri(request.RequestUri, response.Headers.Location);
301+
request = request.Clone();
302+
request.RequestUri = redirectUri;
303+
continue;
304+
}
277305
}
278306

279307
if (response.StatusCode == HttpStatusCode.ExpectationFailed) {
@@ -286,7 +314,9 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
286314
// the web site's global behavior when calling that host.
287315
// TODO: verify that this still works in DNOA 5.0
288316
var servicePoint = ServicePointManager.FindServicePoint(request.RequestUri);
289-
Logger.Http.InfoFormat("HTTP POST to {0} resulted in 417 Expectation Failed. Changing ServicePoint to not use Expect: Continue next time.", request.RequestUri);
317+
Logger.Http.InfoFormat(
318+
"HTTP POST to {0} resulted in 417 Expectation Failed. Changing ServicePoint to not use Expect: Continue next time.",
319+
request.RequestUri);
290320
servicePoint.Expect100Continue = false;
291321
}
292322

@@ -441,30 +471,5 @@ private bool IsUriAllowable(Uri uri) {
441471
}
442472
return true;
443473
}
444-
445-
/// <summary>
446-
/// A <see cref="DelegatingHandler" /> derived type that makes its SendAsync method available internally.
447-
/// </summary>
448-
private class InternalWebRequestHandler : DelegatingHandler {
449-
/// <summary>
450-
/// Initializes a new instance of the <see cref="InternalWebRequestHandler"/> class.
451-
/// </summary>
452-
/// <param name="innerHandler">The inner handler which is responsible for processing the HTTP response messages.</param>
453-
internal InternalWebRequestHandler(HttpMessageHandler innerHandler)
454-
: base(innerHandler) {
455-
}
456-
457-
/// <summary>
458-
/// Creates an instance of <see cref="T:System.Net.Http.HttpResponseMessage" /> based on the information provided in the <see cref="T:System.Net.Http.HttpRequestMessage" /> as an operation that will not block.
459-
/// </summary>
460-
/// <param name="request">The HTTP request message.</param>
461-
/// <param name="cancellationToken">A cancellation token to cancel the operation.</param>
462-
/// <returns>
463-
/// Returns <see cref="T:System.Threading.Tasks.Task`1" />.The task object representing the asynchronous operation.
464-
/// </returns>
465-
protected internal new Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) {
466-
return base.SendAsync(request, cancellationToken);
467-
}
468-
}
469474
}
470475
}

‎src/DotNetOpenAuth.Test/MockingHostFactories.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ namespace DotNetOpenAuth.Test {
1111
using System.Threading;
1212
using System.Threading.Tasks;
1313
using System.Linq;
14+
15+
using DotNetOpenAuth.OpenId;
16+
1417
using Validation;
1518

1619
internal class MockingHostFactories : IHostFactories {
@@ -30,10 +33,16 @@ public List<TestBase.Handler> Handlers {
3033

3134
public bool AllowAutoRedirects { get; set; }
3235

36+
public bool InstallUntrustedWebReqestHandler { get; set; }
37+
3338
public HttpMessageHandler CreateHttpMessageHandler() {
3439
var forwardingMessageHandler = new ForwardingMessageHandler(this.handlers, this);
3540
var cookieDelegatingHandler = new CookieDelegatingHandler(forwardingMessageHandler, this.CookieContainer);
36-
if (this.AllowAutoRedirects) {
41+
if (this.InstallUntrustedWebReqestHandler) {
42+
var untrustedHandler = new UntrustedWebRequestHandler(cookieDelegatingHandler);
43+
untrustedHandler.AllowAutoRedirect = this.AllowAutoRedirects;
44+
return untrustedHandler;
45+
} else if (this.AllowAutoRedirects) {
3746
return new AutoRedirectHandler(cookieDelegatingHandler);
3847
} else {
3948
return cookieDelegatingHandler;

‎src/DotNetOpenAuth.Test/OpenId/OpenIdTestBase.cs‎

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ public override void SetUp() {
7979

8080
this.AutoProviderScenario = Scenarios.AutoApproval;
8181
Identifier.EqualityOnStrings = true;
82+
this.HostFactories.InstallUntrustedWebReqestHandler = true;
8283
}
8384

8485
[TearDown]
@@ -302,37 +303,37 @@ internal async Task RoundtripAsync(
302303
return await op.Channel.PrepareResponseAsync(response);
303304
});
304305

305-
{
306-
var rp = this.CreateRelyingParty();
307-
ExtensionTestUtilities.RegisterExtension(rp.Channel, Mocks.MockOpenIdExtension.Factory);
308-
var requestBase = new CheckIdRequest(protocol.Version, OpenIdTestBase.OPUri, AuthenticationRequestMode.Immediate);
309-
OpenIdTestBase.StoreAssociation(rp, OpenIdTestBase.OPUri, association);
310-
requestBase.AssociationHandle = association.Handle;
311-
requestBase.ClaimedIdentifier = "http://claimedid";
312-
requestBase.LocalIdentifier = "http://localid";
313-
requestBase.ReturnTo = OpenIdTestBase.RPUri;
314-
315-
foreach (IOpenIdMessageExtension extension in requests) {
316-
requestBase.Extensions.Add(extension);
317-
}
306+
{
307+
var rp = this.CreateRelyingParty();
308+
ExtensionTestUtilities.RegisterExtension(rp.Channel, Mocks.MockOpenIdExtension.Factory);
309+
var requestBase = new CheckIdRequest(protocol.Version, OpenIdTestBase.OPUri, AuthenticationRequestMode.Immediate);
310+
OpenIdTestBase.StoreAssociation(rp, OpenIdTestBase.OPUri, association);
311+
requestBase.AssociationHandle = association.Handle;
312+
requestBase.ClaimedIdentifier = "http://claimedid";
313+
requestBase.LocalIdentifier = "http://localid";
314+
requestBase.ReturnTo = OpenIdTestBase.RPUri;
315+
316+
foreach (IOpenIdMessageExtension extension in requests) {
317+
requestBase.Extensions.Add(extension);
318+
}
318319

319-
var redirectingRequest = await rp.Channel.PrepareResponseAsync(requestBase);
320-
Uri redirectingResponseUri;
321-
this.HostFactories.AllowAutoRedirects = false;
322-
using (var httpClient = rp.Channel.HostFactories.CreateHttpClient()) {
323-
using (var redirectingResponse = await httpClient.GetAsync(redirectingRequest.Headers.Location)) {
324-
Assert.AreEqual(HttpStatusCode.Found, redirectingResponse.StatusCode);
325-
redirectingResponseUri = redirectingResponse.Headers.Location;
326-
}
320+
var redirectingRequest = await rp.Channel.PrepareResponseAsync(requestBase);
321+
Uri redirectingResponseUri;
322+
this.HostFactories.AllowAutoRedirects = false;
323+
using (var httpClient = rp.Channel.HostFactories.CreateHttpClient()) {
324+
using (var redirectingResponse = await httpClient.GetAsync(redirectingRequest.Headers.Location)) {
325+
Assert.AreEqual(HttpStatusCode.Found, redirectingResponse.StatusCode);
326+
redirectingResponseUri = redirectingResponse.Headers.Location;
327327
}
328-
329-
var response =
330-
await
331-
rp.Channel.ReadFromRequestAsync<PositiveAssertionResponse>(
332-
new HttpRequestMessage(HttpMethod.Get, redirectingResponseUri), CancellationToken.None);
333-
var receivedResponses = response.Extensions.Cast<IOpenIdMessageExtension>();
334-
CollectionAssert<IOpenIdMessageExtension>.AreEquivalentByEquality(responses.ToArray(), receivedResponses.ToArray());
335328
}
329+
330+
var response =
331+
await
332+
rp.Channel.ReadFromRequestAsync<PositiveAssertionResponse>(
333+
new HttpRequestMessage(HttpMethod.Get, redirectingResponseUri), CancellationToken.None);
334+
var receivedResponses = response.Extensions.Cast<IOpenIdMessageExtension>();
335+
CollectionAssert<IOpenIdMessageExtension>.AreEquivalentByEquality(responses.ToArray(), receivedResponses.ToArray());
336+
}
336337
}
337338
}
338339
}

0 commit comments

Comments
 (0)