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

Use a URL object in OpenInAppResponse #1968

Merged
merged 10 commits into from
Sep 1, 2021

Conversation

ishank011
Copy link
Contributor

No description provided.

go.mod Outdated Show resolved Hide resolved
@wkloucek
Copy link
Contributor

@ishank011 I have this PR now running in owncloud/ocis#2204. Let me check tomorrow morning if I find something.
I already saw some inconsistencies about the access token validity duration / TTL.

@wkloucek
Copy link
Contributor

Doing the list request curl --insecure https://ocis.owncloud.test/app/list | jq I get something like this (shortened):

{
  "mime-types": {
    "application/msword": {
      "app_providers": [
        {
          "address": "127.0.0.1:9164",
          "name": "Collabora"
        }
      ]
    },
    "text/plain": {
      "app_providers": [
        {
          "address": "localhost:9142"
        },
        {
          "address": "127.0.0.1:9164",
          "name": "Collabora"
        }
      ]
    }
  }
}

Technically we should be able to remove the address part since it has no function ( is not needed for the app/open request and just adds payload and could confuse people!?

I would also go for an empty appprovider registry by default (https://github.com/ishank011/reva/blob/48e83351374fb8d647f55bb146ad2c3adbc95ae1/pkg/app/registry/static/static.go#L42-L51) since you will always have the "text/plain" handler as seen also above when you use WOPI server only which does dynamic self registration. I also see currently no way to remove this "plain/text" entry in a proper way except configuring another static appprovider before startup. Futhermore it lacks a name field.

@wkloucek
Copy link
Contributor

When opening a file with curl -k --insecure -X POST 'https://ocis.owncloud.test/app/open?file_id=MTI4NGQyMzgtYWE5Mi00MmNlLWJkYzQtMGIwMDAwMDA5MTU3Ojc0OTdjZDkxLTE3NzAtNGYwMy1iYmQzLTdmOGRiYzQ3ZTdiOQ==&view_mode=write&app_name=Collabora' -u einstein:relativity | jq, I get following response:

{
  "app_url": "https://collabora.owncloud.test/loleaflet/4aa2794/loleaflet.html?&WOPISrc=https%3A%2F%2Fwopiserver.owncloud.test%2Fwopi%2Ffiles%2F1284d238-aa92-42ce-bdc4-0b0000009157-NzQ5N2NkOTEtMTc3MC00ZjAzLWJiZDMtN2Y4ZGJjNDdlN2I5",
  "method": "POST",
  "form_parameters": {
    "access_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1c2VyaWQiOiJleUpoYkdjaU9pSklVekkxTmlJc0luUjVjQ0k2SWtwWFZDSjkuZXlKaGRXUWlPaUp5WlhaaElpd2laWGh3SWpveE5qSTRPVEkwTnpJNExDSnBZWFFpT2pFMk1qZzRNemd6TWpnc0ltbHpjeUk2SW1oMGRIQnpPaTh2YjJOcGN5NXZkMjVqYkc5MVpDNTBaWE4wSWl3aWRYTmxjaUk2ZXlKcFpDSTZleUpwWkhBaU9pSm9kSFJ3Y3pvdkwyOWphWE11YjNkdVkyeHZkV1F1ZEdWemRDSXNJbTl3WVhGMVpWOXBaQ0k2SWpSak5URXdZV1JoTFdNNE5tSXRORGd4TlMwNE9ESXdMVFF5WTJSbU9ESmpNMlExTVNJc0luUjVjR1VpT2pGOUxDSjFjMlZ5Ym1GdFpTSTZJbVZwYm5OMFpXbHVJaXdpYldGcGJDSTZJbVZwYm5OMFpXbHVRR1Y0WVcxd2JHVXViM0puSWl3aVpHbHpjR3hoZVY5dVlXMWxJam9pUVd4aVpYSjBJRVZwYm5OMFpXbHVJaXdpZFdsa1gyNTFiV0psY2lJNk1qQXdNREFzSW1kcFpGOXVkVzFpWlhJaU9qTXdNREF3ZlN3aWMyTnZjR1VpT25zaWRYTmxjaUk2ZXlKeVpYTnZkWEpqWlNJNmV5SmtaV052WkdWeUlqb2lhbk52YmlJc0luWmhiSFZsSWpvaVpYbEtkMWxZVW05SmFtOXBUSGxLT1NKOUxDSnliMnhsSWpveGZYMTkucXBQY21iSk5oV3RtQWpCeGpvVmFTbll1cHM1WmFqS0pReW8weXZlVFZtWSIsImZpbGVuYW1lIjoiL3VzZXJzLzRjNTEwYWRhLWM4NmItNDgxNS04ODIwLTQyY2RmODJjM2Q1MS9OZXcgZmlsZS5vZHQiLCJ1c2VybmFtZSI6ImVpbnN0ZWluIiwidmlld21vZGUiOiJWSUVXX01PREVfUkVBRF9XUklURSIsImZvbGRlcnVybCI6IiUvIiwiZW5kcG9pbnQiOiIxMjg0ZDIzOC1hYTkyLTQyY2UtYmRjNC0wYjAwMDAwMDkxNTciLCJhcHBuYW1lIjoiQ29sbGFib3JhIiwiYXBwZWRpdHVybCI6Imh0dHBzOi8vY29sbGFib3JhLm93bmNsb3VkLnRlc3QvbG9sZWFmbGV0LzRhYTI3OTQvbG9sZWFmbGV0Lmh0bWw_IiwiYXBwdmlld3VybCI6IiIsImV4cCI6MTYyODkyNDcyOX0.3ZYk5vqMCXVJu8XCx3MjNcSiECZGZ8dHS3UkDO_uObY",
    "access_token_ttl": "1628924728"
  }
}

access_token_ttl from the json gives me 1628924728, which is Sat Aug 14 2021 07:05:28 GMT+0000.
The exp of the WOPI server JWT bearer token (outer JWT token) gives me 1628924729, which is Sat Aug 14 2021 07:05:29 GMT+0000.
The exp of the REVA JWT bearer token (inner JWT token) gives me 1628924728, which is Sat Aug 14 2021 07:05:28 GMT+0000.

Therefore access_token_ttl reflects the most shortlived JWT bearer token of them both, which is correct.

The only thing which needs a concept / change is that WOPI assumes that the access_token_ttl is in the Java Script date epoch format (millseconds since Jan 1, 1970 UTC / unix timestamp in milliseconds). This means access_token_ttl should equal 1628924728000 in this case.

@wkloucek
Copy link
Contributor

wkloucek commented Aug 13, 2021

If I try to open a file in view or read mode with curl -k --insecure -X POST 'https://ocis.owncloud.test/app/open?file_id=MTI4NGQyMzgtYWE5Mi00MmNlLWJkYzQtMGIwMDAwMDA5MTU3Ojc0OTdjZDkxLTE3NzAtNGYwMy1iYmQzLTdmOGRiYzQ3ZTdiOQ==&view_mode=view&app_name=Collabora' -u einstein:relativity | jq, the app_url in the reponse is malformed:

{
  "app_url": "&WOPISrc=https%3A%2F%2Fwopiserver.owncloud.test%2Fwopi%2Ffiles%2F1284d238-aa92-42ce-bdc4-0b0000009157-NzQ5N2NkOTEtMTc3MC00ZjAzLWJiZDMtN2Y4ZGJjNDdlN2I5",
  "method": "POST",
  "form_parameters": {
    "access_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1c2VyaWQiOiJleUpoYkdjaU9pSklVekkxTmlJc0luUjVjQ0k2SWtwWFZDSjkuZXlKaGRXUWlPaUp5WlhaaElpd2laWGh3SWpveE5qSTRPVEkyTkRZeUxDSnBZWFFpT2pFMk1qZzROREF3TmpJc0ltbHpjeUk2SW1oMGRIQnpPaTh2YjJOcGN5NXZkMjVqYkc5MVpDNTBaWE4wSWl3aWRYTmxjaUk2ZXlKcFpDSTZleUpwWkhBaU9pSm9kSFJ3Y3pvdkwyOWphWE11YjNkdVkyeHZkV1F1ZEdWemRDSXNJbTl3WVhGMVpWOXBaQ0k2SWpSak5URXdZV1JoTFdNNE5tSXRORGd4TlMwNE9ESXdMVFF5WTJSbU9ESmpNMlExTVNJc0luUjVjR1VpT2pGOUxDSjFjMlZ5Ym1GdFpTSTZJbVZwYm5OMFpXbHVJaXdpYldGcGJDSTZJbVZwYm5OMFpXbHVRR1Y0WVcxd2JHVXViM0puSWl3aVpHbHpjR3hoZVY5dVlXMWxJam9pUVd4aVpYSjBJRVZwYm5OMFpXbHVJaXdpZFdsa1gyNTFiV0psY2lJNk1qQXdNREFzSW1kcFpGOXVkVzFpWlhJaU9qTXdNREF3ZlN3aWMyTnZjR1VpT25zaWRYTmxjaUk2ZXlKeVpYTnZkWEpqWlNJNmV5SmtaV052WkdWeUlqb2lhbk52YmlJc0luWmhiSFZsSWpvaVpYbEtkMWxZVW05SmFtOXBUSGxLT1NKOUxDSnliMnhsSWpveGZYMTkuV3AyWm1SNTRFb1BiQ2V6YmV2czBBVF9rRG9MX3dmQi16VXZEN1N2MWNfYyIsImZpbGVuYW1lIjoiL3VzZXJzLzRjNTEwYWRhLWM4NmItNDgxNS04ODIwLTQyY2RmODJjM2Q1MS9OZXcgZmlsZS5vZHQiLCJ1c2VybmFtZSI6ImVpbnN0ZWluIiwidmlld21vZGUiOiJWSUVXX01PREVfVklFV19PTkxZIiwiZm9sZGVydXJsIjoiJS8iLCJlbmRwb2ludCI6IjEyODRkMjM4LWFhOTItNDJjZS1iZGM0LTBiMDAwMDAwOTE1NyIsImFwcG5hbWUiOiJDb2xsYWJvcmEiLCJhcHBlZGl0dXJsIjoiaHR0cHM6Ly9jb2xsYWJvcmEub3duY2xvdWQudGVzdC9sb2xlYWZsZXQvNGFhMjc5NC9sb2xlYWZsZXQuaHRtbD8iLCJhcHB2aWV3dXJsIjoiIiwiZXhwIjoxNjI4OTI2NDYyfQ.TbLqB3kaM4Q8pGOPiJjnK9cDNRMXBdXXdPGIcjL9L94",
    "access_token_ttl": "1628926462"
  }
}

@wkloucek
Copy link
Contributor

Besides the things above that this PR looks good 👍 If this PR is merged we can also merge owncloud/ocis#2204

@wkloucek
Copy link
Contributor

wkloucek commented Aug 31, 2021

@labkode the fixes to the issues above are in https://github.com/cs3org/reva/pull/2024/files/f1f0a4ae831830ecf1dbe3c61b1d02c3a9340103..0e52867bd94afa5876514baaf3c6e591a503b4c4 and cs3org/wopiserver#46.

If you merge this PR, I can rebase my PR on master (it currently also contains @ishank011 changes)

@glpatcern
Copy link
Member

(Interesting that GitHub automatically closed this PR when I merged the related one on wopi...)

@wkloucek
Copy link
Contributor

wkloucek commented Sep 1, 2021

(Interesting that GitHub automatically closed this PR when I merged the related one on wopi...)

I referenced the issue comment and used the magic word fixes and it seems like it just picked up the pull request part from that url: github.com/cs3org/reva/pull/1968?#issuecomment-898253543

@wkloucek
Copy link
Contributor

wkloucek commented Sep 1, 2021

@labkode #2024 is now also green. If you merge this PR, I will make the other one ready for merging.

@labkode labkode merged commit e2f89ab into cs3org:master Sep 1, 2021
gmgigi96 pushed a commit to gmgigi96/reva that referenced this pull request Sep 1, 2021
wkloucek added a commit to owncloud/ocis that referenced this pull request Sep 1, 2021
@ishank011 ishank011 deleted the appprovider-http branch September 21, 2021 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants