Well, I'm thinking that there are really two issues here:
1. We need to have some other token to go along with the SignOutNow postback action so that we authenticate that the user actually intended to click the button. 2. We need to change the Icon, Picture, and Name/Value pair inputs on the "Personal Information" form so that they do not allow bogus values. For the Icon and Picture, I think a simple regex to validate that they are using a proper URL. For the Name/Value pairs, we need to just make sure that something like htmlentities is used...
For the log-out link, should the key be added to the link href attribute? Should it log-in out be done with Ajax to hide the key? Or should it use a form?
The later is the correct way but it will require updates to style and themes, unless I add the style to the form and button tags.
I say we use a key value in a regular link--its the least impact to users and themes but will still work with javascript turned off. The same FormPostBackKey that the forms use would be ideal.
We can completely remove the key from the URL with a redirect on log-out if desired. Or make sure a delegate is in place for an extension to do that.
Still need work on themes/people_signout_form_nopostback.php with the formatting (and dictionary entry): <?php // Note: This file is included from the library/People/People.Control.Leave.php class.
PostBackControl::IsValidFormPostBack() allow to defining the CSRF validation error: /** * Check a form is valid by check Actions and CSRF protection key. * * Will raised an error if the PostBackAction is not a valid one * and if the CSRF protection key in the form and sessions are different * or empty. * * The CSRF protection only protect logged-in user. * It assumes forms for anonymous users don't need this protection * * @param string $ErrorMessage Error message to display * @return boolean */ function IsValidFormPostBack($CsrfErrorMessage='ErrPostBackKeyInvalid') { if (is_array($this->ValidActions) && in_array($this->PostBackAction, $this->ValidActions)) { if ($this->Context->Session->UserID > 0) { // If this user has permissions in the system, make sure they aren't // being taken advantage of and that they actually posted the form. if ($this->FormPostBackKey != '' && $this->FormPostBackKey == $this->SessionPostBackKey) { // 2007-02-26 - mosullivan - These two lines caused a problem where people who opened // numerous discussions into windows and then started posting comments to them // received an error after their first comment was posted (Because the key had // changed since they loaded the discussion). By commenting this out, I am making // it so that the key is only set once per session, rather than every postback.
// Change the postback key for this user // $this->SessionPostBackKey = DefineVerificationKey(); // $this->Context->Session->SetVariable('SessionPostBackKey', $this->SessionPostBackKey); $this->PostBackParams->Set('FormPostBackKey', $this->SessionPostBackKey, 1, '', 1); return 1; } else if (!empty($CsrfErrorMessage)) { $this->Context->WarningCollector->Add($this->Context->GetDefinition($CsrfErrorMessage)); } } else { // No need to validate the postback if the user viewing has no permissions return 1; } } else { $this->Context->WarningCollector->Add($this->Context->GetDefinition('ErrPostBackActionInvalid')); } return 0; }