accounts
accounts copied to clipboard
Multi-factor authentication
an alternative way for #777 of doing MFA following the auth0
way.
example login flow of user:
-
/loginWithService('password', { username, password })
- if the user has MFA enabled will return an MFA token & challenges (array of challenges:
sms
|email
|...
) to complete - otherwise will return the login response as before.
- if the user has MFA enabled will return an MFA token & challenges (array of challenges:
-
/performMfaChallenge('sms', mfaToken, { codeFromSms })
- upon success returns the mfaLoginToken.
-
/loginWithService('mfa', { mfaLoginToken })
- validates the
mfaLoginToken
and returns login response.
- validates the
db:
- [x] add database interface
- [x] implement database interface for mongo
- [x] implement database interface for manager
- [x] implement database interface for typeorm
server:
- [x] implement login process
- [x] add unit tests
rest api:
- [x] client
- [x] server
graphql api:
- [x] client
- [x] server
client:
- [x] add api of transport
@ozsay since it's pretty hard to have a generic loginWithService
and performMfaChallenge
mutation (you can pass an email + password or a sms code or whatever for other services) what do you think about adding a JSON scalar for graphql that would allow any inputs?
Codecov Report
:exclamation: No coverage uploaded for pull request base (
master@0470532
). Click here to learn what that means. The diff coverage is99.23%
.
@@ Coverage Diff @@
## master #796 +/- ##
========================================
Coverage ? 95.5%
========================================
Files ? 83
Lines ? 1937
Branches ? 401
========================================
Hits ? 1850
Misses ? 79
Partials ? 8
Impacted Files | Coverage Δ | |
---|---|---|
...raphql-api/src/modules/accounts/schema/mutation.ts | 100% <ø> (ø) |
|
...s/graphql-api/src/modules/accounts/schema/types.ts | 66.66% <ø> (ø) |
|
...ges/database-typeorm/src/entity/MfaLoginAttempt.ts | 100% <100%> (ø) |
|
packages/rest-express/src/express-middleware.ts | 100% <100%> (ø) |
|
packages/client/src/accounts-client.ts | 98.9% <100%> (ø) |
|
...t-express/src/endpoints/oauth/provider-callback.ts | 95.23% <100%> (ø) |
|
packages/client-password/src/client-password.ts | 100% <100%> (ø) |
|
...-api/src/modules/accounts/resolvers/loginResult.ts | 100% <100%> (ø) |
|
packages/database-mongo/src/mongo.ts | 99.36% <100%> (ø) |
|
...hql-api/src/modules/accounts/resolvers/mutation.ts | 94.44% <100%> (ø) |
|
... and 8 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 0470532...06c8d67. Read the comment docs.
@pradel
- Do we want to have the ability to limit the sms code to 3 attempts for example?
I think it's related to the #776 PR. The mfa
process is kinda agnostic of which authenticators are being used.
- should the authenticator api a part of this pull request or better to split in another one?
I don't think any changes are required to the authenticators API. I will test an alpha version this week, but I believe that it should work.
mfaChallenges
as a an array of string sounds not really extendable, where will you store the number associated to the user or other?
currently, it will allow authenticating with any of the challenges in the array (only 1 challenge completion is sufficient for login). We can extend it in the future for more complex scenarios. I think the basic scenarios are covered here.
- on the client isn't it better to return the
mfaToken
to the user instead of setting it in the local storage?
I can return it but I don't see any client usage of it, that's why I've "hidden" it inside accounts
thanks for the comments @pradel :)
Will this allow for 2-step authentication through email, or just SMS?
Also is this in the docs yet 😈?
@ozsay Are we missing something here? Could this be merged?
@davidyaha I proposed another design for the MFA implementation which will allow us to support more authenticators. I posted the architecture design on slack and I started an implementation on this pr https://github.com/accounts-js/accounts/pull/813