[BB-3622] Add course creation condition for organisation

[farhaanbukhsh]

Product Change Description

Current State

Studio, as of today doesn't have a way to restrict a user to create a course in a particular organization. What Studio provides right now is a CourseCreator permission which gives an Admin the power to grant a user the permission to create a course.

For example: If the Admin has given a user Spiderman the permission to create courses, Spiderman can now create courses in any organization i.e Marvel as well as DC.

There is no way to restrict Spiderman from creating courses under DC.

Purpose of this PR

The changes done here gives Admin the ability to restrict a user on an Organization level from creating courses,

For example: Now, the Admin can give the user Spiderman the privilege of creating courses only under Marvel organization. The moment Spiderman tries to create a course under some other organization(i.e DC), we are showing the following error message:

Admin's Responsibility

With this PR the Admin can give users the permission to create courses under any organization or under a specific set of organizations.

If Admin checks the All Organization checkbox then the user will be granted permission to create courses under any organization.

If Admin adds specific organizations under the selected lists then the user will only be able to create courses under those organizations.

There are few edge cases that can be encountered and below are the screenshot of how we inform the Admin/User about it

When All Organization is unchecked and no Organization is added to the list

When All Organization is checked and specific Organization are added as well

Error message when the user enters organization name for which user doesn't have permission

Engineering Implementation

The condition added to ensure that if the feature is enabled
user will not be able to create the course outside of the organisation
in which they belong.

JIRA tickets: BB-3622

Discussions: Link to any public dicussions about this PR or the design/architecture. Otherwise omit this.

Dependencies: None

Screenshots:

Sandbox URL: TBD - sandbox is being provisioned.

Merge deadline: "None" if there's no rush, "ASAP" if it's critical, or provide a specific date if there is one.

Testing instructions:

1. Get the master devstack up and running
2. Pull this branch in edx-platform
3. Make sure to run the migration
4. Drop in the studio shell make dev.shell.studio
5. vim /edx/etc/studio.yml and set ENABLE_CREATOR_GROUP = True
   The below changes are also required to be done

diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py
index 9d610b31f0..b548167a35 100644
--- a/cms/envs/devstack.py
+++ b/cms/envs/devstack.py
@@ -152,7 +152,7 @@ CREDENTIALS_SERVICE_USERNAME = 'credentials_worker'
 FEATURES['CERTIFICATES_HTML_VIEW'] = True
 
 ########################## AUTHOR PERMISSION #######################
-FEATURES['ENABLE_CREATOR_GROUP'] = False
+FEATURES['ENABLE_CREATOR_GROUP'] = True
 
 ################### FRONTEND APPLICATION PUBLISHER URL ###################
 FEATURES['FRONTEND_APP_PUBLISHER_URL'] = 'http://localhost:18400'
diff --git a/cms/envs/devstack_decentralized.py b/cms/envs/devstack_decentralized.py
index f7d61d41a1..f115c9e56a 100644
--- a/cms/envs/devstack_decentralized.py
+++ b/cms/envs/devstack_decentralized.py
@@ -107,7 +107,7 @@ CREDENTIALS_SERVICE_USERNAME = 'credentials_worker'
 FEATURES['CERTIFICATES_HTML_VIEW'] = True
 
 ########################## AUTHOR PERMISSION #######################
-FEATURES['ENABLE_CREATOR_GROUP'] = False
+FEATURES['ENABLE_CREATOR_GROUP'] = True
 
 ################### FRONTEND APPLICATION PUBLISHER URL ###################
 FEATURES['FRONTEND_APP_PUBLISHER_URL'] = 'http://localhost:18400'

5. In the FEATURES dictionary add RESTRICT_NON_ORG_COURSE_CREATION: true
6. Now create a user
7. Go to the admin page and in localhost:18010/admin/student/courseaccessrole/ create a new entry, add the user, give it instructor privilege, don't add the Org yet
8. Log in the studio and request for permission to create the course
9. Log in with a user who is a staff and superuser in another browser(edx on devstack)
10. Go to http://localhost:18010/admin/course_creators/coursecreator/
11. Here you can see the permission, all_organization and organization field
12. If you enable all_organzitaions field it will create a CourseCreator Role and works accordingly(The old flow)
13. If you disable all_organization field then you need to add Organizations, this will create OrganiztionContentCreator Role for that organization
14. Then you can try creating a course and in the Org field you need to enter org short name(Same as the one you gave permission to create in the previous role), this is case sensitive field and it will create the course
15. If any other Org is entered, it will not create the course and will through error
16. Another area that can be tested is on enabling all_organization field, course_content_group role s created in http://localhost:18010/admin/student/courseaccessrole/ and on disabling it and adding org, OrgContentCreator role is created.

Additional Manual Tests

1. In the course creator admin, change the status to granted and check if roles are created
2. Change the status to anything but granted and check if roles gets removed, roles can be checked in http://localhost:18010/admin/student/courseaccessrole/

Author notes and concerns:

1. It's a in discovery feature
2. The update to m2m field is non atomic operation, hence https://timonweb.com/django/many-to-many-field-save-method-and-the-django-admin-caveat/ was taken in consideration to obtain the desired result.

Reviewers

• @lgp171188
• @xitij2000

[openedx-webhooks]

Thanks for the pull request, @farhaanbukhsh! I've created OSPR-5631 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[natabene]

@farhaanbukhsh Thank you for your contribution. Please let me know once it is ready for our review.

[lgp171188]

👍 Can you remove the [WIP] tag from the PR and mark it ready by moving it out of the draft status?

• I tested the changes in this PR when reviewing [BB-3622] Restrict user create course open-craft/edx-platform#319 and verified that:
  • Superusers and global staff users can always create courses in any organization with or without this feature flag enabled.
  • When the feature flag is enabled, in addition to having the course creation permission via course creator groups (enabled by default), the user should also have a role (any role would suffice as the user is already a course creator) in the course's organization to be able to create a course under an organization.
  • When the feature flag is disabled (the current and default behavior), the existing behavior allowing course creator users to create courses in all organizations is preserved.
• I read through the code
• I checked for accessibility issues NA
• Includes documentation NA

[natabene]

@farhaanbukhsh Is this ready for edX review?

[farhaanbukhsh]

@natabene this PR is ready for review. :) Thanks for your patience.

[kdmccormick]

Thanks for your patience @farhaanbukhsh , I'll take a look at this soon!

[kdmccormick]

First off, I think this feature is a great addition to CMS. It seems like a no-brainer that we should be able to restrict content creation based on org. Thanks for submitting this PR!

My concern is that we are adding it in a way that is orthogonal to the current content creation auth scheme. As far as I can tell, the current scheme is essentially:

• if DISABLE_{COURSE,LIBRARY}_CREATION, reject all content creation, except for global staff.
• if ENABLE_CREATOR_GROUP, check if the user has the CourseCreatorRole role.
• Otherwise, allow all content creation.

But by building the org-based creation restriction using has_studio_write_access, we are instead checking the Instructor and Staff roles, which seem to be intended for already-created content. The resulting flow is that a complicated interaction between several different roles would determine one's effective creation permissions.

I believe it would be better to instead build this on top of the existing CourseCreatorRole and the related access checking functions. What do you think?

[farhaanbukhsh]

Hey @kdmccormick, thanks a lot for reviewing the code. I agree with the fact that the flow is a bit complicated. I am just wondering what can be a better approach?

As I see it we check CourseCreator permission here.

So we introduce a check here, if the user has a CourseCreator role then it goes ahead and does a check that the user should be present in an organization using the courseaccessrole model.

Am I understanding this correct? Thanks a lot for the help here again :)

[kdmccormick]

@farhaanbukhsh Right, so there is a CMS model called CourseCreator that tracks status of a user's request to be able to create content in Studio. I could see us adding a nullable organization column, foreign-keying to organizations.models.Organization.

That model raises a signal which is caught in course_creators/views.py, which updates the CourseCreatorRole. You could modify these functions so that if organization is defined, you instead update a newly-defined OrgCourseCreatorRole (or, better yet, OrgContentCreatorRole, since this role will allow library creation as well).

That CourseCreatorRole is defined in student/roles.py as a subclass of RoleBase. Similarly, you could define OrgCourseCreatorRole there, much like OrgStaff is defined.

Finally, in contentstore/views/{course,library}, you could update the permissions checks to look at both CourseCreatorRole (for global permissions) and OrgCourseCreatorRole (for org-specific permissions). As a bonus, you don't even need a new feature flag for this -- you can just use the existing COURSE_CREATOR_GROUP flag, since this change should be backwards-compaible.

[farhaanbukhsh]

@kdmccormick Thanks for the elaborate explanation, this looks like a legitimate good solution. I will try to implement this and ping once done 😄

[farhaanbukhsh]

@kdmccormick

Right, so there is a CMS model called CourseCreator that tracks status of a user's request to be able to create content in Studio. I could see us adding a nullable organization column, foreign-keying to organizations.models.Organization.

A Many to many field makes more sense here, what do you say? Since a course creator can be a part of many organizations.

[kdmccormick]

@farhaanbukhsh

A Many to many field makes more sense here, what do you say? Since a course creator can be a part of many organizations.

Good point! How do you suggest we indicate that a CourseCreator has global creation access, though? Treating an empty many-to-many relation as "global access" seems dicey; it would be very surprising if removing every org from the relation gave you access to create courses under every org!

Perhaps: a boolean all_organizations field, which we could backfill as True, so that existing entries continue to be treated as global access.

[farhaanbukhsh]

@kdmccormick

Perhaps: a boolean all_organizations field, which we could backfill as True, so that existing entries continue to be treated as global access.

This looks like a good approach for now where we can use the boolean, to toggle between CourseCreator Role and OrgContentCreator Role.

So if the boolean is False, we don't create CoureCreatorRole for the user instead for each Organization we give the user an OrgContentCreator role. If the boolean is switched we just need to add/remove ContentCreatorRole accordingly.

Hope I am understanding this correctly. I am working on this and will push the changes soon. :)

[kdmccormick]

@farhaanbukhsh Sounds right to me!

[farhaanbukhsh]

@kdmccormick I have rebased the PR, feel free to use the commit message while doing a Squash+Merge. :) Thanks for the amazing reviews and a crazy lot of patience. :)

[edx-status-bot]

Your PR has finished running tests. There were no failures.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.

[kdmccormick]

I added notes about this new feature to the Maple release notes page.