Skip to content

Commit d646b89

Browse files
committed
ref #357
1 parent 4436fa2 commit d646b89

File tree

4 files changed

+77
-44
lines changed

4 files changed

+77
-44
lines changed

grpc-spring-boot-starter-demo/src/test/java/org/lognet/springboot/grpc/auth/SecurityInterceptorTest.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(ServerCall<ReqT, Re
7676

7777
}
7878

79-
@SpyBean
80-
private GRpcErrorHandler errorHandler;
79+
8180

8281
@Test
8382
public void originalCustomInterceptorStatusIsPreserved() {
@@ -88,7 +87,7 @@ public void originalCustomInterceptorStatusIsPreserved() {
8887
.sayAuthHello2(Empty.newBuilder().build()).getMessage();
8988
});
9089
assertThat(statusRuntimeException.getStatus().getCode(), Matchers.is(Status.Code.ALREADY_EXISTS));
91-
verifyZeroInteractions(errorHandler);
90+
9291
}
9392
@Test
9493
public void unsupportedAuthSchemeShouldThrowUnauthenticatedException() {
@@ -104,7 +103,7 @@ public void unsupportedAuthSchemeShouldThrowUnauthenticatedException() {
104103
.sayAuthHello2(Empty.newBuilder().build()).getMessage();
105104
});
106105
assertThat(statusRuntimeException.getStatus().getCode(), Matchers.is(Status.Code.UNAUTHENTICATED));
107-
verify(errorHandler).handle(any(),eq(Status.UNAUTHENTICATED), any(),any(),any());
106+
108107
}
109108

110109

grpc-spring-boot-starter-demo/src/test/java/org/lognet/springboot/grpc/recovery/GRpcRecoveryTest.java

Lines changed: 59 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package org.lognet.springboot.grpc.recovery;
22

3-
import io.grpc.Metadata;
4-
import io.grpc.Status;
5-
import io.grpc.StatusRuntimeException;
3+
import io.grpc.*;
64
import io.grpc.examples.custom.Custom;
75
import io.grpc.examples.custom.CustomServiceGrpc;
86
import io.grpc.stub.StreamObserver;
@@ -11,14 +9,21 @@
119
import org.lognet.springboot.grpc.GRpcService;
1210
import org.lognet.springboot.grpc.GrpcServerTestBase;
1311
import org.lognet.springboot.grpc.demo.DemoApp;
12+
import org.lognet.springboot.grpc.security.*;
1413
import org.mockito.Mockito;
1514
import org.springframework.boot.test.context.SpringBootTest;
1615
import org.springframework.boot.test.context.TestConfiguration;
1716
import org.springframework.boot.test.mock.mockito.SpyBean;
1817
import org.springframework.context.annotation.Import;
18+
import org.springframework.security.authentication.dao.DaoAuthenticationProvider;
19+
import org.springframework.security.core.userdetails.User;
20+
import org.springframework.security.core.userdetails.UserDetailsService;
21+
import org.springframework.security.crypto.password.NoOpPasswordEncoder;
22+
import org.springframework.security.provisioning.InMemoryUserDetailsManager;
1923
import org.springframework.test.context.ActiveProfiles;
2024
import org.springframework.test.context.junit4.SpringRunner;
2125

26+
import java.util.Collections;
2227
import java.util.concurrent.CompletableFuture;
2328
import java.util.concurrent.ExecutionException;
2429
import java.util.concurrent.TimeUnit;
@@ -36,13 +41,13 @@
3641

