Repository: ory/kratos
Pull request #421 (2020-05-17 01:43:13)

fix: Clear error messages after updating settings successfully
## Related issue

#420

## Proposed changes

Clear error messages after updating settings successfully.

## Checklist

<!--
Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of
them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
-->

- [x] I have read the [contributing guidelines](../blob/master/CONTRIBUTING.md).
- [x] I have read the [security policy](../security/policy).
- [x] I confirm that this pull request does not address a security
      vulnerability. If this pull request addresses a security. vulnerability, I
      confirm that I got green light (please contact
      [security@ory.sh](mailto:security@ory.sh)) from the maintainers to push
      the changes.
- [x] I have added tests that prove my fix is effective or that my feature
      works.
- [ ] I have added or changed [the documentation](docs/docs).



aeneasr: Perfect, thank you!

Patch:
diff --git a/selfservice/flow/settings/hook.go b/selfservice/flow/settings/hook.go
index 0f5c40e6b..dcbdbdf6e 100644
--- a/selfservice/flow/settings/hook.go
+++ b/selfservice/flow/settings/hook.go
@@ -122,6 +122,10 @@ func (e *HookExecutor) PostSettingsHook(w http.ResponseWriter, r *http.Request,
 		}
 	}
 
+	if method, ok := ctxUpdate.Request.Methods[settingsType]; ok {
+		method.Config.ResetErrors()
+	}
+
 	if err := e.d.SettingsRequestPersister().UpdateSettingsRequest(r.Context(), ctxUpdate.Request); err != nil {
 		return err
 	}


Test patch:
diff --git a/selfservice/strategy/password/settings_test.go b/selfservice/strategy/password/settings_test.go
index 013460e76..1e2034a78 100644
--- a/selfservice/strategy/password/settings_test.go
+++ b/selfservice/strategy/password/settings_test.go
@@ -57,49 +57,50 @@ func TestSettings(t *testing.T) {
 	secondaryUser := clients["secondary"]
 	adminClient := testhelpers.NewSDKClient(adminTS)
 
-	t.Run("description=should fail if password violates policy", func(t *testing.T) {
-		var run = func(t *testing.T) {
-			rs := testhelpers.GetSettingsRequest(t, primaryUser, publicTS)
-			form := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config
-			values := testhelpers.SDKFormFieldsToURLValues(form.Fields)
-			values.Set("password", "123456")
-			actual, _ := testhelpers.SettingsSubmitForm(t, form, primaryUser, values)
+	t.Run("description=should update the password and clear errors after input error occurred", func(t *testing.T) {
+		rs := testhelpers.GetSettingsRequest(t, primaryUser, publicTS)
+
+		form := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config
+		values := testhelpers.SDKFormFieldsToURLValues(form.Fields)
+
+		t.Run("description=should fail if password violates policy", func(t *testing.T) {
+			var run = func(t *testing.T) {
+				values.Set("password", "123456")
+				actual, _ := testhelpers.SettingsSubmitForm(t, form, primaryUser, values)
 
-			assert.Equal(t, *form.Action, gjson.Get(actual, "methods.password.config.action").String(), "%s", actual)
-			assert.Empty(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).value").String(), "%s", actual)
-			assert.NotEmpty(t, gjson.Get(actual, "methods.password.config.fields.#(name==csrf_token).value").String(), "%s", actual)
-			assert.Contains(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).errors.0.message").String(), "the password does not fulfill the password policy because", "%s", actual)
-		}
+				assert.Equal(t, *form.Action, gjson.Get(actual, "methods.password.config.action").String(), "%s", actual)
+				assert.Empty(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).value").String(), "%s", actual)
+				assert.NotEmpty(t, gjson.Get(actual, "methods.password.config.fields.#(name==csrf_token).value").String(), "%s", actual)
+				assert.Contains(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).errors.0.message").String(), "the password does not fulfill the password policy because", "%s", actual)
+			}
 
-		t.Run("session=with privileged session", run)
+			t.Run("session=with privileged session", run)
 
-		t.Run("session=needs reauthentication", func(t *testing.T) {
-			viper.Set(configuration.ViperKeySelfServicePrivilegedAuthenticationAfter, "1ns")
+			t.Run("session=needs reauthentication", func(t *testing.T) {
+				viper.Set(configuration.ViperKeySelfServicePrivilegedAuthenticationAfter, "1ns")
 
-			_ = testhelpers.NewSettingsLoginAcceptAPIServer(t, adminClient)
-			t.Cleanup(func() {
-				viper.Set(configuration.ViperKeySelfServicePrivilegedAuthenticationAfter, "5m")
+				_ = testhelpers.NewSettingsLoginAcceptAPIServer(t, adminClient)
+				t.Cleanup(func() {
+					viper.Set(configuration.ViperKeySelfServicePrivilegedAuthenticationAfter, "5m")
+				})
+				run(t)
 			})
-			run(t)
 		})
-	})
-
-	t.Run("description=should update the password if everything is ok", func(t *testing.T) {
-		rs := testhelpers.GetSettingsRequest(t, primaryUser, publicTS)
 
-		form := rs.Payload.Methods[string(identity.CredentialsTypePassword)].Config
-		values := testhelpers.SDKFormFieldsToURLValues(form.Fields)
-		values.Set("password", uuid.New().String())
-		actual, _ := testhelpers.SettingsSubmitForm(t, form, primaryUser, values)
+		t.Run("description=should update the password and clear errors if everything is ok", func(t *testing.T) {
+			values.Set("password", uuid.New().String())
+			actual, _ := testhelpers.SettingsSubmitForm(t, form, primaryUser, values)
 
-		assert.Equal(t, true, gjson.Get(actual, "update_successful").Bool(), "%s", actual)
-		assert.Empty(t, gjson.Get(actual, "methods.password.fields.#(name==password).value").String(), "%s", actual)
+			assert.Equal(t, true, gjson.Get(actual, "update_successful").Bool(), "%s", actual)
+			assert.Empty(t, gjson.Get(actual, "methods.password.fields.#(name==password).value").String(), "%s", actual)
+			assert.Empty(t, gjson.Get(actual, "methods.password.config.fields.#(name==password).errors.0.message").String(), actual)
 
-		actualIdentity, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(context.Background(), primaryIdentity.ID)
-		require.NoError(t, err)
-		cfg := string(actualIdentity.Credentials[identity.CredentialsTypePassword].Config)
-		assert.NotContains(t, cfg, "foo")
-		assert.NotEqual(t, `{"hashed_password":"foo"}`, cfg)
+			actualIdentity, err := reg.PrivilegedIdentityPool().GetIdentityConfidential(context.Background(), primaryIdentity.ID)
+			require.NoError(t, err)
+			cfg := string(actualIdentity.Credentials[identity.CredentialsTypePassword].Config)
+			assert.NotContains(t, cfg, "foo")
+			assert.NotEqual(t, `{"hashed_password":"foo"}`, cfg)
+		})
 	})
 
 	t.Run("description=should update the password even if no password was set before", func(t *testing.T) {
diff --git a/test/e2e/cypress/integration/profiles/email/settings/success.spec.js b/test/e2e/cypress/integration/profiles/email/settings/success.spec.js
index 49214590c..67782d07c 100644
--- a/test/e2e/cypress/integration/profiles/email/settings/success.spec.js
+++ b/test/e2e/cypress/integration/profiles/email/settings/success.spec.js
@@ -34,6 +34,19 @@ context('Settings', () => {
 
   describe('password', () => {
     it('modifies the password with privileged session', () => {
+      // Once input weak password to test which error message is cleared after updating successfully
+      cy.get('#user-password input[name="password"]').clear().type('123')
+      cy.get('#user-password button[type="submit"]').click()
+      cy.get('.container').should(
+        'not.contain.text',
+        'Your changes have been saved!'
+      )
+      cy.get('.container').should(
+        'contain.text',
+        'the password does not fulfill the password policy because: password length must be at least 6 characters but only got 3'
+      )
+      cy.get('#user-password input[name="password"]').should('be.empty')
+
       password = up(password)
       cy.get('#user-password input[name="password"]').clear().type(password)
       cy.get('#user-password button[type="submit"]').click()
@@ -41,6 +54,10 @@ context('Settings', () => {
         'contain.text',
         'Your changes have been saved!'
       )
+      cy.get('.container').should(
+        'not.contain.text',
+        'the password does not fulfill the password policy because: password length must be at least 6 characters but only got 3'
+      )
       cy.get('#user-password input[name="password"]').should('be.empty')
     })