Skip to content

Commit

Permalink
add ui tests, update settings, minor changes (#1367)
Browse files Browse the repository at this point in the history
  • Loading branch information
mikkonie committed Jul 10, 2024
1 parent 18092ec commit 41c3ae3
Show file tree
Hide file tree
Showing 12 changed files with 122 additions and 41 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Added
- **General**
- Python v3.11 support (#1157)
- Flake8 rule in ``Makefile`` (#1387)
- OpenID Connect (OIDC) authentication support (#1367)
- **Adminalerts**
- Admin alert email sending (#415)
- ``notify_email_alert`` app setting (#415)
Expand Down Expand Up @@ -43,7 +44,6 @@ Added
- Remote project access status updating in project detail view (#1358)
- ``SidebarContentAjaxView`` for sidebar and project dropdown content retrieval (#1366)
- ``UserDropdownContentAjaxView`` for user dropdown content retrieval (#1366, #1392)
- OpenID Connect (OIDC) authentication support (#1367)
- ``SODARUser.get_auth_type()`` helper (#1367)
- **Timeline**
- ``sodar_uuid`` field in ``TimelineEventObjectRef`` model (#1415)
Expand Down
2 changes: 1 addition & 1 deletion config/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
'markupfield', # For markdown
'rest_framework', # For API views
'knox', # For token auth
'social_django', # For OIDC authentication
'docs', # For the online user documentation/manual
'db_file_storage', # For filesfolders
'dal', # For user search combo box
Expand Down Expand Up @@ -441,7 +442,6 @@
ENABLE_OIDC = env.bool('ENABLE_OIDC', False)

if ENABLE_OIDC:
INSTALLED_APPS.append('social_django')
AUTHENTICATION_BACKENDS = tuple(
itertools.chain(
('social_core.backends.open_id_connect.OpenIdConnectAuth',),
Expand Down
2 changes: 1 addition & 1 deletion config/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@
# OpenID Connect (OIDC) configuration
# ------------------------------------------------------------------------------


ENABLE_OIDC = False


# Logging
# ------------------------------------------------------------------------------

Expand Down
5 changes: 2 additions & 3 deletions config/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
path('api/auth/', include('knox.urls')),
# Iconify SVG icons
path('icons/', include('dj_iconify.urls')),
# Social auth for OIDC support
path('social/', include('social_django.urls')),
# Projectroles URLs
path('project/', include('projectroles.urls')),
# Admin Alerts URLs
Expand Down Expand Up @@ -55,9 +57,6 @@
path('examples/site/', include('example_site_app.urls')),
] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)

if settings.ENABLE_OIDC:
urlpatterns.append(path('social/', include('social_django.urls')))

if settings.DEBUG:
# This allows the error pages to be debugged during development, just visit
# these url in browser to see how these error pages look like.
Expand Down
28 changes: 26 additions & 2 deletions docs/source/app_projectroles_settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,8 @@ This part of the setup is **optional**.
)
.. _app_projectroles_settings_oidc:

OpenID Connect (OIDC) Configuration (Optional)
==============================================

Expand All @@ -482,12 +484,34 @@ variables to your configuration.

This part of the setup is **optional**.

OIDC support is implemented using the ``social_django`` app. You first need to
add the app to your ``INSTALLED_APPS``:

.. code-block:: python
THIRD_PARTY_APPS = [
# ...
'social_django', # For OIDC authentication
]
Next, you must add the app's URL patterns in ``config/urls.py``:

.. code-block:: python
urlpatterns = [
# ...
# Social auth for OIDC support
path('social/', include('social_django.urls')),
]
Finally, you should add the following Django settings in your ``base.py``
settings file:

.. code-block:: python
ENABLE_OIDC = env.bool('ENABLE_OIDC', False)
if ENABLE_OIDC:
INSTALLED_APPS.append('social_django')
AUTHENTICATION_BACKENDS = tuple(
itertools.chain(
('social_core.backends.open_id_connect.OpenIdConnectAuth',),
Expand All @@ -512,7 +536,7 @@ This part of the setup is **optional**.
'SOCIAL_AUTH_OIDC_OIDC_ENDPOINT', None
)
SOCIAL_AUTH_OIDC_KEY = env.str('SOCIAL_AUTH_OIDC_KEY', 'CHANGEME')
SOCIAL_AUTH_OIDC_SECRET = env.str('SOCIAL_AUTH_OIDC_KEY', 'CHANGEME')
SOCIAL_AUTH_OIDC_SECRET = env.str('SOCIAL_AUTH_OIDC_SECRET', 'CHANGEME')
SOCIAL_AUTH_OIDC_USERNAME_KEY = env.str(
'SOCIAL_AUTH_OIDC_USERNAME_KEY', 'username'
)
Expand Down
29 changes: 19 additions & 10 deletions docs/source/major_changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,6 @@ General Python Dependencies
``requirements`` directory for up-to-date package versions and upgrade your
project.

SAML Authentication Support Removed
-----------------------------------

SAML support has been removed and replaced with the possibility to set up OpenID
Connect (OIDC) authentication. The library previously used for SAML in SODAR
Core is incompatible with Django v4.x. We are unaware of SODAR Core based
projects requiring SAML at this time. If there are specific needs to use SAML on
a SODAR Core based site, we are happy to review pull requests to reintroduce it.
Please note the implementation has to support Django v4.2+.

REST API Versioning Overhaul
----------------------------

Expand Down Expand Up @@ -277,6 +267,25 @@ Django Settings Changed
The ``PROJECTROLES_HIDE_APP_LINKS`` Django setting, which was deprecated in
v0.13, has been removed. Use ``PROJECTROLES_HIDE_PROJECT_APPS`` instead.

SAML Authentication Support Removed
-----------------------------------

SAML support has been removed and replaced with the possibility to set up OpenID
Connect (OIDC) authentication. The library previously used for SAML in SODAR
Core is incompatible with Django v4.x. We are unaware of SODAR Core based
projects requiring SAML at this time. If there are specific needs to use SAML on
a SODAR Core based site, we are happy to review pull requests to reintroduce it.
Please note the implementation has to support Django v4.2+.

OpenID Connect (OIDC) Authentication Support Added
--------------------------------------------------

This version adds OIDC support using the ``social_django`` app. In order to
provide OIDC authentication access to your users, you need to add the app and
its URLs to your site config along with appropriate Django settings. See
:ref:`OIDC settings documentation <app_projectroles_settings_oidc>` for
instructions on how to to configure OIDC on your site.

Login Template Updated
----------------------

Expand Down
6 changes: 3 additions & 3 deletions projectroles/templates/projectroles/user_form.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

{% block title %}
{% get_django_setting 'ENABLE_OIDC' as enable_oidc %}
{% if not object and enable_oidc %}Login or{% endif %}
{% if invite and enable_oidc %}Login or{% endif %}
{% if object %}Update{% else %}Create{% endif %} Local User
{% endblock title %}

Expand All @@ -17,13 +17,13 @@
<div class="container-fluid sodar-subtitle-container">
<h2>
<i class="iconify" data-icon="mdi:account"></i>
{% if not object and enable_oidc %}Login or{% endif %}
{% if invite and enable_oidc %}Login or{% endif %}
{% if object %}Update{% else %}Create{% endif %} Local User
</h2>
</div>

<div class="container-fluid sodar-page-container">
{% if not object and enable_oidc %}
{% if invite and enable_oidc %}
<div class="col-md-4 mt-1 mb-4 ml-0 pl-0">
<p>Please log in if you have an existing account.</p>
{% url 'projectroles:invite_process_ldap' secret=invite.secret as oidc_redirect_url %}
Expand Down
42 changes: 41 additions & 1 deletion projectroles/tests/test_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -2361,7 +2361,47 @@ def test_invite_preview(self):
)


# TODO: Test OIDC element and URL on local invite process view
class TestProjectInviteProcessLocalView(ProjectInviteMixin, UITestBase):
"""Tests for ProjectInviteProcessLocalView UI"""

def setUp(self):
super().setUp()
self.invite = self.make_invite(
email='[email protected]',
project=self.project,
role=self.role_contributor,
issuer=self.user_owner,
message='',
)
self.url = self.build_selenium_url(
reverse(
'projectroles:invite_process_local',
kwargs={'secret': self.invite.secret},
)
)

def test_get(self):
"""Test ProjectInviteProcessLocalView GET"""
# NOTE: No login
self.selenium.get(self.url)
with self.assertRaises(NoSuchElementException):
self.selenium.find_element(By.ID, 'sodar-login-oidc-link')
elem = self.selenium.find_element(
By.CLASS_NAME, 'sodar-btn-submit-once'
)
self.assertEqual(elem.text, 'Create')

@override_settings(ENABLE_OIDC=True)
def test_get_oidc(self):
"""Test GET with OIDC authentication enabled"""
self.selenium.get(self.url)
self.assertIsNotNone(
self.selenium.find_element(By.ID, 'sodar-login-oidc-link')
)
elem = self.selenium.find_element(
By.CLASS_NAME, 'sodar-btn-submit-once'
)
self.assertEqual(elem.text, 'Create')


class TestRemoteSiteListView(RemoteSiteMixin, UITestBase):
Expand Down
4 changes: 4 additions & 0 deletions projectroles/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4275,6 +4275,7 @@ def test_post_local_users_email_domain_ldap(self):
self.assertIsNotNone(invite)


# TODO: Refactor and cleanup tests
class TestProjectInviteAcceptView(
ProjectMixin, RoleAssignmentMixin, ProjectInviteMixin, ViewTestBase
):
Expand Down Expand Up @@ -4601,6 +4602,7 @@ def test_get_local(self):
kwargs={'secret': invite.secret},
),
)
self.assertEqual(response.context['invite'], invite)
email = response.context['form']['email'].value()
username = response.context['form']['username'].value()
self.assertEqual(email, invite.email)
Expand Down Expand Up @@ -4972,6 +4974,8 @@ def test_get_role_exists(self):
TimelineEvent.objects.filter(event_name='invite_accept').first()
)

# TODO: Test redirecting to login view if OIDC enabled but local isn't


class TestProjectInviteListView(
ProjectMixin, RoleAssignmentMixin, ProjectInviteMixin, ViewTestBase
Expand Down
2 changes: 1 addition & 1 deletion projectroles/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
),
path(
route='invites/process/ldap/<str:secret>',
view=views.ProjectInviteProcessLDAPView.as_view(),
view=views.ProjectInviteProcessLoggedInView.as_view(),
name='invite_process_ldap',
),
path(
Expand Down
38 changes: 22 additions & 16 deletions projectroles/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,24 +104,22 @@
get_display_name(PROJECT_TYPE_CATEGORY, plural=True)
)
USER_PROFILE_UPDATE_MSG = 'User profile updated, please log in again.'
USER_PROFILE_LDAP_MSG = 'Error: Profile editing not allowed for LDAP users.'
USER_PROFILE_LDAP_MSG = 'Profile editing not allowed for LDAP users.'
INVITE_TYPE_LDAP = 'LDAP'
INVITE_TYPE_LOCAL_OIDC = 'LOCAL-OIDC'
INVITE_LDAP_LOCAL_VIEW_MSG = (
'Error: Invite was issued for LDAP user, but local invite view '
'was requested.'
)
INVITE_LOCAL_NOT_ALLOWED_MSG = (
'Error: Invite of non-LDAP user, but local users are not allowed.'
'Invite was issued for LDAP user, but local invite view was requested.'
)
INVITE_LOCAL_NOT_ALLOWED_MSG = 'Local users are not allowed.'

INVITE_LOGGED_IN_ACCEPT_MSG = (
'Error: Logged in user is not allowed to accept invites for other users.'
'Logged in user is not allowed to accept invites for other users.'
)
INVITE_USER_NOT_EQUAL_MSG = (
'Error: Invited user exists, but logged in user is not invited user.'
'Invited user exists, but logged in user is not invited user.'
)
INVITE_USER_EXISTS_MSG = (
'A user with that email already exists. Please login to accept the invite.'
'User with that email already exists. Please login to accept the invite.'
)
ROLE_CREATE_MSG = 'Membership granted with the role of "{role}".'
ROLE_UPDATE_MSG = 'Member role changed to "{role}".'
Expand Down Expand Up @@ -2764,12 +2762,12 @@ def get(self, *args, **kwargs):
return redirect(reverse('home'))


# TODO: Refactor this into a "logged-in-process-view", redirect from local user
# handling and OIDC login?
class ProjectInviteProcessLDAPView(
class ProjectInviteProcessLoggedInView(
LoginRequiredMixin, ProjectInviteProcessMixin, View
):
"""View to handle accepting a project LDAP invite"""
"""
View to handle accepting a project invite from a logged in LDAP/OIDC user.
"""

def get(self, *args, **kwargs):
invite = self.get_invite(kwargs['secret'])
Expand Down Expand Up @@ -2829,8 +2827,16 @@ def get(self, *args, **kwargs):
if not invite:
return redirect(reverse('home'))
timeline = get_backend_api('timeline_backend')
# Check if local users are enabled
if not settings.PROJECTROLES_ALLOW_LOCAL_USERS:
# Redirect to logged in process view if OIDC is enabled but local isn't
if settings.ENABLE_OIDC and not settings.PROJECTROLES_ALLOW_LOCAL_USERS:
return redirect(
reverse(
'projectroles:invite_process_ldap',
kwargs={'secret': invite.secret},
)
)
# If local users are not allowed but OIDC is, redirect to home
elif not settings.PROJECTROLES_ALLOW_LOCAL_USERS:
messages.error(self.request, INVITE_LOCAL_NOT_ALLOWED_MSG)
return redirect(reverse('home'))
# Check invite for correct type
Expand All @@ -2846,7 +2852,7 @@ def get(self, *args, **kwargs):

# A user is not logged in
if self.request.user.is_anonymous:
# Redirect to login if user exists and
# Redirect to login if user exists
if user:
messages.info(
self.request,
Expand Down
3 changes: 1 addition & 2 deletions requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ pytz>=2024.1
# SVG icon support
django-iconify==0.3 # NOTE: v0.4 requires Python>=3.10

# OpenID authentication support
# TODO: Move to separate file or not?
# OpenID Connect (OIDC) authentication support
social-auth-app-django>=5.4.0, <5.5

# Online documentation via django-docs
Expand Down

0 comments on commit 41c3ae3

Please sign in to comment.