From e9a55761c259ca61bd5e098c765136d1e9e757ea Mon Sep 17 00:00:00 2001 From: LD-Reborn Date: Mon, 24 Nov 2025 11:02:58 +0100 Subject: [PATCH] Improved LdapService thread safety, fixed user images sometimes not loading --- src/Controllers/HomeController.cs | 3 +- src/Services/LdapService.cs | 92 +++++++++++++++++++++++-------- 2 files changed, 70 insertions(+), 25 deletions(-) diff --git a/src/Controllers/HomeController.cs b/src/Controllers/HomeController.cs index fc3e440..a4a467a 100644 --- a/src/Controllers/HomeController.cs +++ b/src/Controllers/HomeController.cs @@ -98,7 +98,6 @@ public class HomeController : Controller [ResponseCache(Duration = 3600, Location = ResponseCacheLocation.Any, VaryByQueryKeys = new[] { "uid", "size" })] public async Task UserPhotoAsync(string uid, int? size) { - Task adminSettingsModelTask = _ldap.GetAdminSettingsModelAsync(); UserModel? user = await _ldap.GetUserByUidAsync(uid, _ldap.UsersAttributes); if (user is null || user.JpegPhoto is null || user.JpegPhoto == "") { @@ -110,7 +109,7 @@ public class HomeController : Controller } if (size is not null) { - AdminSettingsModel adminSettingsModel = await adminSettingsModelTask; + AdminSettingsModel adminSettingsModel = await _ldap.GetAdminSettingsModelAsync(); size = Math.Min((int)size, adminSettingsModel.MaxDownloadableUserImageSize); } byte[] encodedFile = ImageHelper.ResizeAndConvertToWebp(user.JpegPhoto, size ?? 32); diff --git a/src/Services/LdapService.cs b/src/Services/LdapService.cs index bb74a51..17a3f6a 100644 --- a/src/Services/LdapService.cs +++ b/src/Services/LdapService.cs @@ -11,6 +11,7 @@ public partial class LdapService : IDisposable { private readonly LdapConfig _opts; private readonly LdapConnection _conn; + private readonly SemaphoreSlim _connLock = new(1, 1); private AdminSettingsModel? adminSettingsModel; private ILogger _logger; @@ -29,19 +30,27 @@ public partial class LdapService : IDisposable { try { - if (!_conn.Connected) + await _connLock.WaitAsync(); + try { - try + if (!_conn.Connected) { - await _conn.ConnectAsync(_opts.Host, _opts.Port); - } - catch (SystemException ex) - { - _logger.LogWarning("Unable to connect to LDAP: {ex.Message}\n{ex.StackTrace}", [ex.Message, ex.StackTrace]); - throw; + try + { + await _conn.ConnectAsync(_opts.Host, _opts.Port); + } + catch (SystemException ex) + { + _logger.LogWarning("Unable to connect to LDAP: {ex.Message}\n{ex.StackTrace}", [ex.Message, ex.StackTrace]); + throw; + } } + await _conn.BindAsync(_opts.BindDn, _opts.BindPassword); + } + finally + { + _connLock.Release(); } - await _conn.BindAsync(_opts.BindDn, _opts.BindPassword); return; } catch (Exception ex) @@ -109,7 +118,7 @@ public partial class LdapService : IDisposable LdapModification.Replace, new LdapAttribute("description", targetText) ); - await _conn.ModifyAsync(dn, modification); + await ModifyAsync(dn, modification); } catch (Exception) { @@ -151,7 +160,7 @@ public partial class LdapService : IDisposable LdapModification.Replace, new LdapAttribute("description", targetText) ); - await _conn.ModifyAsync(dn, modification); + await ModifyAsync(dn, modification); } catch (Exception) { @@ -201,7 +210,7 @@ public partial class LdapService : IDisposable LdapModification.Replace, new LdapAttribute("description", targetText) ); - await _conn.ModifyAsync(dn, modification); + await ModifyAsync(dn, modification); } catch (Exception) { @@ -388,15 +397,16 @@ public async Task CreateAsset(LdapAttributeSet attributeSet) public async Task>> ListObjectBy(string baseDn, string filter, string[] attributes) { - return await Task.Run(async () => + await ConnectAndBind(); + await _connLock.WaitAsync(); + try { - await ConnectAndBind(); var search = await _conn.SearchAsync( baseDn, LdapConnection.ScopeSub, $"{filter}", attributes, - false); + false).ConfigureAwait(false); var list = new List>(); while (await search.HasMoreAsync()) { @@ -415,7 +425,11 @@ public async Task CreateAsset(LdapAttributeSet attributeSet) catch (LdapException) { } } return list; - }); + } + finally + { + _connLock.Release(); + } } public async Task DeleteUserAsync(string uid) @@ -468,7 +482,15 @@ public async Task CreateAsset(LdapAttributeSet attributeSet) string dn = PrependRDN($"{rdnKey}={rdnValue}", baseDn); if (attributeName == rdnKey) { - await _conn.RenameAsync(dn, $"{rdnKey}={attributeValue}", true); + await _connLock.WaitAsync(); + try + { + await _conn.RenameAsync(dn, $"{rdnKey}={attributeValue}", true); + } + finally + { + _connLock.Release(); + } } else { @@ -476,7 +498,7 @@ public async Task CreateAsset(LdapAttributeSet attributeSet) LdapModification.Replace, new LdapAttribute(attributeName, attributeValue) ); - await _conn.ModifyAsync(dn, modification); + await ModifyAsync(dn, modification); } } @@ -490,24 +512,48 @@ public async Task CreateAsset(LdapAttributeSet attributeSet) new LdapAttribute(attributeName) ); - await _conn.ModifyAsync(dn, modification); + await ModifyAsync(dn, modification); } public async Task DeleteObjectByDnAsync(string dn) { - await _conn.DeleteAsync(dn); + await _connLock.WaitAsync(); + try + { + await _conn.DeleteAsync(dn); + } + finally + { + _connLock.Release(); + } } public async Task CreateObject(string dn, LdapAttributeSet attributeSet) { await ConnectAndBind(); LdapEntry ldapEntry = new(dn, attributeSet); - await _conn.AddAsync(ldapEntry); + await _connLock.WaitAsync(); + try + { + await _conn.AddAsync(ldapEntry); + } + finally + { + _connLock.Release(); + } } - public async Task ModifyAsync(string dn, LdapModification ldapModification) + public async Task ModifyAsync(string dn, LdapModification mod, CancellationToken ct = default) { - await _conn.ModifyAsync(dn, ldapModification); + await _connLock.WaitAsync(ct); + try + { + await _conn.ModifyAsync(dn, mod, ct); + } + finally + { + _connLock.Release(); + } } public void Dispose()