From a7cf7c8987b8847984629293d8eb27908f3de3dd Mon Sep 17 00:00:00 2001 From: vnugent Date: Mon, 8 Apr 2024 22:04:04 -0400 Subject: Squashed commit of the following: commit 44803e06d1aa45496c04127930aa8897272d42f6 Author: vnugent Date: Mon Apr 8 21:41:38 2024 -0400 fix: dangling/expired session security check and cookie cleanup commit 1082bd146549a1aff47877bcd28e6be1ce0ef5e9 Author: vnugent Date: Sat Mar 30 22:20:29 2024 -0400 feat(app): Add AppData client plugin and browser library updated commit ec9b42f4cacbeae8a0b4d96e48bd9e522b3a9145 Merge: 2a11454 27b487b Author: vnugent Date: Sun Mar 24 21:16:05 2024 -0400 Merge branch 'master' into develop commit 2a114541a3bfddae887adaa98c1ed326b125d511 Author: vnugent Date: Sun Mar 24 20:53:38 2024 -0400 refactor: pull apart session authorization for future dev commit f8aea6453ddb2d56c1ce2ecb6a9e67d1af523c2e Author: vnugent Date: Thu Mar 21 14:33:21 2024 -0400 feat: Add optional svg base64 icons for social OAuth2 connections commit cc29bed99dc9e151315cce75e50d55dca306b532 Author: vnugent Date: Sun Mar 10 21:58:27 2024 -0400 source tree project location updated --- .../src/SecurityProvider/AccountSecProvider.cs | 53 +++++++++++++--------- .../src/SecurityProvider/ClientWebAuthManager.cs | 13 ++++-- 2 files changed, 39 insertions(+), 27 deletions(-) (limited to 'plugins/VNLib.Plugins.Essentials.Accounts') diff --git a/plugins/VNLib.Plugins.Essentials.Accounts/src/SecurityProvider/AccountSecProvider.cs b/plugins/VNLib.Plugins.Essentials.Accounts/src/SecurityProvider/AccountSecProvider.cs index 2e0c259..4f8bcd3 100644 --- a/plugins/VNLib.Plugins.Essentials.Accounts/src/SecurityProvider/AccountSecProvider.cs +++ b/plugins/VNLib.Plugins.Essentials.Accounts/src/SecurityProvider/AccountSecProvider.cs @@ -104,33 +104,40 @@ namespace VNLib.Plugins.Essentials.Accounts.SecurityProvider { //Expired ExpireCookies(entity, true); - + //Verbose because this is a normal occurance if (_logger.IsEnabled(LogLevel.Verbose)) { _logger.Verbose("Session {id} expired", session.SessionID[..8]); } } - else + else if (session.IsNew) { - //See if the session might be elevated - if (!ClientWebAuthManager.IsSessionElevated(in session)) - { - //If the session stored a user-agent, make sure it matches the connection - if (session.UserAgent != null && !session.UserAgent.Equals(entity.Server.UserAgent, StringComparison.Ordinal)) - { - _logger.Debug("Denied authorized connection from {ip} because user-agent changed", entity.TrustedRemoteIp); - return ValueTask.FromResult(FileProcessArgs.Deny); - } - } - - //If the session is new, or not supposed to be logged in, clear the login cookies if they were set - if (session.IsNew || string.IsNullOrEmpty(session.Token)) + //explicitly expire cookies on new sessions + ExpireCookies(entity, false); + } + //See if the session might be elevated + else if (ClientWebAuthManager.IsSessionElevated(in session)) + { + //If the session stored a user-agent, make sure it matches the connection + if (session.UserAgent != null && !session.UserAgent.Equals(entity.Server.UserAgent, StringComparison.Ordinal)) { - //Do not force clear cookies (saves bandwidth) - ExpireCookies(entity, false); + _logger.Debug("Denied authorized connection from {ip} because user-agent changed", entity.TrustedRemoteIp); + return ValueTask.FromResult(FileProcessArgs.Deny); } } + else + { + /* + * Attempts to clear client cookies if the session is not elevated + * and the client may still have cookies set from a previous session + * + * Cookies are only sent if the client also sent login cookies to avoid + * sending cookies on every request + */ + ExpireCookies(entity, false); + } + } //Always continue otherwise @@ -147,7 +154,7 @@ namespace VNLib.Plugins.Essentials.Accounts.SecurityProvider if (session.Created.AddSeconds(_config.WebSessionValidForSeconds) < entity.RequestedTimeUtc) { //Invalidate the session, so its technically valid for this request, but will be cleared on this handle close cycle - entity.Session.Invalidate(); + session.Invalidate(); //Clear auth specifc cookies _authManager.DestroyAuthorization(entity); @@ -169,7 +176,7 @@ namespace VNLib.Plugins.Essentials.Accounts.SecurityProvider ArgumentNullException.ThrowIfNull(clientInfo.PublicKey, nameof(clientInfo.PublicKey)); ArgumentNullException.ThrowIfNull(clientInfo.ClientId, nameof(clientInfo.ClientId)); - if (!entity.Session.IsSet || entity.Session.IsNew || entity.Session.SessionType != SessionType.Web) + if (!IsSessionStateValid(in entity.Session)) { throw new ArgumentException("The session is no configured for authorization"); } @@ -189,7 +196,7 @@ namespace VNLib.Plugins.Essentials.Accounts.SecurityProvider IClientAuthorization IAccountSecurityProvider.ReAuthorizeClient(HttpEntity entity) { //Confirm session is configured - if (!entity.Session.IsSet || entity.Session.IsNew || entity.Session.SessionType != SessionType.Web) + if (!IsSessionStateValid(in entity.Session)) { throw new InvalidOperationException("The session is not configured for authorization"); } @@ -219,7 +226,7 @@ namespace VNLib.Plugins.Essentials.Accounts.SecurityProvider bool IAccountSecurityProvider.IsClientAuthorized(HttpEntity entity, AuthorzationCheckLevel level) { //Session must be loaded and not-new for an authorization to exist - if(!entity.Session.IsSet || entity.Session.IsNew) + if(!IsSessionStateValid(in entity.Session)) { return false; } @@ -249,7 +256,9 @@ namespace VNLib.Plugins.Essentials.Accounts.SecurityProvider { //Use the public key supplied by the csecinfo return RsaClientDataEncryption.TryEncrypt(entity.PublicKey, data, outputBuffer); - } + } + + private static bool IsSessionStateValid(in SessionInfo session) => session.IsSet && !session.IsNew && session.SessionType == SessionType.Web; #endregion diff --git a/plugins/VNLib.Plugins.Essentials.Accounts/src/SecurityProvider/ClientWebAuthManager.cs b/plugins/VNLib.Plugins.Essentials.Accounts/src/SecurityProvider/ClientWebAuthManager.cs index c4b0c26..2c2058d 100644 --- a/plugins/VNLib.Plugins.Essentials.Accounts/src/SecurityProvider/ClientWebAuthManager.cs +++ b/plugins/VNLib.Plugins.Essentials.Accounts/src/SecurityProvider/ClientWebAuthManager.cs @@ -34,6 +34,7 @@ using System; using System.Linq; using System.Text.Json; +using System.Diagnostics; using VNLib.Hashing; using VNLib.Hashing.IdentityUtility; @@ -286,7 +287,7 @@ namespace VNLib.Plugins.Essentials.Accounts.SecurityProvider } //Get the client signature - string? base32Sig = GetSigningKey(in entity.Session); + string? base32Sig = GetSigningKey(in entity.Session); if (string.IsNullOrWhiteSpace(base32Sig)) { @@ -352,11 +353,12 @@ namespace VNLib.Plugins.Essentials.Accounts.SecurityProvider private bool VerifyConnectionOTPInternal(HttpEntity entity) { - //Get the token from the client header, the client should always sent this - string? signedMessage = entity.Server.Headers[_config.TokenHeaderName]; + Debug.Assert(IsSessionValid(in entity.Session), "Session was assumed to be valid for this call"); - //Make sure a session is loaded - if (!entity.Session.IsSet || entity.Session.IsNew || string.IsNullOrWhiteSpace(signedMessage)) + //Get the token from the client header, the client should always sent this + string? signedMessage = GetOTPHeaderValue(entity); + + if (string.IsNullOrWhiteSpace(signedMessage)) { return false; } @@ -540,6 +542,7 @@ namespace VNLib.Plugins.Essentials.Accounts.SecurityProvider => string.IsNullOrWhiteSpace(GetLoginToken(in session)) == false; private void SetPubkeyCookie(HttpEntity entity, string value) => _pubkeyCookie.SetCookie(entity, value); + private string? GetOTPHeaderValue(HttpEntity entity) => entity.Server.Headers[_config.TokenHeaderName]; private static void SetSigningKey(ref readonly SessionInfo session, string? value) => session[PUBLIC_KEY_SIG_KEY_ENTRY] = value!; private static void SetLoginToken(ref readonly SessionInfo session, string? value) => session[LOGIN_TOKEN_ENTRY] = value!; -- cgit