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

Client.cs uses HttpBasicAuthenticator which appends invalid header #107

Open
ladotonia opened this issue Mar 30, 2018 · 5 comments
Open

Comments

@ladotonia
Copy link

HttpBasicAuthenticator appends the header:

        public HttpBasicAuthenticator(string username, string password)
        {
            var token = Convert.ToBase64String(Encoding.UTF8.GetBytes(string.Format("{0}:{1}", username, password)));

            authHeader = string.Format("Basic {0}", token);
        }

while all the api docs suggest using the header -H 'Authorization:Bearer <access_token>'

Are access tokens supported in this SDK? or does the authenticator need to be updated to support the updated api?

@kmossco
Copy link
Contributor

kmossco commented Mar 31, 2018

Hey @ladotonia! This library already supports Access Tokens through this part of the code:

protected virtual IRestClient BuildClient()
{
RestClient client = new RestClient(URL);
if (!String.IsNullOrEmpty (AUTH.AppId) && !String.IsNullOrEmpty (AUTH.AppKey))
client.Authenticator = new HttpBasicAuthenticator (AUTH.AppId, AUTH.AppKey);
else
client.Authenticator = new HttpBasicAuthenticator (AUTH.PersonalAccessToken, String.Empty);
return client;
}

It works correctly if you send the access token as the app_id with the password as an empty string, which is what this library is doing for now. We have plans to make sure it matches what the API docs refer but considering we had API keys until recently, it was postponed until we deprecate API keys for good.

Are you having problems sending requests?

@ladotonia
Copy link
Author

Hi @kmossco, I did try using the single argument constructor for Intercom.core.Authentication using only my access token.

I am unable to create a new Contact or User through the .net SDK, however when I directly hit the rest api the same command is successful. The main difference I was noticing, was that the api mentions to send the access key in plain text as Bearer <access-key> where the HttpBasicAuthenticator converts <accesskey>: to a base64 string and sends it as Basic <base64-string>.

Here is my sdk code snippet:

ContactsClient intercomClient = new ContactsClient(new Intercom.Core.Authentication(accessToken));
Contact user = intercomClient.Create(new Contact() { user_id = id, email = email});

user is always null, as is the resturn from intercomClient.List();

@kmossco
Copy link
Contributor

kmossco commented Apr 3, 2018

Hey again @ladotonia! Just to be sure, is Intercom.Core.Authentication a method you created? That doesn't seem to be how you instantiate the authentication in our library. Here's the correct code:

ContactsClient intercomClient = new ContactsClient(new Authentication("MyPersonalAccessToken"));
Contact user = intercomClient.Create(new Contact() { user_id = id, email = email });

Is it possible that you are using some other library than ours? I also can't find the <accesskey>: in our codebase.

@ladotonia
Copy link
Author

Intercom.Core.Authentication is just the full namespace path to the same class Authentication that your code is using, so my snippet is functionally the same as yours.

The "<access-key>:" bit is just the result of what the HttpBasicAuthenticator Base64 encodes into the auth header if it is constructed as new HttpBasicAuthenticator("<access-key>",String.Empty); This is a result of the String.Format() call that's in my original issue comment.

@kmossco
Copy link
Contributor

kmossco commented Apr 7, 2018

Gotcha, the reason I asked is that there is an unofficial client called Intercom.Core.Client and I was wondering if that was related. You are right in that we are not appending the best headers right now as the initial change made to accept access tokens was simply passing it as the app_id identifier and keeping the api_key field empty. This is fully supported and we have no plans to deprecate it but I agree that we should make sure we send the right header.

I'll add this to the roadmap of things to get fixed, but if you have the time/resources feel free to send us a PR to address this. Happy to review it and thank you for reporting this!

Just to set the proper expectations, we are currently looking to push the PR #97 forward and then we will focus on bringing pagination to the SDK. So I will probably only have time to address this after that is implemented. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants