bsphere / node-gapitoken

Node.js Google API service account authorization
51 stars 19 forks source link

Add support for PKCS12 #14

Open Mr0grog opened 9 years ago

Mr0grog commented 9 years ago

This should address #5 and allow developers to seamlessly reference the .p12 file they get from google as the keyFile parameter or use a base64-encoded version (or a buffer) as the key parameter.

I hope this is helpful. At the very least, I know I would have liked to have it when recently working on a project using the official Google API library, which depends on this for auth: https://github.com/google/google-api-nodejs-client

(And, of course, happy to make any tweaks you might like to the way this is done.)

Mr0grog commented 9 years ago

Just rebased this off master since #13 merged in. Should be clean and good to go (assuming you like the approach).

Mr0grog commented 9 years ago

Actually, nevermind. I think this branch is still good, but I re-ran my tests and #13 and it seems to have broken things. Specifically, qs is required, but not listed in package.json. The “qs” module (https://www.npmjs.com/package/qs) doesn’t have an API that matches what’s in use, so I’m not sure what this is supposed to be.

Mr0grog commented 9 years ago

Ok, rebased again; used it successfully to log in and acquire a token from Google.

Mr0grog commented 9 years ago

@bsphere Let me know if there’s anything I can do to make this easier to evaluate for you (or if you just don’t want this functionality built-in), since it’s been sitting for a while.

bsphere commented 9 years ago

I think that you should add this functionality as an option, to maintain backward compatibility with those who already use a .pem file

Mr0grog commented 9 years ago

Oh, that's exactly how it's intended to work; sorry if that wasn't clear. decodeKey() attempts to determine whether we got the content of a .p12 or a .pem and do the right thing: https://github.com/bsphere/node-gapitoken/pull/14/files#diff-fec3b0b3d092a4d91d0a6b7ca3794846R111

Mr0grog commented 9 years ago

I just merged the latest master into this so it’s clean again. If there are any issues, let me know. Would love to see this land if possible :)

Mr0grog commented 9 years ago

Just a friendly ping on this one. Happy to make any changes you’d like, @bsphere.

Mr0grog commented 9 years ago

Just checking in on this again, @bsphere, any changes this needs?

Mr0grog commented 9 years ago

This is now updated with tests.