Secure PHP script

Secure PHP script

gordonc200gordonc200 Posts: 39Questions: 7Answers: 0

I have Editor running with PHP connecting to a MySQL database. The PHP script used by the Ajax script is accessible to anyone who can work out the script location (not difficult) and so I understand I need to secure the script using a validation token.

I know how to generate a token but my thinking was to simply wrap the code in the table php in an "if" statement that checks for the token and if it isn't correct to simply not run the PHP script.

So step one is to test this theory. I created a variable at the top of the script and gave it the value 1.

$test="1";

and then wrap the code in an if statement

if ($test ="1") {
//datatables table code here;
//do nothing and let the script return a json error;
}

If I could get this to work I'd replace "1" with the token value.

But this doesn't work.

Is there any way it can be or am I barking up the wrong tree here?

Thanks anyone who can help.

This question has accepted answers - jump to:

Answers

  • allanallan Posts: 61,822Questions: 1Answers: 10,127 Site admin

    Does your code make use of sessions? If so you could add something like:

    if ( ! isset($_SESSION['userId']) ) {
      // redirect to login page, then:
      exit(0);
    }
    

    Allan

  • gordonc200gordonc200 Posts: 39Questions: 7Answers: 0

    I'm using the CMS Concrete5 and it seems it does going by this

    https://documentation.concrete5.org/tutorials/session-handling-in-concrete5-5-7

    The code is a bit different though.

    In my case the table I'm using is called "drawings" so the script is table.drawings.php

    If I wrap the code in this in the if statement and the if statement is true it still returns no JSON code however. So my problem is more fundamental. From what you are saying I should be possible though?

  • allanallan Posts: 61,822Questions: 1Answers: 10,127 Site admin

    Thanks for the link! The thing that I'm not quite getting from their documentation is how to tell if a user is logged in or not. Do they provide that functionality, or is that something you have to write on top of their APIs?

    That's going to be the key thing to figuring out what the correct code to use here is.

    But in short, yes this will most certainly be possible.

    Allan

  • gordonc200gordonc200 Posts: 39Questions: 7Answers: 0

    The way I have my site setup users always have to be logged on. Concrete5 has a very comprehensive permissions model whereby you can lock down viewing of pages and parts of pages based on the user level of permission.

    My Datatable will be on a access controlled page and so the user will only be able to see that page if I've given them the permission.

    So if the some sort of key is set on the page in question and the datatables php script will not run without this key regardless of whether they are logged in or not it shouldn't matter. That's my thinking anyway!

    I had also been looking at the C5 CSRF token system as a method of doing this but simply cobbling the code together has been my issue.

    General info on C5 users is here https://documentation.concrete5.org/developers/users-groups/overview but more info on gaining information about the current user is here https://documentation.concrete5.org/developers/users-groups/reading-data-existing-users

    I hope this explains better what I'm trying to do.

  • allanallan Posts: 61,822Questions: 1Answers: 10,127 Site admin

    So if the some sort of key is set on the page in question and the datatables php script will not run without this key regardless of whether they are logged in or not it shouldn't matter. That's my thinking anyway!

    You need to make that the URL that the Ajax request is made to is also protected in a similar way. Otherwise someone might be able to guess the url and send malicious data without being authenticated! It would take a fair amount of guess work, but its certainly possible!

    For CSRF tokens, you can use the DataTables ajax or ajax.data options to submit the token with the Ajax request. Editor also has ajax and ajax.data options.

    Regards,
    Allan

  • gordonc200gordonc200 Posts: 39Questions: 7Answers: 0

    I don't think I'm explaining this or understanding it very well probably due to my inexperience. My apologies for this. Why does it matter if a user knows the URL if the code therein won't run without some sort of authentication?

    I think my difficulty has now morphed to something that is mostly outwith the scope of Datatables. However, if there are any ideas as to how I might resolve my problem I'd greatly appreciate it.

    Perhaps I can give a bit more background.

    I had Datatables working in a C5 based site using an older approach which is described here.

    http://c5hub.com/learning/ajax-driven-applications-concrete5/

    I had the datatables html/javascript working in the "view" code of my C5 application and it called the datatables php code in what they call a "tools" folder. A tools folder is basically a place to put third party code and as far as I can tell it more or less ignores the rest of the c5 framework. The problem therefore was to work out how I passed the authentication from the view to the datatables code. It was this I was messing about with when I asked my initial question. I was trying to keep the question in the context of datatables and not c5.

    However the c5 framework has moved on and there may be an oven baked solution. So I looked at the new approach which is described here

    https://documentation.concrete5.org/developers/working-with-blocks/creating-a-new-block-type/implementing-ajax-block-view-templates

    There's also an update on the earlier tutorial explaining what's changed here

    http://c5hub.com/learning/ajax-57-style/

    My reading of this is that the code that used to be in the tools folder could now be copied into a method in the page controller and that this method would check to see if the incoming code was coming from the c5 code and not some outside source.

    Unfortunately however this involves placing the rest of the datatables php in another location (the applications src folder) and then referencing it. This involves understanding namespacing (not just namespacing but C5's approach to namespacing), which I understand can leave even php veterans pulling their hair out but to make matters worse C5 doesn't support standard php "includes" which datatables php code seems to use in a few places. I'm guessing the "includes" in the src folder will be ok but not the code I'd need to paste into the controller.

    So in summation what I'm now trying to do is put the datatables php code (ie the code from table.drawings.php) into a concrete5 controller method similar to the "like" method here https://documentation.concrete5.org/developers/working-with-blocks/creating-a-new-block-type/implementing-ajax-block-view-templates. I can get the "like" method working and indeed the code for this is available to download for free. The problem I have is replacing the "like" code with the datatables code resolving the issue of the include statement not being allowed.

    Or in further summation there's too bits of code and not enough talent sitting here typing to cope!

  • allanallan Posts: 61,822Questions: 1Answers: 10,127 Site admin

    Why does it matter if a user knows the URL if the code therein won't run without some sort of authentication?

    It doesn't. Sorry if I miscommunicated myself. If the Ajax url that you use to submit the data has authentication on it, that's all you need to do. That is the goal here.

    which I understand can leave even php veterans pulling their hair out

    Yup.

    C5 doesn't support standard php "includes" which datatables php code seems to use in a few places. I'm guessing the "includes" in the src folder will be ok but not the code I'd need to paste into the controller.

    Interesting. I'd sort of been looking at this from the opposite view point. Instead of pulling the DataTables libraries into a C5 controller, can you pull the C5 session checking into your table.drawings.php file?

    That's what I had been hoping this stuff would allow, but that probably isn't the case if C5 wants all routes to come through their controllers now.

    The problem I have is replacing the "like" code with the datatables code resolving the issue of the include statement not being allowed.

    What error messages are you getting with that?

    Allan

  • gordonc200gordonc200 Posts: 39Questions: 7Answers: 0
    edited June 2017

    can you pull the C5 session checking into your table.drawings.php file?

    Pulling the session checking in to the file would be ideal if indeed I could get that working. That was my first idea using the "old" c5 approach. I'd be happy to use this if I could even get it to work in principle but if I do anything to the code it doesn't return a JSON response. Initially I was trying to wrap the code within the table.drawings.php file inside an if statement but I can't even get that to work.

    I'd set the if statement to run if the variable $test equaled a given value. Is set this value further up the code so I knew if was correct. If I could get this working I would then replace the $test variable with the authentication if indeed that can be done. I need to get the first but going first of all! I'm probably messing up the code or the syntax as your initial code basically suggested this should work.

    Perhaps I should simply pursue this again. So what am I doing wrong here?

    <?php
    
    /*
     * Editor server script for DB table drawings
     * Created by http://editor.datatables.net/generator
     */
        $test = 1;
        if ( $test = 1) {
    
    // DataTables PHP library and database connection
        include("lib/DataTables.php");
    
    // Alias Editor classes so they are easy to use
        use
            DataTables\Editor,
            DataTables\Editor\Field,
            DataTables\Editor\Format,
            DataTables\Editor\Mjoin,
            DataTables\Editor\Options,
            DataTables\Editor\Upload,
            DataTables\Editor\Validate;
    
    // Build our Editor instance and process the data coming from _POST
        Editor::inst($db, 'drawings', 'id')
            ->fields(
                Field::inst('drawings.site')
                    ->options(Options::inst()
                        ->table('sites')
                        ->value('id')
                        ->label('site_name')
                    )
                    ->validator('Validate::dbValues'),
                Field::inst('sites.site_name')
                    ->validator('Validate::notEmpty'),
                Field::inst('drawings.status')
                    ->options(Options::inst()
                        ->table('status')
                        ->value('id')
                        ->label('status')
                    )
                    ->validator('Validate::dbValues'),
                Field::inst('status.status')
                    ->validator('Validate::notEmpty'),
                Field::inst('drawings.category')
                    ->options(Options::inst()
                        ->table('category')
                        ->value('id')
                        ->label('category')
                    )
                    ->validator('Validate::dbValues'),
                Field::inst('category.category')
                    ->validator('Validate::notEmpty'),
                Field::inst('drawings.drawing_name')
                    ->validator('Validate::notEmpty'),
                Field::inst('drawings.drawing_number')
                    ->validator('Validate::notEmpty'),
                Field::inst('drawings.drawing_date')
                    ->validator('Validate::dateFormat', array('format' => 'D, j M y'))
                    ->getFormatter('Format::date_sql_to_format', 'D, j M y')
                    ->setFormatter('Format::date_format_to_sql', 'D, j M y'),
                Field::inst('drawings.drawing_author'),
                Field::inst('drawings.revision')
                    ->validator('Validate::notEmpty'),
                Field::inst('drawings.sheet_size_and_orientation'),
                Field::inst('drawings.revision_date')
                    ->validator('Validate::notEmpty')
                    ->validator('Validate::dateFormat', array('format' => 'D, j M y'))
                    ->getFormatter('Format::date_sql_to_format', 'D, j M y')
                    ->setFormatter('Format::date_format_to_sql', 'D, j M y'),
                Field::inst('drawings.revision_author')
                    ->validator('Validate::notEmpty'),
                Field::inst('drawings.revision_details')
                    ->validator('Validate::notEmpty')
            )
            ->leftJoin('sites', 'sites.id', '=', 'drawings.site')
            ->leftJoin('category', 'category.id', '=', 'drawings.category')
            ->leftJoin('status', 'status.id', '=', 'drawings.status')
            ->process($_POST)
            ->json();
    }
    

    What error messages are you getting with that?

    With the up to date approach I'm getting

    Class 'DataTables\Editor' not found

    which is probably due to the

    include statement

    include("path/to//DataTables.php");
    

    not working in the c5 controller. No matter how much messing about with the path I try.
    c5 does have an include function but I can't find any documentation on it. Only this forum post

    https://www.concrete5.org/community/forums/customizing_c5/difference-between-include-and-this-andgtinc

    The guy Andrew that answered that is pretty much the c5 project leader as far as I can tell.

  • gordonc200gordonc200 Posts: 39Questions: 7Answers: 0

    Apologies. I didn't backticks to format that code. Just more syntax errrors.

  • allanallan Posts: 61,822Questions: 1Answers: 10,127 Site admin
    Answer ✓

    if ( $test = 1) {

    This is an assignment. It will always pass. You want to use == or ===.

    This is basically what you want to do I think:

    if ( $test === 0~~~~ ) {
      // redirect to login page, then:
      exit(0);
    }
    
    // otherwise, allow it to carry on
    ...
    

    Allan

  • gordonc200gordonc200 Posts: 39Questions: 7Answers: 0
    edited June 2017

    Thanks very much Allan. I got this working thus

    $test = 1;
    
    if ( $test !== 1 ) {
        // if test isn't identical to provided value:
        exit;
    }
    
    // otherwise, allow it to carry on
    

    Checking the user is logged in or retrieving a token in place of the test variable is the next fun part. If I work it out I'll let you know.

  • gordonc200gordonc200 Posts: 39Questions: 7Answers: 0

    Got it working. I've been working with blocks in concrete5. Note to anyone else reading. This advice is related to Datatables in the context of Concrete5. I doubt its of any use for anything else.

    This needs to go in the block's view.php

    $th = Core::make('helper/concrete/urls');
    $bt = BlockType::getByHandle('block_handle_here');
    $toolURL = $th->getBlockTypeToolsURL($bt).'/'.'tables_php_script_name_here_without_php_extension';
    

    you then need to echo the url in the editor js function (your javascript needs to be in the view.php and not a separate .js file for the php to work

    ajax: '<?php echo "$toolURL";?>',
    

    If you use the hardcoded url for the datatables php it'll not work. The core functions of concrete will only work if the script is called by yoursite/index.php/concretes/crazy/route/to/your/file/which/is/different/from/its/actual/path

    The last piece is to put this at the top of the datatables table.php

    use Concrete\Core\User\User;
    
    $u = new User();
    
    if ($u->isRegistered()) {
    
        $test = 1;
    
    } else {
    
        $test=0;
    }
    
    if ( $test !== 1 ) {
        // if test isn't identical to provided value:
        exit;
    }
    
    // otherwise, allow it to carry on
    

    It's pretty clunky but it works. A user has to be logged in to run the script.

  • allanallan Posts: 61,822Questions: 1Answers: 10,127 Site admin
    Answer ✓

    That's superb - thanks for sharing it with us!

    I'd suggest one small change just to simplify your final PHP there:

    use Concrete\Core\User\User;
     
    $u = new User();
     
    if (! $u->isRegistered()) {
       exit;
    }
     
    // otherwise, allow it to carry on
    

    Just saves you a few lines of code.

    In fact, with newer version of PHP I though you could even reduce it to if ( ! new User()->isRegistered() ) { since you can chain off constructors.

    Allan

This discussion has been closed.