Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove migration utility; Modernize configuration for containerization; Use nonce to secure XSRF; And more! #263

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

bradkovach
Copy link
Contributor

@bradkovach bradkovach commented May 14, 2023

@psignoret I have done a lot of work to bring the plugin into a modern devops/containerizable state so that it can be deployed widely and rapidly via unattended installs.

Please enjoy the overview of changes, and let me know if you see anything that needs addressed urgently.

I bumped the version to 1.0.0 but this may be too eager for you. If it is, 0.8.0 would be the next breaking release.

Static configuration via defined constants now supported

This was brought on by a need for some better automation for some of my WP deployments. Specifically, we wanted the automated ability to configure WordPress/Azure AD secrets with Azure KeyVault. The plugin can now be installed and configured without logging in to WordPress. All configuration values can be provided by AADSSO_* constants. The options page displays the constant values in disabled controls, and tells the user that the setting is controlled by a constant.

image

Use as a multi-use plugin now detected and supported.

The plugin now supports and detects when it is being used in must-use mode. By using the wp-content/mu-plugins/ installation, no initial login is needed in order to activate and configure the plugin. Plugin activation AND configuration can take place without logging in. A loader script is required. This is provided in $/load-aad-sso-wordpress.php and can be set up with a copy or a symlink.

image

Escape-Hatch

A new constant, AADSSO_RESET_SETTINGS, is added to allow for a no-login reset of the database settings. Should you get locked out of your site (as I have done a few times), this should make it easier to regain access.

image

WordPress admin actions are all now nonced to stop request forgery.

To ensure that actions are not being taken without a user's permission, all volatile admin actions are now generated as nonced actions. This should protect against a lot of potential misuse.

image

Anti-Forgery Token now assembled with a configurable number of nonces

Instead of using a random GUID that has to be stored in PHP $_SESSION, the plugin now utilizes a configurable number of nonces (AADSSO_NONCE_PASSES) to generate and validate a login attempt. The default number of nonces is 3, but more or less can be added. The nonces are broken apart and verified cryptographically. Since no persistence beyond the WP session ID is needed to validate the nonces, any server in a properly-configured load-balanced scenario should be able to validate the nonces, since the session ID comes from WordPress's internals, not PHP's. If session is kept in redis/etc, these will work without needing session affinity tricks.

Many uses of $_SESSION remain, but these are less critical.

image

AADSSO_Settings class now uses a magic getter

Properties are dynamically resolved in the following order:

  1. Use a constant setting value
  2. Use a db setting value
  3. Use a default setting value
  4. null

Added a one-hour cache for the jwks response

Saw a TODO for this, so GitHub Copilot took care of it.

Lots of formatting, phpdoc and WordPress style changes

Used PHPCodeSniffer to lint and apply a lot of WordPress conventions where they were missing. Class files have been renamed, as have classes, to align with WordPress style convention of Upper_Snake_Case class names and class-lower-kebab-case.php file names.

JSON Settings Migration utility removed entirely.

JSON settings support and migration utility has been removed entirely. The functionality wasn't using WP_Filesystem, and is begging for a security vulnerability, so it is removed.

Miscellaneous

  • New HTML Helper class to help with constructing tags that are always escaped properly.
  • Version from 0.7.0 => 1.0.0
  • Removed GUID/UUID polyfill (no longer needed due to nonce antiforgery_id implementation)
  • Multi-sentence strings have been broken apart into single sentences.
  • Lots of //translator: comments added
  • Added js confirm(...) prompt to the reset settings button.
  • Little sexier construction of options page.

add support for mu-plugins use
rename plugin class files to align with WP code style
modify antiforgery_id to use wp nonces instead of a GUID/UUID
lots of WordPress code style fixes
rework settings class to use magic getter, and a resolution hierarchy constant, db, default, null
secure all admin actions by using nonces
add GUI to support mu-plugins/constants
fix lots of phpdoc
lots of new documentation
Make antiforgery_id security configurable by adding AADSSO_NONCE_PASSES constant
- default nonce passes = 4
Add filters to add safe casting for CONSTANT setting values. i.e. string 'false' => boolean false
fix many doc comments
bump version to 1; huge new feature set and many breaking changes
@bradkovach bradkovach changed the title Modernize configuration for no-gui config and other cleanup/maintenance Remove migration utility; Modernize configuration for containerization; Use nonce to secure XSRF; And more! May 15, 2023
@bradkovach bradkovach marked this pull request as ready for review May 15, 2023 20:49
@bradkovach
Copy link
Contributor Author

@psignoret hope you are well! please have a look at my PR, which includes a host of new features and security improvements.

@bradkovach
Copy link
Contributor Author

Found an issue where AADSSO_RESET_SETTINGS doesn't work unless you are logged in!

@bradkovach
Copy link
Contributor Author

@psignoret any feedback here? Would love to get this merged, and then start collaborating on wordpress plugin directory support.

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.

None yet

1 participant