Story #15577

[API] User ownership reassign

Added by Peter Amstutz 9 months ago. Updated 4 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
11/08/2019
Due date:
% Done:

100%

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

Description

For case where a user leaves the organization and her Arvados stuff needs to be given to someone else.

Admin utility script and/or Workbench 2 feature. Use merge API call to assign ownership of departing user's stuff to new user (must be done on each cluster?). In the API server, must allow redirect_to_user_uuid=nil, in which case ApiClientAuthorization (API tokens) and AuthorizedKey (ssh keys) are not migrated, because the user that departed may have still have credentials and shouldn't gain access to other account.


Subtasks

Task #15703: Review 15577-ownership-transferResolvedPeter Amstutz


Related issues

Related to Arvados - Feature #15531: [SDK] Migrate federation to central LoginClusterResolved09/23/2019

Associated revisions

Revision a1f06e4e
Added by Peter Amstutz 6 months ago

Merge branch '15577-ownership-transfer' refs #15577

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

History

#1 Updated by Peter Amstutz 9 months ago

  • Description updated (diff)

#2 Updated by Tom Clegg 8 months ago

  • Related to Feature #15531: [SDK] Migrate federation to central LoginCluster added

#3 Updated by Tom Clegg 8 months ago

The required API functionality is being added in #15531. Once that's done, this may be a matter of documenting a curl/arv command for admins to use.

#4 Updated by Peter Amstutz 8 months ago

  • Story points set to 1.0

#5 Updated by Peter Amstutz 8 months ago

  • Target version changed from To Be Groomed to 2019-10-23 Sprint

#6 Updated by Peter Amstutz 8 months ago

  • Assigned To set to Peter Amstutz

#7 Updated by Peter Amstutz 7 months ago

  • Target version changed from 2019-10-23 Sprint to 2019-11-06 Sprint

#9 Updated by Peter Amstutz 7 months ago

This is soft-blocked on #15762. In the course of updating the user management documentation I discovered some bugs. I have an in-progress documentation branch but it seems like it would make sense to fix the bugs first so that the documentation doesn't have to include complicated explanations of workarounds.

#10 Updated by Peter Amstutz 7 months ago

  • Status changed from New to In Progress

#11 Updated by Peter Amstutz 7 months ago

  • Target version changed from 2019-11-06 Sprint to 2019-11-20 Sprint

#12 Updated by Peter Amstutz 7 months ago

15577-ownership-transfer is ready for a first look. This is entirely a documentation branch, focused on updating and filling in the gaps on the topic of admin user and group management.

#13 Updated by Eric Biagiotti 7 months ago

Peter Amstutz wrote:

15577-ownership-transfer is ready for a first look. This is entirely a documentation branch, focused on updating and filling in the gaps on the topic of admin user and group management.

Overall, a great update so far! Sorry for the long list. If you think there is a better way to review big doc updates, let me know.

  • doc/Rakefile - Is there an arvbox update missing? How is this change used?
  • admin/troubleshooting - I think the title that shows up in the panel on the left should more closely match what someone might be looking for. So, "Request ids and logging" might not be very helpful. What about just "Error Logging"?
  • admin/troubleshooting, admin/activation - What do you think about changing the names of the files to better match the titles? Could be a bit confusing the way it is now when passing links around.
  • install/cheat_sheet - Very confusing file name, and is actually an admin page not an install page.
  • admin/migrating-providers - I think "Matching on email address" and "Link accounts" should be under another heading like "Migrating to a new account provider". Maybe the page should be called "Changing account providers"?
  • api/methods/users
    • Might as well remove the arguments table under system.
    • Should the user argument under update be marked as required?
    • For the activate and unsetup methods, maybe add a sentence about what it means to be activated/deactivated, or link to somewhere that explains it.
  • admin/activation
    • The first sentence under "User activation" should be replaced with an intro sentence like "This section describes the different user states, permissions ...".
    • I think the diagram should come after the list or be removed. The list is a lot more clear to me. If we keep the diagram, the ability to go from step 3 to 5 via setting "is_active" needs to be expressed.
    • In step 2, mention that these are cluster config values and link to the example config.
    • In step 7, the link should say "ownership reassigned". There are also 2 periods after that link.
    • line 58, "User management can be performed through the web using Workbench"
    • line 62, "user" is repeated.
    • "This is executed as a system user, so it bypasses normal read permission checks." Not sure what this means. Is this referring to the user_agreements list method? Is this an implementation detail, can it be removed?
    • line 102, incomplete sentence.
    • The "Auto-setup federated users from trusted clusters" section could use a config example. The second sentence is confusing and implies that is_active is referring to the cluster. Can this be reworked?
  • api/methods/user_agreements - Remove the empty arguments tables for list and signatures?
  • admin/reassign-ownership - The title can probably be shortened to "Reassign user data ownership".
  • config/config.default.yml - Don't we have to rerun generate if this is changed?
  • Also, please run tests.

Side note: We mention using workbench a lot. It's not great that we don't have user guide docs to point to that shows how to do some of this. Ideally once transitioned to WB2, we would have more complete docs for this. In the mean time, I think it is worth adding tickets for various gaps related to 15577 (e.g. WB2 user management docs) and adding TODO comments in the textile.liquid files where links to the user guide docs would be appropriate. This way we will have clear way to tie the docs together once they are done.

#14 Updated by Peter Amstutz 6 months ago

Eric Biagiotti wrote:

Peter Amstutz wrote:

15577-ownership-transfer is ready for a first look. This is entirely a documentation branch, focused on updating and filling in the gaps on the topic of admin user and group management.

Overall, a great update so far! Sorry for the long list. If you think there is a better way to review big doc updates, let me know.

  • doc/Rakefile - Is there an arvbox update missing? How is this change used?

Just a way to avoid waiting for R SDK docs being regenerated every time "rake" is run.

  • admin/troubleshooting - I think the title that shows up in the panel on the left should more closely match what someone might be looking for. So, "Request ids and logging" might not be very helpful. What about just "Error Logging"?

Just "Logging" now

  • admin/troubleshooting, admin/activation - What do you think about changing the names of the files to better match the titles? Could be a bit confusing the way it is now when passing links around.
  • install/cheat_sheet - Very confusing file name, and is actually an admin page not an install page.

The goal was to avoid breaking existing links to individual documentation pages, but it looks like I can add symlinks (and tweak the doc template) to do the right thing.

  • admin/migrating-providers - I think "Matching on email address" and "Link accounts" should be under another heading like "Migrating to a new account provider". Maybe the page should be called "Changing account providers"?

Now "Changing upstream login providers"

  • api/methods/users
    • Might as well remove the arguments table under system.
    • Should the user argument under update be marked as required?
    • For the activate and unsetup methods, maybe add a sentence about what it means to be activated/deactivated, or link to somewhere that explains it.

I prefer to keep the arguments table, even if empty, for consistency, since it makes it clear there arn't any parameters.

Technically, the 'users' object on update isn't required, because you can do an update with just "is_active=false" and the API server will figure out that is meant to be part of the 'users' parameter.

  • admin/activation
    • The first sentence under "User activation" should be replaced with an intro sentence like "This section describes the different user states, permissions ...".

Done.

  • I think the diagram should come after the list or be removed. The list is a lot more clear to me. If we keep the diagram, the ability to go from step 3 to 5 via setting "is_active" needs to be expressed.

The list is supposed to the details explanation of the diagram. Each numbered step corresponds to a box on the diagram. The ability to go from step 3 to 5 is documented on step 6.

Obviously that wasn't clear in the presentation. I changed it to a side-by-side layout to try and make the connection more obvious.

  • In step 2, mention that these are cluster config values and link to the example config.
  • In step 7, the link should say "ownership reassigned". There are also 2 periods after that link.
  • line 58, "User management can be performed through the web using Workbench"
  • line 62, "user" is repeated.

Done.

  • "This is executed as a system user, so it bypasses normal read permission checks." Not sure what this means. Is this referring to the user_agreements list method? Is this an implementation detail, can it be removed?

Took it out.

  • line 102, incomplete sentence.

Fixed.

  • The "Auto-setup federated users from trusted clusters" section could use a config example. The second sentence is confusing and implies that is_active is referring to the cluster. Can this be reworked?

Reworded this and added a link to the section further down with a config example.

  • api/methods/user_agreements - Remove the empty arguments tables for list and signatures?
  • admin/reassign-ownership - The title can probably be shortened to "Reassign user data ownership".

Done.

  • config/config.default.yml - Don't we have to rerun generate if this is changed?

Fixed.

  • Also, please run tests.

Right, that catches the need to run "go generate" and it appears some workbench integration test might be wonky.

Side note: We mention using workbench a lot. It's not great that we don't have user guide docs to point to that shows how to do some of this. Ideally once transitioned to WB2, we would have more complete docs for this. In the mean time, I think it is worth adding tickets for various gaps related to 15577 (e.g. WB2 user management docs) and adding TODO comments in the textile.liquid files where links to the user guide docs would be appropriate. This way we will have clear way to tie the docs together once they are done.

I added a TODO comment on the user-management file and filed #15834 .

15577-ownership-transfer @ ee341f1fc2579fdf28a8b8f07918f9ae195b0727

https://ci.curoverse.com/job/developer-run-tests/1653/

#15 Updated by Eric Biagiotti 6 months ago

My only nit is that the user activation numbered list formatting could be cleaned up a bit. I think there are some unnecessary newlines (steps 6 and 7) and step 2 is kind of a list...should we make it one? And hopefully as a side bonus there won't be text from the steps under the diagram :)

But otherwise, this LGTM. Thanks!

Passing tests are here: https://ci.curoverse.com/job/developer-run-tests/1654/

#16 Updated by Peter Amstutz 6 months ago

  • Status changed from In Progress to Resolved

#17 Updated by Peter Amstutz 4 months ago

  • Release set to 22

Also available in: Atom PDF