Skip to content

Commit

Permalink
Change format of error message to match previous behavoir
Browse files Browse the repository at this point in the history
Signed-off-by: Peter Nied <[email protected]>
  • Loading branch information
peternied committed Oct 6, 2023
1 parent ddbe342 commit 9cc4f73
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ public void shouldImpersonateUser_negativeJean() {

response.assertStatusCode(403);
String expectedMessage = String.format("'%s' is not allowed to impersonate as '%s'", USER_KIRK, USER_JEAN);
System.out.println("&&&& " + response.getBody());
assertThat(response.getTextFromJsonBody(POINTER_ERROR_REASON), equalTo(expectedMessage));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ private JsonNode getJsonNodeAt(String jsonPointer) {
try {
return toJsonNode().at(jsonPointer);
} catch (IOException e) {
throw new IllegalArgumentException("Cound not convert response body to JSON node ", e);
throw new IllegalArgumentException("Cound not convert response body to JSON node '" + getBody() + "'", e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ private Optional<SecurityResponse> handleLowLevel(RestRequest restRequest) throw
return Optional.of(new SecurityResponse(HttpStatus.SC_OK, SecurityResponse.CONTENT_TYPE_APP_JSON, responseBodyString));
} catch (JsonProcessingException e) {
log.warn("Error while parsing JSON for /_opendistro/_security/api/authtoken", e);
return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, null, "JSON could not be parsed"));
return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, new Exception("JSON could not be parsed")));

Check warning on line 242 in src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java#L242

Added line #L242 was not covered by tests
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public boolean authenticate(final SecurityRequestChannel request) {
log.debug("Rejecting REST request because of blocked address: {}", request.getRemoteAddress().orElse(null));
}

request.queueForSending(new SecurityResponse(SC_UNAUTHORIZED, null, "Authentication finally failed"));
request.queueForSending(new SecurityResponse(SC_UNAUTHORIZED, new Exception("Authentication finally failed")));

Check warning on line 206 in src/main/java/org/opensearch/security/auth/BackendRegistry.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/auth/BackendRegistry.java#L206

Added line #L206 was not covered by tests
return false;
}

Expand All @@ -225,7 +225,7 @@ public boolean authenticate(final SecurityRequestChannel request) {

if (!isInitialized()) {
log.error("Not yet initialized (you may need to run securityadmin)");
request.queueForSending(new SecurityResponse(SC_SERVICE_UNAVAILABLE, null, "OpenSearch Security not initialized."));
request.queueForSending(new SecurityResponse(SC_SERVICE_UNAVAILABLE, new Exception("OpenSearch Security not initialized.")));
return false;
}

Expand Down
22 changes: 22 additions & 0 deletions src/main/java/org/opensearch/security/filter/SecurityResponse.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@

package org.opensearch.security.filter;

import java.io.IOException;
import java.util.Map;

import org.apache.http.HttpHeaders;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestResponse;
Expand All @@ -26,6 +28,12 @@ public class SecurityResponse {
private final Map<String, String> headers;
private final String body;

public SecurityResponse(final int status, final Exception e) {
this.status = status;
this.headers = CONTENT_TYPE_APP_JSON;
this.body = generateFailureMessage(e);
}

public SecurityResponse(final int status, final Map<String, String> headers, final String body) {
this.status = status;
this.headers = headers;
Expand All @@ -52,4 +60,18 @@ public RestResponse asRestResponse() {
return restResponse;
}

protected String generateFailureMessage(final Exception e) {
try {
return XContentFactory.jsonBuilder()
.startObject()
.startObject("error")
.field("status", "error")
.field("reason", e.getMessage())
.endObject()
.endObject()
.toString();
} catch (final IOException ioe) {
throw new RuntimeException(ioe);

Check warning on line 74 in src/main/java/org/opensearch/security/filter/SecurityResponse.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/filter/SecurityResponse.java#L73-L74

Added lines #L73 - L74 were not covered by tests
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t
log.error(exception.toString());
auditLog.logBadHeaders(requestChannel);

requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString()));
requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, exception));
return;
}

Expand All @@ -256,7 +256,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t
log.error(exception.toString());
auditLog.logBadHeaders(requestChannel);

requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString()));
requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, exception));

Check warning on line 259 in src/main/java/org/opensearch/security/filter/SecurityRestFilter.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/filter/SecurityRestFilter.java#L259

Added line #L259 was not covered by tests
return;
}

Expand All @@ -276,7 +276,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t
} catch (SSLPeerUnverifiedException e) {
log.error("No ssl info", e);
auditLog.logSSLException(requestChannel, e);
requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, null));
requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, new Exception("No ssl info")));

Check warning on line 279 in src/main/java/org/opensearch/security/filter/SecurityRestFilter.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/filter/SecurityRestFilter.java#L279

Added line #L279 was not covered by tests
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public void channelRead0(ChannelHandlerContext ctx, DefaultHttpRequest msg) thro
ctx.channel().attr(IS_AUTHENTICATED).set(Boolean.TRUE);
}
} catch (final OpenSearchSecurityException e) {
final SecurityResponse earlyResponse = new SecurityResponse(ExceptionsHelper.status(e).getStatus(), null, e.getMessage());
final SecurityResponse earlyResponse = new SecurityResponse(ExceptionsHelper.status(e).getStatus(), e);
ctx.channel().attr(EARLY_RESPONSE).set(earlyResponse);
} catch (final SecurityRequestChannelUnsupported srcu) {

Check warning on line 124 in src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java#L124

Added line #L124 was not covered by tests
// Use defaults for unsupported channels
Expand Down

0 comments on commit 9cc4f73

Please sign in to comment.