Merge branch 'develop'

e-gov · Dec 18, 2020 · d2f540c · d2f540c
2 parents 3267ac2 + e2e0052
commit d2f540c
Show file tree

Hide file tree

Showing 12 changed files with 202 additions and 34 deletions.
diff --git a/src/main/java/ee/ria/sso/flow/action/AbstractAuthenticationAction.java b/src/main/java/ee/ria/sso/flow/action/AbstractAuthenticationAction.java
@@ -7,11 +7,14 @@
 import ee.ria.sso.flow.ThymeleafSupport;
 import ee.ria.sso.service.ExternalServiceHasFailedException;
 import ee.ria.sso.service.UserAuthenticationFailedException;
+import ee.ria.sso.service.manager.ManagerService;
 import lombok.AllArgsConstructor;
 import lombok.NoArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.apereo.cas.authentication.principal.WebApplicationService;
+import org.apereo.cas.services.AbstractRegisteredService;
 import org.apereo.cas.support.oauth.authenticator.Authenticators;
+import org.apereo.cas.support.oauth.services.OAuthRegisteredService;
 import org.apereo.cas.web.support.WebUtils;
 import org.pac4j.core.context.Pac4jConstants;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -22,6 +25,8 @@
 
 import java.io.IOException;
 import java.security.cert.CertificateException;
+import java.util.List;
+import java.util.Optional;
 
 import static ee.ria.sso.Constants.CAS_SERVICE_ATTRIBUTE_NAME;
 
@@ -36,14 +41,19 @@ public abstract class AbstractAuthenticationAction extends AbstractAction {
     @Autowired
     private ThymeleafSupport thymeleafSupport;
 
+    @Autowired
+    private ManagerService managerService;
+
     protected abstract Event doAuthenticationExecute(RequestContext requestContext) throws IOException, CertificateException;
 
     protected abstract AuthenticationType getAuthenticationType();
 
     @Override
     protected Event doExecute(RequestContext requestContext) throws Exception {
 
-        assertSessionNotExpiredAndAuthMethodAllowed(requestContext);
+        WebApplicationService service = getWebApplicationService(requestContext);
+        assertValidClient(requestContext, service);
+        assertSessionNotExpiredAndAuthMethodAllowed(requestContext, service);
 
         try {
             return this.doAuthenticationExecute(requestContext);
@@ -65,19 +75,39 @@ protected Event doExecute(RequestContext requestContext) throws Exception {
         }
     }
 
-    private void assertSessionNotExpiredAndAuthMethodAllowed(RequestContext requestContext) {
-        WebApplicationService service = getWebApplicationService(requestContext);
+    private void assertValidClient(RequestContext requestContext, WebApplicationService service) {
         if (service == null) {
             log.error("Callback failed! No service parameter found in flow of session! Possible causes: either the user session has expired, server has been restarted in the middle of user transaction or corrupt/invalid cookie value was sent from the browser");
             throw AuthenticationFlowExecutionException.ofUnauthorized(requestContext, this, messageSource.getMessage(Constants.MESSAGE_KEY_SESSION_EXPIRED));
-        } else if (isOauth2Client(service) && !requestContext.getExternalContext().getSessionMap().contains(Pac4jConstants.REQUESTED_URL)) {
-            log.error("Oauth callback url not found in session! Possible causes: either the user session has expired, server has been restarted in the middle of user transaction or corrupt/invalid cookie value was sent from the browser");
-            throw AuthenticationFlowExecutionException.ofUnauthorized(requestContext, this, messageSource.getMessage(Constants.MESSAGE_KEY_SESSION_EXPIRED));
         }
 
-        if (isOauth2Client(service) && !thymeleafSupport.isAuthMethodAllowed(getAuthenticationType())) {
-            log.error("This authentication method usage was not initially specified by the scope parameter when the authentication process was initialized!");
-            throw AuthenticationFlowExecutionException.ofUnauthorized(requestContext, this, messageSource.getMessage(Constants.MESSAGE_KEY_AUTH_METHOD_RESTRICTED_BY_SCOPE));
+        if (!isOauth2Client(service) && !isCASClient(service.getOriginalUrl())) {
+            log.error("Invalid service value in detected in webflow. Either the client_name parameter is invalid or the service URL is not allowed");
+            throw AuthenticationFlowExecutionException.ofUnauthorized(requestContext, this, messageSource.getMessage(Constants.MESSAGE_KEY_GENERAL_ERROR));
+        }
+    }
+
+    private boolean isCASClient(String serviceUrl) {
+        Optional<List<AbstractRegisteredService>> abstractRegisteredServices = managerService.getAllRegisteredServicesExceptType(OAuthRegisteredService.class);
+        if (abstractRegisteredServices.isPresent()) {
+            for (AbstractRegisteredService ars: abstractRegisteredServices.get()) {
+                if (serviceUrl.matches(ars.getServiceId())) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+    private void assertSessionNotExpiredAndAuthMethodAllowed(RequestContext requestContext, WebApplicationService service) {
+        if (isOauth2Client(service)) {
+            if (!requestContext.getExternalContext().getSessionMap().contains(Pac4jConstants.REQUESTED_URL)) {
+                log.error("Oauth callback url not found in session! Possible causes: either the user session has expired, server has been restarted in the middle of user transaction or corrupt/invalid cookie value was sent from the browser");
+                throw AuthenticationFlowExecutionException.ofUnauthorized(requestContext, this, messageSource.getMessage(Constants.MESSAGE_KEY_SESSION_EXPIRED));
+            } else if (!thymeleafSupport.isAuthMethodAllowed(getAuthenticationType())) {
+                log.error("This authentication method usage was not initially specified by the scope parameter when the authentication process was initialized!");
+                throw AuthenticationFlowExecutionException.ofUnauthorized(requestContext, this, messageSource.getMessage(Constants.MESSAGE_KEY_AUTH_METHOD_RESTRICTED_BY_SCOPE));
+            }
         }
     }
 

diff --git a/src/main/java/ee/ria/sso/service/manager/ManagerService.java b/src/main/java/ee/ria/sso/service/manager/ManagerService.java
@@ -1,8 +1,10 @@
 package ee.ria.sso.service.manager;
 
+import org.apereo.cas.services.AbstractRegisteredService;
 import org.apereo.cas.services.OidcRegisteredService;
 import org.apereo.cas.services.RegisteredServiceProperty;
 
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
@@ -15,5 +17,6 @@ public interface ManagerService {
    Optional<OidcRegisteredService> getServiceByName(String serviceName);
    Optional<Map<String, RegisteredServiceProperty>> getServiceNames(String serviceName);
    Optional<String> getServiceShortName();
+   Optional<List<AbstractRegisteredService>> getAllRegisteredServicesExceptType(Class<?> type);
 
 }
diff --git a/src/main/java/ee/ria/sso/service/manager/ManagerServiceImpl.java b/src/main/java/ee/ria/sso/service/manager/ManagerServiceImpl.java
@@ -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;
@@ -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
@@ -64,6 +67,15 @@ public Optional<Map<String, RegisteredServiceProperty>> getServiceNames(String s
         return service.map(AbstractRegisteredService::getProperties);
     }
 
+
+    @Override
+    public Optional<List<AbstractRegisteredService>> getAllRegisteredServicesExceptType(Class<?> type) {
+        return Optional.of(this.servicesManager.getAllServices().stream()
+                .filter(r -> r instanceof AbstractRegisteredService && !(type.isInstance(r)) && !(r.getServiceId().equals(getRegistryServiceURL())))
+                .map(s -> (AbstractRegisteredService) s)
+                .collect(Collectors.toList()));
+    }
+
     @Override
     public Optional<String> getServiceShortName() {
         String serviceName;
@@ -86,4 +98,8 @@ public Optional<String> getServiceShortName() {
 
         return Optional.empty();
     }
+
+    private String getRegistryServiceURL() {
+        return casOAuthConfiguration.oauthCallbackService().getId();
+    }
 }
diff --git a/src/main/webapp/WEB-INF/classes/messages.properties b/src/main/webapp/WEB-INF/classes/messages.properties
@@ -85,7 +85,7 @@ message.idc.doesidcardexist=Do you have your ID-card inserted into the card read
 message.idc.nocertificate=No certificate found! Do you have your ID-card inserted into the card reader?
 message.idc.certnotyetvalid=Your certificates are invalid.
 message.idc.certexpired=Your certificates are invalid.
-message.idc.revoked=Your certificates are invalid.
+message.idc.revoked=Your certificates have been revoked. You can check the status of your ID card certificates in the DigiDoc4 client https://www.id.ee/en/article/validity-of-id-card-certificates-2/
 message.idc.unknown=Your certificates are invalid.
 message.idc.error.ocsp.not.available=Certificate status enquiry failed. Please try again later.
 message.idc.error=Certificate enquiry failed. Please try again later.

diff --git a/src/main/webapp/WEB-INF/classes/messages_et.properties b/src/main/webapp/WEB-INF/classes/messages_et.properties
@@ -85,7 +85,7 @@ message.idc.doesidcardexist=Kas Teie ID-kaart on kaardilugejasse sisestatud?
 message.idc.nocertificate=Sertifikaati ei leitud! Kas Teie ID-kaart on kaardilugejasse sisestatud?
 message.idc.certnotyetvalid=Teie sertifikaadid ei kehti.
 message.idc.certexpired=Teie sertifikaadid ei kehti.
-message.idc.revoked=Teie sertifikaadid ei kehti.
+message.idc.revoked=Teie sertifikaadid on tühistatud. Oma ID-kaardi sertifikaatide olekut saate kontrollida DigiDoc4 kliendis https://www.id.ee/artikkel/id-kaardi-sertifikaatide-kehtivus-2/
 message.idc.unknown=Teie sertifikaadid ei kehti.
 message.idc.error.ocsp.not.available=Sertifikaadi kehtivuse info küsimine ei õnnestunud. Palun proovige mõne aja pärast uuesti.
 message.idc.error=Sertifikaadi küsimine ei õnnestunud. Palun proovige mõne aja pärast uuesti.

diff --git a/src/main/webapp/WEB-INF/classes/messages_ru.properties b/src/main/webapp/WEB-INF/classes/messages_ru.properties
@@ -85,7 +85,7 @@ message.idc.doesidcardexist=Пожалуйста проверьте, встав
 message.idc.nocertificate=Сертификат не найден! Пожалуйста проверьте, вставлена ли Ваша ID-карта в считыватель?
 message.idc.certnotyetvalid=Ваши сертификаты не действительны.
 message.idc.certexpired=Ваши сертификаты не действительны.
-message.idc.revoked=Ваши сертификаты не действительны.
+message.idc.revoked=Ваши сертификаты аннулированы. Вы можете проверить статус сертификатов вашей ID-карты в клиенте DigiDoc4 https://www.id.ee/ru/artikkel/dejstvitelnost-sertifikatov-id-karty-2/
 message.idc.unknown=Ваши сертификаты не действительны.
 message.idc.error.ocsp.not.available=Запрос сертификата не удался. Пожалуйста, попробуйте позже.
 message.idc.error=Запрос сертификата не удался. Пожалуйста, попробуйте позже.

diff --git a/src/test/java/ee/ria/sso/config/TestTaraConfiguration.java b/src/test/java/ee/ria/sso/config/TestTaraConfiguration.java
@@ -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;
@@ -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); }

diff --git a/src/test/java/ee/ria/sso/flow/action/AbstractAuthenticationActionTest.java b/src/test/java/ee/ria/sso/flow/action/AbstractAuthenticationActionTest.java
@@ -2,13 +2,16 @@
 
 import ee.ria.sso.AbstractTest;
 import ee.ria.sso.Constants;
-import ee.ria.sso.service.ExternalServiceHasFailedException;
-import ee.ria.sso.service.UserAuthenticationFailedException;
 import ee.ria.sso.authentication.AuthenticationType;
 import ee.ria.sso.config.TaraResourceBundleMessageSource;
 import ee.ria.sso.flow.AuthenticationFlowExecutionException;
 import ee.ria.sso.flow.ThymeleafSupport;
+import ee.ria.sso.service.ExternalServiceHasFailedException;
+import ee.ria.sso.service.UserAuthenticationFailedException;
+import ee.ria.sso.service.manager.ManagerService;
 import org.apereo.cas.authentication.principal.AbstractWebApplicationService;
+import org.apereo.cas.services.AbstractRegisteredService;
+import org.apereo.cas.support.oauth.services.OAuthRegisteredService;
 import org.hamcrest.Description;
 import org.hamcrest.TypeSafeMatcher;
 import org.junit.Before;
@@ -21,7 +24,10 @@
 import org.springframework.webflow.execution.Event;
 import org.springframework.webflow.execution.RequestContext;
 
+import java.util.ArrayList;
 import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
 
 import static org.junit.Assert.assertTrue;
 
@@ -33,6 +39,9 @@ public abstract class AbstractAuthenticationActionTest {
     @Mock
     private TaraResourceBundleMessageSource messageSource;
 
+    @Mock
+    private ManagerService managerService;
+
     @Rule
     public ExpectedException expectedEx = ExpectedException.none();
 
@@ -47,6 +56,8 @@ public void setUp() {
         requestContext.getExternalContext().getSessionMap().put(Pac4jConstants.REQUESTED_URL, "https://localhost:8451/response");
         requestContext.getExternalContext().getSessionMap().put(Constants.TARA_OIDC_SESSION_AUTH_METHODS, Collections.singletonList(AuthenticationType.SmartID));
         Mockito.when(thymeleafSupport.isAuthMethodAllowed(Mockito.any())).thenReturn(true);
+        Optional<List<AbstractRegisteredService>> mockedAbstractRegisteredServices = mockAbstractRegisteredServices();
+        Mockito.when(managerService.getAllRegisteredServicesExceptType(OAuthRegisteredService.class)).thenReturn(mockedAbstractRegisteredServices);
     }
 
     @Test
@@ -64,6 +75,12 @@ public void successWhenValidServiceMissingFromFlowContextAndPresentInSession() t
         getAction().doExecute(requestContext);
     }
 
+    @Test
+    public void successWhenValidAbstractServicePresentButNoMatchingServiceURL() throws Exception {
+        requestContext.getFlowScope().put(Constants.CAS_SERVICE_ATTRIBUTE_NAME, new AbstractWebApplicationService("id", "https://not-cas.server.url/?client_name=CasOAuthClient", "artifactId") {});
+        getAction().doExecute(requestContext);
+    }
+
     @Test
     public void exceptionWhenAuthenticationMethodNotInAllowedList() throws Exception {
         expectedEx.expect(AuthenticationFlowExecutionException.class);
@@ -77,7 +94,8 @@ public void exceptionWhenAuthenticationMethodNotInAllowedList() throws Exception
 
     @Test
     public void invalidOriginalUrlInService() throws Exception {
-        requestContext.getFlowScope().put(Constants.CAS_SERVICE_ATTRIBUTE_NAME, new AbstractWebApplicationService("id", null, "artifactId") {});
+        expectedEx.expect(AuthenticationFlowExecutionException.class);
+        requestContext.getFlowScope().put(Constants.CAS_SERVICE_ATTRIBUTE_NAME, new AbstractWebApplicationService("id", "", "artifactId") {});
         getAction().doExecute(requestContext);
     }
 
@@ -89,7 +107,7 @@ public void unexpectedExceptionOccursDuringAuthentication() throws Exception {
         try {
 
             Mockito.when(messageSource.getMessage(Mockito.eq(Constants.MESSAGE_KEY_GENERAL_ERROR))).thenReturn("mock general error");
-            new AbstractAuthenticationAction(messageSource, thymeleafSupport) {
+            new AbstractAuthenticationAction(messageSource, thymeleafSupport, managerService) {
 
                 @Override
                 protected Event doAuthenticationExecute(RequestContext requestContext) {
@@ -115,7 +133,7 @@ public void upstreamServiceExceptionOccursDuringAuthentication() throws Exceptio
 
         try {
             Mockito.when(messageSource.getMessage(Mockito.eq("msg.key"))).thenReturn("mock translation");
-            new AbstractAuthenticationAction(messageSource, thymeleafSupport) {
+            new AbstractAuthenticationAction(messageSource, thymeleafSupport, managerService) {
 
                 @Override
                 protected Event doAuthenticationExecute(RequestContext requestContext) {
@@ -141,7 +159,7 @@ public void authFailedExceptionOccursDuringAuthentication() throws Exception {
 
         try {
             Mockito.when(messageSource.getMessage(Mockito.eq("msg.key"))).thenReturn("Mock translation");
-            new AbstractAuthenticationAction(messageSource, thymeleafSupport) {
+            new AbstractAuthenticationAction(messageSource, thymeleafSupport, managerService) {
 
                 @Override
                 protected Event doAuthenticationExecute(RequestContext requestContext) {
@@ -185,6 +203,16 @@ public void errorWhenValidServicePresentAndUsinCasOauthClientIsMissingCallbackUr
         getAction().doExecute(requestContext);
     }
 
+    @Test
+    public void exceptionWhenClientNameIsInvalid() throws Exception {
+        Mockito.when(messageSource.getMessage(Constants.MESSAGE_KEY_GENERAL_ERROR)).thenReturn("Mock general error");
+
+        expectedEx.expect(AuthenticationFlowExecutionException.class);
+        expectedEx.expect(new ExceptionCodeMatches(401, "Mock general error"));
+        requestContext.getFlowScope().put(Constants.CAS_SERVICE_ATTRIBUTE_NAME, new AbstractWebApplicationService("id", "", "artifactId") {});
+        getAction().doExecute(requestContext);
+    }
+
 
     class ExceptionCodeMatches extends TypeSafeMatcher<AuthenticationFlowExecutionException> {
         private int code;
@@ -219,4 +247,13 @@ protected void describeMismatchSafely(AuthenticationFlowExecutionException item,
     private void assertContextCleared(RequestContext requestContext) {
         assertTrue("flow context was not cleared!", requestContext.getFlowScope().isEmpty());
     }
+
+    private Optional<List<AbstractRegisteredService>> mockAbstractRegisteredServices() {
+        List<AbstractRegisteredService> abstractRegisteredServices = new ArrayList<>();
+        AbstractRegisteredService abstractRegisteredService = Mockito.mock(AbstractRegisteredService.class);
+        Mockito.when(abstractRegisteredService.getServiceId()).thenReturn("^https://cas.server.url.*");
+
+        abstractRegisteredServices.add(abstractRegisteredService);
+        return Optional.of(abstractRegisteredServices);
+    }
 }