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

[Sharing-NG]Secure viewer role id is missing in list permission #9128

Closed
Tracked by #8428
amrita-shrestha opened this issue May 10, 2024 · 11 comments · Fixed by #9188
Closed
Tracked by #8428

[Sharing-NG]Secure viewer role id is missing in list permission #9128

amrita-shrestha opened this issue May 10, 2024 · 11 comments · Fixed by #9188
Assignees
Labels

Comments

@amrita-shrestha
Copy link
Contributor

amrita-shrestha commented May 10, 2024

Describe the bug

Share project space with secure viewer permission and list permission of that project space. Api response doesn't contain role for secure viewer. Both list permission end point has same problem.

Steps to reproduce

  1. user Alice create project space new-space
  2. user Alice share project space new-space with user einstein with permission role Secure viewer
  3. Send API request to list permission with root and permission endpoint
    https://localhost:9200/graph/v1beta1/drives/8612d425-2786-446c-8ecb-78a36eeaeeaf$2d50cb6a-ff37-4683-aa06-59db537ef000/root/permissions
    https://localhost:9200/graph/v1beta1/drives/{dive-id}/items/{items-id}/permissions
  4. response doesn't contain secure viewer role

Expected behavior

....
        {
            "grantedToV2": {
                "user": {
                    "displayName": "Albert Einstein",
                    "id": "4c510ada-c86b-4815-8820-42cdf82c3d51"
                }
            },
            "id": "u:4c510ada-c86b-4815-8820-42cdf82c3d51"
            "role": "aa97fe03-7980-45ac-9e50-b325749fd7e6"
        }

Actual behavior

