docs: ADR for publisher external users access #3923

Ali-D-Akbar · 2023-05-03T22:38:03Z

PROD-3327
ADR for granting access to external users in Publisher consumed APIs.

ashultz0 · 2023-05-09T14:06:33Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+=========
+
+A user group will be created for each product line.
+Each group will have a product source(s) and course type(s) associated to them.


There could be a lot of work and maintenance hiding in these two lines.

How are groups created and maintained? Whose job is that?

Thinking about storing the metadata information for each group/role, a better approach seems to be using rbac here. I'm going to revisit that.

ashultz0 · 2023-05-09T14:07:16Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+This has been an internal tool that only allows internal staff users to do these actions.
+With the inclusion of other product lines in the Catalog, there has been a need to allow external users on Publisher so that they can view and edit their respective courses.
+Currently, these courses are added through weekly ingestion and there are no plans to add them through Publisher.
+These external users should be able to view and edit their own courses only and the legacy courses should be kept hidden from them.


Are they really external users or just other division of 2U users? If they're 2U users how much control do we need to apply to them? If the product source and course type is enough, would if be sufficient to expose those as filters for staff level users?

They can be both but to visit Publisher, they need to have staff access. In order for them to view, create and edit courses, they need to have a role called PUBLISHER_EDITOR. We will restrict the external users to only perform actions on their own courses with the help of course_type and product_source.

DawoudSheraz · 2023-05-16T13:14:45Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+========
+
+Frontend App Publisher (MFE) uses course APIs in Discovery to view, create and edit courses.
+This has been an internal tool that only allows internal django staff users.


Incorrect, CourseEditors are not Django staff users.

DawoudSheraz · 2023-05-16T13:15:26Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+Frontend App Publisher (MFE) uses course APIs in Discovery to view, create and edit courses.
+This has been an internal tool that only allows internal django staff users.
+With the inclusion of other product lines in the Catalog, there has been a need to allow external users to be able to perform actions like viewing and updating their courses through Publisher.
+These external users should have staff access to be able to visit Publisher and should only be able to view and edit their own courses only and the legacy courses should be kept hidden from them.


Again, if this staff access is Django staff user, it is incorrect.

Just to have permissions to visit publisher, we do need staff (not django staff) access, right?

Course Editor is more accurate in Publisher/Discovery terms.

DawoudSheraz · 2023-05-16T13:16:49Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+Currently, these courses are added through weekly ingestion and the external users should not be able to add them through Publisher.
+


These are internal 2U details. Rather, you can link how CSV & Degree loaders are used for ingestion of these products.

DawoudSheraz · 2023-05-16T13:17:38Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+* A feature-based role will be created for each product line in Discovery.
+* The role will be assigned to external users within Discovery. The role can be assigned by an exposed endpoint. The role assignment will contain other information, such as the assignment date, the reason for giving access, access history, etc.
+* Publisher will make an additional API call to get the users' assigned role. Based upon the assignment, the behavior of publisher will change.
+* A mapping of the allowed product_source(s) and course_type(s) for each role will be added in edx-internal. Courses will be filtered based on these product sources and course types that a user's role is associated to.


This can be done on model level, like it was done for Source model.

Which model will it be? Some config model or in course?
These mappings are specific to the user role in rbac.

The roles will be in their dedicated tables, inhering from UserRole (from edx_rbac.models import UserRole). https://github.com/openedx/edx-platform/blob/master/openedx/core/djangoapps/system_wide_roles/models.py.

The role model, to be added, can contain the above mentioned information.

Oh yes. That is true. Lemme update this.

DawoudSheraz · 2023-05-16T13:18:18Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+* Using Django User group, assign the users the new group, and make decisions based on the group. This approach, however, falls short in adding metadata of the role assignment.
+* Using a Config Model to save mapping for each user group. The decision was changed to rather use rbac instead of user groups hence this approach isn't usable anymore. We will keep the mapping of each role in edx-internal.


Add more context to these, please.

DawoudSheraz · 2023-05-19T11:03:31Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+Status
+=======
+
+Accepted (May 2023)


lets keep it In-Progress for now.

