Allow failed logins in maintenance mode to be themed differently to other maintenance pages

Just had a client stumble upon this and the custom maintenance page and text made no sense in the context of failed login. They asked for a separate message / theme for failed login.

A separate template option would be great.

As a hack, I used a path-name class in the the body and hid the maintenance stuff and displayed blocked login stuff based on whether the path was `path-login`.

Done. I did not create templates for themes like Claro/Olivero here as I believe those are opinionated and should be their own issues. I also didn't create tests here as no messaging has changed.

What might be needed is a change record, as pages previously utilizing the maintenance page for this will now get a different template. Marking for review for now.

It appears there were tests around this already. Updated and added stable9 template as was expected as well.

Neat. Will definitely need a CR for the template addition.

I'd like to contribute with the Change Record. I've started it here: https://www.drupal.org/node/3458540 →

Can someone please review it?

Looks great! Thanks @carolpettirossi. Leaving for someone else to RTBC as I was original committer.

I reviewed the code, looks good, no issues found.

Also tested the branch locally:
1. Created user "bruteforce".

2. Tried to log in using incorrect password. Received standard error message.

3. After 5 failed attempts received new error message

4. Created new theme "Test" based on Olivero. Copied the template failed-login-page.html.twig and changed it.

5. Cleared cache, tried to log in again. Received error page that used new template.

6. Test passed.

The new template has identical markup to the default maintenance page template. If the intention is to allow themes to theme them differently, then we ought to be able to use template suggestions for this instead of an entirely new theme hook.

See #1804614: [meta] Consolidate similar twig templates and properly use theme suggestions in core → .

This would also provide a smoother upgrade path for themes that provide a custom maintenance template but want it to be the same for the failed login page.

b_sharpe → changed the visibility of the branch 3306107-create-a-failed to hidden.

b_sharpe → changed the visibility of the branch 11.x to hidden.

@catch good call, I've moved it to just an extension of the maintenance page which will add the theme suggestion.

Can IS be updated with latest solution. Not 100% sure if a change record is still needed? If so existing one will need updating.

IS updated. I don't think a change record is needed anymore as there is no longer any changes to core implementations here, just an additional theme suggestion for those who want it.

Going to agree, this seems like a much simpler solution.

Why flood and not login for the theme suggestion? Otherwise looks good though.

@catch because I realized the template is used for two different cases, both ip flood and login failure flood, seemed to be the common name

@catch maybe the issue summary could be updated some? Or not sure if needed

Now this is a bit interesting about the change record. There is one here but #19 says it is not needed. How will the committer (or me who follows up making sure change records are published) are to know to not publish the change record.

I think that needs to be made clear in the issue summary. Also, changing the CR as well would help. There is no set practice for this and I have seen different approaches in the change records themselves. Some people have changed the title, other have added a new first sentence is added like 'do not publish', and others delete the contents as well.

To avoid setting this back to NW I have updated the CR body to indicate that it should not be published.

I love the way this issue evolved and the end result is really simple and elegant. Thanks to everyone involved!

Also +1 for @quietone's concerns regarding change records, it would be nice if we could flag them for removal if throughout the lifetime of an issue it becomes apparent they are no longer needed. I couldn't find an issue for this in the issue queue → , but as I was searching through it I found your comment here (on the same day as your last comment in this issue): ✨ Publish change records when issues are fixed Postponed So I guess that issue can be used for that discussion.

Anyways, big +1 from me.

I went ahead and deleted the change record here.

Committed/pushed to 11.x and cherry-picked to 10.4.x, thanks!

catch → closed merge request !8930

Automatically closed - issue fixed for 2 weeks with no activity.