navigator supports user management #12187

adriaanm-da · 2021-12-17T10:47:29Z

Add basic support for user management to navigator: log in as a user, act/read as its primary party.

When user management is supported & enabled, you can only log in as a user (and that user must have a primary party, which is what you'll actually be acting/reading as).

This behavior is enabled by default, but there's a feature flag you can use to disable it (--feature-user-management).

See #12020

Pull Request Checklist

Read and understand the contribution guidelines
Include appropriate tests
Set a descriptive title and thorough description
Add a reference to the issue this PR will solve, if appropriate
Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
Normal production system change, include purpose of change in description
If you mean to change the status of a component, please make sure you keep the Component Status page up to date.

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

cocreature

Looks very plausible. Have you considered how this interacts with the Navigator config? Do we just stick to not handling user management there and only support party ids & human readable names?

As for tests, you could add some integration tests in https://github.com/digital-asset/daml/blob/main/navigator/backend/src/test/scala/com/digitalasset/navigator/backend/IntegrationTest.scala

...in/scala/com/digitalasset/ledger/client/services/version/withoutledgerid/VersionClient.scala

navigator/backend/src/main/scala/com/digitalasset/navigator/store/platform/PlatformStore.scala

adriaanm-da · 2022-01-19T14:43:32Z

I think this is now mostly complete. I need to factor out a few changes from this PR, but I think it's worth taking a look -- @stefanobaghino-da?

One thing I can offer for bikeshedding is "--disable-user-management" (this PR) vs. "--feature-user-management" (#12420)

adriaanm-da · 2022-01-19T18:56:06Z

The test I added is failing on CI, while it passes on my mac. It's a connection error, and it looks like it's not retrying. I'll try to debug tomorrow.

navigator/backend/src/main/scala/com/digitalasset/navigator/config/Arguments.scala

navigator/backend/src/main/scala/com/digitalasset/navigator/model/PartyState.scala

navigator/backend/src/main/scala/com/digitalasset/navigator/UIBackend.scala

navigator/backend/src/test/scala/com/digitalasset/navigator/backend/IntegrationTest.scala

ledger/ledger-api-domain/src/main/scala/com/digitalasset/ledger/api/domain.scala

cocreature · 2022-01-19T21:24:45Z

The test I added is failing on CI, while it passes on my mac. It's a connection error, and it looks like it's not retrying. I'll try to debug tomorrow.

You can use --runs_per_test=10 or something like that as an argument to bazel test which is usually good enough to reprodue flakes locally.

The test here seems just racy, there is no fundamental reason why we should instantly connect to the ledger. Just wrapping it in an eventually with a reasonable timeout fixes the flakes for me locally.

stefanobaghino-da

Looks good so far. 🙇🏻‍♂️

...ient/src/main/scala/com/digitalasset/ledger/client/services/admin/UserManagementClient.scala

...in/scala/com/digitalasset/ledger/client/services/version/withoutledgerid/VersionClient.scala

ledger/ledger-api-domain/src/main/scala/com/digitalasset/ledger/api/domain.scala

navigator/backend/src/main/scala/com/digitalasset/navigator/model/PartyState.scala

navigator/backend/src/main/scala/com/digitalasset/navigator/store/platform/PlatformStore.scala

...pi-client/src/main/scala/com/digitalasset/ledger/client/services/version/VersionClient.scala

adriaanm-da · 2022-02-02T20:28:57Z

I've factored out the underlying refactorings to separate PRs (#12724, #12725). Once those are merged, I'll rebase this PR.

For now, I've only roughly squashed commits in this PR to simplify potential re-reviews. Eventually, they will all be squashed together anyway.

adriaanm-da · 2022-02-03T08:09:00Z

Windows test failure seems unrelated. Linux passes.

//daml-lf/engine:test-min-version                                        FAILED in 131.9s
  C:/users/u/_bazel_u/vvgx3zjt/execroot/com_github_digital_asset_daml/bazel-out/x64_windows-opt/testlogs/daml-lf/engine/test-min-version/test.log

Executed 35 out of 598 tests: 597 tests pass and 1 fails locally.

cocreature · 2022-02-03T09:13:41Z

Windows test failure seems unrelated. Linux passes.

thx we’ll take a look.

adriaanm-da · 2022-02-04T10:03:14Z

Added a commit to catch up with #12610. TODO: actually deal with user pagination...

CHANGELOG_BEGIN CHANGELOG_END

use parties XOR users use --feature-user-management CHANGELOG_BEGIN CHANGELOG_END

This resolves the cross-talk between tests Co-authored-by: Stefano Baghino <[email protected]> CHANGELOG_BEGIN CHANGELOG_END

CHANGELOG_BEGIN CHANGELOG_END

todo: actually deal with pagination...

stefanobaghino-da

Looks good. Unfortunately at the moment we don't have full UI tests (with Selenium or something along those lines). Have you been able to test the UI manually?

stefanobaghino-da · 2022-02-04T17:51:41Z

navigator/backend/src/main/scala/com/digitalasset/navigator/Session.scala

@@ -15,6 +14,7 @@ import scala.reflect.ClassTag
 final case class User(
    id: String,
    party: PartyState,
+    // TODO: where is `role` used? frontend has some references, but doesn't seem to impact anything?


Perhaps @rautenrieth-da (who will have a chance to have a look at this on Tuesday, so not urgent) can provide some historical context.

In Navigator config, you can assign "roles" to use "users" (see example below). The role is an arbitrary string chosen by the Navigator user. It can be used by the frontend config to customize the theme or custom views depending on the "role" (see here). There might have been other uses in the past, this is the one I know of.

Keep in mind that Navigator user handling was implemented long before we added authentication to the ledger API, which in turn was long before we started to think about the current ledger API user management.

users { OPERATOR { // display name in Navigator party=OPERATOR // Daml party for the ledger API role=operator // metadata for Navigator frontend } BANK1 { party=BANK1 role=bank } BANK2 { party=BANK2 role=bank } }

Thanks, Robert!

navigator/backend/src/main/scala/com/digitalasset/navigator/store/platform/PlatformStore.scala

navigator/backend/src/test/scala/com/digitalasset/navigator/backend/IntegrationTest.scala

adriaanm-da · 2022-02-07T11:30:45Z

Have you been able to test the UI manually?

Yes, I could log in as Alice, Charlie and Bob, and saw User:User and User:Alias templates floating about, but I'm not sure how to test more fully in the UI -- the integration tests do mimic pretty closely what happens in the back-end during normal UI actions, AFAICT.

Happy to do more manual testing, but perhaps this is enough to get this into the hands of actual testers?

stefanobaghino-da · 2022-02-07T13:51:57Z

Have you been able to test the UI manually?

Yes, I could log in as Alice, Charlie and Bob, and saw User:User and User:Alias templates floating about, but I'm not sure how to test more fully in the UI -- the integration tests do mimic pretty closely what happens in the back-end during normal UI actions, AFAICT.

Happy to do more manual testing, but perhaps this is enough to get this into the hands of actual testers?

I think so, yes, thanks, that level of manual testing is good enough for now and we can put it in the hand of users I believe.

rautenrieth-da · 2022-02-08T09:30:12Z

Unfortunately at the moment we don't have full UI tests (with Selenium or something along those lines)

Some more historical context, we have Selenium based tests here, but they haven't been built or run in ages. I suspect we'd have to rewrite large parts of it to get it to run again. They also depend on the BrowserStack service, which we do not use anymore.

One idea that we had in the past was to replace BrowserStack by a headless chrome browser, so that these tests can easily run locally and in CI. This would not catch regressions on non-Chrome browsers (e.g., if you broke the CSS on Firefox), but it would catch functional regressions (e.g., if you broke the GraphQL API such that the frontend doesn't display any data).

navigator/backend/src/main/scala/com/digitalasset/navigator/model/PartyState.scala

rautenrieth-da · 2022-02-08T09:48:27Z

navigator/backend/src/main/scala/com/digitalasset/navigator/store/platform/PlatformStore.scala

+        // we may subscribe to the same party under different display names, but we should only create one actor per party)
+        val partyActorName = partyState.actorName
+        if (context.child(partyActorName).isEmpty)
+          context.actorOf(


This is not new behavior, but mentioning it nevertheless: This starts an actor for each discovered party. Each actor uses its own in-memory database to store a complete copy of the whole transaction stream from ledger begin. This means Navigator backend will blow up unless the ledger is "small".

Thanks again, this is super helpful!

…del/PartyState.scala Co-authored-by: Robert Autenrieth <[email protected]>

Undoing an unnecessary refactoring I did in #12187 CHANGELOG_BEGIN CHANGELOG_END

Undoing an unnecessary refactoring from #12187 stefanobaghino-da confirmed this fixes the memory leak he observed in "long" running navigator sessions. CHANGELOG_BEGIN CHANGELOG_END

adriaanm-da force-pushed the adriaan-usermgmt-navigator branch 3 times, most recently from 4e5bc16 to 498ef0d Compare December 18, 2021 07:52

cocreature reviewed Dec 20, 2021

View reviewed changes

adriaanm-da force-pushed the adriaan-usermgmt-navigator branch from ad3c578 to 73113a1 Compare December 20, 2021 14:58

adriaanm-da force-pushed the adriaan-usermgmt-navigator branch from 73113a1 to 91acdf7 Compare January 10, 2022 14:11

adriaanm-da force-pushed the adriaan-usermgmt-navigator branch from 22bbe07 to 7e7d38c Compare January 17, 2022 21:08

adriaanm-da force-pushed the adriaan-usermgmt-navigator branch 3 times, most recently from a0ae915 to 4389c46 Compare January 19, 2022 15:15

adriaanm-da requested a review from stefanobaghino-da January 19, 2022 15:33

cocreature reviewed Jan 19, 2022

View reviewed changes

stefanobaghino-da reviewed Jan 21, 2022

View reviewed changes

...pi-client/src/main/scala/com/digitalasset/ledger/client/services/version/VersionClient.scala Outdated Show resolved Hide resolved

adriaanm-da force-pushed the adriaan-usermgmt-navigator branch 3 times, most recently from 185d563 to b44a0f5 Compare February 2, 2022 15:22

This was referenced Feb 2, 2022

VersionClient supports getApiFeatures #12724

Merged

Use Seq instead of Vector in user mgmt apis #12725

Merged

adriaanm-da force-pushed the adriaan-usermgmt-navigator branch from b44a0f5 to 4d8929e Compare February 2, 2022 20:26

adriaanm-da force-pushed the adriaan-usermgmt-navigator branch 2 times, most recently from ca14cad to ad0d4c8 Compare February 4, 2022 10:02

adriaanm-da marked this pull request as ready for review February 4, 2022 14:20

adriaanm-da requested review from a team as code owners February 4, 2022 14:20

adriaanm-da marked this pull request as draft February 4, 2022 14:21

adriaanm-da and others added 8 commits February 4, 2022 17:44

context.child is not threadSafe, don't mix three "monads"

3761c6c

CHANGELOG_BEGIN CHANGELOG_END

updating users from ledger

ac708a8

CHANGELOG_BEGIN CHANGELOG_END

update users in platformstore

138ec46

CHANGELOG_BEGIN CHANGELOG_END

Don't embed a UserConfig in PartyState

68c4478

CHANGELOG_BEGIN CHANGELOG_END

test navigator picks up new users

4ec0240

use parties XOR users use --feature-user-management CHANGELOG_BEGIN CHANGELOG_END

Clean up the user list after every test

e640511

This resolves the cross-talk between tests Co-authored-by: Stefano Baghino <[email protected]> CHANGELOG_BEGIN CHANGELOG_END

cleanups and review feedback

8691952

CHANGELOG_BEGIN CHANGELOG_END

add listAllUsers to work around user pagination

491e785

todo: actually deal with pagination...

adriaanm-da force-pushed the adriaan-usermgmt-navigator branch from ad0d4c8 to 491e785 Compare February 4, 2022 16:44

adriaanm-da marked this pull request as ready for review February 4, 2022 16:44

stefanobaghino-da approved these changes Feb 4, 2022

View reviewed changes

rautenrieth-da reviewed Feb 8, 2022

View reviewed changes

Update navigator/backend/src/main/scala/com/digitalasset/navigator/mo…

0feffb9

…del/PartyState.scala Co-authored-by: Robert Autenrieth <[email protected]>

adriaanm-da changed the title ~~usermgmt poc: navigator~~ navigator supports user management Feb 8, 2022

adriaanm-da merged commit 46c3228 into main Feb 9, 2022

adriaanm-da deleted the adriaan-usermgmt-navigator branch February 9, 2022 09:28

adriaanm-da mentioned this pull request Feb 10, 2022

navigator regression: allocate parties in script, no users available for login #12867

Closed

stefanobaghino-da mentioned this pull request Feb 11, 2022

navigator: works for canton, human-readable names; can log in as user (using primary party to act as the current user) #12020

Closed

adriaanm-da added a commit that referenced this pull request Feb 11, 2022

Navigator: try not to leak memory via PartyState

3a8cfa2

Undoing an unnecessary refactoring I did in #12187 CHANGELOG_BEGIN CHANGELOG_END

adriaanm-da mentioned this pull request Feb 11, 2022

Navigator: try not to leak memory via PartyState #12896

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

navigator supports user management #12187

navigator supports user management #12187

adriaanm-da commented Dec 17, 2021 •

edited

Loading

cocreature left a comment

adriaanm-da commented Jan 19, 2022

adriaanm-da commented Jan 19, 2022

cocreature commented Jan 19, 2022

stefanobaghino-da left a comment

adriaanm-da commented Feb 2, 2022 •

edited

Loading

adriaanm-da commented Feb 3, 2022

cocreature commented Feb 3, 2022

adriaanm-da commented Feb 4, 2022

stefanobaghino-da left a comment

stefanobaghino-da Feb 4, 2022

rautenrieth-da Feb 8, 2022

adriaanm-da Feb 8, 2022

adriaanm-da commented Feb 7, 2022 •

edited

Loading

stefanobaghino-da commented Feb 7, 2022

rautenrieth-da commented Feb 8, 2022

rautenrieth-da Feb 8, 2022

adriaanm-da Feb 8, 2022

navigator supports user management #12187

navigator supports user management #12187

Conversation

adriaanm-da commented Dec 17, 2021 • edited Loading

Pull Request Checklist

cocreature left a comment

Choose a reason for hiding this comment

adriaanm-da commented Jan 19, 2022

adriaanm-da commented Jan 19, 2022

cocreature commented Jan 19, 2022

stefanobaghino-da left a comment

Choose a reason for hiding this comment

adriaanm-da commented Feb 2, 2022 • edited Loading

adriaanm-da commented Feb 3, 2022

cocreature commented Feb 3, 2022

adriaanm-da commented Feb 4, 2022

stefanobaghino-da left a comment

Choose a reason for hiding this comment

stefanobaghino-da Feb 4, 2022

Choose a reason for hiding this comment

rautenrieth-da Feb 8, 2022

Choose a reason for hiding this comment

adriaanm-da Feb 8, 2022

Choose a reason for hiding this comment

adriaanm-da commented Feb 7, 2022 • edited Loading

stefanobaghino-da commented Feb 7, 2022

rautenrieth-da commented Feb 8, 2022

rautenrieth-da Feb 8, 2022

Choose a reason for hiding this comment

adriaanm-da Feb 8, 2022

Choose a reason for hiding this comment

adriaanm-da commented Dec 17, 2021 •

edited

Loading

adriaanm-da commented Feb 2, 2022 •

edited

Loading

adriaanm-da commented Feb 7, 2022 •

edited

Loading