SkynetLabs / webportal-nginx

MIT License
1 stars 2 forks source link

validate skylink on all endpoints that accept skylink #22

Closed kwypchlo closed 1 year ago

kwypchlo commented 2 years ago

When trying to access skylink (especially via subdomain) we should check that the provided skylink is valid and return meaningful error message otherwise. Lack of validation is most noticeable when we return "401 Unauthenticated" error for invalid skylink for unauthenticated users instead of giving early feedback that skylink is not valid.

Before: user would get "401 Unauthenticated" error page After: user will get "400 Bad Request" page with plain test message explaining the issue (not a "pretty" page, that can be a follow up though)

Before: Screenshot 2022-09-12 at 18 08 25

After: Screenshot 2022-09-12 at 18 08 33

deployed to https://dev2.siasky.dev/ example: https://foo.dev2.siasky.dev/

linear[bot] commented 2 years ago
SKY-1169 Should URLs with malformed skylinks return a 401 on Account-Only portals?

## Describe the bug Non-existent subdomains are interpreted as incorrectly formed skylinks, and on account-only portals, an error is returned about not being authenticated instead of the expected "your skylink is bunk". ## To Reproduce Visit [https://asdfasdfsadfsadf.siasky.net/](https://asdfasdfsadfsadf.siasky.net/) and see "normal" behavior.\ Visit [https://asdfasdfsadfsadf.skynetfree.net/](https://asdfasdfsadfsadf.skynetfree.net/) and get asked to authenticate, even when logged in. ## Expected behavior Visiting the page should result in the user being informed their skylink is malformed. For not logged in users, either error will work.\ Or, we build a better error page? Or not assume the user is trying to access a skylink when the data doesn't match one of our established server subdomains? ## Screenshots [](https://uploads.linear.app/eda4353b-2c00-40e1-91f8-0994ee46dcfd/51f9f0ab-f3a4-4ada-a232-231271dac016/2015e7f5-f87a-4333-a923-53075758aa6b) [https://github.com/SkynetLabs/webportal-nginx/issues/26](https://github.com/SkynetLabs/webportal-nginx/issues/26)

codecov[bot] commented 2 years ago

Codecov Report

Merging #22 (c88e031) into main (051df52) will increase coverage by 0.23%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #22      +/-   ##
==========================================
+ Coverage   95.43%   95.66%   +0.23%     
==========================================
  Files          18       18              
  Lines         964     1016      +52     
==========================================
+ Hits          920      972      +52     
  Misses         44       44              
Impacted Files Coverage Δ
nginx/libs/skynet/skylink.lua 100.00% <100.00%> (ø)
nginx/libs/skynet/skylink.spec.lua 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

kwypchlo commented 1 year ago

not on a current roadmap