Use pre-signed URLs to download files

[LukasHirt]

Description

Get URL of the file, use sdk to pre-sign it and then download the file via it.

Motivation and Context

No need to fetch the files first.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[LukasHirt]

The download seems to work fine now apart from one issue - if the file was deleted before the download, it lands on a "Not found" page. This is breaking the behaviour we've had before where "only" a fail message was shown.

[LukasHirt]

Trying to download files with oCIS results in Error: Secret key is required

[LukasHirt]

I included a fetch with method HEAD to check if the file exists before download. This is not that nice so if anyone has a better suggestion I am listening 🙂

[individual-it]

added QA team label to look into the tests

[dpakach]

Ocis tests are failing with this logs in server

error="datagateway: error validating transfer token: error parsing token: token contains an invalid number of segments"

2020-07-22T16:22:10+05:45 ERR invalid transfer token error="datagateway: error validating transfer token: error parsing token: token contains an invalid number of segments" pkg=rhttp service=reva token=testimage.eps traceid=ab8e7c5fdfc78687a4f66717bc71c824
2020-07-22T16:22:10+05:45 WRN http end="22/Jul/2020:16:22:10 +0545" host=192.168.100.210 method=GET pkg=rhttp proto=HTTP/1.1 service=reva size=0 start="22/Jul/2020:16:22:10 +0545" status=403 time_ns=156803 traceid=ab8e7c5fdfc78687a4f66717bc71c824 uri=/data/user1/testimage.eps url=/data/user1/testimage.eps
2020-07-22T16:22:10+05:45 ERR http end="22/Jul/2020:16:22:10 +0545" host=127.0.0.1 method=GET pkg=rhttp proto=HTTP/1.1 service=reva size=0 start="22/Jul/2020:16:22:10 +0545" status=500 time_ns=40051953 traceid=ab8e7c5fdfc78687a4f66717bc71c824 uri=/remote.php/dav/files/user1/testimage.eps?a=1&c=6a41de85d8ebdd746d0ebf693d5f99c8&preview=1&scalingup=0 url=/remote.php/dav/files/user1/testimage.eps?a=1&c=6a41de85d8ebdd746d0ebf693d5f99c8&preview=1&scalingup=0

Something with the number of segments.

[dpakach]

@LukasHirt does this feature work with oc10 too? I was unable to reproduce the failure in CI, but when I try to download the file from the UI it sends the request to http://172.17.0.1/core/ocs/v1.php/cloud/user/signing-key URL and fails. This may be causing CI failure.

[kulmann]

Adapted this

@LukasHirt does this feature work with oc10 too? I was unable to reproduce the failure in CI, but when I try to download the file from the UI it sends the request to http://172.17.0.1/core/ocs/v1.php/cloud/user/signing-key URL and fails. This may be causing CI failure.

Not supported in oc10 (edit: up to 10.4), but the code was not taking the signing capability into consideration. That is fixed now, please retry. In oc10 without signing capability the download should again fall back to the BLOB based download (which is limited to 2GB). In ocis, the signing capability is enabled, so ocis-web will trigger a download with presigned url.

[kulmann]

@butonic @IljaN @LukasHirt please test and review, thanks!

[dpakach]

@kulmann Looks like signing url works in oc10 (git master which we use in CI) owncloud/core#37634.
But now downloading does not work properly. If you download files like .zip then it works as expected but when downloading files like .txt it just opens the file content in the browser. I thank that is breaking the tests in CI right now.
When I try to download a file I get this

[LukasHirt]

@kulmann would maybe this endpoint help with the issue of files being opened? /index.php/apps/files/ajax/download.php?${filePath} Not sure about oCIS though 🤷

[kulmann]

Wow.... @dpakach you're right, since owncloud core 10.5, downloads don't have a content disposition header anymore. Because of that, the respective file gets opened directly in the browser. It only worked in phoenix so far, because without presigned urls we were using blobs for download, without minding the content disposition header. Investigating...

[DeepDiver1975]

@kulmann would maybe this endpoint help with the issue of files being opened? /index.php/apps/files/ajax/download.php?${filePath} Not sure about oCIS though shrug

Please only use this endpoint for zipped download - any other download must go via the webdav endpoints

[kulmann]

@kulmann would maybe this endpoint help with the issue of files being opened? /index.php/apps/files/ajax/download.php?${filePath} Not sure about oCIS though shrug

Please only use this endpoint for zipped download - any other download must go via the webdav endpoints

Ok, then I won't bother trying, thanks!

@DeepDiver1975 do you happen to have an idea why the Content-Disposition header is missing when running with oc10.5? 🤔 Tests against oc10.5 are failing because the respective file directly opens in the browser, and thus all the tests don't have a chance to continue after triggering a file download.

[kulmann]

@DeepDiver1975 do you happen to have an idea why the Content-Disposition header is missing when running with oc10.5? thinking Tests against oc10.5 are failing because the respective file directly opens in the browser, and thus all the tests don't have a chance to continue after triggering a file download.

is debug set to true in the config.php?

Wow 😱 Thank you @DeepDiver1975, that was it! 🤯

[kulmann]

Probably CI running with debug enabled as well...

[butonic]

for some more digging to understand what is causing the change in behavior follow owncloud/core#25254 (comment)

[kulmann]

@individual-it could you clarify why we need the debug config variable set to true? It's set here and a full test run debug=false has basically all tests in a failing state, see here: https://drone.owncloud.com/owncloud/phoenix/10511/10/19

However, if it is set to true the download tests against oc10.5 are failing because of what owncloud/core#25254 (comment).

Also, we should probably introduce a test suite for download tests with oc10.4, as those behave different than downloads on oc10.5:

• oc10.4: download as blob, because it doesn't have the capability to sign urls and downloads can't have authentication
• oc10.5: download with signed url

At the moment we're blocked, CI needs to be changed somehow...

[individual-it]

the debug mode is needed for https://github.com/owncloud/openidconnect/blob/bbaaa9cb6e5bd8d9993701db5d7df03312a34087/lib/Client.php#L69-L71
otherwise the openidconnect app will not accept self-signed URLs

{"reqId":"GSkLetvkdTXGMMMLMn1H","level":3,"time":"2020-07-27T09:09:32+00:00","remoteAddr":"192.168.48.2","user":"--","app":"OpenID","method":"GET","url":"\/ocs\/v1.php\/cloud\/capabilities?format=json","message":"Exception: {\"Exception\":\"Jumbojett\\\\OpenIDConnectClientException\",\"Message\":\"Curl error: SSL certificate problem: unable to get local issuer certificate\",\"Code\":0,\"Trace\":\"#0 \\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/openidconnect\\\/lib\\\/Client.php(139): Jumbojett\\\\OpenIDConnectClient->fetchURL('https:\\\/\\\/konnect...', NULL, Array)\\n#1 \\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/openidconnect\\\/vendor\\\/jumbojett\\\/openid-connect-php\\\/src\\\/OpenIDConnectClient.php(503): OCA\\\\OpenIdConnect\\\\Client->fetchURL('https:\\\/\\\/konnect...')\\n#2 \\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/openidconnect\\\/vendor\\\/jumbojett\\\/openid-connect-php\\\/src\\\/OpenIDConnectClient.php(482): Jumbojett\\\\OpenIDConnectClient->getWellKnownConfigValue('jwks_uri', NULL)\\n#3 \\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/openidconnect\\\/vendor\\\/jumbojett\\\/openid-connect-php\\\/src\\\/OpenIDConnectClient.php(890): Jumbojett\\\\OpenIDConnectClient->getProviderConfigValue('jwks_uri')\\n#4 \\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/openidconnect\\\/lib\\\/OpenIdConnectAuthModule.php(161): Jumbojett\\\\OpenIDConnectClient->verifyJWTsignature('eyJhbGciOiJQUzI...')\\n#5 \\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/openidconnect\\\/lib\\\/OpenIdConnectAuthModule.php(101): OCA\\\\OpenIdConnect\\\\OpenIdConnectAuthModule->verifyJWT('eyJhbGciOiJQUzI...')\\n#6 \\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/openidconnect\\\/lib\\\/OpenIdConnectAuthModule.php(87): OCA\\\\OpenIdConnect\\\\OpenIdConnectAuthModule->authToken('eyJhbGciOiJQUzI...')\\n#7 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/private\\\/User\\\/Session.php(977): OCA\\\\OpenIdConnect\\\\OpenIdConnectAuthModule->auth(Object(OC\\\\AppFramework\\\\Http\\\\Request))\\n#8 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/base.php(970): OC\\\\User\\\\Session->tryAuthModuleLogin(Object(OC\\\\AppFramework\\\\Http\\\\Request))\\n#9 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/private\\\/AppFramework\\\/Middleware\\\/Security\\\/SecurityMiddleware.php(129): OC::handleLogin(Object(OC\\\\AppFramework\\\\Http\\\\Request))\\n#10 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/private\\\/AppFramework\\\/Middleware\\\/MiddlewareDispatcher.php(88): OC\\\\AppFramework\\\\Middleware\\\\Security\\\\SecurityMiddleware->beforeController(Object(OC\\\\Core\\\\Controller\\\\CloudController), 'getCapabilities')\\n#11 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/private\\\/AppFramework\\\/Http\\\/Dispatcher.php(83): OC\\\\AppFramework\\\\Middleware\\\\MiddlewareDispatcher->beforeController(Object(OC\\\\Core\\\\Controller\\\\CloudController), 'getCapabilities')\\n#12 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/private\\\/AppFramework\\\/App.php(100): OC\\\\AppFramework\\\\Http\\\\Dispatcher->dispatch(Object(OC\\\\Core\\\\Controller\\\\CloudController), 'getCapabilities')\\n#13 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/private\\\/AppFramework\\\/Routing\\\/RouteActionHandler.php(47): OC\\\\AppFramework\\\\App::main('CloudController', 'getCapabilities', Object(OC\\\\AppFramework\\\\DependencyInjection\\\\DIContainer), Array)\\n#14 \\\/var\\\/www\\\/owncloud\\\/server\\\/lib\\\/private\\\/Route\\\/Router.php(342): OC\\\\AppFramework\\\\Routing\\\\RouteActionHandler->__invoke(Array)\\n#15 \\\/var\\\/www\\\/owncloud\\\/server\\\/ocs\\\/v1.php(84): OC\\\\Route\\\\Router->match('\\\/ocsapp\\\/cloud\\\/c...')\\n#16 {main}\",\"File\":\"\\\/var\\\/www\\\/owncloud\\\/server\\\/apps\\\/openidconnect\\\/vendor\\\/jumbojett\\\/openid-connect-php\\\/src\\\/OpenIDConnectClient.php\",\"Line\":1144}"}

[butonic]

created owncloud/openidconnect#89 to separate the certificate checking configuration from debug mode

[butonic]

@jasson99 @individual-it with owncloud/openidconnect#89 it should be possible to switch from

'php occ config:system:set debug --value=true --type=bool',

to

'php occ config:system:set openid-connect insecure --value=true --type=bool',