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

Add Attribution Header to wire spec #582

Closed
wants to merge 3 commits into from
Closed

Add Attribution Header to wire spec #582

wants to merge 3 commits into from

Conversation

ntrinquier
Copy link

Before this PR

After this PR

==COMMIT_MSG==

==COMMIT_MSG==

Possible downsides?

@@ -170,13 +170,16 @@ Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/5

_Note, this is more restrictive than the User-Agent definition in [RFC 7231](https://tools.ietf.org/html/rfc7231#section-5.5.3). Requests from some browsers may not comply with these requirements as it is impossible to override browser User-Agent headers._

#### 2.4.4. Header Authorization
#### 2.4.4. Attribution Header
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, did you consider using the existing user-agent mechanism to convey attribution?

Copy link
Author

Choose a reason for hiding this comment

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

I considered it and thought that they could have slightly different meanings, e.g.:

User-Agent: foo/1.12.0
X-Conjure-Attribution: Project foo <foo-mailing-list@bar.com>

or

User-Agent: python-requests/2.2.1
X-Conjure-Attribution: 84273d73-7527-45ac-ac02-c2e61cbdc5b3

I don't feel strongly attached to having a new header.

@iamdanfox
Copy link
Contributor

Please could you flesh out the motivation for this? It seems strange to be including another string header that supposedly identifies the caller - this could easily be forged by other callers (if this is bad for the use case, then I think you should parse the userId out of the authorization bearer token). If forgery doesn't matter, then I think the existing user agent should be sufficient?

@ntrinquier
Copy link
Author

@iamdanfox The high level goal is to identify the functional "entity" responsible for requests. By entity I do not mean a user or a micro-service, but the entire block of micro-services or front-end applications that make up a cohesive whole.

As for the use case: The goal is to be able to safely remove endpoints. In my experience in environments with thousands of users, front-end apps, or micro-services, tying requests to these things is not enough to safely remove endpoints because:

a. users are generally end users who know nothing about requests, so userId is not helpful (this is typically what happens for front-end applications);
b. when they are not end users, they are service users, and it requires human investigation to know who to contact (this is typically what happens for back-end applications).


On forgery:

You are right, this is something to consider. For the use case I described (being able to monitor usage of endpoints to safely remove them), the worst thing that could happen is someone making requests with someone's else's attribution ID, thereby feeding you with incorrect information with regards to who is using what endpoints. However in the context of endpoints removal, this is something I'd be OK with (i.e. if an application breaks because it did not provide its unique ID, too bad...).

It might not be sufficient, however, if this starts to be used for other use cases such as rate limiting or API billing where trusting the "functional identity" is obviously more important.

On user-agent:

It very much looks like this would be OK. My only concern with that would be losing information that can be valuable (e.g. the type of browser/OS).

@stale
Copy link

stale bot commented Apr 23, 2020

This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days.

@stale stale bot added the stale label Apr 23, 2020
@stale stale bot closed this Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants