AuthN: Refactor basic auth client to support multiple password auth (#61153)

* AuthN: add interface for password clients

* AuthN: Extract grafana password client

* AuthN: Rewrite basic client tests

* AuthN: Add Ldap client and rename method of PasswordClient

* AuthN: Configure multiple password clients

* AuthN: create ldap service and add tests
This commit is contained in:
Karl Persson
2023-01-09 16:40:29 +01:00
committed by GitHub
parent c3378aff8b
commit a49892c9ac
10 changed files with 413 additions and 76 deletions

View File

@@ -47,6 +47,10 @@ type Client interface {
Test(ctx context.Context, r *Request) bool
}
type PasswordClient interface {
AuthenticatePassword(ctx context.Context, orgID int64, username, password string) (*Identity, error)
}
type Request struct {
// OrgID will be populated by authn.Service
OrgID int64

View File

@@ -53,9 +53,19 @@ func ProvideService(
s.clients[authn.ClientAnonymous] = clients.ProvideAnonymous(cfg, orgService)
}
// FIXME (kalleep): handle cfg.DisableLogin as well?
if s.cfg.BasicAuthEnabled && !s.cfg.DisableLogin {
s.clients[authn.ClientBasic] = clients.ProvideBasic(userService, loginAttempts)
var passwordClients []authn.PasswordClient
if !s.cfg.DisableLogin {
passwordClients = append(passwordClients, clients.ProvideGrafana(userService))
}
if s.cfg.LDAPEnabled {
passwordClients = append(passwordClients, clients.ProvideLDAP(cfg))
}
// only configure basic auth client if it is enabled, and we have at least one password client enabled
if s.cfg.BasicAuthEnabled && len(passwordClients) > 0 {
s.clients[authn.ClientBasic] = clients.ProvideBasic(loginAttempts, passwordClients...)
}
// FIXME (jguer): move to User package

View File

@@ -13,10 +13,9 @@ type FakeService struct {
var _ authn.Client = new(FakeClient)
type FakeClient struct {
ExpectedErr error
ExpectedTest bool
ExpectedIdentity *authn.Identity
ExpectedClientParams *authn.ClientParams
ExpectedErr error
ExpectedTest bool
ExpectedIdentity *authn.Identity
}
func (f *FakeClient) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identity, error) {
@@ -26,3 +25,14 @@ func (f *FakeClient) Authenticate(ctx context.Context, r *authn.Request) (*authn
func (f *FakeClient) Test(ctx context.Context, r *authn.Request) bool {
return f.ExpectedTest
}
var _ authn.PasswordClient = new(FakePasswordClient)
type FakePasswordClient struct {
ExpectedErr error
ExpectedIdentity *authn.Identity
}
func (f FakePasswordClient) AuthenticatePassword(ctx context.Context, orgID int64, username, password string) (*authn.Identity, error) {
return f.ExpectedIdentity, f.ExpectedErr
}

View File

@@ -2,36 +2,35 @@ package clients
import (
"context"
"crypto/subtle"
"errors"
"strings"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/loginattempt"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util"
"github.com/grafana/grafana/pkg/util/errutil"
)
var (
ErrBasicAuthCredentials = errutil.NewBase(errutil.StatusUnauthorized, "basic-auth.invalid-credentials", errutil.WithPublicMessage("Invalid username or password"))
ErrDecodingBasicAuthHeader = errutil.NewBase(errutil.StatusBadRequest, "basic-auth.invalid-header", errutil.WithPublicMessage("Invalid Basic Auth Header"))
errDecodingBasicAuthHeader = errutil.NewBase(errutil.StatusBadRequest, "basic-auth.invalid-header", errutil.WithPublicMessage("Invalid Basic Auth Header"))
errBasicAuthCredentials = errutil.NewBase(errutil.StatusUnauthorized, "basic-auth.invalid-credentials", errutil.WithPublicMessage("Invalid username or password"))
)
var _ authn.Client = new(Basic)
func ProvideBasic(userService user.Service, loginAttempts loginattempt.Service) *Basic {
return &Basic{userService, loginAttempts}
func ProvideBasic(loginAttempts loginattempt.Service, clients ...authn.PasswordClient) *Basic {
return &Basic{clients, loginAttempts}
}
type Basic struct {
userService user.Service
clients []authn.PasswordClient
loginAttempts loginattempt.Service
}
func (c *Basic) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identity, error) {
username, password, err := util.DecodeBasicAuthHeader(getBasicAuthHeaderFromRequest(r))
if err != nil {
return nil, ErrDecodingBasicAuthHeader.Errorf("failed to decode basic auth header: %w", err)
return nil, errDecodingBasicAuthHeader.Errorf("failed to decode basic auth header: %w", err)
}
ok, err := c.loginAttempts.Validate(ctx, username)
@@ -39,37 +38,37 @@ func (c *Basic) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden
return nil, err
}
if !ok {
return nil, ErrBasicAuthCredentials.Errorf("too many consecutive incorrect login attempts for user - login for user temporarily blocked")
return nil, errBasicAuthCredentials.Errorf("too many consecutive incorrect login attempts for user - login for user temporarily blocked")
}
if len(password) == 0 {
return nil, ErrBasicAuthCredentials.Errorf("no password provided")
return nil, errBasicAuthCredentials.Errorf("no password provided")
}
// FIXME (kalleep): decide if we should handle ldap here
usr, err := c.userService.GetByLogin(ctx, &user.GetUserByLoginQuery{LoginOrEmail: username})
if err != nil {
return nil, ErrBasicAuthCredentials.Errorf("failed to fetch user: %w", err)
for _, pwClient := range c.clients {
identity, err := pwClient.AuthenticatePassword(ctx, r.OrgID, username, password)
if err != nil {
if errors.Is(err, errIdentityNotFound) {
// continue to next password client if identity could not be found
continue
}
if errors.Is(err, errInvalidPassword) {
// only add login attempt if identity was found but the provided password was invalid
_ = c.loginAttempts.Add(ctx, username, r.HTTPRequest.RemoteAddr)
}
return nil, errBasicAuthCredentials.Errorf("failed to authenticate identity: %w", err)
}
return identity, nil
}
if ok := comparePassword(password, usr.Salt, usr.Password); !ok {
_ = c.loginAttempts.Add(ctx, username, r.HTTPRequest.RemoteAddr)
return nil, ErrBasicAuthCredentials.Errorf("incorrect password provided")
}
signedInUser, err := c.userService.GetSignedInUserWithCacheCtx(ctx, &user.GetSignedInUserQuery{
UserID: usr.ID,
OrgID: r.OrgID,
})
if err != nil {
return nil, ErrBasicAuthCredentials.Errorf("failed to fetch user: %w", err)
}
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, signedInUser.UserID), signedInUser, authn.ClientParams{}), nil
return nil, errBasicAuthCredentials.Errorf("failed to authenticate identity using basic auth")
}
func (c *Basic) Test(ctx context.Context, r *authn.Request) bool {
if len(c.clients) == 0 {
return false
}
return looksLikeBasicAuthRequest(r)
}
@@ -93,9 +92,3 @@ func getBasicAuthHeaderFromRequest(r *authn.Request) string {
return header
}
func comparePassword(password, salt, hash string) bool {
// It is ok to ignore the error here because util.EncodePassword can never return a error
hashedPassword, _ := util.EncodePassword(password, salt)
return subtle.ConstantTimeCompare([]byte(hashedPassword), []byte(hash)) == 1
}

View File

@@ -6,62 +6,59 @@ import (
"testing"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/authn/authntest"
"github.com/grafana/grafana/pkg/services/loginattempt/loginattempttest"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/util"
"github.com/stretchr/testify/assert"
)
func TestBasic_Authenticate(t *testing.T) {
type TestCase struct {
desc string
req *authn.Request
blockLogin bool
expectedErr error
expectedSignedInUser *user.SignedInUser
expectedIdentity *authn.Identity
desc string
req *authn.Request
blockLogin bool
clients []authn.PasswordClient
expectedErr error
expectedIdentity *authn.Identity
}
tests := []TestCase{
{
desc: "should successfully authenticate user with correct password",
req: &authn.Request{HTTPRequest: &http.Request{Header: map[string][]string{authorizationHeaderName: {encodeBasicAuth("user", "password")}}}},
expectedErr: nil,
expectedSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: "Viewer"},
expectedIdentity: &authn.Identity{ID: "user:1", OrgID: 1, OrgRoles: map[int64]org.RoleType{1: "Viewer"}, IsGrafanaAdmin: boolPtr(false)},
desc: "should success when password client return identity",
req: &authn.Request{HTTPRequest: &http.Request{Header: map[string][]string{authorizationHeaderName: {encodeBasicAuth("user", "password")}}}},
clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: "user:1"}}},
expectedIdentity: &authn.Identity{ID: "user:1"},
},
{
desc: "should fail for incorrect password",
req: &authn.Request{HTTPRequest: &http.Request{Header: map[string][]string{authorizationHeaderName: {encodeBasicAuth("user", "wrong")}}}},
expectedErr: ErrBasicAuthCredentials,
desc: "should success when found in second client",
req: &authn.Request{HTTPRequest: &http.Request{Header: map[string][]string{authorizationHeaderName: {encodeBasicAuth("user", "password")}}}},
clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedErr: errIdentityNotFound}, authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: "user:2"}}},
expectedIdentity: &authn.Identity{ID: "user:2"},
},
{
desc: "should fail for empty password",
req: &authn.Request{HTTPRequest: &http.Request{Header: map[string][]string{authorizationHeaderName: {encodeBasicAuth("user", "")}}}},
expectedErr: ErrBasicAuthCredentials,
expectedErr: errBasicAuthCredentials,
},
{
desc: "should if login is blocked by to many attempts",
req: &authn.Request{HTTPRequest: &http.Request{Header: map[string][]string{authorizationHeaderName: {encodeBasicAuth("user", "")}}}},
blockLogin: true,
expectedErr: ErrBasicAuthCredentials,
expectedErr: errBasicAuthCredentials,
},
{
desc: "should fail when not found in any clients",
req: &authn.Request{HTTPRequest: &http.Request{Header: map[string][]string{authorizationHeaderName: {encodeBasicAuth("user", "password")}}}},
clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedErr: errIdentityNotFound}, authntest.FakePasswordClient{ExpectedErr: errIdentityNotFound}},
expectedErr: errBasicAuthCredentials,
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
hashed, _ := util.EncodePassword("password", "salt")
c := ProvideBasic(&usertest.FakeUserService{
ExpectedUser: &user.User{
Password: hashed,
Salt: "salt",
},
ExpectedSignedInUser: tt.expectedSignedInUser,
}, loginattempttest.FakeLoginAttemptService{
ExpectedValid: !tt.blockLogin,
})
c := ProvideBasic(
loginattempttest.FakeLoginAttemptService{ExpectedValid: !tt.blockLogin},
tt.clients...,
)
identity, err := c.Authenticate(context.Background(), tt.req)
if tt.expectedErr != nil {
@@ -77,9 +74,10 @@ func TestBasic_Authenticate(t *testing.T) {
func TestBasic_Test(t *testing.T) {
type TestCase struct {
desc string
req *authn.Request
expected bool
desc string
req *authn.Request
noClients bool
expected bool
}
tests := []TestCase{
@@ -94,6 +92,18 @@ func TestBasic_Test(t *testing.T) {
},
expected: true,
},
{
desc: "should fail when no password client is configured",
req: &authn.Request{
HTTPRequest: &http.Request{
Header: map[string][]string{
authorizationHeaderName: {encodeBasicAuth("user", "password")},
},
},
},
noClients: true,
expected: false,
},
{
desc: "should fail when no http request is passed",
req: &authn.Request{},
@@ -114,7 +124,10 @@ func TestBasic_Test(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
c := ProvideBasic(usertest.NewUserServiceFake(), loginattempttest.FakeLoginAttemptService{})
c := ProvideBasic(loginattempttest.FakeLoginAttemptService{}, authntest.FakePasswordClient{})
if tt.noClients {
c.clients = nil
}
assert.Equal(t, tt.expected, c.Test(context.Background(), tt.req))
})
}

View File

@@ -1,7 +1,14 @@
package clients
import "github.com/grafana/grafana/pkg/util/errutil"
const (
basicPrefix = "Basic "
bearerPrefix = "Bearer "
authorizationHeaderName = "Authorization"
)
var (
errIdentityNotFound = errutil.NewBase(errutil.StatusNotFound, "identity.not-found")
errInvalidPassword = errutil.NewBase(errutil.StatusBadRequest, "identity.invalid-password", errutil.WithPublicMessage("Invalid password or username"))
)

View File

@@ -0,0 +1,48 @@
package clients
import (
"context"
"crypto/subtle"
"errors"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util"
)
var _ authn.PasswordClient = new(Grafana)
func ProvideGrafana(userService user.Service) *Grafana {
return &Grafana{userService}
}
type Grafana struct {
userService user.Service
}
func (c Grafana) AuthenticatePassword(ctx context.Context, orgID int64, username, password string) (*authn.Identity, error) {
usr, err := c.userService.GetByLogin(ctx, &user.GetUserByLoginQuery{LoginOrEmail: username})
if err != nil {
if errors.Is(err, user.ErrUserNotFound) {
return nil, errIdentityNotFound.Errorf("no user fund: %w", err)
}
return nil, err
}
if ok := comparePassword(password, usr.Salt, usr.Password); !ok {
return nil, errInvalidPassword.Errorf("invalid password")
}
signedInUser, err := c.userService.GetSignedInUserWithCacheCtx(ctx, &user.GetSignedInUserQuery{OrgID: orgID, UserID: usr.ID})
if err != nil {
return nil, err
}
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, signedInUser.UserID), signedInUser, authn.ClientParams{}), nil
}
func comparePassword(password, salt, hash string) bool {
// It is ok to ignore the error here because util.EncodePassword can never return a error
hashedPassword, _ := util.EncodePassword(password, salt)
return subtle.ConstantTimeCompare([]byte(hashedPassword), []byte(hash)) == 1
}

