FlowFuse / device-agent

An agent to run FlowFuse managed instances of Node-RED on devices
Apache License 2.0
15 stars 8 forks source link

Proxy support #272

Closed Steve-Mcl closed 1 month ago

Steve-Mcl commented 1 month ago

closes #270

Description

Notes

Tests added

  Test the AgentManager
    ✔ Should not extend got with proxies if env vars are not set
    Proxy Support
      ✔ Calls GOT with no agent when env vars are not set
      ✔ Calls GOT with an http agent when env var is set (provisionDevice)
      ✔ Calls GOT with an https agent when env var is set (provisionDevice)
      ✔ Calls GOT with an http agent when env var is set (quickConnectDevice)
      ✔ Calls GOT with an https agent when env var is set (quickConnectDevice)
      ✔ Calls GOT with an http agent when env var is set (_getDeviceInfo)
      ✔ Calls GOT with an https agent when env var is set (_getDeviceInfo)

  auditLogger
    ✔ should send log message to the logging URL (100ms)
    ✔ should ignore comms events and any .get events (39ms)
    ✔ should ignore auth events except auth.log events (38ms)
    ✔ should remove username and level properties from the log message
    Proxy support
      ✔ should not have a proxy if env vars are not set
      ✔ should use HTTP proxy agent if http_proxy environment variable is set
      ✔ should use HTTPS proxy agent if https_proxy environment variable is set

  HTTP Comms
    Proxy Support
      ✔ Creates the HTTP Comms Client
      ✔ Extends GOT with http proxy when env var is set
      ✔ Extends GOT with https proxy when env var is set

  Launcher
    ✔ Creates a new launcher instance (107ms)
    Proxy Support
      ✔ Passes proxy env vars to child process when set (83ms)

  MQTT Comms
    Proxy Support
      ✔ MQTT Comms Client does not set a proxy agent if process env does not contain proxy settings
      ✔ MQTT Comms Client has a HttpProxyAgent for ws connection
      ✔ MQTT Comms Client has a HttpsProxyAgent for wss connection
      ✔ MQTT Comms Client can connect with proxy (516ms)
      ✔ MQTT command gets a response when proxy is set (572ms)

  FFTeamLibraryPlugin
    ✔ should throw an error if projectID and applicationID are missing
    ✔ should throw an error if libraryID is missing
    ✔ should throw an error if token is missing
    Proxy support
      ✔ should create an instance with valid configuration and no proxy
      ✔ should extend got to use http proxy when env var http_proxy is set
      ✔ should extend got to use https proxy when env var https_proxy is set
      ✔ should extend got to use http & https proxies when env vars are set

  template-settings
    ✔ should load default settings
    ✔ should call got.get with the correct parameters when verifying token
    ✔ should cache the token verification result for subsequent calls within 30 seconds
    - should not cache invalid token
    - should return without hitting cache or API if the token is not valid
    - should return null for the users function
    - should return null for the authenticate function
    - should set the https options if provided
    - should set the httpStatic option if provided
    Proxy Support
      ✔ should not extend got when env vars http(s)_proxy are not set
      ✔ should extend got to use http proxy when env var http_proxy is set (3180ms)
      ✔ should extend got to use https proxy when env var https_proxy is set (3134ms)

  utils
    getWSProxyAgent
      ✔ should return null when there are no env vars set
      ✔ should not proxy any requests if they are excluded by no_local
      ✔ should return a HttpProxyAgent when http_proxy is set and the URL is ws://
      ✔ should return a HttpsProxyAgent when https_proxy is set and the URL is wss://
      ✔ should set http proxy options
      ✔ should set https proxy options
    getHTTPProxyAgent
      ✔ should return an agent object without any http or https proxy when env vars are not set
      ✔ should not proxy any requests if they are excluded by no_local
      ✔ should return an agent object with http property when http_proxy is set and no_proxy is not in scope
      ✔ should return an agent object with http property when http_proxy is set
      ✔ should return an agent object with https property when https_proxy is set
      ✔ should set http proxy options
      ✔ should set https proxy options

  51 passing (8s)
  6 pending

Related Issue(s)

270

Checklist

Labels

Steve-Mcl commented 1 month ago

Ben spoke up about possible environments where setting a proxy may be necessary for access to external resources but not the forge backend (and vice versa)

We discussed the implementation should be aware of the no_proxy env var or perhaps use separate env vars (e.g. ff_device_http_proxy).

Supporting no_proxy was determined to be the most obvious first choice since it does not need describing or documenting due to being a well known "thing".

Switching PR to draft while I evaluate the options.

Steve-Mcl commented 1 month ago

@hardillb this is now ready for local testing.

Steve-Mcl commented 1 month ago

TLDR

I had a strange issue when connecting the Device Editor tunnel with NR4-beta.4 import('got') did not work in the devices settings.js In the end, I had to add fall back to non esm GOT if not found to get past this.


