Improve audit logging, error handling, and metadata encoding
- Ensure all email/SMS send attempts are audit-logged, including API errors, exceptions, invalid addresses, and disabled service cases; logs now include raw API responses or exception details for better diagnostics. - Fix audit log parameter builder to never pass NULL for NOT NULL columns (`config`, `DateSent`). - Add unit tests covering all send paths and audit log correctness. - Percent-encode non-ASCII/control chars in `DocumentMetadataBuilder` for safe HTTP header metadata; add tests. - Simplify admin test mail dialog JS and improve success handler robustness. - Set `charset=utf-8` in admin controller JSON responses for OCORE consistency. - Update minified JS to match new admin dialog logic.
This commit is contained in:
@@ -85,8 +85,10 @@ public partial class IntranetController
|
||||
ContractResolver = new CamelCasePropertyNamesContractResolver(),
|
||||
};
|
||||
|
||||
// Mirror OCORE's getJSONResult exactly (application/json; charset=utf-8) so the response
|
||||
// parses identically to every other endpoint the frontend's postXT talks to.
|
||||
private ContentResult AdminJson(object payload) =>
|
||||
Content(JsonConvert.SerializeObject(payload, CamelCaseJson), "application/json");
|
||||
Content(JsonConvert.SerializeObject(payload, CamelCaseJson), "application/json; charset=utf-8");
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the calling user's <c>fds_sys</c> module authorization level via the
|
||||
|
||||
+2
-2
@@ -33,8 +33,8 @@
|
||||
<PackageReference Include="HtmlAgilityPack" Version="1.12.4" />
|
||||
<PackageReference Include="MailKit" Version="4.17.0" />
|
||||
<PackageReference Include="Microsoft.IO.RecyclableMemoryStream" Version="3.0.1" />
|
||||
<PackageReference Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" Version="1.16.0" />
|
||||
<PackageReference Include="OpenTelemetry.Extensions.Hosting" Version="1.16.0" />
|
||||
<PackageReference Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" Version="1.17.0" />
|
||||
<PackageReference Include="OpenTelemetry.Extensions.Hosting" Version="1.17.0" />
|
||||
<PackageReference Include="OpenTelemetry.Instrumentation.AspNetCore" Version="1.16.0" />
|
||||
<PackageReference Include="OpenTelemetry.Instrumentation.Http" Version="1.16.0" />
|
||||
<PackageReference Include="OpenTelemetry.Instrumentation.Runtime" Version="1.16.0" />
|
||||
|
||||
@@ -23,7 +23,7 @@ public static class DocumentMetadataBuilder
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(field)) continue;
|
||||
if (!TryGetValue(row, field, out object? value)) continue;
|
||||
metadata[field] = Stringify(value);
|
||||
metadata[field] = ToAsciiHeaderValue(Stringify(value));
|
||||
}
|
||||
return metadata;
|
||||
}
|
||||
@@ -55,4 +55,24 @@ public static class DocumentMetadataBuilder
|
||||
DateTime dt => dt.ToString("O"),
|
||||
_ => value.ToString() ?? ""
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// Azure Blob metadata is transported in HTTP headers, whose values must contain ASCII
|
||||
/// characters only. Preserve readable, printable ASCII and percent-encode every other
|
||||
/// UTF-8 byte so titles and document names containing umlauts cannot break the upload.
|
||||
/// </summary>
|
||||
private static string ToAsciiHeaderValue(string value)
|
||||
{
|
||||
if (value.All(c => c is >= ' ' and <= '~')) return value;
|
||||
|
||||
var result = new System.Text.StringBuilder(value.Length);
|
||||
foreach (byte b in System.Text.Encoding.UTF8.GetBytes(value))
|
||||
{
|
||||
if (b is >= 0x20 and <= 0x7E)
|
||||
result.Append((char)b);
|
||||
else
|
||||
result.Append('%').Append(b.ToString("X2"));
|
||||
}
|
||||
return result.ToString();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -71,6 +71,9 @@ public class ProcessWebComService : IComService
|
||||
_logger.LogWarning("SendEmailAsync: invalid email address '{Email}' for ref {Reference}", email, reference);
|
||||
FuchsTelemetry.EmailsFailed.Add(1, new KeyValuePair<string, object?>("reason", "invalid-email"));
|
||||
act?.SetStatus(ActivityStatusCode.Error, "invalid-email");
|
||||
// Every attempt is audit-logged, including this early rejection (previously it returned unlogged).
|
||||
await WriteAuditLogAsync(reference, "", "", default, false,
|
||||
[$"Invalid recipient email address: '{email}'"]);
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -116,7 +119,7 @@ public class ProcessWebComService : IComService
|
||||
_logger.LogDebug("SendEmailAsync ref={Reference} to={Email} attachments={Count}",
|
||||
reference, email, attachmentPayload.Length);
|
||||
|
||||
var (ok, responseBody) = await PostToApiAsync(reference, "e", communication);
|
||||
var (ok, status, responseBody) = await PostToApiAsync(reference, "e", communication);
|
||||
if (ok)
|
||||
{
|
||||
success = true;
|
||||
@@ -124,13 +127,17 @@ public class ProcessWebComService : IComService
|
||||
}
|
||||
else
|
||||
{
|
||||
errors.Add($"API error: {responseBody}");
|
||||
_logger.LogWarning("SendEmailAsync API error for {Reference}: {Body}", reference, responseBody);
|
||||
// Persist the mailer's raw response so an unsuccessful send is diagnosable from the log.
|
||||
errors.Add($"API error {(int)status} ({status}): {responseBody}");
|
||||
_logger.LogWarning("SendEmailAsync API error for {Reference}: {Status} {Body}",
|
||||
reference, (int)status, responseBody);
|
||||
}
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
errors.Add("Beim Versenden ist ein Fehler aufgetreten.");
|
||||
// Capture the exception detail in the audit log (not just a generic message) so a
|
||||
// transport/serialization failure is diagnosable after the fact.
|
||||
errors.Add($"Exception while sending: {ex.GetType().Name}: {ex.Message}");
|
||||
act?.SetStatus(ActivityStatusCode.Error, ex.Message);
|
||||
_logger.LogError(ex, "SendEmailAsync failed for {Reference}", reference);
|
||||
}
|
||||
@@ -171,7 +178,7 @@ public class ProcessWebComService : IComService
|
||||
body = message
|
||||
};
|
||||
|
||||
var (ok, responseBody) = await PostToApiAsync(mobile, "s", communication);
|
||||
var (ok, status, responseBody) = await PostToApiAsync(mobile, "s", communication);
|
||||
if (ok)
|
||||
{
|
||||
FuchsTelemetry.SmsSent.Add(1);
|
||||
@@ -179,7 +186,7 @@ public class ProcessWebComService : IComService
|
||||
return true;
|
||||
}
|
||||
|
||||
_logger.LogWarning("SendSmsAsync API error for {Mobile}: {Body}", mobile, responseBody);
|
||||
_logger.LogWarning("SendSmsAsync API error for {Mobile}: {Status} {Body}", mobile, (int)status, responseBody);
|
||||
return false;
|
||||
}
|
||||
catch (Exception ex)
|
||||
@@ -191,7 +198,7 @@ public class ProcessWebComService : IComService
|
||||
|
||||
// ── Private helpers ────────────────────────────────────────────────────────
|
||||
|
||||
private async Task<(bool ok, string body)> PostToApiAsync(string reference, string comtype, object communication)
|
||||
private async Task<(bool ok, System.Net.HttpStatusCode status, string body)> PostToApiAsync(string reference, string comtype, object communication)
|
||||
{
|
||||
var client = _httpClientFactory.CreateClient("ProcessWebMailer");
|
||||
|
||||
@@ -214,23 +221,21 @@ public class ProcessWebComService : IComService
|
||||
|
||||
var response = await client.PostAsync($"{_settings.BaseUrl}/mailer/push_com", content);
|
||||
string responseBody = await response.Content.ReadAsStringAsync();
|
||||
return (response.IsSuccessStatusCode, responseBody);
|
||||
return (response.IsSuccessStatusCode, response.StatusCode, responseBody);
|
||||
}
|
||||
|
||||
private async Task WriteAuditLogAsync(string reference, string guid, string config,
|
||||
/// <summary>
|
||||
/// Persists one communication attempt to <c>[dbo].[fds__logEmail]</c>. Called on every path —
|
||||
/// success, API error (with the mailer's raw response in <paramref name="errors"/>), exceptions,
|
||||
/// invalid address and the disabled-service case — so the email log is always complete. Wrapped
|
||||
/// in try/catch so a logging failure never breaks (or masks) the send itself. Overridable for tests.
|
||||
/// </summary>
|
||||
protected virtual async Task WriteAuditLogAsync(string reference, string guid, string config,
|
||||
DateTime sent, bool success, IEnumerable<string> errors)
|
||||
{
|
||||
try
|
||||
{
|
||||
var pl = new List<SqlParameter>
|
||||
{
|
||||
SQL_VarChar("@Ref", reference),
|
||||
SQL_VarChar("@guid", guid),
|
||||
SQL_DateTime("@DateSent", sent == default ? DBNull.Value : (object)sent),
|
||||
SQL_NVarChar("@config", config, dbNull_IfEmpty: true),
|
||||
SQL_Bit("@success", success),
|
||||
SQL_NVarChar("@log", JsonConvert.SerializeObject(errors.ToList()))
|
||||
};
|
||||
var pl = BuildAuditLogParameters(reference, guid, config, sent, success, errors);
|
||||
await setSQLValue_async(
|
||||
"EXECUTE [dbo].[fds__logEmail] @Ref, @guid, @DateSent, @config, @success, @log;",
|
||||
_intranet.Intranet__SQLConnectionString, pl,
|
||||
@@ -242,6 +247,28 @@ public class ProcessWebComService : IComService
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds the parameter list for <c>[dbo].[fds__logEmail]</c>. <c>fds__emaillog</c> declares
|
||||
/// <c>config</c> and <c>DateSent</c> as NOT NULL, so neither may be sent as NULL:
|
||||
/// <list type="bullet">
|
||||
/// <item><c>@config</c> is stored as an empty string (never NULL) when unset.</item>
|
||||
/// <item><c>@DateSent</c> falls back to the current (attempt) time when the send did not
|
||||
/// complete — the <c>success</c> flag distinguishes a real send-time from a failed attempt.</item>
|
||||
/// </list>
|
||||
/// Pure/side-effect-free so the NOT NULL contract can be unit-tested without a database.
|
||||
/// </summary>
|
||||
internal static List<SqlParameter> BuildAuditLogParameters(string reference, string guid,
|
||||
string config, DateTime sent, bool success, IEnumerable<string> errors) =>
|
||||
new()
|
||||
{
|
||||
SQL_VarChar("@Ref", reference),
|
||||
SQL_VarChar("@guid", guid ?? ""),
|
||||
SQL_DateTime("@DateSent", sent == default ? DateTime.UtcNow : sent),
|
||||
SQL_NVarChar("@config", config ?? ""),
|
||||
SQL_Bit("@success", success),
|
||||
SQL_NVarChar("@log", JsonConvert.SerializeObject(errors.ToList()))
|
||||
};
|
||||
|
||||
private string BuildSignature()
|
||||
{
|
||||
string sigPath = Path.Combine(AppContext.BaseDirectory,
|
||||
|
||||
@@ -230,18 +230,13 @@ let $adm = {
|
||||
button: $adt.testmail_btn,
|
||||
size: [420, 560],
|
||||
addcontent: $$.dc('adm_note').text($adt.tm_hint),
|
||||
submit: function (e) {
|
||||
let c = $(this).ldng(1);
|
||||
let qs = c.serializeObject(true, { typedvalues: true });
|
||||
$ocms.postXT({
|
||||
url: $ocms.url('admin/testmail'), data: qs, timeout: 60000, success: function (r) {
|
||||
let res = r.result || {};
|
||||
alert(res.message || (res.sent ? $adt.tm_sent : $adt.tm_failed));
|
||||
c.trigger('modal_close');
|
||||
}, error: function () {
|
||||
alert($adt.tm_failed);
|
||||
}, complete: function () { c.ldng(0); }
|
||||
});
|
||||
url: $ocms.url('admin/testmail'),
|
||||
typedvalues: true,
|
||||
/* Runs inside jQuery's success dispatch — it must never throw, or postXT's
|
||||
`complete` (which clears the page loading indicator) is skipped. */
|
||||
success: function (r) {
|
||||
let res = (r && r.result) || {};
|
||||
alert(res.message || (res.sent === true ? $adt.tm_sent : $adt.tm_failed));
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
@@ -315,18 +315,13 @@ let $adm = {
|
||||
button: $adt.testmail_btn,
|
||||
size: [420, 560],
|
||||
addcontent: $$.dc('adm_note').text($adt.tm_hint),
|
||||
submit: function (e) {
|
||||
let c = $(this).ldng(1);
|
||||
let qs = c.serializeObject(true, { typedvalues: true });
|
||||
$ocms.postXT({
|
||||
url: $ocms.url('admin/testmail'), data: qs, timeout: 60000, success: function (r) {
|
||||
let res = r.result || {};
|
||||
alert(res.message || (res.sent ? $adt.tm_sent : $adt.tm_failed));
|
||||
c.trigger('modal_close');
|
||||
}, error: function () {
|
||||
alert($adt.tm_failed);
|
||||
}, complete: function () { c.ldng(0); }
|
||||
});
|
||||
url: $ocms.url('admin/testmail'),
|
||||
typedvalues: true,
|
||||
/* Runs inside jQuery's success dispatch — it must never throw, or postXT's
|
||||
`complete` (which clears the page loading indicator) is skipped. */
|
||||
success: function (r) {
|
||||
let res = (r && r.result) || {};
|
||||
alert(res.message || (res.sent === true ? $adt.tm_sent : $adt.tm_failed));
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user