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 authentication in production - DO NOT MERGE #274

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

bess
Copy link
Contributor

@bess bess commented Jan 25, 2018

With local database authentication in dev environment

I did not squash these PRs because they track the steps I documented here: https://github.com/curationexperts/playbook/blob/shibboleth/authentication/shibboleth_and_hyrax.md

I think it might be handy for future reference to have one PR per documented step. What do you think?

Closes #18

Do not merge this until @mark-dce can weigh in on whether we want to proceed with this before we have access to OHSU Shibboleth systems.

Add uid field and index it: uid will now be the primary identifier
Add provider field to record which authentication method created a given user
Add uid field to User factory
…pment environment

Always use database auth in test environment
If a user we haven't seen before logs in via shibboleth
create an account for them.
@bess bess requested a review from no-reply January 25, 2018 20:01
@bess bess changed the title Shibboleth authentication in production Shibboleth authentication in production - DO NOT MERGE Jan 25, 2018
@bess bess requested a review from mark-dce January 25, 2018 20:01
@coveralls
Copy link

coveralls commented Jan 25, 2018

Coverage Status

Coverage decreased (-2.6%) to 95.296% when pulling 8de8f2f on shib_auth_3 into e317d49 on master.

no-reply
no-reply previously approved these changes Jan 25, 2018
Copy link
Collaborator

@no-reply no-reply left a comment

Choose a reason for hiding this comment

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

This looks fabulous. Thank you!

Do you have thoughts about how best to handle creator for importer, in a Shibboleth authenticated world? I'm unsure whether changes will be necessary to that work (merged as #273) when rebasing this.

@@ -16,6 +16,7 @@ Rails/TimeZone:
RSpec/MultipleExpectations:
Exclude:
- 'spec/features/**/*'
- 'spec/models/user_spec.rb'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it team opinion that multiple expectations are good (or at least acceptable) across the board? If so, I want to flag this rule for removal in bixby.

Either way, I'm fine with the relevant specs here for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think multiple expectations are fine and even desirable, especially in our context where test setup can be expensive and we want things to run faster. I'd be very happy to see this removed from bixby.

Copy link
Collaborator

Choose a reason for hiding this comment

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

# methods aren't available.
def self.use_database_auth?
return true if Rails.env.test?
Rails.env.development? && ENV['DATABASE_AUTH'] == 'true'
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 really nice.

no-reply pushed a commit that referenced this pull request Jan 26, 2018
This removes the need for a hard coded user with a password, using Hyrax's
system user functionality instead. This prefigures Shibboleth support in #274.
no-reply pushed a commit that referenced this pull request Jan 26, 2018
This removes the need for a hard coded user with a password, using Hyrax's
system user functionality instead. This prefigures Shibboleth support in #274.
no-reply pushed a commit that referenced this pull request Jan 26, 2018
This removes the need for a hard coded user with a password, using Hyrax's
system user functionality instead. This prefigures Shibboleth support in #274.
no-reply pushed a commit that referenced this pull request Jan 26, 2018
This removes the need for a hard coded user with a password, using Hyrax's
system user functionality instead. This prefigures Shibboleth support in #274.
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.

3 participants