docs first pass and some status code / input name normalization

keratin · Oct 20, 2023 · be81fa9 · be81fa9
1 parent 890be18
commit be81fa9
Show file tree

Hide file tree

Showing 20 changed files with 129 additions and 28 deletions.
diff --git a/app/models/account.go b/app/models/account.go
@@ -23,7 +23,7 @@ func (a Account) Archived() bool {
 	return a.DeletedAt != nil
 }
 
-//TOTPEnabled returns true if TOTP is enabled on the account
+// TOTPEnabled returns true if OTP is enabled on the account
 func (a Account) TOTPEnabled() bool {
 	if a.TOTPSecret.Valid && a.TOTPSecret.String != "" {
 		return true

diff --git a/app/services/credentials_verifier.go b/app/services/credentials_verifier.go
@@ -47,7 +47,7 @@ func CredentialsVerifier(store data.AccountStore, cfg *app.Config, username stri
 		return nil, FieldErrors{{"credentials", ErrExpired}}
 	}
 
-	//Check TOTP MFA
+	//Check OTP MFA
 	if account.TOTPEnabled() {
 		secret, err := compat.Decrypt([]byte(account.TOTPSecret.String), cfg.DBEncryptionKey)
 		if err != nil {

diff --git a/app/services/password_resetter.go b/app/services/password_resetter.go
@@ -40,7 +40,7 @@ func PasswordResetter(store data.AccountStore, r ops.ErrorReporter, cfg *app.Con
 		return 0, FieldErrors{{"token", ErrInvalidOrExpired}}
 	}
 
-	//Check TOTP MFA
+	//Check OTP MFA
 	if account.TOTPEnabled() {
 		secret, err := compat.Decrypt([]byte(account.TOTPSecret.String), cfg.DBEncryptionKey)
 		if err != nil {

diff --git a/app/services/passwordless_token_verifier.go b/app/services/passwordless_token_verifier.go
@@ -38,7 +38,7 @@ func PasswordlessTokenVerifier(store data.AccountStore, r ops.ErrorReporter, cfg
 		return 0, FieldErrors{{"token", ErrInvalidOrExpired}}
 	}
 
-	//Check TOTP MFA
+	//Check OTP MFA
 	if account.TOTPEnabled() {
 		secret, err := compat.Decrypt([]byte(account.TOTPSecret.String), cfg.DBEncryptionKey)
 		if err != nil {

diff --git a/app/services/totp_creator.go b/app/services/totp_creator.go
@@ -8,14 +8,20 @@ import (
 	"github.com/pquerna/otp/totp"
 )
 
-//TOTPCreator handles the creation and storage of new TOTP tokens
-func TOTPCreator(accountStore data.AccountStore, totpCache data.TOTPCache, accountID int, audience *route.Domain) (*otp.Key, error) {
+var ErrExistingTOTPSecret = errors.New("a OTP secret has already been established for this account")
 
+// TOTPCreator handles the creation and storage of new OTP tokens
+func TOTPCreator(accountStore data.AccountStore, totpCache data.TOTPCache, accountID int, audience *route.Domain) (*otp.Key, error) {
 	account, err := AccountGetter(accountStore, accountID)
 	if err != nil {
 		return nil, err
 	}
 
+	if account.TOTPEnabled() {
+		// TODO: verify behavior here and test
+		return nil, ErrExistingTOTPSecret
+	}
+
 	//Generate totp key
 	key, err := totp.Generate(totp.GenerateOpts{
 		Issuer:      audience.Hostname,

diff --git a/app/services/totp_deleter.go b/app/services/totp_deleter.go
@@ -5,7 +5,7 @@ import (
 	"github.com/pkg/errors"
 )
 
-//TOTPDeleter removes TOTP from the specified account
+// TOTPDeleter removes OTP from the specified account
 func TOTPDeleter(accountStore data.AccountStore, accountID int) error {
 	//Delete totp secret in database
 	affected, err := accountStore.DeleteTOTPSecret(accountID)

diff --git a/app/services/totp_setter.go b/app/services/totp_setter.go
@@ -8,7 +8,7 @@ import (
 	"github.com/pquerna/otp/totp"
 )
 
-//TOTPSetter persists the TOTP secret to the accountID if code is correct
+// TOTPSetter persists the OTP secret to the accountID if code is correct
 func TOTPSetter(accountStore data.AccountStore, totpCache data.TOTPCache, cfg *app.Config, accountID int, code string) error {
 	if code == "" { //Fail early if code is empty
 		return FieldErrors{{"totp", ErrInvalidOrExpired}}

diff --git a/docs/_sidebar.md b/docs/_sidebar.md
@@ -1,6 +1,7 @@
 * **Implementation**
   * [OAuth](guide-implementing_oauth.md)
   * [Signup](guide-implementing_signup.md)
+  * [TOTP MFA](guide-implementing_totp_mfa_onboarding.md) **BETA**
   * [Login](guide-implementing_login.md)
   * [Logout](guide-implementing_logout.md)
   * [Reset Passwords](guide-implementing_forgotten_passwords.md)

diff --git a/docs/api.md b/docs/api.md
@@ -26,6 +26,10 @@
   * OAuth
     * [Begin OAuth](#begin-oauth)
     * [OAuth Return URL](#oauth-return)
+  * Multi-Factor Authentication (MFA) **BETA**
+    * [New](#totp-new)
+    * [Confirm](#totp-post)
+    * [Delete](#totp-delete)
   * Other
     * [Service Configuration](#service-configuration)
     * [JSON Web Keys](#json-web-keys)
@@ -330,10 +334,11 @@ Visibility: Public
 
 `POST /session`
 
-| Params | Type | Notes |
-| ------ | ---- | ----- |
-| `username` | string | &nbsp; |
-| `password` | string | &nbsp; |
+| Params     | Type   | Notes                               |
+|------------|--------|-------------------------------------|
+| `username` | string | &nbsp;                              |
+| `password` | string | &nbsp;                              |
+| `otp`      | string | required if MFA is setup on account |
 
 #### Success:
 
@@ -499,11 +504,12 @@ Handles password resets (with token) and password changes (with session).
 
 When [`PASSWORD_CHANGE_LOGOUT`](config.md#password_change_logout) is enabled, all existing sessions for the account will be expired before creating a new session on the current device.
 
-| Params | Type | Notes |
-| ------ | ---- | ----- |
-| `password` | string | Must meet minimum complexity scoring per [zxcvbn](https://blogs.dropbox.com/tech/2012/04/zxcvbn-realistic-password-strength-estimation/). |
-| `token` | JWT | As generated by [Request Password Reset](#request-password-reset). This is optional if the user is currently logged in to AuthN. |
-| `currentPassword` | string | Must exist when changing a password while logged in (not using token) |
+| Params            | Type   | Notes                                                                                                                                     |
+|-------------------|--------|-------------------------------------------------------------------------------------------------------------------------------------------|
+| `password`        | string | Must meet minimum complexity scoring per [zxcvbn](https://blogs.dropbox.com/tech/2012/04/zxcvbn-realistic-password-strength-estimation/). |
+| `token`           | JWT    | As generated by [Request Password Reset](#request-password-reset). This is optional if the user is currently logged in to AuthN.          |
+| `currentPassword` | string | Must exist when changing a password while logged in (not using token)                                                                     |
+| `otp`             | string | required if MFA is setup on account                                                                                                       |
 
 > NOTE: `password` must always be accompanied by _either_ `token` _or_ `currentPassword`.
 
@@ -647,6 +653,59 @@ If the OAuth process failed, the redirect will have `status=failed` appended to
     303 See Other
     Location: (redirect URI with status=failed)
 
+### Multi-Factor Authentication (MFA)
+
+**NOTE** - AuthN MFA support is currently considered in beta.  The API will not be considered stable until v2.
+
+#### New:
+Visibility: Public
+
+`POST /totp/new`
+
+#### Success:
+
+    200 Ok
+
+    {
+      "result": {
+        "secret": "XXXXXXXXXXXXX",
+        "url": "otpauth://xxxxxxxxxxxxxxxxxxxx",
+      }
+    }
+
+#### Failure:
+
+    401 Unauthorized
+    422 Unprocessable Entity
+
+#### Confirm:
+Visibility: Public
+
+`POST /totp/confirm`
+
+#### Success:
+
+    200 Ok
+
+#### Failure:
+
+    401 Unauthorized
+    422 Unprocessable Entity
+
+#### Delete: 
+Visibility: Public
+
+`POST /totp/delete`
+
+#### Success:
+
+    200 Ok
+
+#### Failure:
+
+    401 Unauthorized
+    422 Unprocessable Entity
+
 ### Service Configuration
 
 Visibility: Public

diff --git a/docs/guide-implementing_change_password.md b/docs/guide-implementing_change_password.md
@@ -10,8 +10,9 @@ Your users will sometimes want to change their passwords while they are logged i
 
 ### Frontend
 
-* Create a form where a logged-in user may enter their current and new passwords.
-* Submit the current and new passwords to AuthN.
+* Create a form where a logged-in user may enter their current and new passwords with an 
+  optional TOTP MFA code (required if the user has completed MFA onboarding with their authenticator app).
+* Submit the current and new passwords with the MFA code to AuthN.
 
 ## Related Guides
 

diff --git a/docs/guide-implementing_login.md b/docs/guide-implementing_login.md
@@ -16,6 +16,7 @@ will not declare which field was incorrect, but instead fails with a generic cre
 
 ### Frontend
 
-1. Create a form where the user may enter their username and password.
-2. Submit the username and password to AuthN.
+1. Create a form where the user may enter their username and password and on optional TOTP MFA
+   code (required if the user has completed MFA onboarding with their authenticator app).
+2. Submit the username, password and TOTP code to AuthN.
 3. If successful, the user will be logged in and can make authenticated requests to your app.
diff --git a/docs/guide-implementing_signup.md b/docs/guide-implementing_signup.md
@@ -40,6 +40,7 @@ process that submits with two API calls.
 ## Related Guides
 
 * Check for available usernames in real-time
+* [TOTP MFA Onboarding](guide-implementing_totp_mfa_onboarding.md)
 * [Displaying a password strength indicator](guide-displaying_a_password_strength_meter.md)
 * [Restrict signups by email domain](guide-restrict_signups_by_domain.md)
 * [Restrict signups by invitation](guide-restrict_signups_by_invitation.md)

diff --git a/docs/guide-implementing_totp_mfa_onboarding.md b/docs/guide-implementing_totp_mfa_onboarding.md
@@ -0,0 +1,23 @@
+# Multi-Factor Authentication
+
+**NOTE** - AuthN MFA support is currently considered in beta.  The API will not be considered stable until v2.
+
+AuthN supports multi-factor authentication using TOTP codes generated by a user's authenticator app.  It does not 
+presently support externally delivered codes. 
+
+## Configuration
+
+* No configuration needed.
+
+## Implementation
+
+Assuming an authenticated user applications can use the endpoint `/totp/new` to get a new TOTP secret for the user. 
+This response will contain both the secret and an onboarding URL - either of which can be used to present your front-end 
+onboarding flow to the user.  This secret is temporarily stored in redis pending user confirmation.
+
+After completing the onboarding flow your front end should have a valid code generated from the user's authenticator app 
+using the provided secret.  This should be posted to `/totp/confirm` which will persist the secret to the database and require 
+MFA on the user's account.
+
+Once a user's TOTP secret has been persisted it will be required to send a valid code generated from the secret in any 
+future posts to `/session` to authenticate that account. 
diff --git a/server/handlers/delete_totp.go b/server/handlers/delete_totp.go
@@ -18,7 +18,7 @@ func DeleteTOTP(app *app.App) http.HandlerFunc {
 		}
 
 		if err := services.TOTPDeleter(app.AccountStore, accountID); err != nil {
-			w.WriteHeader(http.StatusBadRequest)
+			w.WriteHeader(http.StatusUnprocessableEntity)
 			return
 		}
 

diff --git a/server/handlers/post_password.go b/server/handlers/post_password.go
@@ -16,7 +16,7 @@ func PostPassword(app *app.App) http.HandlerFunc {
 			Token           string
 			Password        string
 			CurrentPassword string
-			TOTP            string
+			OTP             string
 		}
 		if err := parse.Payload(r, &credentials); err != nil {
 			WriteErrors(w, err)
@@ -32,7 +32,7 @@ func PostPassword(app *app.App) http.HandlerFunc {
 				app.Config,
 				credentials.Token,
 				credentials.Password,
-				credentials.TOTP,
+				credentials.OTP,
 			)
 		} else {
 			accountID = sessions.GetAccountID(r)

diff --git a/server/handlers/post_password_test.go b/server/handlers/post_password_test.go
@@ -297,7 +297,7 @@ func TestPostPasswordWithTOTP(t *testing.T) {
 		res, err := client.PostForm("/password", url.Values{
 			"token":    []string{tokenStr},
 			"password": []string{"0a0b0c0d0"},
-			"totp":     []string{code},
+			"otp":      []string{code},
 		})
 		require.NoError(t, err)
 

diff --git a/server/handlers/post_session_test.go b/server/handlers/post_session_test.go
@@ -138,7 +138,7 @@ func TestPostSessionSuccessWithSessionAndTOTP(t *testing.T) {
 	accountID := 8642
 	session := test.CreateSession(app.RefreshTokenStore, app.Config, accountID)
 
-	//Generate TOTP code
+	//Generate OTP code
 	ok, err := app.AccountStore.SetTOTPSecret(account.ID, totpSecretEnc)
 	assert.True(t, ok)
 	require.NoError(t, err)

diff --git a/server/handlers/post_totp_confirm.go b/server/handlers/post_totp_confirm.go
@@ -8,7 +8,7 @@ import (
 	"github.com/keratin/authn-server/server/sessions"
 )
 
-// ConfirmTOTP finishes the TOTP onboarding process
+// ConfirmTOTP finishes the OTP onboarding process
 func ConfirmTOTP(app *app.App) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		// check for valid session with live token

diff --git a/server/handlers/post_totp_confirm_test.go b/server/handlers/post_totp_confirm_test.go
@@ -39,6 +39,11 @@ func TestPostTOTPConfirmSuccess(t *testing.T) {
 
 	assert.Equal(t, http.StatusOK, res.StatusCode)
 	assert.Equal(t, "", string(body))
+
+	// ensure that after confirmation a new secret cannot be requested
+	res, err = client.PostForm("/totp/new", url.Values{})
+	require.NoError(t, err)
+	assert.Equal(t, http.StatusUnprocessableEntity, res.StatusCode)
 }
 
 func TestPostTOTPConfirmFailure(t *testing.T) {

diff --git a/server/handlers/post_totp_create.go b/server/handlers/post_totp_create.go
@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"errors"
 	"net/http"
 
 	"github.com/keratin/authn-server/app"
@@ -9,7 +10,7 @@ import (
 	"github.com/keratin/authn-server/server/sessions"
 )
 
-// CreateTOTP begins the TOTP onboarding process
+// CreateTOTP begins the OTP onboarding process
 func CreateTOTP(app *app.App) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		// check for valid session with live token
@@ -21,6 +22,9 @@ func CreateTOTP(app *app.App) http.HandlerFunc {
 
 		totpKey, err := services.TOTPCreator(app.AccountStore, app.TOTPCache, accountID, route.MatchedDomain(r))
 		if err != nil {
+			if errors.Is(err, services.ErrExistingTOTPSecret) {
+				w.WriteHeader(http.StatusUnprocessableEntity)
+			}
 			panic(err)
 		}