-
-
Notifications
You must be signed in to change notification settings - Fork 141
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
[discuss] code authentication service #776
Open
ozsay
wants to merge
12
commits into
accounts-js:master
Choose a base branch
from
ozsay:code-authentication
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
328b170
code authentication service
ozsay 520f525
fix compilation
ozsay ef901ab
clear code when authentication is successful
ozsay 4e55972
add tests
ozsay 566792b
fix
ozsay a60f702
renaming package to twilio
ozsay 92616b5
api docs
ozsay 8dba844
add updateUser function
ozsay af225e9
Merge branch 'master' of github.com:accounts-js/accounts into code-au…
ozsay b10e182
yarn.lock
ozsay 786f35a
Merge branch 'master' of github.com:accounts-js/accounts into code-au…
ozsay 937f60a
update version
ozsay File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
# @accounts/code-provider-sms | ||
|
||
## Install | ||
|
||
``` | ||
yarn add @accounts/code-provider-sms | ||
``` | ||
|
||
## Usage | ||
|
||
```js | ||
import { AccountsServer } from '@accounts/server'; | ||
import { AccountsCode } from '@accounts/code'; | ||
import { CodeProviderSMS } from '@accounts/code-provider-sms'; | ||
|
||
const codeProvider = new CodeProviderSMS({ | ||
sid: 'TWILIO_SID', | ||
secret: 'TWILIO_SECRET', | ||
phoneNumber: 'TWILIO_FROM_PHONE_NUMBER', | ||
}); | ||
|
||
export const accountsCode = new AccountsCode({ | ||
codeProvider, | ||
// options | ||
}); | ||
|
||
const accountsServer = new AccountsServer( | ||
{ | ||
// options | ||
}, | ||
{ | ||
code: accountsCode, | ||
} | ||
); | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
{ | ||
"name": "@accounts/code-provider-sms", | ||
"version": "0.18.0", | ||
"license": "MIT", | ||
"main": "lib/index.js", | ||
"typings": "lib/index.d.ts", | ||
"scripts": { | ||
"clean": "rimraf lib", | ||
"start": "tsc --watch", | ||
"precompile": "yarn clean", | ||
"compile": "tsc", | ||
"prepublishOnly": "yarn compile" | ||
}, | ||
"jest": { | ||
"testEnvironment": "node", | ||
"preset": "ts-jest" | ||
}, | ||
"dependencies": { | ||
"twilio": "^3.33.3" | ||
}, | ||
"devDependencies": { | ||
"@accounts/code": "^0.18.0" | ||
}, | ||
"peerDependencies": { | ||
"@accounts/code": "^0.18.0" | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import { Twilio } from 'twilio'; | ||
|
||
import { CodeProvider } from '@accounts/code'; | ||
|
||
export type MessageCreator = (code: string) => string; | ||
|
||
export interface TwilioSmsCodeProviderOptions { | ||
sid: string; | ||
secret: string; | ||
phoneNumber?: string; | ||
messagingServiceSid?: string; | ||
messageCreator?: MessageCreator; | ||
} | ||
|
||
export default class TwilioSmsCodeProvider implements CodeProvider { | ||
private messageCreator: MessageCreator; | ||
private phoneNumber?: string; | ||
private messagingServiceSid?: string; | ||
private twilio: Twilio; | ||
|
||
constructor({ | ||
sid, | ||
secret, | ||
phoneNumber, | ||
messagingServiceSid, | ||
messageCreator = code => `This is your authentication code: ${code}`, | ||
}: TwilioSmsCodeProviderOptions) { | ||
this.twilio = new Twilio(sid, secret); | ||
this.phoneNumber = phoneNumber; | ||
this.messagingServiceSid = messagingServiceSid; | ||
this.messageCreator = messageCreator; | ||
} | ||
|
||
public async sendToClient(serviceId: string, code: string): Promise<void> { | ||
const options: any = { | ||
body: this.messageCreator(code), | ||
to: serviceId, | ||
}; | ||
|
||
if (this.phoneNumber) { | ||
options.from = this.phoneNumber; | ||
} else if (this.messagingServiceSid) { | ||
options.messagingServiceSid = this.messagingServiceSid; | ||
} else { | ||
throw new Error('Not enough twilio credentials'); | ||
} | ||
|
||
await this.twilio.messages.create(options); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import CodeProviderSMS from './code-provider-sms'; | ||
|
||
export default CodeProviderSMS; | ||
export { CodeProviderSMS }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"extends": "../../tsconfig", | ||
"compilerOptions": { | ||
"rootDir": "./src", | ||
"outDir": "./lib", | ||
"importHelpers": true | ||
}, | ||
"exclude": ["node_modules", "__tests__", "lib"] | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
# @accounts/code | ||
|
||
## Install | ||
|
||
``` | ||
yarn add @accounts/code | ||
``` | ||
|
||
## Usage | ||
|
||
```js | ||
import { AccountsServer } from '@accounts/server'; | ||
import { AccountsCode } from '@accounts/code'; | ||
|
||
const codeProvider = '...'; | ||
|
||
export const accountsCode = new AccountsCode({ | ||
codeProvider, | ||
// options | ||
}); | ||
|
||
const accountsServer = new AccountsServer( | ||
{ | ||
// options | ||
}, | ||
{ | ||
code: accountsCode, | ||
} | ||
); | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,195 @@ | ||
import { AccountsCode, CodeProvider, ErrorMessages } from '../src'; | ||
|
||
function createCodeProvider(success: boolean): CodeProvider { | ||
return { | ||
sendToClient: jest.fn(() => (success ? Promise.resolve() : Promise.reject())), | ||
}; | ||
} | ||
|
||
const errors: ErrorMessages = { | ||
userNotFound: '0', | ||
codeExpired: '1', | ||
codeWasNotFound: '2', | ||
wrongCode: '3', | ||
failedToProvideCode: '4', | ||
}; | ||
|
||
describe('AccountsCode', () => { | ||
describe('preparation', () => { | ||
it('throws error when no user is found', async () => { | ||
const code = new AccountsCode({ codeProvider: createCodeProvider(true), errors }); | ||
const findUserByServiceId = jest.fn(() => Promise.resolve()); | ||
code.setStore({ findUserByServiceId } as any); | ||
|
||
try { | ||
await code.prepareAuthentication('1234'); | ||
} catch (e) { | ||
expect(e.message).toEqual(errors.userNotFound); | ||
} | ||
|
||
expect.assertions(1); | ||
}); | ||
|
||
it('throws error when failed to send code to the client', async () => { | ||
const code = new AccountsCode({ codeProvider: createCodeProvider(false), errors }); | ||
|
||
const user = { | ||
id: '123', | ||
}; | ||
|
||
const findUserByServiceId = jest.fn(() => Promise.resolve(user)); | ||
const setService = jest.fn(() => Promise.resolve()); | ||
code.setStore({ findUserByServiceId, setService } as any); | ||
|
||
try { | ||
await code.prepareAuthentication('1234'); | ||
} catch (e) { | ||
expect(e.message).toEqual(errors.failedToProvideCode); | ||
} | ||
|
||
expect.assertions(1); | ||
}); | ||
|
||
it('completes successfully', async () => { | ||
const code = new AccountsCode({ codeProvider: createCodeProvider(true), errors }); | ||
|
||
const user = { | ||
id: '123', | ||
}; | ||
|
||
const findUserByServiceId = jest.fn(() => Promise.resolve(user)); | ||
const setService = jest.fn(() => Promise.resolve()); | ||
code.setStore({ findUserByServiceId, setService } as any); | ||
|
||
const res = await code.prepareAuthentication('1234'); | ||
|
||
expect(res).toBeUndefined(); | ||
expect(setService).toHaveBeenCalledTimes(1); | ||
expect(setService).toHaveBeenCalledWith('123', code.serviceName, { | ||
id: '1234', | ||
code: expect.any(String), | ||
expiry: expect.any(Number), | ||
}); | ||
}); | ||
}); | ||
|
||
describe('Authentication', () => { | ||
const code = new AccountsCode({ codeProvider: createCodeProvider(true), errors }); | ||
|
||
it('throws error when no user is found', async () => { | ||
const findUserByServiceId = jest.fn(() => Promise.resolve(null)); | ||
code.setStore({ findUserByServiceId } as any); | ||
|
||
try { | ||
await code.authenticate({ serviceId: '1234', code: '2233' }); | ||
} catch (e) { | ||
expect(e.message).toEqual(errors.userNotFound); | ||
} | ||
|
||
expect.assertions(1); | ||
}); | ||
|
||
it('throws error when no code is found', async () => { | ||
const findUserByServiceId = jest.fn(() => | ||
Promise.resolve({ | ||
id: '123', | ||
}) | ||
); | ||
code.setStore({ findUserByServiceId } as any); | ||
|
||
try { | ||
await code.authenticate({ serviceId: '1234', code: '2233' }); | ||
} catch (e) { | ||
expect(e.message).toEqual(errors.codeWasNotFound); | ||
} | ||
|
||
expect.assertions(1); | ||
}); | ||
|
||
it('throws error when no code is found 2', async () => { | ||
const findUserByServiceId = jest.fn(() => | ||
Promise.resolve({ | ||
id: '123', | ||
services: { | ||
[code.serviceName]: {}, | ||
}, | ||
}) | ||
); | ||
code.setStore({ findUserByServiceId } as any); | ||
|
||
try { | ||
await code.authenticate({ serviceId: '1234', code: '2233' }); | ||
} catch (e) { | ||
expect(e.message).toEqual(errors.codeWasNotFound); | ||
} | ||
|
||
expect.assertions(1); | ||
}); | ||
|
||
it('throws error when code is wrong', async () => { | ||
const findUserByServiceId = jest.fn(() => | ||
Promise.resolve({ | ||
id: '123', | ||
services: { | ||
[code.serviceName]: { | ||
code: '1111', | ||
}, | ||
}, | ||
}) | ||
); | ||
code.setStore({ findUserByServiceId } as any); | ||
|
||
try { | ||
await code.authenticate({ serviceId: '1234', code: '2233' }); | ||
} catch (e) { | ||
expect(e.message).toEqual(errors.wrongCode); | ||
} | ||
|
||
expect.assertions(1); | ||
}); | ||
|
||
it('throws error when code is expired', async () => { | ||
const findUserByServiceId = jest.fn(() => | ||
Promise.resolve({ | ||
id: '123', | ||
services: { | ||
[code.serviceName]: { | ||
code: 'vKw3G1T1mUWhSqSeLkCOXW5NvFk4f12M/GsBXUDVuwI=', | ||
expiry: Date.now() - 10000, | ||
}, | ||
}, | ||
}) | ||
); | ||
code.setStore({ findUserByServiceId } as any); | ||
|
||
try { | ||
await code.authenticate({ serviceId: '1234', code: '2233' }); | ||
} catch (e) { | ||
expect(e.message).toEqual(errors.codeExpired); | ||
} | ||
|
||
expect.assertions(1); | ||
}); | ||
|
||
it('authenticates successfully and returns user', async () => { | ||
const user = { | ||
id: '123', | ||
services: { | ||
[code.serviceName]: { | ||
code: 'vKw3G1T1mUWhSqSeLkCOXW5NvFk4f12M/GsBXUDVuwI=', | ||
expiry: Date.now() + 10000, | ||
}, | ||
}, | ||
}; | ||
const findUserByServiceId = jest.fn(() => Promise.resolve(user)); | ||
const setService = jest.fn(() => Promise.resolve()); | ||
code.setStore({ findUserByServiceId, setService } as any); | ||
|
||
const res = await code.authenticate({ serviceId: '1234', code: '2233' }); | ||
|
||
expect(res).toEqual(user); | ||
expect(setService).toHaveBeenCalledTimes(1); | ||
expect(setService).toHaveBeenCalledWith('123', code.serviceName, { id: '1234' }); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
{ | ||
"name": "@accounts/code", | ||
"version": "0.18.0", | ||
"license": "MIT", | ||
"main": "lib/index.js", | ||
"typings": "lib/index.d.ts", | ||
"scripts": { | ||
"clean": "rimraf lib", | ||
"start": "tsc --watch", | ||
"precompile": "yarn clean", | ||
"compile": "tsc", | ||
"prepublishOnly": "yarn compile", | ||
"testonly": "jest --coverage", | ||
"coverage": "jest --coverage" | ||
}, | ||
"jest": { | ||
"testEnvironment": "node", | ||
"preset": "ts-jest" | ||
}, | ||
"dependencies": { | ||
"@accounts/types": "^0.18.0", | ||
"lodash": "^4.17.15", | ||
"randomstring": "^1.1.5", | ||
"tslib": "1.10.0" | ||
}, | ||
"devDependencies": { | ||
"@accounts/server": "^0.18.0", | ||
"@types/randomstring": "^1.1.6", | ||
"@types/jest": "24.0.16", | ||
"jest": "24.8.0", | ||
"rimraf": "2.6.3" | ||
}, | ||
"peerDependencies": { | ||
"@accounts/server": "^0.18.0" | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Although it's safe to say that twilio pretty much dominates this field, I wonder if we should allow the user to supply some SMSService rather then depending on twilio.
Another option is to just call this package code-provider-twilio so that it will serve as an example for others that need different SMS providers. Once we get another feature request about some other provider we could refactor for generic SMS providers.
@pradel thoughts?
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.
renaming it to
twilio
will prevent another layer of abstraction that we might not need.. so i'm +1 for doing it.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.
notifme is a pretty good abstraction, and their notification catcher is really cool.
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.
Wow didn't know about notifme.. it looks great! What do you think @ozsay?
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.
it may be useful, though I can't see all the features that
twilio
provides (likemessagingServiceSid
)