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.

Sign up for GitHub

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

Jump to bottom

Shib attribute character set conversion enabled #6956

Merged
merged 11 commits into from Jun 5, 2020
Merged

Shib attribute character set conversion enabled #6956

merged 11 commits into from Jun 5, 2020

Conversation

pallinger
Copy link
Contributor

This fixes #6946
(Shibboleth attribute character set conversion error.)

Closes #6946

You can test it by logging in with someone having an accented name (containing e.g. öüóőúéáűä) using shibboleth.

I think the release notes or documentation should include the new ShibAttributeCharacterSetConversionEnabled setting that is now true by default.

@djbrooke
Copy link
Contributor

djbrooke commented Jun 2, 2020

@pallinger thanks for the PR! Let us know when this is ready for review and we'll be happy to take a look. If you need help with docs or have other questions let us know. Thanks again!

@coveralls
Copy link

coveralls commented Jun 2, 2020 •
edited

Coverage Status

Coverage increased (+3.0e-05%) to 19.632% when pulling fd4b160 on dsd-sztaki-hu:ShibAttributeCharacterSetConversionEnabled into 66ed398 on IQSS:develop.

pdurbin
pdurbin requested changes Jun 2, 2020
View reviewed changes
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Overall this looks ok but can you please add docs and look at my other comment? Thanks.

src/main/java/edu/harvard/iq/dataverse/settings/SettingsServiceBean.java Show resolved Hide resolved
src/main/java/edu/harvard/iq/dataverse/settings/SettingsServiceBean.java Outdated Show resolved Hide resolved
@pallinger
Copy link
Contributor Author

pallinger commented Jun 3, 2020 •
edited

I tested it on our dev server, and it fixed the bad characters in names. However, I needed need to do a clean before you deploy it though, else I got a permission error:

permission(("javax.security.jacc.EJBMethodPermission" "SystemConfig" "isShibAttributeCharacterSetConversionEnabled,Local,"))]]

The PR is ready for review.

@djbrooke
Copy link
Contributor

djbrooke commented Jun 3, 2020

Thanks @pallinger for the quick changes, we'll take another look.

pdurbin
pdurbin requested changes Jun 3, 2020
View reviewed changes
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Just a couple small changes left. Thanks! If you prefer, we can make them. Please let us know.

doc/sphinx-guides/source/installation/config.rst Show resolved Hide resolved
@djbrooke djbrooke moved this from Code Review 🦁 to Community Dev 💻❤️ in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Jun 3, 2020
djbrooke
djbrooke reviewed Jun 3, 2020
View reviewed changes
Copy link
Contributor

@djbrooke djbrooke left a comment

Choose a reason for hiding this comment

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

@pallinger Thanks again. Can you add a release note for this in doc/release-notes following the process described in the readme there? If you want to give me push access to your fork, I can make the changes there as well.

pallinger and others added 2 commits June 4, 2020 10:51
@pallinger
fix documentation for ShibAttributeCharacterSetConversionEnabled
6ee3681 
- move section above an unrelated anchor
- rename Glassfish to something more generic
release notes for IQSS#6956
fd4b160
@pallinger
Copy link
Contributor Author

pallinger commented Jun 4, 2020 •
edited

I pushed some release notes, I hope they are all right.

@pallinger pallinger requested a review from pdurbin June 4, 2020 09:16
@djbrooke djbrooke moved this from Community Dev 💻❤️ to Code Review 🦁 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Jun 4, 2020
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Code Review 🦁 to QA 🔎✅ Jun 4, 2020
pdurbin
pdurbin approved these changes Jun 4, 2020
View reviewed changes
Copy link
Member

@pdurbin pdurbin 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, @pallinger ! 🎉

@kcondon kcondon self-assigned this Jun 4, 2020
djbrooke
djbrooke approved these changes Jun 4, 2020
View reviewed changes
Copy link
Contributor

@djbrooke djbrooke left a comment

Choose a reason for hiding this comment

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

Thanks @pallinger -- I may make some edits to the release note when we are getting the next release ready, but this gives me all of the information that I need!

@kcondon kcondon merged commit 498c021 into IQSS:develop Jun 5, 2020
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 Jun 5, 2020
@pallinger pallinger deleted the ShibAttributeCharacterSetConversionEnabled branch June 6, 2020 10:16
@djbrooke djbrooke added this to the Dataverse 5 milestone Jun 8, 2020
@pdurbin pdurbin mentioned this pull request Apr 4, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djbrooke djbrooke djbrooke approved these changes

@pdurbin pdurbin Awaiting requested review from pdurbin

Assignees

@kcondon kcondon

Labels
None yet
Projects
No open projects
IQSS/dataverse (TO BE RETIRED / DELET...
Done 🚀
Milestone
Dataverse 5
Development

Successfully merging this pull request may close these issues.

Shibboleth registration results in badly converted accented characters in user's name
5 participants
@pallinger @djbrooke @coveralls @pdurbin @kcondon