-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
VAULT 18227/introduce cap ldap library #22185
Changes from 3 commits
be83f12
75b1b9c
c6cd37f
b391c03
c7203fd
011a850
11af2cd
c4f3530
e85b139
cc3b46d
2bbc7b2
1449774
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ import ( | |
"fmt" | ||
"strings" | ||
|
||
"github.com/hashicorp/cap/ldap" | ||
"github.com/hashicorp/go-secure-stdlib/strutil" | ||
|
||
"github.com/hashicorp/vault/sdk/framework" | ||
|
@@ -76,82 +77,25 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri | |
return "", nil, logical.ErrorResponse("password cannot be of zero length when passwordless binds are being denied"), nil, nil | ||
} | ||
|
||
ldapClient := ldaputil.Client{ | ||
Logger: b.Logger(), | ||
LDAP: ldaputil.NewLDAP(), | ||
} | ||
|
||
c, err := ldapClient.DialLDAP(cfg.ConfigEntry) | ||
ldapClient, err := ldap.NewClient(ctx, ldaputil.ConvertConfig(cfg.ConfigEntry)) | ||
if err != nil { | ||
return "", nil, logical.ErrorResponse(err.Error()), nil, nil | ||
} | ||
if c == nil { | ||
return "", nil, logical.ErrorResponse("invalid connection returned from LDAP dial"), nil, nil | ||
} | ||
|
||
// Clean connection | ||
defer c.Close() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of the code deleted below is implemented via the We do need to specify options to also get |
||
|
||
userBindDN, err := ldapClient.GetUserBindDN(cfg.ConfigEntry, c, username) | ||
if err != nil { | ||
if b.Logger().IsDebug() { | ||
b.Logger().Debug("error getting user bind DN", "error", err) | ||
} | ||
return "", nil, logical.ErrorResponse(errUserBindFailed), nil, logical.ErrInvalidCredentials | ||
} | ||
|
||
if b.Logger().IsDebug() { | ||
b.Logger().Debug("user binddn fetched", "username", username, "binddn", userBindDN) | ||
} | ||
|
||
// Try to bind as the login user. This is where the actual authentication takes place. | ||
if len(password) > 0 { | ||
err = c.Bind(userBindDN, password) | ||
} else { | ||
err = c.UnauthenticatedBind(userBindDN) | ||
} | ||
if err != nil { | ||
if b.Logger().IsDebug() { | ||
b.Logger().Debug("ldap bind failed", "error", err) | ||
} | ||
return "", nil, logical.ErrorResponse(errUserBindFailed), nil, logical.ErrInvalidCredentials | ||
} | ||
defer ldapClient.Close(ctx) | ||
|
||
// We re-bind to the BindDN if it's defined because we assume | ||
// the BindDN should be the one to search, not the user logging in. | ||
if cfg.BindDN != "" && cfg.BindPassword != "" { | ||
if err := c.Bind(cfg.BindDN, cfg.BindPassword); err != nil { | ||
if b.Logger().IsDebug() { | ||
b.Logger().Debug("error while attempting to re-bind with the BindDN User", "error", err) | ||
} | ||
return "", nil, logical.ErrorResponse("ldap operation failed: failed to re-bind with the BindDN user"), nil, logical.ErrInvalidCredentials | ||
} | ||
if b.Logger().IsDebug() { | ||
b.Logger().Debug("re-bound to original binddn") | ||
} | ||
} | ||
|
||
userDN, err := ldapClient.GetUserDN(cfg.ConfigEntry, c, userBindDN, username) | ||
c, err := ldapClient.Authenticate(ctx, username, password, ldap.WithGroups(), ldap.WithUserAttributes()) | ||
if err != nil { | ||
return "", nil, logical.ErrorResponse(err.Error()), nil, nil | ||
} | ||
|
||
if cfg.AnonymousGroupSearch { | ||
c, err = ldapClient.DialLDAP(cfg.ConfigEntry) | ||
if err != nil { | ||
return "", nil, logical.ErrorResponse("ldap operation failed: failed to connect to LDAP server"), nil, nil | ||
if strings.Contains(err.Error(), "discovery of user bind DN failed") || | ||
strings.Contains(err.Error(), "unable to bind user") { | ||
Comment on lines
+90
to
+91
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A little manual, as the |
||
return "", nil, logical.ErrorResponse(errUserBindFailed), nil, logical.ErrInvalidCredentials | ||
} | ||
defer c.Close() // Defer closing of this connection as the deferal above closes the other defined connection | ||
} | ||
|
||
ldapGroups, err := ldapClient.GetLdapGroups(cfg.ConfigEntry, c, userDN, username) | ||
if err != nil { | ||
return "", nil, logical.ErrorResponse(err.Error()), nil, nil | ||
} | ||
if b.Logger().IsDebug() { | ||
b.Logger().Debug("groups fetched from server", "num_server_groups", len(ldapGroups), "server_groups", ldapGroups) | ||
} | ||
|
||
ldapGroups := c.Groups | ||
ldapResponse := &logical.Response{ | ||
Data: map[string]interface{}{}, | ||
} | ||
|
@@ -162,6 +106,10 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri | |
ldapResponse.AddWarning(errString) | ||
} | ||
|
||
for _, warning := range c.Warnings { | ||
ldapResponse.AddWarning(string(warning)) | ||
} | ||
|
||
var allGroups []string | ||
canonicalUsername := username | ||
cs := *cfg.CaseSensitiveNames | ||
|
@@ -206,13 +154,11 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri | |
return username, policies, ldapResponse, allGroups, nil | ||
} | ||
|
||
entityAliasAttribute, err := ldapClient.GetUserAliasAttributeValue(cfg.ConfigEntry, c, username) | ||
if err != nil { | ||
return "", nil, logical.ErrorResponse(err.Error()), nil, nil | ||
} | ||
if entityAliasAttribute == "" { | ||
Comment on lines
-209
to
-213
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The implementation of this can be found here. I believe it boils down to finding the 'CN' (common name) attribute. |
||
cn := c.UserAttributes["cn"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure this is quite correct. It appears that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, thanks for catching this! |
||
if len(cn) == 0 { | ||
return "", nil, logical.ErrorResponse("missing entity alias attribute value"), nil, nil | ||
} | ||
entityAliasAttribute := cn[0] | ||
|
||
return entityAliasAttribute, policies, ldapResponse, allGroups, nil | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
```release-note:improvement | ||
auth/ldap: introduce cap/ldap.Client for LDAP authentication | ||
auth/ldap: deprecates `connection_timeout` in favor of `request_timeout` for timeouts | ||
sdk/ldaputil: deprecates Client in favor of cap/ldap.Client | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,21 +6,15 @@ package ldap | |
import ( | ||
"context" | ||
"fmt" | ||
"runtime" | ||
"strings" | ||
"testing" | ||
|
||
hclog "github.com/hashicorp/go-hclog" | ||
"github.com/hashicorp/cap/ldap" | ||
|
||
"github.com/hashicorp/vault/sdk/helper/docker" | ||
"github.com/hashicorp/vault/sdk/helper/ldaputil" | ||
) | ||
|
||
func PrepareTestContainer(t *testing.T, version string) (cleanup func(), cfg *ldaputil.ConfigEntry) { | ||
// Skipping on ARM, as this image can't run on ARM architecture | ||
if strings.Contains(runtime.GOARCH, "arm") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was able to run these tests locally on my Mac M1, but if this causes test failures I'll revert these changes. |
||
t.Skip("Skipping, as this image is not supported on ARM architectures") | ||
} | ||
|
||
runner, err := docker.NewServiceRunner(docker.RunOptions{ | ||
// Currently set to "michelvocks" until https://github.com/rroemhild/docker-test-openldap/pull/14 | ||
// has been merged. | ||
|
@@ -48,19 +42,16 @@ func PrepareTestContainer(t *testing.T, version string) (cleanup func(), cfg *ld | |
svc, err := runner.StartService(context.Background(), func(ctx context.Context, host string, port int) (docker.ServiceConfig, error) { | ||
connURL := fmt.Sprintf("ldap://%s:%d", host, port) | ||
cfg.Url = connURL | ||
logger := hclog.New(nil) | ||
client := ldaputil.Client{ | ||
LDAP: ldaputil.NewLDAP(), | ||
Logger: logger, | ||
} | ||
|
||
conn, err := client.DialLDAP(cfg) | ||
client, err := ldap.NewClient(ctx, ldaputil.ConvertConfig(cfg)) | ||
if err != nil { | ||
return nil, err | ||
} | ||
defer conn.Close() | ||
|
||
if _, err := client.GetUserBindDN(cfg, conn, "Philip J. Fry"); err != nil { | ||
defer client.Close(ctx) | ||
|
||
_, err = client.Authenticate(ctx, "Philip J. Fry", "fry") | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
|
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.
👋🏼