Follow up changes

- Create a list of authenticators for each issuer.
  Remove the requirement for providing a clientID.
- This allows adding self signed authenticators to the same multi authenticators interface.

Signed-off-by: Aditya Dani <[email protected]>

Author: adityadani

pkg/auth/auth.go

@@ -62,7 +62,7 @@ type Authenticator interface {
 type MultiAuthenticatorWithClientID interface {
 	Authenticator
 	GetAuthenticators(issuer string) []Authenticator
-	ListIssuersWithClientID() []IssuerWithClientID
+	ListIssuers() []string
 }
 
 // Enabled returns whether auth is enabled.

pkg/auth/multiauthenticator.go

@@ -5,53 +5,41 @@ import (
 	"fmt"
 )
 
-type IssuerWithClientID struct {
-	// Issuer of the token. The issuer cannot be empty.
-	Issuer string
-	// ClientID of the token. The clientID cannot be empty.
-	ClientID string
+type multiAuthenticatorImpl struct {
+	authenticators map[string][]Authenticator
 }
 
-type multiAuthenticatorWithClientIDImpl struct {
-	authenticators map[IssuerWithClientID]Authenticator
-}
-
-func NewMultiAuthenticatorWithClientID(
-	authenticators map[IssuerWithClientID]Authenticator) (MultiAuthenticatorWithClientID, error) {
-	authMap := make(map[IssuerWithClientID]Authenticator)
-	for issuerWithClientID, authenticator := range authenticators {
-		if issuerWithClientID.ClientID == "" {
-			return nil, fmt.Errorf("clientID cannot be empty")
-		}
-		if issuerWithClientID.Issuer == "" {
-			return nil, fmt.Errorf("issuer cannot be empty")
+// NewMultiAuthenticator maintains a list of authenticators for a given issuer.
+// The input argument is a map of issuers to a list of authenticators for that issuer.
+// NOTE: This interface does not check if there are duplicate authenticators.
+func NewMultiAuthenticator(
+	authenticators map[string][]Authenticator,
+) (MultiAuthenticatorWithClientID, error) {
+	authMap := make(map[string][]Authenticator)
+	for issuer, authenticatorsList := range authenticators {
+		if len(authenticatorsList) == 0 {
+			return nil, fmt.Errorf("empty authenticators list for issuer %v", issuer)
 		}
-		authMap[issuerWithClientID] = authenticator
+		authMap[issuer] = authenticatorsList
 	}
-	return &multiAuthenticatorWithClientIDImpl{
+	return &multiAuthenticatorImpl{
 		authenticators: authMap,
 	}, nil
 }
 
-func (m *multiAuthenticatorWithClientIDImpl) ListIssuersWithClientID() []IssuerWithClientID {
-	var issuerWithClientIDs []IssuerWithClientID
-	for issuerWithClientID, _ := range m.authenticators {
-		issuerWithClientIDs = append(issuerWithClientIDs, issuerWithClientID)
-	}
-	return issuerWithClientIDs
+func (m *multiAuthenticatorImpl) GetAuthenticators(issuer string) []Authenticator {
+	return m.authenticators[issuer]
 }
 
-func (m *multiAuthenticatorWithClientIDImpl) GetAuthenticators(issuer string) []Authenticator {
-	var authenticators []Authenticator
-	for issuerWithClientID, authenticator := range m.authenticators {
-		if issuerWithClientID.Issuer == issuer {
-			authenticators = append(authenticators, authenticator)
-		}
+func (m *multiAuthenticatorImpl) ListIssuers() []string {
+	var issuers []string
+	for issuer, _ := range m.authenticators {
+		issuers = append(issuers, issuer)
 	}
-	return authenticators
+	return issuers
 }
 
-func (m *multiAuthenticatorWithClientIDImpl) AuthenticateToken(ctx context.Context, idToken string) (*Claims, error) {
+func (m *multiAuthenticatorImpl) AuthenticateToken(ctx context.Context, idToken string) (*Claims, error) {
 	tokenClaims, err := TokenClaims(idToken)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get claims from token: %w", err)
@@ -68,9 +56,9 @@ func (m *multiAuthenticatorWithClientIDImpl) AuthenticateToken(ctx context.Conte
 		tokenClaims.Issuer, tokenClaims.Audience)
 }
 
-func (m *multiAuthenticatorWithClientIDImpl) Username(claims *Claims) string {
+func (m *multiAuthenticatorImpl) Username(claims *Claims) string {
 	var username string
-	// TODO: This code does not handle the case where there are mulitple authenticators
+	// TODO: This code does not handle the case where there are multiple authenticators
 	// registered with the same issuer but different UsernameClaimTypes.
 	for _, authenticator := range m.GetAuthenticators(claims.Issuer) {
 		if username = authenticator.Username(claims); username != "" {

pkg/auth/multiauthenticator_test.go

@@ -9,40 +9,57 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestNewMultiAuthenticatorWithClientID_EmptyClientID(t *testing.T) {
+func TestNewMultiAuthenticator_EmptyAuthenticators(t *testing.T) {
 	// Given.
-	jwtAuth, err := NewJwtAuthenticator(&JwtAuthConfig{
-		SharedSecret:  []byte("my-secret"),
-		UsernameClaim: UsernameClaimTypeSubject,
+
+	m, err := NewMultiAuthenticator(map[string][]Authenticator{
+		"issuer-1": []Authenticator{},
 	})
-	assert.NoError(t, err)
-	m, err := NewMultiAuthenticatorWithClientID(map[IssuerWithClientID]Authenticator{IssuerWithClientID{"issuer-1", ""}: jwtAuth})
 	assert.Error(t, err)
-	assert.Nil(t, m)
+
+	// When.
+	authenticators := m.GetAuthenticators("issuer-1")
+
+	// Then.
+	assert.Equal(t, 0, len(authenticators))
 }
 
-func TestNewMultiAuthenticatorWithClientID_EmptyIssuer(t *testing.T) {
+func TestNewMultiAuthenticator_ListIssuers(t *testing.T) {
 	// Given.
 	jwtAuth, err := NewJwtAuthenticator(&JwtAuthConfig{
 		SharedSecret:  []byte("my-secret"),
 		UsernameClaim: UsernameClaimTypeSubject,
 	})
 	assert.NoError(t, err)
-	m, err := NewMultiAuthenticatorWithClientID(map[IssuerWithClientID]Authenticator{IssuerWithClientID{"", "1"}: jwtAuth})
-	assert.Error(t, err)
-	assert.Nil(t, m)
-}
+	m, err := NewMultiAuthenticator(map[string][]Authenticator{
+		"issuer-1": []Authenticator{jwtAuth},
+		// TODO: MultiAuthenticator does not check for duplicate authenticators across issuers.
+		"issuer-2": []Authenticator{jwtAuth},
+	})
+	assert.NoError(t, err)
 
-func TestMultiAuthenticatorWithClientID_GetAuthenticators_Ok(t *testing.T) {
+	// When.
+	issuers := m.ListIssuers()
+
+	// Then.
+	assert.Equal(t, 2, len(issuers))
+}
+func TestMultiAuthenticator_GetAuthenticators_Ok(t *testing.T) {
 	// Given.
-	jwtAuth, err := NewJwtAuthenticator(&JwtAuthConfig{
-		SharedSecret:  []byte("my-secret"),
+	jwtAuth1, err := NewJwtAuthenticator(&JwtAuthConfig{
+		SharedSecret:  []byte("my-secret1"),
 		UsernameClaim: UsernameClaimTypeSubject,
 	})
 	assert.NoError(t, err)
-	m, err := NewMultiAuthenticatorWithClientID(map[IssuerWithClientID]Authenticator{
-		IssuerWithClientID{"issuer-1", "1"}: jwtAuth,
-		IssuerWithClientID{"issuer-1", "2"}: jwtAuth,
+
+	jwtAuth2, err := NewJwtAuthenticator(&JwtAuthConfig{
+		SharedSecret:  []byte("my-secret2"),
+		UsernameClaim: UsernameClaimTypeSubject,
+	})
+	assert.NoError(t, err)
+
+	m, err := NewMultiAuthenticator(map[string][]Authenticator{
+		"issuer-1": []Authenticator{jwtAuth1, jwtAuth2},
 	})
 	assert.NoError(t, err)
 
@@ -53,9 +70,9 @@ func TestMultiAuthenticatorWithClientID_GetAuthenticators_Ok(t *testing.T) {
 	assert.Equal(t, 2, len(authenticators))
 }
 
-func TestMultiAuthenticatorWithClientID_GetAuthenticator_FailNoIssuer(t *testing.T) {
+func TestMultiAuthenticator_GetAuthenticator_FailNoIssuer(t *testing.T) {
 	// Given.
-	m, err := NewMultiAuthenticatorWithClientID(map[IssuerWithClientID]Authenticator{})
+	m, err := NewMultiAuthenticator(map[string][]Authenticator{})
 	assert.NoError(t, err)
 
 	// When.
@@ -65,21 +82,26 @@ func TestMultiAuthenticatorWithClientID_GetAuthenticator_FailNoIssuer(t *testing
 	assert.Empty(t, authenticators)
 }
 
-func TestMultiAuthenticatorWithClientID_AuthenticateToken_Ok(t *testing.T) {
+func TestMultiAuthenticator_AuthenticateToken_Ok(t *testing.T) {
 	// Given.
 	jwtAuth1, err := NewJwtAuthenticator(&JwtAuthConfig{
 		SharedSecret:  []byte("my-secret-1"),
 		UsernameClaim: UsernameClaimTypeSubject,
 	})
 	assert.NoError(t, err)
 
-	m, err := NewMultiAuthenticatorWithClientID(map[IssuerWithClientID]Authenticator{
-		IssuerWithClientID{"issuer-1", "1"}: jwtAuth1,
-		IssuerWithClientID{"issuer-1", "2"}: jwtAuth1,
+	jwtAuth2, err := NewJwtAuthenticator(&JwtAuthConfig{
+		SharedSecret:  []byte("my-secret-2"),
+		UsernameClaim: UsernameClaimTypeSubject,
 	})
 	assert.NoError(t, err)
 
-	rawToken, err := Token(&Claims{
+	m, err := NewMultiAuthenticator(map[string][]Authenticator{
+		"issuer-1": []Authenticator{jwtAuth1, jwtAuth2},
+	})
+	assert.NoError(t, err)
+
+	rawToken1, err := Token(&Claims{
 		Audience: "1",
 		Issuer:   "issuer-1",
 		Email:    "[email protected]",
@@ -88,20 +110,89 @@ func TestMultiAuthenticatorWithClientID_AuthenticateToken_Ok(t *testing.T) {
 		Roles:    []string{"tester"},
 	}, &Signature{
 		Type: jwt.SigningMethodHS256,
-		Key:  []byte("my-secret-1"),
+		Key:  []byte("my-secret-2"),
 	}, &Options{
 		Expiration: time.Now().Add(time.Minute * 10).Unix(),
 	})
 	assert.NoError(t, err)
-	assert.NotEmpty(t, rawToken)
+	assert.NotEmpty(t, rawToken1)
 
 	// When.
-	authenticateClaims, err := m.AuthenticateToken(context.Background(), rawToken)
+	authenticateClaims, err := m.AuthenticateToken(context.Background(), rawToken1)
 
 	// Then.
 	assert.NoError(t, err)
 	assert.Equal(t, "issuer-1", authenticateClaims.Issuer)
 	assert.Equal(t, "[email protected]", authenticateClaims.Email)
 	assert.Equal(t, "my-name", authenticateClaims.Name)
 	assert.Equal(t, []string{"tester"}, authenticateClaims.Roles)
+
+	rawToken2, err := Token(&Claims{
+		Audience: "1",
+		Issuer:   "issuer-1",
+		Email:    "[email protected]",
+		Name:     "my-name",
+		Subject:  "my-sub",
+		Roles:    []string{"tester"},
+	}, &Signature{
+		Type: jwt.SigningMethodHS256,
+		Key:  []byte("my-secret-2"),
+	}, &Options{
+		Expiration: time.Now().Add(time.Minute * 10).Unix(),
+	})
+	assert.NoError(t, err)
+	assert.NotEmpty(t, rawToken2)
+
+	// When.
+	authenticateClaims, err = m.AuthenticateToken(context.Background(), rawToken2)
+
+	// Then.
+	assert.NoError(t, err)
+	assert.Equal(t, "issuer-1", authenticateClaims.Issuer)
+	assert.Equal(t, "[email protected]", authenticateClaims.Email)
+	assert.Equal(t, "my-name", authenticateClaims.Name)
+	assert.Equal(t, []string{"tester"}, authenticateClaims.Roles)
+}
+
+func TestMultiAuthenticator_AuthenticateToken_Fail(t *testing.T) {
+	// Given.
+	jwtAuth1, err := NewJwtAuthenticator(&JwtAuthConfig{
+		SharedSecret:  []byte("my-secret-1"),
+		UsernameClaim: UsernameClaimTypeSubject,
+	})
+	assert.NoError(t, err)
+
+	jwtAuth2, err := NewJwtAuthenticator(&JwtAuthConfig{
+		SharedSecret:  []byte("my-secret-2"),
+		UsernameClaim: UsernameClaimTypeSubject,
+	})
+	assert.NoError(t, err)
+
+	m, err := NewMultiAuthenticator(map[string][]Authenticator{
+		"issuer-1": []Authenticator{jwtAuth1, jwtAuth2},
+	})
+	assert.NoError(t, err)
+
+	rawToken1, err := Token(&Claims{
+		Audience: "1",
+		Issuer:   "issuer-1",
+		Email:    "[email protected]",
+		Name:     "my-name",
+		Subject:  "my-sub",
+		Roles:    []string{"tester"},
+	}, &Signature{
+		Type: jwt.SigningMethodHS256,
+		Key:  []byte("my-secret-3"),
+	}, &Options{
+		Expiration: time.Now().Add(time.Minute * 10).Unix(),
+	})
+	assert.NoError(t, err)
+	assert.NotEmpty(t, rawToken1)
+
+	// When.
+	authenticateClaims, err := m.AuthenticateToken(context.Background(), rawToken1)
+
+	// Then.
+	assert.Error(t, err)
+	assert.Nil(t, authenticateClaims)
 }

server/grpc_framework_server.go

@@ -81,8 +81,8 @@ func NewGrpcFrameworkServer(config *ServerConfig) (*GrpcFrameworkServer, error)
 		return nil, fmt.Errorf("must supply role manager when authentication is enabled and default authZ is used")
 	}
 	if config.Security.Authenticators != nil {
-		for _, issuerWithClientID := range config.Security.Authenticators.ListIssuersWithClientID() {
-			log.Infof("Authentication enabled for issuer: %s with clientID %s", issuerWithClientID.Issuer, issuerWithClientID.ClientID)
+		for _, issuer := range config.Security.Authenticators.ListIssuers() {
+			log.Infof("Authentication enabled for issuer: %s ", issuer)
 		}
 	}