dataverse icon indicating copy to clipboard operation
dataverse copied to clipboard

optionally split Shibboleth affiliation and return first or last value

Open Louis-wr opened this issue 3 years ago • 4 comments

What this PR does / why we need it: Feide authentication service returns an array of values. The affiliation in English is expected to be the last element in the array. This modification adds an option to only return the last element of said array.

Which issue(s) this PR closes:

Closes #8882

Special notes for your reviewer:

Suggestions on how to test this: the array is comma separated for example : "UiT Norges Arktiske Universitet,UiT The Arctic University of Norway" The separator is set with the following curl command : curl -X PUT -d "," http://localhost:8080/api/admin/settings/:ShibAffiliationSeparator

The last value is selected using : curl -X PUT -d "lastAffiliation" http://localhost:8080/api/admin/settings/:ShibAffiliationOrder will make it so the affiliation is only "UiT The Arctic University of Norway"

The first value is selected using : curl -X PUT -d "firstAffiliation" http://localhost:8080/api/admin/settings/:ShibAffiliationOrder will make it so the affiliation is only "UiT Norges Arktiske Universitet"

Does this PR introduce a user interface change? If mockups are available, please link/include them here: NO

Is there a release notes update needed for this change?: YES New parameter in configuration : :ShibAffiliationOrder New parameter in configuration : :ShibAffiliationSeparator

Additional documentation: under database settings :ShibAffiliationOrder Returns the last value of an array in affiliations list. The array is separated using :ShibAffiliationSeparator curl -X PUT -d "lastAffiliation" http://localhost:8080/api/admin/settings/:ShibAffiliationOrder

:ShibAffiliationSeparator Set the separator for the affiliation array Default : ";" curl -X PUT -d ";" http://localhost:8080/api/admin/settings/:affiliationSeparator

Louis-wr avatar Aug 02 '22 10:08 Louis-wr

@pdurbin Thank you for your comments. I think i solved all the issues, nevertheless please feel free to get back to me if more changes are required. I took the liberty of using the name doc/release-notes/8880-Dvno-affiliation.md so it will match the documentation and other pull request, please let me know if you had something else in mind.

Louis-wr avatar Aug 02 '22 13:08 Louis-wr

@pdurbin I took care of the issues you pointed out. I am not sure what went wrong with the indentations but it should look a lot more consistent now.

Louis-wr avatar Aug 02 '22 14:08 Louis-wr

Hi @Louis-wr , for this next sprint we are catching up on Community PRs. Would you mind updating/ refreshing thiis PR from develop? Thanks!

scolapasta avatar Aug 04 '22 19:08 scolapasta

@scolapasta done, let me know if you need anything else.

Louis-wr avatar Aug 04 '22 19:08 Louis-wr

Coverage Status

Coverage decreased (-0.007%) to 20.035% when pulling c62deaf1694c269b96e8c5b4a56d4550dcdae16f on DataverseNO:dvno-Affiliation into f9c1a024b95ad194f7fbfcfeb211ef7b8f81780d on IQSS:develop.

coveralls avatar Aug 25 '22 10:08 coveralls

Huh. I clicked the button to run the API tests (they don't run automatically for first-time pull request authors) and HarvestingServerIT.testOaiFunctionality is failing according to https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-8880/17/testReport/junit/edu.harvard.iq.dataverse.api/HarvestingServerIT/testOaiFunctionality/

I know @Louis-wr (and anyone off campus) can't see Jenkins due to a firewall but here's a screenshot:

Screen Shot 2022-08-25 at 9 14 47 AM

Let me ask about re-running the tests or maybe kick off a new run myself.


UPDATE: We're seeing this test failing on the develop branch as well. As I suspected, this failure is unrelated to this pull request. All the other API tests for this pull request succeeded.


UPDATE 2: I opened an issue for the harvest test failure:

  • #8937

pdurbin avatar Aug 25 '22 13:08 pdurbin

Huh. I clicked the button to run the API tests (they don't run automatically for first-time pull request authors) and HarvestingServerIT.testOaiFunctionality is failing according to https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-8880/17/testReport/junit/edu.harvard.iq.dataverse.api/HarvestingServerIT/testOaiFunctionality/

I know @Louis-wr (and anyone off campus) can't see Jenkins due to a firewall but here's a screenshot:

Let me ask about re-running the tests or maybe kick off a new run myself.

Hello, Thank you sharing the error log. From what i can tell it cant connect to a resource at amazoneaws.com, it that case i presume this issue is not related to the code i summited

Also as this anything to do with this error ?

Extension error (sphinxcontrib.icon.icon):
Handler <function download_font_assets at 0x7fcea99cd280> for event 'builder-inited' threw an exception (exception: HTTP Error 503: Egress is over the account limit.)
make: *** [Makefile:64: html] Error 2

from https://github.com/IQSS/dataverse/runs/8014075879?check_suite_focus=true I i interpreted this correctly an asset can not be downloaded from a Microsoft azure blob.

Louis-wr avatar Aug 25 '22 13:08 Louis-wr

Hmm, I'm not too worried about the "Guides Build Status" failure because the OTHER guides job ("docs/readthedocs...") passed and built docs that look good: https://dataverse-guide--8880.org.readthedocs.build/en/8880/installation/config.html#shibaffiliationorder

By the way I opened #8937 about the first error I mentioned about harvesting. It definitely has nothing to do with this pull request.

pdurbin avatar Aug 25 '22 18:08 pdurbin

@Louis-wr I'm having a little trouble testing this PR.

Rather than setting up all of Shibboleth on a server, I'm using the method described at https://guides.dataverse.org/en/5.11.1/developers/remote-users.html

First, I add a shib auth provider:

$ cat shibAuthProvider.json { "id":"shib", "factoryAlias":"shib", "enabled":true }

$ curl -X POST -H 'Content-type: application/json' --upload-file shibAuthProvider.json http://localhost:8080/api/admin/authenticationProviders

Then I set the "debug shib" setting, starting with "random":

$ curl http://localhost:8080/api/admin/settings/:DebugShibAccountType -X PUT -d RANDOM

However, when I go to http://localhost:8080/shib.xhtml and click "Create Account" I get this stacktrace:

[2022-09-07T13:31:43.220-0400] [Payara 5.2022.3] [WARNING] [] [javax.enterprise.web] [tid: _ThreadID=120 _ThreadName=http-thre ad-pool::http-listener-1(2)] [timeMillis: 1662571903220] [levelValue: 900] [[ StandardWrapperValve[Faces Servlet]: Servlet.service() for servlet Faces Servlet threw exception java.lang.NullPointerException at java.base/java.util.HashSet.(HashSet.java:119) at edu.harvard.iq.dataverse.authorization.providers.builtin.DataverseUserPage.init(DataverseUserPage.java:177)

Here's line 177:

mutedEmails = new HashSet<>(currentUser.getMutedEmails());

@ErykKul added this recently and might have some ideas of how to get it working with the "shib debug" method above.

@Louis-wr meanwhile, I can't push to your branch. This isn't a problem necessarily but if I were able to fix the above I'd probably just push rather than making a pull request against your pull request.

Do you want to try the "shib debug" thing above? In addition to "RANDOM" there cases like "TWO_EMAILS" and "INVALID_EMAIL". You could add a case or two having to do with affiliation.

pdurbin avatar Sep 07 '22 17:09 pdurbin

Phew! Ok. I just created https://github.com/DataverseNO/dataverse/pull/44 which is how I tested this PR. Again, instead of setting up all of Shibboleth on a server, it's much easier to test using the :DebugShibAccountType settings (I just added two cases in that PR). @Louis-wr if you're willing to merge that PR, I'd appreciate it.

To test I used that PR to the "two affiliations" case like this:

curl http://localhost:8080/api/admin/settings/:DebugShibAccountType -X PUT -d TWO_AFFILIATIONS

That PR relies on setting the affiliation attribute to "ou" like this:

curl http://localhost:8080/api/admin/settings/:ShibAffiliationAttribute -X PUT -d "ou"

I left the separator undefined to get ";". Setting it should be fine.

Here are the two affiliations at login ("SNPP;Stonecutters"):

Screen Shot 2022-09-07 at 3 45 30 PM

Likewise if you just look at the account page, you can see both affiliations, separated by a semicolon:

Screen Shot 2022-09-07 at 3 45 49 PM

Now, if you say you want the last affiliation, it works:

curl -X PUT -d "lastAffiliation" http://localhost:8080/api/admin/settings/:ShibAffiliationOrder

Screen Shot 2022-09-07 at 3 47 43 PM

It also works to request the first affiliation:

curl -X PUT -d "firstAffiliation" http://localhost:8080/api/admin/settings/:ShibAffiliationOrder

Screen Shot 2022-09-07 at 3 48 14 PM

I'm satisfied enough to move this to QA but again @Louis-wr please consider merging the PR above. Thanks!

pdurbin avatar Sep 07 '22 20:09 pdurbin

I left the separator undefined to get ";". Setting it should be fine.

I decided I should actually test this. I set it to "," to be different than the default (";") and it correctly did NOT split SNPP;Stonecutters into two values:

curl -X PUT -d "," http://localhost:8080/api/admin/settings/:ShibAffiliationSeparator

Screen Shot 2022-09-07 at 4 09 39 PM

pdurbin avatar Sep 07 '22 20:09 pdurbin

@ErykKul added this recently and might have some ideas of how to get it working with the "shib debug" method above.

I've been talking with @ErykKul and he came up with a fix in PR #8969. I haven't played with that code. It wasn't required for me to test this PR. Until a fix like that is merged, you just have to be conscious that you might get errors from Shib with the debug method (and maybe prod Shib?). The work around is to log out and back in so that info about your user comes from the database. It's confusing but out of scope for this PR.

I ran the API tests and they passed. I think we're good so I'm merging this.

pdurbin avatar Sep 09 '22 19:09 pdurbin