{
    "@libre.graph.permissions.actions.allowedValues": [
      .....
    "value": [
     ......
        {
            "grantedToV2": {
                "user": {
                    "displayName": "Albert Einstein",
                    "id": "4c510ada-c86b-4815-8820-42cdf82c3d51"
                }
            },
            "id": "u:4c510ada-c86b-4815-8820-42cdf82c3d51"
        }
    ]
}

Setup

Please describe how you started the server and provide a list of relevant environment variables or configuration files.

ownCloud	Infinite Scale
Edition	Community
Version	5.1.0-prealpha+ceeb4ed71d
Web client version	9.0.0-alpha.5

Additional context

Add any other context about the problem here.

@amrita-shrestha amrita-shrestha changed the title [Share-NG][Secure view]Secure viewer role id is missing in list permission [Sharing-NG]Secure viewer role id is missing in list permission May 10, 2024
@saw-jan
Copy link
Member

saw-jan commented May 10, 2024

Confirmed. No roles field for the drive shared with Secure viewer

CC @rhafer @JammingBen

@micbar
Copy link
Contributor

micbar commented May 10, 2024

@JammingBen @rhafer let us check that out.

@JammingBen
Copy link
Contributor

The issue (or parts of it) seems to be that the permissionsMap in https://github.com/owncloud/ocis/blob/master/services/graph/pkg/service/v0/base.go#L96 maps InitiateFileDownload to true, which should not be the case for the secure view role.

The map gets populated with contents from space.Opaque.Map["grants"] in the lines down below, I don't know how that works and where exactly this data comes from though.

@rhafer
Copy link
Contributor

rhafer commented May 15, 2024

Hm. I think I am hitting a wall here. There is no distinct access control value for the InitiateFileDownload grant. When the grants for a space are persisted to disk we get here at some point:

https://github.com/cs3org/reva/blob/7099ed49d7cc7090663c200963137ce834fc18b9/pkg/storage/utils/ace/ace.go#L439C1-L444C3

	if set.Stat || set.InitiateFileDownload || set.ListContainer || set.GetPath {
		b.WriteString("r")
	}

Which basically means that if any of the Grants Stat, InitiateFileDownload, ListContainer or GetPath is set they're reflected in a "r" permission on the file backend. So basically the information about the missing InitiateFileDownload permission is lost at this point. And when reading back the permissions to Grants the InitiateFileDownload grant will be set.

@micbar
Copy link
Contributor

micbar commented May 15, 2024

I see 👀

@butonic
Copy link
Member

butonic commented May 15, 2024

MS actually has a good list of ACE permissions: https://learn.microsoft.com/en-us/azure/azure-netapp-files/nfs-access-control-lists#nfsv4x-permissions

Discussing with @rhafer we see a solution by adding t for Read attributes (GETATTR) to represent the stat flag in the CS3 permissionset. A view only space share would then only have tx permissions which would be picked up by the assemble permission code. Files would only have the t permission.

We could even introduce a new version of the permissions encoding. The ace byte array starts with a type byte. We currently always use x00, which indicates a key/value encoded ace.

@micbar
Copy link
Contributor

micbar commented May 15, 2024

good idea. Let us implement it cleanly.

@butonic
Copy link
Member

butonic commented May 15, 2024

@rhafer sth like this: cs3org/reva#4685

@rhafer
Copy link
Contributor

rhafer commented May 15, 2024

We could even introduce a new version of the permissions encoding. The ace byte array starts with a type byte. We currently always use x00, which indicates a key/value encoded ace.

I think we currently can do without a new version and stay backwards compatible. I.e. we'd continue to decode r into set.Stat, set.InitiateFileDownload, set.ListContainer and set.GetPath. But only set r when the set contains InitiateFileDownload

@rhafer
Copy link
Contributor

rhafer commented May 16, 2024

@JammingBen #9188 should fix the problem. But now the webui is acting up as soon as I assign the SecureView role to a space member:

TypeError: a[b.roles[0]] is undefined
    mt https://localhost:9200/js/chunks/TextEditor.vue_vue_type_style_index_0_lang-CWCmf39G.mjs:1
    a https://localhost:9200/js/chunks/TextEditor.vue_vue_type_style_index_0_lang-CWCmf39G.mjs:1
    Dt https://localhost:9200/js/chunks/TextEditor.vue_vue_type_style_index_0_lang-CWCmf39G.mjs:1
    loadSpaces https://localhost:9200/js/chunks/TextEditor.vue_vue_type_style_index_0_lang-CWCmf39G.mjs:1
    b https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:25
    Eh https://localhost:9200/js/index.html-CrS3YD27.mjs:1
    Lu https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    rs https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    v https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    Lu https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    jq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    promise callback*Uq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    RS https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    y https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    wR https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    Sq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    Bp https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    set value https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    u https://localhost:9200/js/chunks/TextEditor.vue_vue_type_style_index_0_lang-CWCmf39G.mjs:1
    b https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:25
    updateAccessTokenPromise https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    updateContext https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    initializeContext https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    ld https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    Fc https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    Fc https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    runWithContext https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    A https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    We https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    promise callback*Pnr/We/< https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    We https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    x https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    promise callback*x https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    S https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    w https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    E https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    o https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    signInCallback https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    setup https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    rc https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    Lu https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    rs https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    __weh https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    l2 https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    jq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    promise callback*Uq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    RS https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    y https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    wR https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    Sq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    Bp https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    set value https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    I https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    S https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    promise callback*S https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    w https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    install https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    use https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    Eh https://localhost:9200/js/index.html-CrS3YD27.mjs:1
    runtimeLoaded https://localhost:9200/web-oidc-callback?code=bDz2BwSn6pNYg5TszYlu6xeiPrm1x87K&scope=openid profile email&session_state=45da5357e4c17913799c9346634ec64036da6a48d33a408e464729c65168f977.2DT1_dm2dAnHz-0lyKA1daDMXaj5nzfrjHiwrZAbSRs&state=5c82024a2c0547418dd4b061750c1d2a:141
    <anonymous> https://localhost:9200/js/index.html-CrS3YD27.mjs:1
vendor-Bcptkbbf.mjs:13:1930
    jde https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    xh https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    rs https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    (Async: promise callback)
    rs https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    v https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    Lu https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    jq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    (Async: promise callback)
    Uq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    RS https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    y https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    wR https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    Sq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    Bp https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    set value https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    u https://localhost:9200/js/chunks/TextEditor.vue_vue_type_style_index_0_lang-CWCmf39G.mjs:1
    b https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:25
    updateAccessTokenPromise https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    updateContext https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    initializeContext https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    ld https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    Fc https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    Fc https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    runWithContext https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    A https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    We https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    (Async: promise callback)
    We https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    We https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    x https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    (Async: promise callback)
    x https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    S https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    w https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    E https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    o https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    signInCallback https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    setup https://localhost:9200/js/chunks/authService-CDRVHcsw.mjs:1
    rc https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    Lu https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    rs https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    __weh https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    l2 https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    jq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    (Async: promise callback)
    Uq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    RS https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    y https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    wR https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    Sq https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    Bp https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    set value https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:9
    I https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    S https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    (Async: promise callback)
    S https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    w https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    install https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:66
    use https://localhost:9200/js/chunks/vendor-Bcptkbbf.mjs:13
    Eh https://localhost:9200/js/index.html-CrS3YD27.mjs:1
    runtimeLoaded https://localhost:9200/web-oidc-callback?code=bDz2BwSn6pNYg5TszYlu6xeiPrm1x87K&scope=openid profile email&session_state=45da5357e4c17913799c9346634ec64036da6a48d33a408e464729c65168f977.2DT1_dm2dAnHz-0lyKA1daDMXaj5nzfrjHiwrZAbSRs&state=5c82024a2c0547418dd4b061750c1d2a:141
    <anonymous> https://localhost:9200/js/index.html-CrS3YD27.mjs:1

butonic pushed a commit to rhafer/ocis that referenced this issue May 16, 2024
Fixes: owncloud#9128
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
butonic pushed a commit to rhafer/ocis that referenced this issue May 16, 2024
Fixes: owncloud#9128
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
rhafer added a commit to rhafer/ocis that referenced this issue May 16, 2024
Fixes: owncloud#9128
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@JammingBen
Copy link
Contributor

@JammingBen #9188 should fix the problem. But now the webui is acting up as soon as I assign the SecureView role to a space member:

owncloud/web#10925 fixes that.

rhafer added a commit to rhafer/ocis that referenced this issue May 16, 2024
Fixes: owncloud#9128
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
rhafer added a commit to rhafer/ocis that referenced this issue May 21, 2024
Fixes: owncloud#9128
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
rhafer added a commit to rhafer/ocis that referenced this issue May 21, 2024
To get: cs3org/reva#4685

Fixes: owncloud#9128
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
rhafer added a commit to rhafer/ocis that referenced this issue May 21, 2024
To get: cs3org/reva#4685

Fixes: owncloud#9128
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@github-project-automation github-project-automation bot moved this from In progress to Done in Infinite Scale Team Board May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants