Skip to content

Commit

Permalink
WebSockets Next: secure HTTP upgrade with annotation
Browse files Browse the repository at this point in the history
  • Loading branch information
@michalvavrik
michalvavrik committed Jun 4, 2024
1 parent 082397b commit 3818f2a
Show file tree
Hide file tree
Showing 20 changed files with 1,331 additions and 105 deletions.
54 changes: 47 additions & 7 deletions docs/src/main/asciidoc/websockets-next-reference.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -655,18 +655,58 @@ public class Endpoint {

`SecurityIdentity` is initially created during a secure HTTP upgrade and associated with the websocket connection.

Currently, for an HTTP upgrade be secured, users must configure an HTTP policy protecting the HTTP upgrade path.
For example, to secure the `open()` method in the above websocket endpoint, one can add the following authentication policy:
NOTE: When OpenID Connect extension is used and token expires, Quarkus automatically closes connection.

[source,properties]
== Secure HTTP upgrade

An HTTP upgrade is secured when standard security annotation is placed on an endpoint class or an HTTP Security policy is defined.
The advantage of securing HTTP upgrade is less processing, the authorization is performed early and only once.
You should always prefer HTTP upgrade security unless, like in th example above, you need to perform action on error.

Check warning on line 664 in docs/src/main/asciidoc/websockets-next-reference.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale 

[vale] reported by reviewdog 🐶
[Quarkus.Fluff] Depending on the context, consider using 'Rewrite the sentence, or use 'must', instead of' rather than 'need to'.

Raw Output:
{"message": "[Quarkus.Fluff] Depending on the context, consider using 'Rewrite the sentence, or use 'must', instead of' rather than 'need to'.", "location": {"path": "docs/src/main/asciidoc/websockets-next-reference.adoc", "range": {"start": {"line": 664, "column": 75}}}, "severity": "INFO"}

.Use standard security annotation to secure an HTTP upgrade
[source, java]
----
quarkus.http.auth.permission.secured.paths=/end
quarkus.http.auth.permission.secured.policy=authenticated
package io.quarkus.websockets.next.test.security;
import io.quarkus.security.Authenticated;
import jakarta.inject.Inject;
import io.quarkus.security.identity.SecurityIdentity;
import io.quarkus.websockets.next.OnOpen;
import io.quarkus.websockets.next.OnTextMessage;
import io.quarkus.websockets.next.WebSocket;
@Authenticated <1>
@WebSocket(path = "/end")
public class Endpoint {
@Inject
SecurityIdentity currentIdentity;
@OnOpen
String open() {
return "ready";
}
@OnTextMessage
String echo(String message) {
return message;
}
}
----
<1> Initial HTTP handshake ends with the 401 status for anonymous users.
You can also redirect the handshake request on authorization failure with the `quarkus.websockets-next.server.security.auth-failure-redirect-url` configuration property.

Other options for securing HTTP upgrade requests, such as using the security annotations, will be explored in the future.
IMPORTANT: HTTP upgrade is only secured when a security annotation is declared on an endpoint class next to the `@WebSocket` annotation.
Placing a security annotation on an endpoint bean will not secure bean methods, only the HTTP upgrade.
You must always verify that your endpoint is secured as intended.

NOTE: When OpenID Connect extension is used and token expires, Quarkus automatically closes connection.
.Use HTTP Security policy to secure an HTTP upgrade
[source,properties]
----
quarkus.http.auth.permission.http-upgrade.paths=/end
quarkus.http.auth.permission.http-upgrade.policy=authenticated
----

== Inspect and/or reject HTTP upgrade

Check warning on line 711 in docs/src/main/asciidoc/websockets-next-reference.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale 

[vale] reported by reviewdog 🐶
[Quarkus.TermsWarnings] Consider using 'a or b' or 'a, b, or both' rather than 'and/or' unless updating existing content that uses the term.

Raw Output:
{"message": "[Quarkus.TermsWarnings] Consider using 'a or b' or 'a, b, or both' rather than 'and/or' unless updating existing content that uses the term.", "location": {"path": "docs/src/main/asciidoc/websockets-next-reference.adoc", "range": {"start": {"line": 711, "column": 1}}}, "severity": "WARNING"}

Expand Down
114 changes: 72 additions & 42 deletions ...ity/deployment/src/main/java/io/quarkus/security/deployment/PermissionSecurityChecks.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.security.deployment;

import static io.quarkus.arc.processor.DotNames.CLASS;
import static io.quarkus.arc.processor.DotNames.STRING;
import static io.quarkus.security.PermissionsAllowed.AUTODETECTED;
import static io.quarkus.security.PermissionsAllowed.PERMISSION_TO_ACTION_SEPARATOR;
Expand Down Expand Up @@ -34,7 +35,9 @@

interface PermissionSecurityChecks {

Map<MethodInfo, SecurityCheck> get();
Map<MethodInfo, SecurityCheck> getMethodSecurityChecks();

Map<DotName, SecurityCheck> getClassNameSecurityChecks();

Set<String> permissionClasses();

Expand All @@ -43,8 +46,8 @@ final class PermissionSecurityChecksBuilder {
private static final DotName STRING_PERMISSION = DotName.createSimple(StringPermission.class);
private static final DotName PERMISSIONS_ALLOWED_INTERCEPTOR = DotName
.createSimple(PermissionsAllowedInterceptor.class);
private final Map<MethodInfo, List<List<PermissionKey>>> methodToPermissionKeys = new HashMap<>();
private final Map<MethodInfo, LogicalAndPermissionPredicate> methodToPredicate = new HashMap<>();
private final Map<AnnotationTarget, List<List<PermissionKey>>> targetToPermissionKeys = new HashMap<>();
private final Map<AnnotationTarget, LogicalAndPermissionPredicate> targetToPredicate = new HashMap<>();
private final Map<String, MethodInfo> classSignatureToConstructor = new HashMap<>();
private final SecurityCheckRecorder recorder;

Expand All @@ -53,22 +56,37 @@ public PermissionSecurityChecksBuilder(SecurityCheckRecorder recorder) {
}

PermissionSecurityChecks build() {
final Map<LogicalAndPermissionPredicate, SecurityCheck> cache = new HashMap<>();
final Map<MethodInfo, SecurityCheck> methodToCheck = new HashMap<>();
final Map<DotName, SecurityCheck> classNameToCheck = new HashMap<>();
for (var targetToPredicate : targetToPredicate.entrySet()) {
SecurityCheck check = cache.computeIfAbsent(targetToPredicate.getValue(),
new Function<LogicalAndPermissionPredicate, SecurityCheck>() {
@Override
public SecurityCheck apply(LogicalAndPermissionPredicate predicate) {
return createSecurityCheck(predicate);
}
});

var annotationTarget = targetToPredicate.getKey();
if (annotationTarget.kind() == AnnotationTarget.Kind.CLASS) {
DotName className = annotationTarget.asClass().name();
classNameToCheck.put(className, check);
} else {
MethodInfo securedMethod = annotationTarget.asMethod();
methodToCheck.put(securedMethod, check);
}
}

return new PermissionSecurityChecks() {
@Override
public Map<MethodInfo, SecurityCheck> get() {
final Map<LogicalAndPermissionPredicate, SecurityCheck> cache = new HashMap<>();
final Map<MethodInfo, SecurityCheck> methodToCheck = new HashMap<>();
for (var methodToPredicate : methodToPredicate.entrySet()) {
SecurityCheck check = cache.computeIfAbsent(methodToPredicate.getValue(),
new Function<LogicalAndPermissionPredicate, SecurityCheck>() {
@Override
public SecurityCheck apply(LogicalAndPermissionPredicate predicate) {
return createSecurityCheck(predicate);
}
});
methodToCheck.put(methodToPredicate.getKey(), check);
}
return methodToCheck;
public Map<MethodInfo, SecurityCheck> getMethodSecurityChecks() {
return Map.copyOf(methodToCheck);
}

@Override
public Map<DotName, SecurityCheck> getClassNameSecurityChecks() {
return Map.copyOf(classNameToCheck);
}

@Override
Expand Down Expand Up @@ -99,8 +117,8 @@ public Set<String> permissionClasses() {
*/
PermissionSecurityChecksBuilder createPermissionPredicates() {
Map<PermissionCacheKey, PermissionWrapper> permissionCache = new HashMap<>();
for (Map.Entry<MethodInfo, List<List<PermissionKey>>> entry : methodToPermissionKeys.entrySet()) {
final MethodInfo securedMethod = entry.getKey();
for (var entry : targetToPermissionKeys.entrySet()) {
final AnnotationTarget securedTarget = entry.getKey();
final LogicalAndPermissionPredicate predicate = new LogicalAndPermissionPredicate();

// 'AND' operands
Expand All @@ -113,7 +131,7 @@ PermissionSecurityChecksBuilder createPermissionPredicates() {
// 'AND' operands

for (PermissionKey permissionKey : permissionKeys) {
var permission = createPermission(permissionKey, securedMethod, permissionCache);
var permission = createPermission(permissionKey, securedTarget, permissionCache);
if (permission.isComputed()) {
predicate.markAsComputed();
}
Expand All @@ -128,15 +146,15 @@ PermissionSecurityChecksBuilder createPermissionPredicates() {
predicate.and(orPredicate);

for (PermissionKey permissionKey : permissionKeys) {
var permission = createPermission(permissionKey, securedMethod, permissionCache);
var permission = createPermission(permissionKey, securedTarget, permissionCache);
if (permission.isComputed()) {
predicate.markAsComputed();
}
orPredicate.or(permission);
}
}
}
methodToPredicate.put(securedMethod, predicate);
targetToPredicate.put(securedTarget, predicate);
}
return this;
}
Expand All @@ -153,7 +171,7 @@ private boolean isInclusive(List<PermissionKey> permissionKeys) {
}

PermissionSecurityChecksBuilder validatePermissionClasses(IndexView index) {
for (List<List<PermissionKey>> keyLists : methodToPermissionKeys.values()) {
for (List<List<PermissionKey>> keyLists : targetToPermissionKeys.values()) {
for (List<PermissionKey> keyList : keyLists) {
for (PermissionKey key : keyList) {
if (!classSignatureToConstructor.containsKey(key.classSignature())) {
Expand Down Expand Up @@ -187,7 +205,8 @@ PermissionSecurityChecksBuilder validatePermissionClasses(IndexView index) {

PermissionSecurityChecksBuilder gatherPermissionsAllowedAnnotations(List<AnnotationInstance> instances,
Map<MethodInfo, AnnotationInstance> alreadyCheckedMethods,
Map<ClassInfo, AnnotationInstance> alreadyCheckedClasses) {
Map<ClassInfo, AnnotationInstance> alreadyCheckedClasses,
List<AnnotationInstance> additionalClassInstances) {

// make sure we process annotations on methods first
instances.sort(new Comparator<AnnotationInstance>() {
Expand Down Expand Up @@ -217,7 +236,7 @@ public int compare(AnnotationInstance o1, AnnotationInstance o2) {
methodInfo.name(), methodInfo.declaringClass()));
}

gatherPermissionKeys(instance, methodInfo, cache, methodToPermissionKeys);
gatherPermissionKeys(instance, methodInfo, cache, targetToPermissionKeys);
} else {
// class annotation

Expand Down Expand Up @@ -245,7 +264,7 @@ public int compare(AnnotationInstance o1, AnnotationInstance o2) {
// ignore method annotated with other security annotation
boolean noMethodLevelSecurityAnnotation = !alreadyCheckedMethods.containsKey(methodInfo);
// ignore method annotated with method-level @PermissionsAllowed
boolean noMethodLevelPermissionsAllowed = !methodToPermissionKeys.containsKey(methodInfo);
boolean noMethodLevelPermissionsAllowed = !targetToPermissionKeys.containsKey(methodInfo);
if (noMethodLevelSecurityAnnotation && noMethodLevelPermissionsAllowed) {

gatherPermissionKeys(instance, methodInfo, cache, classMethodToPermissionKeys);
Expand All @@ -261,12 +280,16 @@ public int compare(AnnotationInstance o1, AnnotationInstance o2) {
}
}
}
methodToPermissionKeys.putAll(classMethodToPermissionKeys);
targetToPermissionKeys.putAll(classMethodToPermissionKeys);
for (var instance : additionalClassInstances) {
gatherPermissionKeys(instance, instance.target(), cache, targetToPermissionKeys);
}
return this;
}

private static void gatherPermissionKeys(AnnotationInstance instance, MethodInfo methodInfo, List<PermissionKey> cache,
Map<MethodInfo, List<List<PermissionKey>>> methodToPermissionKeys) {
private static <T extends AnnotationTarget> void gatherPermissionKeys(AnnotationInstance instance, T annotationTarget,
List<PermissionKey> cache,
Map<T, List<List<PermissionKey>>> targetToPermissionKeys) {
// @PermissionsAllowed value is in format permission:action, permission2:action, permission:action2, permission3
// here we transform it to permission -> actions
final var permissionToActions = new HashMap<String, Set<String>>();
Expand Down Expand Up @@ -299,9 +322,15 @@ private static void gatherPermissionKeys(AnnotationInstance instance, MethodInfo
}

if (permissionToActions.isEmpty()) {
throw new RuntimeException(String.format(
"Method '%s' was annotated with '@PermissionsAllowed', but no valid permission was provided",
methodInfo.name()));
if (annotationTarget.kind() == AnnotationTarget.Kind.METHOD) {
throw new RuntimeException(String.format(
"Method '%s' was annotated with '@PermissionsAllowed', but no valid permission was provided",
annotationTarget.asMethod().name()));
} else {
throw new RuntimeException(String.format(
"Class '%s' was annotated with '@PermissionsAllowed', but no valid permission was provided",
annotationTarget.asClass().name()));
}
}

// permissions specified via @PermissionsAllowed has 'one of' relation, therefore we put them in one list
Expand All @@ -324,13 +353,8 @@ private static void gatherPermissionKeys(AnnotationInstance instance, MethodInfo
}

// store annotation value as permission keys
methodToPermissionKeys
.computeIfAbsent(methodInfo, new Function<MethodInfo, List<List<PermissionKey>>>() {
@Override
public List<List<PermissionKey>> apply(MethodInfo methodInfo) {
return new ArrayList<>();
}
})
targetToPermissionKeys
.computeIfAbsent(annotationTarget, at -> new ArrayList<>())
.add(List.copyOf(orPermissions));
}

Expand Down Expand Up @@ -378,10 +402,10 @@ private SecurityCheck createSecurityCheck(LogicalAndPermissionPredicate andPredi
return securityCheck;
}

private PermissionWrapper createPermission(PermissionKey permissionKey, MethodInfo securedMethod,
private PermissionWrapper createPermission(PermissionKey permissionKey, AnnotationTarget securedTarget,
Map<PermissionCacheKey, PermissionWrapper> cache) {
var constructor = classSignatureToConstructor.get(permissionKey.classSignature());
return cache.computeIfAbsent(new PermissionCacheKey(permissionKey, securedMethod, constructor),
return cache.computeIfAbsent(new PermissionCacheKey(permissionKey, securedTarget, constructor),
new Function<PermissionCacheKey, PermissionWrapper>() {
@Override
public PermissionWrapper apply(PermissionCacheKey permissionCacheKey) {
Expand Down Expand Up @@ -568,8 +592,14 @@ private static final class PermissionCacheKey {
private final boolean computed;
private final boolean passActionsToConstructor;

private PermissionCacheKey(PermissionKey permissionKey, MethodInfo securedMethod, MethodInfo constructor) {
private PermissionCacheKey(PermissionKey permissionKey, AnnotationTarget securedTarget, MethodInfo constructor) {
if (isComputed(permissionKey, constructor)) {
if (securedTarget.kind() != AnnotationTarget.Kind.METHOD) {
throw new IllegalArgumentException(
"@PermissionAllowed instance that accepts method arguments must be placed on a method");
}
MethodInfo securedMethod = securedTarget.asMethod();

// computed permission
this.permissionKey = permissionKey;
this.computed = true;
Expand Down
21 changes: 21 additions & 0 deletions ...ent/src/main/java/io/quarkus/security/deployment/RegisterClassSecurityCheckBuildItem.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package io.quarkus.security.deployment;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.DotName;

import io.quarkus.builder.item.MultiBuildItem;

/**
* Registers security check against {@link io.quarkus.security.spi.ClassSecurityCheckStorageBuildItem}
* for security annotation instances passed in this build item.
*/
final class RegisterClassSecurityCheckBuildItem extends MultiBuildItem {

final DotName className;
final AnnotationInstance securityAnnotationInstance;

RegisterClassSecurityCheckBuildItem(DotName className, AnnotationInstance securityAnnotationInstance) {
this.className = className;
this.securityAnnotationInstance = securityAnnotationInstance;
}
}
Loading

0 comments on commit 3818f2a

Please sign in to comment.