-
Notifications
You must be signed in to change notification settings - Fork 68
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
API to use mechanism specific OID #62
Conversation
Travis failure seems to be due npm caching nodejs/node#3922 |
@christkv I'm adding Kerberos Integrated authentication to Tedious driver, it is using your kerberos package and this PR as dependency. It would be great if you can review this PR, so I can add features to the Tedious driver. With regards to the Travis failure, it's happening in the master branch too, for Node v8.4.0 (works fine on my dev machine though). I'm investigating & looking for a solution, will submit PR when I find one. |
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.
@v-suhame Many apologies for our tardiness in reviewing this, it appears notifications were shut off during the migration to the mongodb-js
org. I just have one comment here, and I think we can merge this.
lib/kerberos.js
Outdated
|
||
// uses mechanism specific OID | ||
Kerberos.prototype.authGSSClientInitDefault = function(uri, flags, credentialsCache, callback) { | ||
return authClientInit.call(this, uri, flags, credentialsCache, callback, Kerberos.GSS_C_NO_OID); |
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.
convention is that callback
should always be the final parameter for a function, its fine to break the API here because this is private implementation - could you please swap the oid
and callback
parameters?
Hey @mbroadst, thanks for reviewing it 😄 I have updated the PR. |
@v-suhame thanks for the update! I took a deeper look into this change, and the state of the module, and have decided to boost priority on a general cleanup. We will be incorporating your changes, but as a greater refactor slated for probably a few weeks from now. Thanks for your help! I'll keep you posted on when it is merged. |
Hi @Suraiya-Hameed, this functionality has been merged into the v1.0.0 release. Thank you for you contributions, hope the new module works for y'all! |
Adding API
Kerberos#authGSSClientInitDefault
that uses GSS_C_NO_OID to generate security context. This will extend the usage of package to HTTP and SPN based services.Fixes #13