bcgov / parks-data-register

Apache License 2.0
6 stars 4 forks source link

[L] When API key is missing we need to display a proper user message #231

Closed manuji closed 1 week ago

manuji commented 12 months ago

Scenario:

Issue:

Expected:

Refer to the current response below when API key is missing.

`<!DOCTYPE html>

Authenticated access to Parks Data Register Admin `
JLWade commented 11 months ago

Bringing this ticket into current sprint to address while we are looking at api keys etc. Removed A&R tickets from to-do.

cameronpettit commented 11 months ago

@JLWade @manuji FYI what you are seeing here is CloudFront's default 403/404 response - essentially it is the raw html that will render our 404 page in a browser. Without Lambda@Edge we cannot differentiate the 403/404 responses between the admin portal and the API when we are hitting the vanity url. @manuji try making the same call to the non vanity url ({{base_url}}) in Postman and you will get a much more coherent response.

This is a wontfix until we get Lambda@Edge.

marklise commented 10 months ago

Setting to blocked until Lambda@Edge

JLWade commented 9 months ago

LaMBDA@EDGE is available now so will move this out of blocked

LindsayMacfarlane commented 1 week ago

Hi @Dianadec @marklise - what's the status of this ticket? Looks like it is linked to the Lambda, so curious if this one can be closed? If we need to keep it open, what's our priority given everything else on our plates?

cameronpettit commented 1 week ago

IMO this is not a bug. I believe we have access to Lambda@Edge now but the effort required to implement it just so that an API user gets a more coherent error message when they try to access the API without a key (even though every call in Data Register requires an API key) is not worth the effort. Nothing sensitive is contained in that return object when a key is not provided. If we do nothing, the worst thing that could happen is someone trying to access the API without a key (no good actor should be doing this anyways) won't get an informative error.

Recommend closing with wontfix.

LindsayMacfarlane commented 1 week ago

Based on recommendation above - closing this ticket. If it becomes an issue in the future we can revisit.