Skip to content
This repository has been archived by the owner on Aug 10, 2021. It is now read-only.

Commit

Permalink
AUT-529 - Fixed setting invalid clientId
Browse files Browse the repository at this point in the history 
AUT-529
  • Loading branch information
@martenrebane
martenrebane committed Dec 8, 2020
1 parent 52137a4 commit e2e0052
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 21 deletions.
11 changes: 9 additions & 2 deletions src/main/java/ee/ria/sso/service/manager/ManagerServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import ee.ria.sso.Constants;
import ee.ria.sso.utils.SessionMapUtil;
import org.apereo.cas.config.CasOAuthConfiguration;
import org.apereo.cas.services.AbstractRegisteredService;
import org.apereo.cas.services.OidcRegisteredService;
import org.apereo.cas.services.RegisteredService;
Expand All @@ -28,9 +29,11 @@ public class ManagerServiceImpl implements ManagerService {

private final Logger log = LoggerFactory.getLogger(ManagerServiceImpl.class);
private final ServicesManager servicesManager;
private final CasOAuthConfiguration casOAuthConfiguration;

public ManagerServiceImpl(ServicesManager servicesManager) {
public ManagerServiceImpl(ServicesManager servicesManager, CasOAuthConfiguration casOAuthConfiguration) {
this.servicesManager = servicesManager;
this.casOAuthConfiguration = casOAuthConfiguration;
}

@Override
Expand Down Expand Up @@ -68,7 +71,7 @@ public Optional<Map<String, RegisteredServiceProperty>> getServiceNames(String s
@Override
public Optional<List<AbstractRegisteredService>> getAllRegisteredServicesExceptType(Class<?> type) {
return Optional.of(this.servicesManager.getAllServices().stream()
.filter(r -> r instanceof AbstractRegisteredService && !(type.isInstance(r)))
.filter(r -> r instanceof AbstractRegisteredService && !(type.isInstance(r)) && !(r.getServiceId().equals(getRegistryServiceURL())))
.map(s -> (AbstractRegisteredService) s)
.collect(Collectors.toList()));
}
Expand All @@ -95,4 +98,8 @@ public Optional<String> getServiceShortName() {

return Optional.empty();
}

private String getRegistryServiceURL() {
return casOAuthConfiguration.oauthCallbackService().getId();
}
}
14 changes: 14 additions & 0 deletions src/test/java/ee/ria/sso/config/TestTaraConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
import org.apereo.cas.audit.AuditTrailRecordResolutionPlan;
import org.apereo.cas.audit.AuditableExecution;
import org.apereo.cas.audit.spi.DefaultAuditTrailRecordResolutionPlan;
import org.apereo.cas.authentication.AuthenticationSystemSupport;
import org.apereo.cas.authentication.principal.PrincipalFactory;
import org.apereo.cas.authentication.principal.ServiceFactory;
import org.apereo.cas.authentication.principal.WebApplicationService;
import org.apereo.cas.config.CasOAuthConfiguration;
import org.apereo.cas.oidc.token.OidcIdTokenSigningAndEncryptionService;
import org.apereo.cas.oidc.util.OidcAuthorizationRequestSupport;
import org.apereo.cas.services.OidcRegisteredService;
Expand Down Expand Up @@ -103,6 +105,18 @@ public ServicesManager servicesManager() {
return Mockito.mock(ServicesManager.class);
}

@Bean
@Qualifier("casOAuthConfiguration")
public CasOAuthConfiguration casOAuthConfiguration() {
return Mockito.mock(CasOAuthConfiguration.class);
}

@Bean
@Qualifier("defaultAuthenticationSystemSupport")
public AuthenticationSystemSupport authenticationSystemSupport() {
return Mockito.mock(AuthenticationSystemSupport.class);
}

@Bean
@Qualifier("oidcPrincipalFactory")
public PrincipalFactory oidcPrincipalFactory() {return Mockito.mock(PrincipalFactory.class); }
Expand Down
51 changes: 32 additions & 19 deletions src/test/java/ee/ria/sso/service/manager/ManagerServiceImplTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package ee.ria.sso.service.manager;

import org.apereo.cas.authentication.principal.Service;
import org.apereo.cas.config.CasOAuthConfiguration;
import org.apereo.cas.services.AbstractRegisteredService;
import org.apereo.cas.services.OidcRegisteredService;
import org.apereo.cas.services.RegisteredService;
Expand Down Expand Up @@ -40,14 +42,16 @@ public class ManagerServiceImplTest {
private static final String SERVICE_SHORT_NAME_VALUE_RU = "openIdDemoShortNameRU";
private static final String SERVICE_ID = "https://cas.server.url";

private final CasOAuthConfiguration casOAuthConfiguration = Mockito.mock(CasOAuthConfiguration.class);

@Test
public void getServiceByID_managerReturnsValidService_shouldReturnNonEmptyOptional() {
Collection<RegisteredService> registeredServices = new ArrayList<>();
OidcRegisteredService oidcRegisteredService = new OidcRegisteredService();
oidcRegisteredService.setClientId(SERVICE_NAME);
registeredServices.add(oidcRegisteredService);
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

Assert.assertTrue(managerService.getServiceByName(SERVICE_NAME).isPresent());
}
Expand All @@ -56,7 +60,7 @@ public void getServiceByID_managerReturnsValidService_shouldReturnNonEmptyOption
public void getServiceByID_managerReturnsNoService_shouldReturnEmptyOptional() {
Collection<RegisteredService> registeredServices = new ArrayList<>();
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

Assert.assertFalse(managerService.getServiceByName(SERVICE_NAME).isPresent());
}
Expand All @@ -72,7 +76,7 @@ public void getServiceByID_managerReturnsDuplicateService_shouldReturnEmptyOptio
registeredServices.add(duplicateOidcRegisteredService);

ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

Assert.assertEquals(Optional.empty(), managerService.getServiceByName(SERVICE_NAME));
}
Expand All @@ -81,7 +85,7 @@ public void getServiceByID_managerReturnsDuplicateService_shouldReturnEmptyOptio
public void getServiceByID_serviceManagerThrowsRuntimeException_shouldReturnEmptyOptional() {
ServicesManager servicesManager = Mockito.mock(ServicesManager.class);
when(servicesManager.findServiceBy(SERVICE_NAME)).thenThrow(RuntimeException.class);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

Assert.assertFalse(managerService.getServiceByName(SERVICE_NAME).isPresent());
}
Expand All @@ -95,7 +99,7 @@ public void getServiceName_managerReturnsValidName_shouldReturnName() {
registeredServices.add(oidcRegisteredService);
LocaleContextHolder.setLocale(Locale.forLanguageTag("et"));
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

HashMap<String, RegisteredServiceProperty> serviceNames = new HashMap<>();
serviceNames.put(SERVICE_NAME_KEY, oidcRegisteredService.getProperties().get(SERVICE_NAME_KEY));
Expand All @@ -112,7 +116,7 @@ public void getServiceShortName_managerReturnsValidEnglishShortName_shouldReturn
registeredServices.add(oidcRegisteredService);
LocaleContextHolder.setLocale(Locale.ENGLISH);
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

HashMap<String, RegisteredServiceProperty> serviceShortNames = new HashMap<>();
serviceShortNames.put(SERVICE_SHORT_NAME_KEY_EN, oidcRegisteredService.getProperties().get(SERVICE_SHORT_NAME_KEY_EN));
Expand All @@ -129,7 +133,7 @@ public void getServiceShortName_managerReturnsValidRussianShortName_shouldReturn
registeredServices.add(oidcRegisteredService);
LocaleContextHolder.setLocale(Locale.forLanguageTag("ru"));
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

HashMap<String, RegisteredServiceProperty> serviceShortNames = new HashMap<>();
serviceShortNames.put(SERVICE_SHORT_NAME_KEY_RU, oidcRegisteredService.getProperties().get(SERVICE_SHORT_NAME_KEY_RU));
Expand All @@ -146,7 +150,7 @@ public void getServiceShortName_managerReturnsValidShortName_shouldReturnShortNa
registeredServices.add(oidcRegisteredService);
LocaleContextHolder.setLocale(Locale.forLanguageTag("et"));
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

HashMap<String, RegisteredServiceProperty> serviceShortNames = new HashMap<>();
serviceShortNames.put(SERVICE_SHORT_NAME_KEY, oidcRegisteredService.getProperties().get(SERVICE_SHORT_NAME_KEY));
Expand All @@ -163,7 +167,7 @@ public void getServiceName_managerReturnsValidEnglishName_shouldReturnEnglishNam
registeredServices.add(oidcRegisteredService);
LocaleContextHolder.setLocale(Locale.ENGLISH);
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

HashMap<String, RegisteredServiceProperty> serviceNames = new HashMap<>();
serviceNames.put(SERVICE_NAME_KEY_EN, oidcRegisteredService.getProperties().get(SERVICE_NAME_KEY_EN));
Expand All @@ -180,7 +184,7 @@ public void getServiceName_managerReturnsValidRussianName_shouldReturnRussianNam
registeredServices.add(oidcRegisteredService);
LocaleContextHolder.setLocale(Locale.forLanguageTag("ru"));
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

HashMap<String, RegisteredServiceProperty> serviceNames = new HashMap<>();
serviceNames.put(SERVICE_NAME_KEY_RU, oidcRegisteredService.getProperties().get(SERVICE_NAME_KEY_RU));
Expand All @@ -195,19 +199,22 @@ public void getServiceShortName_managerReturnsNoProperties_shouldReturnEmptyName
oidcRegisteredService.setClientId(SERVICE_NAME);
registeredServices.add(oidcRegisteredService);
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

Assert.assertEquals(Optional.of(new HashMap<String, RegisteredServiceProperty>()), managerService.getServiceNames(SERVICE_NAME));
}

@Test
public void getAllAbstractRegisteredServices_managerReturnsValidServices_shouldReturnNonEmptyOptional() {
Collection<RegisteredService> registeredServices = new ArrayList<>();
Mockito.when(casOAuthConfiguration.oauthCallbackService()).thenReturn(mockService());
Mockito.when(casOAuthConfiguration.oauthCallbackService().getId()).thenReturn(SERVICE_ID);
AbstractRegisteredService abstractRegisteredService = Mockito.mock(AbstractRegisteredService.class);
Mockito.when(abstractRegisteredService.getServiceId()).thenReturn(SERVICE_ID);
registeredServices.add(abstractRegisteredService);

ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

Assert.assertTrue(managerService.getAllRegisteredServicesExceptType(OAuthRegisteredService.class).isPresent());
}
Expand All @@ -219,21 +226,23 @@ public void getAllAbstractRegisteredServices_managerReturnsNoProperties_shouldRe
oidcRegisteredService.setClientId(SERVICE_NAME);
registeredServices.add(oidcRegisteredService);
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

Assert.assertEquals(Optional.of(new ArrayList<AbstractRegisteredService>()), managerService.getAllRegisteredServicesExceptType(OAuthRegisteredService.class));
}

@Test
public void getAllAbstractRegisteredServices_managerFiltersUnnecessaryServices_shouldReturnMultipleServices() {
Collection<RegisteredService> registeredServices = new ArrayList<>(mockGetAllAbstractRegisteredServices().get());
Mockito.when(casOAuthConfiguration.oauthCallbackService()).thenReturn(mockService());
Mockito.when(casOAuthConfiguration.oauthCallbackService().getId()).thenReturn(SERVICE_ID);
OidcRegisteredService oidcRegisteredService = new OidcRegisteredService();
oidcRegisteredService.setClientId(SERVICE_NAME);
registeredServices.add(oidcRegisteredService);
ServicesManager servicesManager = createValidServicesManagerWith(registeredServices);
ManagerService managerService = new ManagerServiceImpl(servicesManager);
ManagerService managerService = new ManagerServiceImpl(servicesManager, casOAuthConfiguration);

Assert.assertEquals(2, managerService.getAllRegisteredServicesExceptType(OAuthRegisteredService.class).get().size());
Assert.assertEquals(1, managerService.getAllRegisteredServicesExceptType(OAuthRegisteredService.class).get().size());
}

private Map<String, RegisteredServiceProperty> mockOidcRegisteredServiceProperties(String key, String value) {
Expand All @@ -251,10 +260,10 @@ private Optional<List<AbstractRegisteredService>> mockGetAllAbstractRegisteredSe
List<AbstractRegisteredService> abstractRegisteredServices = new ArrayList<>();
AbstractRegisteredService oneAbstractRegisteredService = Mockito.mock(AbstractRegisteredService.class);
AbstractRegisteredService twoAbstractRegisteredService = Mockito.mock(AbstractRegisteredService.class);
oneAbstractRegisteredService.setServiceId(SERVICE_ID);
oneAbstractRegisteredService.setName(SERVICE_NAME + "1");
twoAbstractRegisteredService.setServiceId(SERVICE_ID + ".secondurl");
twoAbstractRegisteredService.setName(SERVICE_NAME + "2");
Mockito.when(oneAbstractRegisteredService.getServiceId()).thenReturn(SERVICE_ID);
Mockito.when(oneAbstractRegisteredService.getName()).thenReturn(SERVICE_NAME + "1");
Mockito.when(twoAbstractRegisteredService.getServiceId()).thenReturn(SERVICE_ID + ".secondUrl");
Mockito.when(twoAbstractRegisteredService.getName()).thenReturn(SERVICE_NAME + "2");

abstractRegisteredServices.add(oneAbstractRegisteredService);
abstractRegisteredServices.add(twoAbstractRegisteredService);
Expand All @@ -270,6 +279,10 @@ private static ServicesManager createValidServicesManagerWith(Collection<Registe
return servicesManager;
}

private static Service mockService() {
return Mockito.mock(Service.class);
}

static class RegisteredServicePropertyValues implements RegisteredServiceProperty {

private Set<String> values = new HashSet<>();
Expand Down

0 comments on commit e2e0052

Please sign in to comment.