intercom / intercom-go

Go Bindings For Intercom
https://developers.intercom.io/reference
Other
70 stars 78 forks source link

AppID and APIKey HTTPClient fields are swapped when creating a new client #74

Closed tiwillia closed 6 years ago

tiwillia commented 7 years ago

Version info

Expected behavior

The HTTPClient has an AppID field and an APIKey field. The code suggests that NewClient accepts two parameters, an AppID first and an APIKey second. I expect to create a new client with the following and for it to work:

ic := intercom.NewClient(appID, accessToken)

Actual behavior

The usage of the AppID and APIKey fields in the IntercomHTTPClient appear to be swapped. The app ID provided is sent to intercom as the APIKey, and the APIKey sent as the app ID

Steps to reproduce

  1. Create an intecom client with the following, replacing appID and accessToken with the application ID and access token:
    ic := intercom.NewClient(appID, accessToken)
    1. Attempt to perform any action on the intercom environment

I've provided a small test script that demonstrates the issue:

package main

import (
    "fmt"
    intercom "gopkg.in/intercom/intercom-go.v2"
    "time"
)

func main() {
    //accessToken := "accessToken"
    //appID := "appID"

    accessToken := "YOURACCESSTOKEN"
    appID := "YOURAPPID"

    user := intercom.User{
        UserID:     "sometestuser2",
        Email:      "sometestemail2@gmail.com",
        Name:       "Test User2",
        SignedUpAt: int64(time.Now().Unix()),
    }

    // Create an intercom client using the appID as the fire paramter and the accessToken as the second, as the code suggests is correct
    ic := intercom.NewClient(appID, accessToken)
    _, err := ic.Users.Save(&user)
    if err != nil {
        fmt.Printf("Error encountered attempting to save user: %s\n", err)
        err = nil
    }
    fmt.Printf("Intercom Client HTTPClient: %+v\n", ic.HTTPClient)

    // Create an interocm client using the appID as the second parameter and the accessToken as the first, as the README sorta suggests
    ics := intercom.NewClient(accessToken, appID)
    _, err = ics.Users.Save(&user)
    if err != nil {
        fmt.Printf("Error encountered attempting to save user with swapped parameters: %s\n", err)
    }
    fmt.Printf("Swapped Intercom Client HTTPClient: %+v\n", ics.HTTPClient)
}

The README appears to suggest the proper way to create the client, providing the accessToken as the first parameter to NewClient. However, NewClient is written in a way that describe the parameters to be swapped.

choran commented 6 years ago

Hi @tiwillia, We are no longer using API keys for authorization. Instead we are using access tokens. Via the API directly you can use the bearer token header but we have not update all the SDKs yet to use bearer tokens. But it is implemented so that you can use new access tokens in via basic auth which was used for api keys. This means the access token should be used in place of the appID and the second field should be empty: ic := intercom.NewClient(os.Getenv("AT"), "") Maybe I am missing something but are you saying this is not working when you implement it as described in the README? That should work. But as noted, we are trying to update the SDKs in relation to bearer token headers so that would replace the basic auth method. Let me know if you Thanks Cathal

choran commented 6 years ago

I am going to track thus under #61 since it relates to bearer token and API access. If you have any other questions on that please update that issue