From 666ba61606b0b4060325e4c3d1d88c0ddf9da05b Mon Sep 17 00:00:00 2001 From: Dobromir Zahariev Date: Thu, 30 Sep 2021 12:56:26 +0300 Subject: [PATCH] Fix all system auths are assumed to be OneTimeTokens (#2046) * Fix all system auths are assumed to be OneTimeTokens * Update values.yaml * Check imports --- chart/compass/values.yaml | 2 +- .../internal/domain/application/resolver.go | 24 ++++++------ .../domain/application/resolver_test.go | 37 +++++++++++++++++-- .../internal/domain/systemauth/service.go | 7 +++- .../director/pkg/graphql/one_time_token.go | 2 +- components/director/pkg/graphql/schema_gen.go | 2 - 6 files changed, 54 insertions(+), 20 deletions(-) diff --git a/chart/compass/values.yaml b/chart/compass/values.yaml index c96dffc8f7..8263e559e3 100644 --- a/chart/compass/values.yaml +++ b/chart/compass/values.yaml @@ -75,7 +75,7 @@ global: version: "PR-2027" director: dir: - version: "PR-2040" + version: "PR-2046" gateway: dir: version: "PR-2027" diff --git a/components/director/internal/domain/application/resolver.go b/components/director/internal/domain/application/resolver.go index b451e5434d..440295a5ba 100644 --- a/components/director/internal/domain/application/resolver.go +++ b/components/director/internal/domain/application/resolver.go @@ -553,25 +553,27 @@ func (r *Resolver) Auths(ctx context.Context, obj *graphql.Application) ([]*grap out := make([]*graphql.AppSystemAuth, 0, len(sysAuths)) for _, sa := range sysAuths { - isTokenType := r.sysAuthSvc.IsSystemAuthOneTimeTokenType(&sa) - if _, err := r.oneTimeTokenSvc.IsTokenValid(&sa); isTokenType && err != nil { - log.C(ctx).WithError(err).Errorf("skipping one-time token due to its expiration or usage") - continue - } - c, err := r.sysAuthConv.ToGraphQL(&sa) if err != nil { return nil, err } - if sa.Value.OneTimeToken != nil && sa.Value.OneTimeToken.Type == tokens.ApplicationToken { - oneTimeTokenForApplication, err := r.oneTimeTokenConv.ToGraphQLForApplication(*sa.Value.OneTimeToken) - if err != nil { - return nil, errors.Wrap(err, "while converting one-time token to graphql") + if r.sysAuthSvc.IsSystemAuthOneTimeTokenType(&sa) { + if valid, err := r.oneTimeTokenSvc.IsTokenValid(&sa); !valid { + log.C(ctx).WithError(err).Errorf("skipping one-time token due to its expiration or usage") + continue } - c.(*graphql.AppSystemAuth).Auth.OneTimeToken = &oneTimeTokenForApplication + if sa.Value.OneTimeToken.Type == tokens.ApplicationToken { + oneTimeTokenForApplication, err := r.oneTimeTokenConv.ToGraphQLForApplication(*sa.Value.OneTimeToken) + if err != nil { + return nil, errors.Wrap(err, "while converting one-time token to graphql") + } + + c.(*graphql.AppSystemAuth).Auth.OneTimeToken = &oneTimeTokenForApplication + } } + out = append(out, c.(*graphql.AppSystemAuth)) } diff --git a/components/director/internal/domain/application/resolver_test.go b/components/director/internal/domain/application/resolver_test.go index 7407fa20ce..db382faeac 100644 --- a/components/director/internal/domain/application/resolver_test.go +++ b/components/director/internal/domain/application/resolver_test.go @@ -1330,7 +1330,9 @@ func TestResolver_Auths(t *testing.T) { txGen := txtest.NewTransactionContextGenerator(testError) sysAuthModels := []model.SystemAuth{{ID: "id1", AppID: &id, Value: &auth}, {ID: "id2", AppID: &id, Value: &auth}} + sysAuthModelCert := []model.SystemAuth{{ID: "id1", AppID: &id, Value: nil}} sysAuthGQL := []*graphql.AppSystemAuth{{ID: "id1", Auth: &graphql.Auth{}}, {ID: "id2", Auth: &graphql.Auth{}}} + sysAuthGQLCert := []*graphql.AppSystemAuth{{ID: "id1", Auth: nil}} sysAuthExpected := []*graphql.AppSystemAuth{{ID: "id1", Auth: &graphql.Auth{OneTimeToken: &gqlAuth}}, {ID: "id2", Auth: &graphql.Auth{OneTimeToken: &gqlAuth}}} emptySysAuth := make([]*graphql.AppSystemAuth, 0) testCases := []struct { @@ -1375,6 +1377,34 @@ func TestResolver_Auths(t *testing.T) { ExpectedResult: sysAuthExpected, ExpectedErr: nil, }, + { + Name: "Success when System Auth is certificate", + TransactionerFn: txGen.ThatSucceeds, + ServiceFn: func() *automock.SystemAuthService { + svc := &automock.SystemAuthService{} + svc.On("ListForObject", txtest.CtxWithDBMatcher(), model.ApplicationReference, id).Return(sysAuthModelCert, nil).Once() + svc.On("IsSystemAuthOneTimeTokenType", &sysAuthModelCert[0]).Return(false).Once() + return svc + }, + SysAuthConvFn: func() *automock.SystemAuthConverter { + sysAuthConv := &automock.SystemAuthConverter{} + sysAuthConv.On("ToGraphQL", &sysAuthModelCert[0]).Return(sysAuthGQLCert[0], nil).Once() + return sysAuthConv + }, + TokenSvcFn: func() *automock.OneTimeTokenService { + svc := &automock.OneTimeTokenService{} + svc.AssertNotCalled(t, "IsTokenValid") + return svc + }, + TokenConvFn: func() *automock.TokenConverter { + conv := &automock.TokenConverter{} + conv.AssertNotCalled(t, "ToGraphQLForApplication") + return conv + }, + InputApp: gqlApp, + ExpectedResult: sysAuthGQLCert, + ExpectedErr: nil, + }, { Name: "Returns nothing when tokens are invalid", TransactionerFn: txGen.ThatSucceeds, @@ -1387,7 +1417,8 @@ func TestResolver_Auths(t *testing.T) { }, SysAuthConvFn: func() *automock.SystemAuthConverter { sysAuthConv := &automock.SystemAuthConverter{} - sysAuthConv.AssertNotCalled(t, "ToGraphQL") + sysAuthConv.On("ToGraphQL", &sysAuthModels[0]).Return(sysAuthGQL[0], nil).Once() + sysAuthConv.On("ToGraphQL", &sysAuthModels[1]).Return(sysAuthGQL[1], nil).Once() return sysAuthConv }, TokenSvcFn: func() *automock.OneTimeTokenService { @@ -1466,7 +1497,7 @@ func TestResolver_Auths(t *testing.T) { ServiceFn: func() *automock.SystemAuthService { svc := &automock.SystemAuthService{} svc.On("ListForObject", txtest.CtxWithDBMatcher(), model.ApplicationReference, id).Return(sysAuthModels, nil).Once() - svc.On("IsSystemAuthOneTimeTokenType", &sysAuthModels[0]).Return(true).Once() + svc.AssertNotCalled(t, "IsSystemAuthOneTimeTokenType") return svc }, SysAuthConvFn: func() *automock.SystemAuthConverter { @@ -1476,7 +1507,7 @@ func TestResolver_Auths(t *testing.T) { }, TokenSvcFn: func() *automock.OneTimeTokenService { svc := &automock.OneTimeTokenService{} - svc.On("IsTokenValid", &sysAuthModels[0]).Return(true, nil).Once() + svc.AssertNotCalled(t, "IsTokenValid") return svc }, TokenConvFn: func() *automock.TokenConverter { diff --git a/components/director/internal/domain/systemauth/service.go b/components/director/internal/domain/systemauth/service.go index 8adc73680f..7bf4f9467f 100644 --- a/components/director/internal/domain/systemauth/service.go +++ b/components/director/internal/domain/systemauth/service.go @@ -192,8 +192,11 @@ func (s *service) DeleteByIDForObject(ctx context.Context, objectType model.Syst // IsSystemAuthOneTimeTokenType missing godoc func (s *service) IsSystemAuthOneTimeTokenType(systemAuth *model.SystemAuth) bool { - if (systemAuth != nil && systemAuth.Value != nil) && - (systemAuth.Value.Credential.Basic != nil || systemAuth.Value.Credential.Oauth != nil || systemAuth.Value.RequestAuth != nil) { + if systemAuth == nil || systemAuth.Value == nil { + return false + } + + if systemAuth.Value.Credential.Basic != nil || systemAuth.Value.Credential.Oauth != nil || systemAuth.Value.RequestAuth != nil { return false } diff --git a/components/director/pkg/graphql/one_time_token.go b/components/director/pkg/graphql/one_time_token.go index abf36ad79d..092515a8c7 100644 --- a/components/director/pkg/graphql/one_time_token.go +++ b/components/director/pkg/graphql/one_time_token.go @@ -13,7 +13,7 @@ type OneTimeTokenForApplication struct { } // IsOneTimeToken missing godoc -func (OneTimeTokenForApplication) IsOneTimeToken() {} +func (t *OneTimeTokenForApplication) IsOneTimeToken() {} // OneTimeTokenForRuntime missing godoc type OneTimeTokenForRuntime struct { diff --git a/components/director/pkg/graphql/schema_gen.go b/components/director/pkg/graphql/schema_gen.go index 0bb4d783b8..7a5a9e4c26 100644 --- a/components/director/pkg/graphql/schema_gen.go +++ b/components/director/pkg/graphql/schema_gen.go @@ -22670,8 +22670,6 @@ func (ec *executionContext) _OneTimeToken(ctx context.Context, sel ast.Selection switch obj := (obj).(type) { case nil: return graphql.Null - case OneTimeTokenForApplication: - return ec._OneTimeTokenForApplication(ctx, sel, &obj) case *OneTimeTokenForApplication: if obj == nil { return graphql.Null