Incorrect access token should not be allowed

[AvaniVerma]

Steps to reproduce

1. Add anything random as access token

Result : We get a screen saying fetching results. However, all that happens is failed requests.

In ideal conditions, we should see an error and ask the user to re-enter the access token.

[mayakarabula]

Hello @AvaniVerma

I looked into this issue but I saw that you put v2 tag on it. In v2 you are using passport js and call /user address to get user information and token for github. To some extent it ensures that the token is correct otherwise you won't login. Nonetheless you can put extra check by calling general github API url https://api.github.com/?access_token=${accessToken} when token is incorrect it returns 401 error, otherwise 200. So you can put Error: token is invalid instead of sending many requests.

However I assume you meant that this bug occurs in v1, which is true and can be fixed with a call to general github API url like before. Here is an example code:

https://github.com/curiositylab/curiosity/blob/master/js/main.js#L93

const url = `https://api.github.com/?access_token=${token}`;
axios({
    url,
    method: 'get',
    responseType: 'json',
}).then(() => {
    localStorage.setItem('accessToken', token);
    resolve();
}).catch(() => reject('Error: invalid token'));

I also found a bug in v1 that when you write the token for the first time you don't save USERNAMES in localstorage, this should be done before getData

Would you like me to create a PR to fix it in v1 or v2 or maybe both?

[mubaris]

Doing a single PR for both will be better

[mayakarabula]

Hi @mubaris I don't really understand what you mean here by a single PR for both, the code for v1 and v2 is different now so it have to be separate commits (not cherrypicked)

[kriswep]

wouldn't it be nicer if an OAuth app would be set up? Then users wouldn't have to generate acces tokens and wonder over necessary scope settings at all.

[simeg]

It should say in the README what scopes is required for the token. I can't find this information anywhere and it's very important. If a user doesn't know what scopes to give he/she might give all scopes which gives this application unnecessary power.

[adwsingh]

Is this still open?

[mubaris]

@mehwhatever Yes

[mubaris]

@simeg You can create a token without any scopes

[technolaaji]

const getData = function getData() {
    document.getElementById('searching').innerHTML = '<br/>Fetching projects...';
    usersCurrentCall = 0;
    callInProgress = true;
    reqNo += 1;
    USERNAMES.forEach((username) => {
        const url = `https://api.github.com/users/${username}/starred?per_page=${projectsPerPage}&access_token=${accessToken}&page=${reqNo}`;
        axios({
            url,
            method: 'get',
            responseType: 'json',
        }).then((response) => {
            dataCollector(response, username);
        }).catch((err) => {
            console.error(err);
        });
    });
};

so this is the function that calls for the projects for a specific user right? but the issue is that you are always getting fetching projects if a user provided an incorrect access token

keep in mind that if you provided a incorrect access token then it should be appeared in the console.error that is inside of the axios catch

first you should clear the child inside of the element searching so that the 'fetching products' text goes away https://www.w3schools.com/jsref/met_node_removechild.asp

then you should insert in the inner html of searching 'incorrect access token' or 'something went wrong please try again!'

so it should be something like this:

const getData = function getData() {
    document.getElementById('searching').innerHTML = '<br/>Fetching projects...';
    usersCurrentCall = 0;
    callInProgress = true;
    reqNo += 1;
    USERNAMES.forEach((username) => {
        const url = `https://api.github.com/users/${username}/starred?per_page=${projectsPerPage}&access_token=${accessToken}&page=${reqNo}`;
        axios({
            url,
            method: 'get',
            responseType: 'json',
        }).then((response) => {
            dataCollector(response, username);
        }).catch((err) => {
            let searching = document.getElementById('searching');
            searching.removeChild(searching.childNodes[0]); // assuming it is the first element so it removes the 'fetching projects' text
            searching.innerHTML = '<br /> Something went wrong! please try again later'
            console.error(err);
        });
    });
};

this is a basic example but if you want to be specific, you can either innerHTML the error being catched by axios or you can receive the error and do some validation

hope my answer helped a bit