IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

AuthN: Add check for disabled identities (#61382)

Browse Source 
* AuthN: return error if identity is disabled

* AuthN: Remove isDisabled check in client

* AuthN: Format imports
This commit is contained in:
Karl Persson 2023-01-13 10:28:50 +01:00 committed by GitHub
parent b5255ebfdf
commit 0d572ff2ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 45 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
pkg/services/authn/authnimpl/service.go
View File

@ -23,9 +23,14 @@ import (
"github.com/grafana/grafana/pkg/services/rendering"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util/errutil"
"github.com/grafana/grafana/pkg/web"
)
var (
errDisabledIdentity = errutil.NewBase(errutil.StatusUnauthorized, "identity.disabled")
)
// make sure service implements authn.Service interface
var _ authn.Service = new(Service)
@ -129,6 +134,10 @@ func (s *Service) Authenticate(ctx context.Context, client string, r *authn.Requ
}
}
if identity.IsDisabled {
return nil, true, errDisabledIdentity.Errorf("identity is disabled")
}
return identity, true, nil
}

40
pkg/services/authn/authnimpl/service_test.go
View File

@ -22,17 +22,22 @@ import (
func TestService_Authenticate(t *testing.T) {
type TestCase struct {
desc string
clientName string
expectedOK bool
expectedErr error
desc string
clientName string
clientErr error
clientIdentity *authn.Identity
expectedOK bool
expectedErr error
}
var clientErr = errors.New("some err")
tests := []TestCase{
{
desc: "should succeed with authentication for configured client",
clientName: "fake",
expectedOK: true,
desc: "should succeed with authentication for configured client",
clientIdentity: &authn.Identity{},
clientName: "fake",
expectedOK: true,
},
{
desc: "should return false when client is not configured",
@ -43,7 +48,15 @@ func TestService_Authenticate(t *testing.T) {
desc: "should return true and error when client could be used but failed to authenticate",
clientName: "fake",
expectedOK: true,
expectedErr: errors.New("some error"),
clientErr: clientErr,
expectedErr: clientErr,
},
{
desc: "should return error if identity is disabled",
clientName: "fake",
clientIdentity: &authn.Identity{IsDisabled: true},
expectedOK: true,
expectedErr: errDisabledIdentity,
},
}
@ -51,16 +64,15 @@ func TestService_Authenticate(t *testing.T) {
t.Run(tt.desc, func(t *testing.T) {
svc := setupTests(t, func(svc *Service) {
svc.clients["fake"] = &authntest.FakeClient{
ExpectedErr: tt.expectedErr,
ExpectedTest: tt.expectedOK,
ExpectedIdentity: tt.clientIdentity,
ExpectedErr: tt.clientErr,
ExpectedTest: tt.expectedOK,
}
})
_, ok, err := svc.Authenticate(context.Background(), tt.clientName, &authn.Request{})
assert.Equal(t, tt.expectedOK, ok)
if tt.expectedErr != nil {
assert.Error(t, err)
}
assert.ErrorIs(t, err, tt.expectedErr)
})
}
}
@ -114,7 +126,7 @@ func TestService_AuthenticateOrgID(t *testing.T) {
svc.clients["fake"] = authntest.MockClient{
AuthenticateFunc: func(ctx context.Context, r *authn.Request) (*authn.Identity, error) {
calledWith = r.OrgID
return nil, nil
return &authn.Identity{}, nil
},
TestFunc: func(ctx context.Context, r *authn.Request) bool {
return true

17
pkg/services/authn/clients/api_key.go
View File

@ -18,10 +18,9 @@ import (
)
var (
ErrAPIKeyInvalid = errutil.NewBase(errutil.StatusUnauthorized, "api-key.invalid", errutil.WithPublicMessage("Invalid API key"))
ErrAPIKeyExpired = errutil.NewBase(errutil.StatusUnauthorized, "api-key.expired", errutil.WithPublicMessage("Expired API key"))
ErrAPIKeyRevoked = errutil.NewBase(errutil.StatusUnauthorized, "api-key.revoked", errutil.WithPublicMessage("Revoked API key"))
ErrServiceAccountDisabled = errutil.NewBase(errutil.StatusUnauthorized, "service-account.disabled", errutil.WithPublicMessage("Disabled service account"))
errAPIKeyInvalid = errutil.NewBase(errutil.StatusUnauthorized, "api-key.invalid", errutil.WithPublicMessage("Invalid API key"))
errAPIKeyExpired = errutil.NewBase(errutil.StatusUnauthorized, "api-key.expired", errutil.WithPublicMessage("Expired API key"))
errAPIKeyRevoked = errutil.NewBase(errutil.StatusUnauthorized, "api-key.revoked", errutil.WithPublicMessage("Revoked API key"))
)
var _ authn.Client = new(APIKey)
@ -44,17 +43,17 @@ func (s *APIKey) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide
apiKey, err := s.getAPIKey(ctx, getTokenFromRequest(r))
if err != nil {
if errors.Is(err, apikeygen.ErrInvalidApiKey) {
return nil, ErrAPIKeyInvalid.Errorf("API key is invalid")
return nil, errAPIKeyInvalid.Errorf("API key is invalid")
}
return nil, err
}
if apiKey.Expires != nil && *apiKey.Expires <= time.Now().Unix() {
return nil, ErrAPIKeyExpired.Errorf("API key has expired")
return nil, errAPIKeyExpired.Errorf("API key has expired")
}
if apiKey.IsRevoked != nil && *apiKey.IsRevoked {
return nil, ErrAPIKeyRevoked.Errorf("Api key is revoked")
return nil, errAPIKeyRevoked.Errorf("Api key is revoked")
}
go func(id int64) {
@ -86,10 +85,6 @@ func (s *APIKey) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide
return nil, err
}
if usr.IsDisabled {
return nil, ErrServiceAccountDisabled.Errorf("Disabled service account")
}
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceServiceAccount, usr.UserID), usr, authn.ClientParams{}), nil
}

17
pkg/services/authn/clients/api_key_test.go
View File

@ -7,6 +7,8 @@ import (
"net/http"
"testing"
"github.com/stretchr/testify/assert"
"github.com/grafana/grafana/pkg/components/apikeygen"
apikeygenprefix "github.com/grafana/grafana/pkg/components/apikeygenprefixed"
"github.com/grafana/grafana/pkg/services/apikey"
@ -15,7 +17,6 @@ import (
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/stretchr/testify/assert"
)
var (
@ -90,7 +91,7 @@ func TestAPIKey_Authenticate(t *testing.T) {
Key: hash,
Expires: intPtr(0),
},
expectedErr: ErrAPIKeyExpired,
expectedErr: errAPIKeyExpired,
},
{
desc: "should fail for revoked api key",
@ -99,17 +100,7 @@ func TestAPIKey_Authenticate(t *testing.T) {
Key: hash,
IsRevoked: &revoked,
},
expectedErr: ErrAPIKeyRevoked,
},
{
desc: "should fail if service account is disabled",
req: &authn.Request{HTTPRequest: &http.Request{Header: map[string][]string{"Authorization": {"Bearer " + secret}}}},
expectedKey: &apikey.APIKey{
Key: hash,
ServiceAccountId: intPtr(1),
},
expectedUser: &user.SignedInUser{IsDisabled: true},
expectedErr: ErrServiceAccountDisabled,
expectedErr: errAPIKeyRevoked,
},
}