feat(NODE-6333): Allow callers to specify the 'protect' flag

[arabull]

Description

What is changing?

Allow callers to specify the protect flag. I did this by adding a new BooleanToIntWithNonIntAsFalse() function modeled off of ToStringWithNonStringAsEmpty. I use the new function to interpret the contents of options["protect"].

I am not married to that function name and am happy to change it if there is a better choice.

Is there new documentation needed for these changes?

I've updated the README.

What is the motivation for this change?

Our server requires protected payloads. The library handles this just fine, but the protect flag is currently hard-coded to 0. The caller should be able to influence its value.

Release Highlight

protect is now an option for KerberosClient.wrap()

protect can be provided to KerberosClient.wrap(). When configured, wrapped message will be encrypted.

Thanks @arabull for this contribution!

Double check the following

• Ran npm run check:lint script
• Self-review completed using the steps outlined here
• PR title follows the correct format: type(NODE-xxxx)[!]: description
  • Example: feat(NODE-1234)!: rewriting everything in coffeescript
• Changes are covered by tests
  • The fix was modeled after ToStringWithNonStringAsEmpty(). I didn't see any tests for that guy and am unsure how to properly test the new function.
• New TODOs have a related JIRA ticket
  • n/a

[aditi-khare-mongoDB]

Hi @arabull, thank you for your submission! For this change to be reviewed, it needs to include testing for the new feature. Would you like to add testing yourself or defer to our team?

If you'd like to test your changes locally, you can add tests in test/kerberos_tests.js and run them in a docker image using the following command:

docker run -i -v path_to_kerberos_repo:/app -w /app -e PROJECT_DIRECTORY=/app ubuntu:20.04 /bin/bash /app/.evergreen/run-tests-ubuntu.sh

[arabull]

Hey @aditi-khare-mongoDB, I hate doing this, but I'll probably need to defer to your team for testing. I looked at test/kerberos_tests.js, and I'm not sure how to test this change. I was looking for tests that exercised the ToStringWithNonStringAsEmpty function for the "user" parameter since that's the closest to what I've done, and I can't find any. It's not clear to me how to test my change in a manner consistent with the current tests.

[aditi-khare-mongoDB]

Thanks for your quick reply! Next week, the team will look at what testing work is needed for this feature and figure out when to schedule it.

[addaleax]

LGTM, agree that ideally we'd want to test this, but I also understand that that's quite tricky for this package

[baileympearson]

@arabull I left a few comments. If its easier for you, I can make the changes and push to your branch. Just let me know

[arabull]

@baileympearson I'll take you up on the offer to make the fixes in my branch. I'll leave it up to you, but I'm not sure about referencing things like protect directly in the new function. It's a generic function. Granted, it's only called from a single place right now, but there's no reason it couldn't be called from elsewhere in the future.

Your call, though. Thanks for helping me out!

[baileympearson]

Hey @arabull , I can't push to your branch because it's on your fork's main. Checking out your changes required me to --detach, and I'm reluctant to push a detached head to your fork's main.

I put my changes in this branch, if you'd like to pull them in: https://github.com/mongodb-js/kerberos/tree/requested-changes

Also note that the tests depend on #202

[arabull]

Hey @arabull , I can't push to your branch because it's on your fork's main. Checking out your changes required me to --detach, and I'm reluctant to push a detached head to your fork's main.

I put my changes in this branch, if you'd like to pull them in: https://github.com/mongodb-js/kerberos/tree/requested-changes

@baileympearson Done, thanks!

[baileympearson]

@arabull Can you update this branch to include the latest commit to main?

[arabull]

@baileympearson Done.