Skip to content
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

identity-service-api: Add store_invitation endpoint. #631

Merged
merged 6 commits into from
Jun 27, 2021

Conversation

Frinksy
Copy link
Contributor

@Frinksy Frinksy commented Jun 15, 2021

Resolves: #333

Since we already have an invitation module, I thought I would put it there instead of in invite::store_invitation::v2.

Signed-off-by: Adam Blanchet <adamblanchet@free.fr>
@Frinksy Frinksy requested a review from iinuwa as a code owner June 15, 2021 16:25
Copy link
Member

@iinuwa iinuwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the comment about Medium, I also noticed two other things:

The spec for this endpoint says:

In addition to the request parameters specified below, an arbitrary number of other parameters may also be specified. These may be used in the invite message generation described below.

I think that means that the JSON object can contain any arbitrary parameters at the top level. We should support de-/serializing those properties.

Earlier in the module documentation, it also says:

An identity server can store pending invitations to a user's 3PID, which will be retrieved and can be either notified on or look up when the 3PID is associated with a Matrix user ID.

At a later point, if the owner of that particular 3PID binds it with a Matrix user ID, the identity server will attempt to make an HTTP POST to the Matrix user's homeserver via the /3pid/onbind endpoint. The request MUST be signed with a long-term private key for the identity server.

When it says "the request MUST be signed," is that referring to the store-invite request, or the /3pid/onbind request? If the former, perhaps we should handle that signature here as well.

@jplatte
Copy link
Member

jplatte commented Jun 17, 2021

I think that means that the JSON object can contain any arbitrary parameters at the top level.

Oh, that again 🙄

@Frinksy Would be good to handle this the same way as we handle custom capabilities in ruma-client-api/src/r0/capabilities.rs (that is, private #[serde(flatten)]ed field plus get and set methods).

@Frinksy
Copy link
Contributor Author

Frinksy commented Jun 18, 2021

When it says "the request MUST be signed," is that referring to the store-invite request, or the /3pid/onbind request? If the former, perhaps we should handle that signature here as well.

I'm pretty sure that it is the /3pid/onbind request which must be signed. The request contains a signature from the identity server:

Required. Signature from the identity server using a long-term private key.

This seems to be in accordance with the Invitation storage docs

At a later point, if the owner of that particular 3PID binds it with a Matrix user ID, the identity server will attempt to make an HTTP POST to the Matrix user's homeserver via the /3pid/onbind endpoint. The request MUST be signed with a long-term private key for the identity server.

@iinuwa
Copy link
Member

iinuwa commented Jun 23, 2021

I'm pretty sure that it is the /3pid/onbind request which must be signed. The request contains a signature from the identity server:

Sounds good!

@jplatte jplatte requested a review from iinuwa June 24, 2021 14:28
Copy link
Member

@iinuwa iinuwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@iinuwa iinuwa merged commit f43c9c6 into ruma:main Jun 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

identity-service: Add invitation storage endpoint
3 participants