diff --git a/pkg/services/authn/authn.go b/pkg/services/authn/authn.go index 676c8f8987b..29d6637ba2a 100644 --- a/pkg/services/authn/authn.go +++ b/pkg/services/authn/authn.go @@ -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 diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index ffb04a9e0f9..89b3cc6a46d 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -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 diff --git a/pkg/services/authn/authntest/fake.go b/pkg/services/authn/authntest/fake.go index df95b6ed1cf..6b5f2021577 100644 --- a/pkg/services/authn/authntest/fake.go +++ b/pkg/services/authn/authntest/fake.go @@ -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 +} diff --git a/pkg/services/authn/clients/basic.go b/pkg/services/authn/clients/basic.go index a66b98ee9e1..71279db4c8d 100644 --- a/pkg/services/authn/clients/basic.go +++ b/pkg/services/authn/clients/basic.go @@ -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 -} diff --git a/pkg/services/authn/clients/basic_test.go b/pkg/services/authn/clients/basic_test.go index 727b8a66c9f..1f61ade0dc7 100644 --- a/pkg/services/authn/clients/basic_test.go +++ b/pkg/services/authn/clients/basic_test.go @@ -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)) }) } diff --git a/pkg/services/authn/clients/constants.go b/pkg/services/authn/clients/constants.go index 1cddacfdfaf..d0c2f477f31 100644 --- a/pkg/services/authn/clients/constants.go +++ b/pkg/services/authn/clients/constants.go @@ -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")) +) diff --git a/pkg/services/authn/clients/grafana.go b/pkg/services/authn/clients/grafana.go new file mode 100644 index 00000000000..55032194f59 --- /dev/null +++ b/pkg/services/authn/clients/grafana.go @@ -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 +} diff --git a/pkg/services/authn/clients/grafana_test.go b/pkg/services/authn/clients/grafana_test.go new file mode 100644 index 00000000000..bea7c87c509 --- /dev/null +++ b/pkg/services/authn/clients/grafana_test.go @@ -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) + }) + } +} diff --git a/pkg/services/authn/clients/ldap.go b/pkg/services/authn/clients/ldap.go new file mode 100644 index 00000000000..fb868191e85 --- /dev/null +++ b/pkg/services/authn/clients/ldap.go @@ -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) +} diff --git a/pkg/services/authn/clients/ldap_test.go b/pkg/services/authn/clients/ldap_test.go new file mode 100644 index 00000000000..c29435b9e76 --- /dev/null +++ b/pkg/services/authn/clients/ldap_test.go @@ -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 +}