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

Shibboleth: Remote Authentication Phase 1 #3025

Merged
merged 74 commits into from
May 13, 2016
Merged

Shibboleth: Remote Authentication Phase 1 #3025

merged 74 commits into from
May 13, 2016

Conversation

pdurbin
Copy link
Member

@pdurbin pdurbin commented Mar 16, 2016

RFI Checklist

1. Related Issues


2. Pull Request Checklist


3. Review Checklist

After the pull request has been submitted, fill out this section.

  • Code review completed or waived
  • Testing requirements completed
  • Usability testing completed or waived
  • Support testing completed or waived
  • Merged with develop branch and resolved conflicts

pdurbin and others added 30 commits February 12, 2016 11:52
The basics are in place but code should be cleaned up more and
refactored. Started working a bit on user management. Added PROVIDER_ID
static field for Shib provider.
…mpty render logic to the Affiliation and Position fields. Fixed broken tooltip issue on the Edit Account Information pg. [ref #2046]
…d a link to the Contact Dataverse Support popup. Also added this link to the error message text for 404 and 403 errors. [ref #2046 #2951]
Redirect to 403 (unauthorized) if you try to hack the URL.

Also, only show "user.signup.tip" when you are really about to create.
- Put email addresses throught the same "find single value" logic
  originally developed in #1608 for multiple first and last names.
- Add `@ValidateEmail` to the "email" field on AuthenticatedUser to
  match BuiltinUser.
- Add null check added to EmailValidator to make it testable.
- Add `INVALID_EMAIL` and `MISSING_REQUIRED_ATTR` modes for Shib testing
  in dev.
- Remove red warning when TestShib doesn't provide "mail" attribute.
- Catch authSvc.createAuthenticatedUser exceptions and handle errors
  better.
- Reformat code (getPrettyFacesHomePageString seems ok).
- Some debugging relates to groups #105
- Some code cleanup.
I believe this is a reversal of an earlier design decision but
@mheppler said it should be removed so I'm removing it:

#2950 (comment)
Too buggy. Sometimes you can't log in, especially when clearing history.
Also catch exception calling authSvc.authenticate when builtinuser
password is not populated. Reformat code.
- Documente API to migrate Shib user to local #2915.
- Add Debugging section for #2916.
- Document identity federation stuff #2937.
- Reference :AllowSignup as part of "remote only" #2838.
Having this endpoint makes documenting the conversion from Shib to
builtin for #2915 easier.
This is for #2975. still need to add pictures in.
fixed some inconsistencies and typos
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 5.168% when pulling 3b5503e on 2939-shib into 5e38c9c on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 5.167% when pulling 5dba82d on 2939-shib into 5e38c9c on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 5.167% when pulling 5dba82d on 2939-shib into 5e38c9c on develop.

@eaquigley
Copy link
Contributor

Good on my end. Passing to @kcondon to merge.

@eaquigley eaquigley assigned kcondon and unassigned eaquigley May 11, 2016
@pdurbin
Copy link
Member Author

pdurbin commented May 12, 2016

The merging of several pull requests yesterday has resulted in the message, "This branch has conflicts that must be resolved." I'm stealing this pull request back to resolve merge conflicts.

murphy:dataverse pdurbin$ git merge develop
Auto-merging src/test/java/edu/harvard/iq/dataverse/util/BundleUtilTest.java
CONFLICT (content): Merge conflict in src/test/java/edu/harvard/iq/dataverse/util/BundleUtilTest.java
Auto-merging src/main/webapp/dataverseuser.xhtml
CONFLICT (content): Merge conflict in src/main/webapp/dataverseuser.xhtml
Auto-merging src/main/webapp/dataverse_header.xhtml
Auto-merging src/main/webapp/dataset.xhtml
Auto-merging src/main/java/Bundle.properties
CONFLICT (content): Merge conflict in src/main/java/Bundle.properties
Auto-merging pom.xml
Automatic merge failed; fix conflicts and then commit the result.

@pdurbin pdurbin assigned pdurbin and unassigned kcondon May 12, 2016
Conflicts from #3108 (3089-jsoup):
	src/main/java/Bundle.properties
	src/main/webapp/dataverseuser.xhtml
	src/test/java/edu/harvard/iq/dataverse/util/BundleUtilTest.java

- Had to fix welcome notification on signup (no more
  notification.welcome1 and notification.welcome2)
- Apply dataverse.saved.search.success fix from 09454b2
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 5.167% when pulling fe5b00d on 2939-shib into be5b26e on develop.

@pdurbin
Copy link
Member Author

pdurbin commented May 12, 2016

@scolapasta it seems that merging #3108 (3089-jsoup) into develop cause several merge conflicts for this pull request. I just merged "develop" into 2939-shib (this pull request) and resolved conflicts in fe5b00d. That commit message has the details of the nature of the conflict but I'll repeat them here:

Conflicts from #3108 (3089-jsoup):
src/main/java/Bundle.properties
src/main/webapp/dataverseuser.xhtml
src/test/java/edu/harvard/iq/dataverse/util/BundleUtilTest.java

  • Had to fix welcome notification on signup (no more
    notification.welcome1 and notification.welcome2)
  • Apply dataverse.saved.search.success fix from 09454b2

@scolapasta I'm going to pass this pull request to you to see what you think. If you could also please let me know if you think I should to the refactoring and cleanup I mentioned the other day, I'd appreciate it. I'm feeling a bit guilty about about the state of the code in the "convertShibToBuiltIn" method I added to the API:

* @todo Refactor more of this business logic into ShibServiceBean in case
. I hope the todo's there explain how the logic should be centralized (so it can be called from the GUI someday as well) but my biggest concern is the "half converted" stuff... ideally we would write to the "builtinuser" and "authenticateduser" tables in a single (EJB?) transaction. I'm not quite sure how to do this.

@pdurbin pdurbin assigned scolapasta and unassigned pdurbin May 12, 2016
@scolapasta scolapasta merged commit ce4f43e into develop May 13, 2016
@scolapasta scolapasta added this to the 4.4 milestone May 13, 2016
@pdurbin
Copy link
Member Author

pdurbin commented May 16, 2016

@scolapasta gave me the go ahead to clean up the "convertShibToBuiltIn" code, which I did in a new pull request: #3120.

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.

10 participants