AUT-374 - Validate country against allowed country codes list and val… (

#121) * AUT-374 - Validate country against allowed country codes list and validate configured country codes against regex during application startup * AUT-374 - update translation
e-gov · Jun 21, 2019 · 33f98d1 · 33f98d1
1 parent c2b5028
commit 33f98d1
Show file tree

Hide file tree

Showing 8 changed files with 118 additions and 13 deletions.
diff --git a/src/main/java/ee/ria/sso/config/eidas/EidasConfigurationProvider.java b/src/main/java/ee/ria/sso/config/eidas/EidasConfigurationProvider.java
@@ -1,6 +1,7 @@
 package ee.ria.sso.config.eidas;
 
 import ee.ria.sso.config.TaraResourceBundleMessageSource;
+import ee.ria.sso.utils.CountryCodeUtil;
 import lombok.Getter;
 import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
@@ -17,7 +18,11 @@
 
 import javax.annotation.PostConstruct;
 import java.security.KeyStore;
-import java.util.*;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Locale;
+import java.util.Objects;
 import java.util.stream.Collectors;
 
 @Component
@@ -57,12 +62,22 @@ public void init() {
             Assert.notNull(clientCertificateKeystorePass, "No client certificate keystore password provided");
         }
 
-        listOfCountries = parsePropertiesList(availableCountries);
+        listOfCountries = parseAvailableCountries(availableCountries);
     }
 
