Project

General

Profile

Actions

Idea #11454

closed

Support federated search across a set of Arvados clusters

Added by Tom Morris over 7 years ago. Updated over 6 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
04/11/2017
Due date:
Story points:
2.0
Release relationship:
Auto

Description

System administrator can configure a set of API servers which are part of a federation and which should be contacted to do federated searches.

Workbench uses that list to send search queries to each of the API servers in parallel and displays all search hits federation wide to the user.

Permissions are checked based on the user identity in the same way that they are today for a local user (so this depends on Federated Identity).

Configuration parameters:
- list of clusters in the federation
- enable/disable ability to add additional clusters which are not included in the list (DNS fallback)

Ability to add public clusters is disabled, the default set of clusters searched is the list in the config.


Subtasks 1 (0 open1 closed)

Task #12776: Review 11454-wb-federated-searchResolvedTom Clegg04/11/2017Actions

Related issues

Related to Arvados - Bug #13020: Login into workbench with a salted token activates a previously inactive federated accountDuplicateLucas Di Pentima10/30/2019Actions
Related to Arvados - Bug #12959: [Federation] Wrong cluster ID shown on multisite search pageResolvedTom Clegg02/12/2018Actions
Related to Arvados - Feature #12958: [Federation] Workbench login chooserClosedActions
Blocked by Arvados - Idea #11453: Federated user identity which works across a network of Arvados clustersClosedTom Clegg06/20/2017Actions
Actions #1

Updated by Tom Morris over 7 years ago

  • Description updated (diff)
Actions #2

Updated by Tom Morris about 7 years ago

  • Target version set to Arvados Future Sprints
Actions #3

Updated by Tom Morris about 7 years ago

  • Description updated (diff)
  • Story points set to 2.0
Actions #4

Updated by Tom Morris almost 7 years ago

  • Target version changed from Arvados Future Sprints to 2017-12-20 Sprint
Actions #5

Updated by Lucas Di Pentima almost 7 years ago

  • Assigned To set to Lucas Di Pentima
Actions #6

Updated by Lucas Di Pentima almost 7 years ago

  • Target version changed from 2017-12-20 Sprint to 2018-01-17 Sprint
Actions #7

Updated by Lucas Di Pentima almost 7 years ago

  • Status changed from New to In Progress
Actions #8

Updated by Lucas Di Pentima almost 7 years ago

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

Actions #9

Updated by Lucas Di Pentima almost 7 years ago

  • Target version changed from 2018-01-17 Sprint to 2018-01-31 Sprint
Actions #10

Updated by Lucas Di Pentima almost 7 years ago

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.
Actions #11

Updated by Lucas Di Pentima almost 7 years ago

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?

Actions #12

Updated by Lucas Di Pentima almost 7 years ago

Testing procedure:

