-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Discovery Refactoring - Derived from JOSE replacement #493
Comments
For This interactions shouldn't be kept in the service layers. The service layers are meant to be thin wrappers. |
The |
Oh I just realised |
How exactly do we tell claims apart? We have a This is not so much a problem for our local sigchain since part of this spec is to have the But for remote nodes we can only get the stream of claims from their sigchain without the context. Now claims can contain arbitrary information so we can just add a type property to them. |
There are 2 options:
The second way is much easier for us to deal with and it disambiguates inside the claim payload... whereas the first way fits the JWT spec better, and it is part of the entire signed claim, not the claim. They don't conflict, both could be done at the same time. But for now I think the first way is easiest. Do note that you should be using The |
The discovery logic is doing a weird extra level of logic here. When we process a vertex we request the chain data for that vertex. We then iterate over these claims, verify them, add them to the For some reason we are also requesting the chain data for every node found in these claims and schedule them as well. That should be handled when the original claim is processed. So we're kind of doing this twice here? I don't see a reason for it and it's making the logic here hard to follow in what is already a very long section of code. I'm just going to remove it. |
The entire discovery logic can be rewritten. It's significantly legacy now that the GG and Sigchain have been changed. |
For
For
For In the
In terms of the TTL for the discovery, that can be done separately later. But the nodes and identities should be re-visited according to a priority and TTL policy. #462 |
The refactoring is almost done now. The only build problems left are from the notifications and sessions. I'll have to do something about these before I can do the manual testing. Sessions should be a quick fix but the notifications might be a little trickier since they're using JSON schemas. I'll see if I can stub out the build problems, otherwise I'll have to refactor it with the token changes. |
Did you add an additional method to |
I didn't need to, Both sides were already creating separate claims and singing them for each other. So I replicated that behaviour when refactoring. |
How does that work? It has be the same claim. Wait... are we saying there are 2 separate claims that each is signing? So node 1 has made a claim, that is doubly signed. And node 2 has made a claim, that is doubly signed. I think I remember that there was some debate about this... |
So it is actually 2 separate claims because the sigchains are not shared. Things like the claim Id and the previous hash is different and therefore cannot be shared between the 2. So it's like having 2 copies of a contract, and both contracts are doubly signed (but with additional information on each contract that is a bit different). Therefore we need to ensure some level of commonality between the 2 claims. The issuer and subject should be the same for both claims. The node that initiates the claiming process is always the issuer. And the responding node is always the subject on both sides of the sigchain. Furthermore, this means the This ensures an unambiguous order in the GG, so that when the GG is synchronising, we can easily compare what is different. |
For the Remember that from a permissions perspective, our nodes cannot just allow any node to initiate the Therefore, when Node 1 wants to link up with Node 2, it actually has to 2 things:
For Node 2, the end user has to "act" or ignore the
If If The 2. solution results in extra notifications interaction. This does involve extra work, and it is difficult for the Polykey agent to show the notification, unless we register the Polykey agent to desktop toasting systems. The 2. solution also actually allows the entire gestalt of node 1 to link with node 2, it isn't just the node 1. This is because permissions are at the level of gestalts, not nodes. I think the ultimate solution is combination of 1. and 2. However for the purpose of our manual testing, let's just get 1. done first. |
I'm doing the manual testing for claiming an identity. Right now it's responding with a bad request. I don't know why though. So far as I can tell the URL is correct.
When I change the scope to just gist to see if it's formatting I still get a |
Ensure that the client ID is correct. |
The client ID is correct. I forgot that the application is in fact managed by Matrix AI org. The thing that changed was that GitHub now requires explicit opt-in for device flow. So I enabled it now. Note that the client secret is not necessary for the device flow. This technically means any application can try to masquerade as the Polykey GitHub application... but users should be warned not to trust the Polykey github app, if they aren't using it through the official Polykey application that we are releasing. |
You should catch this exception though:
And explicitly report it in our |
That is just some debug output. It is caught and handled. |
To claim a node, first we need to authenticate with github.
This will direct us to a screen to enter the provided code. After entering the code you are presented with a screen to review the requested permissions and the option to authorise it. After authorising you are shown a confirmation. You can close the page here. After all that it should be authorised. The next step is to claim the identity.
You should now have a gist that contains the Token corresponding to the claim and the claim is added to the sigchain. |
The GG should have that claim added as a |
And the cryptolink.txt file will be a nicely rendered markdown file in the future too. This can just use regex and a static template. It doesn't require any fancy markdown parser. We just need a useful unambiguous identifier in the markdown template to look up. |
Comparing the However But thinking about this further, it seems the User Agent can also not just be a browser. In the case of node to node links. We can imagine that the PK client is the user agent here. And there are 2 realisations of this: the Polykey CLI and Polykey GUI. Therefore a similar UX is possible, if during the Of course this assumes that the 2 nodes are owned by the same entity. This is not always true. It is possible the second node requires a completely separate authentication to accept the linking action. Therefore we can afford to make the UX a little more explicit compared to the claim identity process. |
I need to go over the nodes claiming process and how the notifications and permissions factor into it. I think we just need to allow a node to form a claim with us. At most that just needs to be an ACL permission for it. alternatively we could issue a token that allows someone to claim us. Such that if you come to me trying to form a node claim, you need to provide a token signed by me that authorizes you to do so. For either way the notifications doesn't really need to be used except to inform the other node that you've allowed them to claim you. It doesn't need to be similar to the identity claim method since that's limited by the OAuth flow. |
You need to use the notifications because otherwise the other node will not know if the linking process is legitimate or not. It's an inherent part of the entire flow. The OAuth flow provides a nice UX for authenticating the node claiming process. It should give us a framework for any flow that allows a user that controls 2 nodes to easily link them together. The notification can actually contain the token that allows the reverse node linking to succeed. This would be the same as enabling the permission on the ACL for the gestalt, although it does provide a finer grained process, that forces the other node to use the token in the notification. However we don't have any facility for this yet, because we don't really have a access token system available except under the session system. Remember in the issue #472 the So we shouldn't go with an access token yet because the token management is not yet designed. |
Let's go with: # sends the vault share notification (and sets the permission)
pk vaults share
# does the actual pulling
pk vaults pull
# sends the gestalt invite notification (and sets the permission)
pk gestalts invite
# links up the gestalts, and triggers `claimNode`
pk gestalts link We are separating the sending of the notification from the actual linking process. Note that I'm not sure if we should have these as part of We have to see how all the commands work together. This does mean attempting to link where you don't have the permission to should result in a permission failure. |
so currently when you do a nodes claim the node checks if it has a gestalt invite notification from that node. If it doesn't it sends an invite notification to the other node. If it does it attempts to claim the node. Right now no permissions are applied besides the notifications permission or needing a permission to even try. Nothing would prevent you from claiming a node if you call So some changes that need to be made.
|
I think for 4. we keep it explicit, and so The automatic UX can be done at the GUI level, but for CLI commands, let's make it explicit. |
Claiming nodes is working now. Since we're doing this locally without any seed nodes, we need to add the connection information to the node graph first
Attempting to claim a node at this point will result in a permission failure. I need to update the failure message here.
I need to trust the other node before I can receive notifications.
I can then allow claiming on the other node.
We can now claim the other node.
|
So the flow is a bit complicated right now:
However steps 2 and 3 can happen without step 1. Step 1 is only necessary for the user to see the notification on N2. But if it's the same user that controls N1 and N2, they don't really need to see this. Note that once N1 and N2 are linked, the trust relationship is no longer relevant, since all nodes trust each other within the same gestalt. |
This can lead some improvements for the notifications in the future. Notifications get dropped by N2 if it doesn't trust N1. But N1 should indicate whether the notification was successfully sent or dropped due to permission failure. This allows the user of N1 to resend the notification in the future. |
Can you get the list of all identities and nodes subcommands here so we can compare. |
Nodes commands
and identities
|
I'm starting to see that having a separate
There are a few ambiguities here.
We can sort this out in the future issue. I think the |
Specification
This spec is less well-baked compared to the
GestaltGraph
because this is my initial review of the discovery domain.The discovery domain depends on:
NodeManager
for connecting to nodes and asking about their sigchainTaskManager
to run background tasksGestaltGraph
to update the links they discoverKeyRing
to know if we are finding our ownNodeId
.Sigchain
to add its own sigchain into theGestaltGraph
There are several major changes here required:
GestaltId
instead ofGestaltKey
The
GestaltGraph
now usesGestaltId
, it does not expose theGestaltKey
, all operations must now use theGestaltId
when interacting with theSigchain
.Querying the
Sigchain
of remote nodeWhen connecting to the remote node in order to discover their sigchain, it must be understood as a stream of claims. Rather than one giant claim array. This means instead of calling
NodeManager.requestChainData
, it should be using the NCM and connecting to it.We know that the
NCM
is a supervisor over node connection pools. It should be the primary way by which downstream domains attempt contact to other nodes and to call them.The
NM
by contrast can also use theNCM
and expose a variety of high-level functionality over thenodes
. However in this case, the only user ofNodeManager.requestChainData
is theDiscovery
and it makes more sense for theDiscovery
to directly operate theNodeConnection
and its encapsulated GRPC client.Therefore the
Discovery
should be relying on theNodeConnectionManager
and not theNodeManager
.On the GRPC service handler, the
nodesChainDataGet
should be changed to a server stream. This stream will yieldSignedClaim
.On the other side, when querying the own
Sigchain
, it should not just callSigchain.getClaims
, but get only the "latest" (and unrevoked) claim per node ID or identity ID. To do this, theSigchain
will need some sort of indexing to keep track of this (#327).Processing own Node
With respect to #319, it should be processing its own sigchain immediately on start and adding such data to the
GestaltGraph
.The
Sigchain
can be mutated by itself during the claiming process for node to node and node to identity. This claiming process cannot be initiated by theSigchain
.This part is currently a bit confusing.
For node to node claims, this all starts in
NodeManager.claimNode
, which itself ends up calling theSigchain
.But for node to identity claims, this is done by the GRPC handler
identitiesClaim
which orchestrates operations between theSigchain
andProvider
.We need to clarify on a few things here:
Discovery
be responsible for managing the node's ownSigchain
and putting its data into theGestaltGraph
?Discovery
crawl itsGestaltGraph
and does it also try to discover its own vertex?identities
domain just like claiming a node is in thenodes
domain.Sigchain
is not aware of theGestaltGraph
and theGestaltGraph
is not aware of theSigchain
. This is good, because we can do object composition here. Given that any claiming process will involve theSigchain
, should theGestaltGraph
also be updated by the claiming process?Here's one possible answer to the question:
Here you can see that claiming an identity or a node is what ends up updating the
GestaltGraph
. It's not a reactive thing, because claiming updates both our ownSigchain
and our own gestalt in theGestaltGraph
.The
claimNode
andclaimIdentity
are now part of the domain models. In the case ofclaimIdentity
, this needs to be part of theProvider
. In fact, as an abstract class, we would expect that these dependencies to be injected into theProvider
for it to be used during the claiming process. This may not be entirely good design if we expect provider plugins. In the case of plugins, we have 2 choices, we are either calling into it, or it is calling into us. If we give it theSigchain
andGestaltGraph
, we are giving it quite a lot of power, thus allowing it to call into us, is like providing an entire scripting environment. A safer model is that we call into it. In that case, it would be part of theIdentitiesManager
.Note that the diagram above renames
IdentitiesManager
asProviderManager
atm because it doesn't do anything except managerProvider
. But if the claiming process is a "don't call us, we'll call you", then it can be left asIdentitiesManager
.Finally
Discovery
does not actually need access to theSigchain
nor to theKeyRing
. The discovery does not need discover its own node, because any changes to its own node's gestalt would be reflected immediately during theclaimNode
andclaimIdentity
operations. The discovery only needs to discover the node's own neighbours, which can it do by querying the gestalt graph and prioritising based on TTLs (which are yet to be implemented). There are still some questions as to how does the discovery become aware about the new nodes in the gestalt? This can either be push-based or pull-based. I believe right now it is pull-based, which means there's a timer-based polling onto theGestaltGraph
and we have talked about making theDiscovery
reactive to theGestaltGraph
.Deliberate Operations for
Discovery
It needs to be reviewed how exactly users can directly tell the
Discovery
to start its discovery process against a givenNodeId
orProviderIdentityId
.GestaltLinkNode for doubly signed claims
See: #493 (comment)
Notifications for Claiming Nodes
See: #493 (comment)
Additional context
tokens
domain and specialise tokens for Sigchain, Notifications, Identities and Sessions #481Discovery
process #462Tasks
GestaltKey
usage withGestaltId
.GestaltGraph
when claims are made. This is done for both claim node and claim identity.claimNode
needs to be a part of the nodes domain along with the handler logic for it. The logic behind this needs to be refactored to use the new claims/token changes.claimIdentity
logic and handler needs to be apart of theIdentityManager
orProvider
.The text was updated successfully, but these errors were encountered: