Repository navigation
CXH-3209: register go-mssqldb's krb5 authenticator for keytab auth #156
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| package sqlserver | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/microsoft/go-mssqldb/integratedauth" | ||
| "github.com/microsoft/go-mssqldb/msdsn" | ||
| ) | ||
|
|
||
| // Test_Krb5ProviderRegistered proves the blank import in sqlserver.go registers | ||
| // the "krb5" integrated-auth provider. Normative wording (no specific upstream | ||
| // error label): the assertion is err != nil AND the error message does not | ||
| // contain "not found". Any non-"not found" error from GetIntegratedAuthenticator | ||
| // proves the registry lookup reached the krb5 provider rather than the miss | ||
| // `provider %v not found`; upstream may reword its config-load or keytab error | ||
| // messages, so no specific upstream string is asserted here. | ||
|
Comment on lines
+11
to
+17
There was a problem hiding this comment. nit: This comment is long and reads like review notes. Cut it to one or two lines, e.g. |
||
| func Test_Krb5ProviderRegistered(t *testing.T) { | ||
| config := msdsn.Config{ | ||
| Parameters: map[string]string{ | ||
| "authenticator": "krb5", | ||
| "krb5-realm": "EXAMPLE.COM", | ||
| "krb5-keytabfile": "/nonexistent", | ||
| "krb5-configfile": "/nonexistent", | ||
| }, | ||
| } | ||
|
|
||
| // Both paths point at /nonexistent, so the error is deterministic without touching /etc/krb5.conf. | ||
| _, err := integratedauth.GetIntegratedAuthenticator(config) | ||
| if err == nil { | ||
| t.Fatalf("expected an error from the krb5 provider for a nonexistent krb5 config, got nil") | ||
| } | ||
| if strings.Contains(err.Error(), "not found") { | ||
|
c1-squire-dev[bot] marked this conversation as resolved.
|
||
| t.Fatalf("krb5 provider not registered (lookup missed the registry): %v", err) | ||
| } | ||
| } | ||
|
|
||
| // Test_ProviderMissMessageControl is the negative control for | ||
| // Test_Krb5ProviderRegistered: it asserts the upstream registry-miss error | ||
| // still contains "not found". If upstream ever rewords that miss message, THIS | ||
| // test fails loudly and the guard's "not found" check in | ||
| // Test_Krb5ProviderRegistered must be updated to match. | ||
| func Test_ProviderMissMessageControl(t *testing.T) { | ||
| config := msdsn.Config{ | ||
| Parameters: map[string]string{ | ||
| "authenticator": "krb5-no-such-provider", | ||
| }, | ||
| } | ||
|
|
||
| _, err := integratedauth.GetIntegratedAuthenticator(config) | ||
| if err == nil { | ||
| t.Fatalf("expected a registry-miss error for a bogus authenticator, got nil") | ||
| } | ||
| if !strings.Contains(err.Error(), "not found") { | ||
| t.Fatalf("upstream registry-miss message no longer contains \"not found\"; update the guard in Test_Krb5ProviderRegistered: %v", err) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| //go:build !windows | ||
|
|
||
| package sqlserver | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/microsoft/go-mssqldb/integratedauth" | ||
| ) | ||
|
|
||
| // Test_DefaultProviderNameUnchanged guards that registering the krb5 provider | ||
| // does not change the platform default authenticator, which stays "ntlm" on | ||
| // unix (per auth_unix.go init() being its only unix writer). | ||
|
Comment on lines
+11
to
+13
There was a problem hiding this comment. nit: Drop "(per auth_unix.go init() being its only unix writer)". |
||
| func Test_DefaultProviderNameUnchanged(t *testing.T) { | ||
| if integratedauth.DefaultProviderName != "ntlm" { | ||
| t.Fatalf("DefaultProviderName = %q, want %q", integratedauth.DefaultProviderName, "ntlm") | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back 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.
nit: Consider also covering
connect.userwith noconnect.passwordplusconnect.params(the README's recommended style), since most users configure through params, not a raw DSN. Also note that the keytab and krb5.conf paths must exist inside the connector's runtime (container mount for K8s).