Problem running NR v4.

template-settings.js calls import('got') doesnt work!

Witnessed:

Env:

_NOTE: NODE_PATH was set to the device-agent node_modules so that the NR plugins (auth, library access) can find http-proxy-agent & proxy-from-env which is required for proxies_

npm list got (NR 4) project@0.0.0-0 C:\opt\flowfuse-device\project ├─┬ @flowfuse/nr-project-nodes@0.6.4 │ └── got@11.8.6 └─┬ node-red@4.0.0-beta.4 └─┬ @node-red/nodes@4.0.0-beta.4 └── got@12.6.0

npm list got (NR 3.1.0) project@0.0.0-0 C:\opt\flowfuse-device\project ├─┬ @flowfuse/nr-project-nodes@0.6.4 │ └── got@11.8.6 └─┬ node-red@3.1.10 └─┬ @node-red/nodes@3.1.10 └── got@12.6.0

call stack when settings.js tokens() is called:

Object.tokens (c:\opt\flowfuse-device\project\settings.js:61)
Object.tokens (c:\opt\flowfuse-device\project\node_modules\@node-red\editor-api\lib\auth\users.js:132)
<anonymous> (c:\opt\flowfuse-device\project\node_modules\@node-red\editor-api\lib\auth\strategies.js:148)
global.authenticateUserToken (c:\opt\flowfuse-device\project\node_modules\@node-red\editor-api\lib\auth\strategies.js:133)
TokensStrategy.authenticate (c:\opt\flowfuse-device\project\node_modules\@node-red\editor-api\lib\auth\strategies.js:168)
global.attempt (c:\opt\flowfuse-device\project\node_modules\passport\lib\middleware\authenticate.js:378)
strategy.fail (c:\opt\flowfuse-device\project\node_modules\passport\lib\middleware\authenticate.js:314)
global.verified (c:\opt\flowfuse-device\project\node_modules\passport-http-bearer\lib\strategy.js:124)
<anonymous> (c:\opt\flowfuse-device\project\node_modules\@node-red\editor-api\lib\auth\strategies.js:44)
Promise.then (Unknown Source:0)
bearerStrategy (c:\opt\flowfuse-device\project\node_modules\@node-red\editor-api\lib\auth\strategies.js:32)
Strategy.authenticate (c:\opt\flowfuse-device\project\node_modules\passport-http-bearer\lib\strategy.js:132)
attempt (c:\opt\flowfuse-device\project\node_modules\passport\lib\middleware\authenticate.js:378)
authenticate (c:\opt\flowfuse-device\project\node_modules\passport\lib\middleware\authenticate.js:379)
<anonymous> (c:\opt\flowfuse-device\project\node_modules\@node-red\editor-api\lib\auth\index.js:64)
handle (c:\opt\flowfuse-device\project\node_modules\express\lib\router\layer.js:95)
next (c:\opt\flowfuse-device\project\node_modules\express\lib\router\route.js:149)
dispatch (c:\opt\flowfuse-device\project\node_modules\express\lib\router\route.js:119)
handle (c:\opt\flowfuse-device\project\node_modules\express\lib\router\layer.js:95)
<anonymous> (c:\opt\flowfuse-device\project\node_modules\express\lib\router\index.js:284)

Options

Option 1:

wrap await got = import('got') in try...catch and do a secondary got = require('got') if necessary. This allows got to be loaded from device agent node_modules via NODE_PATH

Options 2:

Add explicit entries for got, http(s)-proxy-agent & proxy-from-env to the device project package.json Careful consideration around existing devices in dev mode or not able to do an npm update, node/node-red versions and other dependencies that may be affected by this change.


I have opted to implement Option 1 in 74ca0dc and this worked (CJS got was loaded) but since implementing this change, i was unable to recreate the original condition (tried switching NR version between 3.x and 4.x streams but the ESM version is always resolved now).

What I didn't capture was actual file system tree of /project/node_module to understand where npm had put things. Looking after the event (as ESM dynamic import is working, got is at node_modules/got - I am unsure if that was the case originally.

Will test again 2x with clean device agent dirs. starting at NR 3.x then upgrading to NR4.x and then in reverse.

hardillb commented 1 month ago

@Steve-Mcl Are you still testing this, or does the "TLDR" header mean you've finished poking at it?

Steve-Mcl commented 1 month ago

Finished (with fingers tightly crossed)

I am still testing locally in case there are any other gotchas of course.

hardillb commented 1 month ago

Basic testing working well once I'd configured the proxy to do websockets properly for none secure traffic.

Had a problem with NR 3.1 as the http-request node appears to be doing some strange things (using CONNECT to access http as well as https sites so had to reconfig the proxy to allow this).

NR 3.0.2 has no http_proxy support, so this "broke" when I upgraded the device to 3.1.x might need to look at NR side of things a little. My best guess is that it's using a https proxy agent for everything.