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

Request with invalid token_auth responds 200 OK #18055

Closed
MrIsak opened this issue Sep 23, 2021 · 4 comments · Fixed by #19813
Closed

Request with invalid token_auth responds 200 OK #18055

MrIsak opened this issue Sep 23, 2021 · 4 comments · Fixed by #19813
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Milestone

Comments

@MrIsak
Copy link

MrIsak commented Sep 23, 2021

When sending requests against the API module with a non existing token, the HTTP response should be 403. Not 200.

Expected Behavior

When sending a request with a non existing token, response code should be 403

Current Behavior

Response code is 200

Steps to Reproduce (for Bugs)

  1. curl -ik 'https://matomo.example.com/index.php?module=API&method=API.getMatomoVersion&token_auth=I_DONT_EXIST'

Your Environment

  • Matomo Version: 4.4.1
  • PHP Version: PHP 7.4.3
  • Server Operating System: Ubuntu 20.04.03
@MrIsak MrIsak added the Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. label Sep 23, 2021
@sgiehl
Copy link
Member

sgiehl commented Sep 23, 2021

Hi @MrIsak
Thanks for your suggestion. You are right, I guess it would make sense to return a proper response code in this case.

@sgiehl sgiehl added c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement. and removed Potential Bug Something that might be a bug, but needs validation and confirmation it can be reproduced. labels Sep 23, 2021
@tsteur
Copy link
Member

tsteur commented Sep 23, 2021

This one we might want to do in Matomo 5.0 just because it's kind of a breaking change. We're using some of these URLs with invalid token in some monitoring tools ourselves and we'd get paged if there's a change and it's no longer HTTP 2XX.

It could also cause issues potentially with the Matomo Mobile app and possibly other apps etc when someone is trying to log in with wrong username/password.

@jdelucaa
Copy link

Hi guys,

Is there any case the API would respond status != 200? So far I've only seen 200s and the only indication that an error has occurred can be found in the response body (result = "error").

@jane-twizel
Copy link

Needs to be included in the developer changelog as well in case anyone is using an invalid token in the monitoring tool.

@peterhashair peterhashair self-assigned this Oct 7, 2022
@peterhashair peterhashair linked a pull request Oct 13, 2022 that will close this issue
11 tasks
@peterhashair peterhashair removed their assignment Dec 21, 2022
@mattab mattab added the 5.0.0 label Jan 4, 2023
@bx80 bx80 changed the title request with invalid token_auth responds 200 OK Request with invalid token_auth responds 200 OK Feb 3, 2023
@bx80 bx80 removed the Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement. label Feb 3, 2023
@sgiehl sgiehl closed this as completed Feb 3, 2023
@sgiehl sgiehl added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants