OAUTH very seriously error with fake base64 decode

[psy21d]

meteor/packages/oauth/oauth_server.js

Google it:
(oauth_server.js:78) Unable to parse state from OAuth query: FAKEFAKEKAKE

People get drubbing together with obtaining query.state on many oauth services, because oauth_server.js parse answer as base 64 without disassembling when it is not base 64 at all.

And, no information on internet about this problem.

Please, fix it recently.

[psy21d]

There is:

string: "�Õ�I¨¯­ª¯spä"
query: {
    close: "close"
    code: "711d643a3b6cb7b523"
    state: "HNWASaivraqvc3Dk4"
}

Need:

string: "HNWASaivraqvc3Dk4"

[psy21d]

http://vk.com/dev/auth_sites

[sebakerckhof]

I've mentioned this earlier as part of this problem: #4497 (step 2 in the latest comment)

However, this seems to be the intended behavior for certain flows. E.g. the oauth flow from meteor tool, which only adds the credentialtoken to the query state. But then still, it is a bit weird that an error is displayed in the console.

[psy21d]

Need to check BASE64 string encoding, and if it is not, return clean string.

[sebakerckhof]

It actually does this, but further down the road.

So basically it tries to base64 decode and if this fails, the OAuth._stateFromQuery will throw, which is usually caught in the calling function (e.g. in OAuth._credentialTokenFromQuery), which thens return the clean string (which is the credential token) instead.

So this seems to be correct behavior, but it's a bit weird that this is being done by throwing and logging errors.

[psy21d]

But there is not fails and fails later!!
Bad situation. Maybe need to create settings (options) way for OAUTH services?

[psy21d]

Not weird at all, because it is server console.
Throws and crutches is not proper way where can make a real checking.

[psy21d]

Whats-a-problem to write properly code? Need to fix it!

How I can make pull request?

HNWASaivraqvc3Dk4 is-not-a Base64 string, so simply to check it.
Why you to write try-catch dummies when have a full specification
Your code SIMPLY NOT WORK as need!!

"HNWASaivraqvc3Dk4".length 
17

https://en.wikipedia.org/wiki/Base64
MUST to read.
Use ONLY specification and RFC!

NOT STUPID try-catch,
it is unprofessional to shift responsibility on "something", "further down the road.". No, does not! Its just because was written dummies!

Many people caught this error and "was made nice code and thats work proper"? Actually not.

No, does it not work how need.
I am a beginner, I am no right to prove, but understand it is not operating properly, even to me it's obvious.

Instead, changes in the two lines code we are Arguing. What For?
https://en.wikipedia.org/wiki/Base64

Meteor is big and expensive project and yes, code must be orderly and straight.

Thank you for understanding!

[psy21d]

Pull Request:
on line 66 then:

OAuth._stateFromQuery = function (query) {
  var string;
    var re = /^([0-9a-zA-Z+/]{4})*(([0-9a-zA-Z+/]{2}==)|([0-9a-zA-Z+/]{3}=))?$/
    if (re.exec(query.state)) {
        string = new Buffer(query.state, 'base64').toString('binary');
    }  else {
        return query.state;
    }
  try {
    return JSON.parse(string);
  } catch (e) {
    //Log.warn('Unable to parse state from OAuth query: ' + string);
    //Log.warn('returned ' + query.state);
    return query.state;
  }
};

[psy21d]

so, this code works good.
line 99:

OAuth._credentialTokenFromQuery = function (query) {
  var state;
  // For backwards-compatibility for older clients, catch any errors
  // that result from parsing the state parameter. If we can't parse it,
  // assume that the state parameter's value is the credential token, as
  // it used to be for older clients.
  try {
    state = OAuth._stateFromQuery(query);
  } catch (err) {
    return query.state;  // there is
  }
  return state.credentialToken;
};

Problem closed.

[stubailo]

@psy21d if you would like to submit a Pull Request on GitHub so that we can review and consider merging, follow these directions: https://help.github.com/articles/using-pull-requests/

[adamgins]

related issue #8678