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: Add support for new 'x-kick-auth' header. #6

Merged
merged 2 commits into from
Jan 29, 2024
Merged

feat: Add support for new 'x-kick-auth' header. #6

merged 2 commits into from
Jan 29, 2024

Conversation

hparcells
Copy link
Contributor

This PR aims to add an optional x-kick-auth header to each callKickApi() call.

In my case, this resolves the issue of being unable to use the kick-token-provider endpoint when using client.api.authentication.login(). This was due to a CloudFlare challenge before the response of the endpoint.

SOMETHING_WENT_WRONG: Failed to retrieve pre-login tokens

This header was verified using curl with and without the header on the machine receiving the CloudFlare challenge.


I wasn't sure if I should have added the auth token as an optional field inside of the LoginCredentials interface or as an extra kickAuth parameter when calling login(), so I chose the latter.

Copy link
Owner

@zSoulweaver zSoulweaver 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 to me. I don't have this issue on my system at all, so I haven't encountered it. thankfully for me. I'm happy to put this little workaround in until the public API is available for those having issues however.

I assume this header value is manually captured via a browser?

In terms of the PR itself, just a few small nitpicky things.

  • I think renaming kickAuth to kickAuthHeader is a bit more obvious to what that parameter is for, what do you think?
  • I've left a comment regarding my other nitpick in regards to the function parameter. I do agree with you in not adding it to the LoginCredentials interface.

src/endpoints/authentication/authentication.endpoint.ts Outdated Show resolved Hide resolved
@hparcells
Copy link
Contributor Author

Resolved!

@zSoulweaver zSoulweaver merged commit 0b320b0 into zSoulweaver:master Jan 29, 2024
@zSoulweaver
Copy link
Owner

zSoulweaver commented Jan 29, 2024

LGTM.

Sorry for delay, I'll go ahead and publish a new version shortly.

Edit: v1.0.1-12 published to npm

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.

2 participants