User tests: Successful: Unsuccessful:
Status | New | ⇒ | Pending |
Category | ⇒ | Administration com_admin com_media com_postinstall Modules Front End com_contact com_content com_newsfeeds com_tags |
Labels |
Added:
?
|
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.
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 |
@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
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.
@HLeithner are you talking about #28352 ?
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
Now you make sense, it was unclear before
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 |
@brianteeman please add the token check also to the other methods of com_postinstall:
https://github.com/joomla/joomla-cms/pull/28633/files#diff-af652d9f9fa9ebd48589cf8354e0056eL85
https://github.com/joomla/joomla-cms/pull/28633/files#diff-af652d9f9fa9ebd48589cf8354e0056eL59
https://github.com/joomla/joomla-cms/pull/28633/files#diff-af652d9f9fa9ebd48589cf8354e0056eL35
Thanks
conflicts resolved
Labels |
Added:
?
|
I have tested this item
CSRF token is implemented in all above mention categories forms
I have tested this item
CSRF token are implemented in all forms
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-02-03 19:30:10 |
Closed_By | ⇒ | brianteeman |
@brianteeman Why closed? And shall one one redo it?
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
and my questions about the need for form tokens was obviously incorrect.
It means they are not needed where this PR would add them?
some of these probably need fixing in J3 as well @HLeithner @zero-24