From 1b3ee79bc246475e1face426f39c3577668ea2e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Mon, 30 Oct 2023 17:13:08 +0100 Subject: [PATCH 1/7] Avoid redundant talk with the contracts server We don't need to querry the contract server if whe have a non-expired, store-backed Ubuntu Pro token. --- windows-agent/internal/config/config.go | 28 +++++++- windows-agent/internal/contracts/contracts.go | 71 ++++++++++++------- .../internal/contracts/contracts_test.go | 2 +- 3 files changed, 75 insertions(+), 26 deletions(-) diff --git a/windows-agent/internal/config/config.go b/windows-agent/internal/config/config.go index 902297700..4cde5447c 100644 --- a/windows-agent/internal/config/config.go +++ b/windows-agent/internal/config/config.go @@ -11,6 +11,7 @@ import ( "path/filepath" "sync" + "github.com/canonical/ubuntu-pro-for-windows/common" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config/registry" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/contracts" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/database" @@ -219,11 +220,36 @@ func (c *Config) FetchMicrosoftStoreSubscription(ctx context.Context) (err error return fmt.Errorf("subscription cannot be user-managed") } - proToken, err := contracts.ProToken(ctx) + _, src, err := c.Subscription(ctx) + if err != nil { + return fmt.Errorf("could not get current subscription status: %v", err) + } + + // Shortcut to avoid spamming the contract server + // We don't need to request a new token if we have a non-expired one + if src == SourceMicrosoftStore { + valid, err := contracts.ValidSubscription() + if err != nil { + return fmt.Errorf("could not obtain current subscription status: %v", err) + } + + if valid { + log.Debug(ctx, "Microsoft Store subscription is active") + return nil + } + + log.Debug(ctx, "No valid Microsoft Store subscription") + } + + proToken, err := contracts.NewProToken(ctx) if err != nil { return fmt.Errorf("could not get ProToken from Microsoft Store: %v", err) } + if proToken != "" { + log.Debugf(ctx, "Obtained Ubuntu Pro token from the Microsoft Store: %q", common.Obfuscate(proToken)) + } + if err := c.setStoreSubscription(ctx, proToken); err != nil { return err } diff --git a/windows-agent/internal/contracts/contracts.go b/windows-agent/internal/contracts/contracts.go index aa7794bd3..ff9e92119 100644 --- a/windows-agent/internal/contracts/contracts.go +++ b/windows-agent/internal/contracts/contracts.go @@ -3,6 +3,7 @@ package contracts import ( "context" + "errors" "fmt" "net/http" "net/url" @@ -10,7 +11,6 @@ import ( "github.com/canonical/ubuntu-pro-for-windows/storeapi/go-wrapper/microsoftstore" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/contracts/contractclient" - "github.com/ubuntu/decorate" ) type options struct { @@ -52,10 +52,35 @@ func (msftStoreDLL) GetSubscriptionExpirationDate() (tm time.Time, err error) { return microsoftstore.GetSubscriptionExpirationDate() } -// ProToken directs the dance between the Microsoft Store and the Ubuntu Pro contract server to +// ValidSubscription returns true if there is a subscription via the Microsoft Store and it is not expired. +func ValidSubscription(args ...Option) (bool, error) { + opts := options{ + microsoftStore: msftStoreDLL{}, + } + + for _, f := range args { + f(&opts) + } + + expiration, err := opts.microsoftStore.GetSubscriptionExpirationDate() + if isErrNotSubscribed(err) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("could not get subscription expiration date: %v", err) + } + + if expiration.Before(time.Now()) { + return false, nil + } + + return true, nil +} + +// NewProToken directs the dance between the Microsoft Store and the Ubuntu Pro contract server to // validate a store entitlement and obtain its associated pro token. If there is no entitlement, // the token is returned as an empty string. -func ProToken(ctx context.Context, args ...Option) (token string, err error) { +func NewProToken(ctx context.Context, args ...Option) (token string, err error) { opts := options{ microsoftStore: msftStoreDLL{}, } @@ -73,41 +98,39 @@ func ProToken(ctx context.Context, args ...Option) (token string, err error) { } contractClient := contractclient.New(opts.proURL, &http.Client{Timeout: 30 * time.Second}) + msftStore := opts.microsoftStore - token, err = proToken(ctx, contractClient, opts.microsoftStore) + adToken, err := contractClient.GetServerAccessToken(ctx) if err != nil { return "", err } - return token, nil -} - -func proToken(ctx context.Context, serverClient *contractclient.Client, msftStore MicrosoftStore) (proToken string, err error) { - defer decorate.OnError(&err, "could not obtain pro token") - - expiration, err := msftStore.GetSubscriptionExpirationDate() + storeToken, err := msftStore.GenerateUserJWT(adToken) if err != nil { - return "", fmt.Errorf("could not get subscription expiration date: %v", err) - } - - if expiration.Before(time.Now()) { - return "", fmt.Errorf("the subscription has been expired since %s", expiration) + return "", err } - adToken, err := serverClient.GetServerAccessToken(ctx) + proToken, err := contractClient.GetProToken(ctx, storeToken) if err != nil { return "", err } - storeToken, err := msftStore.GenerateUserJWT(adToken) - if err != nil { - return "", err + return proToken, nil +} + +func isErrNotSubscribed(err error) bool { + if err == nil { + return false } - proToken, err = serverClient.GetProToken(ctx, storeToken) - if err != nil { - return "", err + var target microsoftstore.StoreAPIError + if !errors.As(err, &target) { + return false } - return proToken, nil + if target != microsoftstore.ErrNotSubscribed { + return false + } + + return true } diff --git a/windows-agent/internal/contracts/contracts_test.go b/windows-agent/internal/contracts/contracts_test.go index cda47b03c..0ea07384b 100644 --- a/windows-agent/internal/contracts/contracts_test.go +++ b/windows-agent/internal/contracts/contracts_test.go @@ -80,7 +80,7 @@ func TestProToken(t *testing.T) { url, err := url.Parse(fmt.Sprintf("http://%s", addr)) require.NoError(t, err, "Setup: Server URL should have been parsed with no issues") - token, err := contracts.ProToken(ctx, contracts.WithProURL(url), contracts.WithMockMicrosoftStore(store)) + token, err := contracts.NewProToken(ctx, contracts.WithProURL(url), contracts.WithMockMicrosoftStore(store)) if tc.wantErr { require.Error(t, err, "ProToken should return an error") return From ff521a796b38c607adde9f75a1ec842bec0b1471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Wed, 15 Nov 2023 09:04:02 +0100 Subject: [PATCH 2/7] Move testing of expiration dates to TestValidSubscription --- .../internal/contracts/contracts_test.go | 76 ++++++++++++++++--- 1 file changed, 64 insertions(+), 12 deletions(-) diff --git a/windows-agent/internal/contracts/contracts_test.go b/windows-agent/internal/contracts/contracts_test.go index 0ea07384b..919a5bcd8 100644 --- a/windows-agent/internal/contracts/contracts_test.go +++ b/windows-agent/internal/contracts/contracts_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/canonical/ubuntu-pro-for-windows/mocks/contractserver/contractsmockserver" + "github.com/canonical/ubuntu-pro-for-windows/storeapi/go-wrapper/microsoftstore" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/contracts" "github.com/stretchr/testify/require" ) @@ -24,9 +25,7 @@ func TestProToken(t *testing.T) { testCases := map[string]struct { // Microsoft store - expired bool - jwtError bool - expDateError bool + jwtError bool // Contract server getServerAccessTokenErr bool @@ -36,9 +35,7 @@ func TestProToken(t *testing.T) { }{ "Success": {}, - "Error when the subscription has expired": {expired: true, wantErr: true}, "Error when the store's GenerateUserJWT fails": {jwtError: true, wantErr: true}, - "Error when the store's GetSubscriptionExpirationDate fails": {expDateError: true, wantErr: true}, "Error when the contract server's GetServerAccessToken fails": {getServerAccessTokenErr: true, wantErr: true}, "Error when the contract server's GetProToken fails": {getProTokenErr: true, wantErr: true}, } @@ -50,18 +47,13 @@ func TestProToken(t *testing.T) { ctx := context.Background() store := mockMSStore{ - expirationDate: time.Now().Add(24 * 365 * time.Hour), // Next year - expirationDateErr: tc.expDateError, + expirationDate: time.Now().Add(24 * 365 * time.Hour), // Next year jwt: "JWT_123", jwtWantADToken: azureADToken, jwtErr: tc.jwtError, } - if tc.expired { - store.expirationDate = time.Now().Add(-24 * 365 * time.Hour) // Last year - } - settings := contractsmockserver.DefaultSettings() settings.Token.OnSuccess.Value = azureADToken @@ -92,11 +84,67 @@ func TestProToken(t *testing.T) { } } +func TestValidSubscription(t *testing.T) { + type subscriptionStatus int + const ( + subscribed subscriptionStatus = iota + expired + unsubscribed + ) + + testCases := map[string]struct { + status subscriptionStatus + expirationErr bool + + wantErr bool + }{ + "Succcess when the current subscription is active": {status: subscribed}, + "Succcess when the current subscription is expired": {status: expired}, + "Success when there is no subscription": {status: unsubscribed}, + + "Error when subscription validity cannot be ascertained": {status: subscribed, expirationErr: true, wantErr: true}, + } + + for name, tc := range testCases { + tc := tc + t.Run(name, func(t *testing.T) { + var store mockMSStore + var want bool + + switch tc.status { + case subscribed: + want = true + store.expirationDate = time.Now().Add(time.Hour * 24 * 365) // Next year + case expired: + want = false + store.expirationDate = time.Now().Add(-time.Hour * 24 * 365) // Last year + case unsubscribed: + want = false + store.notSubscribed = true + } + + if tc.expirationErr { + store.expirationDateErr = true + } + + got, err := contracts.ValidSubscription(contracts.WithMockMicrosoftStore(store)) + if tc.wantErr { + require.Error(t, err, "contracts.ValidSubscription should have returned an error") + return + } + + require.NoError(t, err, "contracts.ValidSubscription should have returned no error") + require.Equal(t, want, got, "Unexpected return from ValidSubscription") + }) + } +} + type mockMSStore struct { jwt string jwtWantADToken string jwtErr bool + notSubscribed bool expirationDate time.Time expirationDateErr bool } @@ -115,7 +163,11 @@ func (s mockMSStore) GenerateUserJWT(azureADToken string) (jwt string, err error func (s mockMSStore) GetSubscriptionExpirationDate() (tm time.Time, err error) { if s.expirationDateErr { - return time.Time{}, errors.New("mock error") + return time.Time{}, fmt.Errorf("mock error: %w", microsoftstore.ErrStoreAPI) + } + + if s.notSubscribed { + return time.Time{}, fmt.Errorf("mock error: %w", microsoftstore.ErrNotSubscribed) } return s.expirationDate, nil From 14730db535f0b6eb785a3b2daf86d53316a124bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Wed, 15 Nov 2023 09:14:01 +0100 Subject: [PATCH 3/7] Allow dependency injection in Config.FetchMicrosoftStoreSubscription This is necessary to test the function with mocked back-ends for the contract server and Microsoft Store. --- windows-agent/internal/config/config.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/windows-agent/internal/config/config.go b/windows-agent/internal/config/config.go index 4cde5447c..3a85db403 100644 --- a/windows-agent/internal/config/config.go +++ b/windows-agent/internal/config/config.go @@ -207,7 +207,7 @@ func (c *Config) LandscapeAgentUID(ctx context.Context) (string, error) { // FetchMicrosoftStoreSubscription contacts Ubuntu Pro's contract server and the Microsoft Store // to check if the user has an active subscription that provides a pro token. If so, that token is used. -func (c *Config) FetchMicrosoftStoreSubscription(ctx context.Context) (err error) { +func (c *Config) FetchMicrosoftStoreSubscription(ctx context.Context, args ...contracts.Option) (err error) { defer decorate.OnError(&err, "could not validate subscription against Microsoft Store") readOnly, err := c.IsReadOnly() @@ -228,7 +228,7 @@ func (c *Config) FetchMicrosoftStoreSubscription(ctx context.Context) (err error // Shortcut to avoid spamming the contract server // We don't need to request a new token if we have a non-expired one if src == SourceMicrosoftStore { - valid, err := contracts.ValidSubscription() + valid, err := contracts.ValidSubscription(args...) if err != nil { return fmt.Errorf("could not obtain current subscription status: %v", err) } @@ -241,7 +241,7 @@ func (c *Config) FetchMicrosoftStoreSubscription(ctx context.Context) (err error log.Debug(ctx, "No valid Microsoft Store subscription") } - proToken, err := contracts.NewProToken(ctx) + proToken, err := contracts.NewProToken(ctx, args...) if err != nil { return fmt.Errorf("could not get ProToken from Microsoft Store: %v", err) } From adc94c6f3627f777860e1d957aee431af519b275 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Tue, 14 Nov 2023 15:30:44 +0100 Subject: [PATCH 4/7] Expand TestFetchMicrosoftStoreSubscription with new mocks These tests were written before we had any mocks for the MS store or the Contract server. The previous commits significantly change the implementation of the FetchMicrosoftStoreSubscription function, so it is now a great time to flesh out its tests with the new mocks. --- windows-agent/internal/config/config_test.go | 87 ++++++++++++++++++-- 1 file changed, 79 insertions(+), 8 deletions(-) diff --git a/windows-agent/internal/config/config_test.go b/windows-agent/internal/config/config_test.go index b093693b3..9ef905819 100644 --- a/windows-agent/internal/config/config_test.go +++ b/windows-agent/internal/config/config_test.go @@ -3,14 +3,19 @@ package config_test import ( "context" "errors" + "fmt" "io/fs" + "net/url" "os" "path/filepath" "testing" + "time" "github.com/canonical/ubuntu-pro-for-windows/common/wsltestutils" + "github.com/canonical/ubuntu-pro-for-windows/mocks/contractserver/contractsmockserver" config "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config/registry" + "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/contracts" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/database" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/distro" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/task" @@ -412,21 +417,37 @@ func TestIsReadOnly(t *testing.T) { func TestFetchMicrosoftStoreSubscription(t *testing.T) { t.Parallel() + //nolint:gosec // These are not real credentials + const ( + proToken = "UBUNTU_PRO_TOKEN_456" + azureADToken = "AZURE_AD_TOKEN_789" + ) + testCases := map[string]struct { - settingsState settingsState + settingsState settingsState + subscriptionExpired bool + registryErr uint32 registryIsReadOnly bool + msStoreJWTErr bool + msStoreExpirationErr bool + wantToken string wantErr bool }{ - // TODO: Implement more test cases when the MS Store mock is available. There is no single successful test in here so far. - "Error when registry is read only": {settingsState: userTokenHasValue, registryIsReadOnly: true, wantToken: "user_token", wantErr: true}, - "Error when registry read-only check fails": {registryErr: registry.MockErrOnCreateKey, wantErr: true}, + // Tests where there is no pre-existing subscription + "Success": {wantToken: proToken}, - // Stub test-case: Must be replaced with Success/Error return values of contracts.ProToken - // when the Microsoft store dance is implemented. - "Error when the microsoft store is not yet implemented": {wantErr: true}, + "Error when registry is read only": {settingsState: userTokenHasValue, registryIsReadOnly: true, wantToken: "user_token", wantErr: true}, + "Error when registry read-only check fails": {registryErr: registry.MockErrOnCreateKey, wantErr: true}, + "Error when the Microsoft Store cannot provide the JWT": {msStoreJWTErr: true, wantErr: true}, + + // Tests where there is a pre-existing subscription + "Success when there is a store token already": {settingsState: storeTokenHasValue, wantToken: "store_token"}, + "Success when there is an expired store token": {settingsState: storeTokenHasValue, subscriptionExpired: true, wantToken: proToken}, + + "Error when the Microsoft Store cannot provide the expiration date": {settingsState: storeTokenHasValue, msStoreExpirationErr: true, wantToken: "store_token", wantErr: true}, } for name, tc := range testCases { @@ -439,7 +460,33 @@ func TestFetchMicrosoftStoreSubscription(t *testing.T) { r, dir := setUpMockSettings(t, tc.registryErr, tc.settingsState, tc.registryIsReadOnly, false) c := config.New(ctx, dir, config.WithRegistry(r)) - err := c.FetchMicrosoftStoreSubscription(ctx) + // Set up the mock Microsoft store + store := mockMSStore{ + expirationDate: time.Now().Add(24 * 365 * time.Hour), // Next year + expirationDateErr: tc.msStoreExpirationErr, + + jwt: "JWT_123", + jwtErr: tc.msStoreJWTErr, + } + + if tc.subscriptionExpired { + store.expirationDate = time.Now().Add(-24 * 365 * time.Hour) // Last year + } + + // Set up the mock contract server + csSettings := contractsmockserver.DefaultSettings() + csSettings.Token.OnSuccess.Value = azureADToken + csSettings.Subscription.OnSuccess.Value = proToken + server := contractsmockserver.NewServer(csSettings) + err := server.Serve(ctx, "localhost:0") + require.NoError(t, err, "Setup: Server should return no error") + //nolint:errcheck // Nothing we can do about it + defer server.Stop() + + csAddr, err := url.Parse(fmt.Sprintf("http://%s", server.Address())) + require.NoError(t, err, "Setup: Server URL should have been parsed with no issues") + + err = c.FetchMicrosoftStoreSubscription(ctx, contracts.WithProURL(csAddr), contracts.WithMockMicrosoftStore(store)) if tc.wantErr { require.Error(t, err, "FetchMicrosoftStoreSubscription should return an error") } else { @@ -455,6 +502,30 @@ func TestFetchMicrosoftStoreSubscription(t *testing.T) { } } +type mockMSStore struct { + jwt string + jwtErr bool + + expirationDate time.Time + expirationDateErr bool +} + +func (s mockMSStore) GenerateUserJWT(azureADToken string) (jwt string, err error) { + if s.jwtErr { + return "", errors.New("mock error") + } + + return s.jwt, nil +} + +func (s mockMSStore) GetSubscriptionExpirationDate() (tm time.Time, err error) { + if s.expirationDateErr { + return time.Time{}, errors.New("mock error") + } + + return s.expirationDate, nil +} + func TestUpdateRegistrySettings(t *testing.T) { if wsl.MockAvailable() { t.Parallel() From a0a349ed11ee94f8571d9e4a82035a32051511e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Wed, 15 Nov 2023 08:34:53 +0100 Subject: [PATCH 5/7] Adapt UI proservice to new FetchMicrosoftStore signature --- windows-agent/internal/proservices/ui/ui.go | 3 ++- windows-agent/internal/proservices/ui/ui_test.go | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/windows-agent/internal/proservices/ui/ui.go b/windows-agent/internal/proservices/ui/ui.go index 7a032f7a1..e5dfddd79 100644 --- a/windows-agent/internal/proservices/ui/ui.go +++ b/windows-agent/internal/proservices/ui/ui.go @@ -9,6 +9,7 @@ import ( agentapi "github.com/canonical/ubuntu-pro-for-windows/agentapi/go" "github.com/canonical/ubuntu-pro-for-windows/common" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config" + "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/contracts" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/database" log "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/grpc/logstreamer" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/tasks" @@ -19,7 +20,7 @@ type Config interface { SetUserSubscription(ctx context.Context, token string) error IsReadOnly() (bool, error) Subscription(context.Context) (string, config.Source, error) - FetchMicrosoftStoreSubscription(context.Context) error + FetchMicrosoftStoreSubscription(context.Context, ...contracts.Option) error } // Service it the UI GRPC service implementation. diff --git a/windows-agent/internal/proservices/ui/ui_test.go b/windows-agent/internal/proservices/ui/ui_test.go index 7f591006d..ca841151f 100644 --- a/windows-agent/internal/proservices/ui/ui_test.go +++ b/windows-agent/internal/proservices/ui/ui_test.go @@ -12,6 +12,7 @@ import ( "github.com/canonical/ubuntu-pro-for-windows/common/wsltestutils" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/config/registry" + "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/contracts" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/database" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/distros/distro" "github.com/canonical/ubuntu-pro-for-windows/windows-agent/internal/proservices/ui" @@ -244,7 +245,11 @@ func (m mockConfig) Subscription(context.Context) (string, config.Source, error) } return m.token, m.source, nil } -func (m *mockConfig) FetchMicrosoftStoreSubscription(ctx context.Context) error { +func (m *mockConfig) FetchMicrosoftStoreSubscription(ctx context.Context, args ...contracts.Option) error { + if len(args) != 0 { + panic("The variadic argument exists solely to match the interface. Do not use.") + } + readOnly, err := m.IsReadOnly() if err != nil { return err From 7f07bf2a2f9447b049d80c9256f3a19312bff830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Thu, 16 Nov 2023 14:23:17 +0100 Subject: [PATCH 6/7] fixup: Move testing of expiration dates to TestValidSubscription --- .../internal/contracts/contracts_test.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/windows-agent/internal/contracts/contracts_test.go b/windows-agent/internal/contracts/contracts_test.go index 919a5bcd8..c014857e6 100644 --- a/windows-agent/internal/contracts/contracts_test.go +++ b/windows-agent/internal/contracts/contracts_test.go @@ -85,6 +85,8 @@ func TestProToken(t *testing.T) { } func TestValidSubscription(t *testing.T) { + t.Parallel() + type subscriptionStatus int const ( subscribed subscriptionStatus = iota @@ -96,11 +98,12 @@ func TestValidSubscription(t *testing.T) { status subscriptionStatus expirationErr bool + want bool wantErr bool }{ - "Succcess when the current subscription is active": {status: subscribed}, - "Succcess when the current subscription is expired": {status: expired}, - "Success when there is no subscription": {status: unsubscribed}, + "Succcess when the current subscription is active": {status: subscribed, want: true}, + "Succcess when the current subscription is expired": {status: expired, want: false}, + "Success when there is no subscription": {status: unsubscribed, want: false}, "Error when subscription validity cannot be ascertained": {status: subscribed, expirationErr: true, wantErr: true}, } @@ -108,18 +111,16 @@ func TestValidSubscription(t *testing.T) { for name, tc := range testCases { tc := tc t.Run(name, func(t *testing.T) { + t.Parallel() + var store mockMSStore - var want bool switch tc.status { case subscribed: - want = true store.expirationDate = time.Now().Add(time.Hour * 24 * 365) // Next year case expired: - want = false store.expirationDate = time.Now().Add(-time.Hour * 24 * 365) // Last year case unsubscribed: - want = false store.notSubscribed = true } @@ -134,7 +135,7 @@ func TestValidSubscription(t *testing.T) { } require.NoError(t, err, "contracts.ValidSubscription should have returned no error") - require.Equal(t, want, got, "Unexpected return from ValidSubscription") + require.Equal(t, tc.want, got, "Unexpected return from ValidSubscription") }) } } From b81048959e8a00f0e42d3795102c70c63b014c86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edu=20G=C3=B3mez=20Escandell?= Date: Thu, 16 Nov 2023 14:30:37 +0100 Subject: [PATCH 7/7] fixup: Avoid redundant talk with the contracts server --- windows-agent/internal/contracts/contracts.go | 28 ++++++------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/windows-agent/internal/contracts/contracts.go b/windows-agent/internal/contracts/contracts.go index ff9e92119..afb2ebc98 100644 --- a/windows-agent/internal/contracts/contracts.go +++ b/windows-agent/internal/contracts/contracts.go @@ -63,17 +63,22 @@ func ValidSubscription(args ...Option) (bool, error) { } expiration, err := opts.microsoftStore.GetSubscriptionExpirationDate() - if isErrNotSubscribed(err) { - return false, nil - } if err != nil { + var target microsoftstore.StoreAPIError + if errors.As(err, &target) && target == microsoftstore.ErrNotSubscribed { + // ValidSubscription -> false: we are not subscribed + return false, nil + } + return false, fmt.Errorf("could not get subscription expiration date: %v", err) } if expiration.Before(time.Now()) { + // ValidSubscription -> false: the subscription is expired return false, nil } + // ValidSubscription -> true: the subscription is not yet expired return true, nil } @@ -117,20 +122,3 @@ func NewProToken(ctx context.Context, args ...Option) (token string, err error) return proToken, nil } - -func isErrNotSubscribed(err error) bool { - if err == nil { - return false - } - - var target microsoftstore.StoreAPIError - if !errors.As(err, &target) { - return false - } - - if target != microsoftstore.ErrNotSubscribed { - return false - } - - return true -}