Move all identity APIs under /namespaces #804

awrichar · 2022-05-10T14:30:31Z

Part of hyperledger/firefly-fir#12

Summary

All identity-related routes are moved from the top level of the API to be nested under /namespaces/{ns}
All newly created orgs and nodes will therefore be created in a given namespace (not in ff_system)
Broadcasting of new namespaces is disabled (new namespaces can only be defined by adding them to config)
New "network version" concept added to blockchain plugin - now baked into the FireFly smart contract, so that previously-deployed networks will be interpreted as "V1", while deployments of the new contract will advertise "V2"
All usage of ff_system is deprecated (but existing identities saved on ff_system can be used to resolve identities in any namespace if network "V1" mode is in use)
Custom (non-org, non-node) identities now have no defined prefix, and will be of the form did:firefly:{identity}.

Network Rules V2

As noted above, the rules for FireFly multiparty networks are now versioned. As of now, the differences are:

V1: identities will first be resolved against the current namespace, then fall back to ff_system
V2: identities will be resolved only against the current namespace

Note that the rules for V2 are still evolving and are planned to change further before the next minor release.

Notable API Changes

Most apps should not be broken by these changes - but usage of deprecated APIs should be corrected, and the migration steps below should be reviewed carefully.

The following top-level APIs are deprecated and replaced:

/network/identities - replaced by existing /namespaces/{ns}/identities
/network/identities/{did} - replaced by new /namespaces/{ns}/identities/{did}

Note the addition of /namespaces/{ns}/identities/{did} in parallel with the existing /namespaces/{ns}/identities/{id} - regex patterns ensure you can look up an identity by DID (if it starts with did:) or ID (if it does not).

The following APIs are moved from the top-level to reside under /namespaces/{ns}:

/network/action
/network/diddocs/{did}
/network/nodes
/network/nodes/{nameOrId}
/network/nodes/self
/network/organizations
/network/organizations/{nameOrId}
/network/organizations/self
/status

Because of the changes in #792, existing apps that query these URLs without a /namespaces/{ns} prefix will still work (they will just be hitting the default namespace instead of the legacy ff_system namespace).

The /status API response is tweaked, so that instead of returning the default namespace in defaults.namespace, it instead returns the name of the current namespace as namespace.

Migration Steps

To migrate existing multi-party networks, the following steps are required:

