-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(NODE-2026): support SERVICE_REALM authentication mechanism property #2865
fix(NODE-2026): support SERVICE_REALM authentication mechanism property #2865
Conversation
5aa9efd
to
a075681
Compare
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.
Just for another example here's the logic in python I believe the only issue we have here is the limiting of SERVICE_REALM only on windows.
src/cmap/auth/gssapi.ts
Outdated
if ('SERVICE_REALM' in mechanismProperties) { | ||
spn = `${spn}@${mechanismProperties.SERVICE_REALM}`; | ||
} | ||
); | ||
} else { | ||
spn = `${serviceName}@${host}`; | ||
} |
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.
We want to keep the logic that uses slash instead of @ on windows, but move attaching the service realm to outside so it gets attached regardless of platform.
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.
The test: validate that SERVICE_REALM and CANONICALIZE_HOST_NAME can be passed in
in: test/manual/kerberos.test.js:54
is now going to fail after making this change, you can safely skip it, we have a ticket (NODE-3060) to track unskipping / removing later.
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.
I discussed that with @mcasimir, too... So we want to just attach that regardless of the platform knowing it will not work on any *nix systems? I've no hard feelings on that - I'll add it :)
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.
Yea I'm just looking at python and mirroring their logic, we will revisit this when we have the right testing environment. You were able to confirm it works on windows so I'm happy with what we have here, thanks for this!
a075681
to
c0544fa
Compare
Note: I was also able to successfully test the code in a Windows Kerberos environment (manually). |
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.
LGTM
Introduces support for the
SERVICE_REALM
authentication mechanism property. It will be work as expected in Windows environments due to the underlying nature of the GSSAPI implementation on *nix.The logic to compute the Service Principal Name (SPN) is as follows:
/
in between for Windows and an@
otherwise)SERVICE_REALM
with an@
to the result of step 1.The logic is taken from the C driver where you can find the steps mangled as follows:
[serviceName, hostName, serviceRealm]
with@
(see https://github.com/mongodb/mongo-c-driver/blob/f1093ba1686b8bf876eed67e0c7f59918649ef15/src/libmongoc/src/mongoc/mongoc-cluster-sspi.c#L61-L71)/
in the SPN (which is not the case after step 1), replace the first@
with a/
(see https://github.com/mongodb/mongo-c-driver/blob/5b91fb983fc5badf38e2debeb4ce6c231b73605b/src/libmongoc/src/mongoc/mongoc-sspi.c#L176-L182)I was able to successfully test this behavior in a Windows environment with Kerberos enabled.