From 7b291da26565e6911aea60df9e02cf7c23fb9bac Mon Sep 17 00:00:00 2001 From: Adam McCoy Date: Fri, 9 Jul 2021 15:15:49 +1000 Subject: [PATCH 1/9] Client secret now comes standard --- .../IOctopusIDConfigurationStore.cs | 3 +-- .../Configuration/OctopusIDConfiguration.cs | 2 +- .../OctopusIDConfigurationResource.cs | 2 +- .../OctopusIDConfigurationStore.cs | 2 +- .../OctopusIDConfigureCommands.cs | 5 ---- .../IOpenIDConnectConfiguration.cs | 3 +++ .../IOpenIDConnectConfigurationStore.cs | 4 ++++ ...nIDConnectConfigurationWithClientSecret.cs | 9 -------- ...nnectWithClientSecretConfigurationStore.cs | 15 ------------ .../OpenIDConnectConfiguration.cs | 5 +++- .../OpenIDConnectConfigurationResource.cs | 6 +++++ .../OpenIDConnectConfigurationStore.cs | 12 ++++++++++ ...nIDConnectConfigurationWithClientSecret.cs | 17 -------------- ...ctConfigurationWithClientSecretResource.cs | 14 ----------- .../OpenIDConnectConfigureCommands.cs | 5 ++++ ...nnectWithClientSecretConfigurationStore.cs | 23 ------------------- .../Tokens/AuthTokenHandler.cs | 4 ++-- 17 files changed, 40 insertions(+), 91 deletions(-) delete mode 100644 source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfigurationWithClientSecret.cs delete mode 100644 source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectWithClientSecretConfigurationStore.cs delete mode 100644 source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationWithClientSecret.cs delete mode 100644 source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationWithClientSecretResource.cs delete mode 100644 source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectWithClientSecretConfigurationStore.cs diff --git a/source/Server.OctopusID/Configuration/IOctopusIDConfigurationStore.cs b/source/Server.OctopusID/Configuration/IOctopusIDConfigurationStore.cs index 4b33730..89121ee 100644 --- a/source/Server.OctopusID/Configuration/IOctopusIDConfigurationStore.cs +++ b/source/Server.OctopusID/Configuration/IOctopusIDConfigurationStore.cs @@ -2,8 +2,7 @@ namespace Octopus.Server.Extensibility.Authentication.OctopusID.Configuration { - interface IOctopusIDConfigurationStore : IOpenIDConnectWithClientSecretConfigurationStore, - IOpenIDConnectConfigurationWithRoleStore + interface IOctopusIDConfigurationStore : IOpenIDConnectConfigurationWithRoleStore { } } \ No newline at end of file diff --git a/source/Server.OctopusID/Configuration/OctopusIDConfiguration.cs b/source/Server.OctopusID/Configuration/OctopusIDConfiguration.cs index fce2918..16214a9 100644 --- a/source/Server.OctopusID/Configuration/OctopusIDConfiguration.cs +++ b/source/Server.OctopusID/Configuration/OctopusIDConfiguration.cs @@ -2,7 +2,7 @@ namespace Octopus.Server.Extensibility.Authentication.OctopusID.Configuration { - class OctopusIDConfiguration : OpenIDConnectConfigurationWithClientSecret, IOpenIDConnectConfigurationWithRole + class OctopusIDConfiguration : OpenIDConnectConfiguration, IOpenIDConnectConfigurationWithRole { public static string DefaultRoleClaimType = "roles"; diff --git a/source/Server.OctopusID/Configuration/OctopusIDConfigurationResource.cs b/source/Server.OctopusID/Configuration/OctopusIDConfigurationResource.cs index d240032..9d48957 100644 --- a/source/Server.OctopusID/Configuration/OctopusIDConfigurationResource.cs +++ b/source/Server.OctopusID/Configuration/OctopusIDConfigurationResource.cs @@ -5,7 +5,7 @@ namespace Octopus.Server.Extensibility.Authentication.OctopusID.Configuration { [Description("Sign in to your Octopus Server with an Octopus ID. [Learn more](https://g.octopushq.com/AuthOctopusID).")] - class OctopusIDConfigurationResource : OpenIDConnectConfigurationWithClientSecretResource + class OctopusIDConfigurationResource : OpenIDConnectConfigurationResource { [ReadOnly(true)] public override string? Issuer { get; set; } diff --git a/source/Server.OctopusID/Configuration/OctopusIDConfigurationStore.cs b/source/Server.OctopusID/Configuration/OctopusIDConfigurationStore.cs index d2e211a..02e9266 100644 --- a/source/Server.OctopusID/Configuration/OctopusIDConfigurationStore.cs +++ b/source/Server.OctopusID/Configuration/OctopusIDConfigurationStore.cs @@ -3,7 +3,7 @@ namespace Octopus.Server.Extensibility.Authentication.OctopusID.Configuration { - class OctopusIDConfigurationStore : OpenIDConnectWithClientSecretConfigurationStore, IOctopusIDConfigurationStore + class OctopusIDConfigurationStore : OpenIdConnectConfigurationStore, IOctopusIDConfigurationStore { public const string SingletonId = "authentication-octopusid"; diff --git a/source/Server.OctopusID/Configuration/OctopusIDConfigureCommands.cs b/source/Server.OctopusID/Configuration/OctopusIDConfigureCommands.cs index 2cd406c..ef3f487 100644 --- a/source/Server.OctopusID/Configuration/OctopusIDConfigureCommands.cs +++ b/source/Server.OctopusID/Configuration/OctopusIDConfigureCommands.cs @@ -26,11 +26,6 @@ public override IEnumerable GetOptions() { yield return option; } - yield return new ConfigureCommandOption($"{ConfigurationSettingsName}ClientSecret=", "Tell Octopus the shared secret to use for Octopus ID authentication requests.", v => - { - ConfigurationStore.Value.SetClientSecret(v.ToSensitiveString()); - Log.Info($"{ConfigurationSettingsName} ClientSecret set"); - }, hide: true); } } } \ No newline at end of file diff --git a/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfiguration.cs b/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfiguration.cs index bedfa0e..ccc6507 100644 --- a/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfiguration.cs +++ b/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfiguration.cs @@ -1,9 +1,12 @@ +using Octopus.Data.Model; + namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Configuration { public interface IOpenIDConnectConfiguration { string? Issuer { get; set; } string? ClientId { get; set; } + SensitiveString? ClientSecret { get; set; } string? Scope { get; set; } string? NameClaimType { get; set; } bool AllowAutoUserCreation { get; set; } diff --git a/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfigurationStore.cs b/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfigurationStore.cs index caac390..87b5289 100644 --- a/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfigurationStore.cs +++ b/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfigurationStore.cs @@ -18,6 +18,10 @@ public interface IOpenIDConnectConfigurationStore : IExtensionConfigurationStore string? GetClientId(); void SetClientId(string? clientId); + SensitiveString? GetClientSecret(); + void SetClientSecret(SensitiveString? clientSecret); + bool HasClientSecret { get; } + string? GetScope(); void SetScope(string? scope); diff --git a/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfigurationWithClientSecret.cs b/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfigurationWithClientSecret.cs deleted file mode 100644 index 4bd96d7..0000000 --- a/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectConfigurationWithClientSecret.cs +++ /dev/null @@ -1,9 +0,0 @@ -using Octopus.Data.Model; - -namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Configuration -{ - public interface IOpenIDConnectConfigurationWithClientSecret : IOpenIDConnectConfiguration - { - SensitiveString? ClientSecret { get; set; } - } -} \ No newline at end of file diff --git a/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectWithClientSecretConfigurationStore.cs b/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectWithClientSecretConfigurationStore.cs deleted file mode 100644 index 4b79213..0000000 --- a/source/Server.OpenIDConnect.Common/Configuration/IOpenIDConnectWithClientSecretConfigurationStore.cs +++ /dev/null @@ -1,15 +0,0 @@ -using Octopus.Data.Model; - -namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Configuration -{ - public interface IOpenIDConnectWithClientSecretConfigurationStore : IOpenIDConnectConfigurationStore, IOpenIDConnectWithClientSecretConfigurationStore - where TConfiguration : OpenIDConnectConfiguration, IId, new() - { - } - - public interface IOpenIDConnectWithClientSecretConfigurationStore : IOpenIDConnectConfigurationStore - { - SensitiveString? GetClientSecret(); - void SetClientSecret(SensitiveString? clientSecret); - } -} \ No newline at end of file diff --git a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfiguration.cs b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfiguration.cs index d3dce86..79167a1 100644 --- a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfiguration.cs +++ b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfiguration.cs @@ -1,4 +1,5 @@ -using Octopus.Server.Extensibility.Extensions.Infrastructure.Configuration; +using Octopus.Data.Model; +using Octopus.Server.Extensibility.Extensions.Infrastructure.Configuration; namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Configuration { @@ -27,6 +28,8 @@ protected OpenIDConnectConfiguration(string id, string name, string author, stri public string? ClientId { get; set; } + public SensitiveString? ClientSecret { get; set; } + public string? Scope { get; set; } public string? NameClaimType { get; set; } diff --git a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationResource.cs b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationResource.cs index 9b5120d..647d0b3 100644 --- a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationResource.cs +++ b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationResource.cs @@ -1,5 +1,6 @@ using System.ComponentModel; using Octopus.Server.Extensibility.Extensions.Infrastructure.Configuration; +using Octopus.Server.MessageContracts; using Octopus.Server.MessageContracts.Attributes; namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Configuration @@ -15,6 +16,11 @@ public class OpenIDConnectConfigurationResource : ExtensionConfigurationResource [Writeable] public virtual string? ClientId { get; set; } + [DisplayName("Client Secret")] + [Description("Shared secret for validating the authentication tokens")] + [Writeable] + public virtual SensitiveValue? ClientSecret { get; set; } + [DisplayName("Allow Auto User Creation")] [Description("Tell Octopus to automatically create a user account when a person signs in for the first time with this identity provider")] [Writeable] diff --git a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationStore.cs b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationStore.cs index 1087aa0..ef4e591 100644 --- a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationStore.cs +++ b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationStore.cs @@ -40,6 +40,18 @@ public void SetClientId(string? clientId) SetProperty(doc => doc.ClientId = clientId); } + public SensitiveString? GetClientSecret() + { + return GetProperty(doc => doc.ClientSecret); + } + + public void SetClientSecret(SensitiveString? clientSecret) + { + SetProperty(doc => doc.ClientSecret = clientSecret); + } + + public bool HasClientSecret => GetClientSecret() != null; + public string? GetScope() { return GetProperty(doc => doc.Scope); diff --git a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationWithClientSecret.cs b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationWithClientSecret.cs deleted file mode 100644 index 51ff55c..0000000 --- a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationWithClientSecret.cs +++ /dev/null @@ -1,17 +0,0 @@ -using Octopus.Data.Model; - -namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Configuration -{ - public abstract class OpenIDConnectConfigurationWithClientSecret : OpenIDConnectConfiguration, IOpenIDConnectConfigurationWithClientSecret - { - protected OpenIDConnectConfigurationWithClientSecret(string id) : base(id) - { - } - - protected OpenIDConnectConfigurationWithClientSecret(string id, string name, string author, string configurationSchemaVersion) : base(id, name, author, configurationSchemaVersion) - { - } - - public SensitiveString? ClientSecret { get; set; } - } -} \ No newline at end of file diff --git a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationWithClientSecretResource.cs b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationWithClientSecretResource.cs deleted file mode 100644 index 5f53df3..0000000 --- a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigurationWithClientSecretResource.cs +++ /dev/null @@ -1,14 +0,0 @@ -using System.ComponentModel; -using Octopus.Server.MessageContracts; -using Octopus.Server.MessageContracts.Attributes; - -namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Configuration -{ - public class OpenIDConnectConfigurationWithClientSecretResource : OpenIDConnectConfigurationResource - { - [DisplayName("Client Secret")] - [Description("Shared secret for validating the authentication tokens")] - [Writeable] - public virtual SensitiveValue? ClientSecret { get; set; } - } -} \ No newline at end of file diff --git a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigureCommands.cs b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigureCommands.cs index cd30ada..1c3201c 100644 --- a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigureCommands.cs +++ b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfigureCommands.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Octopus.Data.Model; using Octopus.Diagnostics; using Octopus.Server.Extensibility.Extensions.Infrastructure.Configuration; using Octopus.Server.Extensibility.HostServices.Web; @@ -61,6 +62,10 @@ protected IEnumerable GetCoreOptions(bool hide) ConfigurationStore.Value.SetClientId(v); Log.Info($"{ConfigurationSettingsName} ClientId set to: {v}"); }, hide: hide); + yield return new ConfigureCommandOption($"{ConfigurationSettingsName}ClientSecret=", $"Follow our documentation to find the Client Secret for {ConfigurationSettingsName}.", v => + { + ConfigurationStore.Value.SetClientSecret(v.ToSensitiveString()); + }, hide: hide); } public virtual IEnumerable GetOptions() diff --git a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectWithClientSecretConfigurationStore.cs b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectWithClientSecretConfigurationStore.cs deleted file mode 100644 index d479094..0000000 --- a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectWithClientSecretConfigurationStore.cs +++ /dev/null @@ -1,23 +0,0 @@ -using Octopus.Data.Model; -using Octopus.Data.Storage.Configuration; - -namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Configuration -{ - public abstract class OpenIDConnectWithClientSecretConfigurationStore : OpenIdConnectConfigurationStore, IOpenIDConnectWithClientSecretConfigurationStore - where TConfiguration : OpenIDConnectConfigurationWithClientSecret, IId, new() - { - protected OpenIDConnectWithClientSecretConfigurationStore(IConfigurationStore configurationStore) : base(configurationStore) - { - } - - public SensitiveString? GetClientSecret() - { - return GetProperty(doc => doc.ClientSecret); - } - - public void SetClientSecret(SensitiveString? clientSecret) - { - SetProperty(doc => doc.ClientSecret = clientSecret); - } - } -} \ No newline at end of file diff --git a/source/Server.OpenIDConnect.Common/Tokens/AuthTokenHandler.cs b/source/Server.OpenIDConnect.Common/Tokens/AuthTokenHandler.cs index 5383a15..dbae003 100644 --- a/source/Server.OpenIDConnect.Common/Tokens/AuthTokenHandler.cs +++ b/source/Server.OpenIDConnect.Common/Tokens/AuthTokenHandler.cs @@ -88,9 +88,9 @@ protected async Task GetPrincipalFromToken(string? acc ClaimsPrincipal ValidateUsingSharedSecret(TokenValidationParameters validationParameters, string? tokenToValidate) { - if (ConfigurationStore is IOpenIDConnectWithClientSecretConfigurationStore clientSecretStore) + if (ConfigurationStore.HasClientSecret) { - var clientSecret = clientSecretStore.GetClientSecret(); + var clientSecret = ConfigurationStore.GetClientSecret(); if (clientSecret == null) throw new ArgumentException("Client secret is not configured."); validationParameters.IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(clientSecret.Value)); From 3853554d014befecd94159792e46301cec07cfda Mon Sep 17 00:00:00 2001 From: Adam McCoy Date: Fri, 9 Jul 2021 15:27:26 +1000 Subject: [PATCH 2/9] Use auth code flow if client secret is configured --- ...ogleAppsAuthorizationEndpointUrlBuilder.cs | 2 +- .../OpenIDConnectConfiguration.cs | 3 +- .../Issuer/AuthorizationEndpointUrlBuilder.cs | 20 ++++++++--- .../IAuthorizationEndpointUrlBuilder.cs | 2 +- .../Web/UserAuthenticationAction.cs | 36 +++++++++++++------ 5 files changed, 45 insertions(+), 18 deletions(-) diff --git a/source/Server.GoogleApps/Issuer/GoogleAppsAuthorizationEndpointUrlBuilder.cs b/source/Server.GoogleApps/Issuer/GoogleAppsAuthorizationEndpointUrlBuilder.cs index cbca793..5cc52e5 100644 --- a/source/Server.GoogleApps/Issuer/GoogleAppsAuthorizationEndpointUrlBuilder.cs +++ b/source/Server.GoogleApps/Issuer/GoogleAppsAuthorizationEndpointUrlBuilder.cs @@ -11,7 +11,7 @@ public GoogleAppsAuthorizationEndpointUrlBuilder(IGoogleAppsConfigurationStore c { } - public override string Build(string requestDirectoryPath, IssuerConfiguration issuerConfiguration, string nonce, string? state = null) + public override string Build(string requestDirectoryPath, IssuerConfiguration issuerConfiguration, string? nonce = null, string? state = null) { var url = base.Build(requestDirectoryPath, issuerConfiguration, nonce, state); diff --git a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfiguration.cs b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfiguration.cs index 79167a1..0d67dba 100644 --- a/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfiguration.cs +++ b/source/Server.OpenIDConnect.Common/Configuration/OpenIDConnectConfiguration.cs @@ -5,7 +5,8 @@ namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Confi { public abstract class OpenIDConnectConfiguration : ExtensionConfigurationDocument, IOpenIDConnectConfiguration { - public const string DefaultResponseType = "code+id_token"; + public const string AuthCodeResponseType = "code"; + public const string HybridResponseType = "code+id_token"; public const string DefaultResponseMode = "form_post"; public const string DefaultScope = "openid%20profile%20email"; public const string DefaultNameClaimType = "name"; diff --git a/source/Server.OpenIDConnect.Common/Issuer/AuthorizationEndpointUrlBuilder.cs b/source/Server.OpenIDConnect.Common/Issuer/AuthorizationEndpointUrlBuilder.cs index 4d4c1fa..162d6d5 100644 --- a/source/Server.OpenIDConnect.Common/Issuer/AuthorizationEndpointUrlBuilder.cs +++ b/source/Server.OpenIDConnect.Common/Issuer/AuthorizationEndpointUrlBuilder.cs @@ -7,7 +7,6 @@ namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Issue public abstract class AuthorizationEndpointUrlBuilder : IAuthorizationEndpointUrlBuilder where TStore : IOpenIDConnectConfigurationStore { - protected readonly TStore ConfigurationStore; readonly IUrlEncoder urlEncoder; protected AuthorizationEndpointUrlBuilder(TStore configurationStore, IUrlEncoder urlEncoder) @@ -16,10 +15,11 @@ protected AuthorizationEndpointUrlBuilder(TStore configurationStore, IUrlEncoder this.urlEncoder = urlEncoder; } - protected virtual string ResponseType => OpenIDConnectConfiguration.DefaultResponseType; + protected TStore ConfigurationStore { get; } + protected virtual string ResponseType => ConfigurationStore.HasClientSecret ? OpenIDConnectConfiguration.AuthCodeResponseType : OpenIDConnectConfiguration.HybridResponseType; protected virtual string ResponseMode => OpenIDConnectConfiguration.DefaultResponseMode; - public virtual string Build(string requestDirectoryPath, IssuerConfiguration issuerConfiguration, string nonce, string? state = null) + public virtual string Build(string requestDirectoryPath, IssuerConfiguration issuerConfiguration, string? nonce = null, string? state = null) { if (issuerConfiguration == null) throw new ArgumentException("issuerConfiguration is required", nameof(issuerConfiguration)); @@ -31,13 +31,23 @@ public virtual string Build(string requestDirectoryPath, IssuerConfiguration iss var responseMode = ResponseMode; var redirectUri = requestDirectoryPath.Trim('/') + ConfigurationStore.RedirectUri; - var url = $"{issuerEndpoint}?client_id={clientId}&scope={scope}&response_type={responseType}&response_mode={responseMode}&nonce={nonce}&redirect_uri={redirectUri}"; - + var url = $"{issuerEndpoint}?client_id={clientId}&scope={scope}&response_type={responseType}&redirect_uri={redirectUri}"; + + if (!ConfigurationStore.HasClientSecret) + { + url += $"&response_mode={responseMode}"; + } + if (!string.IsNullOrWhiteSpace(state)) { url += $"&state={urlEncoder.UrlEncode(state)}"; } + if (!string.IsNullOrWhiteSpace(nonce)) + { + url += $"&nonce={nonce}"; + } + return url; } } diff --git a/source/Server.OpenIDConnect.Common/Issuer/IAuthorizationEndpointUrlBuilder.cs b/source/Server.OpenIDConnect.Common/Issuer/IAuthorizationEndpointUrlBuilder.cs index 81404ae..41f767d 100644 --- a/source/Server.OpenIDConnect.Common/Issuer/IAuthorizationEndpointUrlBuilder.cs +++ b/source/Server.OpenIDConnect.Common/Issuer/IAuthorizationEndpointUrlBuilder.cs @@ -2,6 +2,6 @@ { public interface IAuthorizationEndpointUrlBuilder { - string Build(string requestDirectoryPath, IssuerConfiguration issuerConfiguration, string nonce, string? state = null); + string Build(string requestDirectoryPath, IssuerConfiguration issuerConfiguration, string? nonce = null, string? state = null); } } \ No newline at end of file diff --git a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationAction.cs b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationAction.cs index 409e204..a86e7c2 100644 --- a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationAction.cs +++ b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationAction.cs @@ -23,7 +23,6 @@ public abstract class UserAuthenticationAction : IAsyncApiAction readonly IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer; readonly IAuthorizationEndpointUrlBuilder urlBuilder; - protected readonly TStore ConfigurationStore; readonly IApiActionModelBinder modelBinder; readonly IAuthenticationConfigurationStore authenticationConfigurationStore; @@ -43,6 +42,8 @@ protected UserAuthenticationAction( this.urlBuilder = urlBuilder; } + protected TStore ConfigurationStore { get; } + public async Task ExecuteAsync(IOctoRequest request) { if (ConfigurationStore.GetIsEnabled() == false) @@ -72,16 +73,11 @@ public async Task ExecuteAsync(IOctoRequest request) var issuer = ConfigurationStore.GetIssuer() ?? string.Empty; var issuerConfig = await identityProviderConfigDiscoverer.GetConfigurationAsync(issuer); - // Use a non-deterministic nonce to prevent replay attacks - var nonce = Nonce.GenerateUrlSafeNonce(); - - var stateString = JsonConvert.SerializeObject(state); - var url = urlBuilder.Build(model.ApiAbsUrl, issuerConfig, nonce, stateString); + var response = ConfigurationStore.HasClientSecret + ? BuildAuthorizationCodeResponse(model, state, issuerConfig) + : BuildHybridResponse(model, state, issuerConfig); - // These cookies are used to validate the data returned from the external identity provider - this prevents tampering - return Result.Response(new LoginRedirectLinkResponseModel {ExternalAuthenticationUrl = url}) - .WithCookie(new OctoCookie(UserAuthConstants.OctopusStateCookieName, State.Protect(stateString)) { HttpOnly = true, Secure = state.UsingSecureConnection, Expires = DateTimeOffset.UtcNow.AddMinutes(20) }) - .WithCookie(new OctoCookie(UserAuthConstants.OctopusNonceCookieName, Nonce.Protect(nonce)) { HttpOnly = true, Secure = state.UsingSecureConnection, Expires = DateTimeOffset.UtcNow.AddMinutes(20) }); + return response; } catch (ArgumentException ex) { @@ -94,5 +90,25 @@ public async Task ExecuteAsync(IOctoRequest request) return LoginFailed.Response(); } } + + IOctoResponseProvider BuildAuthorizationCodeResponse(LoginRedirectLinkRequestModel model, LoginState state, IssuerConfiguration issuerConfig) + { + var stateString = JsonConvert.SerializeObject(state); + var url = urlBuilder.Build(model.ApiAbsUrl, issuerConfig, state: stateString); + var response = Result.Response(new LoginRedirectLinkResponseModel {ExternalAuthenticationUrl = url}); + return response; + } + + IOctoResponseProvider BuildHybridResponse(LoginRedirectLinkRequestModel model, LoginState state, IssuerConfiguration issuerConfig) + { + var nonce = Nonce.GenerateUrlSafeNonce(); + var stateString = JsonConvert.SerializeObject(state); + var url = urlBuilder.Build(model.ApiAbsUrl, issuerConfig, nonce, stateString); + // These cookies are used to validate the data returned from the external identity provider - this prevents tampering + var response = Result.Response(new LoginRedirectLinkResponseModel {ExternalAuthenticationUrl = url}) + .WithCookie(new OctoCookie(UserAuthConstants.OctopusStateCookieName, State.Protect(stateString)) {HttpOnly = true, Secure = state.UsingSecureConnection, Expires = DateTimeOffset.UtcNow.AddMinutes(20)}) + .WithCookie(new OctoCookie(UserAuthConstants.OctopusNonceCookieName, Nonce.Protect(nonce)) {HttpOnly = true, Secure = state.UsingSecureConnection, Expires = DateTimeOffset.UtcNow.AddMinutes(20)}); + return response; + } } } \ No newline at end of file From 281764f4bf98c476fe7b73dca8f6c6c5768db360 Mon Sep 17 00:00:00 2001 From: Adam McCoy Date: Mon, 12 Jul 2021 08:50:17 +1000 Subject: [PATCH 3/9] Add endpoint for handling auth code flow --- .../Issuer/IssuerConfiguration.cs | 3 + .../Web/UserAuthenticationCodeAction.cs | 238 ++++++++++++++++++ 2 files changed, 241 insertions(+) create mode 100644 source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs diff --git a/source/Server.OpenIDConnect.Common/Issuer/IssuerConfiguration.cs b/source/Server.OpenIDConnect.Common/Issuer/IssuerConfiguration.cs index ad6d597..5586572 100644 --- a/source/Server.OpenIDConnect.Common/Issuer/IssuerConfiguration.cs +++ b/source/Server.OpenIDConnect.Common/Issuer/IssuerConfiguration.cs @@ -11,5 +11,8 @@ public class IssuerConfiguration [JsonProperty("authorization_endpoint")] public string AuthorizationEndpoint { get; set; } = string.Empty; + + [JsonProperty("token_endpoint")] + public string TokenEndpoint { get; set; } = string.Empty; } } \ No newline at end of file diff --git a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs new file mode 100644 index 0000000..1be51b0 --- /dev/null +++ b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs @@ -0,0 +1,238 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using Newtonsoft.Json; +using Octopus.Data; +using Octopus.Data.Model.User; +using Octopus.Data.Storage.User; +using Octopus.Diagnostics; +using Octopus.Server.Extensibility.Authentication.HostServices; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Configuration; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Identities; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Infrastructure; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Issuer; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Tokens; +using Octopus.Server.Extensibility.Authentication.Resources; +using Octopus.Server.Extensibility.Authentication.Resources.Identities; +using Octopus.Server.Extensibility.Extensions.Infrastructure.Web.Api; +using Octopus.Server.Extensibility.Results; +using Octopus.Time; + +namespace Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Web +{ + public abstract class UserAuthenticationCodeAction : IAsyncApiAction + where TStore : IOpenIDConnectConfigurationStore + where TAuthTokenHandler : IAuthTokenHandler + where TIdentityCreator : IIdentityCreator + { + static readonly BadRequestRegistration LoginFailed = new BadRequestRegistration("User login failed"); + static readonly RedirectRegistration Redirect = new RedirectRegistration("Redirects back to the Octopus portal"); + static readonly RequiredQueryParameterProperty Code = new RequiredQueryParameterProperty("code", "Authorization code provided by the IdP"); + static readonly OptionalQueryParameterProperty State = new OptionalQueryParameterProperty("state", "The state value associated with the authentication session"); + + readonly ISystemLog log; + readonly TAuthTokenHandler authTokenHandler; + readonly IPrincipalToUserResourceMapper principalToUserResourceMapper; + readonly IUpdateableUserStore userStore; + readonly IAuthCookieCreator authCookieCreator; + readonly IInvalidLoginTracker loginTracker; + readonly ISleep sleep; + readonly TIdentityCreator identityCreator; + readonly IClock clock; + readonly IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer; + + protected UserAuthenticationCodeAction( + ISystemLog log, + TAuthTokenHandler authTokenHandler, + IPrincipalToUserResourceMapper principalToUserResourceMapper, + IUpdateableUserStore userStore, + TStore configurationStore, + IAuthCookieCreator authCookieCreator, + IInvalidLoginTracker loginTracker, + ISleep sleep, + TIdentityCreator identityCreator, + IClock clock, + IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer) + { + this.log = log; + this.authTokenHandler = authTokenHandler; + this.principalToUserResourceMapper = principalToUserResourceMapper; + this.userStore = userStore; + this.authCookieCreator = authCookieCreator; + this.loginTracker = loginTracker; + this.sleep = sleep; + this.identityCreator = identityCreator; + this.clock = clock; + this.identityProviderConfigDiscoverer = identityProviderConfigDiscoverer; + ConfigurationStore = configurationStore; + } + + protected abstract string ProviderName { get; } + protected TStore ConfigurationStore { get; } + + public async Task ExecuteAsync(IOctoRequest request) + { + return await request.HandleAsync(Code, State, async (code, state) => + { + if (string.IsNullOrWhiteSpace(code)) + { + return BadRequest("No authorization code was provided."); + } + + var redirectUri = $"{request.Scheme}://{request.Host}{ConfigurationStore.RedirectUri}"; + var response = await RequestAuthToken(code, redirectUri); + + // Step 1: Try and get all of the details from the request making sure there are no errors passed back from the external identity provider + var principalContainer = await authTokenHandler.GetPrincipalAsync(response, out var stateStringFromRequest); + var principal = principalContainer.Principal; + if (principal == null || !string.IsNullOrEmpty(principalContainer.Error)) + { + return BadRequest($"The response from the external identity provider contained an error: {principalContainer.Error}"); + } + + var stateFromRequest = JsonConvert.DeserializeObject(stateStringFromRequest ?? state ?? string.Empty); + + // Step 3: Now the integrity of the request has been validated we can figure out which Octopus User this represents + var authenticationCandidate = principalToUserResourceMapper.MapToUserResource(principal); + if (authenticationCandidate.Username == null) + { + return BadRequest("Unable to determine username."); + } + + // Step 3a: Check if this authentication attempt is already being banned + var action = loginTracker.BeforeAttempt(authenticationCandidate.Username, request.Host); + if (action == InvalidLoginAction.Ban) + { + return BadRequest("You have had too many failed login attempts in a short period of time. Please try again later."); + } + + using (var cts = new CancellationTokenSource(TimeSpan.FromMinutes(1))) + { + // Step 3b: Try to get or create a the Octopus User this external identity represents + var userResult = GetOrCreateUser(authenticationCandidate, principalContainer.ExternalGroupIds, cts.Token); + if (userResult is ISuccessResult successResult) + { + loginTracker.RecordSucess(authenticationCandidate.Username, request.Host); + + if (!successResult.Value.IsActive) + { + return BadRequest($"The Octopus User Account '{authenticationCandidate.Username}' has been disabled by an Administrator. If you believe this to be a mistake, please contact your Octopus Administrator to have your account re-enabled."); + } + + if (successResult.Value.IsService) + { + return BadRequest($"The Octopus User Account '{authenticationCandidate.Username}' is a Service Account, which are prevented from using Octopus interactively. Service Accounts are designed to authorize external systems to access the Octopus API using an API Key."); + } + + var octoResponse = Redirect.Response(stateFromRequest.RedirectAfterLoginTo) + .WithHeader("Expires", new[] {DateTime.UtcNow.AddYears(1).ToString("R", DateTimeFormatInfo.InvariantInfo)}) + .WithCookie(new OctoCookie(UserAuthConstants.OctopusStateCookieName, Guid.NewGuid().ToString()) {HttpOnly = true, Secure = false, Expires = DateTimeOffset.MinValue}) + .WithCookie(new OctoCookie(UserAuthConstants.OctopusNonceCookieName, Guid.NewGuid().ToString()) {HttpOnly = true, Secure = false, Expires = DateTimeOffset.MinValue}); + + var authCookies = authCookieCreator.CreateAuthCookies(successResult.Value.IdentificationToken, SessionExpiry.TwentyDays, request.IsHttps, stateFromRequest.UsingSecureConnection); + + foreach (var cookie in authCookies) + { + octoResponse = octoResponse.WithCookie(cookie); + } + + return octoResponse; + } + + // Step 4: Handle other types of failures + loginTracker.RecordFailure(authenticationCandidate.Username, request.Host); + + // Step 4a: Slow this potential attacker down a bit since they seem to keep failing + if (action == InvalidLoginAction.Slow) + { + sleep.For(1000); + } + + return BadRequest($"User login failed: {((IFailureResult) userResult).ErrorString}"); + } + }); + } + + async Task> RequestAuthToken(string code, string redirectUri) + { + var issuerConfig = await identityProviderConfigDiscoverer.GetConfigurationAsync(ConfigurationStore.GetIssuer() ?? string.Empty); + using var client = new HttpClient(); + var request = new HttpRequestMessage(HttpMethod.Post, issuerConfig.TokenEndpoint); + var formValues = new Dictionary + { + ["grant_type"] = "authorization_code", + ["code"] = code, + ["redirect_uri"] = redirectUri, + ["client_id"] = ConfigurationStore.GetClientId()!, + ["client_secret"] = ConfigurationStore.GetClientSecret()!.Value + }; + request.Content = new FormUrlEncodedContent(formValues); + var response = await client.SendAsync(request, HttpCompletionOption.ResponseContentRead); + var body = await response.Content.ReadAsStringAsync(); + return JsonConvert.DeserializeObject>(body); + } + + IOctoResponseProvider BadRequest(string message) + { + log.Error(message); + return LoginFailed.Response(message); + } + + IResultFromExtension GetOrCreateUser(UserResource userResource, string[] groups, CancellationToken cancellationToken) + { + var identityToMatch = NewIdentity(userResource); + + var matchingUsers = userStore.GetByIdentity(identityToMatch); + if (matchingUsers.Count() > 1) + throw new Exception("There are multiple users with this identity. OpenID Connect identity providers do not support users with duplicate email addresses. Please remove any duplicate users, or make the email addresses unique."); + var user = matchingUsers.SingleOrDefault(); + + if (user != null) + { + userStore.SetSecurityGroupIds(ProviderName, user.Id, groups, clock.GetUtcTime()); + + var identity = user.Identities.FirstOrDefault(x => MatchesProviderAndExternalId(userResource, x)); + if (identity != null) + { + return ResultFromExtension.Success(user); + } + + identity = user.Identities.FirstOrDefault(x => x.IdentityProviderName == ProviderName && x.Claims[ClaimDescriptor.EmailClaimType].Value == userResource.EmailAddress); + if (identity != null) + { + return ResultFromExtension.Success(userStore.UpdateIdentity(user.Id, identityToMatch, cancellationToken)); + } + + return ResultFromExtension.Success(userStore.AddIdentity(user.Id, identityToMatch, cancellationToken)); + } + + if (!ConfigurationStore.GetAllowAutoUserCreation()) + return ResultFromExtension.Failed("User could not be located and auto user creation is not enabled."); + + var userResult = userStore.Create( + userResource.Username ?? string.Empty, + userResource.DisplayName ?? string.Empty, + userResource.EmailAddress ?? string.Empty, + cancellationToken, + new ProviderUserGroups { IdentityProviderName = ProviderName, GroupIds = groups }, + new[] { identityToMatch }); + if (userResult is IFailureResult failureResult) + return ResultFromExtension.Failed(failureResult.Errors); + return ResultFromExtension.Success(((ISuccessResult)userResult).Value); + } + + bool MatchesProviderAndExternalId(UserResource userResource, Identity x) + { + return x.IdentityProviderName == ProviderName && x.Claims.ContainsKey(IdentityCreator.ExternalIdClaimType) && x.Claims[IdentityCreator.ExternalIdClaimType].Value == userResource.ExternalId; + } + + Identity NewIdentity(UserResource userResource) + { + return identityCreator.Create(userResource); + } + } +} From 28822eaab3449ff22f85906aa00a428f92025583 Mon Sep 17 00:00:00 2001 From: Adam McCoy Date: Mon, 12 Jul 2021 08:54:49 +1000 Subject: [PATCH 4/9] Register endpoint for supporting extensions --- source/Server.AzureAD/AzureADApi.cs | 4 +-- source/Server.AzureAD/AzureADExtension.cs | 1 + .../AzureADUserAuthenticationCodeAction.cs | 34 +++++++++++++++++++ source/Server.GoogleApps/GoogleAppsApi.cs | 4 +-- .../Server.GoogleApps/GoogleAppsExtension.cs | 1 + .../GoogleAppsUserAuthenticationCodeAction.cs | 34 +++++++++++++++++++ source/Server.Okta/OktaApi.cs | 5 ++- source/Server.Okta/OktaExtension.cs | 1 + .../Web/OktaUserAuthenticationCodeAction.cs | 34 +++++++++++++++++++ 9 files changed, 111 insertions(+), 7 deletions(-) create mode 100644 source/Server.AzureAD/Web/AzureADUserAuthenticationCodeAction.cs create mode 100644 source/Server.GoogleApps/Web/GoogleAppsUserAuthenticationCodeAction.cs create mode 100644 source/Server.Okta/Web/OktaUserAuthenticationCodeAction.cs diff --git a/source/Server.AzureAD/AzureADApi.cs b/source/Server.AzureAD/AzureADApi.cs index 5cc1a15..3c6ffaa 100644 --- a/source/Server.AzureAD/AzureADApi.cs +++ b/source/Server.AzureAD/AzureADApi.cs @@ -1,5 +1,4 @@ -using System; -using Octopus.Server.Extensibility.Authentication.AzureAD.Configuration; +using Octopus.Server.Extensibility.Authentication.AzureAD.Configuration; using Octopus.Server.Extensibility.Authentication.AzureAD.Identities; using Octopus.Server.Extensibility.Authentication.AzureAD.Tokens; using Octopus.Server.Extensibility.Authentication.AzureAD.Web; @@ -16,6 +15,7 @@ public AzureADApi( { Add("POST", authenticationProvider.AuthenticateUri, RouteCategory.Raw, new AnonymousWhenEnabledEndpointInvocation(), null, "OpenIDConnect"); Add("POST", configurationStore.RedirectUri, RouteCategory.Raw, new AnonymousWhenEnabledEndpointInvocation(), null, "OpenIDConnect"); + Add("GET", configurationStore.RedirectUri, RouteCategory.Raw, new AnonymousWhenEnabledEndpointInvocation(), null, "OpenIDConnect"); } } } \ No newline at end of file diff --git a/source/Server.AzureAD/AzureADExtension.cs b/source/Server.AzureAD/AzureADExtension.cs index 6ade59d..8087706 100644 --- a/source/Server.AzureAD/AzureADExtension.cs +++ b/source/Server.AzureAD/AzureADExtension.cs @@ -61,6 +61,7 @@ public override void Load(ContainerBuilder builder) builder.RegisterType().AsSelf().InstancePerDependency(); builder.RegisterType().AsSelf().InstancePerDependency(); + builder.RegisterType().AsSelf().InstancePerDependency(); builder.RegisterType() .As() diff --git a/source/Server.AzureAD/Web/AzureADUserAuthenticationCodeAction.cs b/source/Server.AzureAD/Web/AzureADUserAuthenticationCodeAction.cs new file mode 100644 index 0000000..40a39a4 --- /dev/null +++ b/source/Server.AzureAD/Web/AzureADUserAuthenticationCodeAction.cs @@ -0,0 +1,34 @@ +using Octopus.Diagnostics; +using Octopus.Server.Extensibility.Authentication.AzureAD.Configuration; +using Octopus.Server.Extensibility.Authentication.AzureAD.Identities; +using Octopus.Server.Extensibility.Authentication.AzureAD.Tokens; +using Octopus.Server.Extensibility.Authentication.HostServices; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Infrastructure; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Issuer; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Web; +using Octopus.Server.Extensibility.HostServices.Web; +using Octopus.Time; + +namespace Octopus.Server.Extensibility.Authentication.AzureAD.Web +{ + class AzureADUserAuthenticationCodeAction : UserAuthenticationCodeAction + { + public AzureADUserAuthenticationCodeAction( + ISystemLog log, + IAzureADAuthTokenHandler authTokenHandler, + IPrincipalToUserResourceMapper principalToUserResourceMapper, + IUpdateableUserStore userStore, + IAzureADConfigurationStore configurationStore, + IAuthCookieCreator authCookieCreator, + IInvalidLoginTracker loginTracker, + ISleep sleep, + IAzureADIdentityCreator identityCreator, + IClock clock, + IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer) + : base(log, authTokenHandler, principalToUserResourceMapper, userStore, configurationStore, authCookieCreator, loginTracker, sleep, identityCreator, clock, identityProviderConfigDiscoverer) + { + } + + protected override string ProviderName => AzureADAuthenticationProvider.ProviderName; + } +} \ No newline at end of file diff --git a/source/Server.GoogleApps/GoogleAppsApi.cs b/source/Server.GoogleApps/GoogleAppsApi.cs index 5b54bef..494a9d9 100644 --- a/source/Server.GoogleApps/GoogleAppsApi.cs +++ b/source/Server.GoogleApps/GoogleAppsApi.cs @@ -1,5 +1,4 @@ -using System; -using Octopus.Server.Extensibility.Authentication.GoogleApps.Configuration; +using Octopus.Server.Extensibility.Authentication.GoogleApps.Configuration; using Octopus.Server.Extensibility.Authentication.GoogleApps.Identities; using Octopus.Server.Extensibility.Authentication.GoogleApps.Tokens; using Octopus.Server.Extensibility.Authentication.GoogleApps.Web; @@ -16,6 +15,7 @@ public GoogleAppsApi( { Add("POST", authenticationProvider.AuthenticateUri, RouteCategory.Raw, new AnonymousWhenEnabledEndpointInvocation(), null, "OpenIDConnect"); Add("POST", configurationStore.RedirectUri, RouteCategory.Raw, new AnonymousWhenEnabledEndpointInvocation(), null, "OpenIDConnect"); + Add("GET", configurationStore.RedirectUri, RouteCategory.Raw, new AnonymousWhenEnabledEndpointInvocation(), null, "OpenIDConnect"); } } } \ No newline at end of file diff --git a/source/Server.GoogleApps/GoogleAppsExtension.cs b/source/Server.GoogleApps/GoogleAppsExtension.cs index 8c1cbd1..614b2aa 100644 --- a/source/Server.GoogleApps/GoogleAppsExtension.cs +++ b/source/Server.GoogleApps/GoogleAppsExtension.cs @@ -59,6 +59,7 @@ public override void Load(ContainerBuilder builder) builder.RegisterType().AsSelf().InstancePerDependency(); builder.RegisterType().AsSelf().InstancePerDependency(); + builder.RegisterType().AsSelf().InstancePerDependency(); builder.RegisterType() .As() diff --git a/source/Server.GoogleApps/Web/GoogleAppsUserAuthenticationCodeAction.cs b/source/Server.GoogleApps/Web/GoogleAppsUserAuthenticationCodeAction.cs new file mode 100644 index 0000000..be833c4 --- /dev/null +++ b/source/Server.GoogleApps/Web/GoogleAppsUserAuthenticationCodeAction.cs @@ -0,0 +1,34 @@ +using Octopus.Diagnostics; +using Octopus.Server.Extensibility.Authentication.GoogleApps.Configuration; +using Octopus.Server.Extensibility.Authentication.GoogleApps.Identities; +using Octopus.Server.Extensibility.Authentication.GoogleApps.Tokens; +using Octopus.Server.Extensibility.Authentication.HostServices; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Infrastructure; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Issuer; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Web; +using Octopus.Server.Extensibility.HostServices.Web; +using Octopus.Time; + +namespace Octopus.Server.Extensibility.Authentication.GoogleApps.Web +{ + class GoogleAppsUserAuthenticationCodeAction : UserAuthenticationCodeAction + { + public GoogleAppsUserAuthenticationCodeAction( + ISystemLog log, + IGoogleAuthTokenHandler authTokenHandler, + IPrincipalToUserResourceMapper principalToUserResourceMapper, + IUpdateableUserStore userStore, + IGoogleAppsConfigurationStore configurationStore, + IAuthCookieCreator authCookieCreator, + IInvalidLoginTracker loginTracker, + ISleep sleep, + IGoogleAppsIdentityCreator identityCreator, + IClock clock, + IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer) + : base(log, authTokenHandler, principalToUserResourceMapper, userStore, configurationStore, authCookieCreator, loginTracker, sleep, identityCreator, clock, identityProviderConfigDiscoverer) + { + } + + protected override string ProviderName => GoogleAppsAuthenticationProvider.ProviderName; + } +} \ No newline at end of file diff --git a/source/Server.Okta/OktaApi.cs b/source/Server.Okta/OktaApi.cs index 83b7392..e07547f 100644 --- a/source/Server.Okta/OktaApi.cs +++ b/source/Server.Okta/OktaApi.cs @@ -1,9 +1,7 @@ -using System; -using Octopus.Server.Extensibility.Authentication.Okta.Configuration; +using Octopus.Server.Extensibility.Authentication.Okta.Configuration; using Octopus.Server.Extensibility.Authentication.Okta.Identities; using Octopus.Server.Extensibility.Authentication.Okta.Tokens; using Octopus.Server.Extensibility.Authentication.Okta.Web; -using Octopus.Server.Extensibility.Authentication.OpenIDConnect; using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common; using Octopus.Server.Extensibility.Extensions.Infrastructure.Web.Api; @@ -16,6 +14,7 @@ public OktaApi(IOktaConfigurationStore configurationStore, OktaAuthenticationPro { Add("POST", authenticationProvider.AuthenticateUri, RouteCategory.Raw, new AnonymousWhenEnabledEndpointInvocation(), null, "OpenIDConnect"); Add("POST", configurationStore.RedirectUri, RouteCategory.Raw, new AnonymousWhenEnabledEndpointInvocation(), null, "OpenIDConnect"); + Add("GET", configurationStore.RedirectUri, RouteCategory.Raw, new AnonymousWhenEnabledEndpointInvocation(), null, "OpenIDConnect"); } } } \ No newline at end of file diff --git a/source/Server.Okta/OktaExtension.cs b/source/Server.Okta/OktaExtension.cs index 17f0f93..48d9249 100644 --- a/source/Server.Okta/OktaExtension.cs +++ b/source/Server.Okta/OktaExtension.cs @@ -63,6 +63,7 @@ public override void Load(ContainerBuilder builder) builder.RegisterType().AsSelf().InstancePerDependency(); builder.RegisterType().AsSelf().InstancePerDependency(); + builder.RegisterType().AsSelf().InstancePerDependency(); builder.RegisterType() .As() diff --git a/source/Server.Okta/Web/OktaUserAuthenticationCodeAction.cs b/source/Server.Okta/Web/OktaUserAuthenticationCodeAction.cs new file mode 100644 index 0000000..13faf31 --- /dev/null +++ b/source/Server.Okta/Web/OktaUserAuthenticationCodeAction.cs @@ -0,0 +1,34 @@ +using Octopus.Diagnostics; +using Octopus.Server.Extensibility.Authentication.HostServices; +using Octopus.Server.Extensibility.Authentication.Okta.Configuration; +using Octopus.Server.Extensibility.Authentication.Okta.Identities; +using Octopus.Server.Extensibility.Authentication.Okta.Tokens; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Infrastructure; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Issuer; +using Octopus.Server.Extensibility.Authentication.OpenIDConnect.Common.Web; +using Octopus.Server.Extensibility.HostServices.Web; +using Octopus.Time; + +namespace Octopus.Server.Extensibility.Authentication.Okta.Web +{ + class OktaUserAuthenticationCodeAction : UserAuthenticationCodeAction + { + public OktaUserAuthenticationCodeAction( + ISystemLog log, + IOktaAuthTokenHandler authTokenHandler, + IPrincipalToUserResourceMapper principalToUserResourceMapper, + IUpdateableUserStore userStore, + IOktaConfigurationStore configurationStore, + IAuthCookieCreator authCookieCreator, + IInvalidLoginTracker loginTracker, + ISleep sleep, + IOktaIdentityCreator identityCreator, + IClock clock, + IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer) + : base(log, authTokenHandler, principalToUserResourceMapper, userStore, configurationStore, authCookieCreator, loginTracker, sleep, identityCreator, clock, identityProviderConfigDiscoverer) + { + } + + protected override string ProviderName => OktaAuthenticationProvider.ProviderName; + } +} \ No newline at end of file From 823c6ffe9b2da5da5fed95b5f6b77561967354dc Mon Sep 17 00:00:00 2001 From: Adam McCoy Date: Mon, 12 Jul 2021 09:32:36 +1000 Subject: [PATCH 5/9] State is not present for auth code flow --- .../Tokens/OpenIDConnectAuthTokenHandler.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source/Server.OpenIDConnect.Common/Tokens/OpenIDConnectAuthTokenHandler.cs b/source/Server.OpenIDConnect.Common/Tokens/OpenIDConnectAuthTokenHandler.cs index e27db25..02ddfdd 100644 --- a/source/Server.OpenIDConnect.Common/Tokens/OpenIDConnectAuthTokenHandler.cs +++ b/source/Server.OpenIDConnect.Common/Tokens/OpenIDConnectAuthTokenHandler.cs @@ -43,7 +43,10 @@ public Task GetPrincipalAsync(IDictionary Date: Mon, 12 Jul 2021 09:37:42 +1000 Subject: [PATCH 6/9] We're only interested in the ID token --- source/Server.OpenIDConnect.Common/Tokens/AuthTokenHandler.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/Server.OpenIDConnect.Common/Tokens/AuthTokenHandler.cs b/source/Server.OpenIDConnect.Common/Tokens/AuthTokenHandler.cs index dbae003..68f1101 100644 --- a/source/Server.OpenIDConnect.Common/Tokens/AuthTokenHandler.cs +++ b/source/Server.OpenIDConnect.Common/Tokens/AuthTokenHandler.cs @@ -47,7 +47,7 @@ protected async Task GetPrincipalFromToken(string? acc { ValidateActor = true, ValidateAudience = true, - ValidAudience = issuer + "/resources", + ValidAudience = ConfigurationStore.GetClientId(), ValidateIssuer = true, ValidIssuer = issuerConfig.Issuer, ValidateIssuerSigningKey = true @@ -56,7 +56,7 @@ protected async Task GetPrincipalFromToken(string? acc if (!string.IsNullOrWhiteSpace(ConfigurationStore.GetNameClaimType())) validationParameters.NameClaimType = ConfigurationStore.GetNameClaimType(); - var tokenToValidate = accessToken; + var tokenToValidate = idToken; if (string.IsNullOrWhiteSpace(tokenToValidate)) { // if we're validating the id_token then the audience is based on the client_id, not the issuer/resource like access_token From da510ce8b7d4c49b7e348cf24c1fa97d51822410 Mon Sep 17 00:00:00 2001 From: Adam McCoy Date: Mon, 12 Jul 2021 10:33:16 +1000 Subject: [PATCH 7/9] Tidy up --- .../Web/UserAuthenticationCodeAction.cs | 124 +++++++++--------- 1 file changed, 63 insertions(+), 61 deletions(-) diff --git a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs index 1be51b0..18b39c9 100644 --- a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs +++ b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs @@ -76,85 +76,87 @@ protected UserAuthenticationCodeAction( public async Task ExecuteAsync(IOctoRequest request) { - return await request.HandleAsync(Code, State, async (code, state) => + return await request.HandleAsync(Code, State, (code, state) => Handle(state, code, request)); + } + + async Task Handle(string code, string state, IOctoRequest request) + { + if (string.IsNullOrWhiteSpace(code)) { - if (string.IsNullOrWhiteSpace(code)) - { - return BadRequest("No authorization code was provided."); - } + return BadRequest("No authorization code was provided."); + } - var redirectUri = $"{request.Scheme}://{request.Host}{ConfigurationStore.RedirectUri}"; - var response = await RequestAuthToken(code, redirectUri); + var redirectUri = $"{request.Scheme}://{request.Host}{ConfigurationStore.RedirectUri}"; + var response = await RequestAuthToken(code, redirectUri); - // Step 1: Try and get all of the details from the request making sure there are no errors passed back from the external identity provider - var principalContainer = await authTokenHandler.GetPrincipalAsync(response, out var stateStringFromRequest); - var principal = principalContainer.Principal; - if (principal == null || !string.IsNullOrEmpty(principalContainer.Error)) - { - return BadRequest($"The response from the external identity provider contained an error: {principalContainer.Error}"); - } + // Step 1: Try and get all of the details from the request making sure there are no errors passed back from the external identity provider + var principalContainer = await authTokenHandler.GetPrincipalAsync(response, out var stateStringFromRequest); + var principal = principalContainer.Principal; + if (principal == null || !string.IsNullOrEmpty(principalContainer.Error)) + { + return BadRequest($"The response from the external identity provider contained an error: {principalContainer.Error}"); + } - var stateFromRequest = JsonConvert.DeserializeObject(stateStringFromRequest ?? state ?? string.Empty); + var stateFromRequest = JsonConvert.DeserializeObject(stateStringFromRequest ?? state ?? string.Empty); - // Step 3: Now the integrity of the request has been validated we can figure out which Octopus User this represents - var authenticationCandidate = principalToUserResourceMapper.MapToUserResource(principal); - if (authenticationCandidate.Username == null) - { - return BadRequest("Unable to determine username."); - } + // Step 3: Now the integrity of the request has been validated we can figure out which Octopus User this represents + var authenticationCandidate = principalToUserResourceMapper.MapToUserResource(principal); + if (authenticationCandidate.Username == null) + { + return BadRequest("Unable to determine username."); + } - // Step 3a: Check if this authentication attempt is already being banned - var action = loginTracker.BeforeAttempt(authenticationCandidate.Username, request.Host); - if (action == InvalidLoginAction.Ban) - { - return BadRequest("You have had too many failed login attempts in a short period of time. Please try again later."); - } + // Step 3a: Check if this authentication attempt is already being banned + var action = loginTracker.BeforeAttempt(authenticationCandidate.Username, request.Host); + if (action == InvalidLoginAction.Ban) + { + return BadRequest("You have had too many failed login attempts in a short period of time. Please try again later."); + } - using (var cts = new CancellationTokenSource(TimeSpan.FromMinutes(1))) + using (var cts = new CancellationTokenSource(TimeSpan.FromMinutes(1))) + { + // Step 3b: Try to get or create a the Octopus User this external identity represents + var userResult = GetOrCreateUser(authenticationCandidate, principalContainer.ExternalGroupIds, cts.Token); + if (userResult is ISuccessResult successResult) { - // Step 3b: Try to get or create a the Octopus User this external identity represents - var userResult = GetOrCreateUser(authenticationCandidate, principalContainer.ExternalGroupIds, cts.Token); - if (userResult is ISuccessResult successResult) - { - loginTracker.RecordSucess(authenticationCandidate.Username, request.Host); + loginTracker.RecordSucess(authenticationCandidate.Username, request.Host); - if (!successResult.Value.IsActive) - { - return BadRequest($"The Octopus User Account '{authenticationCandidate.Username}' has been disabled by an Administrator. If you believe this to be a mistake, please contact your Octopus Administrator to have your account re-enabled."); - } - - if (successResult.Value.IsService) - { - return BadRequest($"The Octopus User Account '{authenticationCandidate.Username}' is a Service Account, which are prevented from using Octopus interactively. Service Accounts are designed to authorize external systems to access the Octopus API using an API Key."); - } + if (!successResult.Value.IsActive) + { + return BadRequest($"The Octopus User Account '{authenticationCandidate.Username}' has been disabled by an Administrator. If you believe this to be a mistake, please contact your Octopus Administrator to have your account re-enabled."); + } - var octoResponse = Redirect.Response(stateFromRequest.RedirectAfterLoginTo) - .WithHeader("Expires", new[] {DateTime.UtcNow.AddYears(1).ToString("R", DateTimeFormatInfo.InvariantInfo)}) - .WithCookie(new OctoCookie(UserAuthConstants.OctopusStateCookieName, Guid.NewGuid().ToString()) {HttpOnly = true, Secure = false, Expires = DateTimeOffset.MinValue}) - .WithCookie(new OctoCookie(UserAuthConstants.OctopusNonceCookieName, Guid.NewGuid().ToString()) {HttpOnly = true, Secure = false, Expires = DateTimeOffset.MinValue}); + if (successResult.Value.IsService) + { + return BadRequest($"The Octopus User Account '{authenticationCandidate.Username}' is a Service Account, which are prevented from using Octopus interactively. Service Accounts are designed to authorize external systems to access the Octopus API using an API Key."); + } - var authCookies = authCookieCreator.CreateAuthCookies(successResult.Value.IdentificationToken, SessionExpiry.TwentyDays, request.IsHttps, stateFromRequest.UsingSecureConnection); + var octoResponse = Redirect.Response(stateFromRequest.RedirectAfterLoginTo) + .WithHeader("Expires", new[] {DateTime.UtcNow.AddYears(1).ToString("R", DateTimeFormatInfo.InvariantInfo)}) + .WithCookie(new OctoCookie(UserAuthConstants.OctopusStateCookieName, Guid.NewGuid().ToString()) {HttpOnly = true, Secure = false, Expires = DateTimeOffset.MinValue}) + .WithCookie(new OctoCookie(UserAuthConstants.OctopusNonceCookieName, Guid.NewGuid().ToString()) {HttpOnly = true, Secure = false, Expires = DateTimeOffset.MinValue}); - foreach (var cookie in authCookies) - { - octoResponse = octoResponse.WithCookie(cookie); - } + var authCookies = authCookieCreator.CreateAuthCookies(successResult.Value.IdentificationToken, SessionExpiry.TwentyDays, request.IsHttps, stateFromRequest.UsingSecureConnection); - return octoResponse; + foreach (var cookie in authCookies) + { + octoResponse = octoResponse.WithCookie(cookie); } - // Step 4: Handle other types of failures - loginTracker.RecordFailure(authenticationCandidate.Username, request.Host); + return octoResponse; + } - // Step 4a: Slow this potential attacker down a bit since they seem to keep failing - if (action == InvalidLoginAction.Slow) - { - sleep.For(1000); - } + // Step 4: Handle other types of failures + loginTracker.RecordFailure(authenticationCandidate.Username, request.Host); - return BadRequest($"User login failed: {((IFailureResult) userResult).ErrorString}"); + // Step 4a: Slow this potential attacker down a bit since they seem to keep failing + if (action == InvalidLoginAction.Slow) + { + sleep.For(1000); } - }); + + return BadRequest($"User login failed: {((IFailureResult) userResult).ErrorString}"); + } } async Task> RequestAuthToken(string code, string redirectUri) From 9dc487971cda95569ff96601c970338835763925 Mon Sep 17 00:00:00 2001 From: Adam McCoy Date: Mon, 12 Jul 2021 11:44:01 +1000 Subject: [PATCH 8/9] Reintroduce state tamper-proofing --- .../AzureADUserAuthenticationCodeAction.cs | 5 ++-- .../GoogleAppsUserAuthenticationCodeAction.cs | 5 ++-- .../Web/OktaUserAuthenticationCodeAction.cs | 5 ++-- .../Web/UserAuthenticationAction.cs | 3 ++- .../Web/UserAuthenticationCodeAction.cs | 27 ++++++++++++++++--- 5 files changed, 35 insertions(+), 10 deletions(-) diff --git a/source/Server.AzureAD/Web/AzureADUserAuthenticationCodeAction.cs b/source/Server.AzureAD/Web/AzureADUserAuthenticationCodeAction.cs index 40a39a4..aa99167 100644 --- a/source/Server.AzureAD/Web/AzureADUserAuthenticationCodeAction.cs +++ b/source/Server.AzureAD/Web/AzureADUserAuthenticationCodeAction.cs @@ -24,8 +24,9 @@ public AzureADUserAuthenticationCodeAction( ISleep sleep, IAzureADIdentityCreator identityCreator, IClock clock, - IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer) - : base(log, authTokenHandler, principalToUserResourceMapper, userStore, configurationStore, authCookieCreator, loginTracker, sleep, identityCreator, clock, identityProviderConfigDiscoverer) + IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer, + IUrlEncoder urlEncoder) + : base(log, authTokenHandler, principalToUserResourceMapper, userStore, configurationStore, authCookieCreator, loginTracker, sleep, identityCreator, clock, identityProviderConfigDiscoverer, urlEncoder) { } diff --git a/source/Server.GoogleApps/Web/GoogleAppsUserAuthenticationCodeAction.cs b/source/Server.GoogleApps/Web/GoogleAppsUserAuthenticationCodeAction.cs index be833c4..798f099 100644 --- a/source/Server.GoogleApps/Web/GoogleAppsUserAuthenticationCodeAction.cs +++ b/source/Server.GoogleApps/Web/GoogleAppsUserAuthenticationCodeAction.cs @@ -24,8 +24,9 @@ public GoogleAppsUserAuthenticationCodeAction( ISleep sleep, IGoogleAppsIdentityCreator identityCreator, IClock clock, - IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer) - : base(log, authTokenHandler, principalToUserResourceMapper, userStore, configurationStore, authCookieCreator, loginTracker, sleep, identityCreator, clock, identityProviderConfigDiscoverer) + IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer, + IUrlEncoder urlEncoder) + : base(log, authTokenHandler, principalToUserResourceMapper, userStore, configurationStore, authCookieCreator, loginTracker, sleep, identityCreator, clock, identityProviderConfigDiscoverer, urlEncoder) { } diff --git a/source/Server.Okta/Web/OktaUserAuthenticationCodeAction.cs b/source/Server.Okta/Web/OktaUserAuthenticationCodeAction.cs index 13faf31..2e43c1b 100644 --- a/source/Server.Okta/Web/OktaUserAuthenticationCodeAction.cs +++ b/source/Server.Okta/Web/OktaUserAuthenticationCodeAction.cs @@ -24,8 +24,9 @@ public OktaUserAuthenticationCodeAction( ISleep sleep, IOktaIdentityCreator identityCreator, IClock clock, - IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer) - : base(log, authTokenHandler, principalToUserResourceMapper, userStore, configurationStore, authCookieCreator, loginTracker, sleep, identityCreator, clock, identityProviderConfigDiscoverer) + IIdentityProviderConfigDiscoverer identityProviderConfigDiscoverer, + IUrlEncoder urlEncoder) + : base(log, authTokenHandler, principalToUserResourceMapper, userStore, configurationStore, authCookieCreator, loginTracker, sleep, identityCreator, clock, identityProviderConfigDiscoverer, urlEncoder) { } diff --git a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationAction.cs b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationAction.cs index a86e7c2..2936341 100644 --- a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationAction.cs +++ b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationAction.cs @@ -95,7 +95,8 @@ IOctoResponseProvider BuildAuthorizationCodeResponse(LoginRedirectLinkRequestMod { var stateString = JsonConvert.SerializeObject(state); var url = urlBuilder.Build(model.ApiAbsUrl, issuerConfig, state: stateString); - var response = Result.Response(new LoginRedirectLinkResponseModel {ExternalAuthenticationUrl = url}); + var response = Result.Response(new LoginRedirectLinkResponseModel {ExternalAuthenticationUrl = url}) + .WithCookie(new OctoCookie(UserAuthConstants.OctopusStateCookieName, State.Protect(stateString)) {HttpOnly = true, Secure = state.UsingSecureConnection, Expires = DateTimeOffset.UtcNow.AddMinutes(20)}); return response; } diff --git a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs index 18b39c9..f0a7c02 100644 --- a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs +++ b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs @@ -19,6 +19,7 @@ using Octopus.Server.Extensibility.Authentication.Resources; using Octopus.Server.Extensibility.Authentication.Resources.Identities; using Octopus.Server.Extensibility.Extensions.Infrastructure.Web.Api; +using Octopus.Server.Extensibility.HostServices.Web; using Octopus.Server.Extensibility.Results; using Octopus.Time; @@ -44,6 +45,7 @@ public abstract class UserAuthenticationCodeAction ExecuteAsync(IOctoRequest request) { - return await request.HandleAsync(Code, State, (code, state) => Handle(state, code, request)); + return await request.HandleAsync(Code, State, (code, state) => Handle(code, state, request)); } async Task Handle(string code, string state, IOctoRequest request) @@ -97,7 +101,24 @@ async Task Handle(string code, string state, IOctoRequest return BadRequest($"The response from the external identity provider contained an error: {principalContainer.Error}"); } - var stateFromRequest = JsonConvert.DeserializeObject(stateStringFromRequest ?? state ?? string.Empty); + // Step 2: Validate the state object we passed wasn't tampered with + const string stateDescription = "As a security precaution, Octopus ensures the state object returned from the external identity provider matches what it expected."; + var expectedStateHash = string.Empty; + stateStringFromRequest ??= state; + if (request.Cookies.ContainsKey(UserAuthConstants.OctopusStateCookieName)) + expectedStateHash = urlEncoder.UrlDecode(request.Cookies[UserAuthConstants.OctopusStateCookieName]); + if (string.IsNullOrWhiteSpace(expectedStateHash)) + { + return BadRequest($"User login failed: Missing State Hash Cookie. {stateDescription} In this case the Cookie containing the SHA256 hash of the state object is missing from the request."); + } + + var stateFromRequestHash = Infrastructure.State.Protect(stateStringFromRequest); + if (stateFromRequestHash != expectedStateHash) + { + return BadRequest($"User login failed: Tampered State. {stateDescription} In this case the state object looks like it has been tampered with. The state object is '{stateStringFromRequest}'. The SHA256 hash of the state was expected to be '{expectedStateHash}' but was '{stateFromRequestHash}'."); + } + + var stateFromRequest = JsonConvert.DeserializeObject(stateStringFromRequest); // Step 3: Now the integrity of the request has been validated we can figure out which Octopus User this represents var authenticationCandidate = principalToUserResourceMapper.MapToUserResource(principal); From 5bc946c2c09444b20fbe601f524e3ea66b2acab0 Mon Sep 17 00:00:00 2001 From: Adam McCoy Date: Mon, 12 Jul 2021 12:04:18 +1000 Subject: [PATCH 9/9] Remove redundant check --- .../Web/UserAuthenticationCodeAction.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs index f0a7c02..bf6f0fa 100644 --- a/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs +++ b/source/Server.OpenIDConnect.Common/Web/UserAuthenticationCodeAction.cs @@ -85,11 +85,6 @@ public async Task ExecuteAsync(IOctoRequest request) async Task Handle(string code, string state, IOctoRequest request) { - if (string.IsNullOrWhiteSpace(code)) - { - return BadRequest("No authorization code was provided."); - } - var redirectUri = $"{request.Scheme}://{request.Host}{ConfigurationStore.RedirectUri}"; var response = await RequestAuthToken(code, redirectUri);