Bug fixes and remove login states (#22)

* add app-env.json and update port * Update app-env.json * Update app-env.json * update detect-secrets, update secrets baseline * update secrets baseline * update makefile versioning * Update Makefile Fix typo * print version * upgrade go to v1.20 * fix secrets * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * [rokwire#659] WebAuthn authentication (#7) * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * fix error handling * fix login issues for mobile * upgrade dependencies * [rokwire#659] webauthn authentication (#8) * initial webauthn implementation (in progress) * refactor webauthn to handle credentials, update docs * avoid creating inaccessible accounts * fix webauthn registration issues, add webauthn test page * fix webauthn login flow * update changelog * fix error handling * fix login issues for mobile * upgrade dependencies * add webauthn to account check types * add configs for authenticator selection to supported auth type params * add configs for authenticator selection to supported auth type params (#10) * start adding verification types (contains errors) [rokwire#665] * continue splitting auth and verification types [rokwire#665] * finish implementing password auth type, start code verification type, add phone verifier interface [rokwire#665] * finish refactoring identifier, auth types, start updating apis [rokwire#665] * upgrade dependencies * finish fixing errors [rokwire#665] * fix passkey errors [rokwire#665] * bug fixes, email with passkey not working because no params in email auth type * update identifier impl and auth impl getters to better handle backwards compatibility (has errors) * bug fixes, email and passkey not completing registration * add json omitempty tags to credential structs * better identifier type parsing * passkeys using email and username identifiers working * start fixing phone, passkey auth * bug fixes for phone and passkey, better error messages * simplify phone verifier interface * phone auth type link working, add authCommunicationChannel interface to handle verification functions * add ability to link webauthn credentials to accounts * only set username if empty * Change messages handling for verification * remove commented blocks * cleanup * return verified auth types when cannot find account with username but not identifier * bug fixes * fix phone auth type docs * make sure usernames are lowercase, do not fail if phone verifier fails to init * start refactoring account auth types into account auth types and account identifiers, do not store identifiers in credentials (contains errors) * more progress, identifier type and auth type simplifications * read email verification settings from config, refactor verify credential APIs to verify identifier * build error fixes, add account identifier storage operations * start preparing for multiple credentials of same type (e.g., passkeys) * some progress implementing passkey with identifier flow * rewrite passkey flows * build error cleanup, start moving account external IDs into identifiers, refactor shared profile stuff * start fixing shared profiles * comment shared profile functionality * update API docs for linking auth types and identifiers, more error fixes * more API doc tweaks, more error fixes, remove all uses of claims.UID * more link and unlink request body tweaks * do not store account auth type ID in login session, instead use identifier to get account * start working on link account auth type implementation updates * more link auth type updates * finish implementing link identifier, some code auth type bug fixes * match LinkAccountIdentifier interface definition * fix remaining build errors, begin implementing DB migration * add support for login using external identifiers, update more request body definitions * identifiers bug fixes, fix build errors * update db indexes * start implementing db migration * do not allow generic oidc auth type code - no specified identity provider * implement app org and auth type migrations * credentials migration working, use json convert utils func * login session migration done * accounts migration done, a few bug fixes related to external IDs * move migration functions into separate file * more bug fixes, accountAuthTypesToDef not working * email and phone login fixes, finish implementing identifier-less login * bug fixes * linking, unlinking bug fixes * more linking, unlinking bug fixes and identifier verification email bug fixes * username login, webauthn backwards compatible login bug fixes * fix identifier-less webauthn login, update canLink * fix passkey sign up * return account on webauthn signup * make OIDC ID tokens optional * add sign-in-options API, update login API to accept account identifier ID * finish implementing sign in options and login with identifier ID * mask email and phone identifiers for sign in options, add regexp to validate emails * clean up linking, unlinking * fix unlink examples, sign in options fix, handle nil identifier when linking * upgrade dependencies, set username in token claims * use error statuses for auth type and identifier linking * allow webauthn credentials to be created after account already exists * update changelog * updates and fixes for conde_oidc, started refactoring email and phone from profile, username from account into identifiers * fix build errors * start handling external email identifiers * implement profile email and phone and account username migrations * set sensitive flags for email and phone migrations * finish implementing identifier sensitive field, return profile email, phone and username for BC * disallow updating account username to empty string * add sensitive field to account identifier api model * bug fixes * mark email as external if it matches external email field * usernames verified by default, identifiers used to sign up with webauthn unverified by default * auth type unlink bug fixes * simplify link auth type transaction, add app type identifier to webauthn aat params * fix link docs example * return identifiers on auth type link and unlink * improve external identifier migration, add IsEmailVerified flag * do not change email sensitivity on update external identifiers * remove API docs comment * fix go mod * fix webauthn beginLogin with identifier (missing user.Name) * move phone and email validation to utils, validate profile phone and email on account migration * delete login states once used successfully, fix email and code login * add context to UpdateCredentialValue storage function * merged changes from rokwire-674 --------- Co-authored-by: Stephen Hurwit <[email protected]> Co-authored-by: Stephen Hurwit <[email protected]> Co-authored-by: akshadpai <[email protected]>
rokmetro · Oct 14, 2023 · 9e6a366 · 9e6a366
1 parent 41cda14
commit 9e6a366
Show file tree

Hide file tree

Showing 11 changed files with 91 additions and 29 deletions.
diff --git a/.secrets.baseline b/.secrets.baseline
@@ -185,21 +185,21 @@
         "filename": "core/auth/auth.go",
         "hashed_secret": "58f3388441fbce0e48aef2bf74413a6f43f6dc70",
         "is_verified": false,
-        "line_number": 991
+        "line_number": 992
       },
       {
         "type": "Secret Keyword",
         "filename": "core/auth/auth.go",
         "hashed_secret": "44e17306b837162269a410204daaa5ecee4ec22c",
         "is_verified": false,
-        "line_number": 2676
+        "line_number": 2682
       },
       {
         "type": "Secret Keyword",
         "filename": "core/auth/auth.go",
         "hashed_secret": "94a7f0195bbbd2260c4e4d02b6348fbcd90b2b30",
         "is_verified": false,
-        "line_number": 2806
+        "line_number": 2812
       }
     ],
     "core/auth/auth_type_oidc.go": [
@@ -233,7 +233,7 @@
         "filename": "core/auth/identifier_type_email.go",
         "hashed_secret": "69411040443be576ce64fc793269d7c26dd0866a",
         "is_verified": false,
-        "line_number": 251
+        "line_number": 249
       }
     ],
     "core/auth/service_static_token.go": [
@@ -377,5 +377,5 @@
       }
     ]
   },
