From 6e5671a8b16f97a0d452c0ba5df31eb276435fed Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Tue, 4 Aug 2026 07:30:28 +0200 Subject: [PATCH 01/13] refactor(deviceauth): split out logic for aggregating device status Signed-off-by: Alf-Rune Siqveland --- .../services/deviceauth/devauth/devauth.go | 31 ++++++++++++------- .../deviceauth/devauth/devauth_test.go | 2 +- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth.go b/backend/services/deviceauth/devauth/devauth.go index 32d93394c..4bd52b0cc 100644 --- a/backend/services/deviceauth/devauth/devauth.go +++ b/backend/services/deviceauth/devauth/devauth.go @@ -454,26 +454,35 @@ func (d *DevAuth) processPreAuthRequest( return d.handlePreAuthDevice(ctx, aset) } +func (d *DevAuth) aggregateDeviceStatus(ctx context.Context, deviceID string) (string, error) { + status, err := d.db.GetDeviceStatus(ctx, deviceID) + if err != nil { + if errors.Is(err, store.ErrAuthSetNotFound) { + status = model.DevStatusNoAuth + } else { + return "", errors.Wrap(err, "cannot determine device status") + } + } + return status, nil + +} + func (d *DevAuth) updateDeviceStatus( ctx context.Context, devId, status string, currentStatus string, ) error { - newStatus, err := d.db.GetDeviceStatus(ctx, devId) - if err == nil && currentStatus == newStatus { - return nil - } + var err error if status == "" { - switch err { - case nil: - status = newStatus - case store.ErrAuthSetNotFound: - status = model.DevStatusNoAuth - default: - return errors.Wrap(err, "Cannot determine device status") + status, err = d.aggregateDeviceStatus(ctx, devId) + if err != nil { + return err } } + if currentStatus == status { + return nil + } // submit device status change job dev, err := d.db.GetDeviceById(ctx, devId) diff --git a/backend/services/deviceauth/devauth/devauth_test.go b/backend/services/deviceauth/devauth/devauth_test.go index 05c3cf26a..82751466b 100644 --- a/backend/services/deviceauth/devauth/devauth_test.go +++ b/backend/services/deviceauth/devauth/devauth_test.go @@ -2997,7 +2997,7 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { DeviceId: oid.NewUUIDv5("devId10").String(), }, dbGetDeviceStatusErr: errors.New("Get Device Status Error"), - outErr: "Cannot determine device status: Get Device Status Error", + outErr: "cannot determine device status: Get Device Status Error", }, { devId: oid.NewUUIDv5("devId11").String(), From 123acdede9c2629caef8eccbac382bca416b4f03 Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Tue, 4 Aug 2026 07:57:55 +0200 Subject: [PATCH 02/13] test(deviceauth): adjusted unit tests with wrong expectations Test was passing due to flawed error handling. Signed-off-by: Alf-Rune Siqveland --- .../deviceauth/devauth/devauth_test.go | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth_test.go b/backend/services/deviceauth/devauth/devauth_test.go index 82751466b..113faa8b1 100644 --- a/backend/services/deviceauth/devauth/devauth_test.go +++ b/backend/services/deviceauth/devauth/devauth_test.go @@ -653,12 +653,13 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { dbGetDevCountByStatusRes: 0, dev: &model.Device{ Id: dummyDevId, - Status: model.DevStatusPending, + Status: model.DevStatusPreauth, }, res: dummyToken, expectedWorkflows: map[string]error{ "provision_device": nil, "update_device_inventory": nil, + "update_device_status": nil, }, }, { @@ -676,7 +677,7 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { dbGetDevCountByStatusRes: 0, dev: &model.Device{ Id: dummyDevId, - Status: model.DevStatusPending, + Status: model.DevStatusPreauth, Decommissioning: true, }, err: ErrDevAuthUnauthorized, @@ -686,7 +687,7 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { dbGetAuthSetByDataKeyErr: errors.New("db error"), dev: &model.Device{ Id: dummyDevId, - Status: model.DevStatusPending, + Status: model.DevStatusPreauth, }, err: errors.New("failed to fetch auth set: db error"), }, @@ -705,7 +706,7 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { dbGetDevCountByStatusRes: 5, dev: &model.Device{ Id: dummyDevId, - Status: model.DevStatusPending, + Status: model.DevStatusPreauth, }, err: ErrMaxDeviceCountReached, }, @@ -721,7 +722,7 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { dbGetLimitErr: errors.New("db error"), dev: &model.Device{ Id: dummyDevId, - Status: model.DevStatusPending, + Status: model.DevStatusPreauth, }, err: errors.New("can't get current device limit: db error"), }, @@ -740,10 +741,11 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { dbGetDevCountByStatusRes: 0, dev: &model.Device{ Id: dummyDevId, - Status: model.DevStatusPending, + Status: model.DevStatusPreauth, }, expectedWorkflows: map[string]error{ - "provision_device": errors.New("workflows failed"), + "provision_device": errors.New("workflows failed"), + "update_device_status": nil, }, err: errors.New("submit device provisioning job error: workflows failed"), }, @@ -767,7 +769,6 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { //coSubmitProvisionDeviceJobErr: errors.New("workflows shouldn't be called"), // MEN-6961: we accept preauth at all times res: "dummytoken", expectedWorkflows: map[string]error{ - "update_device_status": nil, "provision_device": nil, "update_device_inventory": nil, }, @@ -883,12 +884,14 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { for name, err := range tc.expectedWorkflows { co.EXPECT(). StartWorkflow(mtesting.ContextMatcher(), name). + Run(func(context.Context, string) { + co.EXPECT(). + StartWorkflowExecute(mock.Anything). + Return(nil, mockResponseOK, err). + Once() + }). Return(req). Once() - co.EXPECT(). - StartWorkflowExecute(mock.Anything). - Return(nil, mockResponseOK, err). - Once() } // setup devauth From 0c0575d09c44b7b0b247c4a31b7f99d2b5768369 Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Tue, 4 Aug 2026 08:23:06 +0200 Subject: [PATCH 03/13] refactor(deviceauth): push updateDeviceStatus.GetDeviceById to caller Most caller contexts already fetched the device. Changed signature to accept device instead of deviceID and "currentStatus". Signed-off-by: Alf-Rune Siqveland --- .../services/deviceauth/devauth/devauth.go | 82 +++++++++---------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth.go b/backend/services/deviceauth/devauth/devauth.go index 4bd52b0cc..35c6f9702 100644 --- a/backend/services/deviceauth/devauth/devauth.go +++ b/backend/services/deviceauth/devauth/devauth.go @@ -17,6 +17,7 @@ import ( "context" "crypto/sha256" "encoding/json" + "fmt" "strings" "time" @@ -359,7 +360,6 @@ func (d *DevAuth) handlePreAuthDevice( return nil, ErrDevAuthUnauthorized } - currentStatus := dev.Status if dev.Status != model.DevStatusAccepted { // auth set is ok for auto-accepting, check device limit allow, err := d.canAcceptDevice(ctx) @@ -387,9 +387,8 @@ func (d *DevAuth) handlePreAuthDevice( if err := d.updateDeviceStatus( ctx, - aset.DeviceId, + dev, model.DevStatusAccepted, - currentStatus, ); err != nil { return nil, err } @@ -469,39 +468,24 @@ func (d *DevAuth) aggregateDeviceStatus(ctx context.Context, deviceID string) (s func (d *DevAuth) updateDeviceStatus( ctx context.Context, - devId, + device *model.Device, status string, - currentStatus string, ) error { - var err error - if status == "" { - status, err = d.aggregateDeviceStatus(ctx, devId) - if err != nil { - return err - } - } - if currentStatus == status { - return nil - } - - // submit device status change job - dev, err := d.db.GetDeviceById(ctx, devId) - if err != nil { - return errors.Wrap(err, "db get device by id error") + if device.Status == status { + return nil // No-op } - tenantId := "" idData := identity.FromContext(ctx) if idData != nil { tenantId = idData.Tenant } //nolint:bodyclose - _, _, err = d.cOrch.StartWorkflow(ctx, "update_device_status"). + _, _, err := d.cOrch.StartWorkflow(ctx, "update_device_status"). RequestBody(map[string]interface{}{ "request_id": requestid.FromContext(ctx), "devices": []model.DeviceInventoryUpdate{{ - Id: dev.Id, - Revision: dev.Revision + 1, + Id: device.Id, + Revision: device.Revision + 1, }}, "tenant_id": tenantId, "device_status": status, @@ -512,7 +496,7 @@ func (d *DevAuth) updateDeviceStatus( } if err := d.db.UpdateDevice(ctx, - devId, + device.Id, model.DeviceUpdate{ Status: status, UpdatedTs: uto.TimePtr(time.Now().UTC()), @@ -560,8 +544,13 @@ func (d *DevAuth) processAuthRequest( return nil, err } + status, err := d.aggregateDeviceStatus(ctx, dev.Id) + if err != nil { + return nil, err + } + // update the device status - if err := d.updateDeviceStatus(ctx, dev.Id, "", dev.Status); err != nil { + if err := d.updateDeviceStatus(ctx, dev, status); err != nil { return nil, err } @@ -757,8 +746,13 @@ func (d *DevAuth) DeleteAuthSet(ctx context.Context, devID string, authId string return err } + newStatus, err := d.aggregateDeviceStatus(ctx, devID) + if err != nil { + return err + } + // If the auth set status is 'preauthorized' and this is the only auth set - // for this device, the device is deleted from deviceauth. + // for this device (newStatus: noauth), the device is deleted from deviceauth. // We cannot start the decommission_device workflow because // we don't provision devices until they are accepted. Still, we need to // remove the device from the inventory service because we index pre-authorized @@ -766,21 +760,22 @@ func (d *DevAuth) DeleteAuthSet(ctx context.Context, devID string, authId string // from the inventory service, we start the status update workflow with the // special value "decommissioned", which will cause the deletion of the // device from the inventory service's database. - if authSet.Status == model.DevStatusPreauth { - authSets, err := d.db.GetAuthSetsForDevice(ctx, authSet.DeviceId) + if authSet.Status == model.DevStatusPreauth && newStatus == model.DevStatusNoAuth { + err = d.deletePreauthDevice(ctx, authSet.DeviceId) if err != nil { - return errors.Wrap(err, "db get auth sets error") + return errors.Wrap(err, "failed to delete preauthorized device") } - if len(authSets) == 0 { - err = d.deletePreauthDevice(ctx, authSet.DeviceId) - if err != nil { - return errors.Wrap(err, "failed to delete preauthorized device") - } + return nil + } + dev, err := d.db.GetDeviceById(ctx, devID) + if err != nil { + if errors.Is(err, store.ErrDevNotFound) { return nil } + return fmt.Errorf("failed to update device status: %w", err) } - return d.updateDeviceStatus(ctx, devID, "", authSet.Status) + return d.updateDeviceStatus(ctx, dev, newStatus) } func (d *DevAuth) deletePreauthDevice(ctx context.Context, devId string) error { @@ -936,8 +931,6 @@ func (d *DevAuth) setAuthSetStatus( return nil } - currentStatus := aset.Status - if aset.Status == model.DevStatusAccepted && (status == model.DevStatusRejected || status == model.DevStatusPending) { deviceOID := oid.FromString(aset.DeviceId) @@ -963,10 +956,17 @@ func (d *DevAuth) setAuthSetStatus( return errors.Wrap(err, "db update device auth set error") } - if status == model.DevStatusAccepted { - return d.updateDeviceStatus(ctx, deviceID, status, currentStatus) + device, err := d.db.GetDeviceById(ctx, deviceID) + if err != nil { + return err + } + if status != model.DevStatusAccepted { + status, err = d.aggregateDeviceStatus(ctx, deviceID) + if err != nil { + return err + } } - return d.updateDeviceStatus(ctx, deviceID, "", currentStatus) + return d.updateDeviceStatus(ctx, device, status) } func (d *DevAuth) RejectDeviceAuth(ctx context.Context, device_id string, auth_id string) error { From 22f31213d012cea5af70db3d0f6a98b88474e670 Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Tue, 4 Aug 2026 08:24:56 +0200 Subject: [PATCH 04/13] test(deviceauth): update unit tests relying on loose assertions Refactoring made more use of the mocked data which caused several tests to fail due to lack of data in the return arguments. Signed-off-by: Alf-Rune Siqveland --- .../deviceauth/devauth/devauth_test.go | 72 ++++++++----------- 1 file changed, 29 insertions(+), 43 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth_test.go b/backend/services/deviceauth/devauth/devauth_test.go index 113faa8b1..4a6997bba 100644 --- a/backend/services/deviceauth/devauth/devauth_test.go +++ b/backend/services/deviceauth/devauth/devauth_test.go @@ -1489,6 +1489,9 @@ func TestDevAuthRejectDevice(t *testing.T) { if tc.aset != nil { db.On("UpdateAuthSetById", ctx, tc.aset.Id, model.AuthSetUpdate{Status: model.DevStatusRejected}).Return(nil) + db.On("GetDeviceById", ctx, + mock.AnythingOfType("string")). + Return(&model.Device{Id: tc.aset.Id, Status: tc.aset.Status}, nil) } db.On("DeleteTokenByDevId", ctx, dummyDevUUID). @@ -1499,8 +1502,6 @@ func TestDevAuthRejectDevice(t *testing.T) { db.On("UpdateDevice", ctx, dummyDevID, mock.AnythingOfType("model.DeviceUpdate")).Return(nil) - db.On("GetDeviceById", ctx, - mock.AnythingOfType("string")).Return(&model.Device{}, nil) co := oas_mocks.NewMockWorkflowsOtherAPI(t) @@ -1716,6 +1717,9 @@ func TestDevAuthResetDevice(t *testing.T) { if tc.aset != nil { db.On("UpdateAuthSetById", context.Background(), tc.aset.Id, model.AuthSetUpdate{Status: model.DevStatusPending}).Return(nil) + db.On("GetDeviceById", context.Background(), + mock.AnythingOfType("string")). + Return(&model.Device{Id: tc.aset.DeviceId, Status: tc.aset.Status}, nil) } db.On("DeleteTokenByDevId", context.Background(), dummyDevUUID).Return( @@ -1731,8 +1735,6 @@ func TestDevAuthResetDevice(t *testing.T) { return mock.AnythingOfType("string") }(), mock.AnythingOfType("model.DeviceUpdate")).Return(nil) - db.On("GetDeviceById", context.Background(), - mock.AnythingOfType("string")).Return(&model.Device{}, nil) co := oas_mocks.NewMockWorkflowsOtherAPI(t) if tc.submitJob { @@ -2888,8 +2890,6 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { dbGetDeviceStatus string dbGetDeviceStatusErr error dbUpdateDeviceErr error - dbGetAuthSetsForDevice []model.AuthSet - dbGetAuthSetsForDeviceErr error submitJob bool orchestratorErr error @@ -2920,6 +2920,7 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { }, dbDeleteTokenByDevIdErr: errors.New("DeleteTokenByDevId Error"), outErr: "db delete device tokens error: DeleteTokenByDevId Error", + dbGetDeviceStatus: model.DevStatusAccepted, }, { devId: oid.NewUUIDv5("devId4").String(), @@ -2931,6 +2932,7 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { }, submitJob: true, dbDeleteTokenByDevIdErr: errors.New("DeleteTokenByDevId Error"), + dbGetDeviceStatus: model.DevStatusPending, }, { devId: oid.NewUUIDv5("devId5").String(), @@ -2942,6 +2944,7 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { }, submitJob: true, dbDeleteTokenByDevIdErr: store.ErrTokenNotFound, + dbGetDeviceStatus: model.DevStatusAccepted, }, { devId: oid.NewUUIDv5("devId6").String(), @@ -2962,7 +2965,7 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { Status: model.DevStatusPreauth, }, submitJob: true, - dbGetDeviceStatus: "decommissioned", + dbGetDeviceStatus: model.DevStatusNoAuth, dbDeleteDeviceErr: errors.New("DeleteDevice Error"), outErr: "failed to delete preauthorized device: DeleteDevice Error", }, @@ -2975,7 +2978,7 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { Status: model.DevStatusPreauth, }, submitJob: true, - dbGetDeviceStatus: "decommissioned", + dbGetDeviceStatus: model.DevStatusNoAuth, orchestratorErr: errors.New("orchestrator error"), outErr: "failed to delete preauthorized device: failed to start update device status job: orchestrator error", }, @@ -2988,7 +2991,7 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { Status: model.DevStatusPreauth, }, submitJob: true, - dbGetDeviceStatus: "decommissioned", + dbGetDeviceStatus: model.DevStatusNoAuth, dbDeleteDeviceErr: errors.New("DeleteDevice Error"), outErr: "failed to delete preauthorized device: DeleteDevice Error", }, @@ -3011,6 +3014,7 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { Status: model.DevStatusPending, }, submitJob: true, + dbGetDeviceStatus: model.DevStatusPending, dbUpdateDeviceErr: errors.New("Update Device Error"), outErr: "failed to update device status: Update Device Error", }, @@ -3078,25 +3082,8 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { DeviceId: oid.NewUUIDv5("devId16").String(), Status: model.DevStatusPreauth, }, - dbGetDeviceStatus: "decommissioned", - dbGetAuthSetsForDeviceErr: errors.New("GetAuthSetsForDevice Error"), - outErr: "db get auth sets error: GetAuthSetsForDevice Error", - }, - { - devId: oid.NewUUIDv5("devId16").String(), - authId: oid.NewUUIDv5("authId16").String(), - authSet: &model.AuthSet{ - Id: oid.NewUUIDv5("authId16").String(), - DeviceId: oid.NewUUIDv5("devId16").String(), - Status: model.DevStatusPreauth, - }, - dbGetDeviceStatus: "pending", - dbGetAuthSetsForDevice: []model.AuthSet{ - { - Id: "foo", - }, - }, - submitJob: true, + dbGetDeviceStatus: model.DevStatusNoAuth, + submitJob: true, }, } @@ -3141,24 +3128,23 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { tc.devId, mock.AnythingOfType("model.DeviceUpdate")).Return(tc.dbUpdateDeviceErr) db.On("GetDeviceById", ctx, - mock.AnythingOfType("string")).Return(&model.Device{Id: tc.devId}, nil) - db.On("GetAuthSetsForDevice", - ctx, - tc.devId, - ).Return( - tc.dbGetAuthSetsForDevice, - tc.dbGetAuthSetsForDeviceErr) - + mock.AnythingOfType("string")).Return(&model.Device{ + Id: tc.devId, + Provisioned: true, + }, nil) co := oas_mocks.NewMockWorkflowsOtherAPI(t) status := tc.dbGetDeviceStatus var revision uint = 1 - if tc.dbGetDeviceStatusErr == store.ErrAuthSetNotFound { - status = "noauth" - } - if tc.dbGetDeviceStatus == "decommissioned" { + if tc.dbGetDeviceStatus == model.DevStatusNoAuth { revision = 0 } if tc.submitJob { + if status == model.DevStatusNoAuth { + status = "decommissioned" + } + if tc.dbGetDeviceStatusErr == store.ErrAuthSetNotFound { + status = "noauth" + } req := client.ApiStartWorkflowRequest{ ApiService: co, }.RequestBody(map[string]interface{}{ @@ -3209,10 +3195,10 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { assert.EqualError(t, err, tc.outErr) } else { assert.NoError(t, err) - if authSet.Status == model.DevStatusPreauth && len(tc.dbGetAuthSetsForDevice) == 0 { - db.AssertCalled(t, "DeleteDevice", tc.devId) + if authSet.Status == model.DevStatusPreauth && tc.dbGetDeviceStatus == model.DevStatusNoAuth { + db.AssertCalled(t, "DeleteDevice", ctx, tc.devId) } else { - db.AssertNotCalled(t, "DeleteDevice", tc.devId) + db.AssertNotCalled(t, "DeleteDevice", mock.Anything, mock.Anything) } } c.AssertExpectations(t) From ee8e6f4f2d1aeb6d5524e0b2562a6ac12b43906d Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Tue, 4 Aug 2026 08:59:30 +0200 Subject: [PATCH 05/13] refactor(deviceauth): centralize handlers for setting authset status Signed-off-by: Alf-Rune Siqveland --- .../services/deviceauth/devauth/devauth.go | 147 ++++++++---------- 1 file changed, 65 insertions(+), 82 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth.go b/backend/services/deviceauth/devauth/devauth.go index 35c6f9702..216a6949a 100644 --- a/backend/services/deviceauth/devauth/devauth.go +++ b/backend/services/deviceauth/devauth/devauth.go @@ -834,45 +834,19 @@ func (d *DevAuth) deleteAuthSet(ctx context.Context, authSet *model.AuthSet) err } func (d *DevAuth) AcceptDeviceAuth(ctx context.Context, device_id string, auth_id string) error { - l := log.FromContext(ctx) - - aset, err := d.db.GetAuthSetById(ctx, auth_id) - if err != nil { - if err == store.ErrAuthSetNotFound { - return err - } - return errors.Wrap(err, "db get auth set error") - } - - // device authentication set already accepted, nothing to do here - if aset.Status == model.DevStatusAccepted { - l.Debugf("Device %s already accepted", device_id) - return nil - } else if aset.Status != model.DevStatusRejected && aset.Status != model.DevStatusPending { - // device authentication set can be accepted only from 'pending' or 'rejected' statuses - return ErrDevAuthBadRequest - } - - // check the device status - // if the device status is accepted then do not trigger provisioning workflow - // this needs to be checked before changing authentication set status - dev, err := d.db.GetDeviceById(ctx, device_id) - if err != nil { - return err - } - - // possible race, consider accept-count-unaccept pattern if that's problematic - allow, err := d.canAcceptDevice(ctx) - if err != nil { + var ( + dev *model.Device + err error + ) + if dev, _, err = d.setAuthSetStatus( + ctx, device_id, auth_id, model.DevStatusAccepted, + ); err != nil { return err } - if !allow { - return ErrMaxDeviceCountReached - } - - if err := d.setAuthSetStatus(ctx, device_id, auth_id, model.DevStatusAccepted); err != nil { - return err + if dev == nil { + // Status did not change + return nil } if dev.Provisioned { @@ -881,10 +855,6 @@ func (d *DevAuth) AcceptDeviceAuth(ctx context.Context, device_id string, auth_i return nil } - dev.Status = model.DevStatusAccepted - aset.Status = model.DevStatusAccepted - dev.AuthSets = []model.AuthSet{*aset} - reqId := requestid.FromContext(ctx) var tenantID string @@ -897,7 +867,7 @@ func (d *DevAuth) AcceptDeviceAuth(ctx context.Context, device_id string, auth_i _, _, err = d.cOrch.StartWorkflow(ctx, "provision_device"). RequestBody(map[string]interface{}{ "request_id": reqId, - "device_id": aset.DeviceId, + "device_id": dev.Id, "tenant_id": tenantID, "device": dev, "status": dev.Status, @@ -914,30 +884,61 @@ func (d *DevAuth) setAuthSetStatus( deviceID string, authID string, status string, -) error { +) (*model.Device, *model.AuthSet, error) { aset, err := d.db.GetAuthSetById(ctx, authID) if err != nil { if err == store.ErrAuthSetNotFound { - return err + return nil, nil, err } - return errors.Wrap(err, "db get auth set error") + return nil, nil, errors.Wrap(err, "db get auth set error") } if aset.DeviceId != deviceID { - return ErrDevIdAuthIdMismatch + return nil, nil, ErrDevIdAuthIdMismatch } if aset.Status == status { - return nil + return nil, aset, nil } - if aset.Status == model.DevStatusAccepted && - (status == model.DevStatusRejected || status == model.DevStatusPending) { + // Validate status transition + switch status { + case model.DevStatusAccepted: + if aset.Status != model.DevStatusRejected && + aset.Status != model.DevStatusPending { + return nil, nil, ErrDevAuthBadRequest + } + allow, err := d.canAcceptDevice(ctx) + if err != nil { + return nil, nil, err + } + if !allow { + return nil, nil, ErrMaxDeviceCountReached + } + case model.DevStatusPending: + if aset.Status == model.DevStatusPreauth { + return nil, nil, ErrDevAuthBadRequest + } + case model.DevStatusRejected: + if aset.Status != model.DevStatusPending && + aset.Status != model.DevStatusAccepted { + return nil, nil, ErrDevAuthBadRequest + } + default: + return nil, nil, ErrDevAuthBadRequest + } + + if aset.Status == model.DevStatusAccepted { + err = d.cacheDeleteToken(ctx, deviceID) + if err != nil { + return nil, nil, errors.Wrapf(err, + "failed to delete token for %s from cache", deviceID) + } deviceOID := oid.FromString(aset.DeviceId) // delete device token err := d.db.DeleteTokenByDevId(ctx, deviceOID) if err != nil && err != store.ErrTokenNotFound { - return errors.Wrap(err, "db delete device token error") + return nil, nil, errors.Wrap(err, "db delete device token error") } } @@ -946,61 +947,43 @@ func (d *DevAuth) setAuthSetStatus( // reject all accepted auth sets for this device first err := d.db.RejectAuthSetsForDevice(ctx, deviceID, aset.Id) if err != nil && err != store.ErrAuthSetNotFound { - return errors.Wrap(err, "failed to reject auth sets") + return nil, nil, errors.Wrap(err, "failed to reject auth sets") } } if err := d.db.UpdateAuthSetById(ctx, aset.Id, model.AuthSetUpdate{ Status: status, }); err != nil { - return errors.Wrap(err, "db update device auth set error") + return nil, nil, errors.Wrap(err, "db update device auth set error") } device, err := d.db.GetDeviceById(ctx, deviceID) if err != nil { - return err + return nil, nil, err } if status != model.DevStatusAccepted { status, err = d.aggregateDeviceStatus(ctx, deviceID) if err != nil { - return err - } - } - return d.updateDeviceStatus(ctx, device, status) -} - -func (d *DevAuth) RejectDeviceAuth(ctx context.Context, device_id string, auth_id string) error { - aset, err := d.db.GetAuthSetById(ctx, auth_id) - if err != nil { - if err == store.ErrAuthSetNotFound { - return err + return nil, nil, err } - return errors.Wrap(err, "db get auth set error") - } else if aset.Status != model.DevStatusPending && aset.Status != model.DevStatusAccepted { - // device authentication set can be rejected only from 'accepted' or 'pending' statuses - return ErrDevAuthBadRequest } - - err = d.cacheDeleteToken(ctx, device_id) + err = d.updateDeviceStatus(ctx, device, status) if err != nil { - return errors.Wrapf(err, "failed to delete token for %s from cache", device_id) + return nil, nil, err } + device.Status = status + aset.Status = status + return device, aset, nil +} - return d.setAuthSetStatus(ctx, device_id, auth_id, model.DevStatusRejected) +func (d *DevAuth) RejectDeviceAuth(ctx context.Context, device_id string, auth_id string) error { + _, _, err := d.setAuthSetStatus(ctx, device_id, auth_id, model.DevStatusRejected) + return err } func (d *DevAuth) ResetDeviceAuth(ctx context.Context, device_id string, auth_id string) error { - aset, err := d.db.GetAuthSetById(ctx, auth_id) - if err != nil { - if err == store.ErrAuthSetNotFound { - return err - } - return errors.Wrap(err, "db get auth set error") - } else if aset.Status == model.DevStatusPreauth { - // preauthorized auth set should not go into pending state - return ErrDevAuthBadRequest - } - return d.setAuthSetStatus(ctx, device_id, auth_id, model.DevStatusPending) + _, _, err := d.setAuthSetStatus(ctx, device_id, auth_id, model.DevStatusPending) + return err } func parseIdData(idData string) (map[string]interface{}, []byte, error) { From 4860701eebb21ecd5c909a7bb9d9092e9f8ef861 Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Tue, 4 Aug 2026 09:17:32 +0200 Subject: [PATCH 06/13] refactor(deviceauth): split out updateAuthSetStatus from setAuthSetStatus Lower cyclomatic complexity and improve readability. Signed-off-by: Alf-Rune Siqveland --- .../services/deviceauth/devauth/devauth.go | 88 +++++++++---------- backend/services/deviceauth/model/device.go | 21 +++++ 2 files changed, 63 insertions(+), 46 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth.go b/backend/services/deviceauth/devauth/devauth.go index 216a6949a..ab8f50bcd 100644 --- a/backend/services/deviceauth/devauth/devauth.go +++ b/backend/services/deviceauth/devauth/devauth.go @@ -879,82 +879,80 @@ func (d *DevAuth) AcceptDeviceAuth(ctx context.Context, device_id string, auth_i return nil } -func (d *DevAuth) setAuthSetStatus( - ctx context.Context, - deviceID string, - authID string, - status string, -) (*model.Device, *model.AuthSet, error) { +func (d *DevAuth) updateAuthSetStatus( + ctx context.Context, deviceID, authID, status string, +) (*model.AuthSet, error) { aset, err := d.db.GetAuthSetById(ctx, authID) if err != nil { if err == store.ErrAuthSetNotFound { - return nil, nil, err + return nil, err } - return nil, nil, errors.Wrap(err, "db get auth set error") + return nil, errors.Wrap(err, "db get auth set error") } if aset.DeviceId != deviceID { - return nil, nil, ErrDevIdAuthIdMismatch + return nil, ErrDevIdAuthIdMismatch } if aset.Status == status { - return nil, aset, nil + return aset, nil } // Validate status transition - switch status { - case model.DevStatusAccepted: - if aset.Status != model.DevStatusRejected && - aset.Status != model.DevStatusPending { - return nil, nil, ErrDevAuthBadRequest - } + err = model.ValidateStatusTransition(aset.Status, status) + if err != nil { + return nil, ErrDevAuthBadRequest + } + + if status == model.DevStatusAccepted { + // if accepting an auth set allow, err := d.canAcceptDevice(ctx) if err != nil { - return nil, nil, err + return nil, err } if !allow { - return nil, nil, ErrMaxDeviceCountReached - } - case model.DevStatusPending: - if aset.Status == model.DevStatusPreauth { - return nil, nil, ErrDevAuthBadRequest + return nil, ErrMaxDeviceCountReached } - case model.DevStatusRejected: - if aset.Status != model.DevStatusPending && - aset.Status != model.DevStatusAccepted { - return nil, nil, ErrDevAuthBadRequest + // reject all accepted auth sets for this device first + err = d.db.RejectAuthSetsForDevice(ctx, deviceID, aset.Id) + if err != nil && err != store.ErrAuthSetNotFound { + return nil, errors.Wrap(err, "failed to reject auth sets") } - default: - return nil, nil, ErrDevAuthBadRequest - } - - if aset.Status == model.DevStatusAccepted { + } else if aset.Status == model.DevStatusAccepted { + // Authset transitions from accepted err = d.cacheDeleteToken(ctx, deviceID) if err != nil { - return nil, nil, errors.Wrapf(err, + return nil, errors.Wrapf(err, "failed to delete token for %s from cache", deviceID) } deviceOID := oid.FromString(aset.DeviceId) // delete device token - err := d.db.DeleteTokenByDevId(ctx, deviceOID) + err = d.db.DeleteTokenByDevId(ctx, deviceOID) if err != nil && err != store.ErrTokenNotFound { - return nil, nil, errors.Wrap(err, "db delete device token error") - } - } - - // if accepting an auth set - if status == model.DevStatusAccepted { - // reject all accepted auth sets for this device first - err := d.db.RejectAuthSetsForDevice(ctx, deviceID, aset.Id) - if err != nil && err != store.ErrAuthSetNotFound { - return nil, nil, errors.Wrap(err, "failed to reject auth sets") + return nil, errors.Wrap(err, "db delete device token error") } } if err := d.db.UpdateAuthSetById(ctx, aset.Id, model.AuthSetUpdate{ Status: status, }); err != nil { - return nil, nil, errors.Wrap(err, "db update device auth set error") + return nil, errors.Wrap(err, "db update device auth set error") + } + return aset, nil +} + +func (d *DevAuth) setAuthSetStatus( + ctx context.Context, + deviceID string, + authID string, + status string, +) (*model.Device, *model.AuthSet, error) { + + aset, err := d.updateAuthSetStatus(ctx, deviceID, authID, status) + if err != nil { + return nil, nil, err + } else if aset.Status == status { + return nil, aset, nil } device, err := d.db.GetDeviceById(ctx, deviceID) @@ -971,8 +969,6 @@ func (d *DevAuth) setAuthSetStatus( if err != nil { return nil, nil, err } - device.Status = status - aset.Status = status return device, aset, nil } diff --git a/backend/services/deviceauth/model/device.go b/backend/services/deviceauth/model/device.go index cff2d6a5e..9c4fa4a70 100644 --- a/backend/services/deviceauth/model/device.go +++ b/backend/services/deviceauth/model/device.go @@ -15,6 +15,7 @@ package model import ( "encoding/json" + "fmt" "net/url" "time" @@ -48,6 +49,26 @@ var ( } ) +func ValidateStatusTransition(fromStatus, toStatus string) error { + switch toStatus { + case DevStatusAccepted: + if fromStatus == DevStatusRejected || + fromStatus == DevStatusPending { + return nil + } + case DevStatusPending: + if fromStatus != DevStatusPreauth { + return nil + } + case DevStatusRejected: + if fromStatus == DevStatusPending || + fromStatus == DevStatusAccepted { + return nil + } + } + return fmt.Errorf("invalid status transition: %s -> %s", fromStatus, toStatus) +} + // note: fields with underscores need the 'bson' decorator // otherwise the underscore will be removed upon write to mongo type Device struct { From ee510a7cec27110393e7acc1aaa5082ffd821394 Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Tue, 4 Aug 2026 12:18:55 +0200 Subject: [PATCH 07/13] fix(deviceauth): protect device status from concurrent updates Only update the device status and trigger the event if device has the correct revision (entity tag) in the database. Ticket: MEN-9997 Signed-off-by: Alf-Rune Siqveland --- .../services/deviceauth/devauth/devauth.go | 26 ++++++++++++------- .../deviceauth/devauth/devauth_test.go | 24 ++++++++++++++--- .../services/deviceauth/store/datastore.go | 6 +++++ .../deviceauth/store/mocks/DataStore.go | 18 +++++++++++++ .../deviceauth/store/mongo/datastore_mongo.go | 22 +++++++++++++++- 5 files changed, 81 insertions(+), 15 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth.go b/backend/services/deviceauth/devauth/devauth.go index ab8f50bcd..0418772e8 100644 --- a/backend/services/deviceauth/devauth/devauth.go +++ b/backend/services/deviceauth/devauth/devauth.go @@ -474,18 +474,33 @@ func (d *DevAuth) updateDeviceStatus( if device.Status == status { return nil // No-op } + if err := d.db.UpdateDeviceWithRevision(ctx, + device.Id, + device.Revision, + model.DeviceUpdate{ + Status: status, + UpdatedTs: uto.TimePtr(time.Now().UTC()), + }); err != nil { + if errors.Is(err, store.ErrDevNotFound) { + return nil + } + return errors.Wrap(err, "failed to update device status") + } + tenantId := "" idData := identity.FromContext(ctx) if idData != nil { tenantId = idData.Tenant } + + device.Revision += 1 //nolint:bodyclose _, _, err := d.cOrch.StartWorkflow(ctx, "update_device_status"). RequestBody(map[string]interface{}{ "request_id": requestid.FromContext(ctx), "devices": []model.DeviceInventoryUpdate{{ Id: device.Id, - Revision: device.Revision + 1, + Revision: device.Revision, }}, "tenant_id": tenantId, "device_status": status, @@ -495,15 +510,6 @@ func (d *DevAuth) updateDeviceStatus( return errors.Wrap(err, "update device status job error") } - if err := d.db.UpdateDevice(ctx, - device.Id, - model.DeviceUpdate{ - Status: status, - UpdatedTs: uto.TimePtr(time.Now().UTC()), - }); err != nil { - return errors.Wrap(err, "failed to update device status") - } - return nil } diff --git a/backend/services/deviceauth/devauth/devauth_test.go b/backend/services/deviceauth/devauth/devauth_test.go index 4a6997bba..b7fa2b60f 100644 --- a/backend/services/deviceauth/devauth/devauth_test.go +++ b/backend/services/deviceauth/devauth/devauth_test.go @@ -541,6 +541,11 @@ func TestDevAuthSubmitAuthRequest(t *testing.T) { db.On("GetDeviceStatus", ctxMatcher, mock.AnythingOfType("string")).Return( "pending", nil) + db.On("UpdateDeviceWithRevision", ctxMatcher, + devId, + uint(0), + mock.AnythingOfType("model.DeviceUpdate")).Return(nil) + // Update check in time db.On("UpdateDevice", ctxMatcher, devId, mock.AnythingOfType("model.DeviceUpdate")).Return(nil) @@ -848,12 +853,22 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { ).Return(nil) // at the end of processing, updates the device status to 'accepted' + db.On("UpdateDeviceWithRevision", + ctxMatcher, + dummyDevId, + uint(0), + mock.MatchedBy( + func(u model.DeviceUpdate) bool { + return u.Status == model.DevStatusAccepted + }), + ).Return(nil) + // Check in time db.On("UpdateDevice", ctxMatcher, dummyDevId, mock.MatchedBy( func(u model.DeviceUpdate) bool { - return u.Status == model.DevStatusAccepted || !u.CheckInTime.IsZero() + return !u.CheckInTime.IsZero() }), ).Return(nil) @@ -1727,13 +1742,14 @@ func TestDevAuthResetDevice(t *testing.T) { db.On("GetDeviceStatus", context.Background(), dummyDevID).Return( "accepted", nil) - db.On("UpdateDevice", context.Background(), + db.On("UpdateDeviceWithRevision", context.Background(), func() interface{} { if tc.aset != nil { return tc.aset.DeviceId } return mock.AnythingOfType("string") }(), + uint(0), mock.AnythingOfType("model.DeviceUpdate")).Return(nil) co := oas_mocks.NewMockWorkflowsOtherAPI(t) @@ -3013,7 +3029,6 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { DeviceId: oid.NewUUIDv5("devId11").String(), Status: model.DevStatusPending, }, - submitJob: true, dbGetDeviceStatus: model.DevStatusPending, dbUpdateDeviceErr: errors.New("Update Device Error"), outErr: "failed to update device status: Update Device Error", @@ -3124,8 +3139,9 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { tc.devId).Return( tc.dbGetDeviceStatus, tc.dbGetDeviceStatusErr) - db.On("UpdateDevice", ctx, + db.On("UpdateDeviceWithRevision", ctx, tc.devId, + uint(0), mock.AnythingOfType("model.DeviceUpdate")).Return(tc.dbUpdateDeviceErr) db.On("GetDeviceById", ctx, mock.AnythingOfType("string")).Return(&model.Device{ diff --git a/backend/services/deviceauth/store/datastore.go b/backend/services/deviceauth/store/datastore.go index adc1b0e05..8ea959e81 100644 --- a/backend/services/deviceauth/store/datastore.go +++ b/backend/services/deviceauth/store/datastore.go @@ -82,6 +82,12 @@ type DataStore interface { // updates a single device with deviceID, using data from `up` UpdateDevice(ctx context.Context, deviceID string, up model.DeviceUpdate) error + UpdateDeviceWithRevision( + ctx context.Context, + deviceID string, + revision uint, + up model.DeviceUpdate, + ) error // deletes device DeleteDevice(ctx context.Context, id string) error diff --git a/backend/services/deviceauth/store/mocks/DataStore.go b/backend/services/deviceauth/store/mocks/DataStore.go index 517d08dda..fae11bd80 100644 --- a/backend/services/deviceauth/store/mocks/DataStore.go +++ b/backend/services/deviceauth/store/mocks/DataStore.go @@ -723,6 +723,24 @@ func (_m *DataStore) UpdateDevice(ctx context.Context, deviceID string, up model return r0 } +// UpdateDeviceWithRevision provides a mock function with given fields: ctx, deviceID, revision, up +func (_m *DataStore) UpdateDeviceWithRevision(ctx context.Context, deviceID string, revision uint, up model.DeviceUpdate) error { + ret := _m.Called(ctx, deviceID, revision, up) + + if len(ret) == 0 { + panic("no return value specified for UpdateDeviceWithRevision") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, uint, model.DeviceUpdate) error); ok { + r0 = rf(ctx, deviceID, revision, up) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // UpsertAuthSetStatus provides a mock function with given fields: ctx, authSet func (_m *DataStore) UpsertAuthSetStatus(ctx context.Context, authSet *model.AuthSet) error { ret := _m.Called(ctx, authSet) diff --git a/backend/services/deviceauth/store/mongo/datastore_mongo.go b/backend/services/deviceauth/store/mongo/datastore_mongo.go index 1ed35ff6e..e48123c5a 100644 --- a/backend/services/deviceauth/store/mongo/datastore_mongo.go +++ b/backend/services/deviceauth/store/mongo/datastore_mongo.go @@ -339,9 +339,20 @@ func (db *DataStoreMongo) AddDevice(ctx context.Context, d model.Device) error { func (db *DataStoreMongo) UpdateDevice(ctx context.Context, deviceID string, updev model.DeviceUpdate) error { + return db.updateDevice(ctx, deviceID, nil, updev) +} +func (db *DataStoreMongo) updateDevice(ctx context.Context, + deviceID string, revision *uint, updev model.DeviceUpdate) error { c := db.client.Database(DbName).Collection(DbDevicesColl) + filter := bson.M{ + dbFieldID: deviceID, + } + if revision != nil { + filter[DbKeyDeviceRevision] = revision + } + updev.UpdatedTs = uto.TimePtr(time.Now().UTC()) update := bson.M{ "$inc": bson.M{ @@ -350,7 +361,7 @@ func (db *DataStoreMongo) UpdateDevice(ctx context.Context, "$set": updev, } - res, err := c.UpdateOne(ctx, mongostore.WithTenantID(ctx, bson.M{"_id": deviceID}), update) + res, err := c.UpdateOne(ctx, mongostore.WithTenantID(ctx, filter), update) if err != nil { return errors.Wrap(err, "failed to update device") } else if res.MatchedCount < 1 { @@ -360,6 +371,15 @@ func (db *DataStoreMongo) UpdateDevice(ctx context.Context, return nil } +func (db *DataStoreMongo) UpdateDeviceWithRevision( + ctx context.Context, + deviceID string, + revision uint, + up model.DeviceUpdate, +) error { + return db.updateDevice(ctx, deviceID, &revision, up) +} + func (db *DataStoreMongo) DeleteDevice(ctx context.Context, id string) error { c := db.client.Database(DbName).Collection(DbDevicesColl) From e090c298c208033d6341a1a90144618f11a5db39 Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Tue, 4 Aug 2026 14:32:29 +0200 Subject: [PATCH 08/13] feat(iot-manager): device-status-changed webhook event contains auth set Only decommission events does not carry auth sets. Changelog: Commit Signed-off-by: Alf-Rune Siqveland --- .../services/deviceauth/devauth/devauth.go | 73 +++++++++++++------ .../deviceauth/devauth/devauth_test.go | 59 +++++++++------ .../services/iot-manager/api/http/internal.go | 6 +- 3 files changed, 89 insertions(+), 49 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth.go b/backend/services/deviceauth/devauth/devauth.go index 0418772e8..9ee3acba5 100644 --- a/backend/services/deviceauth/devauth/devauth.go +++ b/backend/services/deviceauth/devauth/devauth.go @@ -387,6 +387,7 @@ func (d *DevAuth) handlePreAuthDevice( if err := d.updateDeviceStatus( ctx, + aset, dev, model.DevStatusAccepted, ); err != nil { @@ -466,8 +467,33 @@ func (d *DevAuth) aggregateDeviceStatus(ctx context.Context, deviceID string) (s } +func updateDeviceStatusEvent( + authSet *model.AuthSet, + device *model.Device, + status string, +) client.DeviceAuthEvent { + event := client.DeviceAuthEvent{ + Id: device.Id, + Status: &status, + AdditionalProperties: map[string]any{"revision": device.Revision}, + CreatedTs: &device.CreatedTs, + } + if authSet != nil { + event.AuthSets = append(event.AuthSets, client.AuthSet{ + Id: &authSet.Id, + DeviceId: &authSet.DeviceId, + IdentityData: authSet.IdDataStruct, + Pubkey: &authSet.PubKey, + Status: &status, + Ts: authSet.Timestamp, + }) + } + return event +} + func (d *DevAuth) updateDeviceStatus( ctx context.Context, + authSet *model.AuthSet, device *model.Device, status string, ) error { @@ -496,12 +522,11 @@ func (d *DevAuth) updateDeviceStatus( device.Revision += 1 //nolint:bodyclose _, _, err := d.cOrch.StartWorkflow(ctx, "update_device_status"). - RequestBody(map[string]interface{}{ + RequestBody(map[string]any{ "request_id": requestid.FromContext(ctx), - "devices": []model.DeviceInventoryUpdate{{ - Id: device.Id, - Revision: device.Revision, - }}, + "devices": []client.DeviceAuthEvent{ + updateDeviceStatusEvent(authSet, device, status), + }, "tenant_id": tenantId, "device_status": status, }).Execute() @@ -556,7 +581,7 @@ func (d *DevAuth) processAuthRequest( } // update the device status - if err := d.updateDeviceStatus(ctx, dev, status); err != nil { + if err := d.updateDeviceStatus(ctx, areq, dev, status); err != nil { return nil, err } @@ -781,7 +806,7 @@ func (d *DevAuth) DeleteAuthSet(ctx context.Context, devID string, authId string return fmt.Errorf("failed to update device status: %w", err) } - return d.updateDeviceStatus(ctx, dev, newStatus) + return d.updateDeviceStatus(ctx, authSet, dev, newStatus) } func (d *DevAuth) deletePreauthDevice(ctx context.Context, devId string) error { @@ -971,7 +996,7 @@ func (d *DevAuth) setAuthSetStatus( return nil, nil, err } } - err = d.updateDeviceStatus(ctx, device, status) + err = d.updateDeviceStatus(ctx, aset, device, status) if err != nil { return nil, nil, err } @@ -1060,6 +1085,7 @@ func (d *DevAuth) PreauthorizeDevice( if err != nil { return nil, err } + // FIXME: what about device status and iot-manager? return dev, nil } return dev, ErrDeviceExists @@ -1073,21 +1099,6 @@ func (d *DevAuth) PreauthorizeDevice( tenantId = idData.Tenant } - //nolint:bodyclose - _, _, err = d.cOrch.StartWorkflow(ctx, "update_device_status"). - RequestBody(map[string]interface{}{ - "request_id": requestid.FromContext(ctx), - "devices": []model.DeviceInventoryUpdate{{ - Id: dev.Id, - Revision: dev.Revision, - }}, - "tenant_id": tenantId, - "device_status": dev.Status, - }).Execute() - if err != nil { - return nil, errors.Wrap(err, "update device status job error") - } - // record authentication request authset := model.AuthSet{ Id: req.AuthSetId, @@ -1106,7 +1117,21 @@ func (d *DevAuth) PreauthorizeDevice( if err := d.setDeviceIdentity(ctx, dev, tenantId); err != nil { return nil, err } - return nil, nil + //nolint:bodyclose + _, _, err = d.cOrch.StartWorkflow(ctx, "update_device_status"). + RequestBody(map[string]any{ + "request_id": requestid.FromContext(ctx), + "devices": []client.DeviceAuthEvent{ + updateDeviceStatusEvent(&authset, dev, dev.Status), + }, + "tenant_id": tenantId, + "device_status": dev.Status, + }).Execute() + if err != nil { + return nil, errors.Wrap(err, "update device status job error") + } + + return dev, nil case store.ErrObjectExists: dev, err = d.db.GetDeviceByIdentityDataHash(ctx, idDataSha256) if err != nil { diff --git a/backend/services/deviceauth/devauth/devauth_test.go b/backend/services/deviceauth/devauth/devauth_test.go index b7fa2b60f..c5a2b4c20 100644 --- a/backend/services/deviceauth/devauth/devauth_test.go +++ b/backend/services/deviceauth/devauth/devauth_test.go @@ -973,6 +973,10 @@ func TestDevAuthPreauthorizeDevice(t *testing.T) { updateDeviceStatus: true, updateDeviceInventory: true, callDb: true, + outDev: &model.Device{ + Id: deviceID, + Status: model.DevStatusPreauth, + }, }, { desc: "error: add device, exists", @@ -981,8 +985,11 @@ func TestDevAuthPreauthorizeDevice(t *testing.T) { addDeviceErr: store.ErrObjectExists, - outDev: &model.Device{Id: deviceID}, - err: ErrDeviceExists, + outDev: &model.Device{ + Id: deviceID, + Status: model.DevStatusAccepted, + }, + err: ErrDeviceExists, }, { desc: "error: add device, generic", @@ -994,21 +1001,22 @@ func TestDevAuthPreauthorizeDevice(t *testing.T) { err: errors.New("failed to add device: generic error"), }, { - desc: "error: add auth set, exists", - req: req, - updateDeviceStatus: true, - callDb: true, + desc: "error: add auth set, exists", + req: req, + callDb: true, addAuthSetErr: store.ErrObjectExists, - outDev: &model.Device{Id: deviceID}, - err: ErrDeviceExists, + outDev: &model.Device{ + Id: deviceID, + Status: model.DevStatusAccepted, + }, + err: ErrDeviceExists, }, { - desc: "error: add auth set, exists", - req: req, - updateDeviceStatus: true, - callDb: true, + desc: "error: add auth set, generic error", + req: req, + callDb: true, addAuthSetErr: errors.New("generic error"), @@ -1088,6 +1096,7 @@ func TestDevAuthPreauthorizeDevice(t *testing.T) { return &model.Device{ IdDataSha256: idDataSha256, Id: deviceID, + Status: model.DevStatusAccepted, } } return nil @@ -1105,6 +1114,7 @@ func TestDevAuthPreauthorizeDevice(t *testing.T) { if tc.outDev != nil { assert.Equal(t, tc.outDev.Id, dev.Id) + assert.Equal(t, tc.outDev.Status, dev.Status) } else { assert.Nil(t, dev) } @@ -3155,22 +3165,29 @@ func TestDevAuthDeleteAuthSet(t *testing.T) { revision = 0 } if tc.submitJob { - if status == model.DevStatusNoAuth { - status = "decommissioned" - } if tc.dbGetDeviceStatusErr == store.ErrAuthSetNotFound { status = "noauth" } + var events any = []client.DeviceAuthEvent{updateDeviceStatusEvent(tc.authSet, &model.Device{ + Id: tc.devId, + Revision: revision, + }, status)} + + if tc.authSet.Status == model.DevStatusPreauth && + status == model.DevStatusNoAuth { + status = "decommissioned" + events = []model.DeviceInventoryUpdate{{ + Id: tc.devId, + Revision: revision, + }} + } req := client.ApiStartWorkflowRequest{ ApiService: co, }.RequestBody(map[string]interface{}{ "device_status": status, - "devices": []model.DeviceInventoryUpdate{{ - Id: tc.devId, - Revision: revision, - }}, - "request_id": "", - "tenant_id": tc.tenant, + "devices": events, + "request_id": "", + "tenant_id": tc.tenant, }) co.EXPECT(). StartWorkflow(mtesting.ContextMatcher(), "update_device_status"). diff --git a/backend/services/iot-manager/api/http/internal.go b/backend/services/iot-manager/api/http/internal.go index d2dacd249..4baece99d 100644 --- a/backend/services/iot-manager/api/http/internal.go +++ b/backend/services/iot-manager/api/http/internal.go @@ -134,9 +134,7 @@ const ( // PUT /tenants/:tenant_id/devices/status/{status} func (h *InternalHandler) BulkSetDeviceStatus(c *gin.Context) { - var schema []struct { - DeviceID string `json:"id"` - } + var schema []internalDevice status := model.Status(c.Param("status")) if err := status.Validate(); err != nil { rest.RenderError(c, http.StatusBadRequest, err) @@ -162,7 +160,7 @@ func (h *InternalHandler) BulkSetDeviceStatus(c *gin.Context) { }, ) for _, item := range schema { - _ = h.app.SetDeviceStatus(ctx, item.DeviceID, status) + _ = h.app.SetDeviceStatus(ctx, item.ID, status) } c.Status(http.StatusAccepted) } From 4e240d9fdc030a8ba750e0ac0e5bc7c7ba170b23 Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Tue, 4 Aug 2026 15:54:59 +0200 Subject: [PATCH 09/13] refactor(deviceauth): unify all device status update paths Updated provision device to use status endpoint instead of POST /devices API since the former uses the revision to protect concurrent updates. Removed the duplicate code paths for updating device statuses. Signed-off-by: Alf-Rune Siqveland --- .../deviceauth/api/http/api_devauth.go | 9 +- .../services/deviceauth/devauth/devauth.go | 151 ++++++------------ .../deviceauth/devauth/devauth_test.go | 25 +-- .../services/deviceauth/devauth/mocks/App.go | 77 ++++----- .../mmock/inventory_POST_devices_v2.json | 2 +- .../tests/tests/test_provision_device.py | 8 +- .../worker/workflows/provision_device.json | 37 +++-- 7 files changed, 125 insertions(+), 184 deletions(-) diff --git a/backend/services/deviceauth/api/http/api_devauth.go b/backend/services/deviceauth/api/http/api_devauth.go index 9df4c72b3..fa2fbf8bb 100644 --- a/backend/services/deviceauth/api/http/api_devauth.go +++ b/backend/services/deviceauth/api/http/api_devauth.go @@ -455,14 +455,7 @@ func (i *DevAuthApiHandlers) UpdateDeviceStatusHandler(c *gin.Context) { return } - switch status.Status { - case model.DevStatusAccepted: - err = i.app.AcceptDeviceAuth(ctx, devid, authid) - case model.DevStatusRejected: - err = i.app.RejectDeviceAuth(ctx, devid, authid) - case model.DevStatusPending: - err = i.app.ResetDeviceAuth(ctx, devid, authid) - } + err = i.app.SetAuthSetStatus(ctx, devid, authid, status.Status) if err != nil { switch err { case store.ErrDevNotFound, store.ErrAuthSetNotFound: diff --git a/backend/services/deviceauth/devauth/devauth.go b/backend/services/deviceauth/devauth/devauth.go index 9ee3acba5..cc2b56581 100644 --- a/backend/services/deviceauth/devauth/devauth.go +++ b/backend/services/deviceauth/devauth/devauth.go @@ -93,9 +93,12 @@ type App interface { DecommissionDevice(ctx context.Context, dev_id string) error DeleteDevice(ctx context.Context, dev_id string) error DeleteAuthSet(ctx context.Context, dev_id string, auth_id string) error - AcceptDeviceAuth(ctx context.Context, dev_id string, auth_id string) error - RejectDeviceAuth(ctx context.Context, dev_id string, auth_id string) error - ResetDeviceAuth(ctx context.Context, dev_id string, auth_id string) error + SetAuthSetStatus( + ctx context.Context, + deviceID string, + authID string, + status string, + ) error PreauthorizeDevice(ctx context.Context, req *model.PreAuthReq) (*model.Device, error) RevokeToken(ctx context.Context, tokenID string) error @@ -384,6 +387,7 @@ func (d *DevAuth) handlePreAuthDevice( if err := d.db.UpdateAuthSetById(ctx, aset.Id, update); err != nil { return nil, errors.Wrap(err, "failed to update auth set status") } + aset.Status = model.DevStatusAccepted if err := d.updateDeviceStatus( ctx, @@ -397,28 +401,6 @@ func (d *DevAuth) handlePreAuthDevice( aset.Status = model.DevStatusAccepted dev.Status = model.DevStatusAccepted dev.AuthSets = append(dev.AuthSets, *aset) - - if !dev.Provisioned { - reqId := requestid.FromContext(ctx) - var tenantID string - if idty := identity.FromContext(ctx); idty != nil { - tenantID = idty.Tenant - } - - // submit device accepted job - //nolint:bodyclose - _, _, err := d.cOrch.StartWorkflow(ctx, "provision_device"). - RequestBody(map[string]interface{}{ - "request_id": reqId, - "device_id": aset.DeviceId, - "tenant_id": tenantID, - "device": dev, - "status": dev.Status, - }).Execute() - if err != nil { - return nil, errors.Wrap(err, "submit device provisioning job error") - } - } return aset, nil } @@ -484,7 +466,7 @@ func updateDeviceStatusEvent( DeviceId: &authSet.DeviceId, IdentityData: authSet.IdDataStruct, Pubkey: &authSet.PubKey, - Status: &status, + Status: &authSet.Status, Ts: authSet.Timestamp, }) } @@ -520,19 +502,37 @@ func (d *DevAuth) updateDeviceStatus( } device.Revision += 1 - //nolint:bodyclose - _, _, err := d.cOrch.StartWorkflow(ctx, "update_device_status"). - RequestBody(map[string]any{ - "request_id": requestid.FromContext(ctx), - "devices": []client.DeviceAuthEvent{ - updateDeviceStatusEvent(authSet, device, status), - }, - "tenant_id": tenantId, - "device_status": status, - }).Execute() + device.Status = status - if err != nil { - return errors.Wrap(err, "update device status job error") + if status == model.DevStatusAccepted && !device.Provisioned { + //nolint:bodyclose + _, _, err := d.cOrch.StartWorkflow(ctx, "provision_device"). + RequestBody(map[string]any{ + "request_id": requestid.FromContext(ctx), + "device_id": device.Id, + "tenant_id": tenantId, + "device": updateDeviceStatusEvent(authSet, device, status), + "status": status, + }).Execute() + + if err != nil { + return errors.Wrap(err, "submit device provisioning job error") + } + } else { + //nolint:bodyclose + _, _, err := d.cOrch.StartWorkflow(ctx, "update_device_status"). + RequestBody(map[string]any{ + "request_id": requestid.FromContext(ctx), + "devices": []client.DeviceAuthEvent{ + updateDeviceStatusEvent(authSet, device, status), + }, + "tenant_id": tenantId, + "device_status": status, + }).Execute() + + if err != nil { + return errors.Wrap(err, "update device status job error") + } } return nil @@ -864,52 +864,6 @@ func (d *DevAuth) deleteAuthSet(ctx context.Context, authSet *model.AuthSet) err return nil } -func (d *DevAuth) AcceptDeviceAuth(ctx context.Context, device_id string, auth_id string) error { - var ( - dev *model.Device - err error - ) - if dev, _, err = d.setAuthSetStatus( - ctx, device_id, auth_id, model.DevStatusAccepted, - ); err != nil { - return err - } - - if dev == nil { - // Status did not change - return nil - } - - if dev.Provisioned { - // Device already provisioned - // We're done... - return nil - } - - reqId := requestid.FromContext(ctx) - - var tenantID string - if idty := identity.FromContext(ctx); idty != nil { - tenantID = idty.Tenant - } - - // submit device accepted job - //nolint:bodyclose - _, _, err = d.cOrch.StartWorkflow(ctx, "provision_device"). - RequestBody(map[string]interface{}{ - "request_id": reqId, - "device_id": dev.Id, - "tenant_id": tenantID, - "device": dev, - "status": dev.Status, - }).Execute() - if err != nil { - return errors.Wrap(err, "submit device provisioning job error") - } - - return nil -} - func (d *DevAuth) updateAuthSetStatus( ctx context.Context, deviceID, authID, status string, ) (*model.AuthSet, error) { @@ -926,7 +880,7 @@ func (d *DevAuth) updateAuthSetStatus( } if aset.Status == status { - return aset, nil + return nil, nil } // Validate status transition @@ -969,48 +923,37 @@ func (d *DevAuth) updateAuthSetStatus( }); err != nil { return nil, errors.Wrap(err, "db update device auth set error") } + aset.Status = status return aset, nil } -func (d *DevAuth) setAuthSetStatus( +func (d *DevAuth) SetAuthSetStatus( ctx context.Context, deviceID string, authID string, status string, -) (*model.Device, *model.AuthSet, error) { +) error { aset, err := d.updateAuthSetStatus(ctx, deviceID, authID, status) if err != nil { - return nil, nil, err - } else if aset.Status == status { - return nil, aset, nil + return err } device, err := d.db.GetDeviceById(ctx, deviceID) if err != nil { - return nil, nil, err + return err } if status != model.DevStatusAccepted { status, err = d.aggregateDeviceStatus(ctx, deviceID) if err != nil { - return nil, nil, err + return err } } err = d.updateDeviceStatus(ctx, aset, device, status) if err != nil { - return nil, nil, err + return err } - return device, aset, nil -} - -func (d *DevAuth) RejectDeviceAuth(ctx context.Context, device_id string, auth_id string) error { - _, _, err := d.setAuthSetStatus(ctx, device_id, auth_id, model.DevStatusRejected) - return err -} - -func (d *DevAuth) ResetDeviceAuth(ctx context.Context, device_id string, auth_id string) error { - _, _, err := d.setAuthSetStatus(ctx, device_id, auth_id, model.DevStatusPending) - return err + return nil } func parseIdData(idData string) (map[string]interface{}, []byte, error) { diff --git a/backend/services/deviceauth/devauth/devauth_test.go b/backend/services/deviceauth/devauth/devauth_test.go index c5a2b4c20..21e67fb2e 100644 --- a/backend/services/deviceauth/devauth/devauth_test.go +++ b/backend/services/deviceauth/devauth/devauth_test.go @@ -482,6 +482,7 @@ func TestDevAuthSubmitAuthRequest(t *testing.T) { IdDataSha256: idDataHash, IdDataStruct: idDataStruct, Id: devId, + Provisioned: true, } } return nil @@ -664,7 +665,6 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { expectedWorkflows: map[string]error{ "provision_device": nil, "update_device_inventory": nil, - "update_device_status": nil, }, }, { @@ -749,8 +749,7 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { Status: model.DevStatusPreauth, }, expectedWorkflows: map[string]error{ - "provision_device": errors.New("workflows failed"), - "update_device_status": nil, + "provision_device": errors.New("workflows failed"), }, err: errors.New("submit device provisioning job error: workflows failed"), }, @@ -774,7 +773,6 @@ func TestDevAuthSubmitAuthRequestPreauth(t *testing.T) { //coSubmitProvisionDeviceJobErr: errors.New("workflows shouldn't be called"), // MEN-6961: we accept preauth at all times res: "dummytoken", expectedWorkflows: map[string]error{ - "provision_device": nil, "update_device_inventory": nil, }, }, @@ -1395,8 +1393,9 @@ func TestDevAuthAcceptDevice(t *testing.T) { } devauth := NewDevAuth(&db, nil, nil, nil, Config{}) - err := devauth.AcceptDeviceAuth( - context.Background(), dummyDevID, dummyAuthID) + _, _, err := devauth.SetAuthSetStatus( + context.Background(), dummyDevID, dummyAuthID, model.DevStatusAccepted, + ) if tc.outErr != "" { assert.EqualError(t, err, tc.outErr) @@ -1553,8 +1552,8 @@ func TestDevAuthRejectDevice(t *testing.T) { c.AssertNotCalled(t, "DeleteToken") } - err := devauth.RejectDeviceAuth( - ctx, dummyDevID, dummyAuthID, + _, _, err := devauth.SetAuthSetStatus( + ctx, dummyDevID, dummyAuthID, model.DevStatusRejected, ) if tc.outErr != "" { @@ -1744,7 +1743,11 @@ func TestDevAuthResetDevice(t *testing.T) { model.AuthSetUpdate{Status: model.DevStatusPending}).Return(nil) db.On("GetDeviceById", context.Background(), mock.AnythingOfType("string")). - Return(&model.Device{Id: tc.aset.DeviceId, Status: tc.aset.Status}, nil) + Return(&model.Device{ + Id: tc.aset.DeviceId, + Status: tc.aset.Status, + Provisioned: true, + }, nil) } db.On("DeleteTokenByDevId", context.Background(), dummyDevUUID).Return( @@ -1778,8 +1781,8 @@ func TestDevAuthResetDevice(t *testing.T) { } devauth := NewDevAuth(&db, co, nil, nil, Config{}) - err := devauth.ResetDeviceAuth( - context.Background(), dummyDevID, dummyAuthID, + _, _, err := devauth.SetAuthSetStatus( + context.Background(), dummyDevID, dummyAuthID, model.DevStatusPending, ) if tc.dbErr != nil || diff --git a/backend/services/deviceauth/devauth/mocks/App.go b/backend/services/deviceauth/devauth/mocks/App.go index 15eed72fe..f9444b6bc 100644 --- a/backend/services/deviceauth/devauth/mocks/App.go +++ b/backend/services/deviceauth/devauth/mocks/App.go @@ -29,24 +29,6 @@ type App struct { mock.Mock } -// AcceptDeviceAuth provides a mock function with given fields: ctx, dev_id, auth_id -func (_m *App) AcceptDeviceAuth(ctx context.Context, dev_id string, auth_id string) error { - ret := _m.Called(ctx, dev_id, auth_id) - - if len(ret) == 0 { - panic("no return value specified for AcceptDeviceAuth") - } - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok { - r0 = rf(ctx, dev_id, auth_id) - } else { - r0 = ret.Error(0) - } - - return r0 -} - // DecommissionDevice provides a mock function with given fields: ctx, dev_id func (_m *App) DecommissionDevice(ctx context.Context, dev_id string) error { ret := _m.Called(ctx, dev_id) @@ -363,17 +345,17 @@ func (_m *App) PreauthorizeDevice(ctx context.Context, req *model.PreAuthReq) (* return r0, r1 } -// RejectDeviceAuth provides a mock function with given fields: ctx, dev_id, auth_id -func (_m *App) RejectDeviceAuth(ctx context.Context, dev_id string, auth_id string) error { - ret := _m.Called(ctx, dev_id, auth_id) +// RevokeToken provides a mock function with given fields: ctx, tokenID +func (_m *App) RevokeToken(ctx context.Context, tokenID string) error { + ret := _m.Called(ctx, tokenID) if len(ret) == 0 { - panic("no return value specified for RejectDeviceAuth") + panic("no return value specified for RevokeToken") } var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok { - r0 = rf(ctx, dev_id, auth_id) + if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { + r0 = rf(ctx, tokenID) } else { r0 = ret.Error(0) } @@ -381,40 +363,43 @@ func (_m *App) RejectDeviceAuth(ctx context.Context, dev_id string, auth_id stri return r0 } -// ResetDeviceAuth provides a mock function with given fields: ctx, dev_id, auth_id -func (_m *App) ResetDeviceAuth(ctx context.Context, dev_id string, auth_id string) error { - ret := _m.Called(ctx, dev_id, auth_id) +// SetAuthSetStatus provides a mock function with given fields: ctx, deviceID, authID, status +func (_m *App) SetAuthSetStatus(ctx context.Context, deviceID string, authID string, status string) (*model.Device, *model.AuthSet, error) { + ret := _m.Called(ctx, deviceID, authID, status) if len(ret) == 0 { - panic("no return value specified for ResetDeviceAuth") + panic("no return value specified for SetAuthSetStatus") } - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok { - r0 = rf(ctx, dev_id, auth_id) + var r0 *model.Device + var r1 *model.AuthSet + var r2 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, string) (*model.Device, *model.AuthSet, error)); ok { + return rf(ctx, deviceID, authID, status) + } + if rf, ok := ret.Get(0).(func(context.Context, string, string, string) *model.Device); ok { + r0 = rf(ctx, deviceID, authID, status) } else { - r0 = ret.Error(0) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Device) + } } - return r0 -} - -// RevokeToken provides a mock function with given fields: ctx, tokenID -func (_m *App) RevokeToken(ctx context.Context, tokenID string) error { - ret := _m.Called(ctx, tokenID) - - if len(ret) == 0 { - panic("no return value specified for RevokeToken") + if rf, ok := ret.Get(1).(func(context.Context, string, string, string) *model.AuthSet); ok { + r1 = rf(ctx, deviceID, authID, status) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AuthSet) + } } - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string) error); ok { - r0 = rf(ctx, tokenID) + if rf, ok := ret.Get(2).(func(context.Context, string, string, string) error); ok { + r2 = rf(ctx, deviceID, authID, status) } else { - r0 = ret.Error(0) + r2 = ret.Error(2) } - return r0 + return r0, r1, r2 } // SetTenantLimit provides a mock function with given fields: ctx, tenant_id, limit diff --git a/backend/services/workflows/tests/mmock/inventory_POST_devices_v2.json b/backend/services/workflows/tests/mmock/inventory_POST_devices_v2.json index 57310fe12..e10aaafdc 100644 --- a/backend/services/workflows/tests/mmock/inventory_POST_devices_v2.json +++ b/backend/services/workflows/tests/mmock/inventory_POST_devices_v2.json @@ -2,7 +2,7 @@ "description": "inventory: POST devices v2", "request": { "method": "POST", - "path": "/api/internal/v1/inventory/tenants/1/devices/status/accepted" + "path": "/api/internal/v1/inventory/tenants/*/devices/status/accepted" }, "response": { "statusCode": 200 diff --git a/backend/services/workflows/tests/tests/test_provision_device.py b/backend/services/workflows/tests/tests/test_provision_device.py index 4c63bc8f0..4c53a38f7 100644 --- a/backend/services/workflows/tests/tests/test_provision_device.py +++ b/backend/services/workflows/tests/tests/test_provision_device.py @@ -80,19 +80,19 @@ def do_provision_device(mmock_url, workflows_url, tenant_id): "host": "mender-inventory", "port": "8080", "method": "POST", - "path": "/api/internal/v1/inventory/tenants/" + tenant_id + "/devices", + "path": f"/api/internal/v1/inventory/tenants/{tenant_id}/devices/status/accepted", "queryStringParameters": {}, "fragment": "", "headers": { "Accept-Encoding": ["gzip"], - "Content-Length": ["81"], + "Content-Length": ["12"], "Content-Type": ["application/json"], "User-Agent": ["Go-http-client/1.1"], "X-Men-Requestid": ["1234567890"], }, "cookies": {}, - "body": '{"attributes":[{"name":"status","scope":"identity","value":"accepted"}],"id":"1"}', - }, + "body": '[{"id":"1"}]', + } }, { "request": { diff --git a/backend/services/workflows/worker/workflows/provision_device.json b/backend/services/workflows/worker/workflows/provision_device.json index ceae3d08b..f00c6e5a2 100644 --- a/backend/services/workflows/worker/workflows/provision_device.json +++ b/backend/services/workflows/worker/workflows/provision_device.json @@ -1,24 +1,17 @@ { "name": "provision_device", "description": "Provision device.", - "version": 11, + "version": 12, "tasks": [{ "name": "create_device_inventory", "type": "http", "retries": 3, "retryDelaySeconds": 5, "http": { - "uri": "http://${env.INVENTORY_ADDR|mender-inventory:8080}/api/internal/v1/inventory/tenants/${encoding=url;workflow.input.tenant_id}/devices", + "uri": "http://${env.INVENTORY_ADDR|mender-inventory:8080}/api/internal/v1/inventory/tenants/${encoding=url;workflow.input.tenant_id}/devices/status/${encoding=url;workflow.input.status}", "method": "POST", "contentType": "application/json", - "json": { - "id": "${workflow.input.device_id}", - "attributes": [{ - "name": "status", - "scope": "identity", - "value": "${workflow.input.status}" - }] - }, + "json": ["${workflow.input.device}"], "headers": { "X-MEN-RequestID": "${workflow.input.request_id}" }, @@ -112,6 +105,30 @@ 404 ] } + }, + { + "name": "update_iot_manager_status", + "type": "http", + "retries": 3, + "retryDelaySeconds": 5, + "http": { + "uri": "http://${env.IOT_MANAGER_ADDR|mender-iot-manager:8080}/api/internal/v1/iot-manager/tenants/${encoding=url;workflow.input.tenant_id}/bulk/devices/status/${encoding=url;workflow.input.status}", + "method": "PUT", + "contentType": "application/json", + "json": ["${workflow.input.device}"], + "headers": { + "X-MEN-RequestID": "${workflow.input.request_id}" + }, + "connectionTimeOut": 8000, + "readTimeOut": 8000, + "statusCodes": [ + 200, + 201, + 202, + 204, + 404 + ] + } } ], "inputParameters": [ From 22e51568a9077f236ee340219abe4e384ebdf322 Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Wed, 5 Aug 2026 10:47:19 +0200 Subject: [PATCH 10/13] test(deviceauth): fixup unit tests after interface change Signed-off-by: Alf-Rune Siqveland --- .../deviceauth/api/http/api_devauth_test.go | 162 ++++++++---------- .../deviceauth/devauth/devauth_test.go | 6 +- .../services/deviceauth/devauth/mocks/App.go | 31 +--- 3 files changed, 78 insertions(+), 121 deletions(-) diff --git a/backend/services/deviceauth/api/http/api_devauth_test.go b/backend/services/deviceauth/api/http/api_devauth_test.go index f1be36698..dbad9f110 100644 --- a/backend/services/deviceauth/api/http/api_devauth_test.go +++ b/backend/services/deviceauth/api/http/api_devauth_test.go @@ -568,7 +568,7 @@ func TestApiV2DevAuthUpdateStatusDevice(t *testing.T) { }, } - mockaction := func(_ context.Context, dev_id string, auth_id string) error { + mockaction := func(dev_id string, auth_id string) error { d, ok := devs[dev_id+","+auth_id] if ok == false { return store.ErrDevNotFound @@ -578,116 +578,72 @@ func TestApiV2DevAuthUpdateStatusDevice(t *testing.T) { } return nil } - da := &mocks.App{} - da.On("AcceptDeviceAuth", - mtest.ContextMatcher(), - mock.AnythingOfType("string"), - mock.AnythingOfType("string")).Return(mockaction) - da.On("RejectDeviceAuth", - mtest.ContextMatcher(), - mock.AnythingOfType("string"), - mock.AnythingOfType("string")).Return(mockaction) - da.On("ResetDeviceAuth", - mtest.ContextMatcher(), - mock.AnythingOfType("string"), - mock.AnythingOfType("string")).Return(mockaction) - - apih := makeMockApiHandler(t, da, nil) - - accstatus := DevAuthApiStatus{"accepted"} - rejstatus := DevAuthApiStatus{"rejected"} - penstatus := DevAuthApiStatus{"pending"} tcases := []struct { - req *http.Request - code int - body string + deviceID, authID string + status string + code int + body string }{ { - req: rtest.MakeTestRequest(&rtest.TestRequest{ - Method: "PUT", - Path: "http://localhost/api/management/v2/devauth/devices/123/auth/456/status", - Auth: true, - }), - code: http.StatusBadRequest, - body: RestError("failed to decode status data: invalid request"), + deviceID: "123", + authID: "456", + status: "", + code: http.StatusBadRequest, + body: RestError("failed to decode status data: invalid request"), }, { - req: rtest.MakeTestRequest(&rtest.TestRequest{ - Method: "PUT", - Path: "http://localhost/api/management/v2/devauth/devices/123/auth/456/status", - Auth: true, - Body: DevAuthApiStatus{"foo"}, - }), - code: http.StatusBadRequest, - body: RestError("incorrect device status"), + deviceID: "123", + authID: "456", + status: "foo", + code: http.StatusBadRequest, + body: RestError("incorrect device status"), }, { - req: rtest.MakeTestRequest(&rtest.TestRequest{ - Method: "PUT", - Path: "http://localhost/api/management/v2/devauth/devices/123/auth/456/status", - Auth: true, - Body: accstatus, - }), - code: http.StatusNoContent, + deviceID: "123", + authID: "456", + status: model.DevStatusAccepted, + code: http.StatusNoContent, }, { - req: rtest.MakeTestRequest(&rtest.TestRequest{ - Method: "PUT", - Path: "http://localhost/api/management/v2/devauth/devices/345/auth/678/status", - Auth: true, - Body: accstatus, - }), - code: http.StatusInternalServerError, - body: RestError("internal error"), + deviceID: "345", + authID: "678", + status: model.DevStatusAccepted, + code: http.StatusInternalServerError, + body: RestError("internal error"), }, { - req: rtest.MakeTestRequest(&rtest.TestRequest{ - Method: "PUT", - Path: "http://localhost/api/management/v2/devauth/devices/999/auth/123/status", - Auth: true, - Body: accstatus, - }), - code: http.StatusNotFound, - body: RestError(store.ErrDevNotFound.Error()), + deviceID: "999", + authID: "123", + status: model.DevStatusAccepted, + code: http.StatusNotFound, + body: RestError(store.ErrDevNotFound.Error()), }, { - req: rtest.MakeTestRequest(&rtest.TestRequest{ - Method: "PUT", - Path: "http://localhost/api/management/v2/devauth/devices/123/auth/456/status", - Auth: true, - Body: rejstatus, - }), - code: http.StatusNoContent, + deviceID: "123", + authID: "456", + status: model.DevStatusRejected, + code: http.StatusNoContent, }, { - req: rtest.MakeTestRequest(&rtest.TestRequest{ - Method: "PUT", - Path: "http://localhost/api/management/v2/devauth/devices/123/auth/456/status", - Auth: true, - Body: penstatus, - }), - code: http.StatusNoContent, + deviceID: "123", + authID: "456", + status: model.DevStatusPending, + code: http.StatusNoContent, }, { - req: rtest.MakeTestRequest(&rtest.TestRequest{ - Method: "PUT", - Path: "http://localhost/api/management/v2/devauth/devices/234/auth/567/status", - Auth: true, - Body: penstatus, - }), - code: http.StatusBadRequest, - body: RestError("dev auth: dev ID and auth ID mismatch"), + deviceID: "234", + authID: "567", + status: model.DevStatusPending, + code: http.StatusBadRequest, + body: RestError("dev auth: dev ID and auth ID mismatch"), }, { - req: rtest.MakeTestRequest(&rtest.TestRequest{ - Method: "PUT", - Path: "http://localhost/api/management/v2/devauth/devices/567/auth/890/status", - Auth: true, - Body: accstatus, - }), - code: http.StatusUnprocessableEntity, - body: RestError("maximum number of accepted devices reached"), + deviceID: "567", + authID: "890", + status: model.DevStatusAccepted, + code: http.StatusUnprocessableEntity, + body: RestError("maximum number of accepted devices reached"), }, } @@ -695,8 +651,30 @@ func TestApiV2DevAuthUpdateStatusDevice(t *testing.T) { tc := tcases[idx] t.Run(fmt.Sprintf("tc %d", idx), func(t *testing.T) { t.Parallel() + da := mocks.NewApp(t) + da.On("SetAuthSetStatus", + mtest.ContextMatcher(), + mock.AnythingOfType("string"), + mock.AnythingOfType("string"), + tc.status, + ). + Return(mockaction(tc.deviceID, tc.authID)). + Maybe() - runTestRequest(t, apih, tc.req, tc.code, tc.body) + apih := makeMockApiHandler(t, da, nil) + var body any + if tc.status != "" { + body = DevAuthApiStatus{Status: tc.status} + } + req := rtest.MakeTestRequest(&rtest.TestRequest{ + Method: "PUT", + Path: fmt.Sprintf("http://localhost/api/management/v2/devauth/devices/%s/auth/%s/status", + tc.deviceID, tc.authID), + Auth: true, + Body: body, + }) + + runTestRequest(t, apih, req, tc.code, tc.body) }) } diff --git a/backend/services/deviceauth/devauth/devauth_test.go b/backend/services/deviceauth/devauth/devauth_test.go index 21e67fb2e..ec5525876 100644 --- a/backend/services/deviceauth/devauth/devauth_test.go +++ b/backend/services/deviceauth/devauth/devauth_test.go @@ -1393,7 +1393,7 @@ func TestDevAuthAcceptDevice(t *testing.T) { } devauth := NewDevAuth(&db, nil, nil, nil, Config{}) - _, _, err := devauth.SetAuthSetStatus( + err := devauth.SetAuthSetStatus( context.Background(), dummyDevID, dummyAuthID, model.DevStatusAccepted, ) @@ -1552,7 +1552,7 @@ func TestDevAuthRejectDevice(t *testing.T) { c.AssertNotCalled(t, "DeleteToken") } - _, _, err := devauth.SetAuthSetStatus( + err := devauth.SetAuthSetStatus( ctx, dummyDevID, dummyAuthID, model.DevStatusRejected, ) @@ -1781,7 +1781,7 @@ func TestDevAuthResetDevice(t *testing.T) { } devauth := NewDevAuth(&db, co, nil, nil, Config{}) - _, _, err := devauth.SetAuthSetStatus( + err := devauth.SetAuthSetStatus( context.Background(), dummyDevID, dummyAuthID, model.DevStatusPending, ) diff --git a/backend/services/deviceauth/devauth/mocks/App.go b/backend/services/deviceauth/devauth/mocks/App.go index f9444b6bc..b83c9608d 100644 --- a/backend/services/deviceauth/devauth/mocks/App.go +++ b/backend/services/deviceauth/devauth/mocks/App.go @@ -364,42 +364,21 @@ func (_m *App) RevokeToken(ctx context.Context, tokenID string) error { } // SetAuthSetStatus provides a mock function with given fields: ctx, deviceID, authID, status -func (_m *App) SetAuthSetStatus(ctx context.Context, deviceID string, authID string, status string) (*model.Device, *model.AuthSet, error) { +func (_m *App) SetAuthSetStatus(ctx context.Context, deviceID string, authID string, status string) error { ret := _m.Called(ctx, deviceID, authID, status) if len(ret) == 0 { panic("no return value specified for SetAuthSetStatus") } - var r0 *model.Device - var r1 *model.AuthSet - var r2 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, string) (*model.Device, *model.AuthSet, error)); ok { - return rf(ctx, deviceID, authID, status) - } - if rf, ok := ret.Get(0).(func(context.Context, string, string, string) *model.Device); ok { + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, string) error); ok { r0 = rf(ctx, deviceID, authID, status) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Device) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, string, string, string) *model.AuthSet); ok { - r1 = rf(ctx, deviceID, authID, status) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).(*model.AuthSet) - } - } - - if rf, ok := ret.Get(2).(func(context.Context, string, string, string) error); ok { - r2 = rf(ctx, deviceID, authID, status) - } else { - r2 = ret.Error(2) + r0 = ret.Error(0) } - return r0, r1, r2 + return r0 } // SetTenantLimit provides a mock function with given fields: ctx, tenant_id, limit From 2b7ecae1d50bc369c841ca81da58749739927fc3 Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Wed, 5 Aug 2026 13:45:11 +0200 Subject: [PATCH 11/13] refactor(deviceauth): consolidate auth set update: preauth -> accepted Signed-off-by: Alf-Rune Siqveland --- .../services/deviceauth/devauth/devauth.go | 99 +++++++------------ 1 file changed, 38 insertions(+), 61 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth.go b/backend/services/deviceauth/devauth/devauth.go index cc2b56581..541a84288 100644 --- a/backend/services/deviceauth/devauth/devauth.go +++ b/backend/services/deviceauth/devauth/devauth.go @@ -363,31 +363,10 @@ func (d *DevAuth) handlePreAuthDevice( return nil, ErrDevAuthUnauthorized } - if dev.Status != model.DevStatusAccepted { - // auth set is ok for auto-accepting, check device limit - allow, err := d.canAcceptDevice(ctx) - if err != nil { - return nil, err - } - - if !allow { - return nil, ErrMaxDeviceCountReached - } - } - - // Ensure that the old acceptable auth sets are rejected - if err := d.db.RejectAuthSetsForDevice(ctx, aset.DeviceId, aset.Id); err != nil && - !errors.Is(err, store.ErrAuthSetNotFound) { - return nil, errors.Wrap(err, "failed to reject auth sets") - } - update := model.AuthSetUpdate{ - Status: model.DevStatusAccepted, - } - // persist the 'accepted' status in both auth set, and device - if err := d.db.UpdateAuthSetById(ctx, aset.Id, update); err != nil { - return nil, errors.Wrap(err, "failed to update auth set status") + err = d.updateAuthSetStatus(ctx, aset, model.DevStatusAccepted) + if err != nil { + return nil, err } - aset.Status = model.DevStatusAccepted if err := d.updateDeviceStatus( ctx, @@ -398,8 +377,6 @@ func (d *DevAuth) handlePreAuthDevice( return nil, err } - aset.Status = model.DevStatusAccepted - dev.Status = model.DevStatusAccepted dev.AuthSets = append(dev.AuthSets, *aset) return aset, nil } @@ -865,66 +842,45 @@ func (d *DevAuth) deleteAuthSet(ctx context.Context, authSet *model.AuthSet) err } func (d *DevAuth) updateAuthSetStatus( - ctx context.Context, deviceID, authID, status string, -) (*model.AuthSet, error) { - aset, err := d.db.GetAuthSetById(ctx, authID) - if err != nil { - if err == store.ErrAuthSetNotFound { - return nil, err - } - return nil, errors.Wrap(err, "db get auth set error") - } - - if aset.DeviceId != deviceID { - return nil, ErrDevIdAuthIdMismatch - } - - if aset.Status == status { - return nil, nil - } - - // Validate status transition - err = model.ValidateStatusTransition(aset.Status, status) - if err != nil { - return nil, ErrDevAuthBadRequest - } - + ctx context.Context, aset *model.AuthSet, status string, +) error { if status == model.DevStatusAccepted { // if accepting an auth set allow, err := d.canAcceptDevice(ctx) if err != nil { - return nil, err + return err } if !allow { - return nil, ErrMaxDeviceCountReached + return ErrMaxDeviceCountReached } // reject all accepted auth sets for this device first - err = d.db.RejectAuthSetsForDevice(ctx, deviceID, aset.Id) + err = d.db.RejectAuthSetsForDevice(ctx, aset.DeviceId, aset.Id) if err != nil && err != store.ErrAuthSetNotFound { - return nil, errors.Wrap(err, "failed to reject auth sets") + return errors.Wrap(err, "failed to reject auth sets") } } else if aset.Status == model.DevStatusAccepted { // Authset transitions from accepted - err = d.cacheDeleteToken(ctx, deviceID) + err := d.cacheDeleteToken(ctx, aset.DeviceId) if err != nil { - return nil, errors.Wrapf(err, - "failed to delete token for %s from cache", deviceID) + return errors.Wrapf(err, + "failed to delete token for %s from cache", + aset.DeviceId) } deviceOID := oid.FromString(aset.DeviceId) // delete device token err = d.db.DeleteTokenByDevId(ctx, deviceOID) if err != nil && err != store.ErrTokenNotFound { - return nil, errors.Wrap(err, "db delete device token error") + return errors.Wrap(err, "db delete device token error") } } if err := d.db.UpdateAuthSetById(ctx, aset.Id, model.AuthSetUpdate{ Status: status, }); err != nil { - return nil, errors.Wrap(err, "db update device auth set error") + return errors.Wrap(err, "db update device auth set error") } aset.Status = status - return aset, nil + return nil } func (d *DevAuth) SetAuthSetStatus( @@ -933,8 +889,29 @@ func (d *DevAuth) SetAuthSetStatus( authID string, status string, ) error { + aset, err := d.db.GetAuthSetById(ctx, authID) + if err != nil { + if err == store.ErrAuthSetNotFound { + return err + } + return errors.Wrap(err, "db get auth set error") + } + + if aset.DeviceId != deviceID { + return ErrDevIdAuthIdMismatch + } + + if aset.Status == status { + // No-op + return nil + } + // Validate status transition + err = model.ValidateStatusTransition(aset.Status, status) + if err != nil { + return ErrDevAuthBadRequest + } - aset, err := d.updateAuthSetStatus(ctx, deviceID, authID, status) + err = d.updateAuthSetStatus(ctx, aset, status) if err != nil { return err } From a1554326afa7dd835b14c69056d6f01e94a670e2 Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Thu, 6 Aug 2026 13:49:24 +0200 Subject: [PATCH 12/13] test(deviceauth): fix TestDevAuthRejectDevice status assertions The test would bail out early due to the mock always returning the same status. Signed-off-by: Alf-Rune Siqveland --- .../deviceauth/devauth/devauth_test.go | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth_test.go b/backend/services/deviceauth/devauth/devauth_test.go index ec5525876..18c724f91 100644 --- a/backend/services/deviceauth/devauth/devauth_test.go +++ b/backend/services/deviceauth/devauth/devauth_test.go @@ -1423,6 +1423,8 @@ func TestDevAuthRejectDevice(t *testing.T) { dbErr error dbDelDevTokenErr error + submitJob bool + outErr string }{ { @@ -1431,6 +1433,7 @@ func TestDevAuthRejectDevice(t *testing.T) { DeviceId: dummyDevID, Status: model.DevStatusAccepted, }, + submitJob: true, }, { aset: &model.AuthSet{ @@ -1458,6 +1461,7 @@ func TestDevAuthRejectDevice(t *testing.T) { DeviceId: dummyDevID, Status: model.DevStatusAccepted, }, + submitJob: true, withCache: true, tenant: "acme", }, @@ -1472,6 +1476,7 @@ func TestDevAuthRejectDevice(t *testing.T) { DeviceId: dummyDevID, Status: model.DevStatusAccepted, }, + submitJob: true, dbDelDevTokenErr: store.ErrTokenNotFound, }, { @@ -1510,24 +1515,36 @@ func TestDevAuthRejectDevice(t *testing.T) { db.On("GetAuthSetById", ctx, dummyAuthID). Return(tc.aset, tc.dbErr) + + co := oas_mocks.NewMockWorkflowsOtherAPI(t) if tc.aset != nil { db.On("UpdateAuthSetById", ctx, tc.aset.Id, model.AuthSetUpdate{Status: model.DevStatusRejected}).Return(nil) db.On("GetDeviceById", ctx, mock.AnythingOfType("string")). - Return(&model.Device{Id: tc.aset.Id, Status: tc.aset.Status}, nil) + Return(&model.Device{Id: tc.aset.DeviceId, Status: tc.aset.Status}, nil) + db.On("UpdateDeviceWithRevision", ctx, + tc.aset.DeviceId, + uint(0), + mock.AnythingOfType("model.DeviceUpdate")).Return(nil) + db.On("GetDeviceStatus", ctx, + tc.aset.DeviceId). + Return(model.DevStatusRejected, nil) + db.On("DeleteTokenByDevId", ctx, + oid.FromString(tc.aset.DeviceId)). + Return(tc.dbDelDevTokenErr) + + if tc.submitJob { + req := client.ApiStartWorkflowRequest{ApiService: co} + co.On("StartWorkflow", ctx, "update_device_status"). + Run(func(args mock.Arguments) { + co.On("StartWorkflowExecute", mock.Anything). + Return(nil, mockResponseOK, nil) + }). + Return(req). + Once() + } } - db.On("DeleteTokenByDevId", ctx, - dummyDevUUID). - Return(tc.dbDelDevTokenErr) - db.On("GetDeviceStatus", ctx, - dummyDevID). - Return("accepted", nil) - db.On("UpdateDevice", ctx, - dummyDevID, - mock.AnythingOfType("model.DeviceUpdate")).Return(nil) - - co := oas_mocks.NewMockWorkflowsOtherAPI(t) devauth := NewDevAuth(&db, co, nil, nil, Config{}) From f3d5e5eb10aaaeb1fe77befa5582479da6df7c1b Mon Sep 17 00:00:00 2001 From: Alf-Rune Siqveland Date: Thu, 6 Aug 2026 13:58:51 +0200 Subject: [PATCH 13/13] test(deviceauth): fix TestDevAuthRejectDevice mocked new status Signed-off-by: Alf-Rune Siqveland --- backend/services/deviceauth/devauth/devauth_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/services/deviceauth/devauth/devauth_test.go b/backend/services/deviceauth/devauth/devauth_test.go index 18c724f91..cfa1c7559 100644 --- a/backend/services/deviceauth/devauth/devauth_test.go +++ b/backend/services/deviceauth/devauth/devauth_test.go @@ -1732,8 +1732,8 @@ func TestDevAuthResetDevice(t *testing.T) { DeviceId: dummyDevID, Status: "accepted", }, + submitJob: true, dbDelDevTokenErr: store.ErrTokenNotFound, - outErr: "db delete device token error: token not found", }, { aset: &model.AuthSet{ @@ -1771,7 +1771,7 @@ func TestDevAuthResetDevice(t *testing.T) { tc.dbDelDevTokenErr) db.On("GetDeviceStatus", context.Background(), dummyDevID).Return( - "accepted", nil) + model.DevStatusPending, nil) db.On("UpdateDeviceWithRevision", context.Background(), func() interface{} { if tc.aset != nil {