[MISC] Catch OpenLDAP ppolicy error (#1508)

* [MISC] Catch OpenLDAP ppolicy error Further to the discussion over at #361, this change now ensures that OpenLDAP password complexity errors are caught and appropriately handled. This change also includes the PasswordComplexity test suite in the LDAP integration suite. This is because a ppolicy has been setup and enforced. * Remove password history for integration tests * Adjust max failures due to regulation trigger * Fix error handling for password resets * Refactor and include code suggestions
authelia · Dec 16, 2020 · 7c6a868 · 7c6a868
1 parent 52e6435
commit 7c6a868
Show file tree

Hide file tree

Showing 15 changed files with 111 additions and 29 deletions.
diff --git a/internal/handlers/const.go b/internal/handlers/const.go
@@ -41,7 +41,10 @@ const unableToRegisterSecurityKeyMessage = "Unable to register your security key
 const unableToResetPasswordMessage = "Unable to reset your password."
 const mfaValidationFailedMessage = "Authentication failed, please retry later."
 
-const ldapPasswordComplexityCode = "0000052D"
+const ldapPasswordComplexityCode = "0000052D."
+
+var ldapPasswordComplexityCodes = []string{"0000052D"}
+var ldapPasswordComplexityErrors = []string{"LDAP Result Code 19 \"Constraint Violation\": Password fails quality checking policy"}
 
 const testInactivity = "10"
 const testRedirectionURL = "http://redirection.local"

diff --git a/internal/handlers/handler_reset_password_step2.go b/internal/handlers/handler_reset_password_step2.go
@@ -2,9 +2,9 @@ package handlers
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/authelia/authelia/internal/middlewares"
+	"github.com/authelia/authelia/internal/utils"
 )
 
 // ResetPasswordPost handler for resetting passwords.
