Skip to content

Fix MethodDescriptor lookup in ManualGrpcSecurityMetadataSource for gRPC 1.75.0+ #1190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix MethodDescriptor lookup in ManualGrpcSecurityMetadataSource for gRPC 1.75.0+ #1190

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions .../main/java/net/devh/boot/grpc/server/security/check/ManualGrpcSecurityMetadataSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.springframework.security.access.AccessDecisionManager;
import org.springframework.security.access.ConfigAttribute;
Expand All @@ -31,6 +32,7 @@
import io.grpc.MethodDescriptor;
import io.grpc.ServerCall;
import io.grpc.ServiceDescriptor;
import lombok.extern.slf4j.Slf4j;

/**
* A {@link GrpcSecurityMetadataSource} for manual configuration. For each {@link MethodDescriptor gRPC method} a
Expand All @@ -43,13 +45,34 @@
*
* @author Daniel Theuke ([email protected])
*/
@Slf4j
public final class ManualGrpcSecurityMetadataSource extends AbstractGrpcSecurityMetadataSource {

// Use String (fullMethodName) as key to avoid MethodDescriptor identity issues with gRPC 1.75.0+
private final Map<String, Collection<ConfigAttribute>> accessMapByName = new HashMap<>();
// Keep legacy map for backward compatibility
private final Map<MethodDescriptor<?, ?>, Collection<ConfigAttribute>> accessMap = new HashMap<>();
private Collection<ConfigAttribute> defaultAttributes = wrap(AccessPredicate.denyAll());

private Collection<ConfigAttribute> getAttributes(final MethodDescriptor<?, ?> method) {
return this.accessMap.getOrDefault(method, this.defaultAttributes);
// Try name-based lookup first (more reliable with gRPC 1.75.0+)
final String fullMethodName = method.getFullMethodName();
Collection<ConfigAttribute> attrs = this.accessMapByName.get(fullMethodName);

if (attrs != null) {
log.debug("Found security config for method by name: {}", fullMethodName);
return attrs;
}

// Fallback to object-based lookup for backward compatibility
attrs = this.accessMap.get(method);
if (attrs != null) {
log.debug("Found security config for method by descriptor: {}", fullMethodName);
return attrs;
}

log.debug("No security config found for method {}, using default", fullMethodName);
return this.defaultAttributes;
}

@Override
Expand All @@ -59,7 +82,10 @@ public Collection<ConfigAttribute> getAttributes(final ServerCall<?, ?> call) {

@Override
public Collection<ConfigAttribute> getAllConfigAttributes() {
return this.accessMap.values().stream().flatMap(Collection::stream).collect(Collectors.toSet());
// Combine both maps to ensure we get all config attributes
return Stream.concat(
this.accessMap.values().stream(),
this.accessMapByName.values().stream()).flatMap(Collection::stream).collect(Collectors.toSet());
}

/**
Expand All @@ -76,6 +102,7 @@ public ManualGrpcSecurityMetadataSource set(final ServiceDescriptor service, fin
final Collection<ConfigAttribute> wrappedPredicate = wrap(predicate);
for (final MethodDescriptor<?, ?> method : service.getMethods()) {
this.accessMap.put(method, wrappedPredicate);
this.accessMapByName.put(method.getFullMethodName(), wrappedPredicate);
}
return this;
}
Expand All @@ -92,6 +119,7 @@ public ManualGrpcSecurityMetadataSource remove(final ServiceDescriptor service)
requireNonNull(service, "service");
for (final MethodDescriptor<?, ?> method : service.getMethods()) {
this.accessMap.remove(method);
this.accessMapByName.remove(method.getFullMethodName());
}
return this;
}
Expand All @@ -106,7 +134,9 @@ public ManualGrpcSecurityMetadataSource remove(final ServiceDescriptor service)
*/
public ManualGrpcSecurityMetadataSource set(final MethodDescriptor<?, ?> method, final AccessPredicate predicate) {
requireNonNull(method, "method");
this.accessMap.put(method, wrap(predicate));
final Collection<ConfigAttribute> wrappedPredicate = wrap(predicate);
this.accessMap.put(method, wrappedPredicate);
this.accessMapByName.put(method.getFullMethodName(), wrappedPredicate);
return this;
}

Expand All @@ -120,6 +150,7 @@ public ManualGrpcSecurityMetadataSource set(final MethodDescriptor<?, ?> method,
public ManualGrpcSecurityMetadataSource remove(final MethodDescriptor<?, ?> method) {
requireNonNull(method, "method");
this.accessMap.remove(method);
this.accessMapByName.remove(method.getFullMethodName());
return this;
}

Expand Down