View File

@@ -0,0 +1,69 @@
package clients
import (
"context"
"testing"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
"github.com/grafana/grafana/pkg/util"
"github.com/stretchr/testify/assert"
)
func TestGrafana_AuthenticatePassword(t *testing.T) {
type testCase struct {
desc string
username string
password string
findUser bool
expectedErr error
expectedIdentity *authn.Identity
expectedSignedInUser *user.SignedInUser
}
tests := []testCase{
{
desc: "should successfully authenticate user with correct password",
username: "user",
password: "password",
findUser: true,
expectedSignedInUser: &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: "Viewer"},
expectedIdentity: &authn.Identity{ID: "user:1", OrgID: 1, OrgRoles: map[int64]org.RoleType{1: "Viewer"}, IsGrafanaAdmin: boolPtr(false)},
},
{
desc: "should fail for incorrect password",
username: "user",
password: "wrong",
findUser: true,
expectedErr: errInvalidPassword,
},
{
desc: "should fail if user is not found",
username: "user",
password: "password",
expectedErr: errIdentityNotFound,
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
hashed, _ := util.EncodePassword("password", "salt")
userService := &usertest.FakeUserService{
ExpectedSignedInUser: tt.expectedSignedInUser,
ExpectedUser: &user.User{Password: hashed, Salt: "salt"},
}
if !tt.findUser {
userService.ExpectedUser = nil
userService.ExpectedError = user.ErrUserNotFound
}
c := ProvideGrafana(userService)
identity, err := c.AuthenticatePassword(context.Background(), 1, tt.username, tt.password)
assert.ErrorIs(t, err, tt.expectedErr)
assert.EqualValues(t, tt.expectedIdentity, identity)
})
}
}