3742
@RunWith(SpringRunner.class)
3843
@SpringBootTest(classes = {DemoApp.class}, webEnvironment = NONE)
39-
@ActiveProfiles({"disable-security"})
4044
@Import(GRpcRecoveryTest.Cfg.class)
4145
public class GRpcRecoveryTest extends GrpcServerTestBase {
4246

4347
static class CheckedException extends Exception {
4448

4549
}
50+
4651
static class CheckedException1 extends Exception {
4752

4853
}
@@ -59,9 +64,25 @@ static class Exception1 extends RuntimeException {
5964

6065
}
6166

62-
@TestConfiguration
63-
static class Cfg {
67+
private static User user1 = new User("test1", "test1", Collections.EMPTY_LIST);
6468

69+
private AuthHeader.AuthHeaderBuilder user1AuthHeaderBuilder =
70+
AuthHeader.builder().basic(user1.getUsername(), user1.getPassword().getBytes());
71+
72+
@TestConfiguration
73+
static class Cfg extends GrpcSecurityConfigurerAdapter {
74+
@Override
75+
public void configure(GrpcSecurity builder) throws Exception {
76+
DaoAuthenticationProvider provider = new DaoAuthenticationProvider();
77+
UserDetailsService users = new InMemoryUserDetailsManager(user1);
78+
provider.setUserDetailsService(users);
79+
provider.setPasswordEncoder(NoOpPasswordEncoder.getInstance());
80+
81+
builder
82+
.authenticationProvider(provider)
83+
.authorizeRequests()
84+
.anyMethod().authenticated();
85+
}
6586

6687
@GRpcServiceAdvice
6788
static class CustomErrorHandler {
@@ -102,6 +123,9 @@ public StreamObserver<Custom.CustomRequest> customStream(StreamObserver<Custom.C
102123
return new StreamObserver<Custom.CustomRequest>() {
103124
@Override
104125
public void onNext(Custom.CustomRequest value) {
126+
if ("onNext".equalsIgnoreCase(value.getName())) {
127+
throw new GRpcRuntimeExceptionWrapper(new CheckedException1());
128+
}
105129
responseObserver.onNext(Custom.CustomReply.newBuilder().build());
106130
}
107131

@@ -151,9 +175,24 @@ public Status handleB(ExceptionB e, GRpcExceptionScope scope) {
151175
private Cfg.CustomErrorHandler handler;
152176

153177

178+
protected Channel getChannel() {
179+
return ClientInterceptors.intercept(super.getChannel(), new AuthClientInterceptor(user1AuthHeaderBuilder));
180+
}
181+
182+
154183
@Test
155-
public void streamingServiceErrorHandlerTest() throws ExecutionException, InterruptedException, TimeoutException {
184+
public void parameterizedStreamingServiceErrorHandlerTest() throws ExecutionException, InterruptedException, TimeoutException {
185+
String[] phases = new String[]{
186+
"onNext", // exception will be thrown onNext
187+
"onCompleted" // exception will be thrown onCompleted
188+
};
189+
for (String errorPhase : phases) {
190+
streamingServiceErrorHandlerTest(errorPhase);
191+
Mockito.clearInvocations(srv);
192+
}
193+
}
156194

195+
public void streamingServiceErrorHandlerTest(String errorName) throws ExecutionException, InterruptedException, TimeoutException {
157196

158197

159198
final CompletableFuture<Throwable> errorFuture = new CompletableFuture<>();
@@ -175,20 +214,18 @@ public void onCompleted() {
175214
}
176215
};
177216

178-
final StreamObserver<Custom.CustomRequest> requests = CustomServiceGrpc.newStub(getChannel()).customStream(reply);
179-
requests.onNext(Custom.CustomRequest.newBuilder().build());
217+
final StreamObserver<Custom.CustomRequest> requests = CustomServiceGrpc.newStub(getChannel())
218+
.customStream(reply);
219+
requests.onNext(Custom.CustomRequest.newBuilder().setName(errorName).build());
180220
requests.onCompleted();
181221

182222

183-
184-
185-
186223
final Throwable actual = errorFuture.get(20, TimeUnit.SECONDS);
187224
assertThat(actual, notNullValue());
188225
assertThat(actual, isA(StatusRuntimeException.class));
189-
assertThat(((StatusRuntimeException)actual).getStatus(), is(Status.RESOURCE_EXHAUSTED));
226+
assertThat(((StatusRuntimeException) actual).getStatus(), is(Status.RESOURCE_EXHAUSTED));
190227

191-
Mockito.verify(srv,times(1)).handle(any(CheckedException1.class),any());
228+
Mockito.verify(srv, times(1)).handle(any(CheckedException1.class), any());
192229

193230
}
194231

@@ -199,7 +236,8 @@ public void checkedExceptionHandlerTest() {
199236
.custom(any(Custom.CustomRequest.class), any(StreamObserver.class));
200237

201238
final StatusRuntimeException statusRuntimeException = assertThrows(StatusRuntimeException.class, () ->
202-
CustomServiceGrpc.newBlockingStub(getChannel()).custom(Custom.CustomRequest.newBuilder().build())
239+
CustomServiceGrpc.newBlockingStub(getChannel())
240+
.custom(Custom.CustomRequest.newBuilder().build())
203241
);
204242
assertThat(statusRuntimeException.getStatus(), is(Status.OUT_OF_RANGE));
205243

@@ -221,7 +259,8 @@ public void globalHandlerTest() {
221259
.custom(any(Custom.CustomRequest.class), any(StreamObserver.class));
222260

223261
final StatusRuntimeException statusRuntimeException = assertThrows(StatusRuntimeException.class, () ->
224-
CustomServiceGrpc.newBlockingStub(getChannel()).custom(Custom.CustomRequest.newBuilder().build())
262+
CustomServiceGrpc.newBlockingStub(getChannel())
263+
.custom(Custom.CustomRequest.newBuilder().build())
225264
);
226265
assertThat(statusRuntimeException.getStatus(), is(Status.NOT_FOUND));
227266

@@ -242,7 +281,8 @@ public void globalHandlerWithExceptionHierarchyTest() {
242281
.custom(any(Custom.CustomRequest.class), any(StreamObserver.class));
243282

244283
final StatusRuntimeException statusRuntimeException = assertThrows(StatusRuntimeException.class, () ->
245-
CustomServiceGrpc.newBlockingStub(getChannel()).custom(Custom.CustomRequest.newBuilder().build())
284+
CustomServiceGrpc.newBlockingStub(getChannel())
285+
.custom(Custom.CustomRequest.newBuilder().build())
246286
);
247287
assertThat(statusRuntimeException.getStatus(), is(Status.DATA_LOSS));
248288

@@ -263,7 +303,8 @@ public void privateHandlerHasHigherPrecedence() {
263303
.custom(any(Custom.CustomRequest.class), any(StreamObserver.class));
264304

265305
final StatusRuntimeException statusRuntimeException = assertThrows(StatusRuntimeException.class, () ->
266-
CustomServiceGrpc.newBlockingStub(getChannel()).custom(Custom.CustomRequest.newBuilder().build())
306+
CustomServiceGrpc.newBlockingStub(getChannel())
307+
.custom(Custom.CustomRequest.newBuilder().build())
267308
);
268309
assertThat(statusRuntimeException.getStatus(), is(Status.FAILED_PRECONDITION));
269310

grpc-spring-boot-starter/src/main/java/org/lognet/springboot/grpc/security/AuthenticationSchemeService.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,15 @@ public Optional<Authentication> getAuthScheme(CharSequence authorization) {
2020
.map(selector -> selector.getAuthScheme(authorization))
2121
.filter(Optional::isPresent)
2222
.map(Optional::get)
23-
.collect(Collectors.toList());
23+
.toList();
2424
switch (auth.size()){
2525
case 0:
26-
throw new IllegalStateException(String.format("Authentication scheme '%s' is not supported.",
26+
log.error(String.format("Authentication scheme '%s' is not supported.",
2727
Optional.ofNullable(authorization)
2828
.map(s->s.toString().split(" ",2)[0])
2929
.orElse(null)
3030
));
31+
return Optional.empty();
3132
case 1 :
3233
return Optional.of(auth.get(0));
3334
default:

grpc-spring-boot-starter/src/main/java/org/lognet/springboot/grpc/security/SecurityInterceptor.java

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,14 @@
11
package org.lognet.springboot.grpc.security;
22

3-
import io.grpc.Context;
4-
import io.grpc.Contexts;
5-
import io.grpc.ForwardingServerCall;
6-
import io.grpc.ForwardingServerCallListener;
7-
import io.grpc.Metadata;
8-
import io.grpc.ServerCall;
9-
import io.grpc.ServerCallHandler;
10-
import io.grpc.ServerInterceptor;
3+
import io.grpc.*;
114
import lombok.Getter;
125
import lombok.Setter;
136
import lombok.extern.slf4j.Slf4j;
147
import org.lognet.springboot.grpc.FailureHandlingSupport;
158
import org.lognet.springboot.grpc.GRpcServicesRegistry;
169
import org.lognet.springboot.grpc.MessageBlockingServerCallListener;
1710
import org.lognet.springboot.grpc.autoconfigure.GRpcServerProperties;
11+
import org.lognet.springboot.grpc.recovery.GRpcRuntimeExceptionWrapper;
1812
import org.springframework.beans.factory.annotation.Autowired;
1913
import org.springframework.context.annotation.Lazy;
2014
import org.springframework.core.Ordered;
@@ -36,7 +30,7 @@
3630
public class SecurityInterceptor extends AbstractSecurityInterceptor implements ServerInterceptor, Ordered {
3731

3832
private static final Context.Key<InterceptorStatusToken> INTERCEPTOR_STATUS_TOKEN = Context.key("INTERCEPTOR_STATUS_TOKEN");
39-
private static final Context.Key<GrpcMethodInvocation<?,?>> METHOD_INVOCATION = Context.key("METHOD_INVOCATION");
33+
private static final Context.Key<GrpcMethodInvocation<?, ?>> METHOD_INVOCATION = Context.key("METHOD_INVOCATION");
4034

4135
private final SecurityMetadataSource securityMetadataSource;
4236

@@ -75,8 +69,6 @@ ServerCall<ReqT, RespT> getCall() {
7569
}
7670

7771

78-
79-
8072
public SecurityInterceptor(SecurityMetadataSource securityMetadataSource, AuthenticationSchemeSelector schemeSelector) {
8173
this.securityMetadataSource = securityMetadataSource;
8274
this.schemeSelector = schemeSelector;
@@ -142,11 +134,10 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
142134
final Context grpcSecurityContext;
143135
try {
144136
grpcSecurityContext = setupGRpcSecurityContext(call, headers, next, authorization);
145-
} catch (AccessDeniedException | AuthenticationException e) {
137+
} catch (RuntimeException e) {
146138
return fail(next, call, headers, e);
147139
} catch (Exception e) {
148-
return fail(next, call, headers, new AuthenticationException("Authentication failure.", e) {
149-
});
140+
return fail(next, call, headers, new GRpcRuntimeExceptionWrapper(e));
150141
}
151142
return Contexts.interceptCall(grpcSecurityContext, call, headers, authenticationPropagatingHandler(next));
152143
} finally {
@@ -176,15 +167,16 @@ public void onMessage(ReqT message) {
176167
METHOD_INVOCATION.get().setArguments(new Object[]{message});
177168
break;
178169
default:
179-
throw new AuthenticationException("Unsupported call type "+call.getMethodDescriptor().getType()) {};
170+
log.error("Unsupported call type " + call.getMethodDescriptor().getType());
171+
throw new StatusRuntimeException(Status.UNAUTHENTICATED) ;
180172
}
181173

182174
beforeInvocation(METHOD_INVOCATION.get());
183175
super.onMessage(message);
184-
} catch (AccessDeniedException | AuthenticationException e) {
185-
failureHandlingSupport.closeCall(e,call,headers);
176+
} catch (RuntimeException e) {
177+
failureHandlingSupport.closeCall(e, call, headers);
186178
} catch (Exception e) {
187-
failureHandlingSupport.closeCall( new AuthenticationException("", e) {},call, headers);
179+
failureHandlingSupport.closeCall(new GRpcRuntimeExceptionWrapper(e), call, headers);
188180
} finally {
189181
METHOD_INVOCATION.get().setArguments(null);
190182
}
@@ -243,15 +235,15 @@ private <RespT, ReqT> Context setupGRpcSecurityContext(ServerCall<RespT, ReqT> c
243235
ServerCallHandler<RespT, ReqT> next, CharSequence authorization) {
244236
final Authentication authentication = null == authorization ? null :
245237
schemeSelector.getAuthScheme(authorization)
246-
.orElseThrow(() -> new RuntimeException("Can't get authentication from authorization header"));
238+
.orElseThrow(() -> new StatusRuntimeException(Status.UNAUTHENTICATED));
247239

248240
SecurityContext context = SecurityContextHolder.createEmptyContext();
249241
context.setAuthentication(authentication);
250242
SecurityContextHolder.setContext(context);
251243

252244
final GRpcServicesRegistry.GrpcServiceMethod grpcServiceMethod = registry.getGrpServiceMethod(call.getMethodDescriptor());
253245

254-
final GrpcMethodInvocation<RespT, ReqT> methodInvocation = new GrpcMethodInvocation<>(grpcServiceMethod , call, headers, next);
246+
final GrpcMethodInvocation<RespT, ReqT> methodInvocation = new GrpcMethodInvocation<>(grpcServiceMethod, call, headers, next);
255247
final InterceptorStatusToken interceptorStatusToken = beforeInvocation(methodInvocation);
256248

257249
return Context.current()

0 commit comments

Comments
 (0)