Skip to content

Issue 12 - Security improvements#14

Merged
atheken merged 7 commits into
ActiveCampaign:masterfrom
guillaumemolter:issue-12
Jan 25, 2017
Merged

Issue 12 - Security improvements#14
atheken merged 7 commits into
ActiveCampaign:masterfrom
guillaumemolter:issue-12

Conversation

@guillaumemolter

Copy link
Copy Markdown
Contributor
  • using wp_nonce for ajax calls
  • validating data on the backend
  • escaping data on the frontend
  • checking for user priviledge

Closing #12

This should be done inside the function in case someone to call the
method directly.

The check is also already done for add_options_page.

Related to ActiveCampaign#12
@mgibbs189

Copy link
Copy Markdown
Contributor

Thanks @guillaumemolter

Any specific reason why the access check was moved outside of init?

E.g. df1e2d1 / 7c3dc8b

@guillaumemolter

guillaumemolter commented Aug 6, 2016

Copy link
Copy Markdown
Contributor Author

@mgibbs189

  1. Security wise it's generally recommended to check later than sooner.
  2. I felt it made more sense to have all the validation together
  3. Unless I'm missing something, the way it's currently implemented will only work when the method (ie: save_settings) is called via admin-ajax.php. If someone was to instanciate the class and call the method directly, no check would be done...

@atheken atheken merged commit 7c3dc8b into ActiveCampaign:master Jan 25, 2017
@atheken

atheken commented Jan 25, 2017

Copy link
Copy Markdown
Contributor

@guillaumemolter Thanks for this update. I had to do some tweaking due to the UI overhaul that landed in October, but the nonce and user permission checks are both still intact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants