Added oauth2 to authenticate and added documentation for it. #29
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| const { OAuth2 } = require('oauth'); | ||
| const querystring = require('querystring'); | ||
| const crypto = require("crypto"); | ||
| const { promisify } = require('es6-promisify'); | ||
| const validate = require('validate.js'); | ||
|
|
||
|
|
@@ -217,6 +218,12 @@ const METHODS = { | |
| propName: PROP_NAMES.NOTIFICATIONS, | ||
| paramNames: ['updated_after', 'limit'], | ||
| }, | ||
| CREATE_COMMENT: { | ||
| endpoint: 'create_comment', | ||
| methodName: 'createComment', | ||
| verb: METHOD_VERBS.POST, | ||
| paramNames: ['expense_id', 'content'], | ||
| }, | ||
| GET_MAIN_DATA: { | ||
| endpoint: 'get_main_data', | ||
| methodName: 'getMainData', | ||
|
|
@@ -517,7 +524,7 @@ const getEndpointMethodGenerator = (logger, accessTokenPromise, defaultIDs, oaut | |
|
|
||
| let url = `${endpoint}/${id}`; | ||
| // Get the access token | ||
| let resultPromise = accessTokenPromise(); | ||
|
|
||
| resultPromise.then( | ||
| () => { | ||
|
|
@@ -600,7 +607,13 @@ const getEndpointMethodGenerator = (logger, accessTokenPromise, defaultIDs, oaut | |
| */ | ||
| class Splitwise { | ||
| constructor(options = {}) { | ||
|
|
||
| const { consumerKey, consumerSecret, accessToken, grant_type = "client_credentials", redirect_uri = null, isAuthorizationCode = grant_type === "authorization_code", state = null } = options; | ||
| const SPLITWISE_ENDPOINTS = { | ||
| "path": 'https://secure.splitwise.com/', | ||
| "authorizePath": 'oauth/authorize', | ||
| "accessTokenPath": 'oauth/token', | ||
| } | ||
| const defaultIDs = { | ||
| groupID: options.group_id, | ||
| userID: options.user_id, | ||
|
|
@@ -614,28 +627,70 @@ class Splitwise { | |
| logger({ level: LOG_LEVELS.ERROR, message }); | ||
| throw new Error(message); | ||
| } | ||
| // check if no redirect url supplied to isAuthorizationCode | ||
| if (isAuthorizationCode && !redirect_uri) { | ||
| const message = 'Redirect url required for OAuth2'; | ||
| logger({ level: LOG_LEVELS.ERROR, message }); | ||
| throw new Error(message); | ||
| } | ||
|
|
||
| const oauth2 = new OAuth2( | ||
| consumerKey, | ||
| consumerSecret, | ||
| SPLITWISE_ENDPOINTS.path, | ||
| isAuthorizationCode ? SPLITWISE_ENDPOINTS.authorizePath : null, | ||
| SPLITWISE_ENDPOINTS.accessTokenPath, | ||
| null | ||
| ); | ||
|
|
||
| this.getAuthorizationUrl = () => { | ||
|
There was a problem hiding this comment. i would probably do something like adding state as an argument to this method |
||
| if (!isAuthorizationCode) return ""; | ||
| return oauth2.getAuthorizeUrl({ | ||
| redirect_uri, | ||
| scope: '', | ||
| state, | ||
|
There was a problem hiding this comment.
nvm, as above, we probably don't want to generate state |
||
| response_type: 'code' | ||
| }); | ||
| } | ||
|
|
||
| const getAccessTokenFromAuthCode = () => { | ||
| return new Promise((resolve, reject) => { | ||
| if(!this.authCode) | ||
| return reject(`No auth code generated yet. Visit ${this.getAuthorizationUrl()} and login. You need a callback url in your project as mentioned in callback url in your registered splitwise app. Then call \`getAccessToken()\` to register the access token.`); | ||
| if(this.accessToken) return resolve(this.accessToken); | ||
|
There was a problem hiding this comment. Ah sorry, I realize now this might not work either. What we have to remember is that with the There was a problem hiding this comment. @keriwarr Yaa I thought about this. With this, our clients will need to use the
Also, as per you, will this library be used for a project of multiple users? |
||
| return oauth2.getOAuthAccessToken(this.authCode, { | ||
| code: this.authCode, | ||
| redirect_uri, | ||
| grant_type: 'authorization_code' | ||
| }, (err, accessToken) => { | ||
| if (err) { | ||
| return reject(err); | ||
| } | ||
| // useAuthorizationHeaderforGET attaches auth in header. Else get requests throw 401 in case of authorization_code for missing access token. | ||
| oauth2.useAuthorizationHeaderforGET(true); | ||
keriwarr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| this.accessToken = accessToken; | ||
| return resolve(accessToken); | ||
| }) | ||
| }) | ||
| } | ||
|
|
||
|
|
||
| const getAccessTokenWithClientCredentials = () => { | ||
| if (accessToken) { | ||
| logger({ message: 'using provided access token' }); | ||
| return Promise.resolve(accessToken); | ||
| } | ||
| logger({ message: 'making request for access token' }); | ||
| return getAccessTokenPromise(logger, oauth2); | ||
| }; | ||
| if (!isAuthorizationCode) { | ||
| // earlier an IIFE. But now, this is called only in case of client_credentials. Called by default while initialising so as to acquire a token right away. | ||
| getAccessTokenWithClientCredentials(); | ||
| } | ||
|
There was a problem hiding this comment. I think this code would really benefit from having a clear separation between the client credentials code and the authorization code code. maybe something like a switch statement over the new There was a problem hiding this comment. Called by default while initializing so as to acquire a token right away. This is just a single case. We cant get the token right away in case of authorization_code |
||
|
|
||
| const generateEndpointMethod = getEndpointMethodGenerator( | ||
| logger, | ||
| isAuthorizationCode ? getAccessTokenFromAuthCode : getAccessTokenWithClientCredentials, | ||
| defaultIDs, | ||
| oauth2 | ||
| ); | ||
|
|
@@ -645,8 +700,16 @@ class Splitwise { | |
| R.values(METHODS).forEach((method) => { | ||
| this[method.methodName] = generateEndpointMethod(method); | ||
| }); | ||
|
|
||
| // Seperate methods for authorization code and client credentials | ||
| if (isAuthorizationCode) { | ||
| this.getAccessToken = code => { | ||
| this.authCode = code; | ||
| return getAccessTokenFromAuthCode(); | ||
| } | ||
| } | ||
| else | ||
| this.getAccessToken = getAccessTokenWithClientCredentials; | ||
| } | ||
|
|
||
| // Bonus utility method for easily making transactions from one person to one person | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, this won't work, the state must be distinct for every attempted authentication
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keriwarr oh yaa! I get it. so how about the client supplies a function that will generate state everytime? But do we expose this state to the client so that they can verify it? Because passing a static state will always result in having the same state for each auth request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the client should just supply the state individually for each request. We would have to expose it - not just so that they can verify it - it might be used to store information about how the user initiated the original request (such as which page to redirect back @omkarsk98
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@keriwarr Got it. So this is my understanding. Correct me if I am wrong. we add a state key in
getAuthorizationUrl. If that is present, we use that, else we generate our state and we exposegetStatefor both the cases?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what i was originally thinking, but i re-read the OAuth spec for the "state" parameter, and my new understanding is that in order to verify the state, it's insufficient to check that it matches the outgoing state, you actually have to check that the user for whom you generated the auth url is the same user who visited the callback. Our library isn't in a position to do that so we have to leave it to the client, meaning that we shouldn't generate a state, we should require the client to generate one. Moreover, the client will receive the state from the inbound callback request, so we shouldn't need to provide a getState mechanism. Indeed, we might not be able to as in theory there could be many users doing oauth authentication at the same time - how would we store all their different states and keep them disentangled without knowing who they are?