-  "generated_at": "2023-10-06T23:08:57Z"
+  "generated_at": "2023-10-11T21:26:54Z"
 }
diff --git a/core/auth/apis.go b/core/auth/apis.go
@@ -1094,7 +1094,7 @@ func (a *Auth) UpdateCredential(accountID string, accountAuthTypeID string, para
 
 	//Update the credential with new password
 	accountAuthType.Credential.Value = authTypeCreds
-	if err = a.storage.UpdateCredentialValue(accountAuthType.Credential.ID, accountAuthType.Credential.Value); err != nil {
+	if err = a.storage.UpdateCredentialValue(nil, accountAuthType.Credential.ID, accountAuthType.Credential.Value); err != nil {
 		return errors.WrapErrorAction(logutils.ActionUpdate, model.TypeCredential, nil, err)
 	}
 
@@ -1147,7 +1147,7 @@ func (a *Auth) ResetForgotCredential(credsID string, resetCode string, params st
 	}
 	//Update the credential with new password
 	credential.Value = authTypeCreds
-	if err = a.storage.UpdateCredentialValue(credential.ID, credential.Value); err != nil {
+	if err = a.storage.UpdateCredentialValue(nil, credential.ID, credential.Value); err != nil {
 		return errors.WrapErrorAction(logutils.ActionUpdate, model.TypeCredential, nil, err)
 	}
 
@@ -1243,7 +1243,7 @@ func (a *Auth) ForgotCredential(authenticationType string, identifierJSON string
 
 	//Update the credential with reset code and expiry
 	credential.Value = authTypeCreds
-	if err = a.storage.UpdateCredentialValue(credential.ID, credential.Value); err != nil {
+	if err = a.storage.UpdateCredentialValue(nil, credential.ID, credential.Value); err != nil {
 		return errors.WrapErrorAction(logutils.ActionUpdate, model.TypeCredential, nil, err)
 	}
 	return nil

diff --git a/core/auth/auth.go b/core/auth/auth.go
@@ -625,6 +625,7 @@ func (a *Auth) applyAuthType(supportedAuthType model.SupportedAuthType, appOrg m
 
 	var account *model.Account
 	if identifierImpl == nil {
+		// if given an account identifier ID, find the account and attempt sign in
 		if accountIdentifierID != nil {
 			account, err = a.storage.FindAccountByIdentifierID(nil, *accountIdentifierID)
 			if err != nil {
@@ -2608,6 +2609,11 @@ func (a *Auth) updateExternalIdentifiers(account *model.Account, accountAuthType
 					primary := (externalUser.Email == externalUser.Identifier)
 					account.Identifiers[i].Identifier = externalUser.Email
 					account.Identifiers[i].Primary = &primary
+					// if the external email is not already verified, set verified to the default setting
+					if !account.Identifiers[i].Verified {
+						account.Identifiers[i].Verified = externalUser.IsEmailVerified
+					}
+
 					updated = true
 				}
 				if hasExternalEmail {

diff --git a/core/auth/auth_type_code.go b/core/auth/auth_type_code.go
@@ -99,14 +99,14 @@ func (a *codeAuthImpl) checkCredentials(identifierImpl identifierType, accountID
 	if identifierChannel.requiresCodeGeneration() {
 		if incomingCode == "" {
 			// generate a new code
-			code := strconv.Itoa(utils.GenerateRandomInt(1000000))
-			padLen := 6 - len(code)
+			incomingCode = strconv.Itoa(utils.GenerateRandomInt(1000000))
+			padLen := 6 - len(incomingCode)
 			if padLen > 0 {
-				code = strings.Repeat("0", padLen) + code
+				incomingCode = strings.Repeat("0", padLen) + incomingCode
 			}
 
 			// store generated codes in login state collection
-			state := map[string]interface{}{stateKeyCode: code}
+			state := map[string]interface{}{stateKeyCode: incomingCode}
 			loginState := model.LoginState{ID: uuid.NewString(), AppID: appOrg.Application.ID, OrgID: appOrg.Organization.ID, AccountID: accountID, State: state, DateCreated: time.Now().UTC()}
 			err := a.auth.storage.InsertLoginState(loginState)
 			if err != nil {
@@ -125,6 +125,11 @@ func (a *codeAuthImpl) checkCredentials(identifierImpl identifierType, accountID
 				return "", "", errors.ErrorData(logutils.StatusInvalid, "code", logutils.StringArgs(*incomingCreds.Code))
 			}
 
+			err = a.auth.storage.DeleteLoginState(nil, loginState.ID)
+			if err != nil {
+				return "", "", errors.WrapErrorAction(logutils.ActionDelete, model.TypeLoginState, nil, err)
+			}
+
 			return "", "", nil
 		}
 	}

diff --git a/core/auth/auth_type_webauthn.go b/core/auth/auth_type_webauthn.go
@@ -431,6 +431,12 @@ func (a *webAuthnAuthImpl) completeRegistration(response *protocol.ParsedCredent
 			return errors.WrapErrorAction(logutils.ActionUpdate, model.TypeAccountAuthType, &logutils.FieldArgs{"id": accountAuthType.ID, "account_id": accountIDVal}, err)
 		}
 
+		//3. remove the login state
+		err = a.auth.storage.DeleteLoginState(context, loginState.ID)
+		if err != nil {
+			return errors.WrapErrorAction(logutils.ActionDelete, model.TypeLoginState, nil, err)
+		}
+
 		return nil
 	}
 
@@ -596,9 +602,25 @@ func (a *webAuthnAuthImpl) completeLogin(response *protocol.ParsedCredentialAsse
 		}
 
 		credential.Value[credentialKeyCredential] = string(credentialData)
-		err = a.auth.storage.UpdateCredentialValue(credID, credential.Value)
+		transaction := func(context storage.TransactionContext) error {
+			//1. update credential
+			err = a.auth.storage.UpdateCredentialValue(context, credID, credential.Value)
+			if err != nil {
+				return errors.WrapErrorAction(logutils.ActionUpdate, model.TypeCredential, nil, err)
+			}
+
+			//2. remove the login state
+			err = a.auth.storage.DeleteLoginState(context, loginState.ID)
+			if err != nil {
+				return errors.WrapErrorAction(logutils.ActionDelete, model.TypeLoginState, nil, err)
+			}
+
+			return nil
+		}
+
+		err = a.auth.storage.PerformTransaction(transaction)
 		if err != nil {
-			return "", errors.WrapErrorAction(logutils.ActionUpdate, model.TypeCredential, nil, err)
+			return "", err
 		}
 	}
 

diff --git a/core/auth/identifier_type_email.go b/core/auth/identifier_type_email.go
@@ -21,7 +21,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/url"
-	"regexp"
 	"strings"
 	"time"
 
@@ -72,8 +71,7 @@ func (a *emailIdentifierImpl) withIdentifier(creds string) (identifierType, erro
 	}
 
 	email := strings.TrimSpace(requestCreds.Email)
-	validEmail := regexp.MustCompile(`^[a-zA-Z0-9.!#\$%&'*+/=?^_{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,253}[a-zA-Z0-9])?(?:.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,253}[a-zA-Z0-9])?)*$`)
-	if !validEmail.MatchString(email) {
+	if !utils.IsValidEmail(email) {
 		return nil, errors.ErrorData(logutils.StatusInvalid, typeEmailIdentifier, &logutils.FieldArgs{"email": email})
 	}
 

diff --git a/core/auth/identifier_type_phone.go b/core/auth/identifier_type_phone.go
@@ -19,7 +19,6 @@ import (
 	"core-building-block/utils"
 	"encoding/json"
 	"net/url"
-	"regexp"
 	"time"
 
 	"github.com/google/uuid"
@@ -68,8 +67,7 @@ func (a *phoneIdentifierImpl) withIdentifier(creds string) (identifierType, erro
 		return nil, errors.WrapErrorAction(logutils.ActionValidate, typePhoneIdentifier, nil, err)
 	}
 
-	validPhone := regexp.MustCompile(`^\+[1-9]\d{1,14}$`)
-	if !validPhone.MatchString(requestCreds.Phone) {
+	if !utils.IsValidPhone(requestCreds.Phone) {
 		return nil, errors.ErrorData(logutils.StatusInvalid, typePhoneNumber, &logutils.FieldArgs{"phone": requestCreds.Phone})
 	}
 

diff --git a/core/auth/interfaces.go b/core/auth/interfaces.go
@@ -554,6 +554,7 @@ type Storage interface {
 	//LoginStates
 	FindLoginState(appID string, orgID string, accountID *string, stateParams map[string]interface{}) (*model.LoginState, error)
 	InsertLoginState(loginState model.LoginState) error
+	DeleteLoginState(context storage.TransactionContext, id string) error
 
 	//Accounts
 	FindAccount(context storage.TransactionContext, appOrgID string, code string, identifier string) (*model.Account, error)
@@ -606,7 +607,7 @@ type Storage interface {
 	FindCredential(context storage.TransactionContext, ID string) (*model.Credential, error)
 	FindCredentials(context storage.TransactionContext, ids []string) ([]model.Credential, error)
 	UpdateCredential(context storage.TransactionContext, creds *model.Credential) error
-	UpdateCredentialValue(ID string, value map[string]interface{}) error
+	UpdateCredentialValue(context storage.TransactionContext, ID string, value map[string]interface{}) error
 	DeleteCredential(context storage.TransactionContext, ID string) error
 
 	//MFA

diff --git a/driven/storage/adapter.go b/driven/storage/adapter.go
@@ -612,6 +612,21 @@ func (sa *Adapter) InsertLoginState(loginState model.LoginState) error {
 	return nil
 }
 
+// DeleteLoginState inserts a new login state
+func (sa *Adapter) DeleteLoginState(context TransactionContext, id string) error {
+	filter := bson.M{"_id": id}
+
+	res, err := sa.db.loginStates.DeleteOneWithContext(context, filter, nil)
+	if err != nil {
+		return errors.WrapErrorAction(logutils.ActionDelete, model.TypeLoginState, &logutils.FieldArgs{"_id": id}, err)
+	}
+	if res.DeletedCount > 1 {
+		return errors.ErrorAction(logutils.ActionDelete, model.TypeLoginState, &logutils.FieldArgs{"_id": id, "deleted": res.DeletedCount, "expected": 1})
+	}
+
+	return nil
+}
+
 // FindAccount finds an account for app, org, auth type and identifier
 func (sa *Adapter) FindAccount(context TransactionContext, appOrgID string, code string, identifier string) (*model.Account, error) {
 	filter := bson.M{"app_org_id": appOrgID, "identifiers": bson.M{
@@ -1961,7 +1976,7 @@ func (sa *Adapter) UpdateCredential(context TransactionContext, creds *model.Cre
 }
 
 // UpdateCredentialValue updates the value in credentials collection
-func (sa *Adapter) UpdateCredentialValue(ID string, value map[string]interface{}) error {
+func (sa *Adapter) UpdateCredentialValue(context TransactionContext, ID string, value map[string]interface{}) error {
 	filter := bson.D{primitive.E{Key: "_id", Value: ID}}
 	update := bson.D{
 		primitive.E{Key: "$set", Value: bson.D{
@@ -1970,7 +1985,7 @@ func (sa *Adapter) UpdateCredentialValue(ID string, value map[string]interface{}
 		}},
 	}
 
-	res, err := sa.db.credentials.UpdateOne(filter, update, nil)
+	res, err := sa.db.credentials.UpdateOneWithContext(context, filter, update, nil)
 	if err != nil {
 		return errors.WrapErrorAction(logutils.ActionUpdate, model.TypeCredential, nil, err)
 	}

diff --git a/driven/storage/migrations.go b/driven/storage/migrations.go
@@ -373,9 +373,14 @@ func (sa *Adapter) migrateAccounts(context TransactionContext, appOrg model.Appl
 			authTypes = append(authTypes, newAat)
 		}
 
+		// if there are no valid auth types then the account is inaccessible, so do not re-insert it
+		if len(authTypes) == 0 {
+			continue
+		}
+
 		now := time.Now().UTC()
-		// add profile email to identifiers if not already there
-		if acct.Profile.Email != nil && *acct.Profile.Email != "" {
+		// add profile email to identifiers if valid and not already there
+		if acct.Profile.Email != nil && utils.IsValidEmail(*acct.Profile.Email) {
 			foundEmail := false
 			for _, identifier := range identifiers {
 				if identifier.Code == "email" && identifier.Identifier == *acct.Profile.Email {
@@ -384,12 +389,11 @@ func (sa *Adapter) migrateAccounts(context TransactionContext, appOrg model.Appl
 				}
 			}
 			if !foundEmail {
-				emailIdentifier := accountIdentifier{ID: uuid.NewString(), Code: "email", Identifier: *acct.Profile.Email, Sensitive: true, DateCreated: now}
-				identifiers = append(identifiers, emailIdentifier)
+				identifiers = append(identifiers, accountIdentifier{ID: uuid.NewString(), Code: "email", Identifier: *acct.Profile.Email, Sensitive: true, DateCreated: now})
 			}
 		}
-		// add profile phone to identifiers if not already there
-		if acct.Profile.Phone != nil && *acct.Profile.Phone != "" {
+		// add profile phone to identifiers if valid and not already there
+		if acct.Profile.Phone != nil && utils.IsValidPhone(*acct.Profile.Phone) {
 			foundPhone := false
 			for _, identifier := range identifiers {
 				if identifier.Code == "phone" && identifier.Identifier == *acct.Profile.Phone {

diff --git a/utils/utils.go b/utils/utils.go
@@ -25,6 +25,7 @@ import (
 	"math/rand"
 	"net/http"
 	"reflect"
+	"regexp"
 	"strings"
 	"time"
 
@@ -181,6 +182,18 @@ func GetLogValue(value string, n int) string {
 	return fmt.Sprintf("***%s", lastN)
 }
 
+// IsValidPhone reports whether phone is a valid phone number
+func IsValidPhone(phone string) bool {
+	validPhone := regexp.MustCompile(`^\+[1-9]\d{1,14}$`)
+	return validPhone.MatchString(phone)
+}
+
+// IsValidEmail reports whether email is a valid email address
+func IsValidEmail(email string) bool {
+	validEmail := regexp.MustCompile(`^[a-zA-Z0-9.!#\$%&'*+/=?^_{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,253}[a-zA-Z0-9])?(?:.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,253}[a-zA-Z0-9])?)*$`)
+	return validEmail.MatchString(email)
+}
+
 // FormatTime formats the time value which this pointer points. Gives empty string if the pointer is nil
 func FormatTime(v *time.Time) string {
 	if v == nil {