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

initial WebhookChannel2023 #146

Merged
merged 6 commits into from
Feb 16, 2023
Merged

initial WebhookChannel2023 #146

merged 6 commits into from
Feb 16, 2023

Conversation

elf-pavlik
Copy link
Member

This is initial version following common template using by other subscription types.
We can add separate primer to provide more detail example like in WebhookSubscription2023.

I also created #145 to define unsubscribing on the protocol level, later webhook can further specify it if needed.

Copy link
Contributor

@jaxoncreed jaxoncreed left a comment

Choose a reason for hiding this comment

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

Thanks for this @elf-pavlik . I'll be sure to hop into the meeting next week and we can talk about things in-person as well.

webhook-channel-2023.bs Show resolved Hide resolved
webhook-channel-2023.bs Show resolved Hide resolved

It is beyond the scope of this document to describe how a client fetches an access token.
Solid-OIDC is one example of an authentication mechanism that could be used with Solid Notifications [[!Solid.OIDC]].

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this section is incomplete.

Pointing to Solid OIDC for the first use case (Subscription client MUST perform authenticated subscription request.) is fine as that use case is very clearly a client-pod relationship which is what Solid OIDC was designed to handle.

There are many questions left unanswered about the second use case (Notification Sender MUST perform authenticated request to sendTo webhook endpoint, using identity provided as sender in the subscription response.).

  • Are we saying that a sender's "authenticated request" needs to be Solid OIDC as implied by the Solid Notifications Protocol?
  • If so, what role is the sender? Is the sender the token issuer? Is the sender the token subject?
  • In my opinion, Solid OIDC is an inappropriate Authentication protocol for this use case. It's designed for use cases that involve a user agent, and therefore has more complexities than are needed. This use case is a simple server-to-server communications, but the following unnecessary things would be required if we were forced to use Solid OIDC:
    • An identity provider, not for users, but the send itself
    • The use of DPoP tokens which are not needed because DPoP tokens are designed for ecosystems that have three constituents: Identity Provider, Relying Party, and Resource Server.
  • I've been out of it for a bit. Perhaps Solid OIDC has added the Client Credentials flow. If so, this does solve the DPoP token problem mentioned about as Client Credentials doesn't use DPoP, but it still requires extra complexity because the the sender would need to authenticated with an identity provider before sending notifications.
  • I still prefer the method I outlined in the original document: Just give the sender its own public/private key pair and let it mint a simple token itself without needing to get a Solid OIDC IdP involved. Again, I've been out of it, so if there's an explanation as to why this approach is worse, I'm happy to hear it.
  • But, no matter what approach we go with, I would still prefer to have this section go into a little more detail on how senders send authenticated requests.

Copy link
Member Author

@elf-pavlik elf-pavlik Feb 2, 2023

Choose a reason for hiding this comment

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

@jaxoncreed I agree with all your points, recently I posted relevant comments on gitter
At the same time I don't think the problem is specific for WebHook channel type. I see it as more common requirement to authenticate server-side clients which we should be able to do in consistent way across the Solid ecosystem.
There is also #134 I just suggested that sendTo (former target) could be either a Capability URL or require some specific AuthN/AuthZ.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to add #155 as inline issue. This way we can already start using Capability URL for sendTo, later adding support for Solid-OIDC and other more fitting options.

Copy link
Member Author

Choose a reason for hiding this comment

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

image


1. **Verifiable requests to a notification receiver** - a notification receiver must be able to confirm
if a request truly came from a specific notification sender.
2. **Unsubscribing from a WebHook** - Unlike websockets, where sockets can simply be closed by the client,
Copy link
Contributor

Choose a reason for hiding this comment

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

This document doesn't mention how unsubscribing works. Is it described somewhere else?

Copy link
Member Author

Choose a reason for hiding this comment

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

Below you can see inline issue linking to #145

@jaxoncreed
Copy link
Contributor

@elf-pavlik is the idea to just approve this to have something on the books and then fix the problems later?

@elf-pavlik
Copy link
Member Author

elf-pavlik commented Feb 12, 2023

@jaxoncreed I'd like to add any unresolved issues inline and try to publish 0.1 during next meeting.

EDIT inline issue added #146 (comment)

Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Generally minor tweaks. A number of additional instances of WebhookChannel2023 (and possibly other entities) should also be wrapped in backticks.

webhook-channel-2023.bs Show resolved Hide resolved
webhook-channel-2023.bs Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
Co-authored-by: Ted Thibodeau Jr <[email protected]>
Copy link
Contributor

@jaxoncreed jaxoncreed left a comment

Choose a reason for hiding this comment

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

Given the fact that issues are listed in this spec, I can approve it.

webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
webhook-channel-2023.bs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants