-
Notifications
You must be signed in to change notification settings - Fork 253
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
Define auth adapters #226
Define auth adapters #226
Conversation
#++ | ||
class GSS_SPNEGO < Net::LDAP::AuthAdapter | ||
def bind(auth) | ||
require 'ntlm' |
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.
This adapter depends on ntlm
as the above comments point out. now this can be defined as a gem like net-ldap-auth_adapters-gss_spnego
.
@satoryu thanks for getting this started! It's looking good. |
t3_msg.serialize | ||
} | ||
|
||
bind_sasl(:method => :sasl, :mechanism => "GSS-SPNEGO", |
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.
Why do you need sasl here?
this is because the current version of bind_gss_snego
uses bind_sasl
.
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.
👍
@jch thank you for your comments :) I've fixed all you pointed out. so please check this PR again. |
|
||
class TestAuthAdapter < Test::Unit::TestCase | ||
def test_undefined_auth_adapter | ||
flexmock(TCPSocket).should_receive(:new).ordered.with('ldap.example.com', 379).once.and_return(nil) |
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.
It's too bad that Connection.new
actually tries to open the socket and forces us to stub here. Not suggesting any change, but making an observation.
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.
Thank you for your feedback. I had no idea so I referred to test/test_ldap_connection.rb
What would you do if you were me in this case?
@satoryu thanks for following through with this PR. Would you be interested in making a separate gem for |
@jch Yes. but give me time to implement the separate gem. I guess I would bother much time since there are no test code for BTW, Is the release of v0.12.0 blocked by providing this separate gem? |
@satoryu not blocked. I don't believe there's any breaking changes. |
👍 |
=== Net::LDAP 0.12.1 * Whitespace formatting cleanup {#236}[ruby-ldap/ruby-net-ldap#236] * Set operation result if LDAP server is not accessible {#232}[ruby-ldap/ruby-net-ldap#232] === Net::LDAP 0.12.0 * DRY up connection handling logic {#224}[ruby-ldap/ruby-net-ldap#224] * Define auth adapters {#226}[ruby-ldap/ruby-net-ldap#226] * add slash to attribute value filter {#225}[ruby-ldap/ruby-net-ldap#225] * Add the ability to provide a list of hosts for a connection {#223}[ruby-ldap/ruby-net-ldap#223] * Specify the port of LDAP server by giving INTEGRATION_PORT {#221}[ruby-ldap/ruby-net-ldap#221] * Correctly set BerIdentifiedString values to UTF-8 {#212}[ruby-ldap/ruby-net-ldap#212] * Raise Net::LDAP::ConnectionRefusedError when new connection is refused. {#213}[ruby-ldap/ruby-net-ldap#213] * obscure auth password upon #inspect, added test, closes #216 {#217}[ruby-ldap/ruby-net-ldap#217] * Fixing incorrect error class name {#207}[ruby-ldap/ruby-net-ldap#207] * Travis update {#205}[ruby-ldap/ruby-net-ldap#205] * Remove obsolete rbx-19mode from Travis {#204}[ruby-ldap/ruby-net-ldap#204] * mv "sudo" from script/install-openldap to .travis.yml {#199}[ruby-ldap/ruby-net-ldap#199] * Remove meaningless shebang {#200}[ruby-ldap/ruby-net-ldap#200] * Fix Travis CI build {#202}[ruby-ldap/ruby-net-ldap#202] * README.rdoc: fix travis link {#195}[ruby-ldap/ruby-net-ldap#195]
Define auth adapters
This PR provide a scheme of defining authentication (bind) of LDAP.
By merging this PR, we'll be able to define new auth adapter.
require
d as'net/ldap/auth_adapter/#{auth scheme name}
Net::LDAP::AuthAdapter
Net::LDAP::AuthAdapter.register
I hope this PR would help #210 :)