Bug #16387

arv-federation-migrate-test failing: can't update is_active to false on cached user record

Added by Peter Amstutz 2 months ago. Updated about 1 month ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
05/04/2020
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

This seems to be a regression

https://ci.arvados.org/job/arv-federation-migrate-test/11/console

HttpError 500 when requesting https://172.17.0.4:8000/arvados/v1/users?alt=json returned "error updating local user records: request failed: http://localhost:8004/arvados/v1/users/batch_update: 422 Unprocessable Entity: #<ActiveRecord::RecordInvalid: Validation failed: Is active cannot be set to false directly, use the 'Deactivate' button on Workbench, or the 'unsetup' API call

Subtasks

Task #16404: Review 16387-batch-update-deactivated-userResolvedPeter Amstutz

Associated revisions

Revision 6762d150
Added by Tom Clegg about 2 months ago

Merge branch '16387-batch-update-deactivated-user'

fixes #16387

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <>

History

#1 Updated by Peter Amstutz 2 months ago

  • Description updated (diff)

#2 Updated by Peter Amstutz 2 months ago

  • Assigned To set to Tom Clegg

#3 Updated by Tom Clegg 2 months ago

  • Status changed from New to In Progress
  • Subject changed from arv-federation-migrate-test failing to arv-federation-migrate-test failing: can't update is_active to false on cached user record

#4 Updated by Tom Clegg 2 months ago

  • Description updated (diff)

#6 Updated by Peter Amstutz about 2 months ago

Tom Clegg wrote:

16387-batch-update-deactivated-user @ 95e9d44b567a92664939f0f89bd45eedd6db67ab -- https://ci.arvados.org/view/Developer/job/developer-run-tests/1833/

Ok, so the logic is, remote users are not subject to the check "must unsetup to deactivate", but remote users are not allowed to call "activate" to reactivate themselves.

The one case this breaks is the case where you are using your remote identity to log into a federated cluster, and that federated cluster has a clickthrough agreement. In that case, you have to "sign" the agreement and then you can self-activate.

However most (all?) federations are in a trusted configuration (either using LoginCluster, or NewUsersAreActive is true, or ActivateUsers is true for that cluster) they go directly to 'active' which skips signing the clickthrough agreement.

Workbench2 also currently doesn't support displaying clickthrough agreements.

It's worth noting that ApiClientAuthorization#validate does this:

        if user.is_active && !remote_user['is_active']
          user.unsetup
        end

So I think it would be less disruptive for batch_update simply turn is_active=false into "unsetup" eg

      if u.is_active && !attrs['is_active']
        u.unsetup
      end
      u.update_attributes!(nullify_attrs(attrs))
      @objects << u

#7 Updated by Peter Amstutz about 2 months ago

It might make sense to disallow self activate when LoginCluster is set (and ClusterID != LoginCluster).

#8 Updated by Tom Clegg about 2 months ago

How about just changing the new rule from "if uuid belongs to remote" to "if uuid belongs to remote LoginCluster"? (...then can't self-activate, and can change to is_active=false in a batch update)

I'm wary of changing more than we need to in a bugfix slated for a patchlevel release.

#9 Updated by Peter Amstutz about 2 months ago

Tom Clegg wrote:

How about just changing the new rule from "if uuid belongs to remote" to "if uuid belongs to remote LoginCluster"? (...then can't self-activate, and can change to is_active=false in a batch update)

I'm wary of changing more than we need to in a bugfix slated for a patchlevel release.

Ok, that works.

#10 Updated by Tom Clegg about 2 months ago

  • Target version changed from 2020-05-06 Sprint to 2020-05-20 Sprint

#13 Updated by Anonymous about 2 months ago

  • Status changed from In Progress to Resolved

#14 Updated by Peter Amstutz about 1 month ago

  • Release set to 33

Also available in: Atom PDF