Skip to content

Regression bug in v5: Need to check for a file's ACL when it is served as resource for a container #1063

Closed

Description

@megoth

When a file is served as response to dereferencing a container, we also need to check that file's ACL settings when determining whether a user has access to that resource.

How it works on v4: as a visitor I have read access to both https://megoth.solid.community/ and https://megoth.solid.community/index.html
But on v5: as a visitor I have read access to https://megoth-test1.dev.inrupt.net/index.html, but not to https://megoth-test1.dev.inrupt.net/

Activity

  1. self-assigned this
    on Jan 24, 2019
  2. kjetilk commented on Jan 24, 2019

    @kjetilk
    Member

    So, thinking about it, I'm thinking that we should be using /.acl even if the server serves up /index.html.

    The default ACL file says / is public, so could you double check that your /.acl says that?

  3. megoth commented on Jan 25, 2019

    @megoth
    ContributorAuthor

    Sorry, forgot to answer this.

    The template for root ACL is

    # Root ACL resource for the user account
    @prefix acl: <http://www.w3.org/ns/auth/acl#>.
    
    <#owner>
        a acl:Authorization;
    
        acl:agent <{{webId}}> ;
    
        # Optional owner email, to be used for account recovery:
        {{#if email}}acl:agent <mailto:{{{email}}}>;{{/if}}
    
        # Set the access to the root storage folder itself
        acl:accessTo </>;
    
        # All resources will inherit this authorization, by default
        acl:default </>;
    
        # The owner has all of the access modes allowed
        acl:mode
            acl:Read, acl:Write, acl:Control.
    
    # Data is private by default; no other agents get access unless specifically
    # authorized in other .acls
    

    So the default ACL file for root says that all files are private.

  4. megoth commented on Jan 25, 2019

    @megoth
    ContributorAuthor

    So making the root folder publicly available is a solution for this, that is changing default_templates/new-account/.acl to:

    # Root ACL resource for the user account
    @prefix acl: <http://www.w3.org/ns/auth/acl#>.
    @prefix foaf: <http://xmlns.com/foaf/0.1/>.
    
    <#owner>
        a acl:Authorization;
    
        acl:agent <{{webId}}> ;
    
        # Optional owner email, to be used for account recovery:
        {{#if email}}acl:agent <mailto:{{{email}}}>;{{/if}}
    
        # Set the access to the root storage folder itself
        acl:accessTo </>;
    
        # All resources will inherit this authorization, by default
        acl:default </>;
    
        # The owner has all of the access modes allowed
        acl:mode
            acl:Read, acl:Write, acl:Control.
    
    # Data is private by default; no other agents get access unless specifically
    # authorized in other .acls
    
    # The public has read permissions to root folder
    <#public>
        a acl:Authorization;
        acl:agentClass foaf:Agent;
        acl:accessTo <./>;
        acl:mode acl:Read.
    

    This does not give access all other files in the root folder, so I think this the "right way" of fixing this.

    The problem with this approach (which I think is correct) is that users with outdated root ACLs will need to upgrade their root ACL if they want their homepage be publicly available. The reason this works in v4.x seems to actually be a bug...

    We might want to create a migration script for POD providers to run when upgrading to v5. Any thoughts on this?

  5. csarven commented on Jan 25, 2019

    @csarven
    Member

    Can you elaborate on "This does not give access all other files in the root folder"? meanwhile #public with acl:Read?

    Why not keep private by default and give read to the index or whatever you want explicitly?

  6. mikeadams1 commented on Jan 25, 2019

    @mikeadams1

    I am also confused here as well because, @megoth I thought you stated that the acl issue with regards to home pages being public was an err in NSSv5 but, it appears that your position now is that it was a bug in v4.x that the home pages were public and should not have been, is this correct?

  7. mikeadams1 commented on Jan 25, 2019

    @mikeadams1

    If I am mistaking the discussion here, just let me know.

  8. RubenVerborgh commented on Jan 25, 2019

    @RubenVerborgh
    Contributor

    What is being passed to the ACL checker? / should be passed in the above case, not /index.html.

  9. megoth commented on Jan 25, 2019

    @megoth
    ContributorAuthor

    I am also confused here as well because, @megoth I thought you stated that the acl issue with regards to home pages being public was an err in NSSv5 but, it appears that your position now is that it was a bug in v4.x that the home pages were public and should not have been, is this correct?

    My understanding of the problem evolves, so my hypothesis was something else initially. Is also why I'm stating how I understanding of the problem here, in hopes of getting feedback and maybe correct me if I'm still not understanding the problem correctly.

  10. megoth commented on Jan 25, 2019

    @megoth
    ContributorAuthor

    Can you elaborate on "This does not give access all other files in the root folder"? meanwhile #public with acl:Read?

    Why not keep private by default and give read to the index or whatever you want explicitly?

    Just to note, this ties into solid/web-access-control-spec#36 as well. I tried changing the last part of the root-ACL to:

    <#public>
        a acl:Authorization;
        acl:agentClass foaf:Agent;
        acl:accessTo <./index.html>;
        acl:mode acl:Read.
    

    That doesn't work, I'm afraid =/

    As I have understood the ACL-spec, I need to make the root-folder readable to make it accessable to the public. But the previously mentioned change would only make / readable, not other resources that resides in root (e.g. /test.txt will not be readable unless you have /test.txt.acl that says it should be).

    If I had added acl:default as well, that would be different, i.e.

    <#public>
        a acl:Authorization;
        acl:agentClass foaf:Agent;
        acl:accessTo <./>;
        acl:mode acl:Read.
    

    That would be different, as it would make all resources in root (and other sub-folders that doesn't have an .acl) publicly readable.

    Does that make it clearer?

  11. megoth commented on Jan 25, 2019

    @megoth
    ContributorAuthor

    What is being passed to the ACL checker? / should be passed in the above case, not /index.html.

    Yes, we're passing /; my initial approach to this issue was actually to change it to pass /index.html (or /index.ttl or whichever resource that is "actually" passed to the client) to the ACL checker, but if I understand the ACL spec correctly, that is not how we should do it.

    Am I understanding that part correctly?

  12. RubenVerborgh commented on Jan 25, 2019

    @RubenVerborgh
    Contributor

    Yes, we're passing /

    Is the problem specific to just /, or do we have the same situation with /foo/ and /foo/index.html?

    but if I understand the ACL spec correctly, that is not how we should do it.

    Indeed.

  13. megoth commented on Jan 25, 2019

    @megoth
    ContributorAuthor

    Another effect of this is that you can actually make the root publicly readable, but not the default file that the server ends up serving (by root ACL stating that / is readable, but index.html.acl says that /index.html is not publicly available), e.g. you could visit https://megoth.solid.community/ but not https://megoth.solid.community/index.html.

  14. megoth commented on Jan 25, 2019

    @megoth
    ContributorAuthor

    Yes, we're passing /

    Is the problem specific to just /, or do we have the same situation with /foo/ and /foo/index.html?

    /foo/ (and /foo) and /foo/index.html is not publicly readable by default. (Neither with my proposed change to state that / is publicly readable by all.)

  15. RubenVerborgh commented on Jan 25, 2019

    @RubenVerborgh
    Contributor

    /foo/ (and /foo) and /foo/index.html is not publicly readable by default.

    Indeed, but what I mean is: if we set similar permissions on them, would they have the same problems? I guess /public/ and /public/index.html are in that case (if an index.html exists).

  16. 22 remaining items

  17. kjetilk commented on Jan 28, 2019

    @kjetilk
    Member

    BTW, @RubenVerborgh which repo should get an issue to get the deprecation warning and "Set permission" button?

  18. RubenVerborgh commented on Jan 28, 2019

    @RubenVerborgh
    Contributor

    Depends on where we want to show it. I think NSS, should probably login redirect page.

  19. kjetilk commented on Jan 28, 2019

    @kjetilk
    Member

    Right, we can do that... I was also thinking about some of the other UIs, e.g. databrowser?

  20. RubenVerborgh commented on Jan 28, 2019

    @RubenVerborgh
    Contributor

    A general mechanism for server notifications would be great for sure. I'm thinking #1057.

    Databrowser, was also thinking that. But maybe too specific to couple server to this.

    The Solid way would be to send something to people's inboxes 🙂

  21. mikeadams1 commented on Jan 28, 2019

    @mikeadams1

    "The Solid way would be to send something to people's inboxes 🙂"

    That sounds like a great resolution, and no one can say that they did not receive notice.

  22. kjetilk commented on Jan 28, 2019

    @kjetilk
    Member

    OK! I think we could use the login page, as everyone is on that every now and then.

  23. mikeadams1 commented on Jan 28, 2019

    @mikeadams1

    On second thought, the login page would probably be a great place for notices.

  24. Quantum-Bot commented on Jan 29, 2019

    @Quantum-Bot

    I am very happy to read here that everyone will have a choice if their index.html is visible or not via the button I suggested to control that setting in gitter.
    Thank you to everyone for giving this issue attention and all the hard work everyone contributes to this project.

  25. megoth commented on Jan 29, 2019

    @megoth
    ContributorAuthor

    I've implemented a hack so that the index.html-algorithm works, but I have some questions reg the button-part.

    There are three scenarios that we might want to consider:

    1. The user wants to grant READ access to root: We need to grant public READ access to '/' in /.acl; We can present this option with the button that has been discussed
    2. The user wants to make homepage private explicitly: We need to remove public READ access to index.html on `index.html.acl; Maybe we should give a button for this as well?
    3. The user doesn't care, just wants to remove the message: We need to store that dismissal somewhere; Maybe have a button that dismisses the message, so that the user doesn't have to be bothered. We might store this in users' preferences, but there's really no property to describe that, so would have to create something (right?).

    We could also "force" the user to make a choice by only allowing option 1 and 2. Also, I reckon this section is at the bottom of the homepage, so wouldn't be up in the users' face. (Or do we want to have if up front, to "force" them to make a choice?)

    This section could show only when there's a discrepancy between READ access to '/' and '/index.html', which makes it so that once an option is chosen, the section won't show anymore (unless the user does something with the READ access on either files later, of course).

  26. megoth commented on Jan 29, 2019

    @megoth
    ContributorAuthor

    I see that people have suggested that the button should live in the login-page? I'm confused as to how that would work? Or am I misunderstanding something?

  27. megoth commented on Jan 29, 2019

    @megoth
    ContributorAuthor

    To give people an idea of what I'm thinking wrt two buttons (make public, make private):

      <section>
        <h1>Make this page explicitly public or private</h1>
        <p>We have detected that this page have a discrepancy when handling access. The details might be a bit confusing, but you can <a href="#">read more about it in the forum</a>. To keep it simple, we ask you to consider whether you want to make this page explicitly available for the public.</p>
        <form>
          <div class="form-group">
            <p>By making the page public you can view this page more as a normal webpage, that anyone can access. They still won't get access to resources that requires login, like the private folder.</p>
            <button class="btn btn-primary">Make page public</button>
          </div>
        </form>
        <form>
          <div class="form-group">
            <p>By making the page private you make sure that only you can see this page, and it requires you to login if you don't have a valid session.</p>
            <button class="btn btn-primary">Make page private</button>
          </div>
        </form>
      </section>
    

    This would only be shown to users that have control of this POD.

    The idea is to also link to a thread in the forum that explains the situation in more details.

  28. megoth commented on Jan 29, 2019

    @megoth
    ContributorAuthor

    "Or, could we have an explicit sunset on this hack? I.e., we do this for 5.0.0, but warn our users that they need to set read permission on their root, because this will be removed in the future?"

    Good idea. Practical problem would be lack of a place to show such a notification.

    Even better perhaps is combining this with something I read earlier: making this a button "Set permission" so it is zero effort.

    I am wondering where we should have this button. If I put it in /index.html we need to run the update POD script that was implemented in #953, but as #1030 discuss, this might be problematic.

  29. kjetilk commented on Jan 29, 2019

    @kjetilk
    Member

    I think they should decide, as not deciding will cause problems when the hack is retired.

    I think it is a good idea to have two buttons though, so that they can choose.

    When you say it, I'm not sure what the others meant by "login"-page, because when you see that, you're not yet logged in, I suppose.

  30. kjetilk commented on Jan 30, 2019

    @kjetilk
    Member

    I have merged the backend aspects of this an released a new beta, and made a new ticket for the frontend buttons in #1076 , so we can close this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions