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

Security refinements applied #207

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
81 changes: 49 additions & 32 deletions SettingsPage.php
Expand Up @@ -124,18 +124,18 @@ public function notify_json_migrate_status() {
if( isset( $_GET['aadsso_migrate_from_json_status'] ) ) {
if( 'success' === $_GET['aadsso_migrate_from_json_status'] ) {
echo '<div id="message" class="notice notice-success"><p>'
. __( 'Legacy settings have been migrated and the old configuration file has been deleted.', 'aad-sso-wordpress' )
. __('To finish migration, unset <code>AADSSO_SETTINGS_PATH</code> from <code>wp-config.php</code>. ', 'aad-sso-wordpress')
. esc_html__( 'Legacy settings have been migrated and the old configuration file has been deleted.', 'aad-sso-wordpress' )
. esc_html__('To finish migration, unset <code>AADSSO_SETTINGS_PATH</code> from <code>wp-config.php</code>. ', 'aad-sso-wordpress')
.'</p></div>';
} elseif ( 'manual' === $_GET['aadsso_migrate_from_json_status'] ) {
echo '<div id="message" class="notice notice-warning"><p>'
. esc_html__( 'Legacy settings have been migrated successfully. ', 'aad-sso-wordpress' )
. sprintf( __('To finish migration, delete the file at the path <code>%s</code>. ', 'aad-sso-wordpress'), AADSSO_SETTINGS_PATH )
. sprintf( __('Then, unset <code>AADSSO_SETTINGS_PATH</code> from <code>wp-config.php</code>. ', 'aad-sso-wordpress') )
. sprintf( esc_html__('To finish migration, delete the file at the path <code>%s</code>. ', 'aad-sso-wordpress'), esc_html( AADSSO_SETTINGS_PATH ) )
. sprintf( esc_html__('Then, unset <code>AADSSO_SETTINGS_PATH</code> from <code>wp-config.php</code>. ', 'aad-sso-wordpress') )
.'</p></div>';
} elseif( 'invalid_json' === $_GET['aadsso_migrate_from_json_status'] ) {
echo '<div id="message" class="notice notice-error"><p>'
. sprintf( __('Legacy settings could not be migrated from <code>%s</code>. ', 'aad-sso-wordpress'), AADSSO_SETTINGS_PATH )
. sprintf( esc_html__('Legacy settings could not be migrated from <code>%s</code>. ', 'aad-sso-wordpress'), esc_html( AADSSO_SETTINGS_PATH ) )
. esc_html( 'File could not be parsed as JSON. ', 'aad-sso-wordpress' )
. esc_html( 'Delete the file, or check its syntax.', 'aad-sso-wordpress' )
.'</p></div>';
Expand All @@ -150,7 +150,7 @@ public function notify_if_reset_successful()
{
if ( isset( $_GET['aadsso_reset'] ) && 'success' === $_GET['aadsso_reset'] ) {
echo '<div id="message" class="notice notice-warning"><p>'
. __( 'Single Sign-on with Azure Active Directory settings have been reset to default.',
. esc_html__( 'Single Sign-on with Azure Active Directory settings have been reset to default.',
'aad-sso-wordpress' )
.'</p></div>';
}
Expand Down Expand Up @@ -432,18 +432,23 @@ public function settings_advanced_info() { }
*/
function role_map_callback() {
printf( '<p>%s</p>',
__( 'Map WordPress roles to Azure Active Directory groups.', 'aad-sso-wordpress' )
esc_html__( 'Map WordPress roles to Azure Active Directory groups.', 'aad-sso-wordpress' )
);
echo '<table>';
printf(
'<thead><tr><th>%s</th><th>%s</th></tr></thead>',
__( 'WordPress Role', 'aad-sso-wordpress' ),
__( 'Azure AD Group Object ID', 'aad-sso-wordpress' )
esc_html__( 'WordPress Role', 'aad-sso-wordpress' ),
esc_html__( 'Azure AD Group Object ID', 'aad-sso-wordpress' )
);
echo '<tbody>';
foreach( $this->get_editable_roles( ) as $role_slug => $role ) {
echo '<tr>';
echo '<td>' . htmlentities( $role['name'] ) . '</td>';
/*echo '<td>' . htmlentities( $role['name'] ) . '</td>';*/
/**
* In WordPress coding standards Data Validation APIs are recommended to use
* rather than PHP generic functions.
*/
echo '<td>' . esc_html( $role['name'] ) . '</td>';
echo '<td>';
printf(
'<input type="text" class="regular-text" name="aadsso_settings[role_map][%1$s]" '
Expand All @@ -467,7 +472,7 @@ public function org_display_name_callback() {
$this->render_text_field( 'org_display_name' );
printf(
'<p class="description">%s</p>',
__( 'Display Name will be shown on the WordPress login screen.', 'aad-sso-wordpress' )
esc_html__( 'Display Name will be shown on the WordPress login screen.', 'aad-sso-wordpress' )
);
}

Expand All @@ -478,7 +483,7 @@ public function org_domain_hint_callback() {
$this->render_text_field( 'org_domain_hint' );
printf(
'<p class="description">%s</p>',
__( 'Provides a hint to Azure AD about the domain or tenant they will be logging in to. If '
esc_html__( 'Provides a hint to Azure AD about the domain or tenant they will be logging in to. If '
. 'the domain is federated, the user will be automatically redirected to federation '
. 'endpoint.', 'aad-sso-wordpress' )
);
Expand All @@ -491,7 +496,7 @@ public function client_id_callback() {
$this->render_text_field( 'client_id' );
printf(
'<p class="description">%s</p>',
__( 'The client ID of the Azure AD application representing this blog.', 'aad-sso-wordpress' )
esc_html__( 'The client ID of the Azure AD application representing this blog.', 'aad-sso-wordpress' )
);
}

Expand All @@ -502,7 +507,7 @@ public function client_secret_callback() {
$this->render_text_field( 'client_secret' );
printf(
'<p class="description">%s</p>',
__( 'A secret key for the Azure AD application representing this blog.', 'aad-sso-wordpress' )
esc_html__( 'A secret key for the Azure AD application representing this blog.', 'aad-sso-wordpress' )
);
}

Expand All @@ -514,9 +519,13 @@ public function redirect_uri_callback() {
printf(
' <a href="#" onclick="jQuery(\'#redirect_uri\').val(\'%s\'); return false;">%s</a>'
. '<p class="description">%s</p>',
wp_login_url(),
__( 'Set default', 'aad-sso-wordpress' ),
__( 'The URL where the user is redirected to after authenticating with Azure AD. '
/**
* https://codex.wordpress.org/Data_Validation#Text_Nodes
* Always use esc_url when sanitizing URLs
*/
esc_url( wp_login_url() ),
esc_html__( 'Set default', 'aad-sso-wordpress' ),
esc_html__( 'The URL where the user is redirected to after authenticating with Azure AD. '
. 'This URL must be registered in Azure AD as a valid redirect URL, and it must be a '
. 'page that invokes the "authenticate" filter. If you don\'t know what to set, leave '
. 'the default value (which is this blog\'s login page).', 'aad-sso-wordpress' )
Expand All @@ -531,9 +540,9 @@ public function logout_redirect_uri_callback() {
printf(
' <a href="#" onclick="jQuery(\'#logout_redirect_uri\').val(\'%s\'); return false;">%s</a>'
. '<p class="description">%s</p>',
wp_login_url(),
__( 'Set default', 'aad-sso-wordpress'),
__( 'The URL where the user is redirected to after signing out of Azure AD. '
esc_url( wp_login_url() ),
esc_html__( 'Set default', 'aad-sso-wordpress'),
esc_html__( 'The URL where the user is redirected to after signing out of Azure AD. '
. 'This URL must be registered in Azure AD as a valid redirect URL. (This does not affect '
. ' logging out of the blog, it is only used when logging out of Azure AD.)', 'aad-sso-wordpress' )
);
Expand All @@ -550,16 +559,16 @@ public function field_to_match_to_upn_callback() {
?>
<select name="aadsso_settings[field_to_match_to_upn]" id="field_to_match_to_upn">
<option value="email"<?php echo $selected == 'email' ? ' selected="selected"' : ''; ?>>
<?php echo __( 'Email Address', 'aad-sso-wordpress' ); ?>
<?php echo esc_html__( 'Email Address', 'aad-sso-wordpress' ); ?>
</option>
<option value="login"<?php echo $selected == 'login' ? ' selected="selected"' : ''; ?>>
<?php echo __( 'Login Name', 'aad-sso-wordpress' ); ?>
<?php echo esc_html__( 'Login Name', 'aad-sso-wordpress' ); ?>
</option>
</select>
<?php
printf(
'<p class="description">%s</p>',
__( 'This specifies the WordPress user field which will be used to match to the Azure AD user\'s '
esc_html__( 'This specifies the WordPress user field which will be used to match to the Azure AD user\'s '
. 'UserPrincipalName.', 'aad-sso-wordpress' )
);
}
Expand All @@ -570,7 +579,7 @@ public function field_to_match_to_upn_callback() {
public function match_on_upn_alias_callback() {
$this->render_checkbox_field(
'match_on_upn_alias',
__( 'Match WordPress users based on the alias of their Azure AD UserPrincipalName. For example, '
esc_html__( 'Match WordPress users based on the alias of their Azure AD UserPrincipalName. For example, '
. 'Azure AD username <code>bob@example.com</code> will match WordPress user <code>bob</code>.',
'aad-sso-wordpress' )
);
Expand All @@ -590,15 +599,19 @@ public function default_wp_role_callback() {
printf( '<option value="%s">%s</option>', '', '(None, deny access)' );
foreach( $this->get_editable_roles() as $role_slug => $role ) {
$selected = $this->settings['default_wp_role'] === $role_slug ? ' selected="selected"' : '';
/**
* Since $elected can return both the attribute with value as well as an empty string
* no escaping has been used.
*/
printf(
'<option value="%s"%s>%s</option>',
esc_attr( $role_slug ), $selected, htmlentities( $role['name'] )
esc_attr( $role_slug ), $selected, esc_html( $role['name'] )
);
}
echo '</select>';
printf(
'<p class="description">%s</p>',
__('This is the default role that users will be assigned to if matching Azure AD group to '
esc_html__('This is the default role that users will be assigned to if matching Azure AD group to '
. 'WordPress roles is enabled, but the signed in user isn\'t a member of any of the '
. 'configured Azure AD groups.', 'aad-sso-wordpress')
);
Expand Down Expand Up @@ -645,9 +658,13 @@ public function openid_configuration_endpoint_callback() {
printf(
' <a href="#" onclick="jQuery(\'#openid_configuration_endpoint\').val(\'%s\'); return false;">%s</a>'
. '<p class="description">%s</p>',
/**
* get_defaults can return either an array, a string or null
* no escaping has been used
*/
AADSSO_Settings::get_defaults( 'openid_configuration_endpoint' ),
__( 'Set default', 'aad-sso-wordpress'),
__( 'The OpenID Connect configuration endpoint to use. To support Microsoft Accounts and external '
esc_html__( 'Set default', 'aad-sso-wordpress'),
esc_html__( 'The OpenID Connect configuration endpoint to use. To support Microsoft Accounts and external '
. 'users (users invited in from other Azure AD directories, known sometimes as "B2B users") you '
. 'must use: <code>https://login.microsoftonline.com/{tenant-id}/.well-known/openid-configuration</code>, '
. 'where <code>{tenant-id}</code> is the tenant ID or a verified domain name of your directory.',
Expand All @@ -661,7 +678,7 @@ public function openid_configuration_endpoint_callback() {
public function enable_full_logout_callback() {
$this->render_checkbox_field(
'enable_full_logout',
__( 'Do a full logout of Azure AD when logging out of WordPress.',
esc_html__( 'Do a full logout of Azure AD when logging out of WordPress.',
'aad-sso-wordpress' )
);
}
Expand All @@ -676,7 +693,7 @@ public function render_text_field( $name ) {
printf(
'<input class="regular-text" type="text" '
. 'name="aadsso_settings[%1$s]" id="%1$s" value="%2$s" />',
$name, $value
esc_html( $name ), esc_html( $value )
);
}

Expand All @@ -690,9 +707,9 @@ public function render_checkbox_field( $name, $label ) {
printf(
'<input type="checkbox" name="aadsso_settings[%1$s]" id="%1$s" value="%1$s"%2$s />'
. '<label for="%1$s">%3$s</label>',
$name,
esc_html( $name ),
isset( $this->settings[ $name ] ) && $this->settings[ $name ] ? 'checked' : '',
$label
esc_html( $label )
);
}

Expand Down
15 changes: 8 additions & 7 deletions aad-sso-wordpress.php
Expand Up @@ -615,13 +615,14 @@ public function setup_admin_settings() {
*/
function print_plugin_not_configured() {
echo '<div id="message" class="error"><p>'
. __( 'Single Sign-on with Azure Active Directory required settings are not defined. '
. esc_html__( 'Single Sign-on with Azure Active Directory required settings are not defined. '
. 'Update them under Settings > Azure AD.', 'aad-sso-wordpress' )
.'</p></div>';
}

/**
* Renders some debugging data.
* var_export returns mixed output hence no escaping is used.
*/
function print_debug() {
echo '<p>SESSION</p><pre>' . var_export( $_SESSION, TRUE ) . '</pre>';
Expand All @@ -643,14 +644,14 @@ function print_login_css() {
function print_login_link() {
$html = '<p class="aadsso-login-form-text">';
$html .= '<a href="%s">';
$html .= sprintf( __( 'Sign in with your %s account', 'aad-sso-wordpress' ),
htmlentities( $this->settings->org_display_name ) );
$html .= sprintf( esc_html__( 'Sign in with your %s account', 'aad-sso-wordpress' ),
esc_html( $this->settings->org_display_name ) );
$html .= '</a><br /><a class="dim" href="%s">'
. __( 'Sign out', 'aad-sso-wordpress' ) . '</a></p>';
. esc_html__( 'Sign out', 'aad-sso-wordpress' ) . '</a></p>';
printf(
$html,
$this->get_login_url(),
$this->get_logout_url()
wp_kses_post( $html ),
esc_url( $this->get_login_url() ),
esc_url( $this->get_logout_url() )
);
}

Expand Down
8 changes: 4 additions & 4 deletions view/settings.php
Expand Up @@ -17,11 +17,11 @@
<?php
printf(
'<a href="%s" class="button">%s</a> <span class="description">%s</span>',
wp_nonce_url(
esc_url( wp_nonce_url(
admin_url( 'options-general.php?page=aadsso_settings' ),
'aadsso_reset_settings',
'aadsso_nonce'
),
) ),
esc_html__( 'Reset Settings' , 'aad-sso-wordpress' ),
esc_html__( 'Reset the plugin to default settings. Careful, there is no undo for this.' , 'aad-sso-wordpress' )
)
Expand Down Expand Up @@ -55,11 +55,11 @@
<p><?php
printf(
'<a href="%s" class="button">%s</a> <span class="description">%s</span>',
wp_nonce_url(
esc_url( wp_nonce_url(
admin_url( 'options-general.php?page=aadsso_settings' ),
'aadsso_migrate_from_json',
'aadsso_nonce'
),
) ),
esc_html__( 'Migrate Settings' , 'aad-sso-wordpress' ),
esc_html__( 'Migrate settings from old plugin versions to new configuration. This will overwrite existing settings! Careful, there is no undo for this.' , 'aad-sso-wordpress' )
)
Expand Down