allejo / php-vcr-sanitizer

Bring privacy to php-vcr and erase any sensitive information. Gone are the days for hoping no one steals your API keys.
https://packagist.org/packages/allejo/php-vcr-sanitizer
MIT License
15 stars 5 forks source link

Sanitizing Headers (Creds) Succeeds the First Time, Fails On Consecutive Tests #22

Closed Justintime50 closed 3 years ago

Justintime50 commented 3 years ago

Summary

I'm currently using php-vcr-sanitizer to scrub email/api_key from the headers. When initially recording cassettes, I'm met with a fully passing test suite. When I run the tests again, I'm met with the following error:

LogicException: The request does not match a previously recorded request and the 'mode' is set to 'once'. If you want to send the request anyway, make sure your 'mode' is set to 'new_episodes'. Please see http://php-vcr.github.io/documentation/configuration/#record-modes.

After a bit of testing and printing the Request out, it obviously still has my email/api_key in the request. The cassette however has scrubbed those details from the file (as it should); however, I think this error occurs because the request and the cassette don't match and so it cannot play back the cassette due to the scrubbed headers. If I remove the sanitizing functionality and re-record cassettes, I can run tests dozens of times back to back which succeed because the cassette and request match up.

Code Snippets

# Unit test
    public function testCreate()
    {
        $client = new TuneupTechnology\Client(getenv("API_EMAIL"), getenv("API_KEY"), 'http://tuneapp.localhost/api');

        $response = $client->customers->create(
            $data = [
                "firstname" => "Jake",
                "lastname" => "Peralta",
                "email" => "jake@example.com",
                "phone" => "8015551234",
                "user_id" => 1,
                "notes" => "Believes he is a good detective.",
                "location_id" => 2,
            ]
        );

        $this->assertEquals($response->getStatusCode(), 200);
    }
# bootstrap.php
<?php

use VCR\VCR;
use allejo\VCR\VCRCleaner;

if (!file_exists('test/cassettes')) {
    mkdir('test/cassettes', 0777, true);
}

VCR::configure()->setCassettePath('test/cassettes')
    ->setWhiteList(array('vendor/guzzle'))
    ->setStorage('yaml')
    ->setMode('once');

VCRCleaner::enable(array(
    'request' => array(
        'ignoreHeaders' => array(
            'Email',
            'Api-Key',
        ),
    ),
));

