Skip to content

Commit cfe1607

Browse files
sberyozkinsberyozkin
authored
Merge pull request #50624 from michalvavrik/feature/remember-anonymous-identity
Do not trigger authentication every time anonymous deferred identity is resolved
2 parents cb73382 + 27c22b4 commit cfe1607

File tree

2 files changed

+112
-95
lines changed

2 files changed

+112
-95
lines changed

‎extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/AbstractHttpAuthorizer.java‎

Lines changed: 81 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -59,55 +59,24 @@ abstract class AbstractHttpAuthorizer {
5959
* be invoked, if not appropriate action will be taken to either report the failure or attempt authentication.
6060
*/
6161
public void checkPermission(RoutingContext routingContext) {
62-
if (!controller.isAuthorizationEnabled()) {
62+
if (!controller.isAuthorizationEnabled() || policies.isEmpty()) {
6363
routingContext.next();
6464
return;
6565
}
66-
//check their permissions
67-
doPermissionCheck(routingContext, QuarkusHttpUser.getSecurityIdentity(routingContext, identityProviderManager), 0, null,
68-
policies);
69-
}
7066

71-
private void doPermissionCheck(RoutingContext routingContext,
72-
Uni<SecurityIdentity> identity, int index,
73-
SecurityIdentity augmentedIdentity,
74-
List<HttpSecurityPolicy> permissionCheckers) {
75-
if (index == permissionCheckers.size()) {
76-
QuarkusHttpUser currentUser = (QuarkusHttpUser) routingContext.user();
77-
if (augmentedIdentity != null) {
78-
if (!augmentedIdentity.isAnonymous()
79-
&& (currentUser == null || currentUser.getSecurityIdentity() != augmentedIdentity)) {
80-
setIdentity(augmentedIdentity, routingContext);
81-
}
82-
if (securityEventHelper.fireEventOnSuccess()) {
83-
securityEventHelper.fireSuccessEvent(new AuthorizationSuccessEvent(augmentedIdentity,
84-
Map.of(RoutingContext.class.getName(), routingContext)));
85-
}
86-
} else if (securityEventHelper.fireEventOnSuccess()
87-
&& permissionCheckPerformed(permissionCheckers, routingContext, index)) {
88-
securityEventHelper.fireSuccessEvent(
89-
new AuthorizationSuccessEvent(currentUser == null ? null : currentUser.getSecurityIdentity(),
90-
Map.of(RoutingContext.class.getName(), routingContext)));
91-
}
92-
routingContext.next();
93-
return;
94-
}
95-
//get the current checker
96-
HttpSecurityPolicy res = permissionCheckers.get(index);
97-
res.checkPermission(routingContext, identity, context)
67+
doPermissionCheck(routingContext, QuarkusHttpUser.getSecurityIdentity(routingContext, identityProviderManager), 0, null)
9868
.subscribe().with(new Consumer<HttpSecurityPolicy.CheckResult>() {
9969
@Override
10070
public void accept(HttpSecurityPolicy.CheckResult checkResult) {
101-
if (!checkResult.isPermitted()) {
102-
doDeny(identity, routingContext, res, checkResult.getAugmentedIdentity());
71+
if (routingContext.response().ended() || routingContext.failed()) {
72+
return;
73+
}
74+
if (checkResult.isPermitted()) {
75+
routingContext.next();
10376
} else {
104-
if (checkResult.getAugmentedIdentity() != null) {
105-
doPermissionCheck(routingContext, checkResult.getAugmentedIdentityAsUni(),
106-
index + 1, checkResult.getAugmentedIdentity(), permissionCheckers);
107-
} else {
108-
//attempt to run the next checker
109-
doPermissionCheck(routingContext, identity, index + 1, augmentedIdentity, permissionCheckers);
110-
}
77+
// if this is invoked, there is a bug in our code because all the denied checks must result
78+
// in the DoDenyException exception
79+
routingContext.fail(new IllegalStateException("Received unhandled HttpSecurityPolicy.CheckResult"));
11180
}
11281
}
11382
}, new Consumer<Throwable>() {
@@ -116,7 +85,13 @@ public void accept(Throwable throwable) {
11685
// we don't fail event if it's already failed with same exception as we don't want to process
11786
// the exception twice;at this point, the exception could be failed by the default auth failure handler
11887
if (!routingContext.response().ended() && !throwable.equals(routingContext.failure())) {
119-
routingContext.fail(throwable);
88+
if (throwable instanceof DoDenyException doDenyException) {
89+
// HTTP Security policy has forbidden access
90+
doDeny(doDenyException.augmentedIdentityUni, routingContext,
91+
doDenyException.policyWhichForbiddenAccess, doDenyException.augmentedIdentity);
92+
} else {
93+
routingContext.fail(throwable);
94+
}
12095
} else if (throwable instanceof AuthenticationFailedException) {
12196
log.debug("Authentication challenge is required");
12297
} else if (throwable instanceof AuthenticationRedirectException) {
@@ -125,7 +100,56 @@ public void accept(Throwable throwable) {
125100
} else {
126101
log.error("Exception occurred during authorization", throwable);
127102
}
103+
}
104+
});
105+
}
128106

107+
private Uni<HttpSecurityPolicy.CheckResult> doPermissionCheck(RoutingContext routingContext,
108+
Uni<SecurityIdentity> identityUni, int index, SecurityIdentity identity) {
109+
return policies.get(index)
110+
.checkPermission(routingContext, identityUni, context)
111+
.onItem().transformToUni(checkResult -> {
112+
final SecurityIdentity augmentedIdentity;
113+
final Uni<SecurityIdentity> augmentedIdentityUni;
114+
if (checkResult.getAugmentedIdentity() != null) {
115+
// HTTP Security policy provided a new SecurityIdentity
116+
augmentedIdentity = checkResult.getAugmentedIdentity();
117+
augmentedIdentityUni = checkResult.getAugmentedIdentityAsUni();
118+
} else {
119+
// this is identity either augmented by the previous HTTP Security policy, or from RoutingContext
120+
augmentedIdentity = identity;
121+
augmentedIdentityUni = identityUni;
122+
}
123+
124+
if (!checkResult.isPermitted()) {
125+
var failure = new DoDenyException(policies.get(index), augmentedIdentity, augmentedIdentityUni);
126+
return Uni.createFrom().failure(failure);
127+
}
128+
129+
if (index + 1 == policies.size()) {
130+
// all checks permitted access
131+
QuarkusHttpUser currentUser = (QuarkusHttpUser) routingContext.user();
132+
if (augmentedIdentity != null) {
133+
if (!augmentedIdentity.isAnonymous()
134+
&& (currentUser == null || currentUser.getSecurityIdentity() != augmentedIdentity)) {
135+
// perform security identity augmentation
136+
setIdentity(augmentedIdentity, routingContext);
137+
}
138+
if (securityEventHelper.fireEventOnSuccess()) {
139+
securityEventHelper.fireSuccessEvent(new AuthorizationSuccessEvent(augmentedIdentity,
140+
Map.of(RoutingContext.class.getName(), routingContext)));
141+
}
142+
} else if (securityEventHelper.fireEventOnSuccess()
143+
&& permissionCheckPerformed(policies, routingContext, index)) {
144+
securityEventHelper.fireSuccessEvent(
145+
new AuthorizationSuccessEvent(
146+
currentUser == null ? null : currentUser.getSecurityIdentity(),
147+
Map.of(RoutingContext.class.getName(), routingContext)));
148+
}
149+
return Uni.createFrom().item(checkResult);
150+
} else {
151+
// perform the next check
152+
return doPermissionCheck(routingContext, augmentedIdentityUni, index + 1, augmentedIdentity);
129153
}
130154
});
131155
}
@@ -206,9 +230,23 @@ private static boolean permissionCheckPerformed(List<HttpSecurityPolicy> permiss
206230
RoutingContext routingContext, int index) {
207231
// the path matching policy is not permission check itself, it selects policy based on
208232
// configured HTTP permissions, but if there are no path matching HTTP permissions, there is no check
209-
if (index == 1 && permissionCheckers.get(0) instanceof AbstractPathMatchingHttpSecurityPolicy) {
233+
if (index == 0 && permissionCheckers.get(0) instanceof AbstractPathMatchingHttpSecurityPolicy) {
210234
return AbstractPathMatchingHttpSecurityPolicy.policyApplied(routingContext);
211235
}
212236
return index > 0;
213237
}
238+
239+
private static final class DoDenyException extends RuntimeException {
240+
241+
private final HttpSecurityPolicy policyWhichForbiddenAccess;
242+
private final SecurityIdentity augmentedIdentity;
243+
private final Uni<SecurityIdentity> augmentedIdentityUni;
244+
245+
private DoDenyException(HttpSecurityPolicy policyWhichForbiddenAccess, SecurityIdentity augmentedIdentity,
246+
Uni<SecurityIdentity> augmentedIdentityUni) {
247+
this.policyWhichForbiddenAccess = policyWhichForbiddenAccess;
248+
this.augmentedIdentity = augmentedIdentity;
249+
this.augmentedIdentityUni = augmentedIdentityUni;
250+
}
251+
}
214252
}

‎extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/security/HttpSecurityRecorder.java‎

Lines changed: 31 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -376,63 +376,42 @@ protected void proceed(Throwable throwable) {
376376
}
377377

378378
if (proactiveAuthentication) {
379-
Uni<SecurityIdentity> potentialUser = authenticator.attemptAuthentication(event).memoize().indefinitely();
380-
potentialUser
381-
.subscribe().withSubscriber(new UniSubscriber<SecurityIdentity>() {
379+
authenticator
380+
.attemptAuthentication(event)
381+
.onItem().ifNull().switchTo(new Supplier<Uni<? extends SecurityIdentity>>() {
382382
@Override
383-
public void onSubscribe(UniSubscription subscription) {
384-
383+
public Uni<? extends SecurityIdentity> get() {
384+
return authenticator.getIdentityProviderManager()
385+
.authenticate(setRoutingContextAttribute(new AnonymousAuthenticationRequest(), event));
385386
}
386-
387+
})
388+
.invoke(new Consumer<SecurityIdentity>() {
387389
@Override
388-
public void onItem(SecurityIdentity identity) {
389-
if (event.response().ended()) {
390-
return;
391-
}
392-
if (identity == null) {
393-
Uni<SecurityIdentity> anon = authenticator.getIdentityProviderManager()
394-
.authenticate(
395-
setRoutingContextAttribute(new AnonymousAuthenticationRequest(), event));
396-
anon.subscribe().withSubscriber(new UniSubscriber<SecurityIdentity>() {
397-
@Override
398-
public void onSubscribe(UniSubscription subscription) {
399-
400-
}
401-
402-
@Override
403-
public void onItem(SecurityIdentity item) {
404-
event.put(QuarkusHttpUser.DEFERRED_IDENTITY_KEY, anon);
405-
event.setUser(new QuarkusHttpUser(item));
406-
event.next();
390+
public void accept(SecurityIdentity identity) {
391+
QuarkusHttpUser.setIdentity(identity, event);
392+
}
393+
})
394+
.subscribe().with(
395+
new Consumer<SecurityIdentity>() {
396+
@Override
397+
public void accept(SecurityIdentity ignored) {
398+
if (event.response().ended()) {
399+
return;
407400
}
408-
409-
@Override
410-
public void onFailure(Throwable failure) {
411-
BiConsumer<RoutingContext, Throwable> handler = event
412-
.get(QuarkusHttpUser.AUTH_FAILURE_HANDLER);
413-
if (handler != null) {
414-
handler.accept(event, failure);
415-
}
401+
event.next();
402+
}
403+
},
404+
new Consumer<Throwable>() {
405+
@Override
406+
public void accept(Throwable failure) {
407+
//this can be customised
408+
BiConsumer<RoutingContext, Throwable> handler = event
409+
.get(QuarkusHttpUser.AUTH_FAILURE_HANDLER);
410+
if (handler != null) {
411+
handler.accept(event, failure);
416412
}
417-
});
418-
} else {//when the result is evaluated we set the user, even if it is evaluated lazily
419-
event.setUser(new QuarkusHttpUser(identity));
420-
event.put(QuarkusHttpUser.DEFERRED_IDENTITY_KEY, potentialUser);
421-
event.next();
422-
}
423-
}
424-
425-
@Override
426-
public void onFailure(Throwable failure) {
427-
//this can be customised
428-
BiConsumer<RoutingContext, Throwable> handler = event
429-
.get(QuarkusHttpUser.AUTH_FAILURE_HANDLER);
430-
if (handler != null) {
431-
handler.accept(event, failure);
432-
}
433-
434-
}
435-
});
413+
}
414+
});
436415
} else {
437416

438417
Uni<SecurityIdentity> lazyUser = Uni

0 commit comments

Comments
 (0)