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

Added Globus as an identity provider #158

Merged
merged 1 commit into from
Dec 12, 2018

Conversation

NickolausDS
Copy link
Contributor

  • Globus Login work for any provider Globus Auth supports
  • Added tests, coverage of new code is 100%
  • Added Sphinx docs

@lnielsen
Copy link
Member

Thanks a lot for the contribution. We'll have to keep the contribution on hold for a bit because we're changing the license of Invenio from GPL to MIT. Unfortunately, discussion with our lawyers is taking longer than expected, and thus we cannot integrate code from non-CERN contributors until this has finished. I'm sorry about this.

@lnielsen lnielsen self-assigned this Jan 30, 2018
@NickolausDS
Copy link
Contributor Author

Thanks for the update, I hope the license transition goes smoothly.

@lnielsen lnielsen added this to Backlog in Invenio Sprint Dec 3 - Dec 14 2018 via automation Nov 30, 2018
@lnielsen lnielsen moved this from Backlog to Pending review in Invenio Sprint Dec 3 - Dec 14 2018 Dec 2, 2018
@lnielsen lnielsen removed their assignment Dec 3, 2018
@slint slint self-assigned this Dec 3, 2018
Copy link
Member

@slint slint left a comment

Choose a reason for hiding this comment

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

Tested locally (with CERN institute) and works great! :shipit:

(ping @lnielsen)

@slint
Copy link
Member

slint commented Dec 3, 2018

@NickolausDS we can merge this, though could you tell us what legal entity is the copyright holder for this code, so we can include it in the headers? Also, please check if the case is that since this code might have been written as part of an employment contract which grants your employer the copyright to all source code you write (keep in mind that this could also apply for code written outside of working hours), the copyright holder might be your employer.

@slint slint moved this from Pending review to Pending merge in Invenio Sprint Dec 3 - Dec 14 2018 Dec 3, 2018
@slint slint removed their assignment Dec 3, 2018
@rpwagner
Copy link

rpwagner commented Dec 3, 2018

@slint, @NickolausDS did write this as part of his employment and the copyright holder is the University of Chicago. You can see an example of our copyright headers in some of our code.

Do you need any more information or is that sufficient?

Thanks

@slint slint moved this from Pending merge to Pending review in Invenio Sprint Dec 3 - Dec 14 2018 Dec 5, 2018
@slint slint moved this from Pending review to In progress in Invenio Sprint Dec 3 - Dec 14 2018 Dec 5, 2018
@lnielsen lnielsen self-assigned this Dec 12, 2018
* Adds support for logging in with Globus for any provider that Globus
  Auth supports.
@lnielsen
Copy link
Member

@rpwagner @NickolausDS - I've rebased the PR and updated the copyright headers. Last thing before I press the merge button, could you please confirm that you license your PR under MIT License (since the old headers said GPL - which was probably just due to copy/paste)?

@lnielsen lnielsen moved this from In progress to Pending merge in Invenio Sprint Dec 3 - Dec 14 2018 Dec 12, 2018
@NickolausDS
Copy link
Contributor Author

@lnielsen Yes, confirming here to license this pull request under MIT. Thanks for switching that.

@lnielsen lnielsen merged commit 8d11466 into inveniosoftware:master Dec 12, 2018
Invenio Sprint Dec 3 - Dec 14 2018 automation moved this from Pending merge to Done Dec 12, 2018
@lnielsen
Copy link
Member

Merged. Thanks a lot for the contribution, and apologies for taking so long to merge.

@NickolausDS
Copy link
Contributor Author

No need for an apology, I know licensing can get complex and take time to sort out all the details.

Thanks for picking this back up, your help was much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants