Netlify-cms: Don't allow non repo members into admin

Created on 24 Mar 2017  路  12Comments  路  Source: netlify/netlify-cms

- Do you want to request a feature or report a bug?

  • bug

- What is the current behaviour?

  • If I log in as a GitHub user that is not a member of the repository, I am able to see part of the admin interface. I am even able to go to the collection editing page and fill out the form. I get an error when I attempt to save the new collection entry, but it seems odd that anyone could log into my admin and have limited access.

screen shot 2017-03-23 at 9 38 27 pm

screen shot 2017-03-23 at 9 39 17 pm

- If the current behaviour is a bug, please provide the steps to reproduce.

  • Log in as a GitHub user that is not a member of the repository

- What is the expected behaviour?

  • I would expect that a user would get an error message saying they are not a member of the repository, and then not aloud to see the admin interface.

- Please mention your node.js, and operating system version.

  • Node: v6.10.1
  • macOS Sierra
design good first issue bug

Most helpful comment

GitHub Permission Checking APIs: https://developer.github.com/v3/repos/collaborators/

All 12 comments

Ultimately, the interface is only showing information that is publicly available in the config.yml, so it's not a security concern. However, I think it would be more user friendly to display an error message instead of a non-functioning UI.

GitHub Permission Checking APIs: https://developer.github.com/v3/repos/collaborators/

I was just coming to ask about this. Actually found out by mistake as a collaborator hadn't joined properly. So he was seeing API errors and I was off down a rabbit hole till I saw this.

It's a priority for sure. I don't expect it would be a very heavy lift if anyone wants to take a crack at it.

Not very familiar with React -- would you put this in AuthenticationPage.js or API.js?

Does the same issue apply to the netlify-auth backend?

If it's just GitHub, I think the best place to put it would be in Authenticator in lib/netlify-auth.js, and callback with an error if the user cannot write to the repo.

Good point @josephearl, my only thought was that since we may add GitLab support eventually, we might want to keep the implementations separated.

Yes, the same fix might need to be applied twice. The github backend uses Authenticator from lib/netlify-auth.js. The netlify-auth backend uses NetlifyAuthClient from netlify-auth-js (an npm package).

Need design for denied screen; when a user tries to access/login but they do not have push access to the repo

Would it make sense to also add some kind of .htaccess or or other permissions solution to deny the general public access to the config.yml file?

I'm gonna try and figure out an implementation for both of these asap as I'd like to make sure this is all taken care of before I deploy anything to production.

Actually, @Benaiah is working on #341, which should result in the config file only containing backend details (which must be publicly accessible), while the remainder of the config would be fetched from the repo. So public access of config.yml shouldn't be a problem.

Unless anyone has any issues, I'm going to close this.

Was this page helpful?
0 / 5 - 0 ratings