Traewelling / traewelling

Free check-in service to log your public transit journeys
https://traewelling.de
GNU Affero General Public License v3.0
231 stars 46 forks source link

API: /trains/checkin will always post a tweet #88

Closed ghost closed 4 years ago

ghost commented 4 years ago

Describe the bug Given that your Traewelling account is linked with your Twitter account, checking into a train using /trains/checkin with valid tripID, start and stop station (with workaround described in #72) will always post a tweet no matter what the "tweet" option is set to.

To Reproduce Have your Traewelling account linked with your Twitter account. Then:

curl -v https://traewelling.de/api/v0/trains/checkin -H 'authorization: Bearer ...' -H 'content-type: application/json' -d '{"tripID":"...","start":"...","destination":"...","tweet":false,"toot":false,"body":null}'

This will always result in a tweet on Twitter, even if "tweet" is false.

Expected behavior No tweets should be posted if "tweet" is false.

jeyemwey commented 4 years ago

Hi, thanks for reporting this issue. The issue seems to be that the TransportController only checks whether the variable "tweet" exists (see https://github.com/Traewelling/traewelling/blob/76d53ddb07b458aeec078b2815382e21f7b4e2f2/app/Http/Controllers/TransportController.php#L342).

isset returns true, even if the variable itself is set to false:

<?php

var_dump(isset($a)); // => false

$a = null;
var_dump(isset($a)); // => false

$a = false;
var_dump(isset($a)); // => true

We would have to change the TransportController (see above) and deploy to prod, which can take longer time, but should definitely happen with that bug in mind. In the meantime, can you try sending "tweet": null or no tweet property at all?

I am sorry for the inconvenience that this issue might have caused.

ghost commented 4 years ago

Sending "tweet": null will fail request validation:

{"error":{"tweet":["tweet muss wahr oder falsch sein."]}}

Trying to check in with valid data currently throws HTTP 500, so I will test not sending the "tweet" property later.