Access control, uploads and (maybe) setting a bad example.

default discord avatar
richardvanbergen3 years ago
1 3

Questions

  1. Should access control be mentioned in the Upload section or should collections with Upload have automatic public read access?
  2. Does setting access.create () => true as per a couple of examples in the documentation allow public create permissions and if so, should that be discouraged or noted in the documentation?

Context

I was working with uploads and I couldn't figure out why when I was logged out - my media directory wasn't allowing me to see the static assets I uploaded.

I eventually figured out that it was that I need to allow for read permissions on the collection. Something that I couldn't see noted explicitly in the documentation. (Improvement maybe?)

As I was doing that I noticed an example I copied from the documentation earlier on my main "Pages" collection (kind of a page builder setup) and it seems to imply public access to creating pages, see bellow.

I don't have time to check right now but if it always returns true does that mean all access is allowed or does it use the default checking function mentioned here: https://payloadcms.com/docs/access-control/overview#default-settings

If doesn't use the default checking method, could that maybe be exposed so I don't have to write it myself and import it on every project where I don't need fine-grain access control?

  access: {
    create: () => true,
    read: () => true,
    update: () => true,
    delete: () => true,
  },
  • Selected Answer
    discord user avatar
    jmikrut
    3 years ago

    Hey @richardvanbergen — great questions! Here are a few answers for you.

    TL;DR:

    1. Yes, we can definitely mention access control in our upload docs. Good call. And I don't think we should necessarily open up access control to uploads by default, only because we think it's better to be secure by default and let end-users open up access control at their own discretion.
    2. Yes, access.create: () => true allows public creation of documents in the collection. We can add to our docs to make this more explicit as it should definitely be used only very deliberately.

    Now, here's a bit more info.

    For safety and security reasons, all access control is set to require that a user is logged in by default to do anything with your API. You need to manually set access control to public for everything, including upload data AND uploaded files, to even be readable by anonymous users.

    This documentation could be much more bold, or perhaps feature a warning banner to call emphasis to this fact. We could also clearly specify this under the Upload section. Good call!

    In many cases, setting access.create: () => true makes perfect sense for APIs. Here are a few different examples:

    1. Have a Customers auth collection, with access.create: () => true. This would allow end users of your app to register themselves a Customer account. This collection could be totally separate from an Admins auth collection or similar that gets many more permissions that you define, including maybe being used to access the Payload admin panel, whereas the Customers collection would be restricted.
    2. Have only one Users auth collection, but add a roles field to the collection that features a create access control function requiring the logged in user to already have a protected role of admin or similar. The pattern can be very powerful. This way, anyone can create a User, but only users with admin role can change or set the roles property on any given user.
    3. Maybe you wanted to have a collection for FormSubmissions or similar, where anyone could post directly to your API through a contact form. That would be a case where access.create: () => true would be totally appropriate.

    But in any case, public create permissions should be carefully used! We can definitely call this out more in docs.

    I eventually figured out that it was that I need to allow for read permissions on the collection. Something that I couldn't see noted explicitly in the documentation. (Improvement maybe?)

    Nailed it. We'll add this to the docs for sure. Good call.

    I don't have time to check right now but if it always returns true does that mean all access is allowed or does it use the default checking function mentioned here: https://payloadcms.com/docs/access-control/overview#default-settings

    If the function returns true, that means that anyone can perform the action that the function corresponds to. If you leave the access control property undefined, then the default function will be used.

    If doesn't use the default checking method, could that maybe be exposed so I don't have to write it myself and import it on every project where I don't need fine-grain access control?

    The default access control method is quite simple, and is below:

    const defaultAccess = ({ req: { user } }) => Boolean(user);

    We'll definitely add this to the docs for clarity but there should be no real reason you need to use it - as it's set automatically and used. To make use of it, all you need to do is not set any specific access control property!

    Does this clear things up for you? All GREAT questions. We appreciate it!

    1 reply
  • default discord avatar
    richardvanbergen3 years ago

    Thank you very much, sorry for taking up so much time with such a thorough answer! 😄

    One more very quick question (just trying to learn the APIs and I wasn't able to find an answer from scanning the code). If I do the following:

    access: {
       read: () => true
    }
    

    Does that mean all other other operations get the default access handler? I.e. is my configuration merged?

    Sorry for the long clarification asking again. I get paranoid when it comes to access control and the like.

  • discord user avatar
    jmikrut
    3 years ago

    Hey Richard, no worries! I wrote that up pretty quickly. At this point our team is so familiar with this code that it just flows. We'll keep working on improving our docs and your questions help us do that.

    Your example will only set the access control for the read operations (find, findByID). So, the others (create, update, delete, etc.) will remain set to the default access control which requires a user to be logged in.

    It's a GOOD thing to be paranoid about access control. That's a mark of a good developer.

  • default discord avatar
    mortockslast month

    Agree but I think the docs still need an update. I might be interpreting them wrong but the Access Control section states

    All files that are uploaded to each Collection automatically support the read Access Control function from the Collection itself. You can use this to control who should be allowed to see your uploads, and who should not.

    I intemperate this to mean that read is true and all other operations require auth but currently in my tests, you have the explicitly put

        access: {
            read: () => true,
        },
    

    on your collection to get this behaviour.

    I'm not advocating for a behaviour change, just checking that the documented behaviour and actual behaviour are the same. Perhaps

    All files that are uploaded to each Collection required authentication for all access control methods but they automatically support Access Control functions from the Collection itself. You can use this to control who should be allowed to see your uploads, and who should not.

Star on GitHub

Star

Chat on Discord

Discord

online

Can't find what you're looking for?

Get help straight from the Payload team with an Enterprise License.