Skip to content

Commit

Permalink
fixes
Browse files Browse the repository at this point in the history 
Signed-off-by: Pavel Jareš <pavel.jares@broadcom.com>
  • Loading branch information
@pj892031
pj892031 committed Sep 22, 2023
1 parent 711e557 commit d983592
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 23 deletions.
31 changes: 22 additions & 9 deletions ...c/main/java/org/zowe/apiml/gateway/security/service/ServiceAuthenticationServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,29 +15,31 @@
import com.netflix.discovery.shared.Application;
import com.netflix.loadbalancer.reactive.ExecutionListener;
import com.netflix.zuul.context.RequestContext;
import java.util.Optional;
import lombok.AllArgsConstructor;
import lombok.RequiredArgsConstructor;
import org.apache.commons.lang.StringUtils;
import org.springframework.cache.CacheManager;
import org.springframework.cache.annotation.CacheEvict;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.context.ApplicationContext;
import org.springframework.security.authentication.BadCredentialsException;
import org.springframework.security.core.AuthenticationException;
import org.springframework.stereotype.Service;
import org.zowe.apiml.auth.Authentication;
import org.zowe.apiml.eurekaservice.client.util.EurekaMetadataParser;
import org.zowe.apiml.gateway.cache.RetryIfExpired;
import org.zowe.apiml.gateway.config.CacheConfig;
import org.zowe.apiml.gateway.security.service.schema.IAuthenticationScheme;
import org.zowe.apiml.gateway.security.service.schema.AuthenticationCommand;
import org.zowe.apiml.gateway.security.service.schema.AuthenticationSchemeFactory;
import org.zowe.apiml.gateway.security.service.schema.IAuthenticationScheme;
import org.zowe.apiml.gateway.security.service.schema.ServiceAuthenticationService;
import org.zowe.apiml.gateway.security.service.schema.source.AuthSchemeException;
import org.zowe.apiml.gateway.security.service.schema.source.AuthSource;
import org.zowe.apiml.gateway.security.service.schema.source.AuthSourceService;
import org.zowe.apiml.util.CacheUtils;

import javax.annotation.PostConstruct;
import java.util.List;
import java.util.Optional;

/**
* This bean is responsible for "translating" security to specific service. It decorate request with security data for
Expand All @@ -60,24 +62,34 @@
* also in loadbalancer
*/
@Service
@AllArgsConstructor
@RequiredArgsConstructor
public class ServiceAuthenticationServiceImpl implements ServiceAuthenticationService {

public static final String AUTHENTICATION_COMMAND_KEY = "zoweAuthenticationCommand";

private static final String CACHE_BY_SERVICE_ID = "serviceAuthenticationByServiceId";
private static final String CACHE_BY_AUTHENTICATION = "serviceAuthenticationByAuthentication";
static final String CACHE_BY_AUTHENTICATION = "serviceAuthenticationByAuthentication";

private final LoadBalancerAuthenticationCommand loadBalancerCommand = new LoadBalancerAuthenticationCommand();
private final LoadBalancerAuthentication loadBalancerAuthentication = new LoadBalancerAuthentication();

private final ApplicationContext applicationContext;
private final EurekaClient discoveryClient;
private final EurekaMetadataParser eurekaMetadataParser;
private final AuthenticationSchemeFactory authenticationSchemeFactory;
private final AuthSourceService authSourceService;
private final CacheManager cacheManager;
private final CacheUtils cacheUtils;

// to force calling inside methods with aspects - ie. ehCache aspect
private ServiceAuthenticationService meAsProxy;

@PostConstruct
public void afterPropertiesSet() {
meAsProxy = applicationContext.getBean(ServiceAuthenticationService.class);
}


/**
* Marker type of Authentication, the sole purpose of it is to highlight the fact that
* authentication cannot be determined before load balancer
Expand Down Expand Up @@ -113,8 +125,9 @@ public Authentication getAuthentication(String serviceId) {
}

@Override
@RetryIfExpired
@CacheEvict(value = CACHE_BY_AUTHENTICATION, condition = "#result != null && #result.isExpired()")
@Cacheable(value = CACHE_BY_AUTHENTICATION, condition = "#result != null")
@Cacheable(value = CACHE_BY_AUTHENTICATION, unless = "#result == null")
public AuthenticationCommand getAuthenticationCommand(Authentication authentication, AuthSource authSource) {
final IAuthenticationScheme scheme = authenticationSchemeFactory.getSchema(authentication.getScheme());
return scheme.createCommand(authentication, authSource);
Expand All @@ -127,14 +140,14 @@ public AuthenticationCommand getAuthenticationCommand(Authentication authenticat
condition = "#result != null && #result.isExpired()",
keyGenerator = CacheConfig.COMPOSITE_KEY_GENERATOR
)
@Cacheable(value = CACHE_BY_SERVICE_ID, keyGenerator = CacheConfig.COMPOSITE_KEY_GENERATOR)
@Cacheable(value = CACHE_BY_SERVICE_ID, keyGenerator = CacheConfig.COMPOSITE_KEY_GENERATOR, unless = "#result == null")
public AuthenticationCommand getAuthenticationCommand(String serviceId, Authentication found, AuthSource authSource) {
// Authentication cannot be determined before load balancer
if (found instanceof LoadBalancerAuthentication) return loadBalancerCommand;
// if no instance exist or no metadata found, do nothing
if (found == null || found.isEmpty()) return AuthenticationCommand.EMPTY;

return getAuthenticationCommand(found, authSource);
return meAsProxy.getAuthenticationCommand(found, authSource);
}

public Optional<AuthSource> getAuthSourceByAuthentication(Authentication authentication) {
Expand Down Expand Up @@ -182,7 +195,7 @@ public void apply(InstanceInfo instanceInfo) {
boolean rejected = false;
try {
final Optional<AuthSource> authSource = authSourceService.getAuthSourceFromRequest();
cmd = getAuthenticationCommand(auth, authSource.orElse(null));
cmd = meAsProxy.getAuthenticationCommand(auth, authSource.orElse(null));

// if authentication schema required valid authentication source, check it
if (cmd.isRequiredValidSource()) {
Expand Down
4 changes: 3 additions & 1 deletion ...y-service/src/test/java/org/zowe/apiml/config/service/security/MockedSecurityContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.TestConfiguration;
import org.springframework.cache.CacheManager;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.Bean;
import org.zowe.apiml.eurekaservice.client.util.EurekaMetadataParser;
import org.zowe.apiml.gateway.security.service.AuthenticationService;
Expand Down Expand Up @@ -55,8 +56,9 @@ public AuthSourceService getAuthSourceService() {
}

@Bean
public ServiceAuthenticationService getServiceAuthenticationService(@Autowired CacheManager cacheManager, CacheUtils cacheUtils) {
public ServiceAuthenticationService getServiceAuthenticationService(ApplicationContext applicationContext, @Autowired CacheManager cacheManager, CacheUtils cacheUtils) {
return new ServiceAuthenticationServiceImpl(
applicationContext,
getDiscoveryClient(),
getEurekaMetadataParser(),
getAuthenticationSchemeFactory(),
Expand Down
36 changes: 23 additions & 13 deletions ...st/java/org/zowe/apiml/gateway/security/service/ServiceAuthenticationServiceImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
import com.netflix.discovery.shared.Application;
import com.netflix.loadbalancer.reactive.ExecutionListener;
import com.netflix.zuul.context.RequestContext;
import java.security.cert.X509Certificate;
import java.util.stream.Stream;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.Setter;
Expand All @@ -36,9 +34,12 @@
import org.springframework.aop.framework.Advised;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.cache.CacheManager;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.EnableAspectJAutoProxy;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.zowe.apiml.auth.Authentication;
import org.zowe.apiml.auth.AuthenticationScheme;
import org.zowe.apiml.config.service.security.MockedSecurityContext;
import org.zowe.apiml.eurekaservice.client.util.EurekaMetadataParser;
import org.zowe.apiml.gateway.cache.RetryIfExpiredAspect;
Expand All @@ -51,16 +52,16 @@
import org.zowe.apiml.gateway.security.service.schema.source.JwtAuthSource;
import org.zowe.apiml.gateway.security.service.schema.source.X509AuthSource;
import org.zowe.apiml.gateway.utils.CurrentRequestContextTest;
import org.zowe.apiml.auth.Authentication;
import org.zowe.apiml.auth.AuthenticationScheme;
import org.zowe.apiml.security.common.token.TokenExpireException;
import org.zowe.apiml.security.common.token.TokenNotValidException;
import org.zowe.apiml.util.CacheUtils;

import javax.servlet.http.HttpServletRequest;
import java.security.cert.X509Certificate;
import java.sql.Date;
import java.time.LocalDate;
import java.util.*;
import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
Expand All @@ -69,6 +70,7 @@
import static org.zowe.apiml.constants.EurekaMetadataDefinition.AUTHENTICATION_APPLID;
import static org.zowe.apiml.constants.EurekaMetadataDefinition.AUTHENTICATION_SCHEME;
import static org.zowe.apiml.gateway.security.service.ServiceAuthenticationServiceImpl.AUTHENTICATION_COMMAND_KEY;
import static org.zowe.apiml.gateway.security.service.ServiceAuthenticationServiceImpl.CACHE_BY_AUTHENTICATION;

@ExtendWith(SpringExtension.class)
@ContextConfiguration(classes = {
Expand All @@ -79,6 +81,9 @@
@EnableAspectJAutoProxy
class ServiceAuthenticationServiceImplTest extends CurrentRequestContextTest {

@Autowired
private ApplicationContext applicationContext;

@Autowired
private EurekaClient discoveryClient;

Expand Down Expand Up @@ -107,12 +112,14 @@ void init() {
serviceAuthenticationService.evictCacheAllService();

serviceAuthenticationServiceImpl = new ServiceAuthenticationServiceImpl(
applicationContext,
discoveryClient,
new EurekaMetadataParser(),
authenticationSchemeFactory,
authSourceService,
cacheManager,
new CacheUtils());
serviceAuthenticationServiceImpl.afterPropertiesSet();
}

@AfterEach
Expand Down Expand Up @@ -322,13 +329,13 @@ void testGetAuthenticationCommand(List<ImmutablePair<AuthSource, AuthSource.Pars
assertSame(acValid, serviceAuthenticationService.getAuthenticationCommand(new Authentication(AuthenticationScheme.HTTP_BASIC_PASSTICKET, "applid"), authSource1));
verify(schemeBeanMock, times(1)).createCommand(any(), any());

// new entry - expired, dont cache that
// new entry - expired, dont cache that (+ retry aspect)
assertSame(acExpired, serviceAuthenticationService.getAuthenticationCommand(new Authentication(AuthenticationScheme.HTTP_BASIC_PASSTICKET, "applid"), authSource2));
verify(schemeBeanMock, times(2)).createCommand(any(), any());
verify(schemeBeanMock, times(3)).createCommand(any(), any());
// replace result (to know that expired record is removed and get new one)
when(schemeBeanMock.createCommand(new Authentication(AuthenticationScheme.HTTP_BASIC_PASSTICKET, "applid"), authSource2)).thenReturn(acValid);
assertSame(acValid, serviceAuthenticationService.getAuthenticationCommand(new Authentication(AuthenticationScheme.HTTP_BASIC_PASSTICKET, "applid"), authSource2));
verify(schemeBeanMock, times(3)).createCommand(any(), any());
verify(schemeBeanMock, times(4)).createCommand(any(), any());
}

@Test
Expand Down Expand Up @@ -386,7 +393,8 @@ void testGetAuthenticationCommandByServiceIdCache(AuthSource authSource) {
when(schemeBeanMock.createCommand(eq(authentication), any())).thenReturn(ac1);

assertSame(ac1, serviceAuthenticationService.getAuthenticationCommand("s1", authentication, authSource));
verify(schemeBeanMock, times(2)).createCommand(authentication, authSource);
// two cached method, if response is expired then retry - there for 4 instances - just for test, normally it is not generated as expired
verify(schemeBeanMock, times(4)).createCommand(authentication, authSource);

serviceAuthenticationService.evictCacheAllService();
Mockito.reset(schemeBeanMock);
Expand Down Expand Up @@ -467,28 +475,30 @@ void testEvictCacheService(List<AuthSource> authSourceList) {

assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0001", auth1, authSource2));
assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0002", auth1, authSource1));
verify(schemeBeanMock, times(2)).createCommand(auth1, authSource1);
verify(schemeBeanMock, times(1)).createCommand(auth1, authSource1);
verify(schemeBeanMock, times(1)).createCommand(auth1, authSource2);

serviceAuthenticationService.evictCacheService("service0001");
cacheManager.getCache(CACHE_BY_AUTHENTICATION).clear();
assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0001", auth1, authSource1));
verify(schemeBeanMock, times(3)).createCommand(auth1, authSource1);
verify(schemeBeanMock, times(2)).createCommand(auth1, authSource1);
assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0001", auth1, authSource2));
verify(schemeBeanMock, times(2)).createCommand(auth1, authSource2);
assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0001", auth2, authSource1));
verify(schemeBeanMock, times(2)).createCommand(auth2, authSource1);
assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0002", auth1, authSource1));
verify(schemeBeanMock, times(3)).createCommand(auth1, authSource1);
verify(schemeBeanMock, times(2)).createCommand(auth1, authSource1);

serviceAuthenticationService.evictCacheAllService();
cacheManager.getCache(CACHE_BY_AUTHENTICATION).clear();
assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0001", auth1, authSource1));
verify(schemeBeanMock, times(4)).createCommand(auth1, authSource1);
verify(schemeBeanMock, times(3)).createCommand(auth1, authSource1);
assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0001", auth1, authSource2));
verify(schemeBeanMock, times(3)).createCommand(auth1, authSource2);
assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0001", auth2, authSource2));
verify(schemeBeanMock, times(1)).createCommand(auth2, authSource2);
assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0002", auth1, authSource1));
verify(schemeBeanMock, times(5)).createCommand(auth1, authSource1);
verify(schemeBeanMock, times(3)).createCommand(auth1, authSource1);
assertSame(command, serviceAuthenticationService.getAuthenticationCommand("service0002", auth2, authSource1));
verify(schemeBeanMock, times(3)).createCommand(auth2, authSource1);
}
Expand Down

0 comments on commit d983592

Please sign in to comment.