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 option for full_host to allow Clever login from non-secure pages #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joe1chen
Copy link
Contributor

This change allows you to specify the full_host for the Clever omniauth.

Example using devise:

config.omniauth :clever, Settings.clever.news.client_id, Settings.clever.news.client_secret, { name: 'clever', provider_ignores_state: true, full_host: 'https://myhost.com' } 

Note that you can also set the full_host for all omniauth libraries with the following code:

OmniAuth.config.full_host = "https://myhost.com"

However, since this affects all other oauth logins, we would prefer not to have to re-test everything.

BTW, this change has been used in production for over a year, but seeing now that there is an official Clever fork, we'd like to contribute this back.

@cdsmith16
Copy link

Hi @joe1chen and @jbusser , the one concerning element of this change is the following comment:

option :full_host
+

  •  # Allows full host to be overridden. This is important because Clever is forcing https in production,
    
  •  # so we need to use https redirect url even when our host page is http.
    

Clever's security policy necessitates that all production url's are SSL-signed , lest we send student PII to an unsecure server. Any thoughts?

@jbusser
Copy link

jbusser commented Sep 2, 2016

Our app routes all traffic over HTTPS, so this option doesn't give me anything. That said, I appreciate Clever's position on the matter, and if you decline to merge this change, I would respect your decision.

@joe1chen
Copy link
Contributor Author

joe1chen commented Sep 2, 2016

Our website does not force you to use SSL, unless you decide to login. So
this change is require me when you are using a mixed SSL/non-SSL website.

Bottom line is when you are using Clever with our website, you are using
SSL for all communication between our site and Clever.

So this pull request is to support this kind of mixed SSL/non-SSL
environment. We have been using this in production for over a year.

On Thursday, September 1, 2016, csmooth [email protected] wrote:

Hi @joe1chen https://github.com/joe1chen and @jbusser
https://github.com/jbusser , the one concerning element of this change
is the following comment:

option :full_host
+

  • Allows full host to be overridden. This is important because

    Clever is forcing https in production,
  • so we need to use https redirect url even when our host page is

    http.

Clever's security policy necessitates that all production url's are
SSL-signed , lest we send student PII to an unsecure server. Any thoughts?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#5 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC-AxE9YpMilVvJwLVl2HV83ivHDpQXks5ql1NJgaJpZM4Jo36e
.

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