Fix ORCID login when no family name was given

nikoder commented 6 months ago

Handle the case where the user has not registered a family-name and ORCID returns None.

Proposed changes

This addresses https://github.com/python-social-auth/social-app-django/issues/355 and #602 (which it appears to duplicate). Full credit to paloha, who very clearly explained the issue and proposed the simple fix included here.

As a brief summary: ORCID does not require users to enter a family name and returns a family-name key with value None in this case. The code expects that key to either be absent, or contain a dictionary. This leads to an AttributeError after successful login by a user who has not provided a family name to ORCID.

I considered adding a test case to cover this scenario, but it looks like the testing infrastructure is only set up for a single possible user_data_body? Please let me know if I have missed an easy and obvious way to add a test for an additional case of returned data. The change does resolve the issue in my manual testing.

Types of changes

Please check the type of change your PR introduces:

[ ] Release (new release request)
[x] Bugfix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Code style update (PEP8, lint, formatting, renaming, etc)
[ ] Refactoring (no functional changes, no api changes)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Build related changes (build process, tests runner, etc)
[ ] Other (please describe):

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

[x] Lint and unit tests pass locally with my changes - caveat: 6 of 587 discovered tests fail independently of these changes in my environment but they do not appear to be covering the changed code (they are related to SAML)
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] I have added documentation to https://github.com/python-social-auth/social-docs

Other information

Any other information that is important to this PR such as screenshots of how the component looks before and after the change.

nijel commented 6 months ago

Merged, thanks for your contribution!

nikoder commented 6 months ago

My pleasure! Thanks for your great work on this awesome project :) !

python-social-auth / social-core