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

API to use mechanism specific OID #62

Closed
wants to merge 4 commits into from

Conversation

Suraiya-Hameed
Copy link

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

@Suraiya-Hameed
Copy link
Author

Travis failure seems to be due npm caching nodejs/node#3922

@Suraiya-Hameed
Copy link
Author

@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.

Copy link
Member

@mbroadst mbroadst left a 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);
Copy link
Member

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?

@Suraiya-Hameed
Copy link
Author

Hey @mbroadst, thanks for reviewing it 😄 I have updated the PR.

@mbroadst
Copy link
Member

@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.

@mbroadst
Copy link
Member

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!

@mbroadst mbroadst closed this Aug 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants