Arvados

12995-wb-merge-acct @ 4aa2e9342254971e92b5836a56728015e9cfc714

Adds a /users/link_account page accessible from user menu and on the "inactive user" page.

• User can choose whether to add a login to the current account, or link the current login to another account
• When one of the accounts is inactive, can only merge the login of the inactive account to the active account

Manually tested:

• From existing user, select "Add another login to this account", login as the "new" user, then link accounts.
• From new, active user, select "Use this login to access another account", login as "old" user, then link accounts
• From new, inactive user, select "Use this login to access another account", login as "old" user, then link accounts.

No automated tests. The problem is, this process relies on the SSO server, which the run-tests.sh / workbench test environment doesn't provide. Hard to work around. Could possibly rig something up based on arvbox.

Also:

12995-session-timeout @ commit:21f2ee32fe8fc6391c95b5dcdb59d787629dceff branch on the sso-provider repository.

• Sets session timeout to 1 second so that users always have to log in (otherwise sessions mess up the "log in as a different user" part of the flow.)

• Maybe it would be convenient to add a date on the group name to avoid possible conflicts when the merge action fails after the group is created, or just reuse it.
• Using either direction, I get the api error: "supplied API token is not from a trusted client" - API Request URL: https://172.17.0.2:8000/arvados/v1/users/merge
• Tests done with arvbox

• After adding the trusted client setting, manual tests on both directions worked well.
• The first bulletpoint from note-16 may be a valid one, just in case there's some transient api server problem.
• Is there an explicit way for workbench to ask for the new sso-provider (package dependency)?
• Issues that may not be for this review but instead for the api server side:
  • When linking an admin account into a non-admin account left me without admin access
  • After linking account A into account B, and later on linking account B into account C, when try to login with account A, I get an error like this: {"errors":["#<Exception: identity_url h793v-tpzed-f1svf5ts12yw4c3 redirects to nonexistent uuid 2bs4c-tpzed-2cx7fuz2l783jhi>"],"error_token":"1526663055+db8a355d"}
• After clicking any of the linking buttons, the next thing the user sees is a login dialog saying "Your session expired, please sign in again to continue." and I think it can be confusing, if this is not worth correcting on the SSO's side, maybe we could have a message on wb's link account page warning what will happen after clicking.

A→B + B→C = error during login

Can the merge API detect this when B→C is happening, and flatten the tree by changing A→B to A→C?

admin→non-admin

Yes, Workbench should warn if you're about to do this, but I suppose we might as well go ahead and do it if you accept/ignore the warning.

Lucas Di Pentima wrote:

• The first bulletpoint from note-16 may be a valid one, just in case there's some transient api server problem.

Added ensure_unique_name: true

• Is there an explicit way for workbench to ask for the new sso-provider (package dependency)?

No, there isn't.

• Issues that may not be for this review but instead for the api server side:
  • When linking an admin account into a non-admin account left me without admin access

Fixed workbench so it detects that case and won't let you do that.

• After linking account A into account B, and later on linking account B into account C, when try to login with account A, I get an error like this: {"errors":["#<Exception: identity_url h793v-tpzed-f1svf5ts12yw4c3 redirects to nonexistent uuid 2bs4c-tpzed-2cx7fuz2l783jhi>"],"error_token":"1526663055+db8a355d"}

This was an API server bug in following chained redirects. Fixed & added tests.

• After clicking any of the linking buttons, the next thing the user sees is a login dialog saying "Your session expired, please sign in again to continue." and I think it can be confusing, if this is not worth correcting on the SSO's side, maybe we could have a message on wb's link account page warning what will happen after clicking.

I noticed that too. I don't really want to mess with the SSO server too much. It might be possible to change the flow to have it go through the logout procedure and then immediately to the login procedure, which would also eliminate the need to upgrade SSO server.

Now @ 6237a718e292de02dc06c2885e4a96260616ce03

Still todo: add a documentation page.

12995-wb-merge-acct e2632a25d3aab230bdc44936fa42a3d27ff15d30

• Added username to the link accounts page to further disambiguate the two accounts being linked
• Added documentation pages
• Updated to use a login/logout flow instead of relying on session timeout. However, the SSO server still needs to be updated (see below)

12995-session-timeout 4ed380efb64d50a6c74defe74ffef08166f4f0c7

• Added a "choose" page when more than one provider is configured. Enables users to select between LDAP and Google.

Fix tests @ 7f09dd101fd16830a6e7ebd6dee0df7aa023c9e6

Running tests here:

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

Manually tested updates on both branches, providing that Jenkins' run goes well, LGTM. Thanks!