Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate in store usage and signing keys #3913

Closed
wkloucek opened this issue Jun 2, 2022 · 13 comments
Closed

Investigate in store usage and signing keys #3913

wkloucek opened this issue Jun 2, 2022 · 13 comments

Comments

@wkloucek
Copy link
Contributor

wkloucek commented Jun 2, 2022

Investigate in how the store extensions is used (Background: it is not scalable right now).
Current assumption: It is only used for WebDav signing keys.
Further assumption: Signing keys are not used by clients, eg. ownCloud Web

Conclusion:

  • remove the support for signing keys / backend signing
  • remove the ocs implementation in oCIS and completely rely on the ocs implementation in REVA (we keep the oCIS implementation for the signing endpoint right now)
  • remove the store since no one uses it anymore!?
@wkloucek
Copy link
Contributor Author

If we would drop support for presigned keys we could remove this config options:

// PreSignedURL is the config for the presigned url middleware
type PreSignedURL struct {
AllowedHTTPMethods []string `yaml:"allowed_http_methods"`
Enabled bool `yaml:"enabled" env:"PROXY_ENABLE_PRESIGNEDURLS"`
}

also the connected middleware could be removed.

@wkloucek
Copy link
Contributor Author

see also #2374

@wkloucek
Copy link
Contributor Author

see also #1356

@wkloucek
Copy link
Contributor Author

see also #1357

@wkloucek
Copy link
Contributor Author

If we would drop the signing-key endpoint, we also could drop the OCS service (that one from the oCIS repo), see

{
Type: config.RegexRoute,
Endpoint: "/ocs/v[12].php/cloud/user/signing-key", // only `user/signing-key` is left in ocis-ocs
Backend: "http://localhost:9110",
},

@micbar
Copy link
Contributor

micbar commented Jun 29, 2022

@kulmann @fschade I thought we are using signed urls for the web download?

@wkloucek
Copy link
Contributor Author

The /ocs/v[12].php/cloud/user/signing-key is only used for downloads within a user context. Eg. public links have a downloadUrl property in propfind responses, which is used for downloading files.

@kulmann proposes to also have downloadUrl property for propfind responses within a user context. The property should not be part of the default set of requested properties and will be requested on demand by ownCloud Web.

@michl19 michl19 added the Priority:p3-medium Normal priority label Jul 12, 2022
@michl19
Copy link
Contributor

michl19 commented Jul 12, 2022

needs communication with @C0rby and @kulmann to proceed

@wkloucek wkloucek moved this from Qualification to Bugs Prio 3 or less in Infinite Scale Team Board Jul 12, 2022
@micbar micbar added this to the 2.0.0 General Availability milestone Jul 18, 2022
@butonic
Copy link
Member

butonic commented Jul 19, 2022

AFAICT the having the store implementation as a dedicated service was the wrong approach. we should be using the go micro store interface and implement a getStore() function in ocis-pkg, similar to the ocis-pkg/registry package, which we use to getRegistry(). We implemented the latter to evaluate MICRO_REGISTRY which can then be used to swap the default MDNS registry for eg nats, kubernetes, etcd or consul.

In a similar way we should evaluate MICRO_STORE and instantiate either an in memory implementation for single node instances, or a replicated implementation for kubernetes. We could make this more granular by giving every service a {service}_STORE config option ... but it would already work with a single env var for anything that just wants to cache things.

see also #4193 (comment)

In any case having a dedicated store service is the wrong approach to begin with, IMO.

@wkloucek
Copy link
Contributor Author

the store is meanwhile also used in the proxy

storeClient := storesvc.NewStoreService("com.owncloud.api.store", grpc.DefaultClient())
if err != nil {
logger.Error().Err(err).
Str("gateway", cfg.Reva.Address).
Msg("Failed to create reva gateway service client")
}

@micbar micbar removed the p3-medium label Oct 5, 2023
@wkloucek
Copy link
Contributor Author

@butonic @dragotin If we could use a regular "store" here (as in OCIS_PERSISTENT_STORE) we could get rid of the store service and get rid of one more PVC in our SaaS (multiplied by the amount of installations).

Don't know if that could be a easy win

@micbar micbar removed this from the 2.1.0 Service Pack 1 milestone Jan 22, 2024
@butonic
Copy link
Member

butonic commented Jan 31, 2024

We should use a regular micro store and default it to memory using a ttl to 1h. A new signing key will be generated when it is unset.

To get rid of the /ocs/v[12].php/cloud/user/signing-key we could use /graph/v1.0/drives/{driveid}/items/{itemid}?$expand=thumbnails requests which also works for directories: /graph/v1.0/drives/{driveid}/items/{itemid}/children?$expand=thumbnails. In MS Graph the file content can be downloaded using a presigned url that is the Location of a 302 redirect to a GET /graph/v1.0/drives/{driveid}/items/{itemid}/content response. Since CORS forbids following redirects the same presigned url is part of the driveItem when selecting @microsoft.graph.downloadUrl. See Downloading files in JavaScript apps and driveItem Instance Attributes for details.

MS graph uses presigned download thinks for thumbnails like this: https://db3pap003files.storage.live.com/y4mw70OznhAgB518uvI9y8Z4vdU_2km4p8qE5ZzCWKalb9I0FDx9uHP1clxBJWa9CMMSbx4ffrWE3k2KgpwiArTHAaCgIHrnpsehBt7dornOEoKHZ87oa3MsZkGvX9OVVMP6a00NiuEz0UuDHkKmHCIv_inbVcU6nIWrQ_PEMzIuVf3zqxKCtS01g7Xt1Ke3khes-bx0ruxqn3zHc71CbI5cPcgmboZv65SWRAI7r31tp_k008WZxYxSP7nUu-QluI6b9wOEBaJ5APb3J5PWKfSoNjLskBZMxgmephcbS4Hlyfpk7P62O75PAxTI7wNiTCo?width=400&height=400&cropmode=none

