Skip to content

Commit

Permalink
fix: minor refactors to use validateFactors
Browse files Browse the repository at this point in the history
  • Loading branch information
J0 committed Oct 3, 2024
1 parent cc7aab0 commit 778cf4b
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 33 deletions.
34 changes: 4 additions & 30 deletions internal/api/mfa.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (w *WebAuthnParams) ToConfig() (*webauthn.WebAuthn, error) {

for _, origin := range origins {
parsedURL, err := url.Parse(origin)
if err != nil || (parsedURL.Scheme != "https" && parsedURL.Scheme != "http") || parsedURL.Host == "" {
if err != nil || (parsedURL.Scheme != "https" && !(parsedURL.Scheme == "http" && parsedURL.Hostname() == "localhost")) || parsedURL.Host == "" {
invalidOrigins = append(invalidOrigins, origin)
} else {
validOrigins = append(validOrigins, origin)
Expand Down Expand Up @@ -228,40 +228,14 @@ func (a *API) enrollPhoneFactor(w http.ResponseWriter, r *http.Request, params *

func (a *API) enrollWebAuthnFactor(w http.ResponseWriter, r *http.Request, params *EnrollFactorParams) error {
ctx := r.Context()
config := a.config
user := getUser(ctx)
session := getSession(ctx)
db := a.db.WithContext(ctx)
factors := user.Factors

factorCount := len(factors)
numVerifiedFactors := 0
if err := models.DeleteExpiredFactors(db, config.MFA.FactorExpiryDuration); err != nil {
if err := validateFactors(db, user, params.FriendlyName, a.config, session); err != nil {
return err
}
for _, factor := range user.Factors {
switch {
case factor.FriendlyName == params.FriendlyName:
return unprocessableEntityError(
ErrorCodeMFAFactorNameConflict,
fmt.Sprintf("A factor with the friendly name %q for this user already exists", factor.FriendlyName),
)
case factor.IsVerified():
numVerifiedFactors++
}
}

if factorCount >= int(config.MFA.MaxEnrolledFactors) {
return unprocessableEntityError(ErrorCodeTooManyEnrolledMFAFactors, "Maximum number of verified factors reached, unenroll to continue")
}

if numVerifiedFactors >= config.MFA.MaxVerifiedFactors {
return unprocessableEntityError(ErrorCodeTooManyEnrolledMFAFactors, "Maximum number of verified factors reached, unenroll to continue")
}

if numVerifiedFactors > 0 && !session.IsAAL2() {
return forbiddenError(ErrorCodeInsufficientAAL, "AAL2 required to enroll a new factor")
}
factor := models.NewWebAuthnFactor(user, params.FriendlyName)
err := db.Transaction(func(tx *storage.Connection) error {
if terr := tx.Create(factor); terr != nil {
Expand Down Expand Up @@ -874,9 +848,9 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param
case params.WebAuthn == nil:
return badRequestError(ErrorCodeValidationFailed, "WebAuthn config required")
case factor.IsVerified() && params.WebAuthn.AssertionResponse == nil:
return badRequestError(ErrorCodeValidationFailed, "WebAuthn Assertion Response required to login")
return badRequestError(ErrorCodeValidationFailed, "creation_response required to login")
case factor.IsUnverified() && params.WebAuthn.CreationResponse == nil:
return badRequestError(ErrorCodeValidationFailed, "WebAuthn Creation Response required to login")
return badRequestError(ErrorCodeValidationFailed, "assertion_response required to login")
default:
webAuthn, err = params.WebAuthn.ToConfig()
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions internal/models/factor.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ type Factor struct {
Phone storage.NullString `json:"phone" db:"phone"`
LastChallengedAt *time.Time `json:"last_challenged_at" db:"last_challenged_at"`
WebAuthnCredential *WebAuthnCredential `json:"-" db:"web_authn_credential"`
AAGUID *uuid.UUID `json:"aaguid" db:"aaguid"`
WebAuthnAAGUID *uuid.UUID `json:"web_authn_aaguid,omitempty" db:"aaguid"`
}

type WebAuthnCredential struct {
Expand Down Expand Up @@ -230,9 +230,9 @@ func (f *Factor) SaveWebAuthnCredential(tx *storage.Connection, credential *weba
if err != nil {
return fmt.Errorf("failed to convert AAGUID to UUID: %w", err)
}
f.AAGUID = &aaguidUUID
f.WebAuthnAAGUID = &aaguidUUID
} else {
f.AAGUID = nil
f.WebAuthnAAGUID = nil
}

return tx.UpdateOnly(f, "web_authn_credential", "aaguid", "updated_at")
Expand Down

0 comments on commit 778cf4b

Please sign in to comment.