From d5e0c5258bda21604f1c9290a90fd713fabdd9f7 Mon Sep 17 00:00:00 2001 From: Richard Eckart de Castilho Date: Mon, 22 Jan 2024 20:39:45 +0100 Subject: [PATCH 1/3] #4442 - Option to allow spaces in usernames - Added option - Added test - Added documentation --- .../clarin/webanno/security/UserDaoImpl.java | 13 +++++++-- .../security/config/SecurityProperties.java | 2 ++ .../config/SecurityPropertiesImpl.java | 12 +++++++++ .../security-authentication-policy.adoc | 4 +++ .../webanno/security/UserDaoImplTest.java | 27 ++++++++++++++----- .../ui/core/WicketApplicationBase.properties | 2 +- 6 files changed, 51 insertions(+), 9 deletions(-) diff --git a/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/UserDaoImpl.java b/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/UserDaoImpl.java index cfe9fd2d721..55dc7e7053c 100644 --- a/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/UserDaoImpl.java +++ b/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/UserDaoImpl.java @@ -488,8 +488,8 @@ public List validateUsername(String aName) } // Do not allow space - if (containsAnyCharacterMatching(aName, Character::isWhitespace)) { - errors.add(new ValidationError("Username cannot contain a space character") // + if (containsAnyCharacterMatching(aName, this::isWhitespaceIllegalInUsername)) { + errors.add(new ValidationError("Username cannot contain whitespace") // .addKey(MSG_USERNAME_ERROR_ILLEGAL_SPACE)); return errors; } @@ -552,6 +552,15 @@ public List validateUsername(String aName) return errors; } + private boolean isWhitespaceIllegalInUsername(char ch) + { + if (securityProperties.isSpaceAllowedInUsername() && ch == ' ') { + return false; + } + + return Character.isWhitespace((int) ch); + } + @Override public boolean isValidUsername(String aName) { diff --git a/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/config/SecurityProperties.java b/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/config/SecurityProperties.java index c32877c1af4..c737390f5c7 100644 --- a/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/config/SecurityProperties.java +++ b/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/config/SecurityProperties.java @@ -53,4 +53,6 @@ public interface SecurityProperties Pattern getUsernamePattern(); Pattern getPasswordPattern(); + + boolean isSpaceAllowedInUsername(); } diff --git a/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/config/SecurityPropertiesImpl.java b/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/config/SecurityPropertiesImpl.java index a5471320f50..d9518bd32ce 100644 --- a/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/config/SecurityPropertiesImpl.java +++ b/inception/inception-security/src/main/java/de/tudarmstadt/ukp/clarin/webanno/security/config/SecurityPropertiesImpl.java @@ -31,6 +31,7 @@ public class SecurityPropertiesImpl private String defaultAdminUsername; private String defaultAdminPassword; private boolean defaultAdminRemoteAccess = false; + private boolean spaceAllowedInUsername = false; public static final int HARD_MINIMUM_PASSWORD_LENGTH = 0; public static final int DEFAULT_MINIMUM_PASSWORD_LENGTH = 8; @@ -152,4 +153,15 @@ public void setPasswordPattern(Pattern aPasswordPattern) { passwordPattern = aPasswordPattern; } + + @Override + public boolean isSpaceAllowedInUsername() + { + return spaceAllowedInUsername; + } + + public void setSpaceAllowedInUsername(boolean aSpaceInUsernameAllowed) + { + spaceAllowedInUsername = aSpaceInUsernameAllowed; + } } diff --git a/inception/inception-security/src/main/resources/META-INF/asciidoc/admin-guide/security-authentication-policy.adoc b/inception/inception-security/src/main/resources/META-INF/asciidoc/admin-guide/security-authentication-policy.adoc index f93cae1a6f3..7ac095e595a 100644 --- a/inception/inception-security/src/main/resources/META-INF/asciidoc/admin-guide/security-authentication-policy.adoc +++ b/inception/inception-security/src/main/resources/META-INF/asciidoc/admin-guide/security-authentication-policy.adoc @@ -64,6 +64,10 @@ NOTE: In addition to the restrictions imposed here, {product-name} may impose ad | `.*` | `(?=.\*[a-z])(?=.*[A-Z])(?=.\*[0-9])(?=.*\p{Punct}).*` +| `security.space-allowed-in-username` +| Whether simple space characters are permitted in usernames. Enable this option only when necessary to restore access to existing accounts in certain scenarios such as when using external authentication. Usernames with spaces may lead to problems e.g. when exporting/importing projects or documents or when constructing certain URLs. +| `false` +| `true` |=== \ No newline at end of file diff --git a/inception/inception-security/src/test/java/de/tudarmstadt/ukp/clarin/webanno/security/UserDaoImplTest.java b/inception/inception-security/src/test/java/de/tudarmstadt/ukp/clarin/webanno/security/UserDaoImplTest.java index d8cdf76e4ed..220c831782f 100644 --- a/inception/inception-security/src/test/java/de/tudarmstadt/ukp/clarin/webanno/security/UserDaoImplTest.java +++ b/inception/inception-security/src/test/java/de/tudarmstadt/ukp/clarin/webanno/security/UserDaoImplTest.java @@ -45,7 +45,9 @@ import de.tudarmstadt.ukp.clarin.webanno.security.config.SecurityPropertiesImpl; import de.tudarmstadt.ukp.clarin.webanno.security.model.User; -@DataJpaTest(showSql = false) +@DataJpaTest(showSql = false, // + properties = { // + "spring.main.banner-mode=off" }) @EnableAutoConfiguration @ImportAutoConfiguration( // exclude = { LiquibaseAutoConfiguration.class }, // @@ -70,6 +72,7 @@ void setup() securityProperties.setMaximumUsernameLength(DEFAULT_MAXIMUM_USERNAME_LENGTH); securityProperties.setMinimumPasswordLength(DEFAULT_MINIMUM_PASSWORD_LENGTH); securityProperties.setMaximumPasswordLength(DEFAULT_MINIMUM_PASSWORD_LENGTH); + securityProperties.setSpaceAllowedInUsername(false); } @Test @@ -121,6 +124,18 @@ void thatUsernamePatternIsRespected() assertThat(userDao.isValidUsername("ay")).isFalse(); } + @Test + void thatAllowingSpaceInUsernameWorks() + { + securityProperties.setMinimumUsernameLength(0); + + securityProperties.setSpaceAllowedInUsername(false); + assertThat(userDao.isValidUsername("a z")).isFalse(); + + securityProperties.setSpaceAllowedInUsername(true); + assertThat(userDao.isValidUsername("a z")).isTrue(); + } + @Test void thatInvalidDefaultUserNameIsRejected() { @@ -150,27 +165,27 @@ void testUsernameValidationErrorMessages() assertThat(userDao.validateUsername(" john")) // .hasSize(1) // .extracting(ValidationError::getMessage).first().asString() // - .contains("cannot contain a space"); + .contains("cannot contain whitespace"); assertThat(userDao.validateUsername("john ")) // .hasSize(1) // .extracting(ValidationError::getMessage).first().asString() // - .contains("cannot contain a space"); + .contains("cannot contain whitespace"); assertThat(userDao.validateUsername("jo hn")) // .hasSize(1) // .extracting(ValidationError::getMessage).first().asString() // - .contains("cannot contain a space"); + .contains("cannot contain whitespace"); assertThat(userDao.validateUsername("jo\thn")) // .hasSize(1) // .extracting(ValidationError::getMessage).first().asString() // - .contains("cannot contain a space"); + .contains("cannot contain whitespace"); assertThat(userDao.validateUsername("john\n")) // .hasSize(1) // .extracting(ValidationError::getMessage).first().asString() // - .contains("cannot contain a space"); + .contains("cannot contain whitespace"); assertThat(userDao.validateUsername("john\0")) // .hasSize(1) // diff --git a/inception/inception-ui-core/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/core/WicketApplicationBase.properties b/inception/inception-ui-core/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/core/WicketApplicationBase.properties index 20c76c4ca69..94b6b53c81f 100644 --- a/inception/inception-ui-core/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/core/WicketApplicationBase.properties +++ b/inception/inception-ui-core/src/main/java/de/tudarmstadt/ukp/clarin/webanno/ui/core/WicketApplicationBase.properties @@ -132,7 +132,7 @@ password.error.pattern-mismatch=Password invalid. It must match the pattern ${pa password.error.control-characters=Password cannot contain any control characters username.error.blank=Username cannot be empty or blank -username.error.illegal-space=Username cannot contain a space character +username.error.illegal-space=Username cannot contain whitespace username.error.illegal-characters=Username cannot contain any of these characters: ${chars} username.error.control-characters=Username cannot contain any control characters username.error.reserved=Username is reserved From 80c77b31892c411f61d3572fcbeaa9229ddc778a Mon Sep 17 00:00:00 2001 From: Richard Eckart de Castilho Date: Mon, 22 Jan 2024 21:00:00 +0100 Subject: [PATCH 2/3] #4444 - Make authentication settings more prominent in admin manual - Move authentication to a top-level section in the sidebar --- .../META-INF/asciidoc/admin-guide.adoc | 19 +++-- .../admin-guide/settings_security-policy.adoc | 73 +++++++++++++++++++ 2 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 inception/inception-security/src/main/resources/META-INF/asciidoc/admin-guide/settings_security-policy.adoc diff --git a/inception/inception-doc/src/main/resources/META-INF/asciidoc/admin-guide.adoc b/inception/inception-doc/src/main/resources/META-INF/asciidoc/admin-guide.adoc index 302621b1e17..003b2f6d949 100644 --- a/inception/inception-doc/src/main/resources/META-INF/asciidoc/admin-guide.adoc +++ b/inception/inception-doc/src/main/resources/META-INF/asciidoc/admin-guide.adoc @@ -50,6 +50,16 @@ include::{include-dir}installation_kubernetes.adoc[leveloffset=+1] include::{include-dir}installation_unsupervised.adoc[leveloffset=+1] +include::{include-dir}security-authentication.adoc[] + +include::{include-dir}security-authentication-oauth2.adoc[leveloffset=+1] + +include::{include-dir}security-authentication-saml2.adoc[leveloffset=+1] + +include::{include-dir}security-authentication-auto-login.adoc[leveloffset=+1] + +include::{include-dir}security-authentication-preauth.adoc[leveloffset=+1] + <<< include::{include-dir}logging.adoc[] @@ -80,17 +90,12 @@ include::{include-dir}settings.adoc[] include::{include-dir}settings_general.adoc[leveloffset=+1] +include::{include-dir}settings_security-policy.adoc[leveloffset=+1] + include::{include-dir}settings_database.adoc[leveloffset=+1] include::{include-dir}settings_server.adoc[leveloffset=+1] -include::{include-dir}security-authentication.adoc[leveloffset=+1] -include::{include-dir}security-authentication-oauth2.adoc[leveloffset=+2] -include::{include-dir}security-authentication-saml2.adoc[leveloffset=+2] -include::{include-dir}security-authentication-auto-login.adoc[leveloffset=+2] -include::{include-dir}security-authentication-preauth.adoc[leveloffset=+2] -include::{include-dir}security-authentication-policy.adoc[leveloffset=+2] - include::{include-dir}settings_internal-backup.adoc[leveloffset=+1] include::{include-dir}settings_cas-storage.adoc[leveloffset=+1] diff --git a/inception/inception-security/src/main/resources/META-INF/asciidoc/admin-guide/settings_security-policy.adoc b/inception/inception-security/src/main/resources/META-INF/asciidoc/admin-guide/settings_security-policy.adoc new file mode 100644 index 00000000000..2ce39e75f48 --- /dev/null +++ b/inception/inception-security/src/main/resources/META-INF/asciidoc/admin-guide/settings_security-policy.adoc @@ -0,0 +1,73 @@ +// Licensed to the Technische Universität Darmstadt under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The Technische Universität Darmstadt +// licenses this file to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +[[sect_security_authentication_policy]] += Security policies + +You have several options of configuring which types of usernames and passwords are accepted. +Note that these values are only enforced when creating new users or updating passwords. They +will not invalidate existing usernames or passwords. Length restrictions and patterns are checked +independently. If either fails, the username or password is rejected. + +NOTE: When external pre-authentication is used, these settings are ignored. + +NOTE: In addition to the restrictions imposed here, {product-name} may impose additional restrictions. + E.g. certain usernames are not allowed and certain characters are also not allowed to appear in usernames. + +[cols="4*", options="header"] +|=== +| Setting +| Description +| Default +| Example + +| `security.minimum-password-length` +| Minimum number of characters a password can have +| 8 +| (max 128) + +| `security.maximum-password-length` +| Maximum number of characters a password can have +| 32 +| (max 128) + +| `security.minimum-username-length` +| Minimum number of characters a username can have +| 4 +| (max 128) + +| `security.maximum-username-length` +| Maximum number of characters a username can have +| 64 +| (max 128) + +| `security.username-pattern` +| Regular expression for valid usernames +| `.*` +| `[a-zA-Z0-9]+` + +| `security.password-pattern` +| Regular expression for valid passwords +| `.*` +| `(?=.\*[a-z])(?=.*[A-Z])(?=.\*[0-9])(?=.*\p{Punct}).*` + +| `security.space-allowed-in-username` +| Whether simple space characters are permitted in usernames. Enable this option only when necessary to restore access to existing accounts in certain scenarios such as when using external authentication. Usernames with spaces may lead to problems e.g. when exporting/importing projects or documents or when constructing certain URLs. +| `false` +| `true` +|=== + + \ No newline at end of file From 4eb2b9996447c0549ca8fc3e2567025ced09f0e8 Mon Sep 17 00:00:00 2001 From: Richard Eckart de Castilho Date: Mon, 22 Jan 2024 21:07:28 +0100 Subject: [PATCH 3/3] No issue: Try fixing test by releasing latch only after materializing result --- .../export/controller/ExportServiceControllerImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inception/inception-project-export/src/test/java/de/tudarmstadt/ukp/inception/project/export/controller/ExportServiceControllerImplTest.java b/inception/inception-project-export/src/test/java/de/tudarmstadt/ukp/inception/project/export/controller/ExportServiceControllerImplTest.java index 3ba50170f21..d614173e254 100644 --- a/inception/inception-project-export/src/test/java/de/tudarmstadt/ukp/inception/project/export/controller/ExportServiceControllerImplTest.java +++ b/inception/inception-project-export/src/test/java/de/tudarmstadt/ukp/inception/project/export/controller/ExportServiceControllerImplTest.java @@ -275,8 +275,8 @@ public Type getPayloadType(StompHeaders aHeaders) public void handleFrame(StompHeaders aHeaders, Object aPayload) { LOG.info("GOT MESSAGE: {}", aPayload); - responseRecievedLatch.countDown(); messageRecieved.set(true); + responseRecievedLatch.countDown(); } }); }