Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #6514: Implement affiliation reading from Shibboleth attribute. #6729

Conversation

pkiraly
Copy link
Member

@pkiraly pkiraly commented Mar 6, 2020

What this PR does / why we need it: It adds a new setting :ShibAffiliationAttribute, which contains the name of a Shibboleth attribute (as a string), which attribute holds the affiliation of the user. If it is set, Dataverse reads the content of the attribute from the Shibboleth response, otherwise it reads from the DiscoFeed (which is currently the one and only source of affiliation). DiscoFeed is a wrong source in our use case. A closely related problem is that Dataverse overwrites the affiliation from Shibboleth for every login, so as a site admin it is not an option to fix the wrong data comes from the DiscoFeed by changing the database, because the next login eliminate my changes.

Which issue(s) this PR closes: #6514

Closes #6514

Special notes for your reviewer: The configuration this PR implements adds a new database setting. I am aware that @poikilotherm is working on a related issue #6694 which will provide a common configuration for all authentication related settings. Once that ticket will be ready, we can modify this mechanism to use the JSON configuration. On the meantime I also provided documentation for the database setting.

Suggestions on how to test this:

  1. First the site admin should contact the Shibboleth administration to ask how to modify attribute-mapping.xml in order to retrieve the affiliation value.
  2. Then it should be tested and check the transaction log if the attribute is arrived. Let's suppose the Shibboleth response contain "ou" attribute as the affiliation.
  3. Set the database setting:
curl -X PUT -d "ou" http://localhost:8080/api/admin/settings/:ShibAffiliationAttribute
  1. log in, and check on the account data if the affiliation string has been changed (the value should come now from the Shibboleth attribute).
  2. Log out, and delete the database setting:
curl -X DELETE http://localhost:8080/api/admin/settings/:ShibAffiliationAttribute
  1. log in, and check on the account data if the affiliation string has been changed again back to the previous value (comes from DiscoFeed).

Does this PR introduce a user interface change?: no

Is there a release notes update needed for this change?: Yes, refer to the documentation this PR provides

Additional documentation: Yes, in the commit.

@coveralls
Copy link

coveralls commented Mar 6, 2020

Coverage Status

Coverage increased (+0.0003%) to 19.444% when pulling 5a27cdf on pkiraly:6514-optionally-read-affiliation-from-shibbolet-attribution into 7ed6519 on IQSS:develop.

@djbrooke
Copy link
Contributor

djbrooke commented Mar 6, 2020

Hi @pkiraly - thanks for the PR. Can you give me write access to this branch? I'll work with a developer to review this and also make some doc changes. Thanks!

@pkiraly
Copy link
Member Author

pkiraly commented Mar 6, 2020

@djbrooke I made you a collaborator. Ping me if it is not working.

@djbrooke djbrooke self-assigned this Mar 9, 2020
@djbrooke djbrooke removed their assignment Mar 10, 2020
Copy link
Member Author

@pkiraly pkiraly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am correct with the terminology this is a Database setting, not a JVM option. JVM options are those which can be listed by asadmin list-jvm-options, while database settings can be listed via curl -s http://localhost:8080/api/admin/settings. This one belongs to the later group.

So the text should be changed something line

New database setting ...

@djbrooke
Copy link
Contributor

@pkiraly Yes, you're exactly right. I've updated with 5a27cdf. Thank you!

@kcondon kcondon self-assigned this Mar 10, 2020
@kcondon kcondon merged commit d3418bd into IQSS:develop Mar 18, 2020
@djbrooke djbrooke added this to the 4.20 milestone Mar 19, 2020
@pkiraly
Copy link
Member Author

pkiraly commented Jun 24, 2021

This PR was developed by Göttingen eResearch Alliance, Germany, and funded by SSHOC, "Social Sciences and Humanities Open Cloud". SSHOC has received funding from the European Union’s Horizon 2020 project call H2020-INFRAEOSC-04-2018, grant agreement #823782.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optionally read affiliation from a Shibbolet attribution instead of DiscoFeed
5 participants