From b27f057724e177f860d72652999148a017ddfda9 Mon Sep 17 00:00:00 2001 From: Dimitris Papagiannis Date: Fri, 14 Jul 2023 15:23:02 +0200 Subject: [PATCH 1/7] ffff --- .github/workflows/django.yml | 7 ++++--- .s2i/bin/assemble | 7 ++++--- requirements.txt | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/.github/workflows/django.yml b/.github/workflows/django.yml index e406cd5f..1dfcb375 100644 --- a/.github/workflows/django.yml +++ b/.github/workflows/django.yml @@ -42,9 +42,10 @@ jobs: git config --global url."https://${{ secrets.CERN_GITLAB_USER }}:${{ secrets.CERN_GITLAB_TOKEN }}@gitlab.cern.ch".insteadOf https://gitlab.cern.ch python -m pip install --upgrade pip - mkdir -p /tmp/runner - git clone https://github.com/nothingface0/django-allauth.git /tmp/runner/django-allauth - cd /tmp/runner/django-allauth + ALLAUTH_INSTALL_DIR=/home/runner + mkdir -p $ALLAUTH_INSTALL_DIR + git clone https://github.com/nothingface0/django-allauth.git $ALLAUTH_INSTALL_DIR/django-allauth + cd $ALLAUTH_INSTALL_DIR/django-allauth git checkout 77368a8 wget https://github.com/pennersr/django-allauth/compare/main...nothingface0:django-allauth:fix_cern_sso.patch --output-document=patch git apply patch --reject || true diff --git a/.s2i/bin/assemble b/.s2i/bin/assemble index 72063d1f..5162d080 100644 --- a/.s2i/bin/assemble +++ b/.s2i/bin/assemble @@ -3,9 +3,10 @@ echo "Before assembling" git config --global url."https://$CERN_GITLAB_USER:$CERN_GITLAB_TOKEN@gitlab.cern.ch".insteadOf https://gitlab.cern.ch # TODO: remove once PR in django-allauth merged -mkdir -p /tmp/runner -git clone https://github.com/nothingface0/django-allauth.git /tmp/runner/django-allauth -cd /tmp/runner/django-allauth +ALLAUTH_INSTALL_DIR=/home/runner +mkdir -p $ALLAUTH_INSTALL_DIR +git clone https://github.com/nothingface0/django-allauth.git $ALLAUTH_INSTALL_DIR/django-allauth +cd $ALLAUTH_INSTALL_DIR/django-allauth git checkout 77368a8 wget https://github.com/pennersr/django-allauth/compare/main...nothingface0:django-allauth:fix_cern_sso.patch --output-document=patch git apply patch --reject || true diff --git a/requirements.txt b/requirements.txt index 98b17d60..50d27e88 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ channels[daphne] channels_redis Django<4.2.0 # TODO: Update this once PR is merged to django-allauth --e /tmp/runner/django-allauth +-e /home/runner/django-allauth #django-allauth<1.0.0 django-bootstrap3<24.0 django-filter<24.0 From a413e8dd8d9d79fecf63c53fd52abb98330ebaaf Mon Sep 17 00:00:00 2001 From: Dimitris Papagiannis Date: Fri, 14 Jul 2023 15:27:38 +0200 Subject: [PATCH 2/7] fff --- .github/workflows/django.yml | 6 +++--- .s2i/bin/assemble | 4 ++-- requirements.txt | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/django.yml b/.github/workflows/django.yml index 1dfcb375..6c76c271 100644 --- a/.github/workflows/django.yml +++ b/.github/workflows/django.yml @@ -42,7 +42,7 @@ jobs: git config --global url."https://${{ secrets.CERN_GITLAB_USER }}:${{ secrets.CERN_GITLAB_TOKEN }}@gitlab.cern.ch".insteadOf https://gitlab.cern.ch python -m pip install --upgrade pip - ALLAUTH_INSTALL_DIR=/home/runner + ALLAUTH_INSTALL_DIR=$HOME mkdir -p $ALLAUTH_INSTALL_DIR git clone https://github.com/nothingface0/django-allauth.git $ALLAUTH_INSTALL_DIR/django-allauth cd $ALLAUTH_INSTALL_DIR/django-allauth @@ -50,9 +50,9 @@ jobs: wget https://github.com/pennersr/django-allauth/compare/main...nothingface0:django-allauth:fix_cern_sso.patch --output-document=patch git apply patch --reject || true cd - - + pip install -r requirements.txt - + pip install $ALLAUTH_INSTALL_DIR/django-allauth pip install --index-url https://test.pypi.org/simple runregistry==1.0.0 pip install --upgrade pytest pytest-django pytest-cov codecov mixer selenium diff --git a/.s2i/bin/assemble b/.s2i/bin/assemble index 5162d080..c05234b9 100644 --- a/.s2i/bin/assemble +++ b/.s2i/bin/assemble @@ -3,7 +3,7 @@ echo "Before assembling" git config --global url."https://$CERN_GITLAB_USER:$CERN_GITLAB_TOKEN@gitlab.cern.ch".insteadOf https://gitlab.cern.ch # TODO: remove once PR in django-allauth merged -ALLAUTH_INSTALL_DIR=/home/runner +ALLAUTH_INSTALL_DIR=$HOME mkdir -p $ALLAUTH_INSTALL_DIR git clone https://github.com/nothingface0/django-allauth.git $ALLAUTH_INSTALL_DIR/django-allauth cd $ALLAUTH_INSTALL_DIR/django-allauth @@ -16,7 +16,7 @@ cd - /usr/libexec/s2i/assemble rc=$? -ls +pip install $ALLAUTH_INSTALL_DIR/django-allauth # Temporarily install runregistry api client from test PyPI pip install --index-url https://test.pypi.org/simple runregistry==1.0.0 diff --git a/requirements.txt b/requirements.txt index 50d27e88..8c034a59 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ channels[daphne] channels_redis Django<4.2.0 # TODO: Update this once PR is merged to django-allauth --e /home/runner/django-allauth +#-e /home/runner/django-allauth #django-allauth<1.0.0 django-bootstrap3<24.0 django-filter<24.0 From 210da88b82f4686eb29af3161a3650d8666321fd Mon Sep 17 00:00:00 2001 From: Dimitris Papagiannis Date: Fri, 14 Jul 2023 15:52:45 +0200 Subject: [PATCH 3/7] trying this --- dqmhelper/settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/dqmhelper/settings.py b/dqmhelper/settings.py index 5475a884..0a5206c7 100644 --- a/dqmhelper/settings.py +++ b/dqmhelper/settings.py @@ -226,3 +226,4 @@ # When Upgraded to Django 3.2 - RELEASE 06.04.2021 DEFAULT_AUTO_FIELD = "django.db.models.AutoField" ACCOUNT_EMAIL_VERIFICATION = "none" +SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https") From 91b566570d4fda5b701faa2560ad38b062b38fe0 Mon Sep 17 00:00:00 2001 From: Dimitris Papagiannis Date: Fri, 14 Jul 2023 16:05:43 +0200 Subject: [PATCH 4/7] test --- templates/socialaccount/authentication_error.html | 1 + 1 file changed, 1 insertion(+) create mode 100644 templates/socialaccount/authentication_error.html diff --git a/templates/socialaccount/authentication_error.html b/templates/socialaccount/authentication_error.html new file mode 100644 index 00000000..f3071688 --- /dev/null +++ b/templates/socialaccount/authentication_error.html @@ -0,0 +1 @@ +Code: {{ auth_error.code }}, Error: {{ auth_error.exception }} \ No newline at end of file From 078bca44becb4cac5ed7a9e6b4ece677818f3f41 Mon Sep 17 00:00:00 2001 From: Dimitris Papagiannis Date: Sun, 16 Jul 2023 00:08:18 +0200 Subject: [PATCH 5/7] Use OIDC provider instead of deprecated cern one --- .github/workflows/django.yml | 14 ++------ .s2i/bin/assemble | 14 -------- dqmhelper/settings.py | 35 +++++++++++++++++-- dqmhelper/test_settings.py | 1 + requirements.txt | 5 ++- users/signals.py | 56 ++++++++++++++++++++++++------- users/tests/test_users_signals.py | 10 ++++-- 7 files changed, 89 insertions(+), 46 deletions(-) diff --git a/.github/workflows/django.yml b/.github/workflows/django.yml index 6c76c271..8490a5f4 100644 --- a/.github/workflows/django.yml +++ b/.github/workflows/django.yml @@ -42,17 +42,8 @@ jobs: git config --global url."https://${{ secrets.CERN_GITLAB_USER }}:${{ secrets.CERN_GITLAB_TOKEN }}@gitlab.cern.ch".insteadOf https://gitlab.cern.ch python -m pip install --upgrade pip - ALLAUTH_INSTALL_DIR=$HOME - mkdir -p $ALLAUTH_INSTALL_DIR - git clone https://github.com/nothingface0/django-allauth.git $ALLAUTH_INSTALL_DIR/django-allauth - cd $ALLAUTH_INSTALL_DIR/django-allauth - git checkout 77368a8 - wget https://github.com/pennersr/django-allauth/compare/main...nothingface0:django-allauth:fix_cern_sso.patch --output-document=patch - git apply patch --reject || true - cd - - pip install -r requirements.txt - pip install $ALLAUTH_INSTALL_DIR/django-allauth + pip install --index-url https://test.pypi.org/simple runregistry==1.0.0 pip install --upgrade pytest pytest-django pytest-cov codecov mixer selenium @@ -69,7 +60,8 @@ jobs: DJANGO_SECRET_KEY: ${{ secrets.DJANGO_SECRET_KEY }} OMS_CLIENT_ID: ${{ secrets.OMS_CLIENT_ID }} OMS_CLIENT_SECRET: ${{ secrets.OMS_CLIENT_SECRET }} - + CERN_SSO_REGISTRATION_CLIENT_ID: ${{ secrets.CERN_SSO_REGISTRATION_CLIENT_ID }} + CERN_SSO_REGISTRATION_CLIENT_SECRET: ${{ secrets.CERN_SSO_REGISTRATION_CLIENT_SECRET }} run: | PYTHONWARNINGS=all pytest --ds=dqmhelper.test_ci_settings --cov=./ --ignore addrefrun/tests/test_addrefrun_views.py --ignore certifier/tests/test_certifier_views.py --ignore oms/tests/test_oms_utils.py --ignore oms/tests/test_oms_models.py codecov diff --git a/.s2i/bin/assemble b/.s2i/bin/assemble index c05234b9..3381f123 100644 --- a/.s2i/bin/assemble +++ b/.s2i/bin/assemble @@ -1,23 +1,9 @@ #!/bin/bash echo "Before assembling" git config --global url."https://$CERN_GITLAB_USER:$CERN_GITLAB_TOKEN@gitlab.cern.ch".insteadOf https://gitlab.cern.ch - -# TODO: remove once PR in django-allauth merged -ALLAUTH_INSTALL_DIR=$HOME -mkdir -p $ALLAUTH_INSTALL_DIR -git clone https://github.com/nothingface0/django-allauth.git $ALLAUTH_INSTALL_DIR/django-allauth -cd $ALLAUTH_INSTALL_DIR/django-allauth -git checkout 77368a8 -wget https://github.com/pennersr/django-allauth/compare/main...nothingface0:django-allauth:fix_cern_sso.patch --output-document=patch -git apply patch --reject || true -cd - - - /usr/libexec/s2i/assemble rc=$? -pip install $ALLAUTH_INSTALL_DIR/django-allauth - # Temporarily install runregistry api client from test PyPI pip install --index-url https://test.pypi.org/simple runregistry==1.0.0 diff --git a/dqmhelper/settings.py b/dqmhelper/settings.py index 0a5206c7..764f3a96 100644 --- a/dqmhelper/settings.py +++ b/dqmhelper/settings.py @@ -10,9 +10,7 @@ https://docs.djangoproject.com/en/2.1/ref/settings/ """ -import os from pathlib import Path - from decouple import config from django.contrib.messages import constants as messages @@ -76,7 +74,7 @@ "allauth", "allauth.account", "allauth.socialaccount", - "allauth.socialaccount.providers.cern", + "allauth.socialaccount.providers.openid_connect", "widget_tweaks", "django_extensions", "django_tables2", @@ -227,3 +225,34 @@ DEFAULT_AUTO_FIELD = "django.db.models.AutoField" ACCOUNT_EMAIL_VERIFICATION = "none" SECURE_PROXY_SSL_HEADER = ("HTTP_X_FORWARDED_PROTO", "https") + + +SOCIALACCOUNT_PROVIDERS = { + "openid_connect": { + "SERVERS": [ + { + "id": "cern", # 30 characters or less + "name": "CERN", + "server_url": "https://auth.cern.ch/auth/realms/cern", + # Optional token endpoint authentication method. + # May be one of "client_secret_basic", "client_secret_post" + # If omitted, a method from the the server's + # token auth methods list is used + "token_auth_method": "client_secret_post", + "APP": { + "client_id": config("CERN_SSO_REGISTRATION_CLIENT_ID"), + "secret": config("CERN_SSO_REGISTRATION_CLIENT_SECRET"), + }, + }, + ] + } +} + +# This is used to get the public key and decode access tokens +# for users when they login. The URL can be found under the +# jwks_uri key of the JSON pointed to by the server_url of +# CERN's well-known config URL: +# https://auth.cern.ch/auth/realms/cern/.well-known/openid-configuration +CERN_SSO_JWKS_URI = ( + "https://auth.cern.ch/auth/realms/cern/protocol/openid-connect/certs" +) diff --git a/dqmhelper/test_settings.py b/dqmhelper/test_settings.py index a828d138..0de4509a 100644 --- a/dqmhelper/test_settings.py +++ b/dqmhelper/test_settings.py @@ -1,3 +1,4 @@ +import os from .settings import * if os.environ.get("GITHUB_WORKFLOW"): diff --git a/requirements.txt b/requirements.txt index 8c034a59..05b96066 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,9 +6,7 @@ certifi<2023.0.0 channels[daphne] channels_redis Django<4.2.0 -# TODO: Update this once PR is merged to django-allauth -#-e /home/runner/django-allauth -#django-allauth<1.0.0 +django-allauth<1.0.0 django-bootstrap3<24.0 django-filter<24.0 django-ckeditor<7.0.0 @@ -29,5 +27,6 @@ apscheduler whitenoise factory_boy prettytable +pyjwt git+https://gitlab.cern.ch/cmsoms/oms-api-client git+https://github.com/eea/odfpy.git diff --git a/users/signals.py b/users/signals.py index 6bb546d9..eca9da77 100644 --- a/users/signals.py +++ b/users/signals.py @@ -1,22 +1,29 @@ -import allauth +import jwt import django +import allauth from allauth.account.signals import user_logged_in from allauth.socialaccount.models import SocialAccount -from allauth.socialaccount.signals import social_account_updated, \ - social_account_added, social_account_removed, pre_social_login +from allauth.socialaccount.signals import ( + social_account_updated, + social_account_added, + social_account_removed, + pre_social_login, +) from django.contrib.auth import get_user_model from django.db.models.signals import post_save, pre_save from django.dispatch import receiver from users.utilities.logger import get_configured_logger from users.utilities.utilities import update_user_extradata +from decouple import config +from django.conf import settings logger = get_configured_logger(loggername=__name__, filename="signals.log") + @receiver(django.contrib.auth.signals.user_logged_in) def update_users_on_login(sender, user, request, **kwargs): update_user_extradata(user) - user.save() @receiver(pre_save, sender=get_user_model()) @@ -40,19 +47,35 @@ def log_user_logged_out(sender, user, request, **kwargs): logger.info("User {} has logged out".format(user)) -@receiver(pre_social_login) -def log_pre_social_login(request, sociallogin, **kwargs): +def log_pre_social_login(request, sociallogin): try: logger.debug("Pre social login for User {}".format(sociallogin.user)) except (get_user_model().DoesNotExist, AttributeError): logger.debug("Pre social login for non-existing User") +@receiver(pre_social_login) +def pre_social_login(request, sociallogin, **kwargs): + # Get the token from the login and decode it, + # so that we can get cern_roles out of it + key = jwt.PyJWKClient(settings.CERN_SSO_JWKS_URI).get_signing_keys()[0] + sociallogin.account.extra_data = jwt.decode( + jwt=sociallogin.token.token.encode("utf-8"), + key=key.key, + algorithms=["RS256"], + audience=config("CERN_SSO_REGISTRATION_CLIENT_ID"), + ) + log_pre_social_login(request, sociallogin=sociallogin) + + @receiver(social_account_added) def log_social_account_added(request, sociallogin, **kwargs): try: - logger.info("Social Account {} has been added for User {}" - .format(sociallogin.account, sociallogin.user)) + logger.info( + "Social Account {} has been added for User {}".format( + sociallogin.account, sociallogin.user + ) + ) except (SocialAccount.DoesNotExist, get_user_model().DoesNotExist, AttributeError): logger.debug("Pre social login for non-existing User") @@ -60,8 +83,11 @@ def log_social_account_added(request, sociallogin, **kwargs): @receiver(social_account_updated) def log_social_account_updated(request, sociallogin, **kwargs): try: - logger.debug("Social Account {} has been updated for User {}" - .format(sociallogin.account, sociallogin.user)) + logger.debug( + "Social Account {} has been updated for User {}".format( + sociallogin.account, sociallogin.user + ) + ) except (SocialAccount.DoesNotExist, get_user_model().DoesNotExist, AttributeError): logger.error("Something unexpected happened") @@ -69,8 +95,11 @@ def log_social_account_updated(request, sociallogin, **kwargs): @receiver(social_account_removed) def log_social_account_removed(request, socialaccount, **kwargs): try: - logger.info("Social Account {} has been removed from User {}" - .format(socialaccount, socialaccount.user)) + logger.info( + "Social Account {} has been removed from User {}".format( + socialaccount, socialaccount.user + ) + ) except (get_user_model().DoesNotExist, AttributeError): logger.error("Something unexpected happened") @@ -79,6 +108,7 @@ def log_social_account_removed(request, socialaccount, **kwargs): def log_user_has_login_failed(sender, credentials, request, **kwargs): try: logger.warning( - "User {} has failed to logged in".format(credentials.get("username"))) + "User {} has failed to logged in".format(credentials.get("username")) + ) except AttributeError: logger.error("Username attribute does not exist!") diff --git a/users/tests/test_users_signals.py b/users/tests/test_users_signals.py index 00399752..283dd16d 100644 --- a/users/tests/test_users_signals.py +++ b/users/tests/test_users_signals.py @@ -1,9 +1,9 @@ import pytest from django.db import IntegrityError -from mixer.backend.django import mixer from django.contrib.auth import get_user_model -from users.signals import * from django.test import Client +from users.signals import * +from mixer.backend.django import mixer pytestmark = pytest.mark.django_db @@ -14,6 +14,12 @@ def test_create_user(): assert user +def test_pre_social_login(): + # TODO: Add test for verifying functionality + # of users.signals.pre_social_login, if possible + pass + + def test_logs(): """ Just run them once, nothing really to test here From 37ce666055fad46b2d277b7e6ae068d8b2ce42ba Mon Sep 17 00:00:00 2001 From: Dimitris Papagiannis Date: Sun, 16 Jul 2023 09:30:04 +0200 Subject: [PATCH 6/7] Make sure extra data is stored into SocialAccount --- users/signals.py | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/users/signals.py b/users/signals.py index eca9da77..68c0ee7e 100644 --- a/users/signals.py +++ b/users/signals.py @@ -56,8 +56,11 @@ def log_pre_social_login(request, sociallogin): @receiver(pre_social_login) def pre_social_login(request, sociallogin, **kwargs): - # Get the token from the login and decode it, - # so that we can get cern_roles out of it + log_pre_social_login(request, sociallogin=sociallogin) + # The default OIDC provider does not provide the extra + # data that CERN SSO returns (i.e. cern_roles). + # For that reason we use the token from the login and decode it, + # so that we can get all the extra info from it. key = jwt.PyJWKClient(settings.CERN_SSO_JWKS_URI).get_signing_keys()[0] sociallogin.account.extra_data = jwt.decode( jwt=sociallogin.token.token.encode("utf-8"), @@ -65,7 +68,24 @@ def pre_social_login(request, sociallogin, **kwargs): algorithms=["RS256"], audience=config("CERN_SSO_REGISTRATION_CLIENT_ID"), ) - log_pre_social_login(request, sociallogin=sociallogin) + # We now have to make sure that this data is saved in the + # SocialAccount entry in the Database; there are two distinct + # cases here: + # 1. This is the first login of this specific user, and + # there's no SocialAccount entry for them yet. In this case + # (i.e. sociallogin.account.pk = None), at this point of the + # login flow, this SocialAccount is NOT yet ready for saving, + # as django-allauth is going to add some missing information + # at a later step (during django-allauth's _add_social_account), + # so let django-allauth save the account, later. + # + # 2. This is a login of a returning user we've already registered. + # django-allauth has already updated and saved the account with the + # stripped down information that the SSO returns (i.e. without + # cern_roles) during account lookup, which happens *before* pre_social_login. + # In this case, we have to update the extra info ourselves. + if sociallogin.account.pk: + sociallogin.account.save() @receiver(social_account_added) From 54bc315d54ccbb69ac12a9ff6780444f2a0fc9f3 Mon Sep 17 00:00:00 2001 From: Dimitris Papagiannis Date: Mon, 7 Aug 2023 11:05:46 +0200 Subject: [PATCH 7/7] Use runregistry from normal PyPI --- .s2i/bin/assemble | 3 --- requirements.txt | 4 +--- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/.s2i/bin/assemble b/.s2i/bin/assemble index 3381f123..77f97e55 100644 --- a/.s2i/bin/assemble +++ b/.s2i/bin/assemble @@ -4,9 +4,6 @@ git config --global url."https://$CERN_GITLAB_USER:$CERN_GITLAB_TOKEN@gitlab.cer /usr/libexec/s2i/assemble rc=$? -# Temporarily install runregistry api client from test PyPI -pip install --index-url https://test.pypi.org/simple runregistry==1.0.0 - if [ $rc -eq 0 ]; then echo "After successful assembling" else diff --git a/requirements.txt b/requirements.txt index 05b96066..156b4a8a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -20,9 +20,7 @@ daphne<5.0.0 django-widget-tweaks<2.0.0 django-extensions<4.0.0 paramiko -# This will be uncommented once the tested -# version is released. -# runregistry +runregistry>=1.0.0 apscheduler whitenoise factory_boy