Allow device in developer mode to access team library

[Steve-Mcl]

Description

NOTE: This is part 1 of 2 PRs to enable the feature described in #2294

1. passes a boolean "licensed" flag to device in status updates
   1. device uses this to determine if the settings file should include the code to load the plugin
2. removes the if (request.teamMembership) { test from the pre-handlers to ensure needsPermission is called.
   1. Device uses project session token to authenticate (i.e. user is not present)
3. Update the implicit token scopes to ensure access is permitted
4. adds tests
   1. Add to Library from Devices
   2. Prevents Library access from Device in different team
   3. Device state -> unlicensed -> gets live state
   4. Device state -> licensed -> gets live state
5. updates tests
   1. various tests updated to ensure presence of licensed flag in state

NOTE: Docs do not mention devices cannot access the team library so no docs have been updated at this point

Related Issue(s)

#2294

Checklist

• I have read the contribution guidelines
• Suitable unit/system level tests have been added and they pass
• Documentation has been updated
  • Upgrade instructions
  • Configuration details
  • Concepts
• Changes flowforge.yml?
  • Issue/PR raised on flowforge/helm to update ConfigMap Template
  • Issue/PR raised on flowforge/CloudProject to update values for Staging/Production

Labels

• Backport needed? -> add the backport label
• Includes a DB migration? -> add the area:migration label

[codecov]

Codecov Report

Merging #2472 (dda42ff) into main (8da2139) will increase coverage by 34.78%.
Report is 45 commits behind head on main.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##             main    #2472       +/-   ##
===========================================
+ Coverage   39.84%   74.63%   +34.78%     
===========================================
  Files         494      228      -266     
  Lines       17690     9129     -8561     
  Branches     4125     1879     -2246     
===========================================
- Hits         7049     6813      -236     
+ Misses      10641     2316     -8325     

Flag	Coverage Δ
backend	74.63% <100.00%> (+0.13%)	⬆️
frontend	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
forge/db/controllers/Device.js	82.60% <ø> (+1.75%)	⬆️
forge/routes/auth/permissions.js	89.58% <ø> (ø)
forge/ee/routes/sharedLibrary/index.js	88.00% <100.00%> (+7.41%)	⬆️
forge/routes/api/deviceLive.js	66.03% <100.00%> (+13.40%)	⬆️

... and 352 files with indirect coverage changes

[knolleary]

Were these changes necessary? We aren't changing any behaviour for the managed instances that use project-scope tokens. They access the shared library using a user-token based on who is logged in. The project-scope token should not have these permissions.

In a future iteration we will want to update the device editor to use user-scoped tokens for this.

[Steve-Mcl]

First Answer:

Yes, due to removing the if (request.teamMembership) { from forge/ee/routes/sharedLibrary/index.js

• here and here

removes the if (request.teamMembership) { test from the pre-handlers to ensure needsPermission is called.
Device uses project session token to authenticate (i.e. user is not present)

Essentially, since the routes did not have a user (and therefore no teamMembership) the preHander permitted unfettered access.

---

Second Answer:

I see why you are questioning it & I will need to try this out again - I will reply shortly.

[knolleary]

I think the changes to the Project implicit scope should be reverted - unless I've missed a reason for them to have been changed.

[Steve-Mcl]

I think the changes to the Project implicit scope should be reverted - unless I've missed a reason for them to have been changed.

They were required - but I admit now to thinking there may be a different issue to solve.

I also replied to the suggestion here

I will feedback shortly.

[Steve-Mcl]

ok, so this took me a while to get back to it - my apologies.

From a project/instance

It is the team library plug-in that makes the request to team lib endpoints
https://github.com/flowforge/flowforge-nr-launcher/blob/main/lib/storage/libraryPlugin.js#L21-L26 and it uses a bearer token.

Before these modifications, the plugin got a free ride through the preHander here and here because the needsPermission was wrapped in a test of if (request.teamMembership) { ... }

In this PR, I noted that needsPermission was not being called for the device & so I removed the if (request.teamMembership) { test so that needsPermision would be called. This then highlighted the fact that a request for shared library from a "project" type had never hit the needsPermission code (as it was now failing the verifySession with a 401) & therefore the project also needed implicit permissions. Remember: it is the plugin, with its bearer token making the call to the platform so there is no sid when a library request comes into forge from the instances plugin.

Does that make sense?

[Steve-Mcl]

@hardillb in absence of Nick, can I lean on you for a review please?

Happy to discuss the issue Nick raised and the thoughts behind his comments (they related to a future where user tokens with specific, limited access instead bearer tokens) for RBACs

As this PR stands, to permit the device access - it was necessary to ensure needsPermission was called for the routes and this lead to discovering projects/instances were never actually checking permissions. This then leads to the need, in this iteration to add the implicit permissions for both the device and the project.

If any of this is unclear, I am more than happy to step through it with you.

Ta.

[hardillb]

First pass looks ok, Can we discuss it in the morning, just to make sure I've got everything?

[hardillb]

Merge once library:entry:delete checked

[Steve-Mcl]

Merge once library:entry:delete checked

Checked & fails due to invalid token.

While the end point is seemingly open to use (when a valid bearer is present), the plug in never generates a call to node-red for deletion (there is no code to support it) so the request is not made via NR client to NR server through to forge.

For completeness and piece of mind, I am going to delete the implicit permission regardless.

Will run a batch of manual checks to ensure that doesn't break anything.

[Steve-Mcl]

@hardillb tests on local all good with the implicit scope for deletion removed (though I did find a different UI bug!)

Could you verify/merge pls?

On vacation, reviewed by Ben