From a7b9990be7ef4d1eadd4ea7f01fe95f86c0d2783 Mon Sep 17 00:00:00 2001 From: DevipriyaS17 Date: Mon, 2 Feb 2026 18:44:50 +0530 Subject: [PATCH 1/2] fix: handle password encryption errors in dtoToEntity functions --- internal/usecase/ciraconfigs/usecase.go | 23 +++++++++++++++----- internal/usecase/domains/usecase.go | 22 ++++++++++++++----- internal/usecase/profiles/usecase.go | 29 +++++++++++++++++++------ internal/usecase/wificonfigs/usecase.go | 27 ++++++++++++++++------- 4 files changed, 75 insertions(+), 26 deletions(-) diff --git a/internal/usecase/ciraconfigs/usecase.go b/internal/usecase/ciraconfigs/usecase.go index c58cc0cb1..6f05cc5c0 100644 --- a/internal/usecase/ciraconfigs/usecase.go +++ b/internal/usecase/ciraconfigs/usecase.go @@ -89,7 +89,10 @@ func (uc *UseCase) Delete(ctx context.Context, configName, tenantID string) erro } func (uc *UseCase) Update(ctx context.Context, d *dto.CIRAConfig) (*dto.CIRAConfig, error) { - d1 := uc.dtoToEntity(d) + d1, err := uc.dtoToEntity(d) + if err != nil { + return nil, err + } updated, err := uc.repo.Update(ctx, d1) if err != nil { @@ -111,9 +114,12 @@ func (uc *UseCase) Update(ctx context.Context, d *dto.CIRAConfig) (*dto.CIRAConf } func (uc *UseCase) Insert(ctx context.Context, d *dto.CIRAConfig) (*dto.CIRAConfig, error) { - d1 := uc.dtoToEntity(d) + d1, err := uc.dtoToEntity(d) + if err != nil { + return nil, err + } - _, err := uc.repo.Insert(ctx, d1) + _, err = uc.repo.Insert(ctx, d1) if err != nil { return nil, ErrDatabase.Wrap("Insert", "uc.repo.Insert", err) } @@ -129,7 +135,7 @@ func (uc *UseCase) Insert(ctx context.Context, d *dto.CIRAConfig) (*dto.CIRAConf } // convert dto.CIRAConfig to entity.CIRAConfig. -func (uc *UseCase) dtoToEntity(d *dto.CIRAConfig) *entity.CIRAConfig { +func (uc *UseCase) dtoToEntity(d *dto.CIRAConfig) (*entity.CIRAConfig, error) { d1 := &entity.CIRAConfig{ ConfigName: d.ConfigName, MPSAddress: d.MPSAddress, @@ -145,10 +151,15 @@ func (uc *UseCase) dtoToEntity(d *dto.CIRAConfig) *entity.CIRAConfig { GenerateRandomPassword: d.GenerateRandomPassword, Version: d.Version, } + + var err error // Encrypt password before storing - d1.Password, _ = uc.safeRequirements.Encrypt(d.Password) + d1.Password, err = uc.safeRequirements.Encrypt(d.Password) + if err != nil { + return nil, ErrCIRAConfigsUseCase.Wrap("dtoToEntity", "failed to encrypt password", err) + } - return d1 + return d1, nil } // convert entity.CIRAConfig to dto.CIRAConfig. diff --git a/internal/usecase/domains/usecase.go b/internal/usecase/domains/usecase.go index 8c06b50a9..5fba47756 100644 --- a/internal/usecase/domains/usecase.go +++ b/internal/usecase/domains/usecase.go @@ -172,7 +172,10 @@ func (uc *UseCase) Delete(ctx context.Context, domainName, tenantID string) erro } func (uc *UseCase) Update(ctx context.Context, d *dto.Domain) (*dto.Domain, error) { - d1 := uc.dtoToEntity(d) + d1, err := uc.dtoToEntity(d) + if err != nil { + return nil, err + } updated, err := uc.repo.Update(ctx, d1) if err != nil { @@ -199,7 +202,11 @@ func (uc *UseCase) Insert(ctx context.Context, d *dto.Domain) (*dto.Domain, erro return nil, err } - d1 := uc.dtoToEntity(d) + d1, err := uc.dtoToEntity(d) + if err != nil { + return nil, err + } + d1.ExpirationDate = cert.NotAfter.Format(time.RFC3339) // Store certificate in Vault (if available) - cert goes to Vault, not DB @@ -267,7 +274,7 @@ func DecryptAndCheckCertExpiration(domain dto.Domain) (*x509.Certificate, error) } // convert dto.Domain to entity.Domain. -func (uc *UseCase) dtoToEntity(d *dto.Domain) *entity.Domain { +func (uc *UseCase) dtoToEntity(d *dto.Domain) (*entity.Domain, error) { d1 := &entity.Domain{ ProfileName: d.ProfileName, DomainSuffix: d.DomainSuffix, @@ -278,9 +285,14 @@ func (uc *UseCase) dtoToEntity(d *dto.Domain) *entity.Domain { Version: d.Version, } - d1.ProvisioningCertPassword, _ = uc.safeRequirements.Encrypt(d.ProvisioningCertPassword) + var err error - return d1 + d1.ProvisioningCertPassword, err = uc.safeRequirements.Encrypt(d.ProvisioningCertPassword) + if err != nil { + return nil, ErrDomainsUseCase.Wrap("dtoToEntity", "failed to encrypt provisioning cert password", err) + } + + return d1, nil } // convert entity.Domain to dto.Domain. diff --git a/internal/usecase/profiles/usecase.go b/internal/usecase/profiles/usecase.go index 3f2adad4a..f8a3caab4 100644 --- a/internal/usecase/profiles/usecase.go +++ b/internal/usecase/profiles/usecase.go @@ -481,9 +481,12 @@ func (uc *UseCase) isWifiProfileExists(ctx context.Context, d *dto.Profile, acti } func (uc *UseCase) Update(ctx context.Context, d *dto.Profile) (*dto.Profile, error) { - d1 := uc.dtoToEntity(d) + d1, err := uc.dtoToEntity(d) + if err != nil { + return nil, err + } - err := uc.isWifiProfileExists(ctx, d, "update") + err = uc.isWifiProfileExists(ctx, d, "update") if err != nil { return nil, err } @@ -530,7 +533,10 @@ func (uc *UseCase) Update(ctx context.Context, d *dto.Profile) (*dto.Profile, er } func (uc *UseCase) Insert(ctx context.Context, d *dto.Profile) (*dto.Profile, error) { - d1 := uc.dtoToEntity(d) + d1, err := uc.dtoToEntity(d) + if err != nil { + return nil, err + } if err := uc.isWifiProfileExists(ctx, d, "insert"); err != nil { return nil, err @@ -612,7 +618,7 @@ func (uc *UseCase) createdProfile(ctx context.Context, d *dto.Profile) (*dto.Pro } // convert dto.Profile to entity.Profile. -func (uc *UseCase) dtoToEntity(d *dto.Profile) *entity.Profile { +func (uc *UseCase) dtoToEntity(d *dto.Profile) (*entity.Profile, error) { // convert []string to comma separated string tags := strings.Join(d.Tags, ", ") @@ -642,10 +648,19 @@ func (uc *UseCase) dtoToEntity(d *dto.Profile) *entity.Profile { UEFIWiFiSyncEnabled: d.UEFIWiFiSyncEnabled, } - d1.AMTPassword, _ = uc.safeRequirements.Encrypt(d.AMTPassword) - d1.MEBXPassword, _ = uc.safeRequirements.Encrypt(d.MEBXPassword) + var err error - return d1 + d1.AMTPassword, err = uc.safeRequirements.Encrypt(d.AMTPassword) + if err != nil { + return nil, ErrProfilesUseCase.Wrap("dtoToEntity", "failed to encrypt AMT password", err) + } + + d1.MEBXPassword, err = uc.safeRequirements.Encrypt(d.MEBXPassword) + if err != nil { + return nil, ErrProfilesUseCase.Wrap("dtoToEntity", "failed to encrypt MEBX password", err) + } + + return d1, nil } // convert entity.Profile to dto.Profile. diff --git a/internal/usecase/wificonfigs/usecase.go b/internal/usecase/wificonfigs/usecase.go index 63c0599dd..c82866d54 100644 --- a/internal/usecase/wificonfigs/usecase.go +++ b/internal/usecase/wificonfigs/usecase.go @@ -106,11 +106,14 @@ func (uc *UseCase) Delete(ctx context.Context, profileName, tenantID string) err } func (uc *UseCase) Update(ctx context.Context, d *dto.WirelessConfig) (*dto.WirelessConfig, error) { - d1 := uc.dtoToEntity(d) + d1, err := uc.dtoToEntity(d) + if err != nil { + return nil, err + } // check if the IEEE profile is exists in the database if d1.IEEE8021xProfileName != nil && *d1.IEEE8021xProfileName != "" { - _, err := uc.ieee.GetByName(ctx, *d1.IEEE8021xProfileName, d.TenantID) + _, err = uc.ieee.GetByName(ctx, *d1.IEEE8021xProfileName, d.TenantID) if err != nil { return nil, err } @@ -136,17 +139,20 @@ func (uc *UseCase) Update(ctx context.Context, d *dto.WirelessConfig) (*dto.Wire } func (uc *UseCase) Insert(ctx context.Context, d *dto.WirelessConfig) (*dto.WirelessConfig, error) { - d1 := uc.dtoToEntity(d) + d1, err := uc.dtoToEntity(d) + if err != nil { + return nil, err + } // check if the IEEE profile is exists in the database if d1.IEEE8021xProfileName != nil && *d1.IEEE8021xProfileName != "" { - _, err := uc.ieee.GetByName(ctx, *d1.IEEE8021xProfileName, d.TenantID) + _, err = uc.ieee.GetByName(ctx, *d1.IEEE8021xProfileName, d.TenantID) if err != nil { return nil, err } } - _, err := uc.repo.Insert(ctx, d1) + _, err = uc.repo.Insert(ctx, d1) if err != nil { return nil, ErrDatabase.Wrap("Insert", "uc.repo.Insert", err) } @@ -162,7 +168,7 @@ func (uc *UseCase) Insert(ctx context.Context, d *dto.WirelessConfig) (*dto.Wire } // convert dto.WirelessConfig to entity.WirelessConfig. -func (uc *UseCase) dtoToEntity(d *dto.WirelessConfig) *entity.WirelessConfig { +func (uc *UseCase) dtoToEntity(d *dto.WirelessConfig) (*entity.WirelessConfig, error) { // convert []int to comma separated string linkPolicy := strings.Trim(strings.Join(strings.Fields(fmt.Sprint(d.LinkPolicy)), ","), "[]") @@ -179,9 +185,14 @@ func (uc *UseCase) dtoToEntity(d *dto.WirelessConfig) *entity.WirelessConfig { Version: d.Version, } - d1.PSKPassphrase, _ = uc.safeRequirements.Encrypt(d.PSKPassphrase) + var err error - return d1 + d1.PSKPassphrase, err = uc.safeRequirements.Encrypt(d.PSKPassphrase) + if err != nil { + return nil, ErrDomainsUseCase.Wrap("dtoToEntity", "failed to encrypt PSK passphrase", err) + } + + return d1, nil } // convert entity.WirelessConfig to dto.WirelessConfig. From 4967b9a90ad18cd8a7950aab4da9578271980912 Mon Sep 17 00:00:00 2001 From: DevipriyaS17 Date: Tue, 3 Feb 2026 10:43:23 +0530 Subject: [PATCH 2/2] fix: handle errors in decrypt and fix UT --- internal/mocks/devicemanagement_mocks.go | 5 +-- internal/usecase/amtexplorer/wsman.go | 7 +++- internal/usecase/devices/interceptor.go | 12 +++++-- internal/usecase/devices/interceptor_test.go | 12 +++---- internal/usecase/devices/interfaces.go | 2 +- internal/usecase/devices/redirection.go | 11 +++++-- internal/usecase/devices/redirection_test.go | 34 ++++++-------------- internal/usecase/devices/wsman/message.go | 10 +++++- 8 files changed, 52 insertions(+), 41 deletions(-) diff --git a/internal/mocks/devicemanagement_mocks.go b/internal/mocks/devicemanagement_mocks.go index 79b05eecf..ee3694979 100644 --- a/internal/mocks/devicemanagement_mocks.go +++ b/internal/mocks/devicemanagement_mocks.go @@ -237,11 +237,12 @@ func (mr *MockRedirectionMockRecorder) RedirectSend(ctx, deviceConnection, messa } // SetupWsmanClient mocks base method. -func (m *MockRedirection) SetupWsmanClient(device entity.Device, isRedirection, logMessages bool) wsman0.Messages { +func (m *MockRedirection) SetupWsmanClient(device entity.Device, isRedirection, logMessages bool) (wsman0.Messages, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SetupWsmanClient", device, isRedirection, logMessages) ret0, _ := ret[0].(wsman0.Messages) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // SetupWsmanClient indicates an expected call of SetupWsmanClient. diff --git a/internal/usecase/amtexplorer/wsman.go b/internal/usecase/amtexplorer/wsman.go index fecb17ab0..460a56047 100644 --- a/internal/usecase/amtexplorer/wsman.go +++ b/internal/usecase/amtexplorer/wsman.go @@ -54,7 +54,12 @@ func (g GoWSMANMessages) SetupWsmanClient(device entity.Device, logAMTMessages b clientParams.PinnedCert = *device.CertHash } - clientParams.Password, _ = g.safeRequirements.Decrypt(device.Password) + decryptedPassword, err := g.safeRequirements.Decrypt(device.Password) + if err != nil { + return nil, err + } + + clientParams.Password = decryptedPassword connectionsMu.Lock() defer connectionsMu.Unlock() diff --git a/internal/usecase/devices/interceptor.go b/internal/usecase/devices/interceptor.go index 1fe06c9a0..272162f82 100644 --- a/internal/usecase/devices/interceptor.go +++ b/internal/usecase/devices/interceptor.go @@ -111,9 +111,17 @@ func (uc *UseCase) getOrCreateConnection(c context.Context, conn *websocket.Conn } func (uc *UseCase) createNewConnection(c context.Context, conn *websocket.Conn, key string, device *entity.Device) (*DeviceConnection, error) { - wsmanConnection := uc.redirection.SetupWsmanClient(*device, true, true) + wsmanConnection, err := uc.redirection.SetupWsmanClient(*device, true, true) + if err != nil { + return nil, err + } + + decryptedPassword, err := uc.safeRequirements.Decrypt(device.Password) + if err != nil { + return nil, err + } - device.Password, _ = uc.safeRequirements.Decrypt(device.Password) + device.Password = decryptedPassword ctx, cancel := context.WithCancel(c) now := time.Now() diff --git a/internal/usecase/devices/interceptor_test.go b/internal/usecase/devices/interceptor_test.go index e91f7f73d..d8f3825f3 100644 --- a/internal/usecase/devices/interceptor_test.go +++ b/internal/usecase/devices/interceptor_test.go @@ -66,7 +66,7 @@ func TestRedirect(t *testing.T) { Username: "user", Password: "pass", }, nil) - mockRedir.EXPECT().SetupWsmanClient(gomock.Any(), true, true).Return(wsman.Messages{}) + mockRedir.EXPECT().SetupWsmanClient(gomock.Any(), true, true).Return(wsman.Messages{}, nil) mockRedir.EXPECT().RedirectConnect(gomock.Any(), gomock.Any()).Return(ErrInterceptorGeneral) }, expectedErr: ErrInterceptorGeneral, @@ -138,7 +138,7 @@ func TestRedirectSuccessfulFlow(t *testing.T) { // Mock successful flow up to RedirectConnect, then fail to avoid goroutines mockRepo.EXPECT().GetByID(gomock.Any(), testGUID, "").Return(device, nil) - mockRedirection.EXPECT().SetupWsmanClient(*device, true, true).Return(wsman.Messages{}) + mockRedirection.EXPECT().SetupWsmanClient(*device, true, true).Return(wsman.Messages{}, nil) // Return error to avoid starting problematic goroutines but still test the flow mockRedirection.EXPECT().RedirectConnect(gomock.Any(), gomock.Any()).Return(ErrConnectionFailed) @@ -211,7 +211,7 @@ func TestRedirectConnectionReuse(t *testing.T) { // First call - create new connection but fail at connect to avoid goroutines mockRepo.EXPECT().GetByID(gomock.Any(), testGUID, "").Return(device, nil) - mockRedirection.EXPECT().SetupWsmanClient(*device, true, true).Return(wsman.Messages{}) + mockRedirection.EXPECT().SetupWsmanClient(*device, true, true).Return(wsman.Messages{}, nil) mockRedirection.EXPECT().RedirectConnect(gomock.Any(), gomock.Any()).Return(ErrFirstConnectionFailed) err := uc.Redirect(context.Background(), mockConn, testGUID, testMode) @@ -219,7 +219,7 @@ func TestRedirectConnectionReuse(t *testing.T) { // Second call - also fail to avoid goroutines but test reuse logic mockRepo.EXPECT().GetByID(gomock.Any(), testGUID, "").Return(device, nil) - mockRedirection.EXPECT().SetupWsmanClient(*device, true, true).Return(wsman.Messages{}) + mockRedirection.EXPECT().SetupWsmanClient(*device, true, true).Return(wsman.Messages{}, nil) mockRedirection.EXPECT().RedirectConnect(gomock.Any(), gomock.Any()).Return(ErrSecondConnectionFailed) err = uc.Redirect(context.Background(), mockConn, testGUID, testMode) @@ -305,7 +305,7 @@ func TestRedirectWithErrorScenarios(t *testing.T) { device := &entity.Device{GUID: testGUID, Username: "user", Password: "pass"} mockRepo.EXPECT().GetByID(gomock.Any(), testGUID, "").Return(device, nil) - mockRedir.EXPECT().SetupWsmanClient(*device, true, true).Return(wsman.Messages{}) + mockRedir.EXPECT().SetupWsmanClient(*device, true, true).Return(wsman.Messages{}, nil) mockRedir.EXPECT().RedirectConnect(gomock.Any(), gomock.Any()).Return(ErrConnectionFailed) }, expectedErr: "connection failed", @@ -383,7 +383,7 @@ func TestRedirectConnectionFlowCoverage(t *testing.T) { device := &entity.Device{GUID: "test-device", Username: "user", Password: "pass"} mockRepo.EXPECT().GetByID(gomock.Any(), "test-device", "").Return(device, nil) - mockRedir.EXPECT().SetupWsmanClient(*device, true, true).Return(wsman.Messages{}) + mockRedir.EXPECT().SetupWsmanClient(*device, true, true).Return(wsman.Messages{}, nil) // Return error to avoid starting goroutines, but still exercise connection creation mockRedir.EXPECT().RedirectConnect(gomock.Any(), gomock.Any()).Return(ErrTestError) }, diff --git a/internal/usecase/devices/interfaces.go b/internal/usecase/devices/interfaces.go index 207df2508..f99c07462 100644 --- a/internal/usecase/devices/interfaces.go +++ b/internal/usecase/devices/interfaces.go @@ -28,7 +28,7 @@ type ( } Redirection interface { - SetupWsmanClient(device entity.Device, isRedirection, logMessages bool) wsman.Messages + SetupWsmanClient(device entity.Device, isRedirection, logMessages bool) (wsman.Messages, error) RedirectConnect(ctx context.Context, deviceConnection *DeviceConnection) error RedirectClose(ctx context.Context, deviceConnection *DeviceConnection) error RedirectListen(ctx context.Context, deviceConnection *DeviceConnection) ([]byte, error) diff --git a/internal/usecase/devices/redirection.go b/internal/usecase/devices/redirection.go index 8bec141fa..ad5d28c68 100644 --- a/internal/usecase/devices/redirection.go +++ b/internal/usecase/devices/redirection.go @@ -14,7 +14,7 @@ type Redirector struct { SafeRequirements security.Cryptor } -func (g *Redirector) SetupWsmanClient(device entity.Device, isRedirection, logAMTMessages bool) wsman.Messages { +func (g *Redirector) SetupWsmanClient(device entity.Device, isRedirection, logAMTMessages bool) (wsman.Messages, error) { clientParams := client.Parameters{ Target: device.Hostname, Username: device.Username, @@ -29,9 +29,14 @@ func (g *Redirector) SetupWsmanClient(device entity.Device, isRedirection, logAM clientParams.PinnedCert = *device.CertHash } - clientParams.Password, _ = g.SafeRequirements.Decrypt(device.Password) + decryptedPassword, err := g.SafeRequirements.Decrypt(device.Password) + if err != nil { + return wsman.Messages{}, err + } + + clientParams.Password = decryptedPassword - return wsman.NewMessages(clientParams) + return wsman.NewMessages(clientParams), nil } func NewRedirector(safeRequirements security.Cryptor) *Redirector { diff --git a/internal/usecase/devices/redirection_test.go b/internal/usecase/devices/redirection_test.go index 47e5e7cd2..db28025c5 100644 --- a/internal/usecase/devices/redirection_test.go +++ b/internal/usecase/devices/redirection_test.go @@ -28,9 +28,9 @@ func initRedirectionTest(t *testing.T) (*devices.Redirector, *mocks.MockRedirect } type redTest struct { - name string - redMock func(*mocks.MockRedirection) - res any + name string + res any + err error } func TestSetupWsmanClient(t *testing.T) { @@ -44,21 +44,8 @@ func TestSetupWsmanClient(t *testing.T) { tests := []redTest{ { name: "success", - redMock: func(redirect *mocks.MockRedirection) { - redirect.EXPECT(). - SetupWsmanClient(gomock.Any(), false, true). - Return(wsman.Messages{}) - }, - res: wsman.Messages{}, - }, - { - name: "fail", - redMock: func(redirect *mocks.MockRedirection) { - redirect.EXPECT(). - SetupWsmanClient(gomock.Any(), true, true). - Return(wsman.Messages{}) - }, - res: wsman.Messages{}, + res: wsman.Messages{}, + err: nil, }, } @@ -67,17 +54,14 @@ func TestSetupWsmanClient(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - redirector, redirect, _ := initRedirectionTest(t) - - tc.redMock(redirect) + redirector, _, _ := initRedirectionTest(t) - redirector.SafeRequirements = security.Crypto{ - EncryptionKey: "test", - } + redirector.SafeRequirements = mocks.MockCrypto{} - res := redirector.SetupWsmanClient(*device, true, true) + res, err := redirector.SetupWsmanClient(*device, true, true) require.IsType(t, tc.res, res) + require.Equal(t, tc.err, err) }) } } diff --git a/internal/usecase/devices/wsman/message.go b/internal/usecase/devices/wsman/message.go index 26359e241..67847bfb7 100644 --- a/internal/usecase/devices/wsman/message.go +++ b/internal/usecase/devices/wsman/message.go @@ -131,7 +131,15 @@ func (g GoWSMANMessages) SetupWsmanClient(device entity.Device, isRedirection, l errChan := make(chan error, 1) // Queue the request requestQueue <- func() { - device.Password, _ = g.safeRequirements.Decrypt(device.Password) + decryptedPassword, err := g.safeRequirements.Decrypt(device.Password) + if err != nil { + errChan <- err + + return + } + + device.Password = decryptedPassword + if device.MPSUsername != "" { if len(Connections) == 0 { errChan <- ErrCIRADeviceNotConnected