AuthN: Make client params part of the identity (#61050)

* AuthN: Change client params to be a return value of authenticate

* AuthN: move client params to be part of the identity
This commit is contained in:
Karl Persson
2023-01-05 20:17:41 +01:00
committed by GitHub
parent 183397194a
commit cdd7392f68
14 changed files with 91 additions and 128 deletions

View File

@ -30,8 +30,7 @@ type ClientParams struct {
EnableDisabledUsers bool
}
type PostAuthHookFn func(ctx context.Context,
clientParams *ClientParams, identity *Identity, r *Request) error
type PostAuthHookFn func(ctx context.Context, identity *Identity, r *Request) error
type Service interface {
// RegisterPostAuthHook registers a hook that is called after a successful authentication.
@ -43,7 +42,6 @@ type Service interface {
type Client interface {
// Authenticate performs the authentication for the request
Authenticate(ctx context.Context, r *Request) (*Identity, error)
ClientParams() *ClientParams
// Test should return true if client can be used to authenticate request
Test(ctx context.Context, r *Request) bool
}
@ -84,6 +82,7 @@ type Identity struct {
OAuthToken *oauth2.Token
SessionToken *auth.UserToken
ClientParams ClientParams
}
func (i *Identity) Role() org.RoleType {
@ -157,7 +156,7 @@ func NamespacedID(namespace string, id int64) string {
return fmt.Sprintf("%s:%d", namespace, id)
}
func IdentityFromSignedInUser(id string, usr *user.SignedInUser) *Identity {
func IdentityFromSignedInUser(id string, usr *user.SignedInUser, params ClientParams) *Identity {
return &Identity{
ID: id,
OrgID: usr.OrgID,
@ -172,5 +171,6 @@ func IdentityFromSignedInUser(id string, usr *user.SignedInUser) *Identity {
HelpFlags1: usr.HelpFlags1,
LastSeenAt: usr.LastSeenAt,
Teams: usr.Teams,
ClientParams: params,
}
}

View File

@ -98,9 +98,8 @@ func (s *Service) Authenticate(ctx context.Context, client string, r *authn.Requ
return nil, true, err
}
params := c.ClientParams()
for _, hook := range s.postAuthHooks {
if err := hook(ctx, params, identity, r); err != nil {
if err := hook(ctx, identity, r); err != nil {
return nil, false, err
}
}

View File

@ -27,9 +27,8 @@ type OrgSync struct {
log log.Logger
}
func (s *OrgSync) SyncOrgUser(ctx context.Context,
clientParams *authn.ClientParams, id *authn.Identity, _ *authn.Request) error {
if !clientParams.SyncUser {
func (s *OrgSync) SyncOrgUser(ctx context.Context, id *authn.Identity, _ *authn.Request) error {
if !id.ClientParams.SyncUser {
s.log.Debug("Not syncing org user", "auth_module", id.AuthModule, "auth_id", id.AuthID)
return nil
}

View File

@ -50,9 +50,8 @@ func TestOrgSync_SyncOrgUser(t *testing.T) {
log log.Logger
}
type args struct {
ctx context.Context
clientParams *authn.ClientParams
id *authn.Identity
ctx context.Context
id *authn.Identity
}
tests := []struct {
name string
@ -71,9 +70,6 @@ func TestOrgSync_SyncOrgUser(t *testing.T) {
},
args: args{
ctx: context.Background(),
clientParams: &authn.ClientParams{
SyncUser: true,
},
id: &authn.Identity{
ID: "user:1",
Login: "test",
@ -86,6 +82,9 @@ func TestOrgSync_SyncOrgUser(t *testing.T) {
Email: ptrString("test"),
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
},
wantID: &authn.Identity{
@ -101,6 +100,9 @@ func TestOrgSync_SyncOrgUser(t *testing.T) {
Email: ptrString("test"),
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
wantErr: false,
},
@ -113,7 +115,7 @@ func TestOrgSync_SyncOrgUser(t *testing.T) {
accessControl: tt.fields.accessControl,
log: tt.fields.log,
}
if err := s.SyncOrgUser(tt.args.ctx, tt.args.clientParams, tt.args.id, nil); (err != nil) != tt.wantErr {
if err := s.SyncOrgUser(tt.args.ctx, tt.args.id, nil); (err != nil) != tt.wantErr {
t.Errorf("OrgSync.SyncOrgUser() error = %v, wantErr %v", err, tt.wantErr)
}

View File

@ -25,9 +25,8 @@ type UserSync struct {
}
// SyncUser syncs a user with the database
func (s *UserSync) SyncUser(ctx context.Context,
clientParams *authn.ClientParams, id *authn.Identity, _ *authn.Request) error {
if !clientParams.SyncUser {
func (s *UserSync) SyncUser(ctx context.Context, id *authn.Identity, _ *authn.Request) error {
if !id.ClientParams.SyncUser {
s.log.Debug("Not syncing user", "auth_module", id.AuthModule, "auth_id", id.AuthID)
return nil
}
@ -39,7 +38,7 @@ func (s *UserSync) SyncUser(ctx context.Context,
}
if errors.Is(errUserInDB, user.ErrUserNotFound) {
if !clientParams.AllowSignUp {
if !id.ClientParams.AllowSignUp {
s.log.Warn("Not allowing login, user not found in internal user database and allow signup = false",
"auth_module", id.AuthModule)
return login.ErrSignupNotAllowed
@ -54,7 +53,7 @@ func (s *UserSync) SyncUser(ctx context.Context,
}
// update user
if errUpdate := s.updateUserAttributes(ctx, clientParams, usr, id); errUpdate != nil {
if errUpdate := s.updateUserAttributes(ctx, usr, id); errUpdate != nil {
return errUpdate
}
@ -99,7 +98,7 @@ func (s *UserSync) updateAuthInfo(ctx context.Context, id *authn.Identity) error
return s.authInfoService.UpdateAuthInfo(ctx, updateCmd)
}
func (s *UserSync) updateUserAttributes(ctx context.Context, clientParams *authn.ClientParams, usr *user.User, id *authn.Identity) error {
func (s *UserSync) updateUserAttributes(ctx context.Context, usr *user.User, id *authn.Identity) error {
// sync user info
updateCmd := &user.UpdateUserCommand{
UserID: usr.ID,
@ -131,7 +130,7 @@ func (s *UserSync) updateUserAttributes(ctx context.Context, clientParams *authn
}
}
if usr.IsDisabled && clientParams.EnableDisabledUsers {
if usr.IsDisabled && id.ClientParams.EnableDisabledUsers {
usr.IsDisabled = false
if errDisableUser := s.userService.Disable(ctx,
&user.DisableUserCommand{

View File

@ -85,9 +85,8 @@ func TestUserSync_SyncUser(t *testing.T) {
log log.Logger
}
type args struct {
ctx context.Context
clientParams *authn.ClientParams
id *authn.Identity
ctx context.Context
id *authn.Identity
}
tests := []struct {
name string
@ -106,11 +105,6 @@ func TestUserSync_SyncUser(t *testing.T) {
},
args: args{
ctx: context.Background(),
clientParams: &authn.ClientParams{
SyncUser: false,
AllowSignUp: false,
EnableDisabledUsers: false,
},
id: &authn.Identity{
ID: "",
Login: "test",
@ -121,6 +115,7 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: ptrString("test"),
Login: nil,
},
ClientParams: authn.ClientParams{},
},
},
wantErr: false,
@ -134,6 +129,7 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: ptrString("test"),
Login: nil,
},
ClientParams: authn.ClientParams{},
},
},
{
@ -146,11 +142,6 @@ func TestUserSync_SyncUser(t *testing.T) {
},
args: args{
ctx: context.Background(),
clientParams: &authn.ClientParams{
SyncUser: true,
AllowSignUp: false,
EnableDisabledUsers: false,
},
id: &authn.Identity{
ID: "",
Login: "test",
@ -161,6 +152,9 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: ptrString("test"),
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
},
wantErr: false,
@ -175,6 +169,9 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: ptrString("test"),
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
},
{
@ -187,11 +184,6 @@ func TestUserSync_SyncUser(t *testing.T) {
},
args: args{
ctx: context.Background(),
clientParams: &authn.ClientParams{
SyncUser: true,
AllowSignUp: false,
EnableDisabledUsers: false,
},
id: &authn.Identity{
ID: "",
Login: "test",
@ -202,6 +194,9 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: nil,
Login: ptrString("test"),
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
},
wantErr: false,
@ -216,6 +211,9 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: nil,
Login: ptrString("test"),
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
},
{
@ -228,11 +226,6 @@ func TestUserSync_SyncUser(t *testing.T) {
},
args: args{
ctx: context.Background(),
clientParams: &authn.ClientParams{
SyncUser: true,
AllowSignUp: false,
EnableDisabledUsers: false,
},
id: &authn.Identity{
ID: "",
Login: "test",
@ -243,6 +236,9 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: nil,
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
},
wantErr: false,
@ -257,6 +253,9 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: nil,
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
},
{
@ -269,11 +268,7 @@ func TestUserSync_SyncUser(t *testing.T) {
},
args: args{
ctx: context.Background(),
clientParams: &authn.ClientParams{
SyncUser: true,
AllowSignUp: false,
EnableDisabledUsers: false,
},
id: &authn.Identity{
ID: "",
Login: "test",
@ -284,6 +279,9 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: nil,
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
},
wantErr: false,
@ -298,6 +296,9 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: nil,
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
},
{
@ -310,11 +311,6 @@ func TestUserSync_SyncUser(t *testing.T) {
},
args: args{
ctx: context.Background(),
clientParams: &authn.ClientParams{
SyncUser: true,
AllowSignUp: false,
EnableDisabledUsers: false,
},
id: &authn.Identity{
ID: "",
Login: "test",
@ -327,6 +323,9 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: nil,
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
},
},
},
wantErr: true,
@ -341,11 +340,6 @@ func TestUserSync_SyncUser(t *testing.T) {
},
args: args{
ctx: context.Background(),
clientParams: &authn.ClientParams{
SyncUser: true,
AllowSignUp: true,
EnableDisabledUsers: true,
},
id: &authn.Identity{
ID: "",
Login: "test_create",
@ -359,6 +353,11 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: ptrString("test_create"),
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
AllowSignUp: true,
EnableDisabledUsers: true,
},
},
},
wantErr: false,
@ -375,6 +374,11 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: ptrString("test_create"),
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
AllowSignUp: true,
EnableDisabledUsers: true,
},
},
},
{
@ -387,11 +391,6 @@ func TestUserSync_SyncUser(t *testing.T) {
},
args: args{
ctx: context.Background(),
clientParams: &authn.ClientParams{
SyncUser: true,
AllowSignUp: false,
EnableDisabledUsers: true,
},
id: &authn.Identity{
ID: "",
Login: "test_mod",
@ -404,6 +403,10 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: nil,
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
EnableDisabledUsers: true,
},
},
},
wantErr: false,
@ -419,6 +422,10 @@ func TestUserSync_SyncUser(t *testing.T) {
Email: nil,
Login: nil,
},
ClientParams: authn.ClientParams{
SyncUser: true,
EnableDisabledUsers: true,
},
},
},
}
@ -430,7 +437,7 @@ func TestUserSync_SyncUser(t *testing.T) {
quotaService: tt.fields.quotaService,
log: tt.fields.log,
}
err := s.SyncUser(tt.args.ctx, tt.args.clientParams, tt.args.id, nil)
err := s.SyncUser(tt.args.ctx, tt.args.id, nil)
if tt.wantErr {
require.Error(t, err)
return

View File

@ -13,19 +13,16 @@ type FakeService struct {
var _ authn.Client = new(FakeClient)
type FakeClient struct {
ExpectedErr error
ExpectedTest bool
ExpectedIdentity *authn.Identity
ExpectedErr error
ExpectedTest bool
ExpectedIdentity *authn.Identity
ExpectedClientParams *authn.ClientParams
}
func (f *FakeClient) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identity, error) {
return f.ExpectedIdentity, f.ExpectedErr
}
func (f *FakeClient) ClientParams() *authn.ClientParams {
return &authn.ClientParams{}
}
func (f *FakeClient) Test(ctx context.Context, r *authn.Request) bool {
return f.ExpectedTest
}

View File

@ -10,7 +10,6 @@ var _ authn.Client = new(MockClient)
type MockClient struct {
AuthenticateFunc func(ctx context.Context, r *authn.Request) (*authn.Identity, error)
ClientParamsFunc func() *authn.ClientParams
TestFunc func(ctx context.Context, r *authn.Request) bool
}
@ -21,13 +20,6 @@ func (m MockClient) Authenticate(ctx context.Context, r *authn.Request) (*authn.
return nil, nil
}
func (m MockClient) ClientParams() *authn.ClientParams {
if m.ClientParamsFunc != nil {
return m.ClientParamsFunc()
}
return nil
}
func (m MockClient) Test(ctx context.Context, r *authn.Request) bool {
if m.TestFunc != nil {
return m.TestFunc(ctx, r)

View File

@ -33,16 +33,13 @@ func (a *Anonymous) Authenticate(ctx context.Context, r *authn.Request) (*authn.
}
return &authn.Identity{
OrgID: o.ID,
OrgName: o.Name,
OrgRoles: map[int64]org.RoleType{o.ID: org.RoleType(a.cfg.AnonymousOrgRole)},
OrgID: o.ID,
OrgName: o.Name,
OrgRoles: map[int64]org.RoleType{o.ID: org.RoleType(a.cfg.AnonymousOrgRole)},
ClientParams: authn.ClientParams{},
}, nil
}
func (a *Anonymous) ClientParams() *authn.ClientParams {
return &authn.ClientParams{}
}
func (a *Anonymous) Test(ctx context.Context, r *authn.Request) bool {
// If anonymous client is register it can always be used for authentication
return true

View File

@ -90,7 +90,7 @@ func (s *APIKey) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide
return nil, ErrServiceAccountDisabled.Errorf("Disabled service account")
}
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceServiceAccount, usr.UserID), usr), nil
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceServiceAccount, usr.UserID), usr, authn.ClientParams{}), nil
}
func (s *APIKey) getAPIKey(ctx context.Context, token string) (*apikey.APIKey, error) {
@ -145,10 +145,6 @@ func (s *APIKey) getFromTokenLegacy(ctx context.Context, token string) (*apikey.
return keyQuery.Result, nil
}
func (s *APIKey) ClientParams() *authn.ClientParams {
return &authn.ClientParams{}
}
func (s *APIKey) Test(ctx context.Context, r *authn.Request) bool {
return looksLikeApiKey(getTokenFromRequest(r))
}

View File

@ -66,11 +66,7 @@ func (c *Basic) Authenticate(ctx context.Context, r *authn.Request) (*authn.Iden
return nil, ErrBasicAuthCredentials.Errorf("failed to fetch user: %w", err)
}
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, signedInUser.UserID), signedInUser), nil
}
func (c *Basic) ClientParams() *authn.ClientParams {
return &authn.ClientParams{}
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, signedInUser.UserID), signedInUser, authn.ClientParams{}), nil
}
func (c *Basic) Test(ctx context.Context, r *authn.Request) bool {

View File

@ -38,9 +38,10 @@ func (c *Render) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide
if renderUsr.UserID <= 0 {
return &authn.Identity{
ID: authn.NamespacedID(authn.NamespaceUser, 0),
OrgID: renderUsr.OrgID,
OrgRoles: map[int64]org.RoleType{renderUsr.OrgID: org.RoleType(renderUsr.OrgRole)},
ID: authn.NamespacedID(authn.NamespaceUser, 0),
OrgID: renderUsr.OrgID,
OrgRoles: map[int64]org.RoleType{renderUsr.OrgID: org.RoleType(renderUsr.OrgRole)},
ClientParams: authn.ClientParams{},
}, nil
}
@ -48,11 +49,7 @@ func (c *Render) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide
if err != nil {
return nil, err
}
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, usr.UserID), usr), nil
}
func (c *Render) ClientParams() *authn.ClientParams {
return &authn.ClientParams{}
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, usr.UserID), usr, authn.ClientParams{}), nil
}
func (c *Render) Test(ctx context.Context, r *authn.Request) bool {

View File

@ -36,14 +36,6 @@ type Session struct {
log log.Logger
}
func (s *Session) ClientParams() *authn.ClientParams {
return &authn.ClientParams{
SyncUser: false,
AllowSignUp: false,
EnableDisabledUsers: false,
}
}
func (s *Session) Test(ctx context.Context, r *authn.Request) bool {
if s.loginCookieName == "" {
return false
@ -81,14 +73,13 @@ func (s *Session) Authenticate(ctx context.Context, r *authn.Request) (*authn.Id
}
// FIXME (jguer): oauth token refresh not implemented
identity := authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, signedInUser.UserID), signedInUser)
identity := authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, signedInUser.UserID), signedInUser, authn.ClientParams{})
identity.SessionToken = token
return identity, nil
}
func (s *Session) RefreshTokenHook(ctx context.Context,
clientParams *authn.ClientParams, identity *authn.Identity, r *authn.Request) error {
func (s *Session) RefreshTokenHook(ctx context.Context, identity *authn.Identity, r *authn.Request) error {
if identity.SessionToken == nil {
return nil
}

View File

@ -20,15 +20,6 @@ import (
"github.com/stretchr/testify/require"
)
func TestSession_ClientParams(t *testing.T) {
s := ProvideSession(nil, nil, "", 0)
require.Equal(t, &authn.ClientParams{
SyncUser: false,
AllowSignUp: false,
EnableDisabledUsers: false,
}, s.ClientParams())
}
func TestSession_Test(t *testing.T) {
cookieName := "grafana_session"
@ -178,7 +169,7 @@ func TestSession_RefreshHook(t *testing.T) {
Resp: web.NewResponseWriter(http.MethodConnect, mockResponseWriter),
}
err := s.RefreshTokenHook(context.Background(), &authn.ClientParams{}, sampleID, resp)
err := s.RefreshTokenHook(context.Background(), sampleID, resp)
require.NoError(t, err)
resp.Resp.WriteHeader(201)