Skip to content

Commit a9afa80

Browse files
committed
fix(llm-auth): class-level constraint for the auth-check request, 422 for validation
1 parent d2dd1e4 commit a9afa80

7 files changed

Lines changed: 71 additions & 19 deletions

File tree

apps/opik-backend/src/main/java/com/comet/opik/api/ProviderAuthCheck.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.comet.opik.api;
22

3+
import com.comet.opik.api.validation.ProviderAuthCheckValidation;
34
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
45
import com.fasterxml.jackson.databind.PropertyNamingStrategies;
56
import com.fasterxml.jackson.databind.annotation.JsonNaming;
@@ -18,6 +19,7 @@
1819
@Builder(toBuilder = true)
1920
@JsonIgnoreProperties(ignoreUnknown = true)
2021
@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
22+
@ProviderAuthCheckValidation
2123
public record ProviderAuthCheck(
2224
@Schema(description = "Test the stored auth config of this provider; also the sentinel-resolution target when auth_config is sent") UUID providerId,
2325
@Valid @Schema(description = "Auth config to test as-submitted; omit to test the stored one") ProviderAuthConfig authConfig) {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,8 @@ public Response updateApiKey(@PathParam("id") UUID id,
161161
+
162162
"Send provider_id to test the stored config, auth_config to test submitted values, or both to resolve secret sentinels against the stored config.", responses = {
163163
@ApiResponse(responseCode = "200", description = "Token fetched", content = @Content(schema = @Schema(implementation = ProviderAuthCheck.Result.class))),
164-
@ApiResponse(responseCode = "400", description = "Bad Request", content = @Content(schema = @Schema(implementation = ErrorMessage.class))),
164+
@ApiResponse(responseCode = "400", description = "Bad Request — the token fetch itself failed (unreachable URL, rejected credentials, malformed reply)", content = @Content(schema = @Schema(implementation = ErrorMessage.class))),
165+
@ApiResponse(responseCode = "422", description = "Unprocessable Content — the request is invalid (neither provider_id nor auth_config, or an invalid auth_config)", content = @Content(schema = @Schema(implementation = ErrorMessage.class))),
165166
@ApiResponse(responseCode = "403", description = "Access forbidden", content = @Content(schema = @Schema(implementation = ErrorMessage.class))),
166167
@ApiResponse(responseCode = "404", description = "Not found", content = @Content(schema = @Schema(implementation = ErrorMessage.class)))
167168
})
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package com.comet.opik.api.validation;
2+
3+
import jakarta.validation.Constraint;
4+
import jakarta.validation.Payload;
5+
6+
import java.lang.annotation.Documented;
7+
import java.lang.annotation.ElementType;
8+
import java.lang.annotation.Retention;
9+
import java.lang.annotation.RetentionPolicy;
10+
import java.lang.annotation.Target;
11+
12+
@Target({ElementType.TYPE, ElementType.PARAMETER, ElementType.ANNOTATION_TYPE})
13+
@Retention(RetentionPolicy.RUNTIME)
14+
@Constraint(validatedBy = {ProviderAuthCheckValidator.class})
15+
@Documented
16+
public @interface ProviderAuthCheckValidation {
17+
18+
String message() default "invalid auth check request";
19+
20+
Class<?>[] groups() default {};
21+
22+
Class<? extends Payload>[] payload() default {};
23+
24+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package com.comet.opik.api.validation;
2+
3+
import com.comet.opik.api.ProviderAuthCheck;
4+
import jakarta.validation.ConstraintValidator;
5+
import jakarta.validation.ConstraintValidatorContext;
6+
7+
/**
8+
* Request-only rules of the auth-config test endpoint. What stays in the service is the pair
9+
* that reads the DB: "the provider has no auth_config to test" and the secret-sentinel merge.
10+
*/
11+
public class ProviderAuthCheckValidator implements ConstraintValidator<ProviderAuthCheckValidation, ProviderAuthCheck> {
12+
13+
@Override
14+
public boolean isValid(ProviderAuthCheck request, ConstraintValidatorContext context) {
15+
context.disableDefaultConstraintViolation();
16+
17+
var authConfig = request.authConfig();
18+
if ((authConfig == null || authConfig.isEmpty()) && request.providerId() == null) {
19+
context.buildConstraintViolationWithTemplate("either provider_id or auth_config must be provided")
20+
.addConstraintViolation();
21+
return false;
22+
}
23+
24+
if (authConfig != null && !authConfig.isEmpty()) {
25+
var errors = ProviderAuthConfigValidator.validationErrors(authConfig);
26+
if (!errors.isEmpty()) {
27+
errors.forEach(error -> context.buildConstraintViolationWithTemplate(error)
28+
.addPropertyNode("authConfig")
29+
.addConstraintViolation());
30+
return false;
31+
}
32+
}
33+
return true;
34+
}
35+
}

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

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -200,36 +200,27 @@ public ProviderAuthCheck.Result testAuthConfig(@NonNull ProviderAuthCheck reques
200200
}
201201
}
202202

203+
/**
204+
* Request-only validation (provider_id-or-auth_config, recipe validity) lives in
205+
* {@link com.comet.opik.api.validation.ProviderAuthCheckValidator} at the API boundary; only
206+
* the rules that read the DB remain here.
207+
*/
203208
private ProviderAuthConfig resolveAuthConfigForTest(ProviderAuthCheck request, String workspaceId) {
204209
ProviderAuthConfig incoming = request.authConfig();
205210
if (incoming == null || incoming.isEmpty()) {
206-
if (request.providerId() == null) {
207-
throw new BadRequestException("either provider_id or auth_config must be provided");
208-
}
209211
ProviderAuthConfig stored = find(request.providerId(), workspaceId).authConfig();
210212
if (stored == null) {
211213
throw new BadRequestException("the provider has no auth_config to test");
212214
}
213215
return stored;
214216
}
215217

216-
var errors = ProviderAuthConfigValidator.validationErrors(incoming);
217-
if (!errors.isEmpty()) {
218-
throw new BadRequestException(String.join("; ", errors));
219-
}
220218
ProviderAuthConfig stored = request.providerId() != null
221219
? find(request.providerId(), workspaceId).authConfig()
222220
: null;
223221
return mergeSecretSentinels(incoming, stored);
224222
}
225223

226-
/**
227-
* The resolved auth_config decision plus the effective update to persist. The api_key /
228-
* auth_config mutual exclusion is enforced entirely inside
229-
* {@link #resolveAuthConfigUpdate}: it both validates the incoming pair and blanks the
230-
* stored key when the update switches to token auth, so callers persist
231-
* {@code effectiveUpdate} as-is and cannot hold the invariant wrong.
232-
*/
233224
private record AuthConfigUpdate(ProviderApiKeyUpdate effectiveUpdate, boolean clear,
234225
ProviderAuthConfig authConfig) {
235226
}

apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/customllm/AuthTokenProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ private HttpRequest buildRequest(ProviderAuthConfig authConfig) {
330330
String clientSecret = credentialValue(credentials, CLIENT_SECRET_KEY);
331331
if (clientId == null || clientSecret == null) {
332332
throw new AuthTokenException(
333-
"basic auth mode requires '%s' and '%s' credentials".formatted(CLIENT_ID_KEY,
333+
"requires '%s' and '%s' credentials".formatted(CLIENT_ID_KEY,
334334
CLIENT_SECRET_KEY));
335335
}
336336
var bodyCredentials = credentials.stream()

apps/opik-backend/src/test/java/com/comet/opik/api/resources/v1/priv/LlmProviderApiKeyResourceTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,9 +1034,8 @@ void testInvalidRequestsAreRejected() {
10341034

10351035
try (var response = llmProviderApiKeyResourceClient.callTestAuthConfig(
10361036
ProviderAuthCheck.builder().build(), apiKey, workspaceName)) {
1037-
assertThat(response.getStatus()).isEqualTo(HttpStatus.SC_BAD_REQUEST);
1038-
assertThat(response.readEntity(ErrorMessage.class).getMessage())
1039-
.contains("either provider_id or auth_config");
1037+
assertThat(response.getStatus()).isEqualTo(HttpStatus.SC_UNPROCESSABLE_CONTENT);
1038+
assertThat(response.readEntity(String.class)).contains("either provider_id or auth_config");
10401039
}
10411040

10421041
var staticProvider = llmProviderApiKeyResourceClient.createProviderApiKey(

0 commit comments

Comments
 (0)