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

[NT-692] Prepare Data Lake Request with auth headers #997

Merged
merged 1 commit into from
Dec 17, 2019

Conversation

justinswart
Copy link
Contributor

@justinswart justinswart commented Dec 17, 2019

πŸ“² What

Prepares requests sent to our Data Lake tracking endpoint with authentication headers.

πŸ›  How

Pass the request to our Service.preparedRequest() function before returning it to the client.

βœ… Acceptance criteria

Inspect requests in Charles:

  • Observe that Data Lake requests include the client_id and Authorization header.

@justinswart justinswart merged commit 5971b24 into master Dec 17, 2019
@justinswart justinswart deleted the datalake-authentication-headers branch December 17, 2019 23:38
Copy link
Contributor

@ifbarrera ifbarrera left a comment

Choose a reason for hiding this comment

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

I think this looks good, my only hesitation is maybe wondering whether we're now sending too many headers - the data lake doesn't necessarily an auth header, really just the basic headers like the client_id and the Kickstarter-iOS-App etc. πŸ€” Maybe at a later date we can refactor our default headers to decouple the authentication headers from the basic ksr headers required to validate the client sending the requests.

@eoji
Copy link
Contributor

eoji commented Jan 22, 2020

Hello, it's me with an annoying question πŸ‘€the client_id is a query param and not a header, right? Wanted to make sure I'm doing the same when I open my PR

@justinswart
Copy link
Contributor Author

That's correct, client_id is a query param!

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.

None yet

3 participants