How do I validate user input in the ACP?

[austin881]

Can someone point me to some documentation or forum topics on validating input boxes server-side in my extension?

I thought I could just do something like this...

Code: Select all

static public function getSubscribedEvents()
    {
        return [
            'core.validate_config_variable'  => 'validate_config_variable'
        ];
    }

Code: Select all

    public function validate_config_variable($event)
    {
        $input = $event['cfg_array']['s3_aws_access_key_id'];

        // Check if the validate test is for s3.
          if (($event['config_definition']['validate'] == 's3_aws_access_key_id') && ($input !== '')) {
            // Store the error and input event data.
            $error = $event['error'];

            // Add error message if the input is not a valid AWS Access Key Id.
              if (!preg_match('/[A-Z0-9]{20})/', $input)) {
                $error[] = $this->user->lang('ACP_S3_AWS_ACCESS_KEY_ID_INVALID', $input);
              }

            // Update error event data.
            $event['error'] = $error;
          }
    }

What am I missing? Upon clicking Submit button the success page is displayed, the validation doesn't seem to run at all. I put debug prints error_log() and die() in the validate_config_variable() function and they are never called/printed.

If you'd like to see the rest of the code, it is located here: https://github.com/AustinMaddox/phpbb-extension-s3

[RMcGirr83]

You can't use that within an acp module file. You have to do it within the actual file prior to the submit so something link this.

Code: Select all

        add_form_key('AustinMaddox/s3');
		if (!preg_match('/[A-Z0-9]{20})/', $input)) {
			$error[] = $this->user->lang('ACP_S3_AWS_ACCESS_KEY_ID_INVALID', $input);
		}
        if ($request->is_set_post('submit') && ! sizeof($error))

that event is only for validating entries that have been added to the defautl acp modules.

[austin881]

Thanks Rich. You said...

You can't use that within an acp module file.

I didn't think I was. I was using it in an event listener.

You have to do it within the actual file prior to the submit so something link this.

Do it within what file?

that event is only for validating entries that have been added to the defautl acp modules

That makes sense. I used the Google Analytics module as inspiration which puts a field in the Board settings page of the ACP. So if I can't use the core.validate_config_variable event, what can I use and which file does it go in? Does that code you suggested go in extname/acp/main_module.php?

[RMcGirr83]

Thanks Rich. You said...

You can't use that within an acp module file.

I didn't think I was. I was using it in an event listener.

But you aren't using event listeners for your acp stuffs...you are using an acp module. That's the difference.

Does that code you suggested go in extname/acp/main_module.php?

[austin881]

Ok that helps. Now we're getting somewhere!

I've put this code in ext/AustinMaddox/s3/acp/main_module.php

Code: Select all

if (!preg_match('/[A-Z0-9]{20}/', $request->variable('s3_aws_access_key_id', ''))) {
    trigger_error($user->lang('ACP_S3_AWS_ACCESS_KEY_ID_INVALID', $request->variable('s3_aws_access_key_id', '')) . adm_back_link($this->u_action));
}

if (!preg_match('/[A-Za-z0-9/+=]{40}/', $request->variable('s3_aws_secret_access_key', ''))) {
    trigger_error($user->lang('ACP_S3_AWS_SECRET_ACCESS_KEY_INVALID', $request->variable('s3_aws_secret_access_key', '')) . adm_back_link($this->u_action));
}

Thanks again Rich!

[RMcGirr83]

A green error?

Code: Select all

trigger_error($user->lang('ACP_S3_AWS_SECRET_ACCESS_KEY_INVALID', $request->variable('s3_aws_secret_access_key', '')) . adm_back_link($this->u_action), E_USER_WARNING);

[3Di]

On a side note, you should code an array() of errors, so to provide the exact error to the end user.

The error message(s) should be "user-friendly", the average Jon Doe admin doesn't have a clue what is a regular expression.

[austin881]

Good advice! You mean so they can see all errors? Something like this...

Code: Select all

$errors = [];
if (!preg_match('/[A-Z0-9]{20}/', $request->variable('s3_aws_access_key_id', ''))) {
    $errors[] = $user->lang('ACP_S3_AWS_ACCESS_KEY_ID_INVALID', $request->variable('s3_aws_access_key_id', ''));
}

if (!preg_match('/[A-Za-z0-9\/+=]{40}/', $request->variable('s3_aws_secret_access_key', ''))) {
    $errors[] = $user->lang('ACP_S3_AWS_SECRET_ACCESS_KEY_INVALID', $request->variable('s3_aws_secret_access_key', ''));
}

if ($errors) {
    trigger_error(implode('<br><br>', $errors) . adm_back_link($this->u_action), E_USER_WARNING);
}

[3Di]

You got it, the idea. Something like this, not tested and written on the fly..

Your acp module

Code: Select all

class my_ext_acp_module
{
	var $u_action;

	// .....

	function main($id, $mode)
	{
		$this->tpl_name = 'acp_template.html';

		// ......
		//......

		$errors = [];
		if (!preg_match('/[A-Z0-9]{20}/', $request->variable('s3_aws_access_key_id', '')))
		{
			$errors[] = $user->lang('ACP_S3_AWS_ACCESS_KEY_ID_INVALID';
		}
		if (!preg_match('/[A-Za-z0-9\/+=]{40}/', $request->variable('s3_aws_secret_access_key', '')))
		{
			$errors[] = $user->lang('ACP_S3_AWS_SECRET_ACCESS_KEY_INVALID';
		}

		// .....

		$template->assign_vars(array(
			'S_ERRORS'		=> (sizeof($errors)) ? true : false,
			'ERRORS_MSG'		=> implode('<br />', $errors),
			'U_ACTION'		=> $this->u_action
			)
		);
	}
}

in your /adm/style/acp_template.html file

right before..

Code: Select all

<form id="main" method="post" action="{U_ACTION}// ... etc..

add this snippet

Code: Select all

<!-- IF S_ERRORS -->
	<div class="errorbox">
		<h3>{L_WARNING}</h3>
		<p>{ERRORS_MSG}</p>
	</div>
<!-- ENDIF -->

If you are using 3.2.x only, simply replace the $user objet with the $language one (for langs), your choice.

The *_CPs are decoupled from the framework, still. Old coding method rules, so to speak.

[austin881]

So I'm just curious... Why is your method preferred over what I did? Don't they both achieve the same result?

Also, sorry but I didn't understand your last two sentences at all.

[3Di]

Why is your method preferred over what I did? Don't they both achieve the same result?

Because of code consistency, and because the template stuff uses CSS classes as well.
Let's say also, some styles writer coded a different CSS rule for the class "errorbox"?

I didn't understand your last two sentences at all.

About the User obj against the Language one?
phpBB 3.2 introduced the language obj, you can use
$language->lang.... (only for 3.2)
instead of
$user->lang,,, (3.1 and 3.2)

For the latter, I mean all of the *_CPs (ACP, MCP. UCP) need a re-factoring, use the methods used for 3.0.x more or less. More about that on area51.

Oh, forgot to add, before of your checks in PHP you should check also for this..

Code: Select all

// ......
		if (sizeof($errors))
		{
			$submit = false;
		}
//......

Edit: this method also (if I am not mistaken) will keep the admin on the same page but with the error message(s) on top of it. Yours, instead, tosses the error message(s) like a die(); , the admin needs to click on the link to get back to that page, which is not so elegant IMO. . A Latinism, appropriated, could be "Coitus interruptus".

[austin881]

$submit = false; ??????????
What would that do? That variable doesn't even exist.

[3Di]

Man, I told you I am writing on the fly?

Code: Select all

$submit = $request->is_set_post('submit');

if (sizeof($errors))
{
	$submit = false;
}

or..

Code: Select all

if ($request->is_set_post('submit') && !sizeof($errors))
{
// do your magic..
}

Have a look at some validated extensions and read as much as you can, Docs and everything.

[3Di]

I spent some minute after your github. I meant some thing like this, adjust the template accordingly, NOT tested, on the fly.

Code: Select all

namespace AustinMaddox\s3\acp;

class main_module
{
var $u_action;

function main($id, $mode)
{
	global $config, $request, $template, $user;

	$user->add_lang('acp/common');

	$this->tpl_name = 's3_body';
	$this->page_title = $user->lang('ACP_S3_TITLE');

	add_form_key('AustinMaddox/s3');

	if ($request->is_set_post('submit'))
	{
		if (!check_form_key('AustinMaddox/s3'))
		{
			trigger_error('FORM_INVALID');
		}

		$errors = [];

		if (!preg_match('/[A-Z0-9]{20}/', $request->variable('s3_aws_access_key_id', '')))
		{
			$errors[] = $user->lang('ACP_S3_AWS_ACCESS_KEY_ID_INVALID');
		}
		if (!preg_match('/[A-Za-z0-9\/+=]{40}/', $request->variable('s3_aws_secret_access_key', '')))
		{
			$errors[] = $user->lang('ACP_S3_AWS_SECRET_ACCESS_KEY_INVALID');
		}

		if (!sizeof($errors))
		{
			$config->set('s3_aws_access_key_id', $request->variable('s3_aws_access_key_id', ''));
			$config->set('s3_aws_secret_access_key', $request->variable('s3_aws_secret_access_key', ''));
			$config->set('s3_region', $request->variable('s3_region', ''));
			$config->set('s3_bucket', $request->variable('s3_bucket', ''));

			trigger_error($user->lang('ACP_S3_SETTING_SAVED') . adm_back_link($this->u_action));
		}
	}

	$template->assign_vars([
		'S_ERRORS'			=> (sizeof($errors)) ? true : false,
		'ERRORS_MSG'			=> implode('<br />', $errors),
		'U_ACTION'			=> $this->u_action,
		'S3_AWS_ACCESS_KEY_ID'		=> $config['s3_aws_access_key_id'],
		'S3_AWS_SECRET_ACCESS_KEY'	=> $config['s3_aws_secret_access_key'],
		'S3_REGION'			=> $config['s3_region'],
		'S3_BUCKET'			=> $config['s3_bucket'],
		]);
	}
}