fix(auth): C3 review — surface seam not-found (no silent success), partial-reconcile-failure guidance, create validation order, concurrent-edit reconciler test
This commit is contained in:
@@ -170,6 +170,12 @@
|
|||||||
{
|
{
|
||||||
_formError = null;
|
_formError = null;
|
||||||
|
|
||||||
|
if (!IsEditMode && string.IsNullOrWhiteSpace(_formName))
|
||||||
|
{
|
||||||
|
_formError = "Name is required.";
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// The seam/server reject empty scope sets; validate in the UI for a clear message.
|
// The seam/server reject empty scope sets; validate in the UI for a clear message.
|
||||||
if (_selectedMethodNames.Count == 0)
|
if (_selectedMethodNames.Count == 0)
|
||||||
{
|
{
|
||||||
@@ -182,13 +188,16 @@
|
|||||||
if (_editingKey != null)
|
if (_editingKey != null)
|
||||||
{
|
{
|
||||||
// Edit: name is fixed; only the method-scope set is mutable.
|
// Edit: name is fixed; only the method-scope set is mutable.
|
||||||
await ApiKeyAdmin.SetMethodsAsync(_editingKey.KeyId, _selectedMethodNames.ToList());
|
var ok = await ApiKeyAdmin.SetMethodsAsync(_editingKey.KeyId, _selectedMethodNames.ToList());
|
||||||
|
if (!ok)
|
||||||
|
{
|
||||||
|
_formError = $"API key '{_editingKey.Name}' was not found. Reload and retry.";
|
||||||
|
return;
|
||||||
|
}
|
||||||
NavigationManager.NavigateTo("/admin/api-keys");
|
NavigationManager.NavigateTo("/admin/api-keys");
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
if (string.IsNullOrWhiteSpace(_formName)) { _formError = "Name is required."; return; }
|
|
||||||
|
|
||||||
var created = await ApiKeyAdmin.CreateAsync(_formName.Trim(), _selectedMethodNames.ToList());
|
var created = await ApiKeyAdmin.CreateAsync(_formName.Trim(), _selectedMethodNames.ToList());
|
||||||
_newlyCreatedKeyId = created.KeyId;
|
_newlyCreatedKeyId = created.KeyId;
|
||||||
_newlyCreatedToken = created.Token; // shown once; never persisted client-side.
|
_newlyCreatedToken = created.Token; // shown once; never persisted client-side.
|
||||||
|
|||||||
@@ -146,7 +146,13 @@
|
|||||||
{
|
{
|
||||||
var newEnabled = !key.Enabled;
|
var newEnabled = !key.Enabled;
|
||||||
// The seam persists; there is no separate SaveChangesAsync.
|
// The seam persists; there is no separate SaveChangesAsync.
|
||||||
await ApiKeyAdmin.SetEnabledAsync(key.KeyId, newEnabled);
|
var ok = await ApiKeyAdmin.SetEnabledAsync(key.KeyId, newEnabled);
|
||||||
|
if (!ok)
|
||||||
|
{
|
||||||
|
_toast.ShowError($"API key '{key.Name}' was not found — it may have been removed. Refreshing.");
|
||||||
|
await LoadDataAsync();
|
||||||
|
return;
|
||||||
|
}
|
||||||
_toast.ShowSuccess($"API key '{key.Name}' {(newEnabled ? "enabled" : "disabled")}.");
|
_toast.ShowSuccess($"API key '{key.Name}' {(newEnabled ? "enabled" : "disabled")}.");
|
||||||
await LoadDataAsync();
|
await LoadDataAsync();
|
||||||
}
|
}
|
||||||
@@ -166,7 +172,13 @@
|
|||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
await ApiKeyAdmin.DeleteAsync(key.KeyId);
|
var ok = await ApiKeyAdmin.DeleteAsync(key.KeyId);
|
||||||
|
if (!ok)
|
||||||
|
{
|
||||||
|
_toast.ShowError($"API key '{key.Name}' was not found — it may have been removed. Refreshing.");
|
||||||
|
await LoadDataAsync();
|
||||||
|
return;
|
||||||
|
}
|
||||||
_toast.ShowSuccess($"API key '{key.Name}' deleted.");
|
_toast.ShowSuccess($"API key '{key.Name}' deleted.");
|
||||||
await LoadDataAsync();
|
await LoadDataAsync();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -331,9 +331,21 @@
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
foreach (var update in plan.Updates)
|
try
|
||||||
{
|
{
|
||||||
await ApiKeyAdmin.SetMethodsAsync(update.KeyId, update.NewMethods);
|
foreach (var update in plan.Updates)
|
||||||
|
{
|
||||||
|
var ok = await ApiKeyAdmin.SetMethodsAsync(update.KeyId, update.NewMethods);
|
||||||
|
if (!ok)
|
||||||
|
throw new InvalidOperationException(
|
||||||
|
$"Key '{NameFor(update.KeyId)}' was not found in the key store.");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
catch (Exception ex)
|
||||||
|
{
|
||||||
|
throw new InvalidOperationException(
|
||||||
|
$"Method '{methodName}' was saved, but updating approved-key scopes failed partway: {ex.Message} " +
|
||||||
|
"Some keys may be partially updated — review them on the API Keys page and retry.", ex);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Selection is now the baseline (matters if save is retried without reload).
|
// Selection is now the baseline (matters if save is retried without reload).
|
||||||
@@ -341,6 +353,10 @@
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Returns the display name for a keyId if available from the loaded key list, else the id itself.
|
||||||
|
private string NameFor(string keyId) =>
|
||||||
|
_allKeys.FirstOrDefault(k => string.Equals(k.KeyId, keyId, StringComparison.Ordinal))?.Name ?? keyId;
|
||||||
|
|
||||||
private void GoBack() => NavigationManager.NavigateTo("/design/external-systems");
|
private void GoBack() => NavigationManager.NavigateTo("/design/external-systems");
|
||||||
|
|
||||||
private void ToggleTestRunPanel() => _showTestRun = !_showTestRun;
|
private void ToggleTestRunPanel() => _showTestRun = !_showTestRun;
|
||||||
|
|||||||
+24
@@ -115,4 +115,28 @@ public sealed class ApiMethodKeyScopeReconcilerTests
|
|||||||
Assert.Empty(result.Updates);
|
Assert.Empty(result.Updates);
|
||||||
Assert.Empty(result.EmptyScopeKeyNames);
|
Assert.Empty(result.EmptyScopeKeyNames);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Concurrent-edit guard: if selected == initial (no diff), the reconciler must produce
|
||||||
|
/// ZERO updates even when the live store shows different scopes on that key. The reconciler
|
||||||
|
/// only acts on keys that appear in the diff (added or removed relative to <c>initialKeyIds</c>)
|
||||||
|
/// — it must never touch keys that are not in the diff, regardless of what their current
|
||||||
|
/// live scopes look like.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void NoDiff_ProducesNoUpdates_EvenWhenLiveScopesDiffer()
|
||||||
|
{
|
||||||
|
// k1 was approved at load time and is still approved — no diff.
|
||||||
|
// However, a concurrent edit changed k1's live scopes to include an extra method.
|
||||||
|
var result = ApiMethodKeyScopeReconciler.Reconcile(
|
||||||
|
methodName: "PlaceOrder",
|
||||||
|
selectedKeyIds: new HashSet<string> { "k1" },
|
||||||
|
initialKeyIds: new HashSet<string> { "k1" },
|
||||||
|
currentMethodsByKey: Current(("k1", new[] { "PlaceOrder", "OtherMethod" })),
|
||||||
|
keyNamesById: Names(("k1", "Key One")));
|
||||||
|
|
||||||
|
// k1 is not in the diff → reconciler must not touch it.
|
||||||
|
Assert.Empty(result.Updates);
|
||||||
|
Assert.Empty(result.EmptyScopeKeyNames);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user