Represent OIDC authorities as System.Uri instances to perform automat…

…ic normalization (#3269)
OrchardCMS · Mar 3, 2019 · b555810 · b555810
1 parent 676f80d
commit b555810
Show file tree

Hide file tree

Showing 17 changed files with 49 additions and 48 deletions.
diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdClientConfiguration.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdClientConfiguration.cs
@@ -60,11 +60,11 @@ public void Configure(string name, OpenIdConnectOptions options)
                 return;
             }
 
-            options.Authority = settings.Authority;
+            options.Authority = settings.Authority.AbsoluteUri;
             options.ClientId = settings.ClientId;
             options.SignedOutRedirectUri = settings.SignedOutRedirectUri ?? options.SignedOutRedirectUri;
             options.SignedOutCallbackPath = settings.SignedOutCallbackPath ?? options.SignedOutCallbackPath;
-            options.RequireHttpsMetadata = settings.Authority.StartsWith(Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase);
+            options.RequireHttpsMetadata = string.Equals(settings.Authority.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase);
             options.GetClaimsFromUserInfoEndpoint = true;
             options.ResponseMode = settings.ResponseMode;
             options.ResponseType = settings.ResponseType;

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdServerConfiguration.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdServerConfiguration.cs
@@ -106,18 +106,14 @@ public void Configure(string name, OpenIddictServerOptions options)
             options.ApplicationCanDisplayErrors = true;
             options.EnableRequestCaching = true;
             options.IgnoreScopePermissions = true;
+            options.Issuer = settings.Authority;
             options.UseRollingTokens = settings.UseRollingTokens;
 
             foreach (var key in _serverService.GetSigningKeysAsync().GetAwaiter().GetResult())
             {
                 options.SigningCredentials.AddKey(key);
             }
 
