ubccr / coldfront

HPC Resource Allocation System
https://coldfront.readthedocs.io
GNU General Public License v3.0
101 stars 80 forks source link

ORCID integration for grants and publications #437

Open aestoltm opened 2 years ago

aestoltm commented 2 years ago

Draft PR that is built off PR (#359) created by @RyanChang0369.

Current Bugs:

  1. [FIXED] UI bug on add publication page showing 'import grants from user ORCID' instead of 'import publications from user ORCID'
  2. Sometimes grants added through ORCID are skipped and are not presented on the project detail page. (could be the same case for publications) [NOTE: Skips grants with no grant number so might not be a bug. Every grant should have a grant number associated with it.]

Current Enchantments:

  1. [ADDED] Add user's name next to ORCID when performing ORCID search for pulling publications and grants
  2. [ADDED] Include OAuth for ORCID integration instead of inserting ORCID iD into user profile.

More bugs or enchantments may be added as further testing is performed.

mdzik commented 2 years ago

Would you consider Django Social Auth for providing orcid-cf coupling? I could provided sample config. It provides proper SSO/Oauth exchange and user need to login to orcid as 'proof of ownership'.

And from our expirience: be aware that orcid records dont always have full structure

czw., 21 lip 2022, 22:38 użytkownik aestoltm @.***> napisał:

Draft PR that is built off PR (#359 https://github.com/ubccr/coldfront/pull/359) created by @RyanChang0369 https://github.com/RyanChang0369.

Current Bugs:

  1. UI bug on add publication page showing 'import grants from user ORCID' instead of 'import publications from user ORCID'
  2. Sometimes grants added through ORCID are skipped and are not presented on the project detail page. (could be the same case for publications)

Current Enchantments:

  1. Added user's name next to ORCID when performing ORCID search for pulling publications and grants

More bugs or enchantments may be added as further testing is performed.

You can view, comment on, or merge this pull request online at:

https://github.com/ubccr/coldfront/pull/437 Commit Summary

File Changes

(33 files https://github.com/ubccr/coldfront/pull/437/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/ubccr/coldfront/pull/437, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3A5RZHFWWINT2HTLE6Y7TVVGRJ7ANCNFSM54IYYTZA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

aebruno commented 2 years ago

Would you consider Django Social Auth for providing orcid-cf coupling? I could provided sample config.

Yes absolutely. We'll be making some more changes/fixes to this PR before it gets merged so happy to take any suggestions on how to improve. A sample config would be great.

aestoltm commented 2 years ago

@mdzik For the usage of Django Social Auth, are you using the library django-social-auth (https://github.com/omab/django-social-auth) which is dependent on python-social-auth (https://github.com/omab/python-social-auth)? These libraries are deprecated and I was wondering if you were using the updated version, social-app-django (https://github.com/python-social-auth/social-app-django) which is dependent on social-core (https://github.com/python-social-auth/social-core).

mdzik commented 2 years ago

@aestoltm - sorry for long reply, :palm_tree:

You are obviously right - we use social-core and social-app-django.

our social pipeline looks like that:

#------------------------------------------------------------------------------
# Authentication backends
#------------------------------------------------------------------------------
AUTHENTICATION_BACKENDS = [
    'coldfront.plugins.social_auth.dispatcher_backend.DispatcherBackend'
]

SOCIAL_AUTH_PIPELINE = (
   # 'social_core.pipeline.debug.debug',
    'social_core.pipeline.social_auth.social_details',
    'social_core.pipeline.social_auth.social_uid',
    'social_core.pipeline.social_auth.social_user',
    'coldfront.plugins.social_auth.restrict_to_existing.validate_user',
    #'social_core.pipeline.user.get_username',
    #'social_core.pipeline.social_auth.associate_by_email', DONT
    #'social_core.pipeline.user.create_user',
    'social_core.pipeline.social_auth.associate_user',
    'social_core.pipeline.social_auth.load_extra_data',
    #'social_core.pipeline.user.user_details', Update user model attributes with the new data sent by the current provider
)

AUTHENTICATION_BACKENDS += ['social_core.backends.orcid.ORCIDOAuth2']
AUTHENTICATION_BACKENDS += ['social_core.backends.keycloak.KeycloakOAuth2']

#Those might be needed for Keycloak/ORCID to work, depending on reverse-proxy configuration
SOCIAL_AUTH_VERIFY_SSL = False
SESSION_COOKIE_SECURE = True 
SOCIAL_AUTH_REDIRECT_IS_HTTPS = True
USE_X_FORWARDED_HOST = True

We use 2 social providers: ORCID and our Keykloak IDP. Users could bind account to ORCID, but login is limited to Keycloak. Some subset of our PI's don't have LDAP/Keycloak account, so we permit those accounts to login using Django model auth. They have @ in username

And the coldfront.plugins.social_auth.restrict_to_existing file:

from social_core.exceptions import  AuthForbidden
from social_core.backends.keycloak import KeycloakOAuth2
from django.contrib.auth.models import User
from pprint import pprint
from django.core.exceptions import ObjectDoesNotExist

from django.shortcuts import redirect
from django.contrib.auth.models import User
from social_core.pipeline.partial import partial

@partial
def validate_user(strategy, user=None,  **kwargs):

    if user is None:
        if isinstance(kwargs['backend'], KeycloakOAuth2):

            try:
                user = User.objects.get(username=kwargs['response']['preferred_username'])
            except ObjectDoesNotExist:
                raise AuthForbidden(strategy)

            if not (str(kwargs['response']['preferred_username']) == user.username): # just in case
                raise AuthForbidden(strategy)
            return {'user': user,
                'is_new': False
            }

        else:
            raise AuthForbidden(strategy)

    return

coldfront.plugins.social_auth.dispatcher_backend.DispatcherBackend

from django.contrib.auth.backends import ModelBackend
from django.contrib import messages
from django.utils.translation import gettext as _
from django.http import Http404

class DispatcherBackend(ModelBackend):

    def authenticate(self, request, username=None, password=None):

        if '@' not in username: 
            messages.error(request, _('All ICM users are required to login using IDP, and provide their MFA password.'))
            raise Http404("Authentication method not avalible!")
            return None

        return super().authenticate(request, username, password)

To extract user data from ORCID API we use:

     user = viewed_user
        context['has_orcid'] = False
        context['has_orcid_employments'] = False

        if user.social_auth.exists():

            try:
                social = user.social_auth.get(provider='orcid')

                api = orcid.PublicAPI(SOCIAL_AUTH_ORCID_KEY, SOCIAL_AUTH_ORCID_SECRET)

                employments = api.read_record_public(social.extra_data['id'], 'employments', social.extra_data['access_token'])
                base_orcid = api.read_record_public(social.extra_data['id'], 'record', social.extra_data['access_token'])
                context['has_orcid'] = True

                context['ORCID_url'] = base_orcid['orcid-identifier']['uri']
                context['ORCID'] = base_orcid['orcid-identifier']['uri']

                if 'employment-summary' in employments.keys() and len(employments['employment-summary']) > 0:
                    context['has_orcid_employments'] = True

                    context['ORCID_empoyments'] = employments['employment-summary']

This way you could get 'institutional token' from ORCID, and have broader access to user data - as the geve a consent to you while logging. I must admit that I did not tested that - to much paperwork ;)

mdzik commented 2 years ago

2. Sometimes grants added through ORCID are skipped and are not presented on the project detail page. (could be the same case for publications) [NOTE: Skips grants with no grant number so might not be a bug. Every grant should have a grant number associated with it.]

ORCID data have 'holes'. I think they do not force most of the fields during input. Even some publications don't have full 'basic' info, including authors :/ It;s not frequent, but annoying. I try to always assume that any given element of ORCID tree could be missing.

aestoltm commented 2 years ago

@mdzik Thank you for providing sample usage of social-core and social-app-django! We appreciate the information you provided to help us better configure these libraries for our OAuth needs. We will make sure to keep in mind that ORCID records are not complete for when we pull user data.

dsajdak commented 2 years ago

Additional changes to be made to the general 'Add Publication' and 'Add Grant' workflow that came out of this feature:

On the 'Add Publication' page:

On the 'Add Grant' page: