From 2c4dc2951dee68bf79141337853a33701614da7b Mon Sep 17 00:00:00 2001 From: Margo Crawford Date: Mon, 25 Oct 2021 16:45:30 -0700 Subject: [PATCH] resolved a couple of testing related todos --- internal/oidc/auth/auth_handler.go | 8 +- internal/upstreamldap/upstreamldap.go | 11 ++- internal/upstreamldap/upstreamldap_test.go | 110 ++++++++++++++++++++- 3 files changed, 117 insertions(+), 12 deletions(-) diff --git a/internal/oidc/auth/auth_handler.go b/internal/oidc/auth/auth_handler.go index cc637b4d4..caff98f32 100644 --- a/internal/oidc/auth/auth_handler.go +++ b/internal/oidc/auth/auth_handler.go @@ -491,12 +491,8 @@ func downstreamSubjectFromUpstreamLDAP(ldapUpstream provider.UpstreamLDAPIdentit } func userDNFromAuthenticatedResponse(authenticatedResponse *authenticator.Response) string { - // These errors shouldn't happen, but do some error checking anyway so it doesn't panic - extra := authenticatedResponse.User.GetExtra() - if len(extra) == 0 { - return "" - } - dnSlice := extra["userDN"] + // This error shouldn't happen, but do some error checking anyway so it doesn't panic + dnSlice := authenticatedResponse.User.GetExtra()["userDN"] if len(dnSlice) != 1 { return "" } diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 9f24f7629..fad037119 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -177,20 +177,21 @@ func (p *Provider) PerformRefresh(ctx context.Context, userDN string, expectedUs conn, err := p.dial(ctx) if err != nil { - p.traceAuthFailure(t, err) + p.traceRefreshFailure(t, err) return fmt.Errorf(`error dialing host "%s": %w`, p.c.Host, err) } defer conn.Close() err = conn.Bind(p.c.BindUsername, p.c.BindPassword) if err != nil { - p.traceAuthFailure(t, err) + p.traceRefreshFailure(t, err) return fmt.Errorf(`error binding as "%s" before user search: %w`, p.c.BindUsername, err) } searchResult, err := conn.Search(search) if err != nil { + p.traceRefreshFailure(t, err) return fmt.Errorf(`error searching for user "%s": %w`, userDN, err) } @@ -765,6 +766,12 @@ func (p *Provider) traceSearchBaseDiscoveryFailure(t *trace.Trace, err error) { trace.Field{Key: "reason", Value: err.Error()}) } +func (p *Provider) traceRefreshFailure(t *trace.Trace, err error) { + t.Step("refresh failed", + trace.Field{Key: "reason", Value: err.Error()}, + ) +} + func MicrosoftUUIDFromBinary(attributeName string) func(entry *ldap.Entry) (string, error) { // validation has already been done so we can just get the attribute... return func(entry *ldap.Entry) (string, error) { diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index 259c6e941..c9295b8f3 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -1238,7 +1238,6 @@ func TestUpstreamRefresh(t *testing.T) { }, { Name: testUserSearchUIDAttribute, - Values: []string{testUserSearchResultUIDAttributeValue}, ByteValues: [][]byte{[]byte(testUserSearchResultUIDAttributeValue)}, }, }, @@ -1350,7 +1349,6 @@ func TestUpstreamRefresh(t *testing.T) { }, { Name: testUserSearchUIDAttribute, - Values: []string{"wrong-uid"}, ByteValues: [][]byte{[]byte("wrong-uid")}, }, }, @@ -1397,8 +1395,8 @@ func TestUpstreamRefresh(t *testing.T) { Values: []string{testUserSearchResultUsernameAttributeValue}, }, { - Name: testUserSearchUIDAttribute, - Values: []string{testUserSearchResultUIDAttributeValue}, + Name: testUserSearchUIDAttribute, + ByteValues: [][]byte{[]byte(testUserSearchResultUIDAttributeValue)}, }, }, }, @@ -1408,6 +1406,110 @@ func TestUpstreamRefresh(t *testing.T) { }, wantErr: "searching for user with original DN \"some-upstream-user-dn\" resulted in search result without DN", }, + { + name: "search result has 0 values for username attribute", + providerConfig: providerConfig, + setupMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch).Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: testUserSearchResultDNValue, + Attributes: []*ldap.EntryAttribute{ + { + Name: testUserSearchUsernameAttribute, + Values: []string{}, + }, + { + Name: testUserSearchUIDAttribute, + ByteValues: [][]byte{[]byte(testUserSearchResultUIDAttributeValue)}, + }, + }, + }, + }, + }, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantErr: "found 0 values for attribute \"some-upstream-username-attribute\" while searching for user \"some-upstream-user-dn\", but expected 1 result", + }, + { + name: "search result has more than one value for username attribute", + providerConfig: providerConfig, + setupMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch).Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: testUserSearchResultDNValue, + Attributes: []*ldap.EntryAttribute{ + { + Name: testUserSearchUsernameAttribute, + Values: []string{testUserSearchResultUsernameAttributeValue, "something-else"}, + }, + { + Name: testUserSearchUIDAttribute, + ByteValues: [][]byte{[]byte(testUserSearchResultUIDAttributeValue)}, + }, + }, + }, + }, + }, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantErr: "found 2 values for attribute \"some-upstream-username-attribute\" while searching for user \"some-upstream-user-dn\", but expected 1 result", + }, + { + name: "search result has 0 values for uid attribute", + providerConfig: providerConfig, + setupMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch).Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: testUserSearchResultDNValue, + Attributes: []*ldap.EntryAttribute{ + { + Name: testUserSearchUsernameAttribute, + Values: []string{testUserSearchResultUsernameAttributeValue}, + }, + { + Name: testUserSearchUIDAttribute, + ByteValues: [][]byte{}, + }, + }, + }, + }, + }, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantErr: "found 0 values for attribute \"some-upstream-uid-attribute\" while searching for user \"some-upstream-user-dn\", but expected 1 result", + }, + { + name: "search result has 2 values for uid attribute", + providerConfig: providerConfig, + setupMocks: func(conn *mockldapconn.MockConn) { + conn.EXPECT().Bind(testBindUsername, testBindPassword).Times(1) + conn.EXPECT().Search(expectedUserSearch).Return(&ldap.SearchResult{ + Entries: []*ldap.Entry{ + { + DN: testUserSearchResultDNValue, + Attributes: []*ldap.EntryAttribute{ + { + Name: testUserSearchUsernameAttribute, + Values: []string{testUserSearchResultUsernameAttributeValue}, + }, + { + Name: testUserSearchUIDAttribute, + ByteValues: [][]byte{[]byte(testUserSearchResultUIDAttributeValue), []byte("other-uid-value")}, + }, + }, + }, + }, + }, nil).Times(1) + conn.EXPECT().Close().Times(1) + }, + wantErr: "found 2 values for attribute \"some-upstream-uid-attribute\" while searching for user \"some-upstream-user-dn\", but expected 1 result", + }, } for _, test := range tests {