-    private static List<String> parsePropertiesList(String input) {
-        if (StringUtils.isEmpty(input)) return Collections.emptyList();
-        return Arrays.asList(input.split(","));
+    private static List<String> parseAvailableCountries(String input) {
+        if (StringUtils.isBlank(input)) {
+            return Collections.emptyList();
+        }
+        List<String> countryCodes = Arrays.asList(input.split(","));
+        validateCountryCodes(countryCodes);
+        return countryCodes;
+    }
+
+    private static void validateCountryCodes(List<String> countryCodes) {
+        for (String countryCode : countryCodes) {
+            Assert.isTrue(CountryCodeUtil.isValidCountryCode(countryCode), "Invalid ISO 3166-1 alpha-2 country code '" + countryCode + "'");
+        }
     }
 
     public List<String> getListOfCountries(String locale) {

diff --git a/src/main/java/ee/ria/sso/service/eidas/EidasAuthenticationService.java b/src/main/java/ee/ria/sso/service/eidas/EidasAuthenticationService.java
@@ -5,6 +5,7 @@
 import ee.ria.sso.authentication.AuthenticationType;
 import ee.ria.sso.authentication.LevelOfAssurance;
 import ee.ria.sso.authentication.credential.PreAuthenticationCredential;
+import ee.ria.sso.config.eidas.EidasConfigurationProvider;
 import ee.ria.sso.security.CspDirective;
 import ee.ria.sso.security.CspHeaderUtil;
 import ee.ria.sso.service.AbstractService;
@@ -13,6 +14,7 @@
 import ee.ria.sso.statistics.StatisticsHandler;
 import ee.ria.sso.statistics.StatisticsOperation;
 import ee.ria.sso.statistics.StatisticsRecord;
+import ee.ria.sso.utils.CountryCodeUtil;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
 import org.apereo.cas.web.flow.CasWebflowConstants;
@@ -41,16 +43,18 @@
 public class EidasAuthenticationService extends AbstractService {
 
     public static final Pattern VALID_PERSON_IDENTIFIER_PATTERN = Pattern.compile("^([A-Z]{2,2})\\/([A-Z]{2,2})\\/(.*)$");
-    public static final Pattern VALID_COUNTRY_PATTERN = Pattern.compile("^[A-Z]{2,2}$");
     public static final String SESSION_ATTRIBUTE_COUNTRY = "country";
     public static final String SESSION_ATTRIBUTE_RELAY_STATE = "relayState";
 
     private final EidasAuthenticator eidasAuthenticator;
+    private final EidasConfigurationProvider eidasConfigurationProvider;
 
     public EidasAuthenticationService(StatisticsHandler statistics,
-                                      EidasAuthenticator eidasAuthenticator) {
+                                      EidasAuthenticator eidasAuthenticator,
+                                      EidasConfigurationProvider eidasConfigurationProvider) {
         super(statistics);
         this.eidasAuthenticator = eidasAuthenticator;
+        this.eidasConfigurationProvider = eidasConfigurationProvider;
     }
 
     @Audit(
@@ -180,10 +184,18 @@ private String getCountry(RequestContext context) {
     }
 
     private void validateAndStoreCountry(PreAuthenticationCredential credential, RequestContext context) {
-        if (StringUtils.isBlank(credential.getCountry()) || !VALID_COUNTRY_PATTERN.matcher(credential.getCountry()).matches()) {
-            throw new UserAuthenticationFailedException("message.eidas.invalidcountry", String.format("User provided invalid country code: <%s>", credential.getCountry()));
+        String countryCode = credential.getCountry();
+        if (StringUtils.isBlank(countryCode) || !CountryCodeUtil.isValidCountryCode(countryCode)) {
+            throw new UserAuthenticationFailedException("message.eidas.invalidcountry", String.format("User provided invalid country code: <%s>", countryCode));
         }
-        context.getExternalContext().getSessionMap().put(SESSION_ATTRIBUTE_COUNTRY, credential.getCountry().toUpperCase());
+        if (!isAllowedCountryCode(countryCode)) {
+            throw new UserAuthenticationFailedException("message.eidas.notAllowedCountry", String.format("User provided not allowed country code: <%s>", countryCode));
+        }
+        context.getExternalContext().getSessionMap().put(SESSION_ATTRIBUTE_COUNTRY, countryCode.toUpperCase());
+    }
+
+    private boolean isAllowedCountryCode(String countryCode) {
+        return eidasConfigurationProvider.getAvailableCountries().contains(countryCode);
     }
 
     private void logEvent(RequestContext context, StatisticsOperation operation) {

diff --git a/src/main/java/ee/ria/sso/utils/CountryCodeUtil.java b/src/main/java/ee/ria/sso/utils/CountryCodeUtil.java
@@ -0,0 +1,17 @@
+package ee.ria.sso.utils;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.regex.Pattern;
+
+public class CountryCodeUtil {
+
+    private static final Pattern COUNTRY_CODE_PATTERN = Pattern.compile("^[A-Z]{2,2}$");
+
+    public static boolean isValidCountryCode(String countryCode) {
+        if (StringUtils.isBlank(countryCode)) {
+            return false;
+        }
+        return COUNTRY_CODE_PATTERN.matcher(countryCode).matches();
+    }
+}
diff --git a/src/main/webapp/WEB-INF/classes/messages.properties b/src/main/webapp/WEB-INF/classes/messages.properties
@@ -113,6 +113,7 @@ message.smartId.error.sessionNotFound=User authentication failed in Smart-ID sys
 message.smartId.error.general=Authentication failed due to internal error. Please try again in a while.
 
 message.eidas.invalidcountry=Country code is in an incorrect format.
+message.eidas.notAllowedCountry=Country code is not allowed.
 message.eidas.authfailed=Authentication failed
 message.eidas.error=Authentication failed due to internal error. 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
@@ -113,6 +113,7 @@ message.smartId.error.sessionNotFound = Kasutaja autentimine Smart-ID süsteemis
 message.smartId.error.general = Autentimine ebaõnnestus sisemise vea tõttu. Palun proovige mõne aja pärast uuesti.
 
 message.eidas.invalidcountry=Riigikood on ebakorrektses formaadis.
+message.eidas.notAllowedCountry=Riigikood ei ole lubatud.
 message.eidas.authfailed=Autentimine ebaõnnestus
 message.eidas.error=Autentimine ebaõnnestus sisemise vea tõttu. 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
@@ -113,6 +113,7 @@ message.smartId.error.sessionNotFound = Ошибка аутентификаци
 message.smartId.error.general = Ошибка аутентификации из-за внутренней ошибки. Повторите попытку через некоторое время.
 
 message.eidas.invalidcountry=Код страны указан в неверном формате.
+message.eidas.notAllowedCountry=Код страны не допускается.
 message.eidas.authfailed=Аутентификация не удалась
 message.eidas.error=Ошибка аутентификации из-за внутренней ошибки. Пожалуйста, попробуйте позже.
 

diff --git a/src/test/java/ee/ria/sso/service/eidas/EidasAuthenticationServiceTest.java b/src/test/java/ee/ria/sso/service/eidas/EidasAuthenticationServiceTest.java
@@ -3,12 +3,12 @@
 import ee.ria.sso.CommonConstants;
 import ee.ria.sso.Constants;
 import ee.ria.sso.authentication.AuthenticationType;
-import ee.ria.sso.service.ExternalServiceHasFailedException;
 import ee.ria.sso.authentication.LevelOfAssurance;
 import ee.ria.sso.authentication.credential.PreAuthenticationCredential;
 import ee.ria.sso.config.eidas.EidasConfigurationProvider;
 import ee.ria.sso.config.eidas.TestEidasConfiguration;
 import ee.ria.sso.service.AbstractAuthenticationServiceTest;
+import ee.ria.sso.service.ExternalServiceHasFailedException;
 import ee.ria.sso.service.UserAuthenticationFailedException;
 import ee.ria.sso.statistics.StatisticsHandler;
 import ee.ria.sso.statistics.StatisticsOperation;
@@ -63,6 +63,9 @@ public class EidasAuthenticationServiceTest extends AbstractAuthenticationServic
     @Autowired
     private StatisticsHandler statistics;
 
+    @Autowired
+    private EidasConfigurationProvider eidasConfigurationProvider;
+
     @Mock
     private EidasAuthenticator authenticatorMock;
 
@@ -71,7 +74,7 @@ public class EidasAuthenticationServiceTest extends AbstractAuthenticationServic
     @Before
     public void setUp() {
         Mockito.reset(authenticatorMock);
-        authenticationService = new EidasAuthenticationService(statistics, authenticatorMock);
+        authenticationService = new EidasAuthenticationService(statistics, authenticatorMock, eidasConfigurationProvider);
     }
 
     @After
@@ -81,7 +84,7 @@ public void cleanUp() {
 
     @Test
     public void startLoginByEidasWithoutLoaShouldSucceedAndWriteAuthenticatorResponse() throws Exception {
-        String country = "SE";
+        String country = "FI";
         PreAuthenticationCredential credential = new PreAuthenticationCredential();
         credential.setCountry(country);
 
@@ -134,6 +137,25 @@ public void startLoginByEidasShouldFailWhenInvalidCountryCodeFormat() throws Exc
         Assert.fail("Should not reach this!");
     }
 
+    @Test
+    public void startLoginByEidasShouldFailWhenCountryCodeFormatValidButNotAllowed() {
+        PreAuthenticationCredential credential = new PreAuthenticationCredential();
+        credential.setCountry("RU");
+
+        MockRequestContext requestContext = this.getMockRequestContext(null, credential);
+
+        expectedEx.expect(UserAuthenticationFailedException.class);
+        expectedEx.expectMessage("User provided not allowed country code: <RU>");
+
+        try {
+            this.authenticationService.startLoginByEidas(requestContext);
+            Assert.fail("Expected to throw exception!");
+        } catch (Exception e) {
+            Assert.assertTrue("Should not log to statistics when input is invalid", SimpleTestAppender.events.isEmpty());
+            throw e;
+        }
+    }
+
     @Test
     public void startLoginByEidasShouldFailWhenUnexpectedException() throws Exception {
         PreAuthenticationCredential credential = new PreAuthenticationCredential();

diff --git a/src/test/java/ee/ria/sso/utils/CountryCodeUtilTest.java b/src/test/java/ee/ria/sso/utils/CountryCodeUtilTest.java
@@ -0,0 +1,36 @@
+package ee.ria.sso.utils;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class CountryCodeUtilTest {
+
+    @Test
+    public void validCountryCodesFormat() {
+        assertTrue(CountryCodeUtil.isValidCountryCode("EE"));
+        assertTrue(CountryCodeUtil.isValidCountryCode("EN"));
+        assertTrue(CountryCodeUtil.isValidCountryCode("LT"));
+        assertTrue(CountryCodeUtil.isValidCountryCode("RU"));
+        assertTrue(CountryCodeUtil.isValidCountryCode("TT"));
+        assertTrue(CountryCodeUtil.isValidCountryCode("XX"));
+    }
+
+    @Test
+    public void invalidCountryCodeFormat() {
+        assertFalse(CountryCodeUtil.isValidCountryCode("E"));
+        assertFalse(CountryCodeUtil.isValidCountryCode("EEE"));
+        assertFalse(CountryCodeUtil.isValidCountryCode("ee"));
+        assertFalse(CountryCodeUtil.isValidCountryCode("Ee"));
+        assertFalse(CountryCodeUtil.isValidCountryCode("E E"));
+        assertFalse(CountryCodeUtil.isValidCountryCode(" EE"));
+        assertFalse(CountryCodeUtil.isValidCountryCode("EE "));
+        assertFalse(CountryCodeUtil.isValidCountryCode("--"));
+        assertFalse(CountryCodeUtil.isValidCountryCode("##"));
+        assertFalse(CountryCodeUtil.isValidCountryCode("E2"));
+        assertFalse(CountryCodeUtil.isValidCountryCode(""));
+        assertFalse(CountryCodeUtil.isValidCountryCode("  "));
+        assertFalse(CountryCodeUtil.isValidCountryCode(null));
+    }
+}