@@ -31,7 +31,9 @@ func ResetPasswordPost(ctx *middlewares.AutheliaCtx) {
 
 	if err != nil {
 		switch {
-		case strings.Contains(err.Error(), ldapPasswordComplexityCode):
+		case utils.IsStringInSliceContains(err.Error(), ldapPasswordComplexityCodes):
+			ctx.Error(fmt.Errorf("%s", err), ldapPasswordComplexityCode)
+		case utils.IsStringInSliceContains(err.Error(), ldapPasswordComplexityErrors):
 			ctx.Error(fmt.Errorf("%s", err), ldapPasswordComplexityCode)
 		default:
 			ctx.Error(fmt.Errorf("%s", err), unableToResetPasswordMessage)

diff --git a/internal/suites/LDAP/configuration.yml b/internal/suites/LDAP/configuration.yml
@@ -25,7 +25,7 @@ authentication_backend:
     group_name_attribute: cn
     mail_attribute: mail
     display_name_attribute: displayName
-    user: cn=admin,dc=example,dc=com
+    user: cn=pwmanager,dc=example,dc=com
     password: password
 
 session:

diff --git a/internal/suites/example/compose/ldap/ldif/01-pp.ldif b/internal/suites/example/compose/ldap/ldif/01-pp.ldif
@@ -0,0 +1,14 @@
+dn: cn=module{0},cn=config
+changetype: modify
+add: olcModuleLoad
+olcModuleLoad: ppolicy
+
+dn: olcOverlay=ppolicy,olcDatabase={1}{{ LDAP_BACKEND }},cn=config
+changetype: add
+objectClass: olcOverlayConfig
+objectClass: olcPPolicyConfig
+olcOverlay: ppolicy
+olcPPolicyDefault: cn=password,ou=policies,{{ LDAP_BASE_DN }}
+olcPPolicyHashCleartext: TRUE
+olcPPolicyUseLockout: TRUE
+olcPPolicyForwardUpdates: FALSE
diff --git a/internal/suites/example/compose/ldap/ldif/02-ppcfg.ldif b/internal/suites/example/compose/ldap/ldif/02-ppcfg.ldif
@@ -0,0 +1,25 @@
+dn: ou=policies,{{ LDAP_BASE_DN }}
+ou: policies
+objectClass: organizationalUnit
+
+dn: cn=password,ou=policies,{{ LDAP_BASE_DN }}
+objectClass: pwdPolicy
+objectClass: person
+objectClass: top
+cn: passwordDefault
+sn: passwordDefault
+pwdAttribute: userPassword
+pwdCheckQuality: 1
+pwdMinAge: 0
+pwdMaxAge: 0
+pwdMinLength: 3
+pwdInHistory: 0
+pwdMaxFailure: 5
+pwdFailureCountInterval: 0
+pwdLockout: TRUE
+pwdLockoutDuration: 0
+pwdAllowUserChange: TRUE
+pwdExpireWarning: 0
+pwdGraceAuthNLimit: 0
+pwdMustChange: FALSE
+pwdSafeModify: FALSE
diff --git a/...uites/example/compose/ldap/ldif/base.ldif → ...es/example/compose/ldap/ldif/03-base.ldif b/...uites/example/compose/ldap/ldif/base.ldif → ...es/example/compose/ldap/ldif/03-base.ldif
@@ -1,27 +1,38 @@
-dn: ou=groups,dc=example,dc=com
+dn: cn=pwmanager,{{ LDAP_BASE_DN }}
+cn: Password Manager
+displayname: Password Manager
+givenName: Password
+objectclass: inetOrgPerson
+objectclass: top
+mail: password.manager@authelia.com
+sn: Manager
+uid: pwmanager
+userPassword: {CRYPT}$6$rounds=500000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/
+
+dn: ou=groups,{{ LDAP_BASE_DN }}
 objectclass: organizationalUnit
 objectclass: top
 ou: groups
 
-dn: ou=users,dc=example,dc=com
+dn: ou=users,{{ LDAP_BASE_DN }}
 objectclass: organizationalUnit
 objectclass: top
 ou: users
 
-dn: cn=dev,ou=groups,dc=example,dc=com
+dn: cn=dev,ou=groups,{{ LDAP_BASE_DN }}
 cn: dev
-member: cn=John Doe (external),ou=users,dc=example,dc=com
-member: cn=Bob Dylan,ou=users,dc=example,dc=com
+member: cn=John Doe (external),ou=users,{{ LDAP_BASE_DN }}
+member: cn=Bob Dylan,ou=users,{{ LDAP_BASE_DN }}
 objectclass: groupOfNames
 objectclass: top
 
-dn: cn=admins,ou=groups,dc=example,dc=com
+dn: cn=admins,ou=groups,{{ LDAP_BASE_DN }}
 cn: admins
-member: cn=John Doe (external),ou=users,dc=example,dc=com
+member: cn=John Doe (external),ou=users,{{ LDAP_BASE_DN }}
 objectclass: groupOfNames
 objectclass: top
 
-dn: cn=John Doe (external),ou=users,dc=example,dc=com
+dn: cn=John Doe (external),ou=users,{{ LDAP_BASE_DN }}
 cn: John Doe (external)
 displayname: John Doe
 givenName: John
@@ -32,7 +43,7 @@ sn: Doe
 uid: john
 userpassword: {CRYPT}$6$rounds=500000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/
 
-dn: cn=Harry Potter,ou=users,dc=example,dc=com
+dn: cn=Harry Potter,ou=users,{{ LDAP_BASE_DN }}
 cn: Harry Potter
 displayname: Harry Potter
 givenName: Harry
@@ -43,7 +54,7 @@ sn: Potter
 uid: harry
 userpassword: {CRYPT}$6$rounds=500000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/
 
-dn: cn=Bob Dylan,ou=users,dc=example,dc=com
+dn: cn=Bob Dylan,ou=users,{{ LDAP_BASE_DN }}
 cn: Bob Dylan
 displayname: Bob Dylan
 givenName: Bob
@@ -54,7 +65,7 @@ sn: Dylan
 uid: bob
 userpassword: {CRYPT}$6$rounds=500000$jgiCMRyGXzoqpxS3$w2pJeZnnH8bwW3zzvoMWtTRfQYsHbWbD/hquuQ5vUeIyl9gdwBIt6RWk2S6afBA0DPakbeWgD/4SZPiS0hYtU/
 
-dn: cn=James Dean,ou=users,dc=example,dc=com
+dn: cn=James Dean,ou=users,{{ LDAP_BASE_DN }}
 cn: James Dean
 displayname: James Dean
 givenName: James

diff --git a/internal/suites/example/compose/ldap/ldif/04-access.ldif b/internal/suites/example/compose/ldap/ldif/04-access.ldif
@@ -0,0 +1,5 @@
+dn: olcDatabase={1}{{ LDAP_BACKEND }},cn=config
+changetype: modify
+replace: olcAccess
+olcAccess: {0}to attrs=userPassword,shadowLastChange by self write by dn="cn=admin,{{ LDAP_BASE_DN }}" write by dn="cn=pwmanager,{{ LDAP_BASE_DN }}" write by anonymous auth by * none
+olcAccess: {1}to * by self read by dn="cn=admin,{{ LDAP_BASE_DN }}" write by dn="cn=pwmanager,{{ LDAP_BASE_DN }}" read by * none
diff --git a/internal/suites/example/compose/ldap/ldif/access.rules b/internal/suites/example/compose/ldap/ldif/access.rules
diff --git a/internal/suites/suite_ldap_test.go b/internal/suites/suite_ldap_test.go
@@ -26,6 +26,10 @@ func (s *LDAPSuite) TestResetPassword() {
 	suite.Run(s.T(), NewResetPasswordScenario())
 }
 
+func (s *LDAPSuite) TestPasswordComplexity() {
+	suite.Run(s.T(), NewPasswordComplexityScenario())
+}
+
 func (s *LDAPSuite) TestSigninEmailScenario() {
 	suite.Run(s.T(), NewSigninEmailScenario())
 }

diff --git a/internal/utils/strings.go b/internal/utils/strings.go
@@ -30,6 +30,17 @@ func IsStringInSlice(a string, list []string) (inSlice bool) {
 	return false
 }
 
+// IsStringInSliceContains checks if a single string is in an array of strings.
+func IsStringInSliceContains(a string, list []string) (inSlice bool) {
+	for _, b := range list {
+		if strings.Contains(a, b) {
+			return true
+		}
+	}
+
+	return false
+}
+
 // SliceString splits a string s into an array with each item being a max of int d
 // d = denominator, n = numerator, q = quotient, r = remainder.
 func SliceString(s string, d int) (array []string) {

diff --git a/internal/utils/strings_test.go b/internal/utils/strings_test.go
@@ -70,6 +70,20 @@ func TestShouldNotFindSliceDifferences(t *testing.T) {
 	assert.False(t, diff)
 }
 
+func TestShouldFindStringInSliceContains(t *testing.T) {
+	a := "abc"
+	b := []string{"abc", "onetwothree"}
+	s := IsStringInSliceContains(a, b)
+	assert.True(t, s)
+}
+
+func TestShouldNotFindStringInSliceContains(t *testing.T) {
+	a := "xyz"
+	b := []string{"abc", "onetwothree"}
+	s := IsStringInSliceContains(a, b)
+	assert.False(t, s)
+}
+
 func TestShouldReturnCorrectTLSVersions(t *testing.T) {
 	tls13 := uint16(tls.VersionTLS13)
 	tls12 := uint16(tls.VersionTLS12)

diff --git a/web/src/services/Api.ts b/web/src/services/Api.ts
@@ -58,7 +58,7 @@ export function toData<T>(resp: AxiosResponse<ServiceResponse<T>>): T | undefine
 export function hasServiceError<T>(resp: AxiosResponse<ServiceResponse<T>>) {
     const errResp = toErrorResponse(resp);
     if (errResp && errResp.status === "KO") {
-        return true;
+        return { errored: true, message: errResp.message };
     }
-    return false;
+    return { errored: false, message: null };
 }
diff --git a/web/src/services/Client.ts b/web/src/services/Client.ts
@@ -4,8 +4,8 @@ import { ServiceResponse, hasServiceError, toData } from "./Api";
 export async function PostWithOptionalResponse<T = undefined>(path: string, body?: any) {
     const res = await axios.post<ServiceResponse<T>>(path, body);
 
-    if (res.status !== 200 || hasServiceError(res)) {
-        throw new Error(`Failed POST to ${path}. Code: ${res.status}.`);
+    if (res.status !== 200 || hasServiceError(res).errored) {
+        throw new Error(`Failed POST to ${path}. Code: ${res.status}. Message: ${hasServiceError(res).message}`);
     }
     return toData(res);
 }
@@ -21,7 +21,7 @@ export async function Post<T>(path: string, body?: any) {
 export async function Get<T = undefined>(path: string): Promise<T> {
     const res = await axios.get<ServiceResponse<T>>(path);
 
-    if (res.status !== 200 || hasServiceError(res)) {
+    if (res.status !== 200 || hasServiceError(res).errored) {
         throw new Error(`Failed GET from ${path}. Code: ${res.status}.`);
     }
 

diff --git a/web/src/views/ResetPassword/ResetPasswordStep2.tsx b/web/src/views/ResetPassword/ResetPasswordStep2.tsx
@@ -70,7 +70,7 @@ const ResetPasswordStep2 = function () {
             setFormDisabled(true);
         } catch (err) {
             console.error(err);
-            if (err.message.indexOf("0000052D")) {
+            if (err.message.includes("0000052D.")) {
                 createErrorNotification("Your supplied password does not meet the password policy requirements.");
             } else {
                 createErrorNotification("There was an issue resetting the password.");

diff --git a/web/tsconfig.json b/web/tsconfig.json
@@ -17,7 +17,7 @@
     "resolveJsonModule": true,
     "isolatedModules": true,
     "noEmit": true,
-    "jsx": "react",
+    "jsx": "react-jsx",
     "noFallthroughCasesInSwitch": true
   },
   "include": [