Skip to content

Commit

Permalink
Merge branch 'feature/institutional_access' of https://github.com/Cen…
Browse files Browse the repository at this point in the history
…terForOpenScience/osf.io into feature/institutional_access

* 'feature/institutional_access' of https://github.com/CenterForOpenScience/osf.io:
  Changes for "can_request_access" feature (CenterForOpenScience#10877)
  • Loading branch information
John Tordoff committed Jan 9, 2025
2 parents afc7e26 + d9540f3 commit e8c3287
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 8 deletions.
1 change: 1 addition & 0 deletions api/institutions/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class InstitutionSerializer(JSONAPISerializer):
ser.CharField(read_only=True),
permission='view_institutional_metrics',
)
institutional_request_access_enabled = ser.CharField(read_only=True)
links = LinksField({
'self': 'get_api_url',
'html': 'get_absolute_html_url',
Expand Down
3 changes: 3 additions & 0 deletions api/requests/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ def has_permission(self, request, view):
except Institution.DoesNotExist:
raise exceptions.ValidationError({'institution': 'Institution is does not exist.'})

if not institution.institutional_request_access_enabled:
raise exceptions.PermissionDenied({'institution': 'Institutional request access is not enabled.'})

if get_user_auth(request).user.is_institutional_admin(institution):
return True
else:
Expand Down
2 changes: 1 addition & 1 deletion api/users/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,6 @@ def has_permission(self, request, view) -> bool:

message_type = request.data.get('message_type')
if message_type == MessageTypes.INSTITUTIONAL_REQUEST:
return user.is_institutional_admin(institution)
return user.is_institutional_admin(institution) and institution.institutional_request_access_enabled
else:
raise exceptions.ValidationError('Not valid message type.')
62 changes: 56 additions & 6 deletions api_tests/requests/views/test_node_request_institutional_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ def url(self, project):

@pytest.fixture()
def institution(self):
return InstitutionFactory()
return InstitutionFactory(institutional_request_access_enabled=True)

@pytest.fixture()
def institution2(self):
def institution_without_access(self):
return InstitutionFactory()

@pytest.fixture()
Expand All @@ -30,6 +30,12 @@ def user_with_affiliation(self, institution):
user.add_or_update_affiliated_institution(institution)
return user

@pytest.fixture()
def user_with_affiliation_on_institution_without_access(self, institution_without_access):
user = AuthUserFactory()
user.add_or_update_affiliated_institution(institution_without_access)
return user

@pytest.fixture()
def user_without_affiliation(self):
return AuthUserFactory()
Expand All @@ -40,6 +46,12 @@ def institutional_admin(self, institution):
institution.get_group('institutional_admins').user_set.add(admin_user)
return admin_user

@pytest.fixture()
def institutional_admin_on_institution_without_access(self, institution_without_access):
admin_user = AuthUserFactory()
institution_without_access.get_group('institutional_admins').user_set.add(admin_user)
return admin_user

@pytest.fixture()
def create_payload(self, institution, user_with_affiliation):
return {
Expand All @@ -66,6 +78,32 @@ def create_payload(self, institution, user_with_affiliation):
}
}

@pytest.fixture()
def create_payload_on_institution_without_access(self, institution_without_access, user_with_affiliation_on_institution_without_access):
return {
'data': {
'attributes': {
'comment': 'Wanna Philly Philly?',
'request_type': NodeRequestTypes.INSTITUTIONAL_REQUEST.value,
},
'relationships': {
'institution': {
'data': {
'id': institution_without_access._id,
'type': 'institutions'
}
},
'message_recipient': {
'data': {
'id': user_with_affiliation_on_institution_without_access._id,
'type': 'users'
}
}
},
'type': 'node-requests'
}
}

@pytest.fixture()
def node_with_disabled_access_requests(self, institution):
node = NodeFactory()
Expand Down Expand Up @@ -112,6 +150,18 @@ def test_institutional_admin_can_add_requested_permission(self, app, project, in
assert node_request.request_type == NodeRequestTypes.INSTITUTIONAL_REQUEST.value
assert node_request.requested_permissions == 'admin'

def test_institutional_admin_can_not_add_requested_permission(self, app, project, institutional_admin_on_institution_without_access, url, create_payload_on_institution_without_access):
"""
Test that an institutional admin can not make an institutional access request on institution with disabled access .
"""
create_payload_on_institution_without_access['data']['attributes']['requested_permissions'] = 'admin'

res = app.post_json_api(
url, create_payload_on_institution_without_access, auth=institutional_admin_on_institution_without_access.auth, expect_errors=True
)

assert res.status_code == 403

def test_institutional_admin_needs_institution(self, app, project, institutional_admin, url, create_payload):
"""
Test that the payload needs the institution relationship and gives the correct error message.
Expand All @@ -133,16 +183,16 @@ def test_institutional_admin_invalid_institution(self, app, project, institution
assert res.status_code == 400
assert 'Institution is does not exist.' in res.json['errors'][0]['detail']

def test_institutional_admin_unauth_institution(self, app, project, institution2, institutional_admin, url, create_payload):
def test_institutional_admin_unauth_institution(self, app, project, institution_without_access, institutional_admin, url, create_payload):
"""
Test that the view authenticates the relationship between the institution and the user and gives the correct
error message when it's unauthorized.'
error message when it's unauthorized
"""
create_payload['data']['relationships']['institution']['data']['id'] = institution2._id
create_payload['data']['relationships']['institution']['data']['id'] = institution_without_access._id

res = app.post_json_api(url, create_payload, auth=institutional_admin.auth, expect_errors=True)
assert res.status_code == 403
assert 'You do not have permission to perform this action for this institution.' in res.json['errors'][0]['detail']
assert 'Institutional request access is not enabled.' in res.json['errors'][0]['detail']

@mock.patch('api.requests.serializers.send_mail')
def test_email_not_sent_without_recipient(self, mock_mail, app, project, institutional_admin, url,
Expand Down
41 changes: 41 additions & 0 deletions api_tests/users/views/test_user_message_institutional_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
InstitutionFactory
)
from website.mails import USER_MESSAGE_INSTITUTIONAL_ACCESS_REQUEST
from webtest import AppError


@pytest.mark.django_db
Expand All @@ -17,6 +18,10 @@ class TestUserMessageInstitutionalAccess:

@pytest.fixture()
def institution(self):
return InstitutionFactory(institutional_request_access_enabled=True)

@pytest.fixture()
def institution_without_access(self):
return InstitutionFactory()

@pytest.fixture()
Expand All @@ -33,16 +38,32 @@ def user_with_affiliation(self, institution):
user.add_or_update_affiliated_institution(institution)
return user

@pytest.fixture()
def user_with_affiliation_on_institution_without_access(self, institution_without_access):
user = AuthUserFactory()
user.add_or_update_affiliated_institution(institution_without_access)
return user

@pytest.fixture()
def institutional_admin(self, institution):
admin_user = AuthUserFactory()
institution.get_group('institutional_admins').user_set.add(admin_user)
return admin_user

@pytest.fixture()
def institutional_admin_on_institution_without_access(self, institution_without_access):
admin_user = AuthUserFactory()
institution_without_access.get_group('institutional_admins').user_set.add(admin_user)
return admin_user

@pytest.fixture()
def url_with_affiliation(self, user_with_affiliation):
return f'/{API_BASE}users/{user_with_affiliation._id}/messages/'

@pytest.fixture()
def url_with_affiliation_on_institution_without_access(self, user_with_affiliation_on_institution_without_access):
return f'/{API_BASE}users/{user_with_affiliation_on_institution_without_access._id}/messages/'

@pytest.fixture()
def url_without_affiliation(self, user):
return f'/{API_BASE}users/{user._id}/messages/'
Expand Down Expand Up @@ -89,6 +110,26 @@ def test_institutional_admin_can_create_message(self, mock_send_mail, app, insti
assert 'Requesting user access for collaboration' in mock_send_mail.call_args[1]['message_text']
assert user_message._id == data['id']

@mock.patch('osf.models.user_message.send_mail')
def test_institutional_admin_can_not_create_message(self, mock_send_mail, app, institutional_admin_on_institution_without_access,
institution_without_access, url_with_affiliation_on_institution_without_access,
payload):
"""
Ensure an institutional admin cannot create a `UserMessage` with a `message` and `institution` witch has 'institutional_request_access_enabled' as False
"""
mock_send_mail.return_value = mock.MagicMock()

# Use pytest.raises to explicitly expect the 403 error
with pytest.raises(AppError) as exc_info:
app.post_json_api(
url_with_affiliation_on_institution_without_access,
payload,
auth=institutional_admin_on_institution_without_access.auth
)

# Assert that the raised error contains the 403 Forbidden status
assert '403 Forbidden' in str(exc_info.value)

def test_unauthenticated_user_cannot_create_message(self, app, user, url_with_affiliation, payload):
"""
Ensure that unauthenticated users cannot create a `UserMessage`.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 4.2.13 on 2024-12-12 19:37
# Generated by Django 4.2.15 on 2025-01-08 12:24

from django.conf import settings
from django.db import migrations, models
Expand All @@ -14,6 +14,11 @@ class Migration(migrations.Migration):
]

operations = [
migrations.AddField(
model_name='institution',
name='institutional_request_access_enabled',
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name='noderequest',
name='requested_permissions',
Expand Down
1 change: 1 addition & 0 deletions osf/models/institution.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ class Institution(DirtyFieldsMixin, Loggable, ObjectIDMixin, BaseModel, Guardian
related_name='institutions'
)

institutional_request_access_enabled = models.BooleanField(default=False)
is_deleted = models.BooleanField(default=False, db_index=True)
deleted = NonNaiveDateTimeField(null=True, blank=True)
deactivated = NonNaiveDateTimeField(null=True, blank=True)
Expand Down
1 change: 1 addition & 0 deletions osf_tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ class InstitutionFactory(DjangoModelFactory):
email_domains = FakeList('domain_name', n=1)
orcid_record_verified_source = ''
delegation_protocol = ''
institutional_request_access_enabled = False

class Meta:
model = models.Institution
Expand Down

0 comments on commit e8c3287

Please sign in to comment.