-
Notifications
You must be signed in to change notification settings - Fork 8
Move necessary assets and configuration out for production deployment #2
Changes from 1 commit
6ea4a96
1663a26
6e29482
87d66a5
2cdd185
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,14 +3,14 @@ class CustomAuthenticator < CASServer::Authenticators::Base | |
| def self.setup(options) | ||
| raise CASServer::AuthenticatorError, "Authenticator configuration needs server" unless options[:server] | ||
|
|
||
| @server = options[:server] | ||
| @port = options[:port] ? options[:port].to_i : 80 | ||
| @ssl = options[:ssl] ? options[:ssl].to_b : false | ||
| @@server = options[:server] | ||
| @@port = options[:port] ? options[:port].to_i : 80 | ||
| @@ssl = options[:ssl] ? options[:ssl].to_b : false | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto on above. We should default to true and configure it otherwise for dev setups. |
||
| end | ||
|
|
||
| def validate(credentials) | ||
| http = Net::HTTP.new(@server, @port) | ||
| if @ssl | ||
| http = Net::HTTP.new(@@server, @@port) | ||
| if @@ssl | ||
| http.use_ssl = true | ||
| http.verify_mode = OpenSSL::SSL::VERIFY_NONE # self-signed cert would fail | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, we don't want to have this line in production code, right? We want self-signed certs to fail.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On Wed, Sep 24, 2014 at 10:44:22AM -0700, Brian Sadler wrote:
Probably, as long as we're not using them anymore (I wasn't sure about that). Maybe it should be a config option too. |
||
| end | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should default to 443 if not configured. Login should inherently be secure, so using SSL should be the default.