Upgrade FireFly on all nodes
All parties register their org and node identities with a broadcast on the default namespace (or whichever namespace(s) are in use on the network)
One party deploys a new FireFly contract that advertises network "V2", and all members add the new contract to their configured contract list (see Support migration of the FireFly contract from one location to another #820)
One party issues a "network action" for "terminate" on the ff_system namespace, which will cause all parties to move onto the new contract (and therefore network "V2" rules), and stop using any identities that were previously broadcast on ff_system

The migration is designed to work without a significant "freeze" on use of the system. All members should complete step 1 first, then steps 2-3 can be completed at any time while normal network operations continue. Once all members have performed the setup, usage of the network should be momentarily paused to execute step 4, which will move all members simultaneously to the new logic.

Breaking changes to note:

once a node is upgraded (1) but before all members have re-broadcast their identity (2), the /network/nodes and /network/organizations endpoints will return empty result sets. Applications that rely on these APIs may need to query /namespaces/ff_system/network/nodes and /namespaces/ff_system/network/organizations in order to look up the legacy identities stored there.
Applications that listen for new orgs/nodes to be created will need to register new subscriptions against the default namespace instead of ff_system.

codecov-commenter · 2022-05-10T14:35:33Z

Codecov Report

Merging #804 (f15ac92) into main (d05ff8f) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #804   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          301       300    -1     
  Lines        19743     19811   +68     
=========================================
+ Hits         19743     19811   +68

Impacted Files	Coverage Δ
internal/apiserver/routes.go	`100.00% <ø> (ø)`
internal/broadcast/manager.go	`100.00% <ø> (ø)`
internal/coreconfig/coreconfig.go	`100.00% <ø> (ø)`
internal/apiserver/route_get_identity_by_did.go	`100.00% <100.00%> (ø)`
internal/apiserver/route_get_net_did.go	`100.00% <100.00%> (ø)`
internal/apiserver/route_get_net_diddoc.go	`100.00% <100.00%> (ø)`
internal/apiserver/route_get_net_identities.go	`100.00% <100.00%> (ø)`
internal/apiserver/route_get_net_node.go	`100.00% <100.00%> (ø)`
internal/apiserver/route_get_net_nodes.go	`100.00% <100.00%> (ø)`
internal/apiserver/route_get_net_org.go	`100.00% <100.00%> (ø)`
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d05ff8f...f15ac92. Read the comment docs.

Instead of broadcasting identities globally on the special "ff_system" namespace, they must now be registered on each namespace where you wish to use them. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

This functionality could come back at some point, but it needs to be done in some hierarchical fashion (without assuming there is a global "ff_system" namespace that always exists). Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Some fields are now only unique within a namespace. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

pkg/core/identity.go

Allow forward slash to be used in names so that applications can define their own DID types such as "person/john" and "division/acme". Disallow the reserved prefixes "org/" and "node/" on custom identities. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

peterbroadhurst · 2022-05-13T12:12:29Z

I have a migration question @awrichar. Take this example:

Network has 10 members at v1.0.x
Network has broadcast APIs, datatypes, and data from these members
Network upgrades to v1.1.x
New member joins the network

With the migration approach you described, of disabling legacySystemIdentities at some point, how will the new member receive/trust the data broadcast while the network was running at v1.0.x?

awrichar · 2022-05-13T12:21:26Z

@peterbroadhurst I think the new member would have to join with legacySystemIdentities enabled, at least initially.

internal/apiserver/route_get_identity_by_did.go

peterbroadhurst · 2022-05-13T12:56:49Z

internal/coreconfig/namespace.go

+	// NamespaceName is a long description for a pre-defined namespace
+	NamespaceDescription = "description"
+	// NamespaceOrgKey is a default signing key for blockchain transactions within a namespace (overrides top-level org.key)
+	NamespaceOrgKey = "org.key"


Per comments on the FIR, I think this should be in a multiparty.rootIdentity (or similar) scoped sub-section of the config, in the new world.

Yea, still an open discussion on exactly how to spell that. I think it does need to have "org" in the name though, since the root identity is ultimately always an org.

I guess the question is whether we want to merge this and go through a period of flux in main, or hold it out until we close on the config design.

Seems like the discussion on the FIR has converged for now, so I've reworked the config here to match. Changes in the new commit:

Replace "mode: multiparty" with "multiparty.enabled: true"

Default to multiparty mode if org name or key is set

Remove previously-deprecated "org.identity" root key

Add new "defaultKey" config to namespaces (valid for both multiparty and gateway namespaces, overrides "multiparty.org.key" for signing)

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

internal/namespace/manager.go

internal/orchestrator/orchestrator.go

internal/orchestrator/orchestrator_test.go

pkg/core/identity.go

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

awrichar · 2022-05-27T21:22:08Z

Updated on top of the latest, and replaced the legacySystemIdentities config option with a "network version" queried from the blockchain contract.

internal/blockchain/ethereum/ethereum.go

smart_contracts/ethereum/solidity_firefly/contracts/Firefly.sol

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

peterbroadhurst · 2022-05-31T15:31:34Z

internal/blockchain/ethereum/abi_definitions.go

@@ -96,3 +96,16 @@ var batchPinEventABI = ABIElementMarshaling{
 		},
 	},
 }
