Skip to content

Implement usage of user supplied handle for authentication. This is an optional parameter.#7951

Merged
kcondon merged 6 commits intoIQSS:developfrom
cookie33:7949-HandleAuthHandle
Jul 12, 2021
Merged

Implement usage of user supplied handle for authentication. This is an optional parameter.#7951
kcondon merged 6 commits intoIQSS:developfrom
cookie33:7949-HandleAuthHandle

Conversation

@cookie33
Copy link

@cookie33 cookie33 commented Jun 18, 2021

Implement usage of user supplied handle for authentication. This is an optional parameter.

What this PR does / why we need it:
It fixes the issue if the Dataverse user can not use the default supplied handle to authorise

Which issue(s) this PR closes:

Closes #7949

Special notes for your reviewer: Not tested yet. We do not run Dataverse. We are a handle provider.

Suggestions on how to test this: try it with an existing Dataverse installation which talks to a handle server. Set the new optional parameter HandleAuthHandle to 0.NA/<prefix>. Or if it is an independent handle server to <prefix>/ADMIN.

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

Is there a release notes update needed for this change?:

yes. To state that a new parameter has been introduced.

Additional documentation:

@djbrooke
Copy link
Contributor

Hi @cookie33 thanks for the PR! Jenkins is saying that the commit can't be built:

https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-7951/5/display/redirect

Can you take a look? Thanks!

@cookie33
Copy link
Author

@djbrooke. The Maven unit tests are failing. But I fail to see why? What did I do wrong? Or could have done better.

@poikilotherm
Copy link
Contributor

No worries @cookie33 the unit tests aren't failing - it's just the code coverage not working as it should. See #7977 and #7981

@djbrooke
Copy link
Contributor

Thanks @poikilotherm and apologies to @cookie33 for the confusion!

@cookie33
Copy link
Author

Thanks for the updates.

@sekmiller sekmiller self-assigned this Jul 8, 2021
By default this setting is absent and the Dataverse Software assumes it to be not set.

``curl -X PUT -d '<prefix/<suffix>' http://localhost:8080/api/admin/settings/:HandleAuthHandle``

Copy link
Contributor

Choose a reason for hiding this comment

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

just an inconsistency with the '<prefix/suffix>' in the description/example. please remove the "<" in the example

Copy link
Author

Choose a reason for hiding this comment

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

HI,

I modified the documentation to be more clear with a real world example.
A new commit was done..

Greetings..

Copy link
Contributor

@sekmiller sekmiller left a comment

Choose a reason for hiding this comment

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

Looks good overall. Just one minor change in the doc. Also please update with the latest from the dev branch and make sure there are no conflicts. Thanks!

@sekmiller sekmiller removed their assignment Jul 8, 2021
@djbrooke
Copy link
Contributor

djbrooke commented Jul 9, 2021

Thanks @cookie33! We'll take another look!

@djbrooke djbrooke requested a review from sekmiller July 9, 2021 13:22
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 19.314% when pulling 7b53b56 on cookie33:7949-HandleAuthHandle into d2565b8 on IQSS:develop.

Copy link
Contributor

@sekmiller sekmiller left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@kcondon kcondon self-assigned this Jul 12, 2021
@kcondon kcondon merged commit 57b1224 into IQSS:develop Jul 12, 2021
@djbrooke djbrooke added this to the 5.6 milestone Jul 12, 2021
@cookie33 cookie33 deleted the 7949-HandleAuthHandle branch July 14, 2021 07:09
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.

Add support for handle service with authentication NOT in default handle

6 participants