Use pre-signed URLs to download files #3797

LukasHirt · 2020-07-20T14:32:50Z

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 · 2020-07-20T14:32:53Z

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 · 2020-07-20T16:23:58Z

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 · 2020-07-21T07:33:23Z

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

LukasHirt · 2020-07-21T08:29:57Z

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 · 2020-07-21T11:38:28Z

added QA team label to look into the tests

src/plugins/phoenix.js

dpakach · 2020-07-22T10:38:39Z

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.

src/plugins/phoenix.js

dpakach · 2020-07-23T05:59:01Z

@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 · 2020-07-23T15:06:24Z

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 · 2020-07-23T15:06:45Z

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

dpakach · 2020-07-24T03:30:14Z

@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 · 2020-07-24T10:00:19Z

@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 · 2020-07-24T10:12:48Z

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 · 2020-07-24T13:45:41Z

@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 · 2020-07-24T13:51:51Z

@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 · 2020-07-24T16:22:01Z

@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 · 2020-07-24T16:22:26Z

Probably CI running with debug enabled as well...

butonic · 2020-07-24T18:54:26Z

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

kulmann · 2020-07-25T04:57:16Z

@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 · 2020-07-27T09:38:28Z

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 · 2020-07-27T09:54:14Z

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

butonic · 2020-07-27T10:37:50Z

@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',

The HEAD request has to run on the non-signed URL. If the file exists we can continue to kick off the download on the signed URL.

File version download was only supporting signed url downloads. Blob downloads are still needed when the backend doesn't support url signing.

LukasHirt added the Category:Enhancement Add new functionality label Jul 20, 2020

LukasHirt self-assigned this Jul 20, 2020

LukasHirt force-pushed the feature/signed-urls-download branch 2 times, most recently from fab719d to 7a45dd7 Compare July 20, 2020 16:18

individual-it added the QA:team label Jul 21, 2020

LukasHirt force-pushed the feature/signed-urls-download branch from d65f97f to f284c43 Compare July 21, 2020 11:48

butonic requested changes Jul 22, 2020

View reviewed changes

src/plugins/phoenix.js Outdated Show resolved Hide resolved

butonic mentioned this pull request Jul 22, 2020

Support signed URLs owncloud/ocis-proxy#75

Merged

butonic reviewed Jul 22, 2020

View reviewed changes

src/plugins/phoenix.js Outdated Show resolved Hide resolved

kulmann mentioned this pull request Jul 22, 2020

Remove head request #3809

Closed

kulmann marked this pull request as ready for review July 23, 2020 15:04

kulmann requested a review from butonic July 23, 2020 15:08

kulmann mentioned this pull request Jul 23, 2020

Update deps 2020 07 22 owncloud/ocis#409

Merged

63 tasks

butonic approved these changes Jul 24, 2020

View reviewed changes

jasson99 self-assigned this Jul 27, 2020

jasson99 force-pushed the feature/signed-urls-download branch from 2eb9058 to fd79c5a Compare July 27, 2020 08:54

butonic mentioned this pull request Jul 27, 2020

use dedicated 'insecure' param owncloud/openidconnect#89

Merged

individual-it mentioned this pull request Jul 27, 2020

[tests-only] use openid-connect insecure and not debug setting #3829

Merged

10 tasks

kulmann force-pushed the feature/signed-urls-download branch from 20f7b4c to 4654393 Compare July 27, 2020 12:12

LukasHirt and others added 9 commits July 27, 2020 16:47

Use pre-signed URL to download files

5e09bf7

Use pre-signed URLs to download file versions

03b42ee

Fetch HEAD before download to check for 404

d119b92

Use anchor element to trigger download

fe060d3

Fix HEAD request

e249bd3

The HEAD request has to run on the non-signed URL. If the file exists we can continue to kick off the download on the signed URL.

Take capability for url signing into account on downloads

e94c2bf

Add changelog item

d2d08c4

Code cleanup

58c7e77

Fix file version download

7ad405d

File version download was only supporting signed url downloads. Blob downloads are still needed when the backend doesn't support url signing.

kulmann force-pushed the feature/signed-urls-download branch from 4654393 to 2a2f9f7 Compare July 27, 2020 14:52

Run CI without debug enabled

e2811c3

kulmann force-pushed the feature/signed-urls-download branch from bb4e5e0 to e2811c3 Compare July 28, 2020 05:28

kulmann merged commit d46e754 into master Jul 28, 2020

delete-merged-branch bot deleted the feature/signed-urls-download branch July 28, 2020 06:25

kulmann mentioned this pull request Jul 28, 2020

Update owncloud-sdk to 1.0.0-711 #3796

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use pre-signed URLs to download files #3797

Use pre-signed URLs to download files #3797

LukasHirt commented Jul 20, 2020 •

edited

Loading

update-docs bot commented Jul 20, 2020

LukasHirt commented Jul 20, 2020

LukasHirt commented Jul 21, 2020

LukasHirt commented Jul 21, 2020

individual-it commented Jul 21, 2020

dpakach commented Jul 22, 2020 •

edited

Loading

dpakach commented Jul 23, 2020

kulmann commented Jul 23, 2020 •

edited

Loading

kulmann commented Jul 23, 2020

dpakach commented Jul 24, 2020 •

edited

Loading

LukasHirt commented Jul 24, 2020 •

edited

Loading

kulmann commented Jul 24, 2020 •

edited

Loading

DeepDiver1975 commented Jul 24, 2020

kulmann commented Jul 24, 2020

kulmann commented Jul 24, 2020

kulmann commented Jul 24, 2020

butonic commented Jul 24, 2020

kulmann commented Jul 25, 2020 •

edited

Loading

individual-it commented Jul 27, 2020 •

edited

Loading

butonic commented Jul 27, 2020

butonic commented Jul 27, 2020 •

edited by kulmann

Loading

Use pre-signed URLs to download files #3797

Use pre-signed URLs to download files #3797

Conversation

LukasHirt commented Jul 20, 2020 • edited Loading

Description

Motivation and Context

Types of changes

update-docs bot commented Jul 20, 2020

LukasHirt commented Jul 20, 2020

LukasHirt commented Jul 21, 2020

LukasHirt commented Jul 21, 2020

individual-it commented Jul 21, 2020

dpakach commented Jul 22, 2020 • edited Loading

dpakach commented Jul 23, 2020

kulmann commented Jul 23, 2020 • edited Loading

kulmann commented Jul 23, 2020

dpakach commented Jul 24, 2020 • edited Loading

LukasHirt commented Jul 24, 2020 • edited Loading

kulmann commented Jul 24, 2020 • edited Loading

DeepDiver1975 commented Jul 24, 2020

kulmann commented Jul 24, 2020

kulmann commented Jul 24, 2020

kulmann commented Jul 24, 2020

butonic commented Jul 24, 2020

kulmann commented Jul 25, 2020 • edited Loading

individual-it commented Jul 27, 2020 • edited Loading

butonic commented Jul 27, 2020

butonic commented Jul 27, 2020 • edited by kulmann Loading

LukasHirt commented Jul 20, 2020 •

edited

Loading

dpakach commented Jul 22, 2020 •

edited

Loading

kulmann commented Jul 23, 2020 •

edited

Loading

dpakach commented Jul 24, 2020 •

edited

Loading

LukasHirt commented Jul 24, 2020 •

edited

Loading

kulmann commented Jul 24, 2020 •

edited

Loading

kulmann commented Jul 25, 2020 •

edited

Loading

individual-it commented Jul 27, 2020 •

edited

Loading

butonic commented Jul 27, 2020 •

edited by kulmann

Loading