Skip to content

WIP: Initial feature for OTP / authenticator passwords #5306

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

Closed
wants to merge 2 commits into from

Conversation

flovilmart
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Jan 20, 2019

Codecov Report

Merging #5306 into master will increase coverage by 0.01%.
The diff coverage is 90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5306      +/-   ##
==========================================
+ Coverage   93.88%   93.89%   +0.01%     
==========================================
  Files         123      123              
  Lines        8972     9007      +35     
==========================================
+ Hits         8423     8457      +34     
- Misses        549      550       +1
Impacted Files Coverage Δ
...dapters/Storage/Postgres/PostgresStorageAdapter.js 97.08% <100%> (ø) ⬆️
src/Controllers/DatabaseController.js 94.94% <100%> (ø) ⬆️
src/Adapters/Storage/Mongo/MongoTransform.js 88.18% <80%> (-0.07%) ⬇️
src/Routers/UsersRouter.js 92.69% <90%> (-0.77%) ⬇️
src/Adapters/Auth/httpsRequest.js 95.23% <0%> (-4.77%) ⬇️
src/RestWrite.js 93.06% <0%> (+0.18%) ⬆️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.21% <0%> (+0.72%) ⬆️

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 3851641...b395f15. Read the comment docs.

@awgeorge
Copy link
Contributor

awgeorge commented Feb 2, 2019

Implementing #2FA apps how do you store the TOTP secret so it is safe in case of a data leak? The server needs the secret in plain text to compare the token. What's your strategy?

Tokens should be secured at rest, check out 7.5 of the specification: https://www.ietf.org/rfc/rfc4226.txt. If the master key isn't already in the database, this could be used as the encryption key (not sure how often people change this key).

Depending on how you want the 2FA to be used, IE (only at login) you could use composite keys (Section 8), the user's password/pin is used to generate the token, meaning the database effectively only stores a seed.

@flovilmart
Copy link
Contributor Author

Ok, we could introduce a runtime encryption key for the tokens, different from the master key. Or just use the master key. What do you think?

Sent with GitHawk

@awgeorge
Copy link
Contributor

awgeorge commented Feb 2, 2019 via email

@flovilmart
Copy link
Contributor Author

Makes sense, we can add the option. I also will likely add an OTP adapter, so users will be able to transmit the OTP password by email or phone if they want to.

Sent with GitHawk

@awgeorge
Copy link
Contributor

awgeorge commented Feb 2, 2019

Good idea - I'm looking for an email based solution - (although I want a single auth, email only, passwordless solution)

@flovilmart
Copy link
Contributor Author

@acinader @dplewis closing this one as I don’t intend to work on it anymore

Sent with GitHawk

@flovilmart flovilmart closed this Apr 6, 2019
@acinader
Copy link
Contributor

acinader commented Apr 6, 2019

@awgeorge any interest in getting this across the finish line? If you can pick up where @flovilmart left off, I can get up to speed so I can review it with you.

@awgeorge
Copy link
Contributor

awgeorge commented Apr 6, 2019 via email

@brianmwadime
Copy link

This is something I’m interested in - I’ll take a look and see what’s left to get this implantation merged. —
Hey @awgeorge, were you able to get this to the finish line?

@awgeorge
Copy link
Contributor

Not at this time, feel free to take it on. Thanks.

@brianmwadime
Copy link

Alright @awgeorge. Thanks for the update.

@tehsunnliu
Copy link

Hey guys! is it finished yet? If anyone is interested we can do it together.

@mtrezza mtrezza deleted the feature/totp branch October 24, 2021 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants