Skip to content

Commit

Permalink
Enable cache when a SecurityContext parameter is used
Browse files Browse the repository at this point in the history
This commit restores caching for the main read operation when the
SecurityContext does not expose a principal (i.e. guest access).

Closes spring-projectsgh-13238
  • Loading branch information
snicoll committed May 29, 2018
1 parent 1ce22aa commit faa9910
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
import java.util.function.Function;

import org.springframework.boot.actuate.endpoint.OperationType;
import org.springframework.boot.actuate.endpoint.SecurityContext;
import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
import org.springframework.boot.actuate.endpoint.invoke.OperationInvokerAdvisor;
import org.springframework.boot.actuate.endpoint.invoke.OperationParameter;
import org.springframework.boot.actuate.endpoint.invoke.OperationParameters;

/**
Expand All @@ -40,7 +42,7 @@ public CachingOperationInvokerAdvisor(Function<String, Long> endpointIdTimeToLiv
@Override
public OperationInvoker apply(String endpointId, OperationType operationType,
OperationParameters parameters, OperationInvoker invoker) {
if (operationType == OperationType.READ && !parameters.hasMandatoryParameter()) {
if (operationType == OperationType.READ && !hasMandatoryParameter(parameters)) {
Long timeToLive = this.endpointIdTimeToLive.apply(endpointId);
if (timeToLive != null && timeToLive > 0) {
return new CachingOperationInvoker(invoker, timeToLive);
Expand All @@ -49,4 +51,14 @@ public OperationInvoker apply(String endpointId, OperationType operationType,
return invoker;
}

private boolean hasMandatoryParameter(OperationParameters parameters) {
for (OperationParameter parameter : parameters) {
if (parameter.isMandatory()
&& !SecurityContext.class.isAssignableFrom(parameter.getType())) {
return true;
}
}
return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.mockito.MockitoAnnotations;

import org.springframework.boot.actuate.endpoint.OperationType;
import org.springframework.boot.actuate.endpoint.SecurityContext;
import org.springframework.boot.actuate.endpoint.invoke.OperationInvoker;
import org.springframework.boot.actuate.endpoint.invoke.OperationParameters;
import org.springframework.boot.actuate.endpoint.invoke.reflect.OperationMethod;
Expand Down Expand Up @@ -111,6 +112,14 @@ public void applyWithAllOptionalParametersShouldAddAdvise() {
assertAdviseIsApplied(parameters);
}

@Test
public void applyWithSecurityContextShouldAddAdvise() {
OperationParameters parameters = getParameters("getWithSecurityContext",
SecurityContext.class, String.class);
given(this.timeToLive.apply(any())).willReturn(100L);
assertAdviseIsApplied(parameters);
}

private void assertAdviseIsApplied(OperationParameters parameters) {
OperationInvoker advised = this.advisor.apply("foo", OperationType.READ,
parameters, this.invoker);
Expand Down Expand Up @@ -147,6 +156,11 @@ public String getWithAllOptionalParameters(@Nullable String foo,
return "";
}

public String getWithSecurityContext(SecurityContext securityContext,
@Nullable String bar) {
return "";
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.springframework.boot.actuate.endpoint.invoker.cache;

import java.security.Principal;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -96,6 +97,21 @@ public void targetAlwaysInvokedWithParameters() {
verify(target, times(3)).invoke(context);
}

@Test
public void targetAlwaysInvokedWithPrincipal() {
OperationInvoker target = mock(OperationInvoker.class);
Map<String, Object> parameters = new HashMap<>();
SecurityContext securityContext = mock(SecurityContext.class);
given(securityContext.getPrincipal()).willReturn(mock(Principal.class));
InvocationContext context = new InvocationContext(securityContext, parameters);
given(target.invoke(context)).willReturn(new Object());
CachingOperationInvoker invoker = new CachingOperationInvoker(target, 500L);
invoker.invoke(context);
invoker.invoke(context);
invoker.invoke(context);
verify(target, times(3)).invoke(context);
}

@Test
public void targetInvokedWhenCacheExpires() throws InterruptedException {
OperationInvoker target = mock(OperationInvoker.class);
Expand Down

0 comments on commit faa9910

Please sign in to comment.