anvilresearch / connect-cli Goto Github PK

Numerous catch blocks in the error handling for CLI commands do not report the root errors which can make troubleshooting very difficult.

Example 1 - In a number of places, code like this appears - in our case, on a setup command, we were having SSL issues but could not see the root error until we edited the catch to log the error.

          .catch(function () {
            cli.log.error(issuerUri + ' does not point to an Anvil Connect server')
            process.exit(1)
          })

Example 2 - While this did not cause us issues, I saw several lines like this in the code - once again, no useful output for troubleshooting.

                  } catch (e) {
                    cli.log.error('Couldn\'t write configuration file')
                    process.exit(1)
                  }

I would recommend logging err.stack||err.message||err in all these cases. If worried about it being too verbose, then you could add a command line argument to suppress (or enable) the detail...but just losing the detail makes troubleshooting much more difficult.

The current directions for setting up connect do not work under Windows because docker-compose is currently not available on Windows.

I am working on a bash script that can be run inside an msys (git-bash) shell that will simulate the critical docker-compose commands. It will not be driven by the yaml config but should suffice for the purpose.

Opening this ticket for any discussion on the topic. I will post a link to a draft script once it is ready and will provide a PR once it is functional.

Related note - by default, the boot2docker configuration on Windows only allows mapping of volumes to directories underneath c:\users\ - this should be mentioned in the documentation. If a similar restriction exists on Mac deployments we should also add that in.

We need a way to handle configurations for multiple issuers and also persist logged in user session data for the CLI.

For example, trying to run role:scopes against a role that does not exist results in the entire details of the request, response, connection, and socket being outputted to screen.

This may be applicable to other commands, but I have not had the time to look into it.

Changes need to be made to the server in order to do this.
See lib/commands/client.js

Right now we need to call discover() with every command. We need a good strategy for caching and invalidating this configuration data.

Hi there,

We're trying to automate the initialization of our AnvilConnect setup thus using the CLI quite intensively but have stumbled upon a few problems.

This is the code of the client we're trying to register:

nvl client:register \
  --trusted \
  --name "CMS" \
  --uri https://cms-test/app \
  --logo-uri http://toto.com \
  --application-type web \
  --response-type "id_token token" \
  --grant-type implicit \
  --default-max-age 3600 \
  --redirect-uri "https://cms-test/app/" \
  --redirect-uri "http://localhost:8082/app/" \
  --post-logout-redirect-uri "https://cms-test/app" \
  --post-logout-redirect-uri "http://localhost:8082/app"

This lead to a Validation Error. Since the error is not detailed, we logged the error in the connect-cli Clients API and this is the result:

{
  "valid": false,
  "errors": {
    "redirect_uris": {
      "attribute": "conform",
      "property": "redirect_uris",
      "actual": ["https://cms-test/app/","http://localhost:8082/app/"],
      "message": "Must follow guidelines in OpenID Connect Registration 1.0 specification for client metadata"
    }
  },
  "name": "ValidationError",
  "message": "Validation error.",
  "statusCode": 400
}

Nothing seems wrong to me but I might be missing something... Or is the CLI error message wrong?

Since the CLI was failing, we asked the API using POST /v1/clients with the data and the correct access token:

{
  "trusted": true,
  "client_name": "CMS",
  "client_uri": "https://cms-test/app",
  "logo_uri": "http://toto.com",
  "application_type": "web",
  "response_types": ["id_token token"],
  "grant_types": ["implicit"],
  "default_max_age": 3600,
  "redirect_uris": ["https://cms-test/app/", "http://localhost:8082/app/"],
  "post_logout_redirect_uris": ["https://cms-test/app","http://localhost:8082/app"]
}

But still got a Validation error.

Any idea or suggestion?

Because the output is now sent out raw as JSON, dates are no longer formatted, effectively making them unreadable without a calculator or using the date command.

For example, with client:info

{
  "redirect_uris": [
    "https://app.example.com"
  ],
  "response_types": [
    "id_token token"
  ],
  "grant_types": [
    "implicit"
  ],
  "application_type": "web",
  "client_name": "Example App",
  "client_uri": "https://app.example.com",
  "token_endpoint_auth_method": "client_secret_basic",
  "default_max_age": 3600,
  "post_logout_redirect_uris": [
    "https://app.example.com"
  ],
  "client_secret": "0123456789abcdef0123",
  "trusted": true,
  "scopes": [],
  "_id": "01234567-89ab-cdef-0123-456789abcdef",
  "created": 1445452220355,
  "modified": 1445452220355
}

Saving the logs to a file completely sidesteps Docker's logging functionality and renders it useless. i.e., running docker logs [redis_container_id] with the current image will return no output.

In order to allow Docker's log-capturing to work with the image, we need to output to stdout/stderr. Replacing logfile [logpath] in redis.conf with logfile "" fixes this problem. However, since this is a change in functionality, I'm opening an issue first to make sure we're all OK with this change.

Windows:

