headerToken

[durban89]

var headerToken = this.req.get('Authorization'), getToken = this.req.query.access_token, postToken = this.req.body ? this.req.body.access_token : undefined;

console.log('getToken:',getToken); console.log('headerToken:',headerToken); console.log('postToken:',postToken);

getToken: undefined headerToken: postToken: b80ba6196e247b0fae2d9226d2c722a5d01ae90b methodsUsed: 2

question:why headerToken != undefined

[durban89]

Test example blow: curl -XPOST -d 'uid=49792&[email protected]&pass=xxxx&access_token=b80ba6196e247b0fae2d9226d2c722a5d01ae90b' http://localhost:3050/mail/import

[babsonmatt]

+1

[pi-kei]

Yes, it's a bug in aouth2-server authorise.js. this.req.get('Authorization') returns empty string even if there is no Authorization header at all so this block if (methodsUsed === 0) { return done(error('invalid_request', 'The access token was not found')); } will never executed.

[brownstein]

This is not necessarily a bug in authorise - oauth2-server was simply not written with Koa's missing-header-as-empty-string convention in mind, and Koa isn't really responsible for things that people plug into it. I recommend that we wrap Koa's request into another object with a .get method that returns undefined to address this.

[brownstein]

Update: tried it, and proxying the request object definitely breaks things. I'm exploring a change to Koa to make their .get api look like Express's.

[babsonmatt]

Have you tried it with Koa 2 by any chance?

[brownstein]

@babsonmatt I haven't tried it with Koa 2, but it looks like their API is still returning empty strings. My PR to remedy this was rejected, as in all fairness its a potentially breaking API change.

From here we have a couple options; we can try to proxy the Koa requests with a well-behaving .get method, or we can request that the undefined checks in oauth2-server are downgraded to simply being falsy. Which option do you think sounds more viable?

[brownstein]

I've introduced a PR to proxy the request and make .get return undefined - let me know what you think!

[brownstein]

@babsonmatt Any comments on the proxying solution?

[babsonmatt]

@brownstein sorry for the delay in reply, that looks good to me - looks like the Travis CI build failed though?

[brownstein]

@babsonmatt Looks like the build failed due to an environment issue (unable to find an any-promise implementation). The trace is occurring within thenify, which isn't part of this PR's footprint - do you think you could trigger a build against current master, and perhaps a rebuild of the PR, to help ascertain what's going wrong?