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

Add force_ssl flag to Riak Control. #164

Merged
merged 1 commit into from Oct 8, 2013
Merged

Conversation

cmeiklejohn
Copy link
Contributor

Separate out authentication type and forced SSL requirements.
Specifically, a user should be able to force SSL when using no
authentication, or force non-SSL when using authentication given they
may already be on a protected network.

@seancribbs @joedevivo

Separate out authentication type and forced SSL requirements.
Specifically, a user should be able to force SSL when using no
authentication, or force non-SSL when using authentication given they
may already be on a protected network.
@seancribbs
Copy link

Discussed this in HipChat, but a third option would be to default to the old way when this setting was undefined. This would make upgrades less surprising.

@cmeiklejohn
Copy link
Contributor Author

I just pushed an updated commit, 85378b7, which I believe addresses the upgrade concern correctly.

@cmeiklejohn
Copy link
Contributor Author

Addressed by #164.

@cmeiklejohn
Copy link
Contributor Author

Rather, addresses #156.

_ ->
case wrq:scheme(RD) of
https ->
case app_helper:get_env(riak_control, force_ssl, undefined) of

Choose a reason for hiding this comment

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

undefined is the default.

@seancribbs
Copy link

This looks good at first glance, I'll give it a test tomorrow.

@cmeiklejohn
Copy link
Contributor Author

Did you get a chance to test this?

@seancribbs
Copy link

  • auth:none + force_ssl:undefined = OK
  • auth:none + force_ssl:false = OK
  • auth:none + force_ssl:true = Redirect to HTTPS
  • auth:userlist + force_ssl:undefined = Redirect to HTTPS
  • auth:userlist + force_ssl:false = OK
  • auth:userlist + force_ssl:true = Redirect to HTTPS

@seancribbs
Copy link

All possible combinations check out as expected. 👍 to merge

@cmeiklejohn
Copy link
Contributor Author

Perfect; thanks.

cmeiklejohn added a commit that referenced this pull request Oct 8, 2013
Add force_ssl flag to Riak Control.
@cmeiklejohn cmeiklejohn merged commit 09e3d89 into develop Oct 8, 2013
@cmeiklejohn cmeiklejohn deleted the feature/csm/add-ssl-flag branch October 8, 2013 22:24
@cmeiklejohn
Copy link
Contributor Author

basho/riak_test#412 covers a majority of the cases here, but is not exhaustive.

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.

None yet

2 participants