Currently 9tee4 & c97qk are configured with federation enabled (see #12956), both clusters accept accounts each other.
  • 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
Actions #13

Updated by Tom Morris almost 7 years ago

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

Actions #14

Updated by Peter Amstutz almost 7 years ago

  • 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.
Actions #15

Updated by Peter Amstutz almost 7 years ago

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

Actions #16

Updated by Lucas Di Pentima almost 7 years ago

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?
Actions #17

Updated by Peter Amstutz almost 7 years ago

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.

Actions #18

Updated by Lucas Di Pentima almost 7 years ago

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?
  • 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?
Actions #19

Updated by Peter Amstutz almost 7 years ago

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?

Actions #20

Updated by Tom Clegg almost 7 years ago

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

current token search target session always listed? can add session? can disable session? use token notes
local this cluster yes - - current
local remote cluster no yes yes obtain via login
salted this cluster yes - - current bug #12959

Desired behavior

current token search target session always listed? can add session? can disable session? use token notes
local this cluster yes - - current
local configured remote yes? - - salted
local configured remote via dns no yes yes salted or via login
local user-entered remote no yes yes salted or via login
salted this cluster yes - - current bug #12959
As for whether to fall back to the login process for sites that don't accept a salted token: I think we might need to leave this up to the user, because there are various reasons for a salted token to fail, and not all of them mean the user wants to use a different account on that site. Perhaps:
  • 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
Actions #21

Updated by Lucas Di Pentima almost 7 years ago

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.

Actions #22

Updated by Lucas Di Pentima almost 7 years ago

  • Related to Bug #13020: Login into workbench with a salted token activates a previously inactive federated account added
Actions #23

Updated by Lucas Di Pentima almost 7 years ago

  • Target version changed from 2018-01-31 Sprint to 2018-02-14 Sprint
Actions #24

Updated by Peter Amstutz almost 7 years ago

  • Assigned To deleted (Lucas Di Pentima)

Another thought, when you are a federated user on remote cluster workbench, the multi-site search button should always take you to your home cluster (you can't do federated search from anywhere else).

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.

Actions #25

Updated by Peter Amstutz almost 7 years ago

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.

Actions #26

Updated by Peter Amstutz almost 7 years ago

  • Assigned To set to Lucas Di Pentima
Actions #27

Updated by Lucas Di Pentima almost 7 years ago

Updates at 0e3c1f9c7

Changes:
  • 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)
Actions #28

Updated by Peter Amstutz almost 7 years ago

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.

Actions #29

Updated by Peter Amstutz almost 7 years ago

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.

Actions #30

Updated by Peter Amstutz almost 7 years ago

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.

Actions #31

Updated by Lucas Di Pentima almost 7 years ago

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.
Actions #32

Updated by Lucas Di Pentima almost 7 years ago

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.

Actions #33

Updated by Tom Clegg almost 7 years ago

  • Related to Bug #12959: [Federation] Wrong cluster ID shown on multisite search page added
Actions #34

Updated by Tom Clegg almost 7 years ago

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?)

Adding ?api_token=... to "show collection" links seems like a significant security problem (worse than the other places we do this): if I right-click a "show" button and hit "copy link address" and paste it in chat -- which seems like a reasonable/likely thing to do -- anyone who clicks it will be using my account on that cluster. They won't even realize that they've switched to my account unless they check the "logged in" dropdown or notice all the content they couldn't see before. I think we should make a real effort to avoid this. Some ideas:
  • 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.)

Actions #35

Updated by Peter Amstutz almost 7 years ago

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.

Actions #36

Updated by Tom Clegg almost 7 years ago

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.

Actions #37

Updated by Tom Clegg over 6 years ago

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?

Actions #38

Updated by Lucas Di Pentima over 6 years ago

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.
Actions #39

Updated by Lucas Di Pentima over 6 years ago

Updates at 80eedd862

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

Actions #40

Updated by Tom Clegg over 6 years ago

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}),
The "link to remote workbench" on the session page...
  • 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?
Actions #41

Updated by Tom Clegg over 6 years ago

Tom Clegg wrote:

What's the motivation for having such a link?

Seems to have come from Peter's comment "linking between home and remote workbench instances is an absolutely critical aspect of the story" in note-25 above. Linking from the sessions page to remote Workbench dashboards doesn't seem "absolutely critical". I think the comment is referring to one or both of:
  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.
Actions #42

Updated by Lucas Di Pentima over 6 years ago

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().
Actions #43

Updated by Tom Clegg over 6 years ago

LGTM

Actions #44

Updated by Anonymous over 6 years ago

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

Updated by Lucas Di Pentima over 6 years ago

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.

Actions #46

Updated by Tom Clegg over 6 years ago

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', {
Actions #47

Updated by Lucas Di Pentima over 6 years ago

Update at 9c699bfa8

The first option is better, IMO.

Actions #49

Updated by Tom Morris over 6 years ago

  • Related to Feature #12958: [Federation] Workbench login chooser added
Actions #50

Updated by Tom Morris over 6 years ago

  • Release set to 17
Actions

Also available in: Atom PDF