Skip to content

Commit 4fa5a22

Browse files
TratcherTratcher
authored
Add error details for OAuth and OIDC remote errors dotnet#4682 (dotnet#11002)
1 parent 2ea746f commit 4fa5a22

File tree

4 files changed

+150
-9
lines changed

4 files changed

+150
-9
lines changed

‎src/Security/Authentication/OAuth/src/OAuthHandler.cs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,27 +71,40 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
7171
// Since it's a frequent scenario (that is not caused by incorrect configuration),
7272
// denied errors are handled differently using HandleAccessDeniedErrorAsync().
7373
// Visit https://tools.ietf.org/html/rfc6749#section-4.1.2.1 for more information.
74+
var errorDescription = query["error_description"];
75+
var errorUri = query["error_uri"];
7476
if (StringValues.Equals(error, "access_denied"))
7577
{
7678
var result = await HandleAccessDeniedErrorAsync(properties);
77-
return !result.None ? result
78-
: HandleRequestResult.Fail("Access was denied by the resource owner or by the remote server.", properties);
79+
if (!result.None)
80+
{
81+
return result;
82+
}
83+
var deniedEx = new Exception("Access was denied by the resource owner or by the remote server.");
84+
deniedEx.Data["error"] = error.ToString();
85+
deniedEx.Data["error_description"] = errorDescription.ToString();
86+
deniedEx.Data["error_uri"] = errorUri.ToString();
87+
88+
return HandleRequestResult.Fail(deniedEx, properties);
7989
}
8090

8191
var failureMessage = new StringBuilder();
8292
failureMessage.Append(error);
83-
var errorDescription = query["error_description"];
8493
if (!StringValues.IsNullOrEmpty(errorDescription))
8594
{
8695
failureMessage.Append(";Description=").Append(errorDescription);
8796
}
88-
var errorUri = query["error_uri"];
8997
if (!StringValues.IsNullOrEmpty(errorUri))
9098
{
9199
failureMessage.Append(";Uri=").Append(errorUri);
92100
}
93101

94-
return HandleRequestResult.Fail(failureMessage.ToString(), properties);
102+
var ex = new Exception(failureMessage.ToString());
103+
ex.Data["error"] = error.ToString();
104+
ex.Data["error_description"] = errorDescription.ToString();
105+
ex.Data["error_uri"] = errorUri.ToString();
106+
107+
return HandleRequestResult.Fail(ex, properties);
95108
}
96109

97110
var code = query["code"];

‎src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1309,12 +1309,16 @@ private OpenIdConnectProtocolException CreateOpenIdConnectProtocolException(Open
13091309
Logger.ResponseError(message.Error, description, errorUri);
13101310
}
13111311

1312-
return new OpenIdConnectProtocolException(string.Format(
1312+
var ex = new OpenIdConnectProtocolException(string.Format(
13131313
CultureInfo.InvariantCulture,
13141314
Resources.MessageContainsError,
13151315
message.Error,
13161316
description,
13171317
errorUri));
1318+
ex.Data["error"] = message.Error;
1319+
ex.Data["error_description"] = description;
1320+
ex.Data["error_uri"] = errorUri;
1321+
return ex;
13181322
}
13191323
}
13201324
}

‎src/Security/Authentication/test/GoogleTests.cs

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,50 @@ public async Task ReplyPathWithAccessDeniedError_AllowsCustomizingPath()
420420
Assert.Equal("/custom-denied-page?rurl=http%3A%2F%2Fwww.google.com%2F", transaction.Response.Headers.GetValues("Location").First());
421421
}
422422

423+
[Fact]
424+
public async Task ReplyPathWithAccessDeniedErrorAndNoAccessDeniedPath_FallsBackToRemoteError()
425+
{
426+
var accessDeniedCalled = false;
427+
var remoteFailureCalled = false;
428+
var server = CreateServer(o =>
429+
{
430+
o.ClientId = "Test Id";
431+
o.ClientSecret = "Test Secret";
432+
o.StateDataFormat = new TestStateDataFormat();
433+
o.Events = new OAuthEvents()
434+
{
435+
OnAccessDenied = ctx =>
436+
{
437+
Assert.Null(ctx.AccessDeniedPath.Value);
438+
Assert.Equal("http://testhost/redirect", ctx.ReturnUrl);
439+
Assert.Equal("ReturnUrl", ctx.ReturnUrlParameter);
440+
accessDeniedCalled = true;
441+
return Task.FromResult(0);
442+
},
443+
OnRemoteFailure = ctx =>
444+
{
445+
var ex = ctx.Failure;
446+
Assert.True(ex.Data.Contains("error"), "error");
447+
Assert.True(ex.Data.Contains("error_description"), "error_description");
448+
Assert.True(ex.Data.Contains("error_uri"), "error_uri");
449+
Assert.Equal("access_denied", ex.Data["error"]);
450+
Assert.Equal("whyitfailed", ex.Data["error_description"]);
451+
Assert.Equal("https://example.com/fail", ex.Data["error_uri"]);
452+
remoteFailureCalled = true;
453+
ctx.Response.Redirect("/error?FailureMessage=" + UrlEncoder.Default.Encode(ctx.Failure.Message));
454+
ctx.HandleResponse();
455+
return Task.FromResult(0);
456+
}
457+
};
458+
});
459+
var transaction = await server.SendAsync("https://example.com/signin-google?error=access_denied&error_description=whyitfailed&error_uri=https://example.com/fail&state=protected_state",
460+
".AspNetCore.Correlation.Google.correlationId=N");
461+
Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
462+
Assert.StartsWith("/error?FailureMessage=", transaction.Response.Headers.GetValues("Location").First());
463+
Assert.True(accessDeniedCalled);
464+
Assert.True(remoteFailureCalled);
465+
}
466+
423467
[Theory]
424468
[InlineData(true)]
425469
[InlineData(false)]
@@ -434,24 +478,31 @@ public async Task ReplyPathWithErrorFails(bool redirect)
434478
{
435479
OnRemoteFailure = ctx =>
436480
{
481+
var ex = ctx.Failure;
482+
Assert.True(ex.Data.Contains("error"), "error");
483+
Assert.True(ex.Data.Contains("error_description"), "error_description");
484+
Assert.True(ex.Data.Contains("error_uri"), "error_uri");
485+
Assert.Equal("itfailed", ex.Data["error"]);
486+
Assert.Equal("whyitfailed", ex.Data["error_description"]);
487+
Assert.Equal("https://example.com/fail", ex.Data["error_uri"]);
437488
ctx.Response.Redirect("/error?FailureMessage=" + UrlEncoder.Default.Encode(ctx.Failure.Message));
438489
ctx.HandleResponse();
439490
return Task.FromResult(0);
440491
}
441492
} : new OAuthEvents();
442493
});
443-
var sendTask = server.SendAsync("https://example.com/signin-google?error=OMG&error_description=SoBad&error_uri=foobar&state=protected_state",
494+
var sendTask = server.SendAsync("https://example.com/signin-google?error=itfailed&error_description=whyitfailed&error_uri=https://example.com/fail&state=protected_state",
444495
".AspNetCore.Correlation.Google.correlationId=N");
445496
if (redirect)
446497
{
447498
var transaction = await sendTask;
448499
Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
449-
Assert.Equal("/error?FailureMessage=OMG" + UrlEncoder.Default.Encode(";Description=SoBad;Uri=foobar"), transaction.Response.Headers.GetValues("Location").First());
500+
Assert.Equal("/error?FailureMessage=itfailed" + UrlEncoder.Default.Encode(";Description=whyitfailed;Uri=https://example.com/fail"), transaction.Response.Headers.GetValues("Location").First());
450501
}
451502
else
452503
{
453504
var error = await Assert.ThrowsAnyAsync<Exception>(() => sendTask);
454-
Assert.Equal("OMG;Description=SoBad;Uri=foobar", error.GetBaseException().Message);
505+
Assert.Equal("itfailed;Description=whyitfailed;Uri=https://example.com/fail", error.GetBaseException().Message);
455506
}
456507
}
457508

‎src/Security/Authentication/test/OpenIdConnect/OpenIdConnectAuthenticateTests.cs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using System;
45
using System.Collections.Generic;
6+
using System.Linq;
7+
using System.Net;
58
using System.Net.Http;
9+
using System.Text.Encodings.Web;
610
using System.Threading.Tasks;
11+
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
712
using Microsoft.AspNetCore.Http;
813
using Xunit;
914

@@ -63,5 +68,73 @@ public async Task RegularPostRequestToCallbackPathSkips()
6368
// Assert
6469
Assert.Equal("Hi from the callback path", transaction.ResponseText);
6570
}
71+
72+
[Fact]
73+
public async Task ErrorResponseWithDetails()
74+
{
75+
var settings = new TestSettings(
76+
opt =>
77+
{
78+
opt.StateDataFormat = new TestStateDataFormat();
79+
opt.Authority = TestServerBuilder.DefaultAuthority;
80+
opt.ClientId = "Test Id";
81+
opt.Events = new OpenIdConnectEvents()
82+
{
83+
OnRemoteFailure = ctx =>
84+
{
85+
var ex = ctx.Failure;
86+
Assert.True(ex.Data.Contains("error"), "error");
87+
Assert.True(ex.Data.Contains("error_description"), "error_description");
88+
Assert.True(ex.Data.Contains("error_uri"), "error_uri");
89+
Assert.Equal("itfailed", ex.Data["error"]);
90+
Assert.Equal("whyitfailed", ex.Data["error_description"]);
91+
Assert.Equal("https://example.com/fail", ex.Data["error_uri"]);
92+
ctx.Response.Redirect("/error?FailureMessage=" + UrlEncoder.Default.Encode(ctx.Failure.Message));
93+
ctx.HandleResponse();
94+
return Task.FromResult(0);
95+
}
96+
};
97+
});
98+
99+
var server = settings.CreateTestServer();
100+
101+
var transaction = await server.SendAsync(
102+
"https://example.com/signin-oidc?error=itfailed&error_description=whyitfailed&error_uri=https://example.com/fail&state=protected_state",
103+
".AspNetCore.Correlation.OpenIdConnect.correlationId=N");
104+
Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
105+
Assert.StartsWith("/error?FailureMessage=", transaction.Response.Headers.GetValues("Location").First());
106+
}
107+
108+
private class TestStateDataFormat : ISecureDataFormat<AuthenticationProperties>
109+
{
110+
private AuthenticationProperties Data { get; set; }
111+
112+
public string Protect(AuthenticationProperties data)
113+
{
114+
return "protected_state";
115+
}
116+
117+
public string Protect(AuthenticationProperties data, string purpose)
118+
{
119+
throw new NotImplementedException();
120+
}
121+
122+
public AuthenticationProperties Unprotect(string protectedText)
123+
{
124+
Assert.Equal("protected_state", protectedText);
125+
var properties = new AuthenticationProperties(new Dictionary<string, string>()
126+
{
127+
{ ".xsrf", "correlationId" },
128+
{ "testkey", "testvalue" }
129+
});
130+
properties.RedirectUri = "http://testhost/redirect";
131+
return properties;
132+
}
133+
134+
public AuthenticationProperties Unprotect(string protectedText, string purpose)
135+
{
136+
throw new NotImplementedException();
137+
}
138+
}
66139
}
67140
}

0 commit comments

Comments
 (0)