Deck: BoardService returns 403 instead of not found

Created on 12 Jul 2018  路  4Comments  路  Source: nextcloud/deck

Describe the bug
While developing out the JSON api (issue #83), I noticed that the board service is throwing out a json response of 403: permission denied when trying to look up a non-existent bug.

Since the JSON api is consuming the boardService, it will be throwing the 403 response instead of a 404 response, which will be confusing for the app developers that are attempting to consume the upcoming json api.

To Reproduce
Steps to reproduce the behavior:
Execute a find against the board service against a board id that does not exist in the database. You can checkout the current json-api branch, and send a request to /api/v1.0/boards/{id} to observe the 403 response.

Expected behavior
I think boardService should be returning either a 404: Not found json response, or create a new entity not found exception and allow the calling function to handle the error.


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

bug

Most helpful comment

I unified the response for no permission with not found to not leak any information about existing boards to users who don't have access to. But yes, we should better return a 404 in all of those cases. :+1:

Those exceptions for that are thrown here: https://github.com/nextcloud/deck/blob/b633d82c5e9e406a5c2fb51205f4daca2bcebebc/lib/Service/PermissionService.php#L116

All 4 comments

I unified the response for no permission with not found to not leak any information about existing boards to users who don't have access to. But yes, we should better return a 404 in all of those cases. :+1:

Those exceptions for that are thrown here: https://github.com/nextcloud/deck/blob/b633d82c5e9e406a5c2fb51205f4daca2bcebebc/lib/Service/PermissionService.php#L116

@juliushaertl pretty sure this was resolved with REST API #535 pull request

@Nebri No, just tested, we still return the following for an nonexisting board:

{
    "status": 403,
    "message": "Permission denied"
}

Let's keep it this way for the first API version.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jbonlinea picture jbonlinea  路  4Comments

PanCakeConnaisseur picture PanCakeConnaisseur  路  4Comments

Dubidubiduu picture Dubidubiduu  路  4Comments

aproposnix picture aproposnix  路  4Comments

ampoz picture ampoz  路  4Comments