From 062080e15d28f1b485d0db4cb59a88215ddc4fdc Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Mon, 14 Aug 2017 18:02:44 +0200 Subject: [PATCH 01/14] feat(password-flow): add method to build valid password auth uri affects: @commercetools/sdk-middleware-auth --- .../sdk-middleware-auth/src/build-requests.js | 48 ++++++- .../test/build-requests.spec.js | 119 +++++++++++++++++- types/sdk.js | 17 +++ 3 files changed, 180 insertions(+), 4 deletions(-) diff --git a/packages/sdk-middleware-auth/src/build-requests.js b/packages/sdk-middleware-auth/src/build-requests.js index 73dc0f482..7f656c416 100644 --- a/packages/sdk-middleware-auth/src/build-requests.js +++ b/packages/sdk-middleware-auth/src/build-requests.js @@ -1,5 +1,8 @@ /* @flow */ -import type { AuthMiddlewareOptions } from 'types/sdk' +import type { + AuthMiddlewareOptions, + PasswordAuthMiddlewareOptions, +} from 'types/sdk' import * as authScopes from './scopes' type BuiltRequestParams = { @@ -43,8 +46,47 @@ export function buildRequestForClientCredentialsFlow ( return { basicAuth, url, body } } -export function buildRequestForPasswordFlow () { - // TODO +export function buildRequestForPasswordFlow ( + options: PasswordAuthMiddlewareOptions, +): BuiltRequestParams { + if (!options) + throw new Error('Missing required options') + + if (!options.host) + throw new Error('Missing required option (host)') + + if (!options.projectKey) + throw new Error('Missing required option (projectKey)') + + if (!options.credentials) + throw new Error('Missing required option (credentials)') + + const { + clientId, + clientSecret, + user, + } = options.credentials + const pKey = options.projectKey + if (!(clientId && clientSecret && user)) + throw new Error( + 'Missing required credentials (clientId, clientSecret, user)', + ) + const { username, password } = user + if (!(username && password)) + throw new Error('Missing required user credentials (username, password)') + + const defaultScope = `${authScopes.MANAGE_PROJECT}:${pKey}` + const scope = (options.scopes || [defaultScope]).join(' ') + + const basicAuth = new Buffer(`${clientId}:${clientSecret}`).toString('base64') + // This is mostly useful for internal testing purposes to be able to check + // other oauth endpoints. + const oauthUri = options.oauthUri || `/oauth/${pKey}/token/customers/token` + const url = options.host.replace(/\/$/, '') + oauthUri + // eslint-disable-next-line max-len + const body = `grant_type=password&scope=${scope}&username=${username}&password=${password}` + + return { basicAuth, url, body } } export function buildRequestForRefreshTokenFlow () { diff --git a/packages/sdk-middleware-auth/test/build-requests.spec.js b/packages/sdk-middleware-auth/test/build-requests.spec.js index 8f4499b88..4f709dde7 100644 --- a/packages/sdk-middleware-auth/test/build-requests.spec.js +++ b/packages/sdk-middleware-auth/test/build-requests.spec.js @@ -1,6 +1,6 @@ import { buildRequestForClientCredentialsFlow, - // buildRequestForPasswordFlow, + buildRequestForPasswordFlow, // buildRequestForRefreshTokenFlow, // buildRequestForAnonymousSessionFlow, } from '../src/build-requests' @@ -15,12 +15,129 @@ function createTestOptions (options) { credentials: { clientId: '123', clientSecret: 'secret', + user: { + username: 'foobar', + password: 'verysecurepassword', + }, }, scopes: allScopes, ...options, } } +describe('buildRequestForPasswordFlow', () => { + const body = `grant_type=password&scope=${allScopes.join(' ')}\ +&username=foobar&password=verysecurepassword` + it('build request values with all the given options', () => { + const options = createTestOptions() + expect(buildRequestForPasswordFlow(options)).toEqual({ + basicAuth: 'MTIzOnNlY3JldA==', + url: 'http://localhost:8080/oauth/test/token/customers/token', + body, + }) + }) + + it('uses custom oauth uri, if given', () => { + const options = createTestOptions({ oauthUri: '/foo/bar' }) + expect(buildRequestForPasswordFlow(options)).toEqual({ + basicAuth: 'MTIzOnNlY3JldA==', + url: 'http://localhost:8080/foo/bar', + body, + }) + }) + + it('parses a host that ends with slash', () => { + const options = createTestOptions({ + host: 'http://localhost:8080/', + }) + expect(buildRequestForPasswordFlow(options)).toEqual({ + basicAuth: 'MTIzOnNlY3JldA==', + url: 'http://localhost:8080/oauth/test/token/customers/token', + body, + }) + }) + + it('parses a host that ends without slash', () => { + const options = createTestOptions({ + host: 'http://localhost:8080', + }) + expect(buildRequestForPasswordFlow(options)).toEqual({ + basicAuth: 'MTIzOnNlY3JldA==', + url: 'http://localhost:8080/oauth/test/token/customers/token', + body, + }) + }) + + it('validate required options', () => { + expect( + () => buildRequestForPasswordFlow(), + ).toThrowError('Missing required options') + }) + + it('validate required option (host)', () => { + expect( + () => buildRequestForPasswordFlow({}), + ).toThrowError('Missing required option (host)') + }) + + it('validate required option (projectKey)', () => { + const options = createTestOptions({ + projectKey: undefined, + }) + expect( + () => buildRequestForPasswordFlow(options), + ).toThrowError('Missing required option (projectKey)') + }) + + it('validate required option (credentials)', () => { + const options = createTestOptions({ + credentials: undefined, + }) + expect( + () => buildRequestForPasswordFlow(options), + ).toThrowError('Missing required option (credentials)') + }) + + it('validate required option (clientId, clientSecret)', () => { + const options = createTestOptions({ + credentials: {}, + }) + expect( + () => buildRequestForPasswordFlow(options), + ).toThrowError( + 'Missing required credentials (clientId, clientSecret, user)', + ) + }) + + it('validate required option (username, password)', () => { + const options = createTestOptions({ + credentials: { + clientId: 'yeah', + clientSecret: 'yo', + user: { + username: 'bar', + }, + }, + }) + expect( + () => buildRequestForPasswordFlow(options), + ).toThrowError( + 'Missing required user credentials (username, password)', + ) + }) + + it('validate both credentials are required', () => { + const options = createTestOptions({ + credentials: { clientId: '123' }, + }) + expect( + () => buildRequestForPasswordFlow(options), + ).toThrowError( + 'Missing required credentials (clientId, clientSecret, user)', + ) + }) +}) + describe('buildRequestForClientCredentialsFlow', () => { it('build request values with all the given options', () => { const options = createTestOptions() diff --git a/types/sdk.js b/types/sdk.js index 71b379d16..b251641e9 100644 --- a/types/sdk.js +++ b/types/sdk.js @@ -77,6 +77,23 @@ export type AuthMiddlewareOptions = { // For internal usage only oauthUri: string; } + +export type PasswordAuthMiddlewareOptions = { + host: string; + projectKey: string; + credentials: { + clientId: string; + clientSecret: string; + user: { + username: string; + password: string; + }; + }; + scopes: Array; + // For internal usage only + oauthUri: string; +} + export type HttpMiddlewareOptions = { host: string; includeHeaders?: boolean; From 6e6106df98dbee3e62de7cede6976c280ba46a17 Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Tue, 15 Aug 2017 11:58:11 +0200 Subject: [PATCH 02/14] feat(sdk-middleware-auth): implement password flow affects: @commercetools/sdk-middleware-auth ISSUES CLOSED: #212 --- .../sdk-middleware-auth/src/password-flow.js | 141 +++++++- .../test/password-flow.spec.js | 330 ++++++++++++++++++ 2 files changed, 469 insertions(+), 2 deletions(-) create mode 100644 packages/sdk-middleware-auth/test/password-flow.spec.js diff --git a/packages/sdk-middleware-auth/src/password-flow.js b/packages/sdk-middleware-auth/src/password-flow.js index 80d650528..3a92522a2 100644 --- a/packages/sdk-middleware-auth/src/password-flow.js +++ b/packages/sdk-middleware-auth/src/password-flow.js @@ -1,3 +1,140 @@ -export default function createAuthMiddlewareForPasswordFlow () { - throw new Error('Middleware not implemented yet') +/* @flow */ +import type { + PasswordAuthMiddlewareOptions, + Middleware, + MiddlewareRequest, + MiddlewareResponse, +} from 'types/sdk' + +/* global fetch */ +import 'isomorphic-fetch' +import { buildRequestForPasswordFlow } from './build-requests' + +type TokenCache = { + token: string; + expirationTime: number; +} +type Task = { + request: MiddlewareRequest; + response: MiddlewareResponse; +} + +export default function createAuthMiddlewareForPasswordFlow ( + options: PasswordAuthMiddlewareOptions, +): Middleware { + let cache: TokenCache + let pendingTasks: Array = [] + let isFetchingToken = false + + return next => (request: MiddlewareRequest, response: MiddlewareResponse) => { + // Check if there is already a `Authorization` header in the request. + // If so, then go directly to the next middleware. + if ( + (request.headers && request.headers['authorization']) || + (request.headers && request.headers['Authorization']) + ) { + next(request, response) + return + } + + // If there was a token in the cache, and it's not expired, append + // the token in the `Authorization` header. + if (cache && cache.token && Date.now() < cache.expirationTime) { + const requestWithAuth = mergeAuthHeader(cache.token, request) + next(requestWithAuth, response) + return + } + // Token is not present or is invalid. Request a new token... + + // Keep pending tasks until a token is fetched + pendingTasks.push({ request, response }) + + // If a token is currently being fetched, just wait ;) + if (isFetchingToken) return + + // Mark that a token is being fetched + isFetchingToken = true + + const { + basicAuth, + url, + body, + } = buildRequestForPasswordFlow(options) + + fetch( + url, + { + method: 'POST', + headers: { + Authorization: `Basic ${basicAuth}`, + 'Content-Length': Buffer.byteLength(body).toString(), + 'Content-Type': 'application/x-www-form-urlencoded', + }, + body, + }, + ) + .then((res: Response): Promise<*> => { + if (res.ok) + return res.json() + .then((result: Object) => { + const token = result.access_token + const expiresIn = result.expires_in + const expirationTime = calculateExpirationTime(expiresIn) + // Cache new token + cache = { token, expirationTime } + + // Dispatch all pending requests + isFetchingToken = false + // Freeze and copy pending queue, reset original one for accepting + // new pending tasks + const executionQueue = pendingTasks.slice() + pendingTasks = [] + executionQueue.forEach((task) => { + // Assign the new token in the request header + const requestWithAuth = mergeAuthHeader(token, task.request) + next(requestWithAuth, task.response) + }) + }) + + // Handle error response + return res.text() + .then((text: any) => { + let parsed + try { + parsed = JSON.parse(text) + } catch (error) { + /* noop */ + } + const error: Object = new Error(parsed ? parsed.message : text) + if (parsed) error.body = parsed + response.reject(error) + }) + }) + .catch((error) => { + response.reject(error) + }) + } +} + +function mergeAuthHeader ( + token: string, + req: MiddlewareRequest, +): MiddlewareRequest { + return { + ...req, + headers: { + ...req.headers, + Authorization: `Bearer ${token}`, + }, + } +} + +function calculateExpirationTime (expiresIn: number): number { + return ( + Date.now() + + (expiresIn * 1000) + ) - ( + // Add a gap of 2 hours before expiration time. + 2 * 60 * 60 * 1000 + ) } diff --git a/packages/sdk-middleware-auth/test/password-flow.spec.js b/packages/sdk-middleware-auth/test/password-flow.spec.js new file mode 100644 index 000000000..19abd0cc3 --- /dev/null +++ b/packages/sdk-middleware-auth/test/password-flow.spec.js @@ -0,0 +1,330 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import nock from 'nock' +import { + createAuthMiddlewareForPasswordFlow, +} from '../src' + +function createTestRequest (options) { + return { + url: '', + method: 'GET', + body: null, + headers: {}, + ...options, + } +} + +function createTestResponse (options) { + return { + ...options, + } +} + +function createTestMiddlewareOptions (options) { + return { + host: 'https://auth.commercetools.co', + projectKey: 'foo', + credentials: { + clientId: '123', + clientSecret: 'secret', + user: { + username: 'foobar', + password: 'verysecurepassword', + }, + }, + ...options, + } +} + +describe('Password Flow', () => { + const expectedBody = 'grant_type=password&scope=manage_project:foo' + +'&username=foobar&password=verysecurepassword' + beforeEach(() => { + nock.cleanAll() + }) + + it('get a new auth token if not present in request headers', () => + new Promise((resolve, reject) => { + const request = createTestRequest() + const response = createTestResponse({ + resolve, + reject, + }) + const next = (req /* , res */) => { + expect(req).toEqual({ + ...request, + headers: { + ...request.headers, + Authorization: 'Bearer xxx', + }, + }) + resolve() + } + const middlewareOptions = createTestMiddlewareOptions() + const authMiddleware = createAuthMiddlewareForPasswordFlow( + middlewareOptions, + ) + nock(middlewareOptions.host) + .filteringRequestBody((body) => { + expect(body).toBe(expectedBody) + return '*' + }) + .post('/oauth/foo/token/customers/token', '*') + .reply(200, { + access_token: 'xxx', + expires_in: 100, + }) + + authMiddleware(next)(request, response) + }), + ) + + it('reject if auth request fails (JSON error response)', () => + new Promise((resolve, reject) => { + const request = createTestRequest() + const response = createTestResponse({ + resolve, + reject: (error) => { + expect(error.message).toBe('Oops') + expect(error.body).toEqual({ message: 'Oops' }) + resolve() + }, + }) + const next = () => { + reject( + 'This function should never be called, the response was rejected', + ) + } + const middlewareOptions = createTestMiddlewareOptions() + const authMiddleware = createAuthMiddlewareForPasswordFlow( + middlewareOptions, + ) + nock(middlewareOptions.host) + .filteringRequestBody(/.*/, '*') + .post('/oauth/foo/token/customers/token', '*') + .reply(400, { message: 'Oops' }) // <-- JSON error response + + authMiddleware(next)(request, response) + }), + ) + + it('reject if auth request fails (non JSON error response)', () => + new Promise((resolve, reject) => { + const request = createTestRequest() + const response = createTestResponse({ + resolve, + reject: (error) => { + expect(error.message).toBe('Oops') + expect(error.body).toBeUndefined() + resolve() + }, + }) + const next = () => { + reject( + 'This function should never be called, the response was rejected', + ) + } + const middlewareOptions = createTestMiddlewareOptions() + const authMiddleware = createAuthMiddlewareForPasswordFlow( + middlewareOptions, + ) + nock(middlewareOptions.host) + .filteringRequestBody(/.*/, '*') + .post('/oauth/foo/token/customers/token', '*') + .reply(400, 'Oops') // <-- non JSON error response + + authMiddleware(next)(request, response) + }), + ) + + it('retrieve a new token if previous one expired', () => + new Promise((resolve, reject) => { + const request = createTestRequest() + const response = createTestResponse({ + resolve, + reject, + }) + const middlewareOptions = createTestMiddlewareOptions() + const authMiddleware = createAuthMiddlewareForPasswordFlow( + middlewareOptions, + ) + let requestCount = 0 + nock(middlewareOptions.host) + .persist() // <-- use the same interceptor for all requests + .log(() => { (requestCount += 1) }) // keep track of the request count + .filteringRequestBody(/.*/, '*') + .post('/oauth/foo/token/customers/token', '*') + .reply(200, () => ({ + access_token: 'xxx', + // Return the first 2 requests with an expired token + expires_in: requestCount < 2 + ? 1 // <-- to ensure it expires + : (Date.now() + (60 * 60 * 24)), + })) + + // First call: + // - there is no token yet + // - a new token is fetched + authMiddleware( + () => { + expect(requestCount).toBe(1) + // Second call: + // - we simulate that the request has a token set in the headers + // - the previous token was expired though, so we need to refetch it + authMiddleware( + () => { + expect(requestCount).toBe(2) + // Third call: + // - we simulate that the request has a token set in the headers + // - the previous token is still valid, no more requests + authMiddleware( + () => { + expect(requestCount).toBe(2) + resolve() + }, + )(request, response) + }, + )(request, response) + }, + )(request, response) + }), + ) + + it( + 'do not get a new token if one is already present in request headers ' + + 'but it does not match one of the cached tokens', + () => + new Promise((resolve, reject) => { + const request = createTestRequest() + const response = createTestResponse({ + resolve, + reject, + }) + const middlewareOptions = createTestMiddlewareOptions() + const authMiddleware = createAuthMiddlewareForPasswordFlow( + middlewareOptions, + ) + let requestCount = 0 + nock(middlewareOptions.host) + .persist() // <-- use the same interceptor for all requests + .log(() => { (requestCount += 1) }) // keep track of the request count + .filteringRequestBody(/.*/, '*') + .post('/oauth/foo/token/customers/token', '*') + .reply(200, { + access_token: 'xxx', + expires_in: (Date.now() + (60 * 60 * 24)), + }) + + // First call: + // - there is no token yet + // - a new token is fetched + authMiddleware( + () => { + // Second call: + // - we simulate that the request has a token set in the headers + // which does not match any of the cached tokens. In this case + // do not refetch and keep going. + const requestWithHeaders = { + ...request, + headers: { + Authorization: 'Bearer yyy', + }, + } + authMiddleware( + () => { + expect(requestCount).toBe(1) + resolve() + }, + )(requestWithHeaders, response) + }, + )(request, response) + }), + ) + + it( + 'ensure to fetch new token only once and keep track of pending tasks', + () => + new Promise((resolve, reject) => { + const request = createTestRequest() + const response = createTestResponse({ + resolve, + reject, + }) + const middlewareOptions = createTestMiddlewareOptions() + const authMiddleware = createAuthMiddlewareForPasswordFlow( + middlewareOptions, + ) + let requestCount = 0 + nock(middlewareOptions.host) + .persist() // <-- use the same interceptor for all requests + .log(() => { (requestCount += 1) }) // keep track of the request count + .filteringRequestBody(/.*/, '*') + .post('/oauth/foo/token/customers/token', '*') + .reply(200, { + access_token: 'xxx', + expires_in: (Date.now() + (60 * 60 * 24)), + }) + + let nextCount = 0 + const next = () => { + nextCount += 1 + if (nextCount === 6) { + expect(requestCount).toBe(1) + resolve() + } + } + // Execute multiple requests at once. + // This should queue all of them until the token has been fetched. + authMiddleware(next)(request, response) + authMiddleware(next)(request, response) + authMiddleware(next)(request, response) + authMiddleware(next)(request, response) + authMiddleware(next)(request, response) + authMiddleware(next)(request, response) + }), + ) + + it( + 'if a token has been fetched, use it for the new incoming requests', + () => + new Promise((resolve, reject) => { + const request = createTestRequest() + const response = createTestResponse({ + resolve, + reject, + }) + const middlewareOptions = createTestMiddlewareOptions() + const authMiddleware = createAuthMiddlewareForPasswordFlow( + middlewareOptions, + ) + let requestCount = 0 + nock(middlewareOptions.host) + .persist() // <-- use the same interceptor for all requests + .log(() => { (requestCount += 1) }) // keep track of the request count + .filteringRequestBody(/.*/, '*') + .post('/oauth/foo/token/customers/token', '*') + .reply(200, { + access_token: 'xxx', + expires_in: (Date.now() + (60 * 60 * 24)), + }) + + // 1. Execute multiple requests at once. + // This should queue all of them until the token has been fetched. + authMiddleware(() => { + // 1. Got a token + expect(requestCount).toBe(1) + + authMiddleware((rq2) => { + // 2. Should not get a new token + expect(requestCount).toBe(1) + expect(rq2).toEqual(expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: 'Bearer xxx', + }), + })) + resolve() + })(request, response) + })(request, response) + }), + ) +}) From 4b2c0cd460cd09c0610df612047c7046096d23c1 Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Thu, 17 Aug 2017 12:27:11 +0200 Subject: [PATCH 03/14] refactor(base-auth-flow): refactor fetching token logic affects: @commercetools/sdk-middleware-auth --- .../sdk-middleware-auth/src/base-auth-flow.js | 154 ++++++++++++++++++ .../src/client-credentials-flow.js | 144 +++------------- .../sdk-middleware-auth/src/password-flow.js | 144 +++------------- packages/sdk-middleware-auth/src/utils.js | 10 ++ types/sdk.js | 8 + 5 files changed, 212 insertions(+), 248 deletions(-) create mode 100644 packages/sdk-middleware-auth/src/base-auth-flow.js create mode 100644 packages/sdk-middleware-auth/src/utils.js diff --git a/packages/sdk-middleware-auth/src/base-auth-flow.js b/packages/sdk-middleware-auth/src/base-auth-flow.js new file mode 100644 index 000000000..0e2c2605e --- /dev/null +++ b/packages/sdk-middleware-auth/src/base-auth-flow.js @@ -0,0 +1,154 @@ +/* @flow */ +import type { + MiddlewareRequest, + MiddlewareResponse, + Next, + Task, +} from 'types/sdk' + +/* global fetch */ +import 'isomorphic-fetch' + +type RequestState = boolean +type TokenStore = { + token: string; + expirationTime: number; +} +type AuthMiddlewareBaseOptions = { + request: MiddlewareRequest; + response: MiddlewareResponse; + url: string; + body: string; + basicAuth: string; + pendingTasks: Array; + requestState: { + get: () => RequestState; + set: (requestState: RequestState) => RequestState; + }; + tokenCache: { + get: () => TokenStore; + set: (cache: TokenStore) => TokenStore; + } +} + +export default function createAuthMiddlewareBase ({ + request, + response, + url, + basicAuth, + body, + pendingTasks, + requestState, + tokenCache, + }: AuthMiddlewareBaseOptions, + next: Next, +) { + // Check if there is already a `Authorization` header in the request. + // If so, then go directly to the next middleware. + if ( + (request.headers && request.headers['authorization']) || + (request.headers && request.headers['Authorization']) + ) { + next(request, response) + return + } + + // If there was a token in the tokenCache, and it's not expired, append + // the token in the `Authorization` header. + const tokenObj = tokenCache.get() + if (tokenObj && tokenObj.token && Date.now() < tokenObj.expirationTime) { + const requestWithAuth = mergeAuthHeader(tokenObj.token, request) + next(requestWithAuth, response) + return + } + // Token is not present or is invalid. Request a new token... + + // Keep pending tasks until a token is fetched + pendingTasks.push({ request, response }) + + // If a token is currently being fetched, just wait ;) + if (requestState.get()) return + + // Mark that a token is being fetched + requestState.set(true) + + fetch( + url, + { + method: 'POST', + headers: { + Authorization: `Basic ${basicAuth}`, + 'Content-Length': Buffer.byteLength(body).toString(), + 'Content-Type': 'application/x-www-form-urlencoded', + }, + body, + }, + ) + .then((res: Response): Promise<*> => { + if (res.ok) + return res.json() + .then((result: Object) => { + const token = result.access_token + const expiresIn = result.expires_in + const expirationTime = calculateExpirationTime(expiresIn) + + // Cache new token + tokenCache.set({ token, expirationTime }) + + // Dispatch all pending requests + requestState.set(false) + + // Freeze and copy pending queue, reset original one for accepting + // new pending tasks + const executionQueue = pendingTasks.slice() + // eslint-disable-next-line no-param-reassign + pendingTasks = [] + executionQueue.forEach((task: Task) => { + // Assign the new token in the request header + const requestWithAuth = mergeAuthHeader(token, task.request) + // console.log('test', cache, pendingTasks) + next(requestWithAuth, task.response) + }) + }) + + // Handle error response + return res.text() + .then((text: any) => { + let parsed + try { + parsed = JSON.parse(text) + } catch (error) { + /* noop */ + } + const error: Object = new Error(parsed ? parsed.message : text) + if (parsed) error.body = parsed + response.reject(error) + }) + }) + .catch((error: Error) => { + response.reject(error) + }) +} + +function mergeAuthHeader ( + token: string, + req: MiddlewareRequest, +): MiddlewareRequest { + return { + ...req, + headers: { + ...req.headers, + Authorization: `Bearer ${token}`, + }, + } +} + +function calculateExpirationTime (expiresIn: number): number { + return ( + Date.now() + + (expiresIn * 1000) + ) - ( + // Add a gap of 2 hours before expiration time. + 2 * 60 * 60 * 1000 + ) +} diff --git a/packages/sdk-middleware-auth/src/client-credentials-flow.js b/packages/sdk-middleware-auth/src/client-credentials-flow.js index 6339a1c13..df25bf563 100644 --- a/packages/sdk-middleware-auth/src/client-credentials-flow.js +++ b/packages/sdk-middleware-auth/src/client-credentials-flow.js @@ -4,137 +4,33 @@ import type { Middleware, MiddlewareRequest, MiddlewareResponse, + Next, + Task, } from 'types/sdk' -/* global fetch */ -import 'isomorphic-fetch' import { buildRequestForClientCredentialsFlow } from './build-requests' - -type TokenCache = { - token: string; - expirationTime: number; -} -type Task = { - request: MiddlewareRequest; - response: MiddlewareResponse; -} +import createAuthMiddlewareBase from './base-auth-flow' +import store from './utils' export default function createAuthMiddlewareForClientCredentialsFlow ( options: AuthMiddlewareOptions, ): Middleware { - let cache: TokenCache - let pendingTasks: Array = [] - let isFetchingToken = false - - return next => (request: MiddlewareRequest, response: MiddlewareResponse) => { - // Check if there is already a `Authorization` header in the request. - // If so, then go directly to the next middleware. - if ( - (request.headers && request.headers['authorization']) || - (request.headers && request.headers['Authorization']) - ) { - next(request, response) - return + const tokenCache = store({}) + const pendingTasks: Array = [] + + const requestState = store(false) + return (next: Next) => ( + request: MiddlewareRequest, + response: MiddlewareResponse, + ) => { + const params = { + request, + response, + ...buildRequestForClientCredentialsFlow(options), + pendingTasks, + requestState, + tokenCache, } - - // If there was a token in the cache, and it's not expired, append - // the token in the `Authorization` header. - if (cache && cache.token && Date.now() < cache.expirationTime) { - const requestWithAuth = mergeAuthHeader(cache.token, request) - next(requestWithAuth, response) - return - } - // Token is not present or is invalid. Request a new token... - - // Keep pending tasks until a token is fetched - pendingTasks.push({ request, response }) - - // If a token is currently being fetched, just wait ;) - if (isFetchingToken) return - - // Mark that a token is being fetched - isFetchingToken = true - - const { - basicAuth, - url, - body, - } = buildRequestForClientCredentialsFlow(options) - - fetch( - url, - { - method: 'POST', - headers: { - Authorization: `Basic ${basicAuth}`, - 'Content-Length': Buffer.byteLength(body).toString(), - 'Content-Type': 'application/x-www-form-urlencoded', - }, - body, - }, - ) - .then((res: Response): Promise<*> => { - if (res.ok) - return res.json() - .then((result: Object) => { - const token = result.access_token - const expiresIn = result.expires_in - const expirationTime = calculateExpirationTime(expiresIn) - // Cache new token - cache = { token, expirationTime } - - // Dispatch all pending requests - isFetchingToken = false - // Freeze and copy pending queue, reset original one for accepting - // new pending tasks - const executionQueue = pendingTasks.slice() - pendingTasks = [] - executionQueue.forEach((task) => { - // Assign the new token in the request header - const requestWithAuth = mergeAuthHeader(token, task.request) - next(requestWithAuth, task.response) - }) - }) - - // Handle error response - return res.text() - .then((text: any) => { - let parsed - try { - parsed = JSON.parse(text) - } catch (error) { - /* noop */ - } - const error: Object = new Error(parsed ? parsed.message : text) - if (parsed) error.body = parsed - response.reject(error) - }) - }) - .catch((error) => { - response.reject(error) - }) + createAuthMiddlewareBase(params, next) } } - -function mergeAuthHeader ( - token: string, - req: MiddlewareRequest, -): MiddlewareRequest { - return { - ...req, - headers: { - ...req.headers, - Authorization: `Bearer ${token}`, - }, - } -} - -function calculateExpirationTime (expiresIn: number): number { - return ( - Date.now() + - (expiresIn * 1000) - ) - ( - // Add a gap of 2 hours before expiration time. - 2 * 60 * 60 * 1000 - ) -} diff --git a/packages/sdk-middleware-auth/src/password-flow.js b/packages/sdk-middleware-auth/src/password-flow.js index 3a92522a2..7d3467e46 100644 --- a/packages/sdk-middleware-auth/src/password-flow.js +++ b/packages/sdk-middleware-auth/src/password-flow.js @@ -4,137 +4,33 @@ import type { Middleware, MiddlewareRequest, MiddlewareResponse, + Next, + Task, } from 'types/sdk' -/* global fetch */ -import 'isomorphic-fetch' import { buildRequestForPasswordFlow } from './build-requests' - -type TokenCache = { - token: string; - expirationTime: number; -} -type Task = { - request: MiddlewareRequest; - response: MiddlewareResponse; -} +import createAuthMiddlewareBase from './base-auth-flow' +import store from './utils' export default function createAuthMiddlewareForPasswordFlow ( options: PasswordAuthMiddlewareOptions, ): Middleware { - let cache: TokenCache - let pendingTasks: Array = [] - let isFetchingToken = false - - return next => (request: MiddlewareRequest, response: MiddlewareResponse) => { - // Check if there is already a `Authorization` header in the request. - // If so, then go directly to the next middleware. - if ( - (request.headers && request.headers['authorization']) || - (request.headers && request.headers['Authorization']) - ) { - next(request, response) - return + const tokenCache = store({}) + const pendingTasks: Array = [] + const requestState = store(false) + + return (next: Next) => ( + request: MiddlewareRequest, + response: MiddlewareResponse, + ) => { + const params = { + request, + response, + ...buildRequestForPasswordFlow(options), + pendingTasks, + requestState, + tokenCache, } - - // If there was a token in the cache, and it's not expired, append - // the token in the `Authorization` header. - if (cache && cache.token && Date.now() < cache.expirationTime) { - const requestWithAuth = mergeAuthHeader(cache.token, request) - next(requestWithAuth, response) - return - } - // Token is not present or is invalid. Request a new token... - - // Keep pending tasks until a token is fetched - pendingTasks.push({ request, response }) - - // If a token is currently being fetched, just wait ;) - if (isFetchingToken) return - - // Mark that a token is being fetched - isFetchingToken = true - - const { - basicAuth, - url, - body, - } = buildRequestForPasswordFlow(options) - - fetch( - url, - { - method: 'POST', - headers: { - Authorization: `Basic ${basicAuth}`, - 'Content-Length': Buffer.byteLength(body).toString(), - 'Content-Type': 'application/x-www-form-urlencoded', - }, - body, - }, - ) - .then((res: Response): Promise<*> => { - if (res.ok) - return res.json() - .then((result: Object) => { - const token = result.access_token - const expiresIn = result.expires_in - const expirationTime = calculateExpirationTime(expiresIn) - // Cache new token - cache = { token, expirationTime } - - // Dispatch all pending requests - isFetchingToken = false - // Freeze and copy pending queue, reset original one for accepting - // new pending tasks - const executionQueue = pendingTasks.slice() - pendingTasks = [] - executionQueue.forEach((task) => { - // Assign the new token in the request header - const requestWithAuth = mergeAuthHeader(token, task.request) - next(requestWithAuth, task.response) - }) - }) - - // Handle error response - return res.text() - .then((text: any) => { - let parsed - try { - parsed = JSON.parse(text) - } catch (error) { - /* noop */ - } - const error: Object = new Error(parsed ? parsed.message : text) - if (parsed) error.body = parsed - response.reject(error) - }) - }) - .catch((error) => { - response.reject(error) - }) + createAuthMiddlewareBase(params, next) } } - -function mergeAuthHeader ( - token: string, - req: MiddlewareRequest, -): MiddlewareRequest { - return { - ...req, - headers: { - ...req.headers, - Authorization: `Bearer ${token}`, - }, - } -} - -function calculateExpirationTime (expiresIn: number): number { - return ( - Date.now() + - (expiresIn * 1000) - ) - ( - // Add a gap of 2 hours before expiration time. - 2 * 60 * 60 * 1000 - ) -} diff --git a/packages/sdk-middleware-auth/src/utils.js b/packages/sdk-middleware-auth/src/utils.js new file mode 100644 index 000000000..9eff25bef --- /dev/null +++ b/packages/sdk-middleware-auth/src/utils.js @@ -0,0 +1,10 @@ +export default function store (initVal) { + let value = initVal + return { + get: () => value, + set: (val) => { + value = val + return value + }, + } +} diff --git a/types/sdk.js b/types/sdk.js index b251641e9..f9c5bdcd9 100644 --- a/types/sdk.js +++ b/types/sdk.js @@ -118,6 +118,14 @@ export type UserAgentMiddlewareOptions = { contactEmail?: string; } +export type Next = ( + request: MiddlewareRequest, response: MiddlewareResponse +) => mixed + +export type Task = { + request: MiddlewareRequest; + response: MiddlewareResponse; +} /* API Request Builder */ export type ServiceBuilderDefaultParams = { From 92bb68f5d8cde827eaac19a797637652096c7c39 Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Fri, 18 Aug 2017 11:27:33 +0200 Subject: [PATCH 04/14] test(base-auth-flow): add tests affects: @commercetools/sdk-middleware-auth --- .../sdk-middleware-auth/src/base-auth-flow.js | 2 +- .../src/client-credentials-flow.js | 4 +- .../sdk-middleware-auth/src/password-flow.js | 4 +- .../test/base-auth-flow.spec.js | 331 ++++++++++++++++++ 4 files changed, 336 insertions(+), 5 deletions(-) create mode 100644 packages/sdk-middleware-auth/test/base-auth-flow.spec.js diff --git a/packages/sdk-middleware-auth/src/base-auth-flow.js b/packages/sdk-middleware-auth/src/base-auth-flow.js index 0e2c2605e..303627f7c 100644 --- a/packages/sdk-middleware-auth/src/base-auth-flow.js +++ b/packages/sdk-middleware-auth/src/base-auth-flow.js @@ -31,7 +31,7 @@ type AuthMiddlewareBaseOptions = { } } -export default function createAuthMiddlewareBase ({ +export default function authMiddlewareBase ({ request, response, url, diff --git a/packages/sdk-middleware-auth/src/client-credentials-flow.js b/packages/sdk-middleware-auth/src/client-credentials-flow.js index df25bf563..6b69840e4 100644 --- a/packages/sdk-middleware-auth/src/client-credentials-flow.js +++ b/packages/sdk-middleware-auth/src/client-credentials-flow.js @@ -9,7 +9,7 @@ import type { } from 'types/sdk' import { buildRequestForClientCredentialsFlow } from './build-requests' -import createAuthMiddlewareBase from './base-auth-flow' +import authMiddlewareBase from './base-auth-flow' import store from './utils' export default function createAuthMiddlewareForClientCredentialsFlow ( @@ -31,6 +31,6 @@ export default function createAuthMiddlewareForClientCredentialsFlow ( requestState, tokenCache, } - createAuthMiddlewareBase(params, next) + authMiddlewareBase(params, next) } } diff --git a/packages/sdk-middleware-auth/src/password-flow.js b/packages/sdk-middleware-auth/src/password-flow.js index 7d3467e46..5da6c0453 100644 --- a/packages/sdk-middleware-auth/src/password-flow.js +++ b/packages/sdk-middleware-auth/src/password-flow.js @@ -9,7 +9,7 @@ import type { } from 'types/sdk' import { buildRequestForPasswordFlow } from './build-requests' -import createAuthMiddlewareBase from './base-auth-flow' +import authMiddlewareBase from './base-auth-flow' import store from './utils' export default function createAuthMiddlewareForPasswordFlow ( @@ -31,6 +31,6 @@ export default function createAuthMiddlewareForPasswordFlow ( requestState, tokenCache, } - createAuthMiddlewareBase(params, next) + authMiddlewareBase(params, next) } } diff --git a/packages/sdk-middleware-auth/test/base-auth-flow.spec.js b/packages/sdk-middleware-auth/test/base-auth-flow.spec.js new file mode 100644 index 000000000..c18718dc4 --- /dev/null +++ b/packages/sdk-middleware-auth/test/base-auth-flow.spec.js @@ -0,0 +1,331 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import nock from 'nock' +import createAuthMiddlewareBase from '../src/base-auth-flow' +import { + buildRequestForClientCredentialsFlow, +} from '../src/build-requests' +import store from '../src/utils' + +function createTestRequest (options) { + return { + url: '', + method: 'GET', + body: null, + headers: {}, + ...options, + } +} + +function createTestResponse (options) { + return { + ...options, + } +} + +function createBaseMiddleware (options, next) { + const params = { + request: createTestRequest(), + response: createTestResponse(), + pendingTasks: [], + ...buildRequestForClientCredentialsFlow(createTestMiddlewareOptions()), + requestState: store(false), + tokenCache: store({}), + ...options, + } + return createAuthMiddlewareBase(params, next) +} + +function createTestMiddlewareOptions (options) { + return { + host: 'https://auth.commercetools.co', + projectKey: 'foo', + credentials: { + clientId: '123', + clientSecret: 'secret', + }, + ...options, + } +} + +describe('Base Auth Flow', () => { + beforeEach(() => { + nock.cleanAll() + }) + + it('get a new auth token if not present in request headers', () => + new Promise((resolve) => { + const request = createTestRequest() + const next = (req /* , res */) => { + expect(req).toEqual({ + ...request, + headers: { + ...request.headers, + Authorization: 'Bearer xxx', + }, + }) + resolve() + } + const middlewareOptions = createTestMiddlewareOptions() + + nock(middlewareOptions.host) + .filteringRequestBody((body) => { + expect(body).toBe( + 'grant_type=client_credentials&scope=manage_project:foo', + ) + return '*' + }) + .post('/oauth/token', '*') + .reply(200, { + access_token: 'xxx', + expires_in: 100, + }) + createBaseMiddleware({ }, next) + }), + ) + + it('reject if network error occur while fetching token', () => + new Promise((resolve, reject) => { + const response = createTestResponse({ + resolve, + reject: (error) => { + expect(error.message).toMatch('socket timeout') + resolve() + }, + }) + const next = () => { + reject(new Error('this method should not be called.')) + } + const middlewareOptions = createTestMiddlewareOptions() + + nock(middlewareOptions.host) + .filteringRequestBody((body) => { + expect(body).toBe( + 'grant_type=client_credentials&scope=manage_project:foo', + ) + return '*' + }) + .post('/oauth/token', '*') + .replyWithError('socket timeout') + createBaseMiddleware({ response }, next) + }), + ) + + it('reject if auth request fails (JSON error response)', () => + new Promise((resolve, reject) => { + const response = createTestResponse({ + resolve, + reject: (error) => { + expect(error.message).toBe('Oops') + expect(error.body).toEqual({ message: 'Oops' }) + resolve() + }, + }) + const next = () => { + reject( + 'This function should never be called, the response was rejected', + ) + } + const middlewareOptions = createTestMiddlewareOptions() + nock(middlewareOptions.host) + .filteringRequestBody(/.*/, '*') + .post('/oauth/token', '*') + .reply(400, { message: 'Oops' }) // <-- JSON error response + + createBaseMiddleware({ response }, next) + }), + ) + + it('reject if auth request fails (non JSON error response)', () => + new Promise((resolve, reject) => { + const response = createTestResponse({ + resolve, + reject: (error) => { + expect(error.message).toBe('Oops') + expect(error.body).toBeUndefined() + resolve() + }, + }) + const next = () => { + reject( + 'This function should never be called, the response was rejected', + ) + } + const middlewareOptions = createTestMiddlewareOptions() + nock(middlewareOptions.host) + .filteringRequestBody(/.*/, '*') + .post('/oauth/token', '*') + .reply(400, 'Oops') // <-- non JSON error response + + createBaseMiddleware({ response }, next) + }), + ) + + it('retrieve a new token if previous one expired', () => + new Promise((resolve) => { + const middlewareOptions = createTestMiddlewareOptions() + let requestCount = 0 + nock(middlewareOptions.host) + .persist() // <-- use the same interceptor for all requests + .log(() => { (requestCount += 1) }) // keep track of the request count + .filteringRequestBody(/.*/, '*') + .post('/oauth/token', '*') + .reply(200, () => ({ + access_token: 'xxx', + // Return the first 2 requests with an expired token + expires_in: requestCount < 2 + ? 1 // <-- to ensure it expires + : (Date.now() + (60 * 60 * 24)), + })) + + const next = () => { + expect(requestCount).toBe(2) + resolve() + } + const tokenCache = store({}) + const requestState = store(false) + const call3 = () => { + expect(requestCount).toBe(2) + createBaseMiddleware({ requestState, tokenCache }, next) + } + const call2 = () => { + expect(requestCount).toBe(1) + createBaseMiddleware({ requestState, tokenCache }, call3) + } + // First call: + // - there is no token yet + // - a new token is fetched + createBaseMiddleware({ requestState, tokenCache }, call2) + }), + ) + + it( + 'do not get a new token if one is already present in request headers ' + + 'but it does not match one of the cached tokens', + () => + new Promise((resolve, reject) => { + const response = createTestResponse({ + resolve, + reject, + }) + const middlewareOptions = createTestMiddlewareOptions() + let requestCount = 0 + nock(middlewareOptions.host) + .persist() // <-- use the same interceptor for all requests + .log(() => { (requestCount += 1) }) // keep track of the request count + .filteringRequestBody(/.*/, '*') + .post('/oauth/token', '*') + .reply(200, { + access_token: 'xxx', + expires_in: (Date.now() + (60 * 60 * 24)), + }) + + const next = () => { + expect(requestCount).toBe(1) + resolve() + } + const tokenCache = store({}) + const requestState = store(false) + // Second call: + // - we simulate that the request has a token set in the headers + // which does not match any of the cached tokens. In this case + // do not refetch and keep going. + const call2 = (rq) => { + const requestWithHeaders = { + ...rq, + headers: { + Authorization: 'Bearer yyy', + }, + } + createBaseMiddleware({ request: requestWithHeaders }, next) + createAuthMiddlewareBase({ + request: requestWithHeaders, + response, + pendingTasks: [], + ...buildRequestForClientCredentialsFlow(middlewareOptions), + requestState, + tokenCache, + }, next) + } + // First call: + // - there is no token yet + // - a new token is fetched + createBaseMiddleware({}, call2) + }), + ) + + it( + 'ensure to fetch new token only once and keep track of pending tasks', + () => + new Promise((resolve) => { + const middlewareOptions = createTestMiddlewareOptions() + let requestCount = 0 + nock(middlewareOptions.host) + .persist() // <-- use the same interceptor for all requests + .log(() => { (requestCount += 1) }) // keep track of the request count + .filteringRequestBody(/.*/, '*') + .post('/oauth/token', '*') + .reply(200, { + access_token: 'xxx', + expires_in: (Date.now() + (60 * 60 * 24)), + }) + + let nextCount = 0 + const next = () => { + nextCount += 1 + if (nextCount === 6) { + expect(requestCount).toBe(1) + resolve() + } + } + const pendingTasks = [] + const tokenCache = store({}) + const requestState = store(false) + // Execute multiple requests at once. + // This should queue all of them until the token has been fetched. + createBaseMiddleware({ pendingTasks, tokenCache, requestState }, next) + createBaseMiddleware({ pendingTasks, tokenCache, requestState }, next) + createBaseMiddleware({ pendingTasks, tokenCache, requestState }, next) + createBaseMiddleware({ pendingTasks, tokenCache, requestState }, next) + createBaseMiddleware({ pendingTasks, tokenCache, requestState }, next) + createBaseMiddleware({ pendingTasks, tokenCache, requestState }, next) + createBaseMiddleware({ pendingTasks, tokenCache, requestState }, next) + }), + ) + + it( + 'if a token has been fetched, use it for the new incoming requests', + () => + new Promise((resolve) => { + const middlewareOptions = createTestMiddlewareOptions() + let requestCount = 0 + nock(middlewareOptions.host) + .persist() // <-- use the same interceptor for all requests + .log(() => { (requestCount += 1) }) // keep track of the request count + .filteringRequestBody(/.*/, '*') + .post('/oauth/token', '*') + .reply(200, { + access_token: 'xxx', + expires_in: (Date.now() + (60 * 60 * 24)), + }) + const pendingTasks = [] + const tokenCache = store({}) + const requestState = store(false) + const next = (rq2) => { + // 2. Should not get a new token + expect(requestCount).toBe(1) + expect(rq2).toEqual(expect.objectContaining({ + headers: expect.objectContaining({ + Authorization: 'Bearer xxx', + }), + })) + resolve() + } + // 2. Should not get a new token + const call2 = createBaseMiddleware( + { pendingTasks, tokenCache, requestState }, + next, + ) + createBaseMiddleware({ pendingTasks, tokenCache, requestState }, call2) + }), + ) +}) From f6a9636b52762650cb39eb1d24851d66e4518fe6 Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Fri, 18 Aug 2017 11:39:25 +0200 Subject: [PATCH 05/14] chore(base-auth-flow): add comments to tests affects: @commercetools/sdk-middleware-auth --- packages/sdk-middleware-auth/test/base-auth-flow.spec.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/sdk-middleware-auth/test/base-auth-flow.spec.js b/packages/sdk-middleware-auth/test/base-auth-flow.spec.js index c18718dc4..3aedd4626 100644 --- a/packages/sdk-middleware-auth/test/base-auth-flow.spec.js +++ b/packages/sdk-middleware-auth/test/base-auth-flow.spec.js @@ -183,10 +183,16 @@ describe('Base Auth Flow', () => { } const tokenCache = store({}) const requestState = store(false) + // Third call: + // - we simulate that the request has a token set in the headers + // - the previous token is still valid, no more requests const call3 = () => { expect(requestCount).toBe(2) createBaseMiddleware({ requestState, tokenCache }, next) } + // Second call: + // - we simulate that the request has a token set in the headers + // - the previous token was expired though, so we need to refetch it const call2 = () => { expect(requestCount).toBe(1) createBaseMiddleware({ requestState, tokenCache }, call3) From ba0358b252cd46772a4e7365c0df2e74165a9dc4 Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Fri, 18 Aug 2017 12:05:46 +0200 Subject: [PATCH 06/14] docs(password-flow): add docs about new auth flow --- docs/sdk/api/sdkMiddlewareAuth.md | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/docs/sdk/api/sdkMiddlewareAuth.md b/docs/sdk/api/sdkMiddlewareAuth.md index 557a8c682..e870601c7 100644 --- a/docs/sdk/api/sdkMiddlewareAuth.md +++ b/docs/sdk/api/sdkMiddlewareAuth.md @@ -49,3 +49,39 @@ const client = createClient({ ], }) ``` + +## `createAuthMiddlewareForPasswordFlow(options)` + +Creates a [middleware](/sdk/Glossary.md#middleware) to handle authentication for the [Password Flow](http://dev.commercetools.com/http-api-authorization.html#password-flow) of the commercetools platform API. + +#### Named arguments (options) + +1. `host` *(String)*: the host of the OAuth API service +2. `projectKey` *(String)*: the key of the project to assign the default scope to +3. `credentials` *(Object)*: the client credentials for authentication (`clientId`, `clientSecret`) +4. `scopes` *(Array)*: a list of [scopes](http://dev.commercetools.com/http-api-authorization.html#scopes) (default `manage_project:{projectKey}`) to assign to the OAuth token + + +#### Usage example + +```js +import { createClient } from '@commercetools/sdk-client' +import { createAuthMiddlewareForPasswordFlow } from '@commercetools/sdk-middleware-auth' + +const client = createClient({ + middlewares: [ + createAuthMiddlewareForPasswordFlow({ + host: 'https://auth.commercetools.com', + projectKey: 'test', + credentials: { + clientId: '123', + clientSecret: 'secret', + }, + scopes: [ + 'view_products:test', + 'manage_orders:test', + ], + }), + ], +}) +``` From 0249b2d96cb6a83cb11bbd4aa21469279787933e Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Fri, 18 Aug 2017 12:10:04 +0200 Subject: [PATCH 07/14] docs(password-flow): update docs --- docs/sdk/api/sdkMiddlewareAuth.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/sdk/api/sdkMiddlewareAuth.md b/docs/sdk/api/sdkMiddlewareAuth.md index e870601c7..a17116774 100644 --- a/docs/sdk/api/sdkMiddlewareAuth.md +++ b/docs/sdk/api/sdkMiddlewareAuth.md @@ -76,6 +76,10 @@ const client = createClient({ credentials: { clientId: '123', clientSecret: 'secret', + user: { + username: string; + password: string; + } }, scopes: [ 'view_products:test', From a8312ed0361298575f75b030fd48251ee2551926 Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Fri, 18 Aug 2017 12:47:42 +0200 Subject: [PATCH 08/14] refactor(middleware-auth): remove duplicate tests affects: @commercetools/sdk-middleware-auth --- .../test/client-crendentials-flow.spec.js | 303 ++--------------- .../test/password-flow.spec.js | 304 ++---------------- 2 files changed, 39 insertions(+), 568 deletions(-) diff --git a/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js b/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js index 970ffaca1..78251c5a0 100644 --- a/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js +++ b/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js @@ -1,8 +1,9 @@ -// eslint-disable-next-line import/no-extraneous-dependencies -import nock from 'nock' import { createAuthMiddlewareForClientCredentialsFlow, } from '../src' +import authMiddlewareBase from '../src/base-auth-flow' + +jest.mock('../src/base-auth-flow') function createTestRequest (options) { return { @@ -14,12 +15,6 @@ function createTestRequest (options) { } } -function createTestResponse (options) { - return { - ...options, - } -} - function createTestMiddlewareOptions (options) { return { host: 'https://auth.commercetools.co', @@ -33,294 +28,34 @@ function createTestMiddlewareOptions (options) { } describe('Client Crentials Flow', () => { - beforeEach(() => { - nock.cleanAll() - }) - - it('get a new auth token if not present in request headers', () => + it('should call the base-auth-flow method with the right params', () => new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject, + authMiddlewareBase.mockImplementation((params, next) => { + next(params) // makes it easy to test what was passed in }) - const next = (req /* , res */) => { - expect(req).toEqual({ - ...request, - headers: { - ...request.headers, - Authorization: 'Bearer xxx', - }, - }) - resolve() - } - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForClientCredentialsFlow( - middlewareOptions, - ) - nock(middlewareOptions.host) - .filteringRequestBody((body) => { - expect(body).toBe( - 'grant_type=client_credentials&scope=manage_project:foo', - ) - return '*' - }) - .post('/oauth/token', '*') - .reply(200, { - access_token: 'xxx', - expires_in: 100, - }) - - authMiddleware(next)(request, response) - }), - ) - - it('reject if auth request fails (JSON error response)', () => - new Promise((resolve, reject) => { const request = createTestRequest() - const response = createTestResponse({ + const response = { resolve, - reject: (error) => { - expect(error.message).toBe('Oops') - expect(error.body).toEqual({ message: 'Oops' }) - resolve() - }, - }) - const next = () => { - reject( - 'This function should never be called, the response was rejected', - ) + reject, } - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForClientCredentialsFlow( - middlewareOptions, - ) - nock(middlewareOptions.host) - .filteringRequestBody(/.*/, '*') - .post('/oauth/token', '*') - .reply(400, { message: 'Oops' }) // <-- JSON error response - - authMiddleware(next)(request, response) - }), - ) - - it('reject if auth request fails (non JSON error response)', () => - new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject: (error) => { - expect(error.message).toBe('Oops') - expect(error.body).toBeUndefined() - resolve() - }, - }) - const next = () => { - reject( - 'This function should never be called, the response was rejected', - ) + const next = (actualParams) => { + expect(request).toEqual(actualParams.request) + expect(response).toEqual(actualParams.response) + expect(response).toEqual(actualParams.response) + expect([]).toEqual(actualParams.pendingTasks) + expect( + 'https://auth.commercetools.co/oauth/token', + ).toBe(actualParams.url) + expect('MTIzOnNlY3JldA==').toBe(actualParams.basicAuth) + expect(authMiddlewareBase).toHaveBeenCalledTimes(1) + resolve() } const middlewareOptions = createTestMiddlewareOptions() const authMiddleware = createAuthMiddlewareForClientCredentialsFlow( middlewareOptions, ) - nock(middlewareOptions.host) - .filteringRequestBody(/.*/, '*') - .post('/oauth/token', '*') - .reply(400, 'Oops') // <-- non JSON error response - - authMiddleware(next)(request, response) - }), - ) - - it('retrieve a new token if previous one expired', () => - new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject, - }) - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForClientCredentialsFlow( - middlewareOptions, - ) - let requestCount = 0 - nock(middlewareOptions.host) - .persist() // <-- use the same interceptor for all requests - .log(() => { (requestCount += 1) }) // keep track of the request count - .filteringRequestBody(/.*/, '*') - .post('/oauth/token', '*') - .reply(200, () => ({ - access_token: 'xxx', - // Return the first 2 requests with an expired token - expires_in: requestCount < 2 - ? 1 // <-- to ensure it expires - : (Date.now() + (60 * 60 * 24)), - })) - // First call: - // - there is no token yet - // - a new token is fetched - authMiddleware( - () => { - expect(requestCount).toBe(1) - // Second call: - // - we simulate that the request has a token set in the headers - // - the previous token was expired though, so we need to refetch it - authMiddleware( - () => { - expect(requestCount).toBe(2) - // Third call: - // - we simulate that the request has a token set in the headers - // - the previous token is still valid, no more requests - authMiddleware( - () => { - expect(requestCount).toBe(2) - resolve() - }, - )(request, response) - }, - )(request, response) - }, - )(request, response) - }), - ) - - it( - 'do not get a new token if one is already present in request headers ' + - 'but it does not match one of the cached tokens', - () => - new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject, - }) - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForClientCredentialsFlow( - middlewareOptions, - ) - let requestCount = 0 - nock(middlewareOptions.host) - .persist() // <-- use the same interceptor for all requests - .log(() => { (requestCount += 1) }) // keep track of the request count - .filteringRequestBody(/.*/, '*') - .post('/oauth/token', '*') - .reply(200, { - access_token: 'xxx', - expires_in: (Date.now() + (60 * 60 * 24)), - }) - - // First call: - // - there is no token yet - // - a new token is fetched - authMiddleware( - () => { - // Second call: - // - we simulate that the request has a token set in the headers - // which does not match any of the cached tokens. In this case - // do not refetch and keep going. - const requestWithHeaders = { - ...request, - headers: { - Authorization: 'Bearer yyy', - }, - } - authMiddleware( - () => { - expect(requestCount).toBe(1) - resolve() - }, - )(requestWithHeaders, response) - }, - )(request, response) - }), - ) - - it( - 'ensure to fetch new token only once and keep track of pending tasks', - () => - new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject, - }) - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForClientCredentialsFlow( - middlewareOptions, - ) - let requestCount = 0 - nock(middlewareOptions.host) - .persist() // <-- use the same interceptor for all requests - .log(() => { (requestCount += 1) }) // keep track of the request count - .filteringRequestBody(/.*/, '*') - .post('/oauth/token', '*') - .reply(200, { - access_token: 'xxx', - expires_in: (Date.now() + (60 * 60 * 24)), - }) - - let nextCount = 0 - const next = () => { - nextCount += 1 - if (nextCount === 6) { - expect(requestCount).toBe(1) - resolve() - } - } - // Execute multiple requests at once. - // This should queue all of them until the token has been fetched. - authMiddleware(next)(request, response) authMiddleware(next)(request, response) - authMiddleware(next)(request, response) - authMiddleware(next)(request, response) - authMiddleware(next)(request, response) - authMiddleware(next)(request, response) - }), - ) - - it( - 'if a token has been fetched, use it for the new incoming requests', - () => - new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject, - }) - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForClientCredentialsFlow( - middlewareOptions, - ) - let requestCount = 0 - nock(middlewareOptions.host) - .persist() // <-- use the same interceptor for all requests - .log(() => { (requestCount += 1) }) // keep track of the request count - .filteringRequestBody(/.*/, '*') - .post('/oauth/token', '*') - .reply(200, { - access_token: 'xxx', - expires_in: (Date.now() + (60 * 60 * 24)), - }) - - // 1. Execute multiple requests at once. - // This should queue all of them until the token has been fetched. - authMiddleware(() => { - // 1. Got a token - expect(requestCount).toBe(1) - - authMiddleware((rq2) => { - // 2. Should not get a new token - expect(requestCount).toBe(1) - expect(rq2).toEqual(expect.objectContaining({ - headers: expect.objectContaining({ - Authorization: 'Bearer xxx', - }), - })) - resolve() - })(request, response) - })(request, response) }), ) }) diff --git a/packages/sdk-middleware-auth/test/password-flow.spec.js b/packages/sdk-middleware-auth/test/password-flow.spec.js index 19abd0cc3..e2de626f4 100644 --- a/packages/sdk-middleware-auth/test/password-flow.spec.js +++ b/packages/sdk-middleware-auth/test/password-flow.spec.js @@ -1,9 +1,11 @@ -// eslint-disable-next-line import/no-extraneous-dependencies -import nock from 'nock' import { createAuthMiddlewareForPasswordFlow, } from '../src' +import authMiddlewareBase from '../src/base-auth-flow' + +jest.mock('../src/base-auth-flow') + function createTestRequest (options) { return { url: '', @@ -14,12 +16,6 @@ function createTestRequest (options) { } } -function createTestResponse (options) { - return { - ...options, - } -} - function createTestMiddlewareOptions (options) { return { host: 'https://auth.commercetools.co', @@ -37,294 +33,34 @@ function createTestMiddlewareOptions (options) { } describe('Password Flow', () => { - const expectedBody = 'grant_type=password&scope=manage_project:foo' + -'&username=foobar&password=verysecurepassword' - beforeEach(() => { - nock.cleanAll() - }) - - it('get a new auth token if not present in request headers', () => + it('should call the base-auth-flow method with the right params', () => new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject, + authMiddlewareBase.mockImplementation((params, next) => { + next(params) // makes it easy to test what was passed in }) - const next = (req /* , res */) => { - expect(req).toEqual({ - ...request, - headers: { - ...request.headers, - Authorization: 'Bearer xxx', - }, - }) - resolve() - } - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForPasswordFlow( - middlewareOptions, - ) - nock(middlewareOptions.host) - .filteringRequestBody((body) => { - expect(body).toBe(expectedBody) - return '*' - }) - .post('/oauth/foo/token/customers/token', '*') - .reply(200, { - access_token: 'xxx', - expires_in: 100, - }) - - authMiddleware(next)(request, response) - }), - ) - - it('reject if auth request fails (JSON error response)', () => - new Promise((resolve, reject) => { const request = createTestRequest() - const response = createTestResponse({ + const response = { resolve, - reject: (error) => { - expect(error.message).toBe('Oops') - expect(error.body).toEqual({ message: 'Oops' }) - resolve() - }, - }) - const next = () => { - reject( - 'This function should never be called, the response was rejected', - ) + reject, } - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForPasswordFlow( - middlewareOptions, - ) - nock(middlewareOptions.host) - .filteringRequestBody(/.*/, '*') - .post('/oauth/foo/token/customers/token', '*') - .reply(400, { message: 'Oops' }) // <-- JSON error response - - authMiddleware(next)(request, response) - }), - ) - - it('reject if auth request fails (non JSON error response)', () => - new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject: (error) => { - expect(error.message).toBe('Oops') - expect(error.body).toBeUndefined() - resolve() - }, - }) - const next = () => { - reject( - 'This function should never be called, the response was rejected', - ) + const next = (actualParams) => { + expect(request).toEqual(actualParams.request) + expect(response).toEqual(actualParams.response) + expect(response).toEqual(actualParams.response) + expect([]).toEqual(actualParams.pendingTasks) + expect( + 'https://auth.commercetools.co/oauth/foo/token/customers/token', + ).toBe(actualParams.url) + expect('MTIzOnNlY3JldA==').toBe(actualParams.basicAuth) + expect(authMiddlewareBase).toHaveBeenCalledTimes(1) + resolve() } const middlewareOptions = createTestMiddlewareOptions() const authMiddleware = createAuthMiddlewareForPasswordFlow( middlewareOptions, ) - nock(middlewareOptions.host) - .filteringRequestBody(/.*/, '*') - .post('/oauth/foo/token/customers/token', '*') - .reply(400, 'Oops') // <-- non JSON error response authMiddleware(next)(request, response) }), ) - - it('retrieve a new token if previous one expired', () => - new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject, - }) - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForPasswordFlow( - middlewareOptions, - ) - let requestCount = 0 - nock(middlewareOptions.host) - .persist() // <-- use the same interceptor for all requests - .log(() => { (requestCount += 1) }) // keep track of the request count - .filteringRequestBody(/.*/, '*') - .post('/oauth/foo/token/customers/token', '*') - .reply(200, () => ({ - access_token: 'xxx', - // Return the first 2 requests with an expired token - expires_in: requestCount < 2 - ? 1 // <-- to ensure it expires - : (Date.now() + (60 * 60 * 24)), - })) - - // First call: - // - there is no token yet - // - a new token is fetched - authMiddleware( - () => { - expect(requestCount).toBe(1) - // Second call: - // - we simulate that the request has a token set in the headers - // - the previous token was expired though, so we need to refetch it - authMiddleware( - () => { - expect(requestCount).toBe(2) - // Third call: - // - we simulate that the request has a token set in the headers - // - the previous token is still valid, no more requests - authMiddleware( - () => { - expect(requestCount).toBe(2) - resolve() - }, - )(request, response) - }, - )(request, response) - }, - )(request, response) - }), - ) - - it( - 'do not get a new token if one is already present in request headers ' + - 'but it does not match one of the cached tokens', - () => - new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject, - }) - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForPasswordFlow( - middlewareOptions, - ) - let requestCount = 0 - nock(middlewareOptions.host) - .persist() // <-- use the same interceptor for all requests - .log(() => { (requestCount += 1) }) // keep track of the request count - .filteringRequestBody(/.*/, '*') - .post('/oauth/foo/token/customers/token', '*') - .reply(200, { - access_token: 'xxx', - expires_in: (Date.now() + (60 * 60 * 24)), - }) - - // First call: - // - there is no token yet - // - a new token is fetched - authMiddleware( - () => { - // Second call: - // - we simulate that the request has a token set in the headers - // which does not match any of the cached tokens. In this case - // do not refetch and keep going. - const requestWithHeaders = { - ...request, - headers: { - Authorization: 'Bearer yyy', - }, - } - authMiddleware( - () => { - expect(requestCount).toBe(1) - resolve() - }, - )(requestWithHeaders, response) - }, - )(request, response) - }), - ) - - it( - 'ensure to fetch new token only once and keep track of pending tasks', - () => - new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject, - }) - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForPasswordFlow( - middlewareOptions, - ) - let requestCount = 0 - nock(middlewareOptions.host) - .persist() // <-- use the same interceptor for all requests - .log(() => { (requestCount += 1) }) // keep track of the request count - .filteringRequestBody(/.*/, '*') - .post('/oauth/foo/token/customers/token', '*') - .reply(200, { - access_token: 'xxx', - expires_in: (Date.now() + (60 * 60 * 24)), - }) - - let nextCount = 0 - const next = () => { - nextCount += 1 - if (nextCount === 6) { - expect(requestCount).toBe(1) - resolve() - } - } - // Execute multiple requests at once. - // This should queue all of them until the token has been fetched. - authMiddleware(next)(request, response) - authMiddleware(next)(request, response) - authMiddleware(next)(request, response) - authMiddleware(next)(request, response) - authMiddleware(next)(request, response) - authMiddleware(next)(request, response) - }), - ) - - it( - 'if a token has been fetched, use it for the new incoming requests', - () => - new Promise((resolve, reject) => { - const request = createTestRequest() - const response = createTestResponse({ - resolve, - reject, - }) - const middlewareOptions = createTestMiddlewareOptions() - const authMiddleware = createAuthMiddlewareForPasswordFlow( - middlewareOptions, - ) - let requestCount = 0 - nock(middlewareOptions.host) - .persist() // <-- use the same interceptor for all requests - .log(() => { (requestCount += 1) }) // keep track of the request count - .filteringRequestBody(/.*/, '*') - .post('/oauth/foo/token/customers/token', '*') - .reply(200, { - access_token: 'xxx', - expires_in: (Date.now() + (60 * 60 * 24)), - }) - - // 1. Execute multiple requests at once. - // This should queue all of them until the token has been fetched. - authMiddleware(() => { - // 1. Got a token - expect(requestCount).toBe(1) - - authMiddleware((rq2) => { - // 2. Should not get a new token - expect(requestCount).toBe(1) - expect(rq2).toEqual(expect.objectContaining({ - headers: expect.objectContaining({ - Authorization: 'Bearer xxx', - }), - })) - resolve() - })(request, response) - })(request, response) - }), - ) }) From 45e01189b88635878d9ba3f81e38c98dd9fbd9ac Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Fri, 18 Aug 2017 14:02:53 +0200 Subject: [PATCH 09/14] refactor(middleware-auth): update base on PR review affects: @commercetools/sdk-middleware-auth --- .../sdk-middleware-auth/test/build-requests.spec.js | 8 ++++++-- .../test/client-crendentials-flow.spec.js | 12 ++++++------ .../sdk-middleware-auth/test/password-flow.spec.js | 11 +++++------ 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/sdk-middleware-auth/test/build-requests.spec.js b/packages/sdk-middleware-auth/test/build-requests.spec.js index 4f709dde7..5cfad0972 100644 --- a/packages/sdk-middleware-auth/test/build-requests.spec.js +++ b/packages/sdk-middleware-auth/test/build-requests.spec.js @@ -1,3 +1,5 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import { oneLineTrim } from 'common-tags' import { buildRequestForClientCredentialsFlow, buildRequestForPasswordFlow, @@ -26,8 +28,10 @@ function createTestOptions (options) { } describe('buildRequestForPasswordFlow', () => { - const body = `grant_type=password&scope=${allScopes.join(' ')}\ -&username=foobar&password=verysecurepassword` + const body = oneLineTrim`grant_type=password& + scope=${allScopes.join(' ')}& + username=foobar&password=verysecurepassword + ` it('build request values with all the given options', () => { const options = createTestOptions() expect(buildRequestForPasswordFlow(options)).toEqual({ diff --git a/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js b/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js index 78251c5a0..3d9738d18 100644 --- a/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js +++ b/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js @@ -39,16 +39,16 @@ describe('Client Crentials Flow', () => { reject, } const next = (actualParams) => { - expect(request).toEqual(actualParams.request) - expect(response).toEqual(actualParams.response) - expect(response).toEqual(actualParams.response) - expect([]).toEqual(actualParams.pendingTasks) - expect( + expect(actualParams.request).toEqual(actualParams.request) + expect(actualParams.response).toEqual(actualParams.response) + expect(actualParams.pendingTasks).toEqual([]) + expect(actualParams.url).toBe( 'https://auth.commercetools.co/oauth/token', - ).toBe(actualParams.url) + ) expect('MTIzOnNlY3JldA==').toBe(actualParams.basicAuth) expect(authMiddlewareBase).toHaveBeenCalledTimes(1) resolve() + resolve() } const middlewareOptions = createTestMiddlewareOptions() const authMiddleware = createAuthMiddlewareForClientCredentialsFlow( diff --git a/packages/sdk-middleware-auth/test/password-flow.spec.js b/packages/sdk-middleware-auth/test/password-flow.spec.js index e2de626f4..b06a56dc8 100644 --- a/packages/sdk-middleware-auth/test/password-flow.spec.js +++ b/packages/sdk-middleware-auth/test/password-flow.spec.js @@ -44,13 +44,12 @@ describe('Password Flow', () => { reject, } const next = (actualParams) => { - expect(request).toEqual(actualParams.request) - expect(response).toEqual(actualParams.response) - expect(response).toEqual(actualParams.response) - expect([]).toEqual(actualParams.pendingTasks) - expect( + expect(actualParams.request).toEqual(actualParams.request) + expect(actualParams.response).toEqual(actualParams.response) + expect(actualParams.pendingTasks).toEqual([]) + expect(actualParams.url).toBe( 'https://auth.commercetools.co/oauth/foo/token/customers/token', - ).toBe(actualParams.url) + ) expect('MTIzOnNlY3JldA==').toBe(actualParams.basicAuth) expect(authMiddlewareBase).toHaveBeenCalledTimes(1) resolve() From 8fbdc12869050c3ea7ff12c3cc5d6d3699ce10f5 Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Fri, 18 Aug 2017 15:54:20 +0200 Subject: [PATCH 10/14] refactor(middleware-auth): improve code affects: @commercetools/sdk-middleware-auth --- .../sdk-middleware-auth/test/client-crendentials-flow.spec.js | 2 +- packages/sdk-middleware-auth/test/password-flow.spec.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js b/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js index 3d9738d18..e46ab0545 100644 --- a/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js +++ b/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js @@ -48,7 +48,7 @@ describe('Client Crentials Flow', () => { expect('MTIzOnNlY3JldA==').toBe(actualParams.basicAuth) expect(authMiddlewareBase).toHaveBeenCalledTimes(1) resolve() - resolve() + jest.unmock('../src/base-auth-flow') } const middlewareOptions = createTestMiddlewareOptions() const authMiddleware = createAuthMiddlewareForClientCredentialsFlow( diff --git a/packages/sdk-middleware-auth/test/password-flow.spec.js b/packages/sdk-middleware-auth/test/password-flow.spec.js index b06a56dc8..cdb17997e 100644 --- a/packages/sdk-middleware-auth/test/password-flow.spec.js +++ b/packages/sdk-middleware-auth/test/password-flow.spec.js @@ -52,6 +52,7 @@ describe('Password Flow', () => { ) expect('MTIzOnNlY3JldA==').toBe(actualParams.basicAuth) expect(authMiddlewareBase).toHaveBeenCalledTimes(1) + jest.unmock('../src/base-auth-flow') resolve() } const middlewareOptions = createTestMiddlewareOptions() From 6b781d72093b84b67e88c07ea66242296424f6a2 Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Mon, 21 Aug 2017 13:04:12 +0200 Subject: [PATCH 11/14] test(integration): add integration tests for customer login affects: @commercetools/sdk-middleware-auth --- docs/sdk/api/sdkMiddlewareAuth.md | 2 +- integration-tests/sdk/customer-login.it.js | 70 +++++++++++++++++++ .../sdk-middleware-auth/src/base-auth-flow.js | 38 +++++----- .../sdk-middleware-auth/src/build-requests.js | 9 +-- .../test/build-requests.spec.js | 10 +-- .../test/password-flow.spec.js | 2 +- 6 files changed, 101 insertions(+), 30 deletions(-) create mode 100644 integration-tests/sdk/customer-login.it.js diff --git a/docs/sdk/api/sdkMiddlewareAuth.md b/docs/sdk/api/sdkMiddlewareAuth.md index a17116774..c4b58313a 100644 --- a/docs/sdk/api/sdkMiddlewareAuth.md +++ b/docs/sdk/api/sdkMiddlewareAuth.md @@ -59,7 +59,7 @@ Creates a [middleware](/sdk/Glossary.md#middleware) to handle authentication for 1. `host` *(String)*: the host of the OAuth API service 2. `projectKey` *(String)*: the key of the project to assign the default scope to 3. `credentials` *(Object)*: the client credentials for authentication (`clientId`, `clientSecret`) -4. `scopes` *(Array)*: a list of [scopes](http://dev.commercetools.com/http-api-authorization.html#scopes) (default `manage_project:{projectKey}`) to assign to the OAuth token +4. `scopes` *(Array)*: a list of [scopes](http://dev.commercetools.com/http-api-authorization.html#scopes) to assign to the OAuth token. _No default scope is sent_ #### Usage example diff --git a/integration-tests/sdk/customer-login.it.js b/integration-tests/sdk/customer-login.it.js new file mode 100644 index 000000000..f65e2a6ca --- /dev/null +++ b/integration-tests/sdk/customer-login.it.js @@ -0,0 +1,70 @@ +import { createClient } from '@commercetools/sdk-client' +import { getCredentials } from '@commercetools/get-credentials' +import { + createAuthMiddlewareForPasswordFlow, +} from '@commercetools/sdk-middleware-auth' +import { + createHttpMiddleware, +} from '@commercetools/sdk-middleware-http' +import { clearData, createData } from './../cli/helpers/utils' + +let projectKey +if (process.env.CI === 'true') + projectKey = 'customers-login-integration-tests' +else + projectKey = process.env.npm_config_projectkey + +describe('Customer Login', () => { + const httpMiddleware = createHttpMiddleware({ + host: 'https://api.sphere.io', + }) + let apiConfig + const userEmail = `abi${Date.now()}@commercetooler.com` + const userPassword = 'asdifficultaspossible' + beforeAll(() => getCredentials(projectKey) + .then((credentials) => { + apiConfig = { + host: 'https://auth.sphere.io', + apiUrl: 'https://api.sphere.io', + projectKey, + credentials: { + clientId: credentials.clientId, + clientSecret: credentials.clientSecret, + }, + } + }) + .then(() => clearData(apiConfig, 'customers')) + .then(() => createData(apiConfig, 'customers', [{ + email: userEmail, + password: userPassword, + }])), + ) + afterAll(() => clearData(apiConfig, 'customers')) + it('should log customer and fetch customer profile', () => { + const userConfig = { + ...apiConfig, + ...{ scopes: [ `manage_project:${projectKey}` ] }, + ...{ credentials: { + clientId: apiConfig.credentials.clientId, + clientSecret: apiConfig.credentials.clientSecret, + user: { + username: userEmail, + password: userPassword, + }, + } }, + } + const client = createClient({ + middlewares: [ + createAuthMiddlewareForPasswordFlow(userConfig), + httpMiddleware, + ], + }) + return client.execute({ + uri: `/${projectKey}/me`, + method: 'GET', + }).then((response) => { + const user = response.body + expect(user.email).toBe(userEmail) + }) + }) +}) diff --git a/packages/sdk-middleware-auth/src/base-auth-flow.js b/packages/sdk-middleware-auth/src/base-auth-flow.js index 303627f7c..de99a49d7 100644 --- a/packages/sdk-middleware-auth/src/base-auth-flow.js +++ b/packages/sdk-middleware-auth/src/base-auth-flow.js @@ -87,29 +87,29 @@ export default function authMiddlewareBase ({ .then((res: Response): Promise<*> => { if (res.ok) return res.json() - .then((result: Object) => { - const token = result.access_token - const expiresIn = result.expires_in - const expirationTime = calculateExpirationTime(expiresIn) + .then((result: Object) => { + const token = result.access_token + const expiresIn = result.expires_in + const expirationTime = calculateExpirationTime(expiresIn) - // Cache new token - tokenCache.set({ token, expirationTime }) + // Cache new token + tokenCache.set({ token, expirationTime }) - // Dispatch all pending requests - requestState.set(false) + // Dispatch all pending requests + requestState.set(false) - // Freeze and copy pending queue, reset original one for accepting - // new pending tasks - const executionQueue = pendingTasks.slice() - // eslint-disable-next-line no-param-reassign - pendingTasks = [] - executionQueue.forEach((task: Task) => { - // Assign the new token in the request header - const requestWithAuth = mergeAuthHeader(token, task.request) - // console.log('test', cache, pendingTasks) - next(requestWithAuth, task.response) + // Freeze and copy pending queue, reset original one for accepting + // new pending tasks + const executionQueue = pendingTasks.slice() + // eslint-disable-next-line no-param-reassign + pendingTasks = [] + executionQueue.forEach((task: Task) => { + // Assign the new token in the request header + const requestWithAuth = mergeAuthHeader(token, task.request) + // console.log('test', cache, pendingTasks) + next(requestWithAuth, task.response) + }) }) - }) // Handle error response return res.text() diff --git a/packages/sdk-middleware-auth/src/build-requests.js b/packages/sdk-middleware-auth/src/build-requests.js index 7f656c416..b95f03152 100644 --- a/packages/sdk-middleware-auth/src/build-requests.js +++ b/packages/sdk-middleware-auth/src/build-requests.js @@ -75,16 +75,17 @@ export function buildRequestForPasswordFlow ( if (!(username && password)) throw new Error('Missing required user credentials (username, password)') - const defaultScope = `${authScopes.MANAGE_PROJECT}:${pKey}` - const scope = (options.scopes || [defaultScope]).join(' ') + const scope = (options.scopes || []).join(' ') + const scopeStr = scope ? `&scope=${scope}` : '' + const basicAuth = new Buffer(`${clientId}:${clientSecret}`).toString('base64') // This is mostly useful for internal testing purposes to be able to check // other oauth endpoints. - const oauthUri = options.oauthUri || `/oauth/${pKey}/token/customers/token` + const oauthUri = options.oauthUri || `/oauth/${pKey}/customers/token` const url = options.host.replace(/\/$/, '') + oauthUri // eslint-disable-next-line max-len - const body = `grant_type=password&scope=${scope}&username=${username}&password=${password}` + const body = `grant_type=password&username=${username}&password=${password}${scopeStr}` return { basicAuth, url, body } } diff --git a/packages/sdk-middleware-auth/test/build-requests.spec.js b/packages/sdk-middleware-auth/test/build-requests.spec.js index 5cfad0972..46ef1d268 100644 --- a/packages/sdk-middleware-auth/test/build-requests.spec.js +++ b/packages/sdk-middleware-auth/test/build-requests.spec.js @@ -29,14 +29,14 @@ function createTestOptions (options) { describe('buildRequestForPasswordFlow', () => { const body = oneLineTrim`grant_type=password& - scope=${allScopes.join(' ')}& - username=foobar&password=verysecurepassword + username=foobar&password=verysecurepassword& + scope=${allScopes.join(' ')} ` it('build request values with all the given options', () => { const options = createTestOptions() expect(buildRequestForPasswordFlow(options)).toEqual({ basicAuth: 'MTIzOnNlY3JldA==', - url: 'http://localhost:8080/oauth/test/token/customers/token', + url: 'http://localhost:8080/oauth/test/customers/token', body, }) }) @@ -56,7 +56,7 @@ describe('buildRequestForPasswordFlow', () => { }) expect(buildRequestForPasswordFlow(options)).toEqual({ basicAuth: 'MTIzOnNlY3JldA==', - url: 'http://localhost:8080/oauth/test/token/customers/token', + url: 'http://localhost:8080/oauth/test/customers/token', body, }) }) @@ -67,7 +67,7 @@ describe('buildRequestForPasswordFlow', () => { }) expect(buildRequestForPasswordFlow(options)).toEqual({ basicAuth: 'MTIzOnNlY3JldA==', - url: 'http://localhost:8080/oauth/test/token/customers/token', + url: 'http://localhost:8080/oauth/test/customers/token', body, }) }) diff --git a/packages/sdk-middleware-auth/test/password-flow.spec.js b/packages/sdk-middleware-auth/test/password-flow.spec.js index cdb17997e..d7b27a524 100644 --- a/packages/sdk-middleware-auth/test/password-flow.spec.js +++ b/packages/sdk-middleware-auth/test/password-flow.spec.js @@ -48,7 +48,7 @@ describe('Password Flow', () => { expect(actualParams.response).toEqual(actualParams.response) expect(actualParams.pendingTasks).toEqual([]) expect(actualParams.url).toBe( - 'https://auth.commercetools.co/oauth/foo/token/customers/token', + 'https://auth.commercetools.co/oauth/foo/customers/token', ) expect('MTIzOnNlY3JldA==').toBe(actualParams.basicAuth) expect(authMiddlewareBase).toHaveBeenCalledTimes(1) From 76eb40f6301c4878fea115b81dca4aec1f66d8cb Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Mon, 21 Aug 2017 13:14:24 +0200 Subject: [PATCH 12/14] refactor(auth): refactor code base on PR review affects: @commercetools/sdk-middleware-auth --- .../sdk-middleware-auth/src/base-auth-flow.js | 23 +------------ .../test/client-crendentials-flow.spec.js | 6 ++-- .../test/password-flow.spec.js | 6 ++-- types/sdk.js | 33 ++++++++++++++++--- 4 files changed, 35 insertions(+), 33 deletions(-) diff --git a/packages/sdk-middleware-auth/src/base-auth-flow.js b/packages/sdk-middleware-auth/src/base-auth-flow.js index de99a49d7..fc77cd65a 100644 --- a/packages/sdk-middleware-auth/src/base-auth-flow.js +++ b/packages/sdk-middleware-auth/src/base-auth-flow.js @@ -1,35 +1,14 @@ /* @flow */ import type { MiddlewareRequest, - MiddlewareResponse, Next, Task, + AuthMiddlewareBaseOptions, } from 'types/sdk' /* global fetch */ import 'isomorphic-fetch' -type RequestState = boolean -type TokenStore = { - token: string; - expirationTime: number; -} -type AuthMiddlewareBaseOptions = { - request: MiddlewareRequest; - response: MiddlewareResponse; - url: string; - body: string; - basicAuth: string; - pendingTasks: Array; - requestState: { - get: () => RequestState; - set: (requestState: RequestState) => RequestState; - }; - tokenCache: { - get: () => TokenStore; - set: (cache: TokenStore) => TokenStore; - } -} export default function authMiddlewareBase ({ request, diff --git a/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js b/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js index e46ab0545..bd36eb63a 100644 --- a/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js +++ b/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js @@ -39,13 +39,13 @@ describe('Client Crentials Flow', () => { reject, } const next = (actualParams) => { - expect(actualParams.request).toEqual(actualParams.request) - expect(actualParams.response).toEqual(actualParams.response) + expect(actualParams.request).toEqual(request) + expect(actualParams.response).toEqual(response) expect(actualParams.pendingTasks).toEqual([]) expect(actualParams.url).toBe( 'https://auth.commercetools.co/oauth/token', ) - expect('MTIzOnNlY3JldA==').toBe(actualParams.basicAuth) + expect(actualParams.basicAuth).toBe('MTIzOnNlY3JldA==') expect(authMiddlewareBase).toHaveBeenCalledTimes(1) resolve() jest.unmock('../src/base-auth-flow') diff --git a/packages/sdk-middleware-auth/test/password-flow.spec.js b/packages/sdk-middleware-auth/test/password-flow.spec.js index d7b27a524..9c61b71df 100644 --- a/packages/sdk-middleware-auth/test/password-flow.spec.js +++ b/packages/sdk-middleware-auth/test/password-flow.spec.js @@ -44,13 +44,13 @@ describe('Password Flow', () => { reject, } const next = (actualParams) => { - expect(actualParams.request).toEqual(actualParams.request) - expect(actualParams.response).toEqual(actualParams.response) + expect(actualParams.request).toEqual(request) + expect(actualParams.response).toEqual(response) expect(actualParams.pendingTasks).toEqual([]) expect(actualParams.url).toBe( 'https://auth.commercetools.co/oauth/foo/customers/token', ) - expect('MTIzOnNlY3JldA==').toBe(actualParams.basicAuth) + expect(actualParams.basicAuth).toBe('MTIzOnNlY3JldA==') expect(authMiddlewareBase).toHaveBeenCalledTimes(1) jest.unmock('../src/base-auth-flow') resolve() diff --git a/types/sdk.js b/types/sdk.js index f9c5bdcd9..eb4e0ebef 100644 --- a/types/sdk.js +++ b/types/sdk.js @@ -78,6 +78,34 @@ export type AuthMiddlewareOptions = { oauthUri: string; } +export type Task = { + request: MiddlewareRequest; + response: MiddlewareResponse; +} + +export type RequestState = boolean +export type TokenStore = { + token: string; + expirationTime: number; +} + +export type AuthMiddlewareBaseOptions = { + request: MiddlewareRequest; + response: MiddlewareResponse; + url: string; + body: string; + basicAuth: string; + pendingTasks: Array; + requestState: { + get: () => RequestState; + set: (requestState: RequestState) => RequestState; + }; + tokenCache: { + get: () => TokenStore; + set: (cache: TokenStore) => TokenStore; + } +} + export type PasswordAuthMiddlewareOptions = { host: string; projectKey: string; @@ -122,11 +150,6 @@ export type Next = ( request: MiddlewareRequest, response: MiddlewareResponse ) => mixed -export type Task = { - request: MiddlewareRequest; - response: MiddlewareResponse; -} - /* API Request Builder */ export type ServiceBuilderDefaultParams = { expand: Array; From 84a3057eea79cad5a3ec2cb4309f4b80216ea08f Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Mon, 21 Aug 2017 13:32:24 +0200 Subject: [PATCH 13/14] fix(integration-tests): correct projectKey --- integration-tests/sdk/customer-login.it.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/sdk/customer-login.it.js b/integration-tests/sdk/customer-login.it.js index f65e2a6ca..f619b4559 100644 --- a/integration-tests/sdk/customer-login.it.js +++ b/integration-tests/sdk/customer-login.it.js @@ -10,7 +10,7 @@ import { clearData, createData } from './../cli/helpers/utils' let projectKey if (process.env.CI === 'true') - projectKey = 'customers-login-integration-tests' + projectKey = 'customers-login-integration-test' else projectKey = process.env.npm_config_projectkey From 3a4290044ec789798feabd7f933d2ae4cf60d04c Mon Sep 17 00:00:00 2001 From: Abimbola Idowu Date: Mon, 21 Aug 2017 18:05:51 +0200 Subject: [PATCH 14/14] refactor(tests): use more elegant assertions affects: @commercetools/sdk-middleware-auth --- integration-tests/sdk/customer-login.it.js | 2 +- .../test/base-auth-flow.spec.js | 11 ++--------- .../test/client-crendentials-flow.spec.js | 14 +++++++------- .../sdk-middleware-auth/test/password-flow.spec.js | 14 +++++++------- 4 files changed, 17 insertions(+), 24 deletions(-) diff --git a/integration-tests/sdk/customer-login.it.js b/integration-tests/sdk/customer-login.it.js index f619b4559..ab1a79456 100644 --- a/integration-tests/sdk/customer-login.it.js +++ b/integration-tests/sdk/customer-login.it.js @@ -64,7 +64,7 @@ describe('Customer Login', () => { method: 'GET', }).then((response) => { const user = response.body - expect(user.email).toBe(userEmail) + expect(user).toHaveProperty('email', userEmail) }) }) }) diff --git a/packages/sdk-middleware-auth/test/base-auth-flow.spec.js b/packages/sdk-middleware-auth/test/base-auth-flow.spec.js index 3aedd4626..8d86b8014 100644 --- a/packages/sdk-middleware-auth/test/base-auth-flow.spec.js +++ b/packages/sdk-middleware-auth/test/base-auth-flow.spec.js @@ -54,15 +54,8 @@ describe('Base Auth Flow', () => { it('get a new auth token if not present in request headers', () => new Promise((resolve) => { - const request = createTestRequest() - const next = (req /* , res */) => { - expect(req).toEqual({ - ...request, - headers: { - ...request.headers, - Authorization: 'Bearer xxx', - }, - }) + const next = (req) => { + expect(req).toHaveProperty('headers.Authorization', 'Bearer xxx') resolve() } const middlewareOptions = createTestMiddlewareOptions() diff --git a/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js b/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js index bd36eb63a..cf1f41c43 100644 --- a/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js +++ b/packages/sdk-middleware-auth/test/client-crendentials-flow.spec.js @@ -39,13 +39,13 @@ describe('Client Crentials Flow', () => { reject, } const next = (actualParams) => { - expect(actualParams.request).toEqual(request) - expect(actualParams.response).toEqual(response) - expect(actualParams.pendingTasks).toEqual([]) - expect(actualParams.url).toBe( - 'https://auth.commercetools.co/oauth/token', - ) - expect(actualParams.basicAuth).toBe('MTIzOnNlY3JldA==') + expect(actualParams).toMatchObject({ + request, + response, + pendingTasks: [], + url: 'https://auth.commercetools.co/oauth/token', + basicAuth: 'MTIzOnNlY3JldA==', + }) expect(authMiddlewareBase).toHaveBeenCalledTimes(1) resolve() jest.unmock('../src/base-auth-flow') diff --git a/packages/sdk-middleware-auth/test/password-flow.spec.js b/packages/sdk-middleware-auth/test/password-flow.spec.js index 9c61b71df..0005003ab 100644 --- a/packages/sdk-middleware-auth/test/password-flow.spec.js +++ b/packages/sdk-middleware-auth/test/password-flow.spec.js @@ -44,13 +44,13 @@ describe('Password Flow', () => { reject, } const next = (actualParams) => { - expect(actualParams.request).toEqual(request) - expect(actualParams.response).toEqual(response) - expect(actualParams.pendingTasks).toEqual([]) - expect(actualParams.url).toBe( - 'https://auth.commercetools.co/oauth/foo/customers/token', - ) - expect(actualParams.basicAuth).toBe('MTIzOnNlY3JldA==') + expect(actualParams).toMatchObject({ + request, + response, + pendingTasks: [], + url: 'https://auth.commercetools.co/oauth/foo/customers/token', + basicAuth: 'MTIzOnNlY3JldA==', + }) expect(authMiddlewareBase).toHaveBeenCalledTimes(1) jest.unmock('../src/base-auth-flow') resolve()