fix(llm-auth): own the api_key/auth_config exclusion in one method, document the clearing

Author: miguelgrc

apps/opik-backend/src/main/java/com/comet/opik/api/resources/v1/priv/LlmProviderApiKeyResource.java

@@ -135,7 +135,7 @@ public Response saveApiKey(
     @PATCH
     @Path("{id}")
     @RequiredPermissions(WorkspaceUserPermission.AI_PROVIDER_UPDATE)
-    @Operation(operationId = "updateLlmProviderApiKey", summary = "Update LLM Provider's ApiKey", description = "Update LLM Provider's ApiKey", responses = {
+    @Operation(operationId = "updateLlmProviderApiKey", summary = "Update LLM Provider's ApiKey", description = "Update LLM Provider's ApiKey. api_key and auth_config are mutually exclusive: setting a valid auth_config on a provider that holds a static api_key clears the stored key; send auth_config as an empty object to clear the recipe and switch back to a static key", responses = {
             @ApiResponse(responseCode = "204", description = "No Content"),
             @ApiResponse(responseCode = "401", description = "Bad Request", content = @Content(schema = @Schema(implementation = ErrorMessage.class))),
             @ApiResponse(responseCode = "403", description = "Access forbidden", content = @Content(schema = @Schema(implementation = ErrorMessage.class))),

apps/opik-backend/src/main/java/com/comet/opik/domain/LlmProviderApiKeyService.java

@@ -176,15 +176,10 @@ public void updateApiKey(@NonNull UUID id, @NonNull ProviderApiKeyUpdate provide
 
             var authConfigUpdate = resolveAuthConfigUpdate(providerApiKeyUpdate, providerApiKey);
 
-            var update = providerApiKeyUpdate;
-            if (authConfigUpdate.authConfig() != null && update.apiKey() == null) {
-                update = update.toBuilder().apiKey(EncryptionUtils.encrypt("")).build();
-            }
-
             repository.update(providerApiKey.id(),
                     workspaceId,
                     userName,
-                    update,
+                    authConfigUpdate.effectiveUpdate(),
                     authConfigUpdate.clear(),
                     authConfigUpdate.authConfig());
 
@@ -228,7 +223,15 @@ private ProviderAuthConfig resolveAuthConfigForTest(ProviderAuthCheck request, S
         return mergeSecretSentinels(incoming, stored);
     }
 
-    private record AuthConfigUpdate(boolean clear, ProviderAuthConfig authConfig) {
+    /**
+     * The resolved auth_config decision plus the effective update to persist. The api_key /
+     * auth_config mutual exclusion is enforced entirely inside
+     * {@link #resolveAuthConfigUpdate}: it both validates the incoming pair and blanks the
+     * stored key when the update switches to token auth, so callers persist
+     * {@code effectiveUpdate} as-is and cannot hold the invariant wrong.
+     */
+    private record AuthConfigUpdate(ProviderApiKeyUpdate effectiveUpdate, boolean clear,
+            ProviderAuthConfig authConfig) {
     }
 
     /**
@@ -241,10 +244,10 @@ private AuthConfigUpdate resolveAuthConfigUpdate(ProviderApiKeyUpdate update, Pr
         if (incoming == null) {
             validateNoStaticKeyConflict(
                     update.apiKey() != null ? update.apiKey() : stored.apiKey(), stored.authConfig());
-            return new AuthConfigUpdate(false, null);
+            return new AuthConfigUpdate(update, false, null);
         }
         if (incoming.isEmpty()) {
-            return new AuthConfigUpdate(true, null);
+            return new AuthConfigUpdate(update, true, null);
         }
         if (!stored.provider().supportsDynamicTokenAuth()) {
             throw new BadRequestException(
@@ -254,9 +257,15 @@ private AuthConfigUpdate resolveAuthConfigUpdate(ProviderApiKeyUpdate update, Pr
         if (!errors.isEmpty()) {
             throw new BadRequestException(String.join("; ", errors));
         }
+        // only an api_key set in this same request conflicts: an update carrying none switches
+        // the provider to token auth, which implicitly clears the stored static key below —
+        // the dialog hides the key field in token mode, so the swap must be self-contained
         validateNoStaticKeyConflict(update.apiKey(), incoming);
         var merged = mergeSecretSentinels(incoming, stored.authConfig());
-        return new AuthConfigUpdate(false, merged);
+        var effectiveUpdate = update.apiKey() == null
+                ? update.toBuilder().apiKey(EncryptionUtils.encrypt("")).build()
+                : update;
+        return new AuthConfigUpdate(effectiveUpdate, false, merged);
     }
 
     private void validateNoStaticKeyConflict(String encryptedApiKey, ProviderAuthConfig authConfig) {