Skip to content

fix: Show error message when CSRF check fails at login - #40799

Merged
ChristophWurst merged 1 commit into
masterfrom
fix/login-csrf-check-error-message
Nov 8, 2023
Merged

fix: Show error message when CSRF check fails at login#40799
ChristophWurst merged 1 commit into
masterfrom
fix/login-csrf-check-error-message

Conversation

@ChristophWurst

@ChristophWurstChristophWurst commented Oct 6, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Summary

This is also known as the "double login"

How to test

  1. Be logged out
  2. Open the login page
  3. Clear browser cookies
  4. Enter credentials and submit the form

master: you see the same page again, no changes
here: you see the same page again but with an error message

BeforeAfter
Bildschirmfoto vom 2023-10-06 10-52-06Bildschirmfoto vom 2023-10-06 12-41-26

Checklist

@ChristophWurstChristophWurst added this to the Nextcloud 28 milestone Oct 6, 2023
@ChristophWurstChristophWurst self-assigned this Oct 6, 2023
@ChristophWurst

ChristophWurst commented Oct 6, 2023

Copy link
Copy Markdown
MemberAuthor

/backport to stable27

@ChristophWurst

ChristophWurst commented Oct 6, 2023

Copy link
Copy Markdown
MemberAuthor

/backport to stable26

@ChristophWurstChristophWurst mentioned this pull request Oct 6, 2023
Open
8 tasks
@ChristophWurstChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 6, 2023
@solracsf

solracsf commented Oct 6, 2023

Copy link
Copy Markdown
Member

I would recommend a message like "Internal checks failed. Please try again.".
Why ? Because at least user knows this is not a password or user problem, but a temporary server (internal) issue.

@ChristophWurst

ChristophWurst commented Oct 6, 2023

Copy link
Copy Markdown
MemberAuthor

Adjusted. I also fixed that the NcNoteCard was used incorrectly.

@ChristophWurst
ChristophWurstforce-pushed the fix/login-csrf-check-error-message branch from cb08db2 to 129c7b7CompareOctober 9, 2023 10:32
@ChristophWurst

ChristophWurst commented Oct 9, 2023

Copy link
Copy Markdown
MemberAuthor

/compile amend /

@ChristophWurst
ChristophWurstforce-pushed the fix/login-csrf-check-error-message branch from 129c7b7 to 4d0e11eCompareOctober 10, 2023 09:37
@ChristophWurst

ChristophWurst commented Oct 16, 2023

Copy link
Copy Markdown
MemberAuthor

/rebase

@ChristophWurst
ChristophWurstforce-pushed the fix/login-csrf-check-error-message branch 3 times, most recently from 1fe02f9 to e7d8876CompareOctober 17, 2023 18:33
@skjnldsvskjnldsv mentioned this pull request Nov 1, 2023
Merged
@ChristophWurst
ChristophWurstforce-pushed the fix/login-csrf-check-error-message branch from e7d8876 to 269dc52CompareNovember 2, 2023 09:44
@blizzzblizzz mentioned this pull request Nov 6, 2023
Merged
@ChristophWurst
fix: Show error message when CSRF check fails at login
a5422a3
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst
ChristophWurstforce-pushed the fix/login-csrf-check-error-message branch from 269dc52 to a5422a3CompareNovember 8, 2023 14:18
@ChristophWurst
ChristophWurst merged commit d751bae into masterNov 8, 2023
@ChristophWurst
ChristophWurst deleted the fix/login-csrf-check-error-message branch November 8, 2023 15:33
@ChristophWurstChristophWurst mentioned this pull request Dec 1, 2023
Merged
@solracsfsolracsf mentioned this pull request Feb 2, 2024
Closed
8 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miaulalalamiaulalalamiaulalala approved these changes
@skjnldsvskjnldsvskjnldsv approved these changes
@provokateurinprovokateurinprovokateurin approved these changes
@nickvergessennickvergessenAwaiting requested review from nickvergessen
@artongeartongeAwaiting requested review from artonge
@CarlSchwanCarlSchwanAwaiting requested review from CarlSchwan

Assignees

@ChristophWurstChristophWurst

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: authentication

Projects

💌 📅 👥 Groupware team
Archived in project

Milestone

Nextcloud 28

Development

Successfully merging this pull request may close these issues.

5 participants

@ChristophWurst@solracsf@miaulalala@skjnldsv@provokateurin