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

feat(0035): update to include account identification #525

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions 0035-ilp-over-http/0035-ilp-over-http.md
@@ -1,6 +1,6 @@
---
title: ILP Over HTTP
draft: 2
draft: 3
---

# ILP Over HTTP
Expand Down Expand Up @@ -32,6 +32,7 @@ Host: example.com
Accept: application/octet-stream
Content-Type: application/octet-stream
Authorization: Bearer zxcljvoizuu09wqqpowipoalksdflksjdgxclvkjl0s909asdf
ILP-Account-Name: alice
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we call this ILP-Account-Id instead?

Copy link
Author

Choose a reason for hiding this comment

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

I prefer the id suffix over name. I know @adrianhopebailie was a fan of using ilp-peer-id. Do we have a consensus on peer vs account

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use account here, but perhaps it's a naming ambiguity thing. If peer == account, then it doesn't matter.

However, in my mind, peer != account because a peer could have multiple accounts potentially (e.g., imagine you and me are peered with one credential that can operate across currencies/accounts).

However, in ILP, I don't think we should introduce that type of peering because it's much simpler to just have accounts. So, I vote for account and we can (maybe never) use the peer term in the future.

Copy link
Member

Choose a reason for hiding this comment

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

For all the reasons @sappenin lists above I agree that peer != account.

But, in this context I think we are talking about a peer not an account 😄

Copy link
Contributor

@sappenin sappenin Apr 9, 2019

Choose a reason for hiding this comment

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

But, in this context I think we are talking about a peer not an account

Can you clarify why? I'd like to not introduce peering into ILP if we can help it. In my mind it's actually simpler and more secure to only have accounts, with each account using a different auth credential.

Copy link
Member

Choose a reason for hiding this comment

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

I'd like to not introduce peering into ILP if we can help it

Isn't peering what ILP is all about?

I guess in this context we should be clear about what "type" of account we're talking about.

E.g. If I have an "account" with a an ILSP then I have both a "peer/user account" (identified by my peer id or something) and a "financial account" (which may be identified by some other identifier or just my peer id) which is the record of my balance.

Therefor I think we need to distinguish between my peer id and account id and in the context of sending an ILP packet to a peer I want to identify myself using my peer id and let my peer decide which financial account my packets impact.


< Body: Binary OER-Encoded ILP Prepare Packet >
```
Expand All @@ -40,7 +41,8 @@ Authorization: Bearer zxcljvoizuu09wqqpowipoalksdflksjdgxclvkjl0s909asdf

- **Path** - A connector MAY specify any HTTP path for their peer to send ILP packets to.
- **Host Header** - The standard [HTTP Host Header](https://tools.ietf.org/html/rfc2616#section-14.23) indicating the domain of the HTTP server the Request is sent to.
- **Content-Type / Accept Headers** - MUST be set to `application/octet-stream`.
- **Content-Type / Accept Headers** - MUST be set to `application/octet-stream`.\
- **ILP-Account-Name** - An optional header used to identify the account for which the request is being made.
Copy link
Member

Choose a reason for hiding this comment

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

Could you expand on what this header is for? I'm wondering about a couple of points:

  • When you say it's optional, does that mean that it's optional from the perspective of this protocol in general or optional from the perspective of every ILP-over-HTTP server?
  • If a server is set up to use this header, how should it behave if that header is not present?
  • How do you find out this value? Is that another field that needs to be communicated when peering?
  • How do you know whether a peer requires this or not (if it can be required) and do all client implementations need to support sending this field?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should either close this PR or else move this header into a section that details (non-normatively) how peers might use extra headers to help with authentication.

For example, the current Java implementation of ILP-over-HTTP supports a JWT authentication mode where the entire token can trivially be used in a performant manner without any additional headers.

Also, read more about my current opinion here: https://forum.interledger.org/t/connector-auth-model/517/4?u=sappenin

Copy link
Member

Choose a reason for hiding this comment

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

Agree. I don't think having this as a normative requirement makes sense.

I do think we should have at least one auth scheme that all implementations MUST support and this should be a well defined standard.

That manifests as some changes to the "Authentication" section to include some normative requirements. I'll write those up.

- **Body** - ILP Packet encoded using OER, as specified in [RFC 27: Interledger Protocol V4](./0027-interledger-protocol-4/0027-interledger-protocol-4.md).

### Response
Expand Down