Skip to content

Commit

Permalink
#2868: cleanup, add tests and translate AuthenticationExceptions on t…
Browse files Browse the repository at this point in the history 
…he server side
  • Loading branch information
Mattia Brescia committed May 21, 2024
1 parent 10b3ea7 commit 46ee15e
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 22 deletions.
18 changes: 18 additions & 0 deletions factcast-client-grpc/src/test/java/org/factcast/client/grpc/ClientExceptionHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,24 @@ void extractsTransportedException() {
.isEqualTo("disappointed");
}

@Test
void extractsCredentialsNotFoundException() {
Metadata metadata = new Metadata();

StatusRuntimeException ex = new StatusRuntimeException(Status.UNAUTHENTICATED, metadata);
assertThat(ClientExceptionHelper.from(ex))
.isInstanceOf(StatusRuntimeException.class);
}

@Test
void extractsAuthenticationException() {
Metadata metadata = new Metadata();

StatusRuntimeException ex = new StatusRuntimeException(Status.PERMISSION_DENIED, metadata);
assertThat(ClientExceptionHelper.from(ex))
.isInstanceOf(StatusRuntimeException.class);
}

@Test
void wrapsRetryable() {
StatusRuntimeException ex = new StatusRuntimeException(Status.UNKNOWN);
Expand Down
105 changes: 104 additions & 1 deletion ...ecurity/src/test/java/org/factcast/itests/security/ClientWithSeparateCredentialsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,27 @@

import static org.assertj.core.api.Assertions.*;

import io.grpc.Channel;
import io.grpc.StatusRuntimeException;

import java.util.List;
import java.util.UUID;
import lombok.extern.slf4j.Slf4j;
import net.devh.boot.grpc.client.channelfactory.GrpcChannelFactory;
import org.factcast.client.grpc.FactCastGrpcStubsFactory;
import org.factcast.core.Fact;
import org.factcast.core.FactCast;
import org.factcast.core.spec.FactSpec;
import org.factcast.core.subscription.Subscription;
import org.factcast.core.subscription.SubscriptionRequest;
import org.factcast.core.subscription.observer.FactObserver;
import org.factcast.grpc.api.conv.ProtoConverter;
import org.factcast.test.AbstractFactCastIntegrationTest;
import org.factcast.test.FactcastTestConfig;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.TestPropertySource;

Expand All @@ -36,6 +48,37 @@
public class ClientWithSeparateCredentialsTest extends AbstractFactCastIntegrationTest {
@Autowired FactCast fc;

@Autowired
FactCastGrpcStubsFactory stubsFactory;

@Autowired
GrpcChannelFactory channelFactory;

@Autowired
JdbcTemplate jdbcTemplate;

private final ProtoConverter converter = new ProtoConverter();

private final FactObserver nopFactObserver = f -> {
// do nothing
};

@BeforeEach
void setup() {
List<Fact> facts = List.of(
Fact.of("{\"id\":\"" + UUID.randomUUID() + "\", \"ns\":\"users\",\"type\":\"UserCreated\"}", "{}"),
Fact.of("{\"id\":\"" + UUID.randomUUID() + "\", \"ns\":\"no-permissions\",\"type\":\"UserCreated\"}", "{}"));
String insertFact = "INSERT INTO fact(header,payload) VALUES (cast(? as jsonb),cast (? as jsonb))";
jdbcTemplate.batchUpdate(
insertFact,
facts,
Integer.MAX_VALUE,
(statement, fact) -> {
statement.setString(1, fact.jsonHeader());
statement.setString(2, fact.jsonPayload());
});
}

@Test
public void allowedToPublish() {
fc.publish(
Expand All @@ -45,7 +88,7 @@ public void allowedToPublish() {
}

@Test
public void failToPublish() {
public void failsToPublish() {
assertThatThrownBy(
() ->
fc.publish(
Expand All @@ -57,4 +100,64 @@ public void failToPublish() {
.isInstanceOf(StatusRuntimeException.class)
.hasMessageContaining("PERMISSION_DENIED");
}

@Test
public void allowedToCatchup() throws Exception {
SubscriptionRequest req = SubscriptionRequest.catchup(FactSpec.ns("users").type("UserCreated")).fromScratch();
try(Subscription sub = fc.subscribe(req, nopFactObserver)) {
sub.awaitCatchup();
}
}

@Test
public void failsToCatchup() throws Exception {
SubscriptionRequest req = SubscriptionRequest.catchup(FactSpec.ns("no-permissions").type("UserCreated")).fromScratch();
try(Subscription sub = fc.subscribe(req, nopFactObserver)) {
assertThatThrownBy(
sub::awaitCatchup)
.isInstanceOf(StatusRuntimeException.class)
.hasMessageContaining("PERMISSION_DENIED");
}
}

@Test
public void allowedToFollow() throws Exception {
SubscriptionRequest req = SubscriptionRequest.follow(FactSpec.ns("users").type("UserCreated")).fromScratch();
try(Subscription sub = fc.subscribe(req, nopFactObserver)) {
sub.awaitCatchup();
}
}

@Test
public void failsToFollow() throws Exception {
SubscriptionRequest req = SubscriptionRequest.follow(FactSpec.ns("no-permissions").type("UserCreated")).fromScratch();
try(Subscription sub = fc.subscribe(req, nopFactObserver)) {
assertThatThrownBy(
sub::awaitCatchup)
.isInstanceOf(StatusRuntimeException.class)
.hasMessageContaining("PERMISSION_DENIED");
}
}

@Test
public void allowedToEnumerate() {
assertThat(fc.enumerateNamespaces()).contains("users");
assertThat(fc.enumerateTypes("users")).contains("UserCreated");
}

@Test
public void failsToEnumerate() {
assertThat(fc.enumerateNamespaces()).doesNotContain("no-permissions");
assertThatThrownBy(() -> fc.enumerateTypes("no-permissions"))
.isInstanceOf(StatusRuntimeException.class)
.hasMessageContaining("PERMISSION_DENIED");
}

@Test
public void failsUnauthenticatedHandshake() {
Channel channel = channelFactory.createChannel("factstore");
assertThatThrownBy(() -> stubsFactory.createBlockingStub(channel).handshake(converter.empty()))
.isInstanceOf(StatusRuntimeException.class)
.hasMessageContaining("UNAUTHENTICATED");
}
}
12 changes: 1 addition & 11 deletions ...st-server-grpc/src/main/java/org/factcast/server/grpc/GrpcServerExceptionInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,8 @@ void handleException(
return;
}

// in case someone knows exactly what status to throw
if (exception instanceof StatusRuntimeException) {
var e = (StatusRuntimeException) exception;
if (!Status.PERMISSION_DENIED.equals(e.getStatus())) {
log.error("", e);
}
serverCall.close(e.getStatus(), metadata);
return;
}

logIfNecessary(log, exception);
StatusRuntimeException sre = ServerExceptionHelper.translate(exception);
StatusRuntimeException sre = ServerExceptionHelper.translate(exception, metadata);
serverCall.close(sre.getStatus(), sre.getTrailers());
}

Expand Down
16 changes: 14 additions & 2 deletions factcast-server-grpc/src/main/java/org/factcast/server/grpc/ServerExceptionHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@
import io.grpc.Metadata;
import io.grpc.Status;
import io.grpc.StatusRuntimeException;
import lombok.experimental.UtilityClass;
import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
import org.springframework.security.core.AuthenticationException;

@UtilityClass
public class ServerExceptionHelper {

public static StatusRuntimeException translate(Throwable e) {
Metadata meta = new Metadata();
public static StatusRuntimeException translate(Throwable e, Metadata meta) {
if (e instanceof StatusRuntimeException) // prevent double wrap
{
return (StatusRuntimeException) e;
Expand All @@ -33,11 +36,20 @@ public static StatusRuntimeException translate(Throwable e) {
} else if (e instanceof UnsupportedOperationException) {
// UNIMPLEMENTED is technically not fully correct but best we can do here
return new StatusRuntimeException(Status.UNIMPLEMENTED, meta);
} else if (e instanceof AuthenticationException) {
if (e instanceof AuthenticationCredentialsNotFoundException) {
return new StatusRuntimeException(Status.UNAUTHENTICATED, meta);
}
return new StatusRuntimeException(Status.PERMISSION_DENIED, meta);
} else {
return new StatusRuntimeException(Status.UNKNOWN, meta);
}
}

public static StatusRuntimeException translate(Throwable e) {
return translate(e, new Metadata());
}

private static Metadata addMetaData(Metadata metadata, Throwable e) {
metadata.put(
Metadata.Key.of("msg-bin", Metadata.BINARY_BYTE_MARSHALLER), e.getMessage().getBytes());
Expand Down
36 changes: 28 additions & 8 deletions ...erver-grpc/src/test/java/org/factcast/server/grpc/GrpcServerExceptionInterceptorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
import org.springframework.security.authentication.BadCredentialsException;

@ExtendWith(MockitoExtension.class)
class GrpcServerExceptionInterceptorTest {
Expand Down Expand Up @@ -182,39 +184,34 @@ class whenHandlingException {

@Test
void handlesCancelByClient() {

ExceptionHandlingServerCallListener<Req, Res> uut = spy(underTest);

String msg = "123";
var ex = new RequestCanceledByClientException(msg);

uut.handleException(ex, serverCall, metadata);

var cap = ArgumentCaptor.forClass(Status.class);
verify(serverCall).close(cap.capture(), same(metadata));

assertThat(cap.getValue().getCode()).isEqualTo(Code.CANCELLED);
assertThat(cap.getValue().getDescription()).isEqualTo(msg);
}

@Test
void closesOnStatusRuntimeException() {

ExceptionHandlingServerCallListener<Req, Res> uut = spy(underTest);
String msg = "456";
var ex = new StatusRuntimeException(Status.ALREADY_EXISTS.withDescription(msg));
var metadata = new Metadata();
var ex = new StatusRuntimeException(Status.ALREADY_EXISTS.withDescription(msg), metadata);
uut.handleException(ex, serverCall, metadata);

var cap = ArgumentCaptor.forClass(Status.class);
verify(serverCall).close(cap.capture(), same(metadata));

assertThat(cap.getValue().getCode()).isEqualTo(Code.ALREADY_EXISTS);
assertThat(cap.getValue().getDescription()).isEqualTo(msg);
}

@Test
void closesWithTranslatedException() {

ExceptionHandlingServerCallListener<Req, Res> uut = spy(underTest);
String msg = "456";
var ex = new FactValidationException(msg);
Expand All @@ -223,7 +220,6 @@ void closesWithTranslatedException() {

var cap = ArgumentCaptor.forClass(Metadata.class);
verify(serverCall).close(any(), cap.capture());

assertThat(
cap.getValue()
.containsKey(Metadata.Key.of("msg-bin", Metadata.BINARY_BYTE_MARSHALLER)))
Expand All @@ -233,6 +229,30 @@ void closesWithTranslatedException() {
.containsKey(Metadata.Key.of("exc-bin", Metadata.BINARY_BYTE_MARSHALLER)))
.isTrue();
}

@Test
void translatesCredentialsNotFoundException() {
ExceptionHandlingServerCallListener<Req, Res> uut = spy(underTest);
var ex = new AuthenticationCredentialsNotFoundException("test");

uut.handleException(ex, serverCall, metadata);

var cap = ArgumentCaptor.forClass(Status.class);
verify(serverCall).close(cap.capture(), any());
assertThat(cap.getValue().getCode()).isEqualTo(Code.UNAUTHENTICATED);
}

@Test
void translatesAuthenticationException() {
ExceptionHandlingServerCallListener<Req, Res> uut = spy(underTest);
var ex = new BadCredentialsException("test");

uut.handleException(ex, serverCall, metadata);

var cap = ArgumentCaptor.forClass(Status.class);
verify(serverCall).close(cap.capture(), any());
assertThat(cap.getValue().getCode()).isEqualTo(Code.PERMISSION_DENIED);
}
}
}
}

0 comments on commit 46ee15e

Please sign in to comment.