-
Notifications
You must be signed in to change notification settings - Fork 26
Support referral chasing for Active Directory member validation #95
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
Conversation
- Set empty search base in user? rather than in GitHub::Ldap - Get rid of indirection in Domain to global_catalog_search - Update test
test/referral_chaser_test.rb
Outdated
| admin_user: "Joe", | ||
| admin_password: "passworD1", | ||
| port: 389 | ||
| }) |
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.
Want to use a mock here too? Or use the options hash to pull configuration for working connections?
test/referral_chaser_test.rb
Outdated
| admin_user: "uid=admin,dc=github,dc=com", | ||
| admin_password: "passworD1", | ||
| port: 389 | ||
| }) |
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.
Still need to use the options defined in GitHub::Ldap::Test for the right host.
Note, though, that these failures are because capabilities are being checked to aggressively, causing a query to occur when creating a new object, rather than lazily loading the capabilities as needed. This is out-of-scope from this PR, but it's a constraint to getting the build to pass now.
Also use that port value, as it changes depending on the integration test LDAP server in use.
Stylistic preference to be more consistent with other tests.
|
Looks like we're 🍏. Let's get this merged, @davesims. |
This PR adds support for chasing referrals during an Active Directory member validation. Added a
ReferralChaserobject that wrapsGitHub::Ldap#searchfunctionality with the ability to pursue any referred URLs that come back from the server. Will replace #91 and depends on #94./cc #94 #91
/cc @mtodd @jch @jatoben @lildude @sbryant
/cc @github/platform-iam