View File

@@ -0,0 +1,81 @@
package clients
import (
"context"
"errors"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/multildap"
"github.com/grafana/grafana/pkg/setting"
)
var _ authn.PasswordClient = new(LDAP)
func ProvideLDAP(cfg *setting.Cfg) *LDAP {
return &LDAP{cfg, &ldapServiceImpl{cfg}}
}
type LDAP struct {
cfg *setting.Cfg
service ldapService
}
func (c *LDAP) AuthenticatePassword(ctx context.Context, orgID int64, username, password string) (*authn.Identity, error) {
info, err := c.service.Login(&models.LoginUserQuery{
Username: username,
Password: password,
})
if err != nil {
if errors.Is(err, multildap.ErrInvalidCredentials) {
return nil, errInvalidPassword.Errorf("invalid password: %w", err)
}
// FIXME: disable user in grafana if not found
if errors.Is(err, multildap.ErrCouldNotFindUser) {
return nil, errIdentityNotFound.Errorf("no user found: %w", err)
}
return nil, err
}
return &authn.Identity{
OrgID: orgID,
OrgRoles: info.OrgRoles,
Login: info.Login,
Name: info.Name,
Email: info.Email,
IsGrafanaAdmin: info.IsGrafanaAdmin,
AuthModule: info.AuthModule,
AuthID: info.AuthId,
LookUpParams: models.UserLookupParams{
Login: &info.Login,
Email: &info.Email,
},
Groups: info.Groups,
ClientParams: authn.ClientParams{
SyncUser: true,
SyncTeamMembers: true,
AllowSignUp: c.cfg.LDAPAllowSignup,
EnableDisabledUsers: true,
},
}, nil
}
type ldapService interface {
Login(query *models.LoginUserQuery) (*models.ExternalUserInfo, error)
}
// FIXME: remove the implementation if we convert ldap to an actual service
type ldapServiceImpl struct {
cfg *setting.Cfg
}
func (s *ldapServiceImpl) Login(query *models.LoginUserQuery) (*models.ExternalUserInfo, error) {
cfg, err := multildap.GetConfig(s.cfg)
if err != nil {
return nil, err
}
return multildap.New(cfg.Servers).Login(query)
}