I'm using guzzle alongside php-vcr and this library. My hope is that I can record cassettes properly while sanitizing my credentials from the file I'll be checking into the repo but by doing so, tests fail because they cannot play back the cassette as the request appears to be different (as it's scrubbed). I'm interested to know if I have this configured incorrectly or if there is indeed some bug here.

Thanks in advance! php-vcr should definitely support this functionality natively, I was very excited to see someone built out a library to do it in the meantime and would love to get this working.

allejo commented 3 years ago

Could you provide what both the sanitized and unsanitized cassettes look like? Also, is there any chance that TuneupTechnology\Client is open source and I can look at how it's making URL calls?

allejo commented 3 years ago

Also, I can't remember how php-vcr boots up, but have you called VCR::turnOn() and VCR::insertCassette() before enabling the VCRCleaner?

Justintime50 commented 3 years ago

Certainly, here is the WIP PR.

I'm using php-vcr/phpunit-testlistener-vcr to boot up VCR which inherently runs VCR::turnOn() and VCR::insertCassette() via a decorator on each test function.

Here is the sanitized cassette:


-
    request:
        method: POST
        url: 'http://tuneapp.localhost/api/customers'
        headers:
            Host: ''
            Expect: ''
            Accept-Encoding: ''
            Content-Type: application/json
            Accept: application/json
            User-Agent: TuneupTechnologyApp/PHPClient/2.0.0
            Email: ''
            Api-Key: ''
        body: '{"firstname":"Jake","lastname":"Peralta","email":"jake@example.com","phone":"8015551234","user_id":1,"notes":"Believes he is a good detective.","location_id":2}'
    response:
        status:
            http_version: '1.1'
            code: '200'
            message: OK
        body: '{"data":{"firstname":"Jake","lastname":"Peralta","email":"jake@example.com","phone":"8015551234","user_id":1,"notes":"Believes he is a good detective.","location_id":2,"updated_at":"2021-05-14 03:19:18","created_at":"2021-05-14 03:19:18","id":6},"message":"Customer created successfully."}'
        curl_info:
            url: 'http://tuneapp.localhost/api/customers'
            content_type: application/json
            http_code: 200
            header_size: 234
            request_size: 431
            filetime: -1
            ssl_verify_result: 0
            redirect_count: 0
            total_time: 0.751479
            namelookup_time: 0.001995
            connect_time: 0.00217
            pretransfer_time: 0.00224
            size_upload: !!float 160
            size_download: !!float 289
            speed_download: !!float 384
            speed_upload: !!float 213
            download_content_length: !!float 289
            upload_content_length: !!float 160
            starttransfer_time: 0.751452
            redirect_time: !!float 0
            redirect_url: ''
            primary_ip: 127.0.0.1
            certinfo: {  }
            primary_port: 80
            local_ip: 127.0.0.1
            local_port: 52205
            http_version: 2
            protocol: 1
            ssl_verifyresult: 0
            scheme: HTTP
            appconnect_time_us: 0
            connect_time_us: 2170
            namelookup_time_us: 1995
            pretransfer_time_us: 2240
            redirect_time_us: 0
            starttransfer_time_us: 751452
            total_time_us: 751479

Not sanitized:


-
    request:
        method: POST
        url: 'http://tuneapp.localhost/api/customers'
        headers:
            Host: tuneapp.localhost
            Expect: ''
            Accept-Encoding: ''
            Content-Type: application/json
            Accept: application/json
            User-Agent: TuneupTechnologyApp/PHPClient/2.0.0
            Email: SOME_EMAIL_HERE
            Api-Key: SOME_API_KEY_HERE
        body: '{"firstname":"Jake","lastname":"Peralta","email":"jake@example.com","phone":"8015551234","user_id":1,"notes":"Believes he is a good detective.","location_id":2}'
    response:
        status:
            http_version: '1.1'
            code: '200'
            message: OK
        headers:
            Cache-Control: 'no-cache, private'
            Content-Type: application/json
            Date: 'Mon, 17 May 2021 03:59:00 GMT'
            Server: nginx
            X-Powered-By: PHP/7.4.15
            X-Ratelimit-Limit: '60'
            X-Ratelimit-Remaining: '58'
            Content-Length: '289'
        body: '{"data":{"firstname":"Jake","lastname":"Peralta","email":"jake@example.com","phone":"8015551234","user_id":1,"notes":"Believes he is a good detective.","location_id":2,"updated_at":"2021-05-17 03:59:00","created_at":"2021-05-17 03:59:00","id":7},"message":"Customer created successfully."}'
        curl_info:
            url: 'http://tuneapp.localhost/api/customers'
            content_type: application/json
            http_code: 200
            header_size: 234
            request_size: 431
            filetime: -1
            ssl_verify_result: 0
            redirect_count: 0
            total_time: 0.642223
            namelookup_time: 0.002327
            connect_time: 0.002499
            pretransfer_time: 0.002588
            size_upload: !!float 160
            size_download: !!float 289
            speed_download: !!float 450
            speed_upload: !!float 249
            download_content_length: !!float 289
            upload_content_length: !!float 160
            starttransfer_time: 0.642196
            redirect_time: !!float 0
            redirect_url: ''
            primary_ip: 127.0.0.1
            certinfo: {  }
            primary_port: 80
            local_ip: 127.0.0.1
            local_port: 50702
            http_version: 2
            protocol: 1
            ssl_verifyresult: 0
            scheme: HTTP
            appconnect_time_us: 0
            connect_time_us: 2499
            namelookup_time_us: 2327
            pretransfer_time_us: 2588
            redirect_time_us: 0
            starttransfer_time_us: 642196
            total_time_us: 642223
Justintime50 commented 3 years ago

I believe I found the issue:

There is no check if the user asked to sanitize the Host field here: https://github.com/allejo/php-vcr-sanitizer/blob/master/src/VCRCleanerEventSubscriber.php#L115. This means that even if the user configures 'ignoreHostname' => false, (which is also the default) that the hostname is still scrubbed and replaced with ''. After looking at the sanitized/non-sanitized cassettes again, this was the only notable difference aside from some timing numbers etc. Once I filled in the Host field appropriately, my tests started passing again even with the headers scrubbed (which restores the expected behavior and intent behind using this library.)

I propose we wrap the setHeader logic with a check to ensure this is what the user wants OR, if needed, add an additional config option since I believe this also touches the host part of the URL.

Example I used in my local vendored copy of this library

if (Config::ignoreReqHostname()) {
    $request->setHeader('Host', '');
}

I'm more than happy to put up a PR, want to hear back on the direction you'd prefer before moving on this.

Justintime50 commented 3 years ago

I put up a PR (#23) that will correct this behavior.

allejo commented 3 years ago

Thank you so much @Justintime50 for investigating this and submitting a PR! Apologies for the slow responses, it's been a super busy week for me.

I've tagged v1.0.9 with your fix :tada: