Conflicting Files ? Success

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
10 Apr 2020

all forms should have a csrf.token as discussed in #25634

#25634 (comment)

code review

avatar brianteeman brianteeman - open - 10 Apr 2020
avatar brianteeman brianteeman - change - 10 Apr 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 10 Apr 2020
Category Administration com_admin com_media com_postinstall Modules Front End com_contact com_content com_newsfeeds com_tags
avatar brianteeman
brianteeman - comment - 10 Apr 2020

some of these probably need fixing in J3 as well @HLeithner @zero-24

avatar brianteeman brianteeman - change - 10 Apr 2020
Labels Added: ?
avatar HLeithner
HLeithner - comment - 11 Apr 2020

Adding the token in the form but not validating it makes the token worth less, what I mean is if we add a token we also have to validate it on submit.

avatar joomla-cms-bot joomla-cms-bot - change - 11 Apr 2020
Category Administration com_admin com_media com_postinstall Modules Front End com_contact com_content com_newsfeeds com_tags Administration com_admin com_postinstall Modules Front End com_contact com_content com_newsfeeds com_tags
avatar brianteeman
brianteeman - comment - 11 Apr 2020

@HLeithner validation is a different issue. I agree 100% with you that the validation need to be enforced. I did raise that before to the usual echo chamber and lack of response from jsst

avatar HLeithner
HLeithner - comment - 13 Apr 2020

The problem is if it's not done at the same time it looks like an unfixed security issue, so adding the check on the other side is a one liner and have to be done at the same time.

avatar brianteeman
brianteeman - comment - 13 Apr 2020

@HLeithner are you talking about #28352 ?

avatar HLeithner
HLeithner - comment - 16 Apr 2020

No I talk about the server validation, if you add the token in the form you have to add the corresponding $this->checkToken(); in the controller too

avatar brianteeman
brianteeman - comment - 18 Apr 2020

Now you make sense, it was unclear before

avatar joomla-cms-bot joomla-cms-bot - change - 8 May 2020
Category Administration com_admin com_postinstall Modules Front End com_contact com_content com_newsfeeds com_tags Administration com_admin com_postinstall Front End com_contact com_content com_newsfeeds com_tags
avatar zero-24
zero-24 - comment - 13 May 2020

Thanks

avatar brianteeman
brianteeman - comment - 22 Jun 2020

conflicts resolved

avatar brianteeman brianteeman - change - 2 Aug 2020
Labels Added: ?
avatar priyankaSahutekdi priyankaSahutekdi - test_item - 7 Nov 2020 - Tested successfully
avatar priyankaSahutekdi
priyankaSahutekdi - comment - 7 Nov 2020

I have tested this item successfully on b992817

CSRF token is implemented in all above mention categories forms


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28633.

avatar punambaravkar punambaravkar - test_item - 7 Nov 2020 - Tested successfully
avatar punambaravkar
punambaravkar - comment - 7 Nov 2020

I have tested this item successfully on b992817

CSRF token are implemented in all forms


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28633.

avatar brianteeman brianteeman - change - 3 Feb 2021
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2021-02-03 19:30:10
Closed_By brianteeman
avatar brianteeman brianteeman - close - 3 Feb 2021
avatar richard67
richard67 - comment - 3 Feb 2021

@brianteeman Why closed? And shall one one redo it?

avatar brianteeman
brianteeman - comment - 3 Feb 2021

After 10 months with no progress and going back even further to 2019, with j4 hopefully now in its final beta, its clear that its never going to be merged, and my questions about the need for form tokens was obviously incorrect. So no point in leaving it open if its never going to go anywhere. I have triaged my own code

avatar richard67
richard67 - comment - 3 Feb 2021

and my questions about the need for form tokens was obviously incorrect.

It means they are not needed where this PR would add them?

Add a Comment

Login with GitHub to post a comment