From 7c6a86882f93515a148cadcce8eccd77c3f24433 Mon Sep 17 00:00:00 2001 From: Amir Zarrinkafsh Date: Wed, 16 Dec 2020 12:30:03 +1100 Subject: [PATCH] [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 --- internal/handlers/const.go | 5 ++- .../handlers/handler_reset_password_step2.go | 6 ++-- internal/suites/LDAP/configuration.yml | 2 +- .../example/compose/ldap/ldif/01-pp.ldif | 14 ++++++++ .../example/compose/ldap/ldif/02-ppcfg.ldif | 25 ++++++++++++++ .../ldap/ldif/{base.ldif => 03-base.ldif} | 33 ++++++++++++------- .../example/compose/ldap/ldif/04-access.ldif | 5 +++ .../example/compose/ldap/ldif/access.rules | 7 ---- internal/suites/suite_ldap_test.go | 4 +++ internal/utils/strings.go | 11 +++++++ internal/utils/strings_test.go | 14 ++++++++ web/src/services/Api.ts | 4 +-- web/src/services/Client.ts | 6 ++-- .../ResetPassword/ResetPasswordStep2.tsx | 2 +- web/tsconfig.json | 2 +- 15 files changed, 111 insertions(+), 29 deletions(-) create mode 100644 internal/suites/example/compose/ldap/ldif/01-pp.ldif create mode 100644 internal/suites/example/compose/ldap/ldif/02-ppcfg.ldif rename internal/suites/example/compose/ldap/ldif/{base.ldif => 03-base.ldif} (61%) create mode 100644 internal/suites/example/compose/ldap/ldif/04-access.ldif delete mode 100644 internal/suites/example/compose/ldap/ldif/access.rules diff --git a/internal/handlers/const.go b/internal/handlers/const.go index 03395bf0e..faed58997 100644 --- 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 index 304875386..93d7dc385 100644 --- 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 index 5e20f7e30..b0c11d78a 100644 --- 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 new file mode 100644 index 000000000..26f7d6d73 --- /dev/null +++ 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 \ No newline at end of file diff --git a/internal/suites/example/compose/ldap/ldif/02-ppcfg.ldif b/internal/suites/example/compose/ldap/ldif/02-ppcfg.ldif new file mode 100644 index 000000000..7cb87440d --- /dev/null +++ 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 \ No newline at end of file diff --git a/internal/suites/example/compose/ldap/ldif/base.ldif b/internal/suites/example/compose/ldap/ldif/03-base.ldif similarity index 61% rename from internal/suites/example/compose/ldap/ldif/base.ldif rename to internal/suites/example/compose/ldap/ldif/03-base.ldif index f3cc58224..4a56821f8 100644 --- a/internal/suites/example/compose/ldap/ldif/base.ldif +++ b/internal/suites/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 new file mode 100644 index 000000000..2f0f2e1a5 --- /dev/null +++ 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 \ No newline at end of file diff --git a/internal/suites/example/compose/ldap/ldif/access.rules b/internal/suites/example/compose/ldap/ldif/access.rules deleted file mode 100644 index 8762639e2..000000000 --- a/internal/suites/example/compose/ldap/ldif/access.rules +++ /dev/null @@ -1,7 +0,0 @@ -olcAccess: {0}to attrs=userPassword,shadowLastChange by self write by anonymou - s auth by * none -# olcAccess: {1}to dn.base="" by * read -# olcAccess: {2}to * by * read - -olcPasswordHash: {CRYPT} -olcPasswordCryptSaltFormat: $6$rounds=50000$%.16s diff --git a/internal/suites/suite_ldap_test.go b/internal/suites/suite_ldap_test.go index 7ef04f02f..607ddbbd0 100644 --- 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 index 2de982742..092873b2b 100644 --- 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 index 22b44032e..9e6cbf051 100644 --- 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 index 07446b163..0ec8ae90f 100644 --- a/web/src/services/Api.ts +++ b/web/src/services/Api.ts @@ -58,7 +58,7 @@ export function toData(resp: AxiosResponse>): T | undefine export function hasServiceError(resp: AxiosResponse>) { const errResp = toErrorResponse(resp); if (errResp && errResp.status === "KO") { - return true; + return { errored: true, message: errResp.message }; } - return false; + return { errored: false, message: null }; } \ No newline at end of file diff --git a/web/src/services/Client.ts b/web/src/services/Client.ts index b48e3c6fc..e30d10148 100644 --- 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(path: string, body?: any) { const res = await axios.post>(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(path: string, body?: any) { export async function Get(path: string): Promise { const res = await axios.get>(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 index 870a9dc86..43c4b8a7f 100644 --- 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 index 7b587d632..8ba0c002f 100644 --- 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": [