Create viewer role for teams

[ZJvandeWeg]

Description

As a user I may want to invite other users into my team just to view the progress or to audit flows. Not allowing changing any settings, environment variables and the like, but just viewing the project.

This will require slight changes when inviting users to a team, as the invite right now defaults to adding users as members. The role needs to be added when creating the invite.

Which customers would this be availble to

• All Users, (CE)
• Specific Project Types (EE)
• Specific Team Tiers (EE)
• Other (See comments)

[cymplecy]

Just clarify, I think I'd want them to be able double-click and see inside nodes but just not alter/redeploy the flow in any way

[knolleary]

That is the standard read-only level of access Node-RED provides - user can do anything in the editor, but no changes can be written back to the runtime.

We need to add the team role that maps to that level of access.

[knolleary]

Added a medium sizing to this feature. This is the first time we'll be adding a new role to the original set of roles. This will need to be done with some care as there could be places in the code that assume "Not Owner" means "Member" (or vice versa), whereas it could now be "Member" or "Viewer".

[ZJvandeWeg]

Updated the description as there's a requirement to set the role when an invite is created, not after.

[sammachin]

Adding this as a TeamTier feature

[sammachin]

A consideration around billing is that if teams are charged per member then what should the charging for a viewer be, are they included, charged the same as a member or something else

[ZJvandeWeg]

@sammachin A team has member which each assume a role in the team. Viewers should be charged as a member in my book.

[knolleary]

Tasks

• Add viewer to list of defined role constants.
• Update frontend access controls to limit viewer role from any action that modifies the project
• Update API access controls to restrict viewer role (existing needsPermission framework should already do this... need to verify). Add tests.
• Add viewer as selectable role when inviting user.
• Add viewer as selectable role when modifying a user's membership

[knolleary]

Having created a user with their role set to 'viewer', here's a brief summary of what the existing code allows. I have highlighted the things that are obviously wrong and need addressing.

• Team Views
  • They can see the Overview, Projects, Devices and Members views
  • They do not get shown any actions on those views
• Project View
  • Overview View
    • overview box is shown on left.
    • ⚠️ 'Recent Activity' box shows the loader image - the underlying api call returns a 403
  • ⚠️ Activity View - same as the Recent Activity summary box (loader image, underlying api call hits 403)
  • ⚠️ Snapshots View - sees all snapshots with menu to 'rollback' and 'set device target' options
  • Devices View - sees all devices. No actions available
  • Logs view - sees the Node-RED logs.
  • Settings view
    • General - sees the settings, no options available
    • ⚠️ Env Var - can add env vars and click 'save' - but results in a 403 error
• Device View
  • Overview View - Can see overview and deployment details. No actions available
  • Settings View
    • General - sees the settings, no options available
    • ⚠️ Env Var - can add env vars and click 'save' - but results in a 403 error

But as I navigate around the view, thinking about what purpose the viewer role serves, I wonder if we start from a position of a far more restricted set of views. Specifically, only allow them to see:

• Team View
  • List of projects
  • List of members
  • No devices
• Project View
  • Overview only.
• Device View
  • Nothing (as they don't have devices views above)

This is much more focused on the purpose being to enable viewing of the flows themselves rather than the full details of the project. For example, if I have set 'sensitive' info in env vars to configure the flows, I may not want a viewer to be able to freely see them. I am not proposing are sort of optional configuration here - but I think starting from a more secure stand-point (whilst satisfying the core requirement) is a better place to start. We can then be more considered around what additional views get added to their access level based on specific needs.

[sammachin]

I disagree, in my view the view only user should be able to see anything that a regular user can see but not make any changes,
We talked about having the public view of a flow being a separate story where the viewer just gets a URL that allows them RO access to the editor and they have no view or knowledge of the Forge application. Thats in #855

[knolleary]

@sammachin that makes sense. So a Viewer can see everything but not change anything. Will make it so.

[knolleary]

Having dug into how Node-RED user-permissions are handled, this will require an update to the nr-auth plugin we use on the Node-RED side of things. Without that update, all authenticated users (regardless of role) will get full read/write access.

So we will have a slightly awkward situation that all existing projects will give viewer's full r/w access until they update to the latest stack. I cannot see any way around that - but needs to be documented.

[sammachin]

how feasible is it to prevent a team from creating viewers until they've updated their projects, or at least have a warning in the forge application, I'm concerned that they won't read docs.

[knolleary]

Not very feasible. But just had a thought that we might be able to prevent a viewer from logging in to Node-RED if the stack hasn't been updated - with a suitable error message to prompt the stack update. That would be better than letting them in with full r/w access

[knolleary]

Having tested out what we can achieve with the existing Node-RED + auth plugin, it isn't ideal, but its something.

We added the 'auto login' flag to Node-RED to make for a more seamless login experience - so whenever the editor loads, if the flag is set, it redirects immediately to the url needed to kick-off the oauth login flow.

However, if that attempt fails, Node-RED redirects back to the default editor url. Which in turn, because auto-login is set, kicks off the login flow again - and you are caught in a redirect loop.

There was some effort made to allow a 'session message' to get passed back that does break the loop - but it doesn't get a look-in here due to the way passport (the js library NR uses to do auth) handles error redirects.

Even if there was a change to make in Node-RED, we're back at the original issue of needing the stack to be updated to get that change running.

The immediate question is what can we achieve with the current version of Node-RED.

The answer is this:

We could put a bit more effort to return a more complete HTML page wrapping it, but that point is, this isn't the Node-RED editor login screen - its a FF platform page.

[sammachin]

That seems fine, given that this is a time limited issue as new projects come along they will be on the new stack anyway so its not worth putting huge effort into something with diminishing returns.

I would tweak the wording slightly on that message, i assume that this would be seen by the viewer user trying to login so they won't be able to update the stack only an owner can do so, so the message should read:
Please ask the team owner to update this project to the latest stack to support viewer access

[knolleary]

I think I have it working (including your suggested wording).

There are still some Node-RED specific UX oddities we should fix upstream (some already being thought about with the Locking Flows work I recently started).

The read-only mode in NR still allows you to edit things in the editor - but when you come to deploy, you get the following notification:

... which isn't quite right as you are logged in. But that's what we have today. I will raise an upstream issue to improve that message in the case of a read-only user.

Given the Locking Flows work will lead to not allowing changes to be made in the editor, we could reuse that same logic for a read-only user.

[knolleary]

@hardillb this PR is good to review/merge. I am going to follow up the RBAC doc changes in a separate PR as well as the ability to set the membership level as part of the user invite flow. Decided that latter part would make this PR too big to review and there's no need to pile it all into one.

[hardillb]

I assume you mean the PR #1005, I'm going through that now

[sammachin]

verfied on staging