+
+var networkVersionMethodABI = ABIElementMarshaling{


I note this might have a merge conflict with @nguyer who I believe is moving all of these to the firefly-signer/pkg/abi package.

... or if it doesn't then this is another place we should remove a duplicate interface definition.

looks like this one is ready first - sorry @nguyer

peterbroadhurst · 2022-05-31T15:32:51Z

internal/blockchain/ethereum/abi_definitions.go

+	Inputs:          []ABIArgumentMarshaling{},
+	Outputs: []ABIArgumentMarshaling{
+		{
+			InternalType: "uint8",


I wonder whether a bytes32 would be the most efficient option here, and using a semver structure. uint8 gets stored as a unit256 anyway.

(added more discussion on this in a separate comment)

peterbroadhurst · 2022-05-31T15:34:02Z

internal/blockchain/ethereum/ethereum.go

+		mux            sync.Mutex
+		address        string
+		fromBlock      string
+		networkVersion int


Note comment above over int vs. semver. Probably an open discussion, rather than a proposal.

As I ramble with half-formed thoughts here... I wonder whether it's actually a network-options block, where we've reserved a byte for a version integer, but other contract implementations could express additional details in that.

I guess I may be completely overthinking this, as fundamentally this is just a constructor argument with a getter on the contract. So any extensibility to other options might be contract specific.

I'm just feeling an extensibility worry, and wondering if there's some way to do something now that would give future-us some more flexibility for some (currently unknown) migration challenge ahead, that could avoid the need for another contract migration by allowing more options to be encoded in the existing contract without change.

I think these are all good points, and probably worthy of a lengthy discussion in their own right. Perhaps if an int is "good enough" for this PR, that discussion can be deferred to a follow-up issue or FIR? We can make a separate call on whether it's all part of 1.1 (and thus can break/replace all the logic here).

I've opened #846 as a placeholder to continue this discussion.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

internal/coremsgs/en_error_messages.go

pkg/core/identity.go

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

- Replace "mode: multiparty" with "multiparty.enabled: true" - Default to multiparty mode if org name or key is set - Remove previously-deprecated "org.identity" root key - Add new "defaultKey" config to namespaces (valid for both multiparty and gateway namespaces, overrides "multiparty.org.key" for signing) Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

internal/coreconfig/coreconfig.go

peterbroadhurst · 2022-06-01T17:17:27Z

internal/identity/identitymanager.go

-		orgKey = config.GetString(coreconfig.OrgIdentityDeprecated)
-		if orgKey != "" {
-			log.L(ctx).Warnf("The %s config key has been deprecated. Please use %s instead", coreconfig.OrgIdentityDeprecated, coreconfig.OrgKey)
+	defaultKey := im.namespace.GetDefaultKey(namespace)


I think it's important that we do not use the defaultKey in the case of a root org registration.

e.g. when I have an NS config like this:

defaultKey: 0xaaa multiparty: enabled: true org: name: bob key: 0xbbb

Then my organization must be registered as 0xbbb not 0xaaa

Alright, pushed another commit to differentiate and ensure that the "register org" path only looks at the multiparty.org.key, but all other paths should let defaultKey take precedence.

- Replace "node owner org" terminology with "multiparty root org" - Use "default key" first in all identity resolution paths, then fall back to "root org key", except when registering the root org (which will always use the "root org key"). Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

peterbroadhurst

👍 - sorry this one turned got so intertwined with other things. Looks great

awrichar requested review from peterbroadhurst, nguyer and nickgaski as code owners May 10, 2022 14:30

awrichar marked this pull request as draft May 10, 2022 14:30

awrichar added 7 commits May 10, 2022 12:08

Move all identity APIs under /namespaces

1a6ea05

Instead of broadcasting identities globally on the special "ff_system" namespace, they must now be registered on each namespace where you wish to use them. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Clean up some comments and vars in Identity Manager

fc8a3ac

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Clean up which identity APIs are deprecated

ffeb8eb

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Move all identities to namespaces

11a690d

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Replace default namespace with current namespace in /status response

6ff70c9

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Add namespace to identity db indexes

5c7af54

Some fields are now only unique within a namespace. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

awrichar force-pushed the identity branch 2 times, most recently from 5f0b28b to 26fbb0b Compare May 10, 2022 17:30

awrichar marked this pull request as ready for review May 12, 2022 18:04

awrichar commented May 12, 2022

View reviewed changes

pkg/core/identity.go Outdated Show resolved Hide resolved

awrichar force-pushed the identity branch from 26fbb0b to 0877770 Compare May 12, 2022 19:25

awrichar added 4 commits May 12, 2022 15:51

Factor out NamespaceManager

227fb90

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Merge remote-tracking branch 'origin/main' into identity

0ec47ac

Minor coverage fix for Namespace Manager

bc39a53

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Restore spelling of "/network/diddocs" route

5fd6e7e

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

peterbroadhurst reviewed May 13, 2022

View reviewed changes

internal/apiserver/route_get_identity_by_did.go Show resolved Hide resolved

peterbroadhurst reviewed May 13, 2022

View reviewed changes

awrichar added 2 commits May 13, 2022 11:53

Move namespace config parsing to Namespace Manager

df4b932

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Merge remote-tracking branch 'origin/main' into identity

a32c772

peterbroadhurst reviewed May 13, 2022

View reviewed changes

internal/namespace/manager.go Show resolved Hide resolved

peterbroadhurst reviewed May 13, 2022

View reviewed changes

internal/orchestrator/orchestrator.go Show resolved Hide resolved

internal/orchestrator/orchestrator_test.go Outdated Show resolved Hide resolved

pkg/core/identity.go Outdated Show resolved Hide resolved

Replace "legacySystemIdentities" flag with new NetworkVersion query

6ac8749

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

awrichar commented May 28, 2022

View reviewed changes

internal/blockchain/ethereum/ethereum.go Outdated Show resolved Hide resolved

awrichar commented May 28, 2022

View reviewed changes

internal/blockchain/ethereum/ethereum.go Outdated Show resolved Hide resolved

awrichar commented May 31, 2022

View reviewed changes

smart_contracts/ethereum/solidity_firefly/contracts/Firefly.sol Show resolved Hide resolved

awrichar force-pushed the identity branch from c3f30cc to 03ef58a Compare May 31, 2022 14:57

awrichar added 2 commits May 31, 2022 11:15

Use uint8 for network version

8068c7f

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Variable cleanup in blockchain connectors

2d21ccb

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

awrichar force-pushed the identity branch from 03ef58a to 2d21ccb Compare May 31, 2022 15:15

peterbroadhurst added the migration_consideration label May 31, 2022

peterbroadhurst reviewed May 31, 2022

View reviewed changes

Minor optimization of ff_system identity lookup

8ce0786

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

peterbroadhurst reviewed May 31, 2022

View reviewed changes

internal/coremsgs/en_error_messages.go Outdated Show resolved Hide resolved

peterbroadhurst reviewed May 31, 2022

View reviewed changes

pkg/core/identity.go Show resolved Hide resolved

awrichar added this to the v1.1.0 milestone May 31, 2022

awrichar self-assigned this May 31, 2022

awrichar mentioned this pull request May 31, 2022

Update FireFly smart contract(s) to be more extensible for the future #846

Closed

awrichar added 4 commits May 31, 2022 16:16

Merge remote-tracking branch 'origin/main' into identity

341620b

Re-word error message

27c308f

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>

Merge remote-tracking branch 'origin/main' into identity

5205fa6

peterbroadhurst reviewed Jun 1, 2022

View reviewed changes

internal/coreconfig/coreconfig.go Outdated Show resolved Hide resolved

peterbroadhurst reviewed Jun 1, 2022

View reviewed changes

peterbroadhurst approved these changes Jun 1, 2022

View reviewed changes

peterbroadhurst merged commit 698e8b3 into hyperledger:main Jun 1, 2022

peterbroadhurst deleted the identity branch June 1, 2022 21:49

awrichar mentioned this pull request Jun 6, 2022

Fix status, org, and node parsing with latest core hyperledger/firefly-ui#186

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move all identity APIs under /namespaces #804

Move all identity APIs under /namespaces #804

awrichar commented May 10, 2022 •

edited

Loading

codecov-commenter commented May 10, 2022 •

edited

Loading

peterbroadhurst commented May 13, 2022

awrichar commented May 13, 2022

peterbroadhurst May 13, 2022

awrichar May 13, 2022 •

edited

Loading

peterbroadhurst May 13, 2022

awrichar Jun 1, 2022

awrichar commented May 27, 2022

peterbroadhurst May 31, 2022 •

edited

Loading

peterbroadhurst Jun 1, 2022

peterbroadhurst May 31, 2022

peterbroadhurst May 31, 2022

peterbroadhurst May 31, 2022

peterbroadhurst May 31, 2022

peterbroadhurst May 31, 2022

awrichar May 31, 2022

awrichar May 31, 2022

peterbroadhurst Jun 1, 2022 •

edited

Loading

awrichar Jun 1, 2022

peterbroadhurst left a comment

Move all identity APIs under /namespaces #804

Move all identity APIs under /namespaces #804

Conversation

awrichar commented May 10, 2022 • edited Loading

codecov-commenter commented May 10, 2022 • edited Loading

Codecov Report

peterbroadhurst commented May 13, 2022

awrichar commented May 13, 2022

Choose a reason for hiding this comment

awrichar May 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awrichar commented May 27, 2022

peterbroadhurst May 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterbroadhurst Jun 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterbroadhurst left a comment

Choose a reason for hiding this comment

awrichar commented May 10, 2022 •

edited

Loading

codecov-commenter commented May 10, 2022 •

edited

Loading

awrichar May 13, 2022 •

edited

Loading

peterbroadhurst May 31, 2022 •

edited

Loading

peterbroadhurst Jun 1, 2022 •

edited

Loading