resolved a couple of testing related todos

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
+	// This error shouldn't happen, but do some error checking anyway so it doesn't panic
-	extra := authenticatedResponse.User.GetExtra()
+	dnSlice := authenticatedResponse.User.GetExtra()["userDN"]
-	if len(extra) == 0 {
-		return ""
-	}
-	dnSlice := extra["userDN"]
 	if len(dnSlice) != 1 {
 		return ""
 	}

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) {

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,
+									Name:       testUserSearchUIDAttribute,
-									Values: []string{testUserSearchResultUIDAttributeValue},
+									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 {