Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

39 query param support #47

Merged
merged 4 commits into from
Apr 26, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ tokenCache.get('service-foo')
* `tokenInfoEndpoint` string - mandatory for TokenCache
* `realm` string (`SERVICES_REALM` | `EMPLOYEES_REALM`)
* `scopes` string optional
* `queryParams` {} optional
* `redirect_uri` string optional (required with `AUTHORIZATION_CODE_GRANT`)
* `code` string optional (required with `AUTHORIZATION_CODE_GRANT`)

Expand Down Expand Up @@ -118,6 +119,7 @@ getAccessToken(options)
* `accessTokenEndpoint` string
* `realm` string (`SERVICES_REALM` | `EMPLOYEES_REALM`)
* `scopes` string optional
* `queryParams` {} optional
* `redirect_uri` string optional (required with `AUTHORIZATION_CODE_GRANT`)
* `code` string optional (required with `AUTHORIZATION_CODE_GRANT`)
* `refreshToken` string optional (required with REFRESH_TOKEN_GRANT)
Expand Down
28 changes: 28 additions & 0 deletions integration-test/oauth-tooling/getAccessToken.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ function setupTestEnvironment(authHeader: string, authServerApp: Express.Applica

authServerApp.use(bodyParser.urlencoded({extended: true}));
authServerApp.post('/oauth2/access_token', function(req, res) {

if (req.body.grant_type === PASSWORD_CREDENTIALS_GRANT) {
const valid = req.headers['authorization'] === authHeader;
if (valid) {
Expand Down Expand Up @@ -73,6 +74,33 @@ describe('getAccessToken', () => {
nock.cleanAll();
});

it('should add optional query parameters to the request', () => {

//given
const responseObject = { 'access_token': '4b70510f-be1d-4f0f-b4cb-edbca2c79d41' };

nock('http://127.0.0.1:30001/oauth2/')
.post('/access_token')
.query({
realm: SERVICES_REALM,
foo: 'bar'
})
.reply(HttpStatus.OK, responseObject);

//when
const promise = getAccessToken({
realm: SERVICES_REALM,
scopes: ['campaing.edit_all', 'campaign.read_all'],
accessTokenEndpoint: 'http://127.0.0.1:30001/oauth2/access_token',
credentialsDir: 'integration-test/data/credentials',
grantType: PASSWORD_CREDENTIALS_GRANT,
queryParams: { foo: 'bar' }
});

//then
return expect(promise).to.be.fulfilled;
});

describe('password credentials grant', () => {

before(() => {
Expand Down
94 changes: 35 additions & 59 deletions integration-test/oauth-tooling/getTokenInfo.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,39 @@ import * as Express from 'express';
import * as Http from 'http';

import {
PASSWORD_CREDENTIALS_GRANT,
getTokenInfo
} from '../../src/index';

chai.use(chaiAsPromised);
const expect = chai.expect;

const port = '30001';
const host = `http://127.0.0.1:${30001}`;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

port ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 ^^

const tokenInfoEndpoint = '/oauth2/tokeninfo';

const validToken = 'valid-token';
const invalidToken = 'invalid-token';

function addStandardAuthenticationEndpoint(app, _validToken) {

app.get(tokenInfoEndpoint, function(req, res) {

const valid = req.query.access_token === _validToken;

if (valid) {
res
.status(200)
.send({
'access_token': _validToken
});
} else {
res
.status(401)
.send();
}
});
}

describe('getTokenInfo', () => {

let authenticationServer: Http.Server;
Expand All @@ -19,58 +45,22 @@ describe('getTokenInfo', () => {
// Setup AuthServer
beforeEach(() => {
authServerApp = Express();
authenticationServer = authServerApp.listen(30001);
authenticationServer = authServerApp.listen(port);
});

// stop server after test
afterEach(() => {
authenticationServer.close();
});

function addStandardAuthenticationEndpoint() {

authServerApp.get('/oauth2/tokeninfo', function(req, res) {
const valid = req.query.access_token === '4b70510f-be1d-4f0f-b4cb-edbca2c79d41';

if (valid) {
res
.status(200)
.send({
'expires_in': 3515,
'token_type': 'Bearer',
'realm': 'employees',
'scope': [
'campaign.editall',
'campaign.readall'
],
'grant_type': PASSWORD_CREDENTIALS_GRANT,
'uid': 'services',
'access_token': '4b70510f-be1d-4f0f-b4cb-edbca2c79d41'
});
} else {
res
.status(401)
.send({
'error': 'invalid_request',
'error_description': 'Access Token not valid'
});
}
});
}

it('should return error if token is not valid', () => {

// given
const authToken = 'invalid';
addStandardAuthenticationEndpoint();
addStandardAuthenticationEndpoint(authServerApp, validToken);

// when
const url = 'http://127.0.0.1:30001/oauth2/tokeninfo';
const promise = getTokenInfo(url, authToken)
.then((jsonData) => {

return jsonData;
});
const url = `${host}${tokenInfoEndpoint}`;
const promise = getTokenInfo(url, invalidToken);

// then
return expect(promise).be.rejected;
Expand All @@ -79,29 +69,15 @@ describe('getTokenInfo', () => {
it('should return the token info if token is valid', function() {

// given
const authToken = '4b70510f-be1d-4f0f-b4cb-edbca2c79d41';
addStandardAuthenticationEndpoint();
addStandardAuthenticationEndpoint(authServerApp, validToken);

// when
const url = 'http://127.0.0.1:30001/oauth2/tokeninfo';
const promise = getTokenInfo(url, authToken)
.then((jsonData) => {

return jsonData;
});
const url = `${host}${tokenInfoEndpoint}`;
const promise = getTokenInfo(url, validToken);

// then
return expect(promise).to.become({
'access_token': '4b70510f-be1d-4f0f-b4cb-edbca2c79d41',
'expires_in': 3515,
'grant_type': 'password',
'realm': 'employees',
'scope': [
'campaign.editall',
'campaign.readall'
],
'token_type': 'Bearer',
'uid': 'services'
'access_token': validToken
});
});
});
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "lib-oauth-tooling",
"version": "1.0.0",
"version": "1.1.0",
"description": "A simple typescript based oauth tooling library",
"main": "./lib/src/index.js",
"typings": "./lib/src/index.d.ts",
Expand Down Expand Up @@ -46,7 +46,6 @@
"chai": "3.5.0",
"chai-as-promised": "6.0.0",
"express": "4.15.2",
"istanbul":"0.4.5",
"mocha": "3.2.0",
"nyc":"10.2.0",
"tslint": "4.5.1",
Expand Down
1 change: 1 addition & 0 deletions src/TokenCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class TokenCache {
* `tokenInfoEndpoint` string
* `realm` string
* `scopes` string optional
* `queryParams` {} optional
* `redirect_uri` string optional (required with `AUTHORIZATION_CODE_GRANT`)
* `code` string optional (required with `AUTHORIZATION_CODE_GRANT`)
*
Expand Down
42 changes: 30 additions & 12 deletions src/oauth-tooling.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as qs from 'querystring';
import * as HttpStatus from 'http-status';
import fetch from 'node-fetch';
import * as formurlencoded from 'form-urlencoded';
Expand All @@ -22,18 +23,27 @@ const OAUTH_CONTENT_TYPE = 'application/x-www-form-urlencoded';
/**
* Returns URI to request authorization code with the given parameters.
*
* @param authorizationEndpoint
* @param clientId
* @param redirectUri
* @param authorizationEndpoint string
* @param clientId string
* @param redirectUri string
* @param queryParams {} optional
* @returns {string}
*/
function createAuthCodeRequestUri(authorizationEndpoint: string, clientId: string,
redirectUri: string) {
return authorizationEndpoint +
'?client_id=' + clientId +
'&redirect_uri=' + redirectUri +
'&response_type=code' +
'&realm=' + EMPLOYEES_REALM;
redirectUri: string, queryParams?: {}) {

const _queryParams = Object.assign({
'client_id': clientId,
'redirect_uri': redirectUri,
'response_type': 'code',
'realm': EMPLOYEES_REALM
}, queryParams);

const queryString = qs.stringify(_queryParams);
// we are unescaping again since we did not escape before using querystring and we do not want to break anything
const unescapedQueryString = qs.unescape(queryString);

return `${authorizationEndpoint}?${unescapedQueryString}`;
}

/**
Expand All @@ -45,14 +55,21 @@ function createAuthCodeRequestUri(authorizationEndpoint: string, clientId: strin
* @param authorizationHeaderValue
* @param accessTokenEndpoint
* @param realm
* @param queryParams optional
* @returns {Promise<T>|Q.Promise<U>}
*/
function requestAccessToken(bodyObject: any, authorizationHeaderValue: string,
accessTokenEndpoint: string, realm: string): Promise<Token> {
accessTokenEndpoint: string, realm: string,
queryParams?: {}): Promise<Token> {

const promise = new Promise(function(resolve, reject) {

fetch(accessTokenEndpoint + '?realm=' + realm, {
const queryString = qs.stringify(Object.assign({ realm: realm }, queryParams));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is safe if queryParams is not set?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, see examples in https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign

"[...]null and undefined will be ignored."

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Object.assign({ realm }, queryParams));

or with spread:

const x = { 
   ...{ realm },
   ...queryParams}
);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, will tackle that.


// we are unescaping again since we did not escape before using querystring and we do not want to break anything
const unescapedQueryString = qs.unescape(queryString);

fetch(`${accessTokenEndpoint}?${unescapedQueryString}`, {
method: 'POST',
body: formurlencoded(bodyObject),
headers: {
Expand Down Expand Up @@ -145,6 +162,7 @@ function getTokenInfo(tokenInfoUrl: string, accessToken: string): Promise<TokenI
* - accessTokenEndpoint string
* - realm string
* - scopes string optional
* - queryParams {} optional
* - redirect_uri string optional (required with AUTHORIZATION_CODE_GRANT)
* - code string optional (required with AUTHORIZATION_CODE_GRANT)
* - refreshToken string optional (required with REFRESH_TOKEN_GRANT)
Expand Down Expand Up @@ -197,7 +215,7 @@ function getAccessToken(options: OAuthConfig): Promise<Token> {
const authorizationHeaderValue = getBasicAuthHeaderValue(clientData.client_id, clientData.client_secret);

return requestAccessToken(bodyParameters, authorizationHeaderValue,
options.accessTokenEndpoint, options.realm);
options.accessTokenEndpoint, options.realm, options.queryParams);
});
}

Expand Down
1 change: 1 addition & 0 deletions src/types/OAuthConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ interface OAuthConfig {
code?: string; // (required with `AUTHORIZATION_CODE_GRANT`)
redirectUri?: string;
refreshToken?: string;
queryParams?: {};
}
25 changes: 25 additions & 0 deletions test/unit/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,31 @@ describe('oauth tooling', () => {

expect(result).to.equal(expected);
});

it('should return the correct uri as string with queryParams specified', () => {

// given
const authorizationEndpoint = '/oauth2/authorize';
const clientId = 'clientID';
const redirectUri = '/redirect';
const queryParams = {
foo: 'bar'
};

// when
const result = createAuthCodeRequestUri(authorizationEndpoint, clientId,
redirectUri, queryParams);

// then
const expected = authorizationEndpoint +
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use template strings?

Copy link
Collaborator Author

@bzums bzums Apr 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you are right, I would suggest this solution:

      const expected = `${authorizationEndpoint}` +
        `?client_id=${clientId}` +
        `&redirect_uri=${redirectUri}` +
        `&response_type=code` +
        `&realm=${EMPLOYEES_REALM}` +
        `&foo=bar`;

cause with multi line template strings we have the problem that
whitespaces are not ignored. So that would destroy the formatting, right?

Copy link
Collaborator

@ISO50 ISO50 Apr 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, alternative idea:

const expected = [
  ``,
   ``
].join();

'?client_id=' + clientId +
'&redirect_uri=' + redirectUri +
'&response_type=code' +
'&realm=' + EMPLOYEES_REALM +
'&foo=bar';

expect(result).to.equal(expected);
});
});

describe('TokenCache', () => {
Expand Down