-            if (!string.IsNullOrEmpty(settings.Authority))
-            {
-                options.Issuer = new Uri(settings.Authority, UriKind.Absolute);
-            }
-
             if (settings.AccessTokenFormat == OpenIdServerSettings.TokenFormat.JWT)
             {
                 options.AccessTokenHandler = new JwtSecurityTokenHandler();
@@ -158,9 +154,9 @@ public void Configure(string name, JwtBearerOptions options)
 
             // If an authority was explicitly set in the OpenID server options,
             // prefer it to the dynamic tenant comparison as it's more efficient.
-            if (!string.IsNullOrEmpty(settings.Authority))
+            if (settings.Authority != null)
             {
-                options.TokenValidationParameters.ValidIssuer = settings.Authority;
+                options.TokenValidationParameters.ValidIssuer = settings.Authority.AbsoluteUri;
             }
             else
             {

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdValidationConfiguration.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdValidationConfiguration.cs
@@ -56,7 +56,7 @@ public void Configure(AuthenticationOptions options)
                 return;
             }
 
-            if (!string.IsNullOrEmpty(settings.Authority))
+            if (settings.Authority != null)
             {
                 options.AddScheme<JwtBearerHandler>(JwtBearerDefaults.AuthenticationScheme, displayName: null);
 
@@ -115,11 +115,11 @@ public void Configure(string name, JwtBearerOptions options)
             // Otherwise, set the authority to allow the JWT handler to retrieve the endpoint URLs/signing keys
             // from the remote server's metadata by sending an OpenID Connect/OAuth2 discovery request.
 
-            if (!string.IsNullOrEmpty(settings.Authority))
+            if (settings.Authority != null)
             {
-                options.RequireHttpsMetadata = settings.Authority.StartsWith(Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase);
+                options.RequireHttpsMetadata = string.Equals(settings.Authority.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase);
                 options.Audience = settings.Audience;
-                options.Authority = settings.Authority;
+                options.Authority = settings.Authority.AbsoluteUri;
 
                 return;
             }
@@ -148,9 +148,9 @@ public void Configure(string name, JwtBearerOptions options)
 
                 // If an authority was explicitly set in the OpenID server options,
                 // prefer it to the dynamic tenant comparison as it's more efficient.
-                if (!string.IsNullOrEmpty(configuration.Authority))
+                if (configuration.Authority != null)
                 {
-                    options.TokenValidationParameters.ValidIssuer = configuration.Authority;
+                    options.TokenValidationParameters.ValidIssuer = configuration.Authority.AbsoluteUri;
                 }
                 else
                 {

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Drivers/OpenIdClientSettingsDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Drivers/OpenIdClientSettingsDisplayDriver.cs
@@ -5,11 +5,9 @@
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.DataProtection;
 using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Mvc.Localization;
 using Microsoft.IdentityModel.Protocols.OpenIdConnect;
 using OrchardCore.DisplayManagement.Entities;
 using OrchardCore.DisplayManagement.Handlers;
-using OrchardCore.DisplayManagement.Notify;
 using OrchardCore.DisplayManagement.Views;
 using OrchardCore.Environment.Shell;
 using OrchardCore.OpenId.Configuration;
@@ -27,8 +25,6 @@ public class OpenIdClientSettingsDisplayDriver : SectionDisplayDriver<ISite, Ope
         private readonly IAuthorizationService _authorizationService;
         private readonly IDataProtectionProvider _dataProtectionProvider;
         private readonly IHttpContextAccessor _httpContextAccessor;
-        private readonly INotifier _notifier;
-        private readonly IHtmlLocalizer<OpenIdClientSettingsDisplayDriver> T;
         private readonly IOpenIdClientService _clientService;
         private readonly IShellHost _shellHost;
         private readonly ShellSettings _shellSettings;
@@ -38,19 +34,15 @@ public OpenIdClientSettingsDisplayDriver(
             IDataProtectionProvider dataProtectionProvider,
             IOpenIdClientService clientService,
             IHttpContextAccessor httpContextAccessor,
-            INotifier notifier,
-            IHtmlLocalizer<OpenIdClientSettingsDisplayDriver> stringLocalizer,
             IShellHost shellHost,
             ShellSettings shellSettings)
         {
             _authorizationService = authorizationService;
             _dataProtectionProvider = dataProtectionProvider;
             _clientService = clientService;
             _httpContextAccessor = httpContextAccessor;
-            _notifier = notifier;
             _shellHost = shellHost;
             _shellSettings = shellSettings;
-            T = stringLocalizer;
         }
 
         public override async Task<IDisplayResult> EditAsync(OpenIdClientSettings settings, BuildEditorContext context)
@@ -65,7 +57,7 @@ public override async Task<IDisplayResult> EditAsync(OpenIdClientSettings settin
             {
                 model.DisplayName = settings.DisplayName;
                 model.Scopes = settings.Scopes != null ? string.Join(" ", settings.Scopes) : null;
-                model.Authority = settings.Authority;
+                model.Authority = settings.Authority?.AbsoluteUri;
                 model.CallbackPath = settings.CallbackPath;
                 model.ClientId = settings.ClientId;
                 model.ClientSecret = settings.ClientSecret;
@@ -120,7 +112,7 @@ public override async Task<IDisplayResult> UpdateAsync(OpenIdClientSettings sett
 
                 settings.DisplayName = model.DisplayName;
                 settings.Scopes = model.Scopes.Split(new char[] { ' ', ',' }, StringSplitOptions.RemoveEmptyEntries);
-                settings.Authority = model.Authority;
+                settings.Authority = !string.IsNullOrEmpty(model.Authority) ? new Uri(model.Authority, UriKind.Absolute) : null;
                 settings.CallbackPath = model.CallbackPath;
                 settings.ClientId = model.ClientId;
                 settings.SignedOutCallbackPath = model.SignedOutCallbackPath;

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Drivers/OpenIdServerSettingsDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Drivers/OpenIdServerSettingsDisplayDriver.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
 using OrchardCore.DisplayManagement.Handlers;
@@ -21,7 +22,7 @@ public override Task<IDisplayResult> EditAsync(OpenIdServerSettings settings, Bu
             => Task.FromResult<IDisplayResult>(Initialize<OpenIdServerSettingsViewModel>("OpenIdServerSettings_Edit", async model =>
             {
                 model.AccessTokenFormat = settings.AccessTokenFormat;
-                model.Authority = settings.Authority;
+                model.Authority = settings.Authority?.AbsoluteUri;
 
                 model.CertificateStoreLocation = settings.CertificateStoreLocation;
                 model.CertificateStoreName = settings.CertificateStoreName;
@@ -64,7 +65,7 @@ public override async Task<IDisplayResult> UpdateAsync(OpenIdServerSettings sett
             await context.Updater.TryUpdateModelAsync(model, Prefix);
 
             settings.AccessTokenFormat = model.AccessTokenFormat;
-            settings.Authority = model.Authority;
+            settings.Authority = !string.IsNullOrEmpty(model.Authority) ? new Uri(model.Authority, UriKind.Absolute) : null;
 
             settings.CertificateStoreLocation = model.CertificateStoreLocation;
             settings.CertificateStoreName = model.CertificateStoreName;

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Drivers/OpenIdValidationSettingsDisplayDriver.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Drivers/OpenIdValidationSettingsDisplayDriver.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
@@ -22,7 +23,7 @@ public OpenIdValidationSettingsDisplayDriver(IShellHost shellHost)
         public override Task<IDisplayResult> EditAsync(OpenIdValidationSettings settings, BuildEditorContext context)
             => Task.FromResult<IDisplayResult>(Initialize<OpenIdValidationSettingsViewModel>("OpenIdValidationSettings_Edit", async model =>
             {
-                model.Authority = settings.Authority;
+                model.Authority = settings.Authority?.AbsoluteUri;
                 model.Audience = settings.Audience;
                 model.Tenant = settings.Tenant;
 
@@ -51,7 +52,7 @@ public override async Task<IDisplayResult> UpdateAsync(OpenIdValidationSettings
 
             await context.Updater.TryUpdateModelAsync(model, Prefix);
 
-            settings.Authority = model.Authority?.Trim();
+            settings.Authority = !string.IsNullOrEmpty(model.Authority) ? new Uri(model.Authority, UriKind.Absolute) : null;
             settings.Audience = model.Audience?.Trim();
             settings.Tenant = model.Tenant;
 

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Recipes/OpenIdClientSettingsStep.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Recipes/OpenIdClientSettingsStep.cs
@@ -1,10 +1,9 @@
 using System;
-using System.Security.Cryptography.X509Certificates;
+using System.ComponentModel.DataAnnotations;
 using System.Threading.Tasks;
 using OrchardCore.OpenId.Services;
 using OrchardCore.Recipes.Models;
 using OrchardCore.Recipes.Services;
-using static OrchardCore.OpenId.Settings.OpenIdServerSettings;
 
 namespace OrchardCore.OpenId.Recipes
 {
@@ -31,7 +30,7 @@ public async Task ExecuteAsync(RecipeExecutionContext context)
 
             var settings = await _clientService.GetSettingsAsync();
             settings.Scopes = model.Scopes.Split(' ', ',');
-            settings.Authority = model.Authority;
+            settings.Authority = !string.IsNullOrEmpty(model.Authority) ? new Uri(model.Authority, UriKind.Absolute) : null;
             settings.CallbackPath = model.CallbackPath;
             settings.ClientId = model.ClientId;
             settings.ClientSecret = model.ClientSecret;
@@ -48,6 +47,7 @@ public async Task ExecuteAsync(RecipeExecutionContext context)
     public class OpenIdClientSettingsStepModel
     {
         public string DisplayName { get; set; }
+        [Url]
         public string Authority { get; set; }
         public string ClientId { get; set; }
         public string ClientSecret { get; set; }

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Recipes/OpenIdServerSettingsStep.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Recipes/OpenIdServerSettingsStep.cs
@@ -1,4 +1,5 @@
 using System;
+using System.ComponentModel.DataAnnotations;
 using System.Security.Cryptography.X509Certificates;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
@@ -32,7 +33,7 @@ public async Task ExecuteAsync(RecipeExecutionContext context)
 
             var settings = await _serverService.GetSettingsAsync();
             settings.AccessTokenFormat = model.AccessTokenFormat;
-            settings.Authority = model.Authority;
+            settings.Authority = !string.IsNullOrEmpty(model.Authority) ? new Uri(model.Authority, UriKind.Absolute) : null;
 
             settings.CertificateStoreLocation = model.CertificateStoreLocation;
             settings.CertificateStoreName = model.CertificateStoreName;
@@ -92,6 +93,7 @@ public async Task ExecuteAsync(RecipeExecutionContext context)
     public class OpenIdServerSettingsStepModel
     {
         public TokenFormat AccessTokenFormat { get; set; } = TokenFormat.Encrypted;
+        [Url]
         public string Authority { get; set; }
         public StoreLocation CertificateStoreLocation { get; set; } = StoreLocation.LocalMachine;
         public StoreName CertificateStoreName { get; set; } = StoreName.My;

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Services/OpenIdClientService.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Services/OpenIdClientService.cs
@@ -55,21 +55,21 @@ public Task<ImmutableArray<ValidationResult>> ValidateSettingsAsync(OpenIdClient
 
             var results = ImmutableArray.CreateBuilder<ValidationResult>();
 
-            if (string.IsNullOrEmpty(settings.Authority))
+            if (settings.Authority == null)
             {
                 results.Add(new ValidationResult(T["The authority cannot be null or empty."], new[]
                 {
                     nameof(settings.Authority)
                 }));
             }
-            else if (!Uri.TryCreate(settings.Authority, UriKind.Absolute, out Uri uri) || !uri.IsWellFormedOriginalString())
+            else if (!settings.Authority.IsAbsoluteUri || !settings.Authority.IsWellFormedOriginalString())
             {
                 results.Add(new ValidationResult(T["The authority must be a valid absolute URL."], new[]
                 {
                     nameof(settings.Authority)
                 }));
             }
-            else if (!string.IsNullOrEmpty(uri.Query) || !string.IsNullOrEmpty(uri.Fragment))
+            else if (!string.IsNullOrEmpty(settings.Authority.Query) || !string.IsNullOrEmpty(settings.Authority.Fragment))
             {
                 results.Add(new ValidationResult(T["The authority cannot contain a query string or a fragment."], new[]
                 {

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Services/OpenIdServerService.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Services/OpenIdServerService.cs
@@ -97,17 +97,17 @@ public Task<ImmutableArray<ValidationResult>> ValidateSettingsAsync(OpenIdServer
                 results.Add(new ValidationResult(T["At least one OpenID Connect flow must be enabled."]));
             }
 
-            if (!string.IsNullOrEmpty(settings.Authority))
+            if (settings.Authority != null)
             {
-                if (!Uri.TryCreate(settings.Authority, UriKind.Absolute, out Uri uri) || !uri.IsWellFormedOriginalString())
+                if (!settings.Authority.IsAbsoluteUri || !settings.Authority.IsWellFormedOriginalString())
                 {
                     results.Add(new ValidationResult(T["The authority must be a valid absolute URL."], new[]
                     {
                         nameof(settings.Authority)
                     }));
                 }
 
-                if (!string.IsNullOrEmpty(uri.Query) || !string.IsNullOrEmpty(uri.Fragment))
+                if (!string.IsNullOrEmpty(settings.Authority.Query) || !string.IsNullOrEmpty(settings.Authority.Fragment))
                 {
                     results.Add(new ValidationResult(T["The authority cannot contain a query string or a fragment."], new[]
                     {

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Services/OpenIdValidationService.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Services/OpenIdValidationService.cs
@@ -78,7 +78,7 @@ public async Task<ImmutableArray<ValidationResult>> ValidateSettingsAsync(OpenId
 
             var results = ImmutableArray.CreateBuilder<ValidationResult>();
 
-            if (!(string.IsNullOrEmpty(settings.Authority) ^ string.IsNullOrEmpty(settings.Tenant)))
+            if (!(settings.Authority == null ^ string.IsNullOrEmpty(settings.Tenant)))
             {
                 results.Add(new ValidationResult(T["Either a tenant or an authority must be registered."], new[]
                 {
@@ -87,17 +87,17 @@ public async Task<ImmutableArray<ValidationResult>> ValidateSettingsAsync(OpenId
                 }));
             }
 
-            if (!string.IsNullOrEmpty(settings.Authority))
+            if (settings.Authority != null)
             {
-                if (!Uri.TryCreate(settings.Authority, UriKind.Absolute, out Uri uri) || !uri.IsWellFormedOriginalString())
+                if (!settings.Authority.IsAbsoluteUri || !settings.Authority.IsWellFormedOriginalString())
                 {
                     results.Add(new ValidationResult(T["The specified authority is not valid."], new[]
                     {
                         nameof(settings.Authority)
                     }));
                 }
 
-                if (!string.IsNullOrEmpty(uri.Query) || !string.IsNullOrEmpty(uri.Fragment))
+                if (!string.IsNullOrEmpty(settings.Authority.Query) || !string.IsNullOrEmpty(settings.Authority.Fragment))
                 {
                     results.Add(new ValidationResult(T["The authority cannot contain a query string or a fragment."], new[]
                     {
@@ -114,7 +114,7 @@ public async Task<ImmutableArray<ValidationResult>> ValidateSettingsAsync(OpenId
                 }));
             }
 
-            if (!string.IsNullOrEmpty(settings.Authority) && string.IsNullOrEmpty(settings.Audience))
+            if (settings.Authority != null && string.IsNullOrEmpty(settings.Audience))
             {
                 results.Add(new ValidationResult(T["An audience must be set when configuring the authority."], new[]
                 {

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Settings/OpenIdClientSettings.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Settings/OpenIdClientSettings.cs
@@ -1,12 +1,12 @@
+using System;
 using System.Collections.Generic;
-using Microsoft.AspNetCore.Http;
 
 namespace OrchardCore.OpenId.Settings
 {
     public class OpenIdClientSettings
     {
         public string DisplayName { get; set; }
-        public string Authority { get; set; }
+        public Uri Authority { get; set; }
         public string ClientId { get; set; }
         public string ClientSecret { get; set; }
         public string CallbackPath { get; set; }

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Settings/OpenIdServerSettings.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Settings/OpenIdServerSettings.cs
@@ -8,7 +8,7 @@ namespace OrchardCore.OpenId.Settings
     public class OpenIdServerSettings
     {
         public TokenFormat AccessTokenFormat { get; set; }
-        public string Authority { get; set; }
+        public Uri Authority { get; set; }
         public StoreLocation? CertificateStoreLocation { get; set; }
         public StoreName? CertificateStoreName { get; set; }
         public string CertificateThumbprint { get; set; }

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Settings/OpenIdValidationSettings.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/Settings/OpenIdValidationSettings.cs
@@ -1,9 +1,11 @@
+using System;
+
 namespace OrchardCore.OpenId.Settings
 {
     public class OpenIdValidationSettings
     {
         public string Audience { get; set; }
-        public string Authority { get; set; }
+        public Uri Authority { get; set; }
         public string Tenant { get; set; }
     }
 }
diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/ViewModels/OpenIdServerSettingsViewModel.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/ViewModels/OpenIdServerSettingsViewModel.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.ComponentModel.DataAnnotations;
 using System.Security.Cryptography.X509Certificates;
 using static OrchardCore.OpenId.Settings.OpenIdServerSettings;
 
@@ -8,6 +9,7 @@ namespace OrchardCore.OpenId.ViewModels
     public class OpenIdServerSettingsViewModel
     {
         public TokenFormat AccessTokenFormat { get; set; }
+        [Url]
         public string Authority { get; set; }
         public StoreLocation? CertificateStoreLocation { get; set; }
         public StoreName? CertificateStoreName { get; set; }

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/ViewModels/OpenIdValidationSettingsViewModel.cs b/src/OrchardCore.Modules/OrchardCore.OpenId/ViewModels/OpenIdValidationSettingsViewModel.cs
@@ -1,9 +1,11 @@
 using System.Collections.Generic;
+using System.ComponentModel.DataAnnotations;
 
 namespace OrchardCore.OpenId.ViewModels
 {
     public class OpenIdValidationSettingsViewModel
     {
+        [Url]
         public string Authority { get; set; }
         public string Audience { get; set; }
         public string Tenant { get; set; }

diff --git a/src/OrchardCore.Modules/OrchardCore.OpenId/Views/OpenIdServerSettings.Edit.cshtml b/src/OrchardCore.Modules/OrchardCore.OpenId/Views/OpenIdServerSettings.Edit.cshtml
@@ -81,6 +81,9 @@
 </fieldset>
 
 <h3>Advanced options</h3>
+
+<p class="alert alert-info">@T["These options are all optional and are for advanced users only."]</p>
+
 <fieldset class="form-group" asp-validation-class-for="Authority">
     <label asp-for="Authority">@T["Authority"]</label>
     <input asp-for="Authority" class="form-control" />