github-education-resources / classroom

GitHub Classroom automates repository creation and access control, making it easy for teachers to distribute starter code and collect assignments on GitHub.
https://classroom.github.com
1.35k stars 569 forks source link

Creating classroom changes (without warning) organization's default repository permissions #1611

Open Blackbaud-BobbyEarl opened 6 years ago

Blackbaud-BobbyEarl commented 6 years ago

Describe the bug

When creating a classroom, the org.update_default_repository_permission is changed to none, as seen in: https://github.com/education/classroom/blob/cfa82c8608042709fc0ea806a5e18998f2393254/app/services/organization_default_repository_permission_migrator.rb#L73

We'd previously set this value to read and relied on this behavior. Once this change went in to affect, it removed all forks from users who didn't otherwise have explicit permissions to a repo.

Given https://education.github.community/t/default-repository-permissions-for-classroom-organization/16090/4, I could see why this step is helpful, but I feel like it should be very explicitly called out.

To Reproduce Steps to reproduce the behavior:

  1. Grant Classroom access to your org.
  2. Click "Create Classroom."
  3. Watch forks disappear.

Expected behavior Not having the org.update_default_repository_permission changed to none, or at a minimum warning me that I was about to make such a change.

johndbritton commented 6 years ago

Thanks for the report and apologies for the trouble.

Not having the org.update_default_repository_permission changed to none, or at a minimum warning me that I was about to make such a change.

This is required for Classroom to function. Without this setting repositories will not be kept private to individual students.

warning me that I was about to make such a change.

This makes sense. Additionally we should make it clear that we recommend using Classroom with a dedicated organization.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

BenEmdon commented 5 years ago

This might be a good candidate issue for the classroom roadmap CC: @d12 @education/classroom-reviewers

d12 commented 4 years ago

We got another report of this bug, if Classroom is setup with an established org with members and repositories, changing the default permissions can revoke access from a lot of people. At the very least, let's give the user a big warning when setting up Classroom on a repository with existing members and repositories.

cc @andrewbredow for triage.