Refactoring Login Form to Symfony Forms

[LordSimal]

As already decided in #1976 here is one part of the great "Symfony Forms Refactoring"

What did I do

Basically I looked at how Symfony Forms work in #1976 since the Reset Password Form was automatically generated.

Creating the Form Type

Then I created src/Form/LoginType.php with the 2 main fields username and password with a basic NoBlank constraint and the default placeholder which was previously set in the template.

The configureOptions basically lets us set some configs for the generated form, mainly used here to alter with the generated CSRF token name and ID (which is later used in the credentials check process)

Adjusting the Authentication Controller

After that I went into the src/Controller/Backend/AuthenticationController.php and changed the login() function to create the new LoginForm instance.

Since the default value for the "last known username" functionality was now part of the Form it has been transfered into the previously created src/Form/LoginType.php as 'data' => $last_username

Adjusting the LoginFormAuthenticator

With that there were also some changes needed in the src/Security/LoginFormAuthenticator.php due to the fact, that we now have to read out the username, password and CSRF-Token slightly differently.

In the getUser function the previously mentioned CSRF-Token ID is now used to check if its valid.

Adjusting the Template

Finally only the template for the login page templates/security/login.html.twig just needed to render the form with its fields.

About the "Remember Me" checkbox

I tried to implement the Remember me checkbox the same as username and password but somehow the Cookie wasn't being created with that. I guess it has to with the fact, that the input name wouldn't be exactly _remember_me but login[_remember_me]

About the "NonBlank" constraint

I tried to add a basic constraint to both fields which should throw a validation error "early" if the inputs are empty.
But no matter what I try I either get

• Invalid credentials. with a valid user but empty password
• Username could not be found. with an empty user but a filled out password

For that I temporarily removed the required option from both fields, but somehow the other validation is done before the NonBlank constraint of the Symfony Forms.

And thats basically it 😄

I first thought it would me much harder to adapt to Symfony Forms but it was rather straight forward in my opinion.

[simongroenewolt]

Just a note: I see a lot of failures in the (Travis CI) behat tests - maybe you can adjust you PR in a way that makes the tests work again? (hoping the tests are not to 'coupled' to the code) Otherwise the tests need to be updated to match the changes to the login form.

[LordSimal]

I see, I'l look into that

[LordSimal]

basically I found, that some tests filled in the login form with the "old" input fields.
But due to the fact, that Symfony Forms always wraps the name of the field like login[username] instead of username I had to adjust some field names (tests/behat/bootstrap/WebContext.php and tests/e2e/login.feature )

[LordSimal]

I also removed some "double login as admin" commands in tests/e2e/edit_record_1.feature and tests/e2e/record_localization.feature because they were now triggering erros.
due to the new login handling you can't go back to the login form if you are already logged

[LordSimal]

The only thing I don't understand is the fact, that only for PHP 7.2 there is a "missing" yes content in the #results-six for the URL /page/setcontent-test-page

For PHP 7.3 and 7.4 everything was OK - so Travis/Behat quirk and try again?

[simongroenewolt]

The only thing I don't understand is the fact, that only for PHP 7.2 there is a "missing" yes content in the #results-six for the URL /page/setcontent-test-page

For PHP 7.3 and 7.4 everything was OK - so Travis/Behat quirk and try again?

I'd think with only 1 error it's probably a (very annoying) 'random' error. Good work on getting the tests running again!

[I-Valchev]

@LordSimal @simongroenewolt I'll restart it brute force till it works ;)

[I-Valchev]

Hi @LordSimal , thanks for submitting this PR, it looks good! I found your comments in the initial post very helpful for reviewing, so thanks for that :-)

As for the remember me, it can be done by specifying the remember_me_parameter in security.yaml according to the Symfony documentation. I did that locally and it works, so what I am going to do now is to merge this in and follow up with a small PR to update this field.

Here comes the merge train! 🚂 🚃 🚃 🚃