-
Notifications
You must be signed in to change notification settings - Fork 70
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
212 password flow #281
212 password flow #281
Changes from all commits
062080e
6e6106d
4b2c0cd
92bb68f
f6a9636
ba0358b
0249b2d
a8312ed
45e0118
8fbdc12
6b781d7
76eb40f
84a3057
3a42900
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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-test' | ||
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).toHaveProperty('email', userEmail) | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
/* @flow */ | ||
import type { | ||
MiddlewareRequest, | ||
Next, | ||
Task, | ||
AuthMiddlewareBaseOptions, | ||
} from 'types/sdk' | ||
|
||
/* global fetch */ | ||
import 'isomorphic-fetch' | ||
|
||
|
||
export default function authMiddlewareBase ({ | ||
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 | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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,48 @@ 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shorter var names to avoid eslint error and unnecessary multiline I think it's a good trade off 🤷♂️ |
||
if (!(clientId && clientSecret && user)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For better handling, this could be checked one after the other There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain what you mean by "better handling"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Better usability" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, all is at the same level else we can as well begin to validate each field, and that won't make too much sense. That's why I am validating That's my thinking |
||
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 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}/customers/token` | ||
const url = options.host.replace(/\/$/, '') + oauthUri | ||
// eslint-disable-next-line max-len | ||
const body = `grant_type=password&username=${username}&password=${password}${scopeStr}` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought we were moving towards using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is in the main code (that is being used in the browser also), I don't think it make sense to add a new module to the build just to trim new line. Not a valid trade off from my point of view. |
||
|
||
return { basicAuth, url, body } | ||
} | ||
|
||
export function buildRequestForRefreshTokenFlow () { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are you covering both?
isn't it better to go for one of them and allow the other to fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it's can be an issue that occurred from the http module used when fetching OAuth token from the API