Skip to content

Commit a4f1470

Browse files
authored
[3.x] Unauthenticated status code fix (helidon-io#6482)
Unauthenticated status code fix Signed-off-by: David Kral <david.k.kral@oracle.com>
1 parent f8fcaf3 commit a4f1470

5 files changed

Lines changed: 33 additions & 19 deletions

File tree

security/integration/grpc/src/main/java/io/helidon/security/integration/grpc/GrpcSecurityHandler.java

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2022 Oracle and/or its affiliates.
2+
* Copyright (c) 2019, 2023 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -368,11 +368,13 @@ private <ReqT, RespT> ServerCall.Listener<ReqT> processSecurity(SecurityContext
368368
.customObjects(customObjects.orElse(new ClassToInstanceStore<>()))
369369
.build());
370370

371-
CompletionStage<Boolean> stage = processAuthentication(call, headers, securityContext, tracing.atnTracing())
371+
CallWrapper<ReqT, RespT> callWrapper = new CallWrapper<>(call);
372+
373+
CompletionStage<Boolean> stage = processAuthentication(callWrapper, headers, securityContext, tracing.atnTracing())
372374
.thenCompose(atnResult -> {
373375
if (atnResult.proceed) {
374376
// authentication was OK or disabled, we should continue
375-
return processAuthorization(securityContext, tracing.atzTracing());
377+
return processAuthorization(securityContext, tracing.atzTracing(), callWrapper);
376378
} else {
377379
// authentication told us to stop processing
378380
return CompletableFuture.completedFuture(AtxResult.STOP);
@@ -392,15 +394,13 @@ private <ReqT, RespT> ServerCall.Listener<ReqT> processSecurity(SecurityContext
392394
});
393395

394396
ServerCall.Listener<ReqT> listener;
395-
CallWrapper<ReqT, RespT> callWrapper = new CallWrapper<>(call);
396397

397398
try {
398399
boolean proceed = stage.toCompletableFuture().get();
399400

400401
if (proceed) {
401402
listener = next.startCall(callWrapper, headers);
402403
} else {
403-
callWrapper.close(Status.PERMISSION_DENIED, new Metadata());
404404
listener = new EmptyListener<>();
405405
}
406406
} catch (Throwable throwable) {
@@ -463,18 +463,18 @@ private CompletionStage<AtxResult> processAuthentication(ServerCall<?, ?> call,
463463
//everything is fine, we can continue with processing
464464
break;
465465
case FAILURE_FINISH:
466-
if (atnFinishFailure(future)) {
466+
if (atnFinishFailure(future, call)) {
467467
atnSpanFinish(atnTracing, response);
468468
return;
469469
}
470470
break;
471471
case SUCCESS_FINISH:
472-
atnFinish(future);
472+
atnFinish(future, call);
473473
atnSpanFinish(atnTracing, response);
474474
return;
475475
case ABSTAIN:
476476
case FAILURE:
477-
if (atnAbstainFailure(future)) {
477+
if (atnAbstainFailure(future, call)) {
478478
atnSpanFinish(atnTracing, response);
479479
return;
480480
}
@@ -505,28 +505,31 @@ private void atnSpanFinish(AtnTracing atnTracing, AuthenticationResponse respons
505505
atnTracing.finish();
506506
}
507507

508-
private boolean atnAbstainFailure(CompletableFuture<AtxResult> future) {
508+
private boolean atnAbstainFailure(CompletableFuture<AtxResult> future, ServerCall<?, ?> call) {
509509
if (authenticationOptional.orElse(false)) {
510510
LOGGER.finest("Authentication failed, but was optional, so assuming anonymous");
511511
return false;
512512
}
513513

514+
call.close(Status.UNAUTHENTICATED, new Metadata());
514515
future.complete(AtxResult.STOP);
515516
return true;
516517
}
517518

518-
private boolean atnFinishFailure(CompletableFuture<AtxResult> future) {
519+
private boolean atnFinishFailure(CompletableFuture<AtxResult> future, ServerCall<?, ?> call) {
519520

520521
if (authenticationOptional.orElse(false)) {
521522
LOGGER.finest("Authentication failed, but was optional, so assuming anonymous");
522523
return false;
523524
} else {
525+
call.close(Status.UNAUTHENTICATED, new Metadata());
524526
future.complete(AtxResult.STOP);
525527
return true;
526528
}
527529
}
528530

529-
private void atnFinish(CompletableFuture<AtxResult> future) {
531+
private void atnFinish(CompletableFuture<AtxResult> future, ServerCall<?, ?> call) {
532+
call.close(Status.OK, new Metadata());
530533
future.complete(AtxResult.STOP);
531534
}
532535

@@ -539,7 +542,8 @@ private void configureSecurityRequest(SecurityRequestBuilder<? extends SecurityR
539542

540543
private CompletionStage<AtxResult> processAuthorization(
541544
SecurityContext context,
542-
AtzTracing atzTracing) {
545+
AtzTracing atzTracing,
546+
ServerCall<?, ?> call) {
543547
CompletableFuture<AtxResult> future = new CompletableFuture<>();
544548

545549
if (!authorize.orElse(false)) {
@@ -555,12 +559,14 @@ private CompletionStage<AtxResult> processAuthorization(
555559
// first validate roles - RBAC is supported out of the box by security, no need to invoke provider
556560
if (explicitAuthorizer.isPresent()) {
557561
if (rolesSet.stream().noneMatch(role -> context.isUserInRole(role, explicitAuthorizer.get()))) {
562+
call.close(Status.PERMISSION_DENIED, new Metadata());
558563
future.complete(AtxResult.STOP);
559564
atzTracing.finish();
560565
return future;
561566
}
562567
} else {
563568
if (rolesSet.stream().noneMatch(context::isUserInRole)) {
569+
call.close(Status.PERMISSION_DENIED, new Metadata());
564570
future.complete(AtxResult.STOP);
565571
atzTracing.finish();
566572
return future;
@@ -580,14 +586,20 @@ private CompletionStage<AtxResult> processAuthorization(
580586
case SUCCESS:
581587
//everything is fine, we can continue with processing
582588
break;
583-
case FAILURE_FINISH:
584589
case SUCCESS_FINISH:
585590
atzTracing.finish();
591+
call.close(Status.OK, new Metadata());
592+
future.complete(AtxResult.STOP);
593+
return;
594+
case FAILURE_FINISH:
595+
atzTracing.finish();
596+
call.close(Status.PERMISSION_DENIED, new Metadata());
586597
future.complete(AtxResult.STOP);
587598
return;
588599
case ABSTAIN:
589600
case FAILURE:
590601
atzTracing.finish();
602+
call.close(Status.PERMISSION_DENIED, new Metadata());
591603
future.complete(AtxResult.STOP);
592604
return;
593605
default:

security/integration/grpc/src/test/java/io/helidon/security/integration/grpc/OutboundSecurityIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2021 Oracle and/or its affiliates.
2+
* Copyright (c) 2019, 2023 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.

security/integration/grpc/src/test/java/io/helidon/security/integration/grpc/SecurityFromConfigIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2020 Oracle and/or its affiliates.
2+
* Copyright (c) 2019, 2023 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -119,7 +119,7 @@ public void shouldBeSecuredWithGlobalSettingsDenyAccess() {
119119
StatusRuntimeException thrown = assertThrows(StatusRuntimeException.class, () ->
120120
noCredsEchoStub.lower(toMessage("FOO")));
121121

122-
assertThat(thrown.getStatus().getCode(), is(Status.PERMISSION_DENIED.getCode()));
122+
assertThat(thrown.getStatus().getCode(), is(Status.UNAUTHENTICATED.getCode()));
123123
}
124124

125125
/**

security/integration/grpc/src/test/java/io/helidon/security/integration/grpc/ServiceAndMethodLevelSecurityIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2020 Oracle and/or its affiliates.
2+
* Copyright (c) 2019, 2023 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -126,7 +126,7 @@ public void shouldBeSecuredWithGlobalSettingsDenyAccess() {
126126
StatusRuntimeException thrown = assertThrows(StatusRuntimeException.class, () ->
127127
noCredsEchoStub.lower(toMessage("FOO")));
128128

129-
assertThat(thrown.getStatus().getCode(), is(Status.PERMISSION_DENIED.getCode()));
129+
assertThat(thrown.getStatus().getCode(), is(Status.UNAUTHENTICATED.getCode()));
130130
}
131131

132132
@Test

security/integration/grpc/src/test/resources/application.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#
2-
# Copyright (c) 2019, 2021 Oracle and/or its affiliates.
2+
# Copyright (c) 2019, 2023 Oracle and/or its affiliates.
33
#
44
# Licensed under the Apache License, Version 2.0 (the "License");
55
# you may not use this file except in compliance with the License.
@@ -32,3 +32,5 @@ http-basic-auth:
3232
- login: "Bob"
3333
password: "password"
3434
roles: ["user"]
35+
outbound:
36+
- name: "propagate-everything"

0 commit comments

Comments
 (0)