From 51a76ee29687d55f0baf621f36dfa5d3497c205d Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Fri, 12 May 2023 19:12:09 +0200 Subject: [PATCH 1/7] feat: first draft of mfa --- 021-multi-factor-authentication.md | 130 +++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 021-multi-factor-authentication.md diff --git a/021-multi-factor-authentication.md b/021-multi-factor-authentication.md new file mode 100644 index 0000000..7cf5f1d --- /dev/null +++ b/021-multi-factor-authentication.md @@ -0,0 +1,130 @@ +# Multi Factor Authentication + +- Implementation Owner: @TorstenDittmann +- Start Date: 12-06-2023 +- Target Date: 01-07-2023 +- Appwrite Issue: [#5240](https://github.com/appwrite/appwrite/issues/5240) + +## Summary + +[summary]: #summary + +Single factor authentication poses a security risk to user accounts. If a user's password is compromised, their account can be easily accessed by an attacker. To mitigate this risk, I propose adding Multi Factor Authentication (MFA) to Appwrite's Accounts Service. + +## Problem Statement (Step 1) + +[problem-statement]: #problem-statement + +**What problem are you trying to solve?** + +Appwrite currently only supports single factor authentication. This presents a security risk as it leaves user accounts vulnerable to attacks such as password guessing and phishing. To address this, I propose implementing Multi Factor Authentication (MFA) in Appwrite's Accounts Service. + +## Design proposal (Step 2) + +[design-proposal]: #design-proposal + +### Flow + +Scenario: User **Walter** has 2FA on his account enabled. + +- Walter attempts to create a session using his e-mail and password + - Appwrite identifies that he has MFA enabled + - Appwrite creates a challenge + - Appwrite returns an exception with the type `mfa_required`, challenge token and user id, indicating to the user that MFA is needed to continue +- Walter's application retrieves the list of available providers with the user id +- Walter's application initiates the MFA challenge with the token and a provider of choice +- Walter chooses TOTP, enters the current authentication key to verifiy the challenge +- The challenge was successful and a session is created + +It is important to point out, that the attempt to create a session when MFA is enabled will throw an exception - making the developer aware of additional steps. + +The exception will contain the informations needed to create and verify a challenge. + +### API Endpoints + +I propose the following endpoints for MFA implementation: + +#### Account Service +`PATCH: /v1/account/mfa` enables/disables MFA for the account +`POST: /v1/account/mfa/totp` adds TOTP authenticator to account +`PUT: /v1/account/mfa/totp` verifies added TOTP authenticator +`DELETE: /v1/account/mfa/totp` removes TOTP authenticator from account +`GET: /v1/account/mfa/providers`: returns a list of available MFA providers +`POST: /v1/account/mfa/challenge`: initiates an MFA challenge with the provider of choice and returns a challenge token +`PUT: /v1/account/mfa/challenge`: verifies an MFA challenge and generates a session + +#### User Service +`PATCH: /v1/user/mfa`: enables/disables disables MFA for the user +`GET: /v1/user/mfa/providers`: returns a list of available MFA providers +`DELETE: /v1/user/mfa/totp`: removes authenticator from user + +Obviously all crucial/sensitive endpoints will be covered by rate limits. + +### Data Structure + +For my proposal I need to add a new internal collection called `challenges` with following data structure (excluding default `$` attributes): +- `token` secret to be used for identification, similiar to the `secret` of the tokens collections +- `provider` the provider used for the challenge (TOTP, SMS, E-Mail, etc) +- `session` the type of session that has triggered the creation of the challenge + +Additionally we need to add new properties to the `user` collection: +- `mfa` boolean to whether MFA is enabled or not +- `totp` boolean to whether a TOTP device is active +- `totp_secret` the secret key to be used with a TOTP authentication app +- `totp_backup` array of backup codes that can each used once + +### Supporting Libraries + +Following libraries can be used as dependency or inspiration: + +- https://github.com/Spomky-Labs/otphp +- https://github.com/lfkeitel/php-totp + +### Breaking Changes + +No breaking changes will be introduced on the API. But a few data structure changes will have to be migrated. + +### Reliability (Tests & Benchmarks) + +#### Scaling + +This should already be scalable out-of-the-box with existing Appwrite scalability. + +#### Benchmarks + +Due to the nature of this feature, I don't expect any complications with performance. Of course this will be manually checked during migration. + +#### Tests (UI, Unit, E2E) + +- Unit Tests with highest Coverage possible +- E2E Tests covering: + - all new endpoints + - all challenges tested for endpoints creating a session + +### Documentation & Content + +We probably need a new Section in the Documentation explainig the steps to implement MFA. + +### Prior art + +[prior-art]: #prior-art + +I investigate existing MFA implementations and best practices to inform our design proposal. Specifically, I will review MFA implementation in other authentication services, such as Auth0 and Okta. + +Also existing RFC's for TOTP are a helpful resource ([RFC 6238](https://datatracker.ietf.org/doc/html/rfc6238)). + +Additionally, this feature is planned in a way - that we can extend the MFA to be used with more than 2 factors in the future. + +### Unresolved questions + +[unresolved-questions]: #unresolved-questions + +- A project wide setting that MFA needs to be used +- A team wide setting that MFA is required +- We might wanna think about adding additional roles for users with MFA + +### Future possibilities + +[future-possibilities]: #future-possibilities + +Right now this RFC only covers a challenge adding a second factor, in the future we can extend with even more factors need in total and also different factors than TOTP, E-Mail and SMS. Something like Captcha, HOTP, wabauthn and co. From 386a097f0a132c2a71cd4ead31858af38472460b Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Fri, 12 May 2023 19:15:04 +0200 Subject: [PATCH 2/7] Update 021-multi-factor-authentication.md --- 021-multi-factor-authentication.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/021-multi-factor-authentication.md b/021-multi-factor-authentication.md index 7cf5f1d..14604ba 100644 --- a/021-multi-factor-authentication.md +++ b/021-multi-factor-authentication.md @@ -45,18 +45,18 @@ The exception will contain the informations needed to create and verify a challe I propose the following endpoints for MFA implementation: #### Account Service -`PATCH: /v1/account/mfa` enables/disables MFA for the account -`POST: /v1/account/mfa/totp` adds TOTP authenticator to account -`PUT: /v1/account/mfa/totp` verifies added TOTP authenticator -`DELETE: /v1/account/mfa/totp` removes TOTP authenticator from account -`GET: /v1/account/mfa/providers`: returns a list of available MFA providers -`POST: /v1/account/mfa/challenge`: initiates an MFA challenge with the provider of choice and returns a challenge token -`PUT: /v1/account/mfa/challenge`: verifies an MFA challenge and generates a session +- `PATCH: /v1/account/mfa` enables/disables MFA for the account +- `POST: /v1/account/mfa/totp` adds TOTP authenticator to account +- `PUT: /v1/account/mfa/totp` verifies added TOTP authenticator +- `DELETE: /v1/account/mfa/totp` removes TOTP authenticator from account +- `GET: /v1/account/mfa/providers`: returns a list of available MFA providers +- `POST: /v1/account/mfa/challenge`: initiates an MFA challenge with the provider of choice and returns a challenge token +- `PUT: /v1/account/mfa/challenge`: verifies an MFA challenge and generates a session #### User Service -`PATCH: /v1/user/mfa`: enables/disables disables MFA for the user -`GET: /v1/user/mfa/providers`: returns a list of available MFA providers -`DELETE: /v1/user/mfa/totp`: removes authenticator from user +- `PATCH: /v1/user/mfa`: enables/disables disables MFA for the user +- `GET: /v1/user/mfa/providers`: returns a list of available MFA providers +- `DELETE: /v1/user/mfa/totp`: removes authenticator from user Obviously all crucial/sensitive endpoints will be covered by rate limits. From 73308bd80e150a724f1a7468b3b4ebc67f783f97 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Fri, 12 May 2023 19:17:34 +0200 Subject: [PATCH 3/7] Update 021-multi-factor-authentication.md --- 021-multi-factor-authentication.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/021-multi-factor-authentication.md b/021-multi-factor-authentication.md index 14604ba..2370f90 100644 --- a/021-multi-factor-authentication.md +++ b/021-multi-factor-authentication.md @@ -50,8 +50,8 @@ I propose the following endpoints for MFA implementation: - `PUT: /v1/account/mfa/totp` verifies added TOTP authenticator - `DELETE: /v1/account/mfa/totp` removes TOTP authenticator from account - `GET: /v1/account/mfa/providers`: returns a list of available MFA providers -- `POST: /v1/account/mfa/challenge`: initiates an MFA challenge with the provider of choice and returns a challenge token -- `PUT: /v1/account/mfa/challenge`: verifies an MFA challenge and generates a session +- `POST: /v1/account/mfa/challenge`: initiates a MFA challenge with the provider of choice and returns a challenge +- `PUT: /v1/account/mfa/challenge`: verifies a MFA challenge and generates a session #### User Service - `PATCH: /v1/user/mfa`: enables/disables disables MFA for the user From f668fe9915ae1df9c1683727a3c4b2ad2acf5179 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Fri, 12 May 2023 19:18:40 +0200 Subject: [PATCH 4/7] fix: add more links --- 021-multi-factor-authentication.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/021-multi-factor-authentication.md b/021-multi-factor-authentication.md index 2370f90..02934d9 100644 --- a/021-multi-factor-authentication.md +++ b/021-multi-factor-authentication.md @@ -3,7 +3,9 @@ - Implementation Owner: @TorstenDittmann - Start Date: 12-06-2023 - Target Date: 01-07-2023 -- Appwrite Issue: [#5240](https://github.com/appwrite/appwrite/issues/5240) +- Appwrite Issue: + - [#2725](https://github.com/appwrite/appwrite/issues/2752) + - [#5240](https://github.com/appwrite/appwrite/issues/5240) ## Summary From e3dd5de52a0ace7268ebf3c28346dfc4c352d540 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Sat, 13 May 2023 17:21:08 +0200 Subject: [PATCH 5/7] Update 021-multi-factor-authentication.md --- 021-multi-factor-authentication.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/021-multi-factor-authentication.md b/021-multi-factor-authentication.md index 02934d9..7d0e4fa 100644 --- a/021-multi-factor-authentication.md +++ b/021-multi-factor-authentication.md @@ -72,8 +72,8 @@ For my proposal I need to add a new internal collection called `challenges` with Additionally we need to add new properties to the `user` collection: - `mfa` boolean to whether MFA is enabled or not - `totp` boolean to whether a TOTP device is active -- `totp_secret` the secret key to be used with a TOTP authentication app -- `totp_backup` array of backup codes that can each used once +- `totpSecret` the secret key to be used with a TOTP authentication app +- `totpBackup` array of backup codes that can each used once ### Supporting Libraries From d615afade793ee528557e69d3250708c6af7e2a7 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Tue, 16 May 2023 00:16:25 +0200 Subject: [PATCH 6/7] extend rfc with hotp --- 021-multi-factor-authentication.md | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/021-multi-factor-authentication.md b/021-multi-factor-authentication.md index 7d0e4fa..dfa1e8b 100644 --- a/021-multi-factor-authentication.md +++ b/021-multi-factor-authentication.md @@ -48,9 +48,9 @@ I propose the following endpoints for MFA implementation: #### Account Service - `PATCH: /v1/account/mfa` enables/disables MFA for the account -- `POST: /v1/account/mfa/totp` adds TOTP authenticator to account -- `PUT: /v1/account/mfa/totp` verifies added TOTP authenticator -- `DELETE: /v1/account/mfa/totp` removes TOTP authenticator from account +- `POST: /v1/account/mfa/:provider` adds TOTP/HOTP authenticator to account +- `PUT: /v1/account/mfa/:provider` verifies added TOTP/HOTP authenticator +- `DELETE: /v1/account/mfa/:provider` removes TOTP/HOTP authenticator from account - `GET: /v1/account/mfa/providers`: returns a list of available MFA providers - `POST: /v1/account/mfa/challenge`: initiates a MFA challenge with the provider of choice and returns a challenge - `PUT: /v1/account/mfa/challenge`: verifies a MFA challenge and generates a session @@ -58,7 +58,12 @@ I propose the following endpoints for MFA implementation: #### User Service - `PATCH: /v1/user/mfa`: enables/disables disables MFA for the user - `GET: /v1/user/mfa/providers`: returns a list of available MFA providers -- `DELETE: /v1/user/mfa/totp`: removes authenticator from user +- `DELETE: /v1/user/mfa/totp`: removes TOTP authenticator from user +- `DELETE: /v1/user/mfa/hotp`: removes HOTP authenticator from user + +#### Project Service +- `PATCH: /v1/projects/:projectId/mfa`: enables/disables disables MFA for the project +- `PATCH: /v1/projects/:projectId/mfa/:provider` enable/disable available providers to be used for MFA Obviously all crucial/sensitive endpoints will be covered by rate limits. @@ -66,14 +71,17 @@ Obviously all crucial/sensitive endpoints will be covered by rate limits. For my proposal I need to add a new internal collection called `challenges` with following data structure (excluding default `$` attributes): - `token` secret to be used for identification, similiar to the `secret` of the tokens collections -- `provider` the provider used for the challenge (TOTP, SMS, E-Mail, etc) -- `session` the type of session that has triggered the creation of the challenge +- `provider` the provider used for the challenge (TOTP, HOTP, SMS, E-Mail, etc) +- `session` the session that will be created when the challenge is completed Additionally we need to add new properties to the `user` collection: - `mfa` boolean to whether MFA is enabled or not - `totp` boolean to whether a TOTP device is active - `totpSecret` the secret key to be used with a TOTP authentication app - `totpBackup` array of backup codes that can each used once +- `hotp` boolean to whether a HOTP device is active +- `hotpSecret` the secret key to be used with a HOTP authentication app +- `hotpBackup` array of backup codes that can each used once ### Supporting Libraries @@ -121,7 +129,6 @@ Additionally, this feature is planned in a way - that we can extend the MFA to b [unresolved-questions]: #unresolved-questions -- A project wide setting that MFA needs to be used - A team wide setting that MFA is required - We might wanna think about adding additional roles for users with MFA From 4e147806faf873f2cf315ab40deae19f675b47f3 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Tue, 16 May 2023 00:19:24 +0200 Subject: [PATCH 7/7] add algorithms part --- 021-multi-factor-authentication.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/021-multi-factor-authentication.md b/021-multi-factor-authentication.md index dfa1e8b..696baeb 100644 --- a/021-multi-factor-authentication.md +++ b/021-multi-factor-authentication.md @@ -90,6 +90,11 @@ Following libraries can be used as dependency or inspiration: - https://github.com/Spomky-Labs/otphp - https://github.com/lfkeitel/php-totp +### Algorithms + +- https://en.wikipedia.org/wiki/Time-based_one-time_password +- https://en.wikipedia.org/wiki/HMAC-based_one-time_password + ### Breaking Changes No breaking changes will be introduced on the API. But a few data structure changes will have to be migrated.