View File

@@ -0,0 +1,102 @@
package clients
import (
"context"
"testing"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/ldap"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/setting"
"github.com/stretchr/testify/assert"
)
func TestLDAP_AuthenticatePassword(t *testing.T) {
type testCase struct {
desc string
username string
password string
expectedErr error
expectedLDAPErr error
expectedInfo *models.ExternalUserInfo
expectedIdentity *authn.Identity
}
tests := []testCase{
{
desc: "should successfully authenticate with correct username and password",
username: "test",
password: "test123",
expectedInfo: &models.ExternalUserInfo{
AuthModule: login.LDAPAuthModule,
AuthId: "123",
Email: "test@test.com",
Login: "test",
Name: "test test",
Groups: []string{"1", "2"},
OrgRoles: map[int64]org.RoleType{1: org.RoleViewer},
},
expectedIdentity: &authn.Identity{
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleViewer},
Login: "test",
Name: "test test",
Email: "test@test.com",
AuthModule: login.LDAPAuthModule,
AuthID: "123",
Groups: []string{"1", "2"},
ClientParams: authn.ClientParams{
SyncUser: true,
SyncTeamMembers: true,
AllowSignUp: false,
EnableDisabledUsers: true,
},
LookUpParams: models.UserLookupParams{
Email: strPtr("test@test.com"),
Login: strPtr("test"),
},
},
},
{
desc: "should fail if provided password was incorrect",
username: "test",
password: "wrong",
expectedErr: errInvalidPassword,
expectedLDAPErr: ldap.ErrInvalidCredentials,
},
{
desc: "should fail if not found",
username: "test",
password: "wrong",
expectedErr: errIdentityNotFound,
expectedLDAPErr: ldap.ErrCouldNotFindUser,
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
c := &LDAP{cfg: setting.NewCfg(), service: fakeLDAPService{ExpectedInfo: tt.expectedInfo, ExpectedErr: tt.expectedLDAPErr}}
identity, err := c.AuthenticatePassword(context.Background(), 1, tt.username, tt.password)
assert.ErrorIs(t, err, tt.expectedErr)
assert.EqualValues(t, tt.expectedIdentity, identity)
})
}
}
func strPtr(s string) *string {
return &s
}
var _ ldapService = new(fakeLDAPService)
type fakeLDAPService struct {
ExpectedErr error
ExpectedInfo *models.ExternalUserInfo
}
func (f fakeLDAPService) Login(query *models.LoginUserQuery) (*models.ExternalUserInfo, error) {
return f.ExpectedInfo, f.ExpectedErr
}