alexanderwe / the-traveler

The Traveler is a small npm package which wraps around the Destiny 2 API.
https://alexanderwe.github.io/the-traveler/
MIT License
56 stars 10 forks source link

Error with getAccessToken() #46

Open Jakosaur opened 5 years ago

Jakosaur commented 5 years ago

Bug Report

Your Environment

Software Version(s)
the-traveler 1.0.1
Node 10.16.0
Operating System Windows 10

Expected Behavior

obtain the OAuth Access token with the getAccessToken() method

Current Behavior

image

Code Sample

Using the sample given in the documentation traveler.oauth.getAccessToken(hereComesTheCode).then(oauth => { // Provide your traveler object with the oauth object. This is later used for making authenticated calls traveler.oauth = oauth; }).catch(err => { console.log(err) })

Just changed hereComesTheCode to my own variable which has the code stored.

Jakosaur commented 5 years ago

Also, just a question. If I've given the Traveler object my clientID - image

should I need to parse my clientID again when using generateOAuthURL? Doesn't seem to work without parsing my clientID. traveler.oauth.generateOAuthURL(settings.api.clientID)

Was just following the documentation and it didn't show parsing the clientID 🙂

const authUrl = traveler.oauth.generateOAuthURL(); // The URL your users have to visit to give your application access

Not sure if it's possible for it to read what you've already given to the object, that'd be really convenient, or whether the documentation just needs a quick update. Thanks again 🙂👍

alexanderwe commented 5 years ago

@Jakosaur Thanks for your issue. Indeed there seems to be an issue with the OAuth methods. Additionally I should update the documentations to clear things up:

Unfortunately I do not have time to fix the issue this weekend.. I hope I can get to work on it until next Sunday, hope that does not causes some problems on your side. But you are also free to open a PR if you already found a fix for it :)

Jakosaur commented 5 years ago

You are also right that generateOAuthURL needs the ClientID to be provided again. I will think about if this a good approach or if I should use the clientID set on the traveler object.

If it's possible to use the clientID set on the Traveler object, it'd be more convenient - in my opinion. Further up the documentation for OAuth, the example tells you to give your clientID so would make more sense to not have to give it again 🙂

Jakosaur commented 5 years ago

Haven't had a chance to take a look yet, if I get a chance I'll have a look this weekend to "hopefully" fix this error. 🙂 image

alexanderwe commented 5 years ago

I also didn't have a change to look at it, really sorry for that - I assume it has something to do with the imports of FormData but I am not 100% sure about it

roxaloxa commented 5 years ago

I am also hitting this error in the same scenario, but on macOS 10.14.5.

AdonisJS is kind enough to show the code, presumably it's an import or scoping issue. image

roxaloxa commented 5 years ago

I threw a var FormData = require('form-data') at the top of OAuthResource.js to see if that'd be enough and now I'm getting another error deeper in from the got package: TypeError: The `body` option must be an Object or Array when the `json` option is used

Now I'm out of my depth so I hope this can be fixed soon, would love to use this package. :)

roxaloxa commented 5 years ago

(Apologies for the triple comment!)

I got it working locally by ditching FormData altogether and just manually building an object with client_id, code, and grant_type in it in OAuthResource.js. Presumably there's a benefit to FormData I'm missing, but at least I can move forward with OAuth while this gets looked at. 😃

Jakosaur commented 5 years ago

Just remembered about this issue, haven't had a chance to try and fix it and push a PR. I'll try the solution above 🙂

Jakosaur commented 5 years ago

@roxaloxa I can't seem to get this working for some reason. Able to send a screenshot of the changes you made? 🙂

roxaloxa commented 5 years ago

@roxaloxa I can't seem to get this working for some reason. Able to send a screenshot of the changes you made? 🙂

My app is kinda broken right now 😅 but here's the relevant code from the-traveler/build/resources/OAuthResource.js:

    OAuthResource.prototype.getAccessToken = function (code, oauthClientId, oauthClientSecret) {
        var _this = this;
        // var form = new FormData();
        // form.append('client_id', oauthClientId);
        // form.append('code', code);
        // form.append('grant_type', 'authorization_code');

        // form = {
        //     client_id: oauthClientId,
        //     code: code,
        //     grant_type: 'authorization_code'
        // };

        form = "client_id=" + oauthClientId + "&code=" + code + "&grant_type=authorization_code"

        var options = {
            body: form,
            headers: oauthClientId && oauthClientSecret
                ? {
                    authorization: "Basic " + new Buffer(oauthClientId + ":" + oauthClientSecret).toString('base64'),
                    'content-type': 'application/x-www-form-urlencoded',
                    'user-agent': this.userAgent
                }
                : {
                    'content-type': 'application/x-www-form-urlencoded',
                    'user-agent': this.userAgent
                },
            //json: true,
            //method: "post"
        };
        console.log("----- OauthResource.js");
        console.log(options);
        console.log("----- /OauthResource.js");
        return new Promise(function (resolve, reject) {
            _this.httpService
                .post('https://www.bungie.net/platform/app/oauth/token/', options)
                .then(function (response) {
                resolve(response);
            })
                .catch(function (err) {
                reject(err);
            });
        });
    };

You can kinda see my thought process with what's commented out. Basically ditching the FormData stuff. Back when I made the comment, the object worked fine and something changed since so now I do the manually built string of params. It's not pretty, but it let me move on for the time being.

Jakosaur commented 5 years ago

@roxaloxa Thanks for that! Working perfectly for me 😊