fix(proxy): add domain validation for proxy requests#1740
fix(proxy): add domain validation for proxy requests#1740gustavobtflores merged 3 commits intokernelci:mainfrom
Conversation
MarceloRobert
left a comment
MarceloRobert
left a comment
There was a problem hiding this comment.
When accessing a not-allowed domain, the log viewer page just says "Sorry... something went wrong". I think we should at least give a hint to the user as to what is wrong with that log so that they know where to start fixing things.
MarceloRobert
added
the
Backend
gustavobtflores
force-pushed
the
fix/add-domain-validation-proxy
branch
from
11cc0c2 to
7d9fddf
Compare
gustavobtflores
force-pushed
the
fix/add-domain-validation-proxy
branch
from
5ae9cf0 to
18cd0ad
Compare
- Added tests for allowed and forbidden domains, including wildcard and exact matches. - Updated existing tests to use new allowed domains and paths for consistency.
gustavobtflores
force-pushed
the
fix/add-domain-validation-proxy
branch
from
18cd0ad to
ec812a4
Compare
MarceloRobert
left a comment
MarceloRobert
left a comment
There was a problem hiding this comment.
LGTM, only some minor benefits
| "*.linaro.org", | ||
| ] | ||
|
|
||
| ALLOWED_S3_PATHS = [ |
There was a problem hiding this comment.
very nit: I'd add a comment saying that we don't want to allow the entirety of s3 to be allowed, so we only allow the buckets that we know have log data
| error instanceof AxiosError && | ||
| error.response?.status === HttpStatusCode.Forbidden | ||
| ) { | ||
| throw new Error('403:Domain not allowed'); |
There was a problem hiding this comment.
It's great, but we could still improve this because when an error 403 is received, the page still retries the request even though it doesn't have to
|
I think we might be able to handle this differently. |
4 participants
No description provided.