In the ms graph explorer the demo data for a thumbnail uses a jwt in the presigned url: https://northcentralus1-mediap.svc.ms/transform/thumbnail?provider=spo&inputFormat=xlsx&cs=MDViMTBhMmQtNjJkYi00MjBjLTg2MjYtNTVmM2E1ZTc4NjVifFNQTw&docid=https%3A%2F%2Fm365x214355-my.sharepoint.com%2F_api%2Fv2.0%2Fdrives%2Fb!-RIj2DuyvEyV1T4NlOaMHk8XkS_I8MdFlUCq1BlcjgmhRfAj3-Z8RY2VpuvV_tpd%2Fitems%2F01BYE5RZZ6FUE5272C5JCY3L7CLZ7XOUYM%3Ftempauth%3DeyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJhdWQiOiIwMDAwMDAwMy0wMDAwLTBmZjEtY2UwMC0wMDAwMDAwMDAwMDAvbTM2NXgyMTQzNTUtbXkuc2hhcmVwb2ludC5jb21AZGNkMjE5ZGQtYmM2OC00YjliLWJmMGItNGEzM2E3OTZiZTM1IiwiaXNzIjoiMDAwMDAwMDMtMDAwMC0wZmYxLWNlMDAtMDAwMDAwMDAwMDAwIiwibmJmIjoiMTcwNjY5MTYwMCIsImV4cCI6IjE3MDY3MTMyMDAiLCJlbmRwb2ludHVybCI6IlltSlA4aUlIWjg5UWxLTWZiTnJYV3JrTjdad0VSaDYvdDlscDVDVzdPTFE9IiwiZW5kcG9pbnR1cmxMZW5ndGgiOiIxNjIiLCJpc2xvb3BiYWNrIjoiVHJ1ZSIsInZlciI6Imhhc2hlZHByb29mdG9rZW4iLCJzaXRlaWQiOiJaRGd5TXpFeVpqa3RZakl6WWkwMFkySmpMVGsxWkRVdE0yVXdaRGswWlRZNFl6RmwiLCJhcHBfZGlzcGxheW5hbWUiOiJhcGlzYW5kYm94cHJveHkiLCJnaXZlbl9uYW1lIjoiTWVnYW4iLCJmYW1pbHlfbmFtZSI6IkJvd2VuIiwiYXBwaWQiOiIwNWIxMGEyZC02MmRiLTQyMGMtODYyNi01NWYzYTVlNzg2NWIiLCJ0aWQiOiJkY2QyMTlkZC1iYzY4LTRiOWItYmYwYi00YTMzYTc5NmJlMzUiLCJ1cG4iOiJtZWdhbmJAbTM2NXgyMTQzNTUub25taWNyb3NvZnQuY29tIiwicHVpZCI6IjEwMDNCRkZEQTM4MTMxQUYiLCJjYWNoZWtleSI6IjBoLmZ8bWVtYmVyc2hpcHwxMDAzYmZmZGEzODEzMWFmQGxpdmUuY29tIiwic2NwIjoibXlmaWxlcy5yZWFkIGdyb3VwLnJlYWQgYWxsc2l0ZXMucmVhZCBhbGxwcm9maWxlcy5yZWFkIGFsbHByb2ZpbGVzLnJlYWQgdGVybXN0b3JlLnJlYWQiLCJ0dCI6IjIiLCJpcGFkZHIiOiI0MC4xMjYuNDEuOTYifQ.1sVXfxFj4tiePRH-hUdxmbjFw_346jcE1ymRQPx83F8%26version%3DPublished&width=800&height=800&cb=63637718830

{
  "aud": "00000003-0000-0ff1-ce00-000000000000/m365x214355-my.sharepoint.com@dcd219dd-bc68-4b9b-bf0b-4a33a796be35",
  "iss": "00000003-0000-0ff1-ce00-000000000000",
  "nbf": "1706691600",
  "exp": "1706713200",
  "endpointurl": "YmJP8iIHZ89QlKMfbNrXWrkN7ZwERh6/t9lp5CW7OLQ=",
  "endpointurlLength": "162",
  "isloopback": "True",
  "ver": "hashedprooftoken",
  "siteid": "ZDgyMzEyZjktYjIzYi00Y2JjLTk1ZDUtM2UwZDk0ZTY4YzFl",
  "app_displayname": "apisandboxproxy",
  "given_name": "Megan",
  "family_name": "Bowen",
  "appid": "05b10a2d-62db-420c-8626-55f3a5e7865b",
  "tid": "dcd219dd-bc68-4b9b-bf0b-4a33a796be35",
  "upn": "meganb@m365x214355.onmicrosoft.com",
  "puid": "1003BFFDA38131AF",
  "cachekey": "0h.f|membership|1003bffda38131af@live.com",
  "scp": "myfiles.read group.read allsites.read allprofiles.read allprofiles.read termstore.read",
  "tt": "2",
  "ipaddr": "40.126.41.96"
}

The tempauth token here is valid for 6h and the endpointurl seems to be a hash of the request url.

so ... we could use a jwt in the download urls for files and thumbnails to 'sign' the request. In ms graph they are valid for 1h btw.

@wkloucek
Copy link
Contributor Author

Using a regular micro store, eg. nats-js kv-store is now possible. Close here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

4 participants