Add env settings support #488

joshcanhelp · 2018-06-19T22:34:43Z

Add support for defining settings in a constant. Closes #480.

This will allow for sensitive data like the client secret, API token, and migration token to be stored in constants. The options class now looks for constants of a specific name being defined, like so:

// Storing the client secret in a constant.
// Add to wp-config.php or anywhere before WP_Auth0_Options_Generic is instantiated.
// Constant name is "AUTH0_ENV_" + option key in caps.
define( 'AUTH0_ENV_CLIENT_SECRET', 'YOUR_CLIENT_SECRET_HERE' );

The constant name should be AUTH0_ENV_ followed by the option name to override in all caps. Option keys listed here can all be overridden.

Note: the migration_token value is generated by the plugin when user migration is turned on. If there is already a value in the admin, make sure to set the constant to the same value. If that value needs to change, it also must be changed in the user migration script for the database connection being used.

Once that is set, WP_Auth0_Options_Generic::get_options() will look for values before loading all options. WP_Auth0_Options_Generic::get() will pull from this adjusted array.

The settings field will change display based on this new value and show the constant being used:

Saving the settings page after setting a constant value will validate the constant-set values and delete them from the options array being saved to the database (important to mention that in upcoming docs).

joshcanhelp · 2018-06-19T22:35:08Z

lib/WP_Auth0_Options.php

@@ -23,14 +37,6 @@ public function set_connection( $key, $value ) {
 $this->set( 'connections', $options['connections'] );
 }

- public function get_connection( $key, $default = null ) {


Moved to parent, WP_Auth0_Options_Generic

joshcanhelp · 2018-06-20T15:24:58Z

bin/install-wp-tests.sh

@@ -0,0 +1,152 @@
+#!/usr/bin/env bash


All boilerplate from https://developer.wordpress.org/cli/commands/scaffold/plugin-tests/

joshcanhelp · 2018-06-20T15:25:50Z

tests/bootstrap.php

@@ -0,0 +1,33 @@
+<?php


Boilerplate from https://developer.wordpress.org/cli/commands/scaffold/plugin-tests/

joshcanhelp · 2018-06-20T15:26:59Z

lib/admin/WP_Auth0_Admin.php

 foreach ( $this->sections as $name => $section ) {
- $input = $section->input_validator( $input, $old_options );


Handles $old_options the same internally.

joshcanhelp · 2018-06-20T15:27:10Z

lib/admin/WP_Auth0_Admin.php

-
- $old_options = $this->a0_options->get_options();
-
- $input['connections'] = $old_options['connections'];


This was stopping certain options from being saved.

joshcanhelp · 2018-06-20T15:29:13Z

lib/WP_Auth0_Options_Generic.php


- if ( $should_update ) {
- update_option( $this->_options_name, $options );
+ if ( null === $this->get_constant_val( $key ) ) {


Do not save if we have a constant value

why not do the opposite check and return faster?

if {! defined as constant){ return; } //continue

avoids shifting all the remaining code to the right and IMO helps readability

joshcanhelp · 2018-06-20T15:29:53Z

lib/WP_Auth0_Options_Generic.php

+ *
+ * @link https://auth0.com/docs/cms/wordpress/extending#wp_auth0_get_option
+ */
+ public function get_connection( $key, $default = null ) {


Moved from lib/WP_Auth0_Options.php and simplified

joshcanhelp · 2018-06-25T16:05:41Z

phpunit.xml.dist

@@ -0,0 +1,15 @@
+<?xml version="1.0"?>


Boilerplate from https://developer.wordpress.org/cli/commands/scaffold/plugin-tests/

lbalmaceda

As a general comment, it's missing a small doc (in readme why not? or https://auth0.com/docs/cms/wordpress ) about how to set up the plugin using constants. I know you have a line on the wp-dashboard for each overridable input, but might be good to see a mention of the feature somewhere else.

lbalmaceda · 2018-06-25T22:09:35Z

.gitignore

@@ -8,3 +8,4 @@ wp-phptidy.php
 /account_cleanup/vendor/
 /vendor/
 /rules/
+composer.lock


are you sure this file should be ignored? AFAIK, lock files are defining specific dependencies' versions this project runs fine with and should always be committed.

It can go either way but, in thinking about it, we test this thoroughly with a specific combination and this is closer to an application than a library ... probably best to keep. Will add back!

lbalmaceda · 2018-06-25T22:20:52Z

lib/WP_Auth0_Options_Generic.php

+ foreach ( $option_keys as $key ) {
+ $const_name = $this->get_constant_name( $key );
+ if ( defined( $const_name ) ) {
+ $this->constant_opts[$key] = constant( $const_name );


why do you need to call constant ( {value} ) and not use it directly?

Because I'm using a dynamic constant name. If I did $this->constant_opts[$key] = $const_name then the value would be the constant name.

lbalmaceda · 2018-06-25T22:24:23Z

lib/WP_Auth0_Options_Generic.php

+ // Check for constant overrides and replace.
+ if ( ! empty( $this->constant_opts ) ) {
+ $options = array_merge( $options, $this->constant_opts );
+ }

 $this->_opt = $options;


once this is set I guess the user can't override a constant value on runtime, can they?

Nope, compile-time only. That's also way I bail on the set method if there is a constant already present.

lbalmaceda · 2018-06-25T22:32:09Z

lib/WP_Auth0_Options_Generic.php

+ * @return array
+ */
+ public function get_all_constant_keys() {
+ return ! empty( $this->constant_opts ) ? array_keys( $this->constant_opts ) : array();


$constant_opts is already an empty array (default value at the top of this class). Calling array_keys passing an empty array returns an empty array. No need the IF here. Should be: array_keys( $this->constant_opts ) .

lbalmaceda · 2018-06-25T22:33:33Z

lib/WP_Auth0_Options_Generic.php


- if ( $should_update ) {
- update_option( $this->_options_name, $options );
+ if ( null === $this->get_constant_val( $key ) ) {


why not do the opposite check and return faster?

if {! defined as constant){ return; } //continue

avoids shifting all the remaining code to the right and IMO helps readability

lbalmaceda · 2018-06-25T23:10:43Z

lib/admin/WP_Auth0_Admin_Generic.php

 esc_attr( $this->_option_name ),
 esc_attr( $input_name ),
 esc_attr( $id ),
 $this->_textarea_rows,
+ $field_is_const ? ' disabled' : '',


lbalmaceda · 2018-06-25T23:10:51Z

lib/admin/WP_Auth0_Admin_Generic.php

 esc_attr( $id ),
 esc_attr( $this->_option_name ),
 esc_attr( $input_name ),
 esc_attr( $id ),
 esc_attr( $value ),
 checked( $selected, true, false ),
+ $disabled ? ' disabled' : '',


lbalmaceda · 2018-06-25T23:14:03Z

tests/testWPAuth0Options.php

+ public function testThatConstructorStoresConstants()
+ {
+ // Set a few constant overrides.
+ define( 'AUTH0_ENV_DOMAIN', rand() );


While this is a test, rand() is setting an invalid value for this constant. I thought these were being validated, or is that run only after the user changes something in the wp dashboard?

Just validated to be not empty at this point. Definitely should have better validation (I have a task already)

YEAH. (Sorry caps lock on). In fact, tests values should look (and be validated!) like the actual expected values. No rush on this, but good to take note 👍

lbalmaceda · 2018-06-25T23:17:02Z

lib/WP_Auth0_Options_Generic.php

+ *
+ * @return string|null
+ */
+ public function get_constant_val( $key ) {


I think this be transparent. What about making this method private and having a single "source" of values? The call to get ($key) would return either the constant defined value or the one from options. Found coming back from the tests that were asserting that both get and get_constant_val returned the same value. https://github.com/auth0/wp-auth0/pull/488/files#diff-25c8182a46a822c28131501910ea29faR73

get_constant_val() is used in another class. I also don't want to pollute getter methods with that logic, I'd like to keep it in get_options() (there are 2 getters based on the settings type, both use the override)

lbalmaceda · 2018-06-25T23:19:06Z

tests/testWPAuth0Options.php

+ /**
+ * ≤
+ */
+ public function testThatFiltersOverrideValues () {


is wp_auth0_get_option a random name you decided to use for this test or why is it hardcoded?
I don't follow what's happening in this test. Can you please explain it ?

wp_auth0_get_option is a filter, hard-coded machine name. Checks that this happens:

https://auth0.com/docs/cms/wordpress/extending#wp_auth0_get_option

joshcanhelp · 2018-06-25T23:32:56Z

@lbalmaceda - RE: the documentation ... it's not there because the feature is not active until this PR is merged and the plugin is released :)

lbalmaceda

LG 👨‍🎓

Pending bug resolution. Discussing internally.

joshcanhelp · 2018-06-26T23:07:35Z

lib/WP_Auth0_Options.php

@@ -16,21 +16,6 @@ public function is_wp_registration_enabled() {
 return is_multisite() ? users_can_register_signup_filter() : get_site_option( 'users_can_register' );
 }

- public function set_connection( $key, $value ) {


Moved to super class WP_Auth0_Options_Generic

joshcanhelp · 2018-06-26T23:07:39Z

lib/WP_Auth0_Options.php

- $this->set( 'connections', $options['connections'] );
- }
-
- public function get_connection( $key, $default = null ) {


Moved to super class WP_Auth0_Options_Generic

joshcanhelp · 2018-06-26T23:11:16Z

assets/js/lock-init.js

@@ -26,7 +26,7 @@ jQuery(document).ready(function ($) {
 }

 // Look for additional fields to display
- if ( wpAuth0LockGlobalFields ) {
+ if ( typeof wpAuth0LockGlobalFields !== 'undefined' ) {


Sneaking this in ... found a JS error from #491

joshcanhelp · 2018-07-31T19:54:00Z

Re-done in #509

joshcanhelp added the CH: Added label Jun 19, 2018

joshcanhelp commented Jun 19, 2018

View reviewed changes

joshcanhelp force-pushed the add-env-settings-support branch 2 times, most recently from b80735d to 077bb49 Compare June 19, 2018 23:52

joshcanhelp commented Jun 20, 2018

View reviewed changes

joshcanhelp added the Ready for Review label Jun 20, 2018

joshcanhelp added this to the v3-Next milestone Jun 20, 2018

joshcanhelp added CH: Security and removed CH: Added labels Jun 20, 2018

joshcanhelp commented Jun 25, 2018

View reviewed changes

lbalmaceda reviewed Jun 25, 2018

View reviewed changes

joshcanhelp added 5 commits June 26, 2018 11:48

Add the ability for settings to be stored as constants.

edf6e66

Add support for radio buttons; better comments

5c1d8b7

add tests

8b54146

removing composer.lock

46e021b

review feedback

cee40f7

lbalmaceda previously approved these changes Jun 26, 2018

View reviewed changes

joshcanhelp force-pushed the add-env-settings-support branch from 57a9c1f to 27ffbe1 Compare June 26, 2018 23:07

joshcanhelp commented Jun 26, 2018

View reviewed changes

joshcanhelp removed the Ready for Review label Jun 27, 2018

joshcanhelp removed this from the v3-Next milestone Jun 27, 2018

joshcanhelp mentioned this pull request Jun 27, 2018

Optionally load client ID, secret and domain from environment #480

Closed

fix problem with connection settings and saving to DB; more tests

90da155

joshcanhelp force-pushed the add-env-settings-support branch from 27ffbe1 to 90da155 Compare June 27, 2018 15:03

joshcanhelp added the work in progress label Jun 27, 2018

joshcanhelp removed CH: Security labels Jul 31, 2018

joshcanhelp closed this Jul 31, 2018

joshcanhelp deleted the add-env-settings-support branch July 31, 2018 19:54

joshcanhelp mentioned this pull request Jul 31, 2018

Add constant settings support #509

Merged

github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add env settings support #488

Add env settings support #488

joshcanhelp commented Jun 19, 2018 •

edited

Loading

joshcanhelp Jun 19, 2018

joshcanhelp Jun 20, 2018

joshcanhelp Jun 20, 2018

joshcanhelp Jun 20, 2018

joshcanhelp Jun 20, 2018

joshcanhelp Jun 20, 2018

lbalmaceda Jun 25, 2018

joshcanhelp Jun 20, 2018

joshcanhelp Jun 25, 2018

lbalmaceda left a comment

lbalmaceda Jun 25, 2018

joshcanhelp Jun 25, 2018

lbalmaceda Jun 25, 2018

joshcanhelp Jun 25, 2018 •

edited

Loading

lbalmaceda Jun 25, 2018

joshcanhelp Jun 25, 2018 •

edited

Loading

lbalmaceda Jun 25, 2018

lbalmaceda Jun 25, 2018

lbalmaceda Jun 25, 2018

lbalmaceda Jun 25, 2018

lbalmaceda Jun 25, 2018

joshcanhelp Jun 25, 2018

lbalmaceda Jun 26, 2018

lbalmaceda Jun 25, 2018

joshcanhelp Jun 25, 2018

lbalmaceda Jun 25, 2018

joshcanhelp Jun 26, 2018

joshcanhelp commented Jun 25, 2018

lbalmaceda left a comment

joshcanhelp Jun 26, 2018

joshcanhelp Jun 26, 2018

joshcanhelp Jun 26, 2018

joshcanhelp commented Jul 31, 2018

		foreach ( $this->sections as $name => $section ) {
		$input = $section->input_validator( $input, $old_options );


		$old_options = $this->a0_options->get_options();

		$input['connections'] = $old_options['connections'];

Add env settings support #488

Add env settings support #488

Conversation

joshcanhelp commented Jun 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbalmaceda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshcanhelp Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshcanhelp Jun 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshcanhelp commented Jun 25, 2018

lbalmaceda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshcanhelp commented Jul 31, 2018

joshcanhelp commented Jun 19, 2018 •

edited

Loading

joshcanhelp Jun 25, 2018 •

edited

Loading

joshcanhelp Jun 25, 2018 •

edited

Loading