DawoudSheraz · 2023-05-19T11:04:47Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+Context
+========
+
+Frontend App Publisher (MFE) uses course APIs in Discovery to view, create and edit courses.


Course, CourseRun, Organization, Collaborator, Staff -- there are different APIs. Just stick with Discovery APIs to avoid confusion.

DawoudSheraz · 2023-05-19T11:05:42Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+Frontend App Publisher (MFE) uses course APIs in Discovery to view, create and edit courses.
+This has been an internal tool that only allows Course Editors to perform these actions.
+With the inclusion of other product lines in the Catalog, there has been a need to allow external users to be able to perform actions like viewing and updating their courses through Publisher.
+These external users should have Course Editor access to be able to visit Publisher and should only be able to view and edit their own courses only and the legacy courses should be kept hidden from them.


Instead of legacy, mention they should have access to their own source products.

DawoudSheraz · 2023-05-19T11:06:17Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+With the inclusion of other product lines in the Catalog, there has been a need to allow external users to be able to perform actions like viewing and updating their courses through Publisher.
+These external users should have Course Editor access to be able to visit Publisher and should only be able to view and edit their own courses only and the legacy courses should be kept hidden from them.
+Currently, these products are created and added in Discovery using `csv_loader`_ and `degrees_loader`_
+The external users should not be able to create these products through Publisher.


To add -> A note that Degrees are not authored on Publisher

Programs aren't support for the moment this is being written. Is this really necessary to add this here?

It will be good for historical purposes.

DawoudSheraz · 2023-05-19T11:06:49Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+ .. _csv_loader: https://github.com/openedx/course-discovery/blob/master/course_discovery/apps/course_metadata/data_loaders/csv_loader.py#L32
+ .. _degrees_loader: https://github.com/openedx/course-discovery/blob/master/course_discovery/apps/course_metadata/data_loaders/degrees_loader.py#L25


nit: No need to pin to a specific line.

DawoudSheraz · 2023-05-19T11:07:40Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+The access checks will be implemented using role based authorization using `edx-rbac`_
+
+* A feature-based role will be created for each product line in Discovery. Each role will have product_source(s) and course_type(s) which will be used in permissions and filtering.
+* The role will be assigned to external users within Discovery. The role can be assigned by an exposed endpoint. The role assignment will contain other information, such as the assignment date, the reason for giving access, access history, etc.


The role will be accessed by an exposed endpoint and that user would only be able to see its assigned role.

DawoudSheraz · 2023-05-19T11:08:55Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

+
+The access checks will be implemented using role based authorization using `edx-rbac`_
+
+* A feature-based role will be created for each product line in Discovery. Each role will have product_source(s) and course_type(s) which will be used in permissions and filtering.


Examples here would be +1

What kind of examples? example of how a role would look like for a usecase?

Showcase what a role would look like and if we can add in multiple sources and/or types in a single role.

DawoudSheraz · 2023-05-19T11:09:39Z

docs/decisions/0023-publisher-externnal-source-course-filtering.rst

@@ -0,0 +1,42 @@
+23. External Source based course filtering for Publisher


The file has incorrect spelling for external (externnal)

DawoudSheraz

A few followup nit comments

zawan-ila · 2023-05-23T06:53:09Z

docs/decisions/0023-publisher-external-source-course-filtering.rst

+Alternates Considered
+-----------------------
+
+* Using Django User group, assign the users the new group, and make decisions based on the group. Additional information like product_source and course_type specific to each user group was to be added in the django groups. This approach, however, falls short in adding metadata of the role assignment.


What is meant by metadata of the assignment?

To keep data related to each role assignment like when was it added, who added it, reason to assign role etc

DawoudSheraz · 2023-09-26T05:47:28Z

Closing this as this is out of scope and wont be needed anytime soon.

Ali-D-Akbar force-pushed the aakbar/PROD-3327 branch 2 times, most recently from b6b3d56 to 21a19fc Compare May 5, 2023 10:07

ashultz0 reviewed May 9, 2023

View reviewed changes

Ali-D-Akbar force-pushed the aakbar/PROD-3327 branch 2 times, most recently from 0ca5eb6 to 8ec390b Compare May 15, 2023 07:08

