Browse Source

Relax the relative URLs constraints and fix the Uri construction logic to correctly compute absolute URLs

pull/897/head
Kévin Chalet 6 years ago
parent
commit
a8f9e53f9d
  1. 50
      src/OpenIddict.Server/OpenIddictServerBuilder.cs
  2. 110
      src/OpenIddict.Server/OpenIddictServerHandlers.Discovery.cs
  3. 47
      src/OpenIddict.Server/OpenIddictServerHandlers.cs
  4. 6
      src/OpenIddict.Validation/OpenIddictValidationConfiguration.cs
  5. 118
      test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Discovery.cs
  6. 2
      test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.cs

50
src/OpenIddict.Server/OpenIddictServerBuilder.cs

@ -1137,11 +1137,6 @@ namespace Microsoft.Extensions.DependencyInjection
throw new ArgumentException("One of the specified addresses is not valid.", nameof(addresses));
}
if (addresses.Any(address => !address.IsAbsoluteUri && !address.OriginalString.StartsWith("/", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException("Relative URLs must start with a '/'.", nameof(addresses));
}
return Configure(options =>
{
options.AuthorizationEndpointUris.Clear();
@ -1189,11 +1184,6 @@ namespace Microsoft.Extensions.DependencyInjection
throw new ArgumentException("One of the specified addresses is not valid.", nameof(addresses));
}
if (addresses.Any(address => !address.IsAbsoluteUri && !address.OriginalString.StartsWith("/", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException("Relative URLs must start with a '/'.", nameof(addresses));
}
return Configure(options =>
{
options.ConfigurationEndpointUris.Clear();
@ -1241,11 +1231,6 @@ namespace Microsoft.Extensions.DependencyInjection
throw new ArgumentException("One of the specified addresses is not valid.", nameof(addresses));
}
if (addresses.Any(address => !address.IsAbsoluteUri && !address.OriginalString.StartsWith("/", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException("Relative URLs must start with a '/'.", nameof(addresses));
}
return Configure(options =>
{
options.CryptographyEndpointUris.Clear();
@ -1293,11 +1278,6 @@ namespace Microsoft.Extensions.DependencyInjection
throw new ArgumentException("One of the specified addresses is not valid.", nameof(addresses));
}
if (addresses.Any(address => !address.IsAbsoluteUri && !address.OriginalString.StartsWith("/", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException("Relative URLs must start with a '/'.", nameof(addresses));
}
return Configure(options =>
{
options.DeviceEndpointUris.Clear();
@ -1345,11 +1325,6 @@ namespace Microsoft.Extensions.DependencyInjection
throw new ArgumentException("One of the specified addresses is not valid.", nameof(addresses));
}
if (addresses.Any(address => !address.IsAbsoluteUri && !address.OriginalString.StartsWith("/", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException("Relative URLs must start with a '/'.", nameof(addresses));
}
return Configure(options =>
{
options.IntrospectionEndpointUris.Clear();
@ -1397,11 +1372,6 @@ namespace Microsoft.Extensions.DependencyInjection
throw new ArgumentException("One of the specified addresses is not valid.", nameof(addresses));
}
if (addresses.Any(address => !address.IsAbsoluteUri && !address.OriginalString.StartsWith("/", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException("Relative URLs must start with a '/'.", nameof(addresses));
}
return Configure(options =>
{
options.LogoutEndpointUris.Clear();
@ -1449,11 +1419,6 @@ namespace Microsoft.Extensions.DependencyInjection
throw new ArgumentException("One of the specified addresses is not valid.", nameof(addresses));
}
if (addresses.Any(address => !address.IsAbsoluteUri && !address.OriginalString.StartsWith("/", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException("Relative URLs must start with a '/'.", nameof(addresses));
}
return Configure(options =>
{
options.RevocationEndpointUris.Clear();
@ -1501,11 +1466,6 @@ namespace Microsoft.Extensions.DependencyInjection
throw new ArgumentException("One of the specified addresses is not valid.", nameof(addresses));
}
if (addresses.Any(address => !address.IsAbsoluteUri && !address.OriginalString.StartsWith("/", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException("Relative URLs must start with a '/'.", nameof(addresses));
}
return Configure(options =>
{
options.TokenEndpointUris.Clear();
@ -1553,11 +1513,6 @@ namespace Microsoft.Extensions.DependencyInjection
throw new ArgumentException("One of the specified addresses is not valid.", nameof(addresses));
}
if (addresses.Any(address => !address.IsAbsoluteUri && !address.OriginalString.StartsWith("/", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException("Relative URLs must start with a '/'.", nameof(addresses));
}
return Configure(options =>
{
options.UserinfoEndpointUris.Clear();
@ -1605,11 +1560,6 @@ namespace Microsoft.Extensions.DependencyInjection
throw new ArgumentException("One of the specified addresses is not valid.", nameof(addresses));
}
if (addresses.Any(address => !address.IsAbsoluteUri && !address.OriginalString.StartsWith("/", StringComparison.OrdinalIgnoreCase)))
{
throw new ArgumentException("Relative URLs must start with a '/'.", nameof(addresses));
}
return Configure(options =>
{
options.VerificationEndpointUris.Clear();

110
src/OpenIddict.Server/OpenIddictServerHandlers.Discovery.cs

@ -393,99 +393,69 @@ namespace OpenIddict.Server
// Note: while OpenIddict allows specifying multiple endpoint addresses, the OAuth 2.0
// and OpenID Connect discovery specifications only allow a single address per endpoint.
context.AuthorizationEndpoint ??= context.Options.AuthorizationEndpointUris.FirstOrDefault();
context.CryptographyEndpoint ??= context.Options.CryptographyEndpointUris.FirstOrDefault();
context.DeviceEndpoint ??= context.Options.DeviceEndpointUris.FirstOrDefault();
context.IntrospectionEndpoint ??= context.Options.IntrospectionEndpointUris.FirstOrDefault();
context.LogoutEndpoint ??= context.Options.LogoutEndpointUris.FirstOrDefault();
context.RevocationEndpoint ??= context.Options.RevocationEndpointUris.FirstOrDefault();
context.TokenEndpoint ??= context.Options.TokenEndpointUris.FirstOrDefault();
context.UserinfoEndpoint ??= context.Options.UserinfoEndpointUris.FirstOrDefault();
// Note: this handler doesn't have any access to the request context. As such, it depends
// on another handler to determine the issuer location from the ambient request if it was not
// explicitly set in the server options. If the issuer is not set, an exception is thrown.
if (context.AuthorizationEndpoint != null && !context.AuthorizationEndpoint.IsAbsoluteUri)
{
if (context.Issuer == null || !context.Issuer.IsAbsoluteUri)
{
throw new InvalidOperationException("An absolute URL cannot be built for the authorization endpoint path.");
}
context.AuthorizationEndpoint = new Uri(context.Issuer, context.AuthorizationEndpoint);
}
context.AuthorizationEndpoint ??= GetEndpointAbsoluteUrl(context.Issuer,
context.Options.AuthorizationEndpointUris.FirstOrDefault());
if (context.CryptographyEndpoint != null && !context.CryptographyEndpoint.IsAbsoluteUri)
{
if (context.Issuer == null || !context.Issuer.IsAbsoluteUri)
{
throw new InvalidOperationException("An absolute URL cannot be built for the cryptography endpoint path.");
}
context.CryptographyEndpoint ??= GetEndpointAbsoluteUrl(context.Issuer,
context.Options.CryptographyEndpointUris.FirstOrDefault());
context.CryptographyEndpoint = new Uri(context.Issuer, context.CryptographyEndpoint);
}
context.DeviceEndpoint ??= GetEndpointAbsoluteUrl(context.Issuer,
context.Options.DeviceEndpointUris.FirstOrDefault());
if (context.DeviceEndpoint != null && !context.DeviceEndpoint.IsAbsoluteUri)
{
if (context.Issuer == null || !context.Issuer.IsAbsoluteUri)
{
throw new InvalidOperationException("An absolute URL cannot be built for the device endpoint path.");
}
context.IntrospectionEndpoint ??= GetEndpointAbsoluteUrl(context.Issuer,
context.Options.IntrospectionEndpointUris.FirstOrDefault());
context.DeviceEndpoint = new Uri(context.Issuer, context.DeviceEndpoint);
}
context.LogoutEndpoint ??= GetEndpointAbsoluteUrl(context.Issuer,
context.Options.LogoutEndpointUris.FirstOrDefault());
if (context.IntrospectionEndpoint != null && !context.IntrospectionEndpoint.IsAbsoluteUri)
{
if (context.Issuer == null || !context.Issuer.IsAbsoluteUri)
{
throw new InvalidOperationException("An absolute URL cannot be built for the introspection endpoint path.");
}
context.RevocationEndpoint ??= GetEndpointAbsoluteUrl(context.Issuer,
context.Options.RevocationEndpointUris.FirstOrDefault());
context.IntrospectionEndpoint = new Uri(context.Issuer, context.IntrospectionEndpoint);
}
context.TokenEndpoint ??= GetEndpointAbsoluteUrl(context.Issuer,
context.Options.TokenEndpointUris.FirstOrDefault());
if (context.LogoutEndpoint != null && !context.LogoutEndpoint.IsAbsoluteUri)
{
if (context.Issuer == null || !context.Issuer.IsAbsoluteUri)
{
throw new InvalidOperationException("An absolute URL cannot be built for the logout endpoint path.");
}
context.UserinfoEndpoint ??= GetEndpointAbsoluteUrl(context.Issuer,
context.Options.UserinfoEndpointUris.FirstOrDefault());
context.LogoutEndpoint = new Uri(context.Issuer, context.LogoutEndpoint);
}
return default;
if (context.RevocationEndpoint != null && !context.RevocationEndpoint.IsAbsoluteUri)
static Uri GetEndpointAbsoluteUrl(Uri issuer, Uri endpoint)
{
if (context.Issuer == null || !context.Issuer.IsAbsoluteUri)
// If the endpoint is disabled (i.e a null address is specified), return null.
if (endpoint == null)
{
throw new InvalidOperationException("An absolute URL cannot be built for the revocation endpoint path.");
return null;
}
context.RevocationEndpoint = new Uri(context.Issuer, context.RevocationEndpoint);
}
// If the endpoint address is already an absolute URL, return it as-is.
if (endpoint.IsAbsoluteUri)
{
return endpoint;
}
if (context.TokenEndpoint != null && !context.TokenEndpoint.IsAbsoluteUri)
{
if (context.Issuer == null || !context.Issuer.IsAbsoluteUri)
// At this stage, throw an exception if the issuer cannot be retrieved.
if (issuer == null || !issuer.IsAbsoluteUri)
{
throw new InvalidOperationException("An absolute URL cannot be built for the token endpoint path.");
throw new InvalidOperationException("The issuer must be a non-null, non-empty absolute URL.");
}
context.TokenEndpoint = new Uri(context.Issuer, context.TokenEndpoint);
}
// Ensure the issuer ends with a trailing slash, as it is necessary
// for Uri's constructor to correctly compute correct absolute URLs.
if (!issuer.OriginalString.EndsWith("/"))
{
issuer = new Uri(issuer.OriginalString + "/", UriKind.Absolute);
}
if (context.UserinfoEndpoint != null && !context.UserinfoEndpoint.IsAbsoluteUri)
{
if (context.Issuer == null || !context.Issuer.IsAbsoluteUri)
// Ensure the endpoint does not start with a leading slash, as it is necessary
// for Uri's constructor to correctly compute correct absolute URLs.
if (endpoint.OriginalString.StartsWith("/"))
{
throw new InvalidOperationException("An absolute URL cannot be built for the userinfo endpoint path.");
endpoint = new Uri(endpoint.OriginalString.Substring(1, endpoint.OriginalString.Length - 1), UriKind.Relative);
}
context.UserinfoEndpoint = new Uri(context.Issuer, context.UserinfoEndpoint);
return new Uri(issuer, endpoint);
}
return default;
}
}

47
src/OpenIddict.Server/OpenIddictServerHandlers.cs

@ -4038,15 +4038,9 @@ namespace OpenIddict.Server
throw new ArgumentNullException(nameof(context));
}
var endpoint = context.Options.VerificationEndpointUris.FirstOrDefault();
if (endpoint != null)
var address = GetEndpointAbsoluteUrl(context.Issuer, context.Options.VerificationEndpointUris.FirstOrDefault());
if (address != null)
{
if (context.Issuer == null || !context.Issuer.IsAbsoluteUri)
{
throw new InvalidOperationException("An absolute URL cannot be built for the device endpoint path.");
}
var address = new Uri(context.Issuer, endpoint);
var builder = new UriBuilder(address)
{
Query = string.Concat(Parameters.UserCode, "=", context.Response.UserCode)
@ -4064,6 +4058,43 @@ namespace OpenIddict.Server
}
return default;
static Uri GetEndpointAbsoluteUrl(Uri issuer, Uri endpoint)
{
// If the endpoint is disabled (i.e a null address is specified), return null.
if (endpoint == null)
{
return null;
}
// If the endpoint address is already an absolute URL, return it as-is.
if (endpoint.IsAbsoluteUri)
{
return endpoint;
}
// At this stage, throw an exception if the issuer cannot be retrieved.
if (issuer == null || !issuer.IsAbsoluteUri)
{
throw new InvalidOperationException("The issuer must be a non-null, non-empty absolute URL.");
}
// Ensure the issuer ends with a trailing slash, as it is necessary
// for Uri's constructor to correctly compute correct absolute URLs.
if (!issuer.OriginalString.EndsWith("/"))
{
issuer = new Uri(issuer.OriginalString + "/", UriKind.Absolute);
}
// Ensure the endpoint does not start with a leading slash, as it is necessary
// for Uri's constructor to correctly compute correct absolute URLs.
if (endpoint.OriginalString.StartsWith("/"))
{
endpoint = new Uri(endpoint.OriginalString.Substring(1, endpoint.OriginalString.Length - 1), UriKind.Relative);
}
return new Uri(issuer, endpoint);
}
}
}

6
src/OpenIddict.Validation/OpenIddictValidationConfiguration.cs

@ -60,6 +60,12 @@ namespace OpenIddict.Validation
options.Issuer = new Uri(options.Issuer.OriginalString + "/", UriKind.Absolute);
}
if (options.MetadataAddress.OriginalString.StartsWith("/"))
{
options.MetadataAddress = new Uri(options.MetadataAddress.OriginalString.Substring(
1, options.MetadataAddress.OriginalString.Length - 1), UriKind.Relative);
}
options.MetadataAddress = new Uri(options.Issuer, options.MetadataAddress);
}
}

118
test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Discovery.cs

@ -245,19 +245,19 @@ namespace OpenIddict.Server.FunctionalTests
}
[Fact]
public async Task HandleConfigurationRequest_EnabledEndpointsAreExposed()
public async Task HandleConfigurationRequest_AbsoluteEndpointsAreCorrectlyExposed()
{
// Arrange
var client = CreateClient(options =>
{
options.SetIssuer(new Uri("https://www.fabrikam.com/"));
options.SetAuthorizationEndpointUris("/path/authorization_endpoint")
.SetIntrospectionEndpointUris("/path/introspection_endpoint")
.SetLogoutEndpointUris("/path/logout_endpoint")
.SetRevocationEndpointUris("/path/revocation_endpoint")
.SetTokenEndpointUris("/path/token_endpoint")
.SetUserinfoEndpointUris("/path/userinfo_endpoint");
options.SetAuthorizationEndpointUris("https://www.fabrikam.com/path/authorization_endpoint")
.SetCryptographyEndpointUris("https://www.fabrikam.com/path/cryptography_endpoint")
.SetDeviceEndpointUris("https://www.fabrikam.com/path/device_endpoint")
.SetIntrospectionEndpointUris("https://www.fabrikam.com/path/introspection_endpoint")
.SetLogoutEndpointUris("https://www.fabrikam.com/path/logout_endpoint")
.SetRevocationEndpointUris("https://www.fabrikam.com/path/revocation_endpoint")
.SetTokenEndpointUris("https://www.fabrikam.com/path/token_endpoint")
.SetUserinfoEndpointUris("https://www.fabrikam.com/path/userinfo_endpoint");
});
// Act
@ -267,6 +267,15 @@ namespace OpenIddict.Server.FunctionalTests
Assert.Equal("https://www.fabrikam.com/path/authorization_endpoint",
(string) response[Metadata.AuthorizationEndpoint]);
Assert.Equal("https://www.fabrikam.com/path/cryptography_endpoint",
(string) response[Metadata.JwksUri]);
Assert.Equal("https://www.fabrikam.com/path/authorization_endpoint",
(string) response[Metadata.AuthorizationEndpoint]);
Assert.Equal("https://www.fabrikam.com/path/device_endpoint",
(string) response[Metadata.DeviceAuthorizationEndpoint]);
Assert.Equal("https://www.fabrikam.com/path/introspection_endpoint",
(string) response[Metadata.IntrospectionEndpoint]);
@ -283,6 +292,97 @@ namespace OpenIddict.Server.FunctionalTests
(string) response[Metadata.UserinfoEndpoint]);
}
[Theory]
[InlineData("https://www.fabrikam.com/tenant1", new[]
{
"path/authorization_endpoint",
"path/cryptography_endpoint",
"path/device_endpoint",
"path/introspection_endpoint",
"path/logout_endpoint",
"path/revocation_endpoint",
"path/token_endpoint",
"path/userinfo_endpoint"
})]
[InlineData("https://www.fabrikam.com/tenant1/", new[]
{
"path/authorization_endpoint",
"path/cryptography_endpoint",
"path/device_endpoint",
"path/introspection_endpoint",
"path/logout_endpoint",
"path/revocation_endpoint",
"path/token_endpoint",
"path/userinfo_endpoint"
})]
[InlineData("https://www.fabrikam.com/tenant1", new[]
{
"/path/authorization_endpoint",
"/path/cryptography_endpoint",
"/path/device_endpoint",
"/path/introspection_endpoint",
"/path/logout_endpoint",
"/path/revocation_endpoint",
"/path/token_endpoint",
"/path/userinfo_endpoint"
})]
[InlineData("https://www.fabrikam.com/tenant1/", new[]
{
"/path/authorization_endpoint",
"/path/cryptography_endpoint",
"/path/device_endpoint",
"/path/introspection_endpoint",
"/path/logout_endpoint",
"/path/revocation_endpoint",
"/path/token_endpoint",
"/path/userinfo_endpoint"
})]
public async Task HandleConfigurationRequest_RelativeEndpointsAreCorrectlyComputed(string issuer, string[] endpoints)
{
// Arrange
var client = CreateClient(options =>
{
options.SetIssuer(new Uri(issuer, UriKind.Absolute));
options.SetAuthorizationEndpointUris(endpoints[0])
.SetCryptographyEndpointUris(endpoints[1])
.SetDeviceEndpointUris(endpoints[2])
.SetIntrospectionEndpointUris(endpoints[3])
.SetLogoutEndpointUris(endpoints[4])
.SetRevocationEndpointUris(endpoints[5])
.SetTokenEndpointUris(endpoints[6])
.SetUserinfoEndpointUris(endpoints[7]);
});
// Act
var response = await client.GetAsync("/.well-known/openid-configuration");
// Assert
Assert.Equal("https://www.fabrikam.com/tenant1/path/authorization_endpoint",
(string) response[Metadata.AuthorizationEndpoint]);
Assert.Equal("https://www.fabrikam.com/tenant1/path/cryptography_endpoint",
(string) response[Metadata.JwksUri]);
Assert.Equal("https://www.fabrikam.com/tenant1/path/device_endpoint",
(string) response[Metadata.DeviceAuthorizationEndpoint]);
Assert.Equal("https://www.fabrikam.com/tenant1/path/introspection_endpoint",
(string) response[Metadata.IntrospectionEndpoint]);
Assert.Equal("https://www.fabrikam.com/tenant1/path/logout_endpoint",
(string) response[Metadata.EndSessionEndpoint]);
Assert.Equal("https://www.fabrikam.com/tenant1/path/revocation_endpoint",
(string) response[Metadata.RevocationEndpoint]);
Assert.Equal("https://www.fabrikam.com/tenant1/path/token_endpoint",
(string) response[Metadata.TokenEndpoint]);
Assert.Equal("https://www.fabrikam.com/tenant1/path/userinfo_endpoint",
(string) response[Metadata.UserinfoEndpoint]);
}
[Fact]
public async Task HandleConfigurationRequest_NoClientAuthenticationMethodIsIncludedWhenTokenEndpointIsDisabled()
{

2
test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.cs

@ -659,7 +659,7 @@ namespace OpenIddict.Server.FunctionalTests
public async Task ProcessSignIn_UnknownEndpointCausesAnException()
{
// Arrange
var client = CreateClient();
var client = CreateClient(options => options.EnableDegradedMode());
// Act and assert
var exception = await Assert.ThrowsAsync<InvalidOperationException>(delegate

Loading…
Cancel
Save