diff --git a/CHANGELOG.md b/CHANGELOG.md index 23de516c21..4e39906b83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added + +- Allow all users created via shibboleth to have their own playlist + ## [5.1.0] - 2024-10-09 ### Added diff --git a/src/backend/marsha/account/social_pipeline/playlist.py b/src/backend/marsha/account/social_pipeline/playlist.py index 69ae36504a..65a36f93fb 100644 --- a/src/backend/marsha/account/social_pipeline/playlist.py +++ b/src/backend/marsha/account/social_pipeline/playlist.py @@ -3,7 +3,10 @@ from django.conf import settings from django.db.transaction import atomic +from waffle import switch_is_active + from marsha.account.models import IdpOrganizationAssociation +from marsha.core.defaults import ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES from marsha.core.models import ADMINISTRATOR, INSTRUCTOR, Playlist, PlaylistAccess @@ -90,13 +93,15 @@ def create_playlist_from_saml( # pylint:disable=too-many-arguments # Only create a new playlist for new user association. return - is_instructor = details.get("roles", None) and any( - role in details["roles"] for role in settings.SOCIAL_AUTH_SAML_FER_TEACHER_ROLES - ) + if not switch_is_active(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES): + is_instructor = details.get("roles", None) and any( + role in details["roles"] + for role in settings.SOCIAL_AUTH_SAML_FER_TEACHER_ROLES + ) - # If the user has no instructor role we don't create a new playlist. - if not is_instructor: - return + # If the user has no instructor role we don't create a new playlist. + if not is_instructor: + return idp = backend.get_idp(response["idp_name"]) diff --git a/src/backend/marsha/account/tests/test_social_pipeline_playlist.py b/src/backend/marsha/account/tests/test_social_pipeline_playlist.py index 476f9842bb..d590347837 100644 --- a/src/backend/marsha/account/tests/test_social_pipeline_playlist.py +++ b/src/backend/marsha/account/tests/test_social_pipeline_playlist.py @@ -5,10 +5,12 @@ from django.test import TestCase from social_django.utils import load_strategy +from waffle.testutils import override_switch from marsha.account.factories import IdpOrganizationAssociationFactory from marsha.account.social_pipeline.playlist import create_playlist_from_saml from marsha.account.tests.utils import MockedFERSAMLAuth, override_saml_fer_settings +from marsha.core.defaults import ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES from marsha.core.factories import PlaylistAccessFactory, UserFactory from marsha.core.models import ADMINISTRATOR, INSTRUCTOR, Playlist, PlaylistAccess @@ -58,6 +60,7 @@ def setUp(self) -> None: self.assertEqual(Playlist.objects.count(), 0) self.assertEqual(PlaylistAccess.objects.count(), 0) + @override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False) def test_create_playlist_from_saml_student_or_unknown_new(self): """No playlist should be created when the user is a student.""" @@ -75,6 +78,7 @@ def test_create_playlist_from_saml_student_or_unknown_new(self): self.assertEqual(Playlist.objects.count(), 0) self.assertEqual(PlaylistAccess.objects.count(), 0) + @override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False) def test_create_playlist_no_new_association(self): """When new_association is False, no playlist should be created.""" saml_details = {"roles": ["teacher"], **self.default_saml_details} @@ -91,6 +95,7 @@ def test_create_playlist_no_new_association(self): self.assertEqual(Playlist.objects.count(), 0) self.assertEqual(PlaylistAccess.objects.count(), 0) + @override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False) def test_create_playlist_new_association_no_existing_idporganization(self): """With a new association but no existing IdpOrganizationAssociation an exception should be raised""" @@ -107,6 +112,7 @@ def test_create_playlist_new_association_no_existing_idporganization(self): new_association=True, ) + @override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False) def test_create_playlist_new_association_existing_idporganization(self): """A new playlist should be created when new_association is True and an idpOrganizationAssociation exists.""" @@ -136,6 +142,7 @@ def test_create_playlist_new_association_existing_idporganization(self): self.assertEqual(playlist_access.user, self.user) self.assertEqual(playlist_access.role, ADMINISTRATOR) + @override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False) def test_create_playlist_new_association_existing_idp_organization_user_full_name( self, ): @@ -182,6 +189,7 @@ def test_create_playlist_new_association_existing_idp_organization_user_full_nam self.assertEqual(playlist_access.user, user) self.assertEqual(playlist_access.role, ADMINISTRATOR) + @override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False) def test_create_playlist_already_existing_playlist_access(self): """No playlist should be created if an existing playlist access for the user and the organization is found.""" @@ -211,3 +219,48 @@ def test_create_playlist_already_existing_playlist_access(self): self.assertEqual(Playlist.objects.count(), 1) self.assertEqual(PlaylistAccess.objects.count(), 1) + + @override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=True) + def test_create_playlist_student_active_flag( + self, + ): + """A new playlist hsould be created for a student when the flag is active.""" + + saml_details = { + "roles": ["student"], + "first_name": "jon", + "last_name": "snow", + "username": "jsnow@samltest.id", # unused + "email": "jsnow@samltest.id", # unused + } + + user = UserFactory( + first_name=saml_details["first_name"], + last_name=saml_details["last_name"], + username=saml_details["username"], + email=saml_details["email"], + ) + + idp_organization_association = IdpOrganizationAssociationFactory( + idp_identifier="http://marcha.local/idp/" + ) + + create_playlist_from_saml( + None, # strategy is unused + saml_details, + self.backend, + user=user, + response=self.saml_response, + new_association=True, + ) + + playlist = Playlist.objects.last() + playlist_access = PlaylistAccess.objects.last() + + self.assertEqual( + playlist.organization, idp_organization_association.organization + ) + self.assertEqual(playlist.title, user.get_full_name()) + self.assertEqual(playlist_access.playlist, playlist) + self.assertEqual(playlist_access.user, user) + self.assertEqual(playlist_access.role, ADMINISTRATOR) diff --git a/src/backend/marsha/core/defaults.py b/src/backend/marsha/core/defaults.py index 7186f0b5cb..2bce1d6cb2 100644 --- a/src/backend/marsha/core/defaults.py +++ b/src/backend/marsha/core/defaults.py @@ -95,6 +95,7 @@ RENATER_FER_SAML = "renater_fer_saml" VOD_CONVERT = "vod_convert" TRANSCRIPTION = "transcription" +ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES = "allow_playlist_creation_for_all_roles" # LTI view states expected by the frontend LTI application APP_DATA_STATE_ERROR = "error"