Bug #16914

[federation] [login cluster] at satellite cluster, log into inactive account on login cluster

Added by Ward Vandewege about 1 year ago. Updated about 1 year ago.

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

100%

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

Description

Click "log in" on tordo, using an account that exists on ce8i5 but has is_active false:

https://workbench.tordo.arvadosapi.com/users#

Oh... fiddlesticks.
An error occurred when Workbench sent a request to the Arvados API server. Try reloading this page. If the problem is temporary, your request might go through next time. If that doesn't work, the information below can help system administrators track down the problem.

API request URL
https://tordo.arvadosapi.com/arvados/v1/users/ce8i5-tpzed-tn806jsy8sjhmpv/activate
API response {
":errors":[
"request failed: https://ce8i5.arvadosapi.com/arvados/v1/users/ce8i5-tpzed-tn806jsy8sjhmpv/activate: 422 Unprocessable Entity: request failed: http://localhost:8004/arvados/v1/users/ce8i5-tpzed-tn806jsy8sjhmpv/activate: 422 Unprocessable Entity: #<ArgumentError: Cannot activate without being invited.> (req-p695alwdpuif0g9mrg09)"
]
}
Report problem or email us if you suspect this is a bug.


Subtasks

Task #16918: Review 16914-inactive-userResolvedPeter Amstutz

Associated revisions

Revision bc25855a
Added by Peter Amstutz about 1 year ago

Merge branch '16914-inactive-user' closes #16914

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

Revision 2a1a1439 (diff)
Added by Peter Amstutz about 1 year ago

UserGet() passes "select" option along to batch update refs #16914

Fixes issue of user being deactivated because something called "get
user" and selected fields didn't include is_active.

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

History

#1 Updated by Peter Amstutz about 1 year ago

Test with both wb1 and wb2, failure to activate should land them at the inactive user page, no error.

#2 Updated by Peter Amstutz about 1 year ago

  • Assigned To set to Peter Amstutz

#3 Updated by Peter Amstutz about 1 year ago

  • Status changed from New to In Progress

#4 Updated by Peter Amstutz about 1 year ago

This is weird because workbench is getting is_invited true (which should allow the user to self-activate) but when the user tries to activate, they get "cannot activate because not invited".

I don't know where "is_invited: true" is coming from. When I use "arv", the same field is "false" as I would expect.

#5 Updated by Peter Amstutz about 1 year ago

A few notes.

The "is_invited" flag is returned for the local cluster, not the LoginCluster. That's because it isn't a real field, it is a virtual field defined as:

self.is_active ||
Rails.configuration.Users.NewUsersAreActive ||
self.groups_i_can(:read).select { |x| x.match(/-f+$/) }.first

User is "invited" if they are active, or NewUsersAreActive is set, or they have been granted permission to read things shared with the "all users" group. An "invited" user can self-activate.

When a user is "set up", it sets that last permission.

AutoSetupUser is "true" on tordo but "false" on ce8i5.

This creates a situation where users on tordo are "set up" and invited (but not active), but not invited on ce8i5. Since ce8i5 is the login cluster which calls the shots, the user shouldn't be "set up" or invited on tordo at all.

If the user tries to activate themselves on tordo, this fails because they are not invited on ce8i5. But the "is_invited" flag is how workbench decides when to try to self activate, so it gets stuck in a loop.

On one hand this is "just" a configuration error. However, we should handle it better because it is very hard to debug.

It checks if the user should be unsetup before user.save! in ApiClientAuthorization#validate. But if the user is a new user, they will be set up in an after_create hook, which doesn't happen until user.save!. For the case where we need to unsetup the user, it probably needs to happen after user.save!.

The config check should probably also issue a warning that AutoSetupNewUsers will be ignored on satellite cluster.

#7 Updated by Lucas Di Pentima about 1 year ago

This LGTM, please merge.

#8 Updated by Peter Amstutz about 1 year ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved

#9 Updated by Peter Amstutz about 1 year ago

16914-inactive-user @ arvados|1a343a507d7e4bdf56bc5e6108996f0894b11a9c

Fix bug introduced by previous merge, the UserGet method didn't pass along which fields were selected, as a result if is_active had not been selected, it would send the default value of is_active (false).

Includes a couple of other cleanups and additional testing.

https://ci.arvados.org/view/Developer/job/developer-run-tests/2125/

#10 Updated by Peter Amstutz about 1 year ago

  • Status changed from Resolved to In Progress

#11 Updated by Lucas Di Pentima about 1 year ago

Updates at 1a343a5 LGTM, thanks for the detailed testing!

#12 Updated by Peter Amstutz about 1 year ago

  • Status changed from In Progress to Resolved

#13 Updated by Peter Amstutz about 1 year ago

  • Release set to 25

Also available in: Atom PDF