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

Fix token auth #11

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Nekroze
Copy link
Contributor

@Nekroze Nekroze commented Nov 27, 2018

Hello,

In using this library we have found issues with the FatZebra->token_authorization method attempting to use several variables that do not exist.

This PR consists of 3 commits, the first adds some docker-compose.yml files for the test and example directories to more easily run tests locally with different PHP versions.

The next commit implements a test that if run will fail on:

PHPUnit 6.5.13 by Sebastian Bergmann, Julien Breux (Docker) and contributors.

...............IE                                                 17 / 17 (100%)

Time: 13.29 seconds, Memory: 4.00MB

There was 1 error:

1) GatewayTest::test_token_authorization
Undefined variable: token

/app/FatZebra.class.php:238
/app/test/gateway_tests.php:253

ERRORS!
Tests: 17, Assertions: 39, Errors: 1, Incomplete: 1.

The final commit passes the previously added test by fixing the test_authorization method.

@Nekroze
Copy link
Contributor Author

Nekroze commented Nov 27, 2018

Should this PR also bump the semver PATCH level?

@Nekroze
Copy link
Contributor Author

Nekroze commented Nov 27, 2018

Should this PR also bump the semver PATCH level?

Added 4th commit to do this, once merged the top commit should be git tagged 1.2.2

@Nekroze
Copy link
Contributor Author

Nekroze commented Nov 29, 2018

I have also had to patch the customer_ip logic as it made too many assumptions about the environment making it harder to run in various test/staging environments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant