Auth: Remove code for authenticating API keys (#105998)

* remove auth for plain API keys

* move condition to validateApiKey()

* fix typo

* remove GetApiKeyById() method

pkg/services/apikey/apikey.go

@@ -9,7 +9,6 @@ type Service interface {
 	GetAllAPIKeys(ctx context.Context, orgID int64) ([]*APIKey, error)
 	DeleteApiKey(ctx context.Context, cmd *DeleteCommand) error
 	AddAPIKey(ctx context.Context, cmd *AddCommand) (res *APIKey, err error)
-	GetApiKeyById(ctx context.Context, query *GetByIDQuery) (res *APIKey, err error)
 	GetApiKeyByName(ctx context.Context, query *GetByNameQuery) (res *APIKey, err error)
 	GetAPIKeyByHash(ctx context.Context, hash string) (*APIKey, error)
 	UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error

pkg/services/apikey/apikeyimpl/apikey.go

@@ -43,9 +43,6 @@ func (s *Service) GetAPIKeys(ctx context.Context, query *apikey.GetApiKeysQuery)
 func (s *Service) GetAllAPIKeys(ctx context.Context, orgID int64) ([]*apikey.APIKey, error) {
 	return s.store.GetAllAPIKeys(ctx, orgID)
 }
-func (s *Service) GetApiKeyById(ctx context.Context, query *apikey.GetByIDQuery) (*apikey.APIKey, error) {
-	return s.store.GetApiKeyById(ctx, query)
-}
 func (s *Service) GetApiKeyByName(ctx context.Context, query *apikey.GetByNameQuery) (*apikey.APIKey, error) {
 	return s.store.GetApiKeyByName(ctx, query)
 }

pkg/services/apikey/apikeyimpl/store.go

@@ -13,7 +13,6 @@ type store interface {
 	CountAPIKeys(ctx context.Context, orgID int64) (int64, error)
 	DeleteApiKey(ctx context.Context, cmd *apikey.DeleteCommand) error
 	AddAPIKey(ctx context.Context, cmd *apikey.AddCommand) (res *apikey.APIKey, err error)
-	GetApiKeyById(ctx context.Context, query *apikey.GetByIDQuery) (res *apikey.APIKey, err error)
 	GetApiKeyByName(ctx context.Context, query *apikey.GetByNameQuery) (res *apikey.APIKey, err error)
 	GetAPIKeyByHash(ctx context.Context, hash string) (*apikey.APIKey, error)
 	UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error

pkg/services/apikey/apikeyimpl/xorm_store.go

@@ -136,23 +136,6 @@ func (ss *sqlStore) AddAPIKey(ctx context.Context, cmd *apikey.AddCommand) (res
 	return res, err
 }
 
-func (ss *sqlStore) GetApiKeyById(ctx context.Context, query *apikey.GetByIDQuery) (res *apikey.APIKey, err error) {
-	err = ss.db.WithDbSession(ctx, func(sess *db.Session) error {
-		var key apikey.APIKey
-		has, err := sess.ID(query.ApiKeyID).Get(&key)
-
-		if err != nil {
-			return err
-		} else if !has {
-			return apikey.ErrInvalid
-		}
-
-		res = &key
-		return nil
-	})
-	return res, err
-}
-
 func (ss *sqlStore) GetApiKeyByName(ctx context.Context, query *apikey.GetByNameQuery) (res *apikey.APIKey, err error) {
 	err = ss.db.WithDbSession(ctx, func(sess *db.Session) error {
 		var key apikey.APIKey

pkg/services/apikey/apikeytest/fake.go

@@ -19,9 +19,6 @@ func (s *Service) GetAPIKeys(ctx context.Context, query *apikey.GetApiKeysQuery)
 func (s *Service) GetAllAPIKeys(ctx context.Context, orgID int64) ([]*apikey.APIKey, error) {
 	return s.ExpectedAPIKeys, s.ExpectedError
 }
-func (s *Service) GetApiKeyById(ctx context.Context, query *apikey.GetByIDQuery) (*apikey.APIKey, error) {
-	return s.ExpectedAPIKey, s.ExpectedError
-}
 func (s *Service) GetApiKeyByName(ctx context.Context, query *apikey.GetByNameQuery) (*apikey.APIKey, error) {
 	return s.ExpectedAPIKey, s.ExpectedError
 }

pkg/services/authn/clients/api_key.go

@@ -15,7 +15,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/apikey"
 	"github.com/grafana/grafana/pkg/services/authn"
 	"github.com/grafana/grafana/pkg/services/login"
-	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/util"
 )
 
@@ -24,14 +23,11 @@ var (
 	errAPIKeyExpired     = errutil.Unauthorized("api-key.expired", errutil.WithPublicMessage("Expired API key"))
 	errAPIKeyRevoked     = errutil.Unauthorized("api-key.revoked", errutil.WithPublicMessage("Revoked API key"))
 	errAPIKeyOrgMismatch = errutil.Unauthorized("api-key.organization-mismatch", errutil.WithPublicMessage("API key does not belong to the requested organization"))
-
-	errAPIKeyInvalidType = errutil.BadRequest("api-key.invalid-type-id")
 )
 
 var (
 	_ authn.HookClient         = new(APIKey)
 	_ authn.ContextAwareClient = new(APIKey)
-	_ authn.IdentityResolverClient = new(APIKey)
 )
 
 const (
@@ -80,11 +76,6 @@ func (s *APIKey) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide
 		r.SetMeta(metaKeySkipLastUsed, "true")
 	}
 
-	// if the api key don't belong to a service account construct the identity and return it
-	if key.ServiceAccountId == nil || *key.ServiceAccountId < 1 {
-		return newAPIKeyIdentity(key), nil
-	}
-
 	return newServiceAccountIdentity(key), nil
 }
 
@@ -152,38 +143,6 @@ func (s *APIKey) Priority() uint {
 	return 30
 }
 
-func (s *APIKey) IdentityType() claims.IdentityType {
-	return claims.TypeAPIKey
-}
-
-func (s *APIKey) ResolveIdentity(ctx context.Context, orgID int64, typ claims.IdentityType, id string) (*authn.Identity, error) {
-	if !claims.IsIdentityType(typ, claims.TypeAPIKey) {
-		return nil, errAPIKeyInvalidType.Errorf("got unexpected type: %s", typ)
-	}
-
-	apiKeyID, err := strconv.ParseInt(id, 10, 64)
-	if err != nil {
-		return nil, err
-	}
-
-	key, err := s.apiKeyService.GetApiKeyById(ctx, &apikey.GetByIDQuery{
-		ApiKeyID: apiKeyID,
-	})
-	if err != nil {
-		return nil, err
-	}
-
-	if err := validateApiKey(orgID, key); err != nil {
-		return nil, err
-	}
-
-	if key.ServiceAccountId != nil && *key.ServiceAccountId >= 1 {
-		return nil, errAPIKeyInvalidType.Errorf("api key belongs to service account")
-	}
-
-	return newAPIKeyIdentity(key), nil
-}
-
 func (s *APIKey) Hook(ctx context.Context, identity *authn.Identity, r *authn.Request) error {
 	if r.GetMeta(metaKeySkipLastUsed) != "" {
 		return nil
@@ -248,18 +207,12 @@ func validateApiKey(orgID int64, key *apikey.APIKey) error {
 		return errAPIKeyOrgMismatch.Errorf("API does not belong in Organization")
 	}
 
-	return nil
+	// plain API keys are no longer supported so an error is returned if the api key doesn't belong to a service account
-}
+	if key.ServiceAccountId == nil || *key.ServiceAccountId < 1 {
-
+		return errAPIKeyInvalid.Errorf("API key does not belong to a service account")
-func newAPIKeyIdentity(key *apikey.APIKey) *authn.Identity {
-	return &authn.Identity{
-		ID:              strconv.FormatInt(key.ID, 10),
-		Type:            claims.TypeAPIKey,
-		OrgID:           key.OrgID,
-		OrgRoles:        map[int64]org.RoleType{key.OrgID: key.Role},
-		ClientParams:    authn.ClientParams{SyncPermissions: true},
-		AuthenticatedBy: login.APIKeyAuthModule,
 	}
+
+	return nil
 }
 
 func newServiceAccountIdentity(key *apikey.APIKey) *authn.Identity {

pkg/services/authn/clients/api_key_test.go

@@ -35,7 +35,7 @@ func TestAPIKey_Authenticate(t *testing.T) {
 
 	tests := []TestCase{
 		{
-			desc: "should success for valid token that is not connected to a service account",
+			desc: "should fail for valid token that is not connected to a service account",
 			req: &authn.Request{HTTPRequest: &http.Request{
 				Header: map[string][]string{
 					"Authorization": {"Bearer " + secret},
@@ -47,16 +47,7 @@ func TestAPIKey_Authenticate(t *testing.T) {
 				Key:   hash,
 				Role:  org.RoleAdmin,
 			},
-			expectedIdentity: &authn.Identity{
+			expectedErr: errAPIKeyInvalid,
-				ID:       "1",
-				Type:     claims.TypeAPIKey,
-				OrgID:    1,
-				OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin},
-				ClientParams: authn.ClientParams{
-					SyncPermissions: true,
-				},
-				AuthenticatedBy: login.APIKeyAuthModule,
-			},
 		},
 		{
 			desc: "should success for valid token that is connected to service account",
@@ -188,109 +179,6 @@ func TestAPIKey_Test(t *testing.T) {
 	}
 }
 
-func TestAPIKey_ResolveIdentity(t *testing.T) {
-	type testCase struct {
-		desc string
-		typ  claims.IdentityType
-		id   string
-
-		exptedApiKey *apikey.APIKey
-
-		expectedIdenity *authn.Identity
-		expectedErr     error
-	}
-
-	tests := []testCase{
-		{
-			desc:        "should return error for invalid type",
-			id:          "1",
-			typ:         claims.TypeUser,
-			expectedErr: errAPIKeyInvalidType,
-		},
-		{
-			desc: "should return error when api key has expired",
-			id:   "1",
-			typ:  claims.TypeAPIKey,
-			exptedApiKey: &apikey.APIKey{
-				ID:      1,
-				OrgID:   1,
-				Expires: intPtr(0),
-			},
-			expectedErr: errAPIKeyExpired,
-		},
-		{
-			desc: "should return error when api key is revoked",
-			id:   "1",
-			typ:  claims.TypeAPIKey,
-			exptedApiKey: &apikey.APIKey{
-				ID:        1,
-				OrgID:     1,
-				IsRevoked: boolPtr(true),
-			},
-			expectedErr: errAPIKeyRevoked,
-		},
-		{
-			desc: "should return error when api key is connected to service account",
-			id:   "1",
-			typ:  claims.TypeAPIKey,
-
-			exptedApiKey: &apikey.APIKey{
-				ID:               1,
-				OrgID:            1,
-				ServiceAccountId: intPtr(1),
-			},
-			expectedErr: errAPIKeyInvalidType,
-		},
-		{
-			desc: "should return error when api key is belongs to different org",
-			id:   "1",
-			typ:  claims.TypeAPIKey,
-			exptedApiKey: &apikey.APIKey{
-				ID:               1,
-				OrgID:            2,
-				ServiceAccountId: intPtr(1),
-			},
-			expectedErr: errAPIKeyOrgMismatch,
-		},
-		{
-			desc: "should return valid idenitty",
-			id:   "1",
-			typ:  claims.TypeAPIKey,
-			exptedApiKey: &apikey.APIKey{
-				ID:    1,
-				OrgID: 1,
-				Role:  org.RoleEditor,
-			},
-			expectedIdenity: &authn.Identity{
-				OrgID:           1,
-				OrgRoles:        map[int64]org.RoleType{1: org.RoleEditor},
-				ID:              "1",
-				Type:            claims.TypeAPIKey,
-				AuthenticatedBy: login.APIKeyAuthModule,
-				ClientParams:    authn.ClientParams{SyncPermissions: true},
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.desc, func(t *testing.T) {
-			c := ProvideAPIKey(&apikeytest.Service{
-				ExpectedAPIKey: tt.exptedApiKey,
-			})
-
-			identity, err := c.ResolveIdentity(context.Background(), 1, tt.typ, tt.id)
-			if tt.expectedErr != nil {
-				assert.Nil(t, identity)
-				assert.ErrorIs(t, err, tt.expectedErr)
-				return
-			}
-
-			assert.NoError(t, err)
-			assert.EqualValues(t, *tt.expectedIdenity, *identity)
-		})
-	}
-}
-
 func intPtr(n int64) *int64 {
 	return &n
 }