Ali-D-Akbar marked this pull request as ready for review May 15, 2023 07:50

Ali-D-Akbar changed the title ~~feat: filter queryset based on source and user group~~ docs: ADR for publisher external users access May 15, 2023

Ali-D-Akbar force-pushed the aakbar/PROD-3327 branch 3 times, most recently from 291d94c to 026eb08 Compare May 15, 2023 18:47

Ali-D-Akbar requested review from AliAdnanSohail, DawoudSheraz and zawan-ila May 16, 2023 07:19

DawoudSheraz reviewed May 16, 2023

View reviewed changes

Ali-D-Akbar force-pushed the aakbar/PROD-3327 branch 2 times, most recently from 742e6bf to 1a60982 Compare May 18, 2023 13:21

Ali-D-Akbar requested a review from DawoudSheraz May 19, 2023 09:46

DawoudSheraz reviewed May 19, 2023

View reviewed changes

docs: ADR for publisher external users access

9be67c4

Ali-D-Akbar force-pushed the aakbar/PROD-3327 branch from 1a60982 to 9be67c4 Compare May 19, 2023 20:04

zawan-ila approved these changes May 23, 2023

View reviewed changes

docs: ADR updates

bb754a4

Ali-D-Akbar marked this pull request as draft May 30, 2023 19:09

DawoudSheraz closed this Sep 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: ADR for publisher external users access #3923

docs: ADR for publisher external users access #3923

Ali-D-Akbar commented May 3, 2023 •

edited

Loading

ashultz0 May 9, 2023

Ali-D-Akbar May 10, 2023

ashultz0 May 9, 2023

Ali-D-Akbar May 10, 2023

DawoudSheraz May 16, 2023

DawoudSheraz May 16, 2023

Ali-D-Akbar May 17, 2023

DawoudSheraz May 18, 2023

DawoudSheraz May 16, 2023

DawoudSheraz May 16, 2023

Ali-D-Akbar May 18, 2023

DawoudSheraz May 18, 2023

Ali-D-Akbar May 18, 2023

DawoudSheraz May 16, 2023

DawoudSheraz May 19, 2023

DawoudSheraz May 19, 2023

DawoudSheraz May 19, 2023

DawoudSheraz May 19, 2023

Ali-D-Akbar May 19, 2023

DawoudSheraz May 22, 2023

DawoudSheraz May 19, 2023

DawoudSheraz May 19, 2023

DawoudSheraz May 19, 2023

Ali-D-Akbar May 19, 2023

DawoudSheraz May 22, 2023

DawoudSheraz May 19, 2023

DawoudSheraz left a comment

zawan-ila May 23, 2023

Ali-D-Akbar May 30, 2023

DawoudSheraz commented Sep 26, 2023

		Currently, these courses are added through weekly ingestion and the external users should not be able to add them through Publisher.

		* Using Django User group, assign the users the new group, and make decisions based on the group. This approach, however, falls short in adding metadata of the role assignment.
		* Using a Config Model to save mapping for each user group. The decision was changed to rather use rbac instead of user groups hence this approach isn't usable anymore. We will keep the mapping of each role in edx-internal.

		.. _csv_loader: https://github.com/openedx/course-discovery/blob/master/course_discovery/apps/course_metadata/data_loaders/csv_loader.py#L32
		.. _degrees_loader: https://github.com/openedx/course-discovery/blob/master/course_discovery/apps/course_metadata/data_loaders/degrees_loader.py#L25


		The access checks will be implemented using role based authorization using `edx-rbac`_

		* A feature-based role will be created for each product line in Discovery. Each role will have product_source(s) and course_type(s) which will be used in permissions and filtering.

		@@ -0,0 +1,42 @@
		23. External Source based course filtering for Publisher

docs: ADR for publisher external users access #3923

docs: ADR for publisher external users access #3923

Conversation

Ali-D-Akbar commented May 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DawoudSheraz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DawoudSheraz commented Sep 26, 2023

Ali-D-Akbar commented May 3, 2023 •

edited

Loading