C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:75
throw e;
^
SyntaxError: Invalid regular expression: /.{1,0}/: numbers out of order in {} quantifier.
at new RegExp (native)
at breakLines (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\lib\utils\screen-manager.js:108:15)
at ScreenManager.render (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\lib\utils\screen-manager.js:55:22)
at Prompt.inquirer.prompt.prompts.input.render (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\lib\plugins\prompt.js:440:17)
at Prompt._run (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\lib\prompts\input.js:49:8)
at Prompt.run (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\lib\prompts\base.js:57:8)
at Object. (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\lib\ui\prompt.js:84:12)
at module.exports (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\run-async\index.js:15:21)
at AnonymousObservable.__subscribe (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\lib\utils\utils.js:16:7)
at AnonymousObservable.tryCatcher (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:63:31)
at setDisposable (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:5741:44)
at AnonymousObservable.innerSubscribe as _subscribe
at AnonymousObservable.Rx.Observable.observableProto.subscribe.observableProto.forEach (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:1604:19)
at AnonymousObservable.__subscribe (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:2112:21)
at AnonymousObservable.tryCatcher (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:63:31)
at setDisposable (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:5741:44)
at AnonymousObservable.innerSubscribe as _subscribe
at AnonymousObservable.Rx.Observable.observableProto.subscribe.observableProto.forEach (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:1604:19)
at MergeObserver.handleSubscribe (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:2910:28)
at MergeObserver.onNext (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:2916:16)
at MergeObserver.tryCatcher (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:63:31)
at AutoDetachObserverPrototype.next (C:\Users\Ivan\AppData\Roaming\npm\node_modules\anvil-connect-cli\node_modules\inquirer\node_modules\rx-lite\rx.lite.js:5782:51)

nvl setup http://localhost:3000 --token-file C:\Users\..\instance\connect\keys\setup.token Warning: you are communicating over plain text. ? Choose an email [email protected] ? Choose a password ******** ? Choose a name for this configuration localhost:3000 ? Choose an ID for this configuration localhost-3000 StatusCodeError: 400 - [object Object]

From Windows command prompt.

The following failed:

dev$ nvl client:register \
>   --issuer "localhost-3000" \
>   --trusted \
>   --name "Angular example with page for http://localhost:9000" \
>   --uri "http://localhost:9000" \
>   --logo-uri "http://localhost:9000/logo" \
>   --application-type "web" \
>   --response-type "id_token token" \
>   --grant-type "implicit" \
>   --default-max-age "3600" \
>   --redirect-uri "http://localhost:9000/callback_page" \
>   --redirect-uri "http://localhost:9000/rp.html" \
>   --post-logout-redirect-uri "http://localhost:9000"

A workaround was to duplicate the --post-logout-redirect-uri argument.

It would be nice if the CLI used semver to grab the latest Anvil Connect version within a specified leniency range. Then we'd only need to update the CLI when the files/structure/requirements change.

The container names configured in docker-compose.yml have very generic names. When I went to install this on my development machine, the container name redis and nginx would conflict with other general containers I have. I feel it would be best if connect would prefix the container names with the connect instance name - so if my instance name was "xyzzy", name it "xyzzy_redis"...

WARNING: As docker-compose does not work on Windows - I might be missing something. If so please correct me so I can ensure my scripts for #29 are consistent with docker-compose behavior.

If this team is in agreement with this, I can provide a PR for this change.

nvl init creates a docker-compose.yml that maps most of the data and config directories for the server components. HOWEVER, the redis log volume is not mapped. Given that the nginx log volume is mapped, it would seem to make sense to map this also.

If the team is in agreement, I can provide a PR for this change.

nvl init should not generate views. Instead, a near future change to the server will enable it to use built in views if none are present in the project. We can create a command to copy views from the package if a user decides to customize them.

anvilresearch/connect#165

nginz/conf.d/upstream.conf line 2 makes reference to connect_1 for a server name. The docker-compose yaml only makes a server called connect. This caused an nginx failure when I tried to connect via nginx.

I am using my draft Windows script (see #29) so it is possible this is a difference between how my script works and how docker-compose works.

Should this _1 be there? If not I can provide a PR as part of my work on Windows support to remove it.

Received StatusCodeError: 400 - [object Object] when running nvl setup.... After some debugging the issue was that I had met part of the requirement for a complex password, but the password did not include a CAPITAL letter. The connect cli needs to notified the user with an error message other than the generic StatusCodeError: 400..... This will help not only with password strength issues, but all CLI errors.

The attached screenshot shows the generic error message.

This may also help resolve issue #52.

In order to manage servers remotely, we want to be able to use the HTTP API via the CLI. This means we need authenticated users with an OAuth 2.0 token scoped for administration. There are a number of implications in this requirement along with questions regarding the implementation.

1. The CLI could potentially be used to manage more than one deployment.

How do we manage such context? Centralized storage of server credentials? Running the command from a specific directory relative to a server config file?

2. The CLI must be registered as a client.

How is this accomplished? OIDC dynamic registration? Predefined client credentials?

3. Like all native clients, there's a disconnect between the browser and the CLI.

How do we handle the "transfer problem"?

There's a related issue on the Anvil Connect repo for native clients: anvilresearch/connect#153

When running nvl init, the resulting docker-compose.yml references a number of folders for mapping volumes that do not exist. I believe it would be best if these folders were created by the init process.

If the team agrees, I can provide a PR to add this to the initialization.

The apk add in both the nginx and redis dockerfile do not specify a version. Should these be pinned to some level to ensure a compatible version?

As an alternative for discussion, is there a reason the official nginx and redis Docker Hub images are not used instead? I'm not clear why the team is maintaining it's own images for these components when there are official ones.

In either case, once the team comes to an opinion, I would be happy to provide a PR for either scenario.

Login still works but this seems to happen whenever I try use nvl client:info or nvl client:list