Arvados

Should the configuration parameters be published from the home API server or at the workbench level?

Updates at 95c228e25

• Added a javascript SHA1 library to be able to create salted tokens from the multi site search.
• Modified the SessionDB.login() function to use a salted token when necessary.
• Fixed SessionDB.fillMissingUUIDs() function to use user.owner_uuid as a source to get the remote cluster's prefix uuid instead of user.uuid that can be from another cluster.
• Added a way to pass a list of remote hosts to the workbench. Pending: auto-login to those remote hosts using a salted token if reviewer confirms that's a correct behavior.

Also pending: configuration to avoid adding sites not listed on remote_hosts via DNS fallback, should these 2 configurations (remote_hosts & remote_hosts_via_dns) be added to workbench as they are meant to be on the "home cluster" of a federation?

Testing procedure:

• Log into any of these clusters (let's say, c97qk) using the branch workbench.
• Go to /search url (multi site search) & check if 9tee4 is already added to the multi site search.
  • If that's the case, you should clear the local storage cache as it'll have conflicts with the new version.
• Go to the "add site" section, adding 9tee4.arvadosapi.com.
  • What's expected is that you'll log into 9tee4 using your c97qk's account.
    • If the account wasn't already created on 9tee4, it'll get created but not activated: You'll land on the "account inactive" page on 9tee4 and will need to be activated manually before retrying.
    • If the account was already created & active, you'll get logged in automatically without being asked for your login credentials, as the log in procedure is made using a salted token from c97qk

Can we offer the user the option of clearing local storage from the UI instead of forcing them to use the browser interface?

• Putting in just "9tee4" doesn't work.
• When you put in a bad hostname, it takes a long time to tell you that it doesn't work.
• It successfully creates a federated user on the remote cluster, but the remote account is inactive. There is no indication that the account is inactive.
• If you log out of the remote workbench, there is no way to log in again.
• Clicking through to the remote cluster takes you to the "not found, try logging in" page.
• The "sessions" page should have a button that goes to the remote workbench dashboard with your federated identity.

To be clear, the actual federated search seems to work, the main problem is integration with remote workbench.

Some notes related to discussions on chat and more:

• Prototype implementation aimed to take advantage of the salted token to login automatically using the same method that’s being used currently
  • This has the side effect of saving the user’s “normal” token on SessionDB instead of just using the salted token for searches. (as discussed on chat, this is not desirable)
• The app should ask (and cache) the API server for the local account token’s uuid on startup, to have it available for generating salted tokens.
• We need a way to distinguish between federated and non-federated clusters, as federated clusters just accept the salted token versus the other ones need a login procedure.
  • Maybe if we define that remote_hosts clusters are the ones that are federated and the ones added through the “add/remove site” UI are the non-federated clusters, it would be easy to implement because we don’t need to check if a federated search is successful before retrying a classic login on a manually added site.
  • Other way is getting the remote cluster discovery doc and see if the local one is listed on the remote_hosts setting, or if remote_hosts_via_dns is true. This implies a request delay.
• As discussed on the chat, discovery doc’s remote_hosts & remote_hosts_via_dns settings should be used to set up home cluster’s multi-site search app, taking into consideration that the home cluster would be also allowing logins from the listed clusters’ accounts.
• I suppose that the current behavior should be maintained when remote_hosts_via_dns is set to true, otherwise it shouldn’t allow adding remote sites that aren’t listed on remote_hosts, please confirm this.
• As Peter pointed out, links to results on federated workbenches should include ‘?api_token=v2/…” salted token so that the user can auto-login to the destination workbench when clicking.
• What happens if an old multi site search session has non-federated sessions already saved? This produces strange behavior on the current session management UI.
  • Also, this brings up the question: Should the user be able to have a federated & non-federated session to the same remote cluster?

Lucas Di Pentima wrote:

Some notes related to discussions on chat and more:

• Prototype implementation aimed to take advantage of the salted token to login automatically using the same method that’s being used currently
  • This has the side effect of saving the user’s “normal” token on SessionDB instead of just using the salted token for searches. (as discussed on chat, this is not desirable)

Yes, we should use salted tokens only, which means we need the token uuid.

• We need a way to distinguish between federated and non-federated clusters, as federated clusters just accept the salted token versus the other ones need a login procedure.

I think we attempt the salted token login, and if that fails, try normal login. If salted token login takes a long time to fail, that's something we need to address directly.

• As discussed on the chat, discovery doc’s remote_hosts & remote_hosts_via_dns settings should be used to set up home cluster’s multi-site search app, taking into consideration that the home cluster would be also allowing logins from the listed clusters’ accounts.

Yes, I think we agreed that was a simplifying assumption.

• I suppose that the current behavior should be maintained when remote_hosts_via_dns is set to true, otherwise it shouldn’t allow adding remote sites that aren’t listed on remote_hosts, please confirm this.

I agree.

• As Peter pointed out, links to results on federated workbenches should include ‘?api_token=v2/…” salted token so that the user can auto-login to the destination workbench when clicking.

Yes. I would also like to request we add a remote workbench link on the "sessions" page.

• What happens if an old multi site search session has non-federated sessions already saved? This produces strange behavior on the current session management UI.

Can we detect this and reset it? The user will just have to set it up again.

• Also, this brings up the question: Should the user be able to have a federated & non-federated session to the same remote cluster?

Probably not. Even if a user wants to have multiple accounts, this interface probably won't be the right way to manage it.

Further question from Peter's answers & confirmations:

• remote_hosts & remote_hosts_via_dns on the “home cluster” will be used to set up the multi site search app behavior. Current app behavior will be maintained when remote_hosts_via_dns=true.
• Links to remote workbenches will use api_token=v2/… on their urls and the app’s session page will also have a link to every workbench with it.

• We should use salted tokens only, so we need the token uuid: We could get it by querying the API server from the Mithril app (like it’s being done on the branch currently) or pass it through the mount tag when loading the app. Thoughts/preference?
• About using the salted token and having a fallback to the usual login method: The salted token method does not involve login in, so we would need to make some no-op operation with the salted token on app initialization to check if it works or mark the session as inactive instead of redirecting the browser to a login page when some search fails with the salted token, right?
• On app initialization, check if active non-federated sessions on SessionDB exists on clusters listed on remote_hosts. Reset any of these kind of sessions if the fact that some cluster is listed there implicitly tells the system that federated mode should be used.
• If federated & non-federated sessions shouldn’t happen on the same app, remote_hosts_via_dns setting will behave differently when remote_hosts is empty (non-federated case) versus when it has some value? Or do we need an additional boolean setting that selects federated / non-federated modes?

Lucas Di Pentima wrote:

Further question from Peter's answers & confirmations:

Clarified requirements:

• remote_hosts & remote_hosts_via_dns on the “home cluster” will be used to set up the multi site search app behavior. Current app behavior will be maintained when remote_hosts_via_dns=true.
• Links to remote workbenches will use api_token=v2/… on their urls and the app’s session page will also have a link to every workbench with it.

Questions:

• We should use salted tokens only, so we need the token uuid: We could get it by querying the API server from the Mithril app (like it’s being done on the branch currently) or pass it through the mount tag when loading the app. Thoughts/preference?

Querying it from Mithril is probably better, the less it relies on Rails the better.

• About using the salted token and having a fallback to the usual login method: The salted token method does not involve login in, so we would need to make some no-op operation with the salted token on app initialization to check if it works or mark the session as inactive instead of redirecting the browser to a login page when some search fails with the salted token, right?

It should request "/arvados/v1/users/current" to test if a salted token works.

• On app initialization, check if active non-federated sessions on SessionDB exists on clusters listed on remote_hosts. Reset any of these kind of sessions if the fact that some cluster is listed there implicitly tells the system that federated mode should be used.

That makes sense.

• If federated & non-federated sessions shouldn’t happen on the same app, remote_hosts_via_dns setting will behave differently when remote_hosts is empty (non-federated case) versus when it has some value? Or do we need an additional boolean setting that selects federated / non-federated modes?

I think having federated & non-federated sessions are okay, but there should only be one session per remote cluster. Also, for clusters listed in remote_hosts, only federated sessions should be allowed. We can always determine a given cluster's prefix by querying the discovery document.

Additional thought: what happens if a user visiting a foreign workbench with a remote token goes to multi-site search or /sessions? Maybe that page should only be available to local users?

Re token UUID: yes, any new API call we need to do should be done from the client, not the Rails app.

Re testing a salted token: yes, get /arvados/v1/users/current

Re auto-replacing old non-federated sessions: given the migration stuff, yes, it might be really useful to test a salted token on each remote, and if that works, replace the non-federated session with the salted token. One less opportunity for users to wonder why they're not getting any search results after the admin migrates their accounts.

Mixing federated and non-federated seems fine. I think we should support this.

Current behavior

Desired behavior

• If site is specifically listed in discovery.remoteHosts, add it to the sessions list
• If user types hostname into text box (or xyzzy as a shortcut for xyzzy.arvadosapi.com), add it to the sessions list
• For each site in the list, try a salted token
• If site accepts salted token, enable it, using the salted token
• If site listed in discovery.remoteHosts does not accept salted token, disable it (maybe an "enable" button just retries with the salted token?)
• If user-entered site does not accept salted token, and we have a stored remote session token, use the stored remote session token (just like old behavior)
• If user-entered site does not accept salted token, and we don't have a stored remote session token, offer a "Log in to remote" button

Working on this story I think I might have stumbled into a (couple?) bug:

Having a salted token on an inactive account allows me to re-active it by using the token to login to workbench. Let's say I have clusterH (home) & clusterR (remote), and my clusterH account on clusterR is inactive, if I go to the link https://workbench.clusterR.arvadosapi.com/?api_token=v2/clusterHsaltedtoken, it logs me in without any issue, and the account is suddenly active.

Also, I was able to do a search using the salted token on an inactive account.

On a process note, I know we all hate adding requirements in the review process, but linking between home and remote workbench instances is an absolutely critical aspect of the story that was overlooked in the original story description. Without it, federated identity / remote search is unusable.

Updates at 0e3c1f9c7

• Do federated logins by requesting the current user object instead of the classic login flow.
• When the user is inactive on the remote cluster, label the session properly and exclude it from SessionDB.loadActive()'s results.
• Show a link to the remote workbench on the sessions page.
• On page load, detect those non-federated active sessions and try to migrate them to federated logins just in case they're stale sessions. If the federated login doesn't succeed, keep the session's previous state.
• Removed workbench remote_hosts config.
• Added auto loading of remote hosts listed on home cluster's api discovery doc. (WIP - there are some UI issues as this operation is asynchronous and the search and sessions pages don't always update correctly)

Peter Amstutz wrote:

What happens if you visit the /sessions page as a federated user on a remote cluster? It should probably say something like "can only set sessions on home cluster" and give you a link to your home workbench.

I tried this, and the results are incredibly confusing.

1. I logged in to c97qk
2. I added 9tee4 as a federated identity
3. I went to the 9tee4 workbench, and clicked on multi-site search. Despite being on the 9tee4 workbench, only "c97qk" showed up as a search option.
4. I tried adding 9tee4 on the remote session. (I realize now, since the remote workbench isn't running this branch, it isn't federation aware)
5. The sessions page reloaded showing that it would search of 9tee4 but not c97qk. On subsequent investigation I realized that adding 9tee4 had the result of logging me in as a local user to 9tee4 (because I already have a user account.)

I think even if both clusters were federation aware, we would still end up with some strange behavior because the federated user on the remote workbench can't do federated logins to other clusters (it doesn't have access to the original token). Then it falls back to regular login which results in being logged in as a different user, and not the federated user you intended.

The search links are missing "?api_token=" so if you log out from a remote workbench and then try to follow the search link it won't work.

When you are a federated user on a remote workbench, the /search and /sessions pages should either say "click here to go to your home cluster" or just automatically redirect there.

Updates at 0312a5e5a

• Remote search result links now have ?api_token=v2/... on their URL.
• Automatic redirection when a remote user tries to get to /search or /sessions pages.

Updates at 1b350dc78

• When using just a uuid prefix on the "Log in" box, append .arvadosapi.com before trying to get the API server url.
• Remotes listed on remoteHosts show "enable/disable" action button instead of "Log in/Log out".
• When a listed federated remote rejects the salted token, the session shows as disabled.
• When a user-entered (ie: not listed) federated remote rejects the salted token, classic login is attempted.

To test automatic redirection described on note-31, you can have the branch workbench pointing to c97qk, add 9tee4 as a remote, and get one of the search result links pointing to 9tee4, then restart workbench pointing to 9tee4, modify the link accordingly to use it with localhost and log in using the salted token. When going to /search or /sessions you'll get redirected to workbench.c97qk.arvadosapi.com/....
I tried to set up 2 wb instances on localhost IP addresses and make the /etc/hosts file to point the real domains to the local IPs but couldn't make it work properly.

Did a quick test, everything works.

It looks like migrateNonFederatedSessions happens on every page load. Would it be easy enough to run it only when the "sessions" page is loaded -- e.g., in SessionsTable.oninit? (Unless I'm forgetting something, it nearly always ends up achieving nothing, and even in cases where a session is migrate-able, it's not really necessary to migrate right away, because the old session will still work. The only real reason for migrating is to make the /sessions page stay consistent across all users as clusters get upgraded/reconfigured. Is that right?)

• use ajax reqs to check for an existing session and, if needed, create one (this might be hard/impossible due to browser security model, and might involve some CORS support on the remote workbench)
• drop the query string from the href attr (so "copy link" doesn't have it), but intercept the onclick handler to pass it when clicking the link (this might be hard to do without breaking shift/ctrl-click)
• pass the cluster ID or API host, but not a token. The remote workbench can handle the "no session" case by initiating login through the indicated cluster. Pasted links would behave strangely for users who don't have an account on the same "home" cluster, but at least they wouldn't be a gaping security hole.
• link to a local page (without a token), and have the local page add the ?api_token= part and redirect.

A session for a remote site that's listed in remote_hosts should not offer a "Remove" button. (Currently, I can hit "remove", and it disappears from the list, but then it gets re-added automatically when I reload.)

"Disable" doesn't survive reload. Maybe sessions need a "disabled" flag to remember this state, or we need to avoid automatically adding a salted token to a session whose token has been cleared? Alternatively, we could just hide the button entirely -- but this would mean a user can't disable an unresponsive/irrelevant cluster that's interfering with searches.

Could we call db.autoRedirectToHomeCluster() from SessionsTable.oninit and Search.oninit, which already have a db, rather than adding new Rails html templates and creating additional SessionDB objects?

Currently SearchTable.view instantiates a SessionDB. This means it runs in every render cycle! But Search.oninit already creates a session db and keeps it around for the life of the page. Instead of adding all the session stuff in SearchTable, just have Search pass along the appropriate tokenParam the same way it passes along objectType and workbenchBaseURL.

(TBC. I haven't had a proper look at session_db.js yet.)

Regarding ?api_token, what about using a form post button instead of a link? It would supply api_token and _method=GET as hidden fields.

Peter Amstutz wrote:

Regarding ?api_token, what about using a form post button instead of a link? It would supply api_token and _method=GET as hidden fields.

I think this is the easiest fix. Drawback: it would also prevent the "copy/paste link" operation entirely, as well as breaking shift/ctrl-click.

apiHostname.indexOf('arvadosapi.com') >= 0 ...should be... apiHostname.endsWith('.arvadosapi.com')

session.token.indexOf('v2/') < 0 is being used as a test for "not a salted token" (therefore try migrating), but it really only tests token format, so it'll stop working when the remote starts handing out v2 tokens. Maybe the condition we're really interested in here is session.token != saltedToken(uuidPrefix) ...?

Although session.user.owner_uuid.slice(0,5) should be equal to it anyway, would it be clearer to use localDD.uuidPrefix?

Updates at 71e1465f6
Test run: https://ci.curoverse.com/job/developer-run-tests/570/

• Changed search results links to form buttons.
• Addressed comments on note-37.
• Removed useless local workbench link on the /sessions page.

Updates at 80eedd862

Replaced javascript events with proper form values on the search results form buttons.

Even though we are fairly certain tokenParam will never have square brackets etc., we should encourage good habits:

- m('input[type=hidden][name=api_token][value='+tokenParam+']'),
+ m('input[type=hidden][name=api_token]', {value: tokenParam}),

• has the same copy-paste-secret problem again (let's solve it the same way)
• has a tooltip that basically lies about the link target, which seems weird
• seems misplaced/unnecessary. If we need a link, we can add an explicit "visit remote Workbench" button somewhere, but I don't think overloading the "cluster ID" label is the way to do it. The point of this page is to use multiple clusters from a single Workbench. What's the motivation for having such a link?

Tom Clegg wrote:

What's the motivation for having such a link?

1. The "click Show button/link, despite note having a session at the remote workbench" scenario should continue to work (in the non-federated case, we rely on the remote workbench to initiate a login in this case, but that doesn't work yet with federated sites, especially without #12958)
2. The "multi-site search" button should do something sensible on a remote workbench (i.e., when the Rails session token is itself a salted token). We already had a separate bug #12959 for this, but once we started setting up federated-token sessions #12959 became a blocker, so fixing it right here seemed even better.

Updates at 3c31a5e64
Test run: https://ci.curoverse.com/job/developer-run-tests/575/

• Removed workbench links on /sessions page.
• Only try to migrate non federated sessions on /sessions page instead of everywhere.
• autoLoadRemoteHosts() only loads missing remotes, without reactivating disabled sessions.
• Avoid instantiating new SessionDB objects before calling autoRedirectToHomeCluster().

LGTM

Branch 11454-remove-button-removal - 9333d6053
Test run: https://ci.curoverse.com/job/developer-run-tests/588/

This is a small update to not show the "Remove" session button on /sessions when the remote is listed on remoteHosts.

Not a huge fan of the nested ternary. How about something like:

-                            m('td', session.isFromRails ? null :
-                                session.listedHost ? null :
-                                m('button.btn.btn-xs.btn-default', {
+                            m('td', (session.isFromRails || session.listedHost) ? null :
+                                m('button.btn.btn-xs.btn-default', {

or

+                            m('td', !session.isFromRails && !session.listedHost &&
+                                m('button.btn.btn-xs.btn-default', {

Update at 9c699bfa8

The first option is better, IMO.

Test run successful: https://ci.curoverse.com/job/developer-run-tests/589/