From 7f1e097478a9f072feed46144d0fd27300cb2c9c Mon Sep 17 00:00:00 2001 From: fadymak Date: Mon, 20 Apr 2026 13:20:28 +0200 Subject: [PATCH] fix(mfa): source WebAuthn RP config from env vars --- internal/api/mfa.go | 50 ++++++---------------------------- internal/api/mfa_test.go | 12 ++++---- internal/conf/configuration.go | 2 +- openapi.yaml | 26 ------------------ 4 files changed, 17 insertions(+), 73 deletions(-) diff --git a/internal/api/mfa.go b/internal/api/mfa.go index 4b6467eadb..2d9f2fa7b2 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -4,10 +4,8 @@ import ( "bytes" "crypto/subtle" "encoding/json" - "fmt" "net/http" "net/url" - "strings" "time" "github.com/aaronarduino/goqrsvg" @@ -77,8 +75,6 @@ type WebAuthnChallengeData struct { } type WebAuthnParams struct { - RPID string `json:"rpId,omitempty"` - RPOrigins []string `json:"rpOrigins,omitempty"` Type string `json:"type"` // "create" or "request" CredentialResponse json.RawMessage `json:"credential_response"` } @@ -87,39 +83,14 @@ type UnenrollFactorResponse struct { ID uuid.UUID `json:"id"` } -func (w *WebAuthnParams) ToConfig() (*webauthn.WebAuthn, error) { - if w.RPID == "" { - return nil, fmt.Errorf("webAuthn RP ID cannot be empty") - } - - if len(w.RPOrigins) == 0 { - return nil, fmt.Errorf("webAuthn RP Origins cannot be empty") - } - - var validOrigins []string - var invalidOrigins []string - - for _, origin := range w.RPOrigins { - parsedURL, err := url.Parse(origin) - if err != nil || (parsedURL.Scheme != "https" && !(parsedURL.Scheme == "http" && parsedURL.Hostname() == "localhost")) || parsedURL.Host == "" { - invalidOrigins = append(invalidOrigins, origin) - } else { - validOrigins = append(validOrigins, origin) - } - } +func (a *API) getWebAuthnMFA() (*webauthn.WebAuthn, error) { + rpConfig := a.config.WebAuthn - if len(invalidOrigins) > 0 { - return nil, fmt.Errorf("invalid RP origins: %s", strings.Join(invalidOrigins, ", ")) - } - - wconfig := &webauthn.Config{ - // DisplayName is optional in spec but required to be non-empty in libary, we use the RPID as a placeholder. - RPDisplayName: w.RPID, - RPID: w.RPID, - RPOrigins: validOrigins, - } - - return webauthn.New(wconfig) + return webauthn.New(&webauthn.Config{ + RPDisplayName: rpConfig.RPDisplayName, + RPID: rpConfig.RPID, + RPOrigins: rpConfig.RPOrigins, + }) } const ( @@ -500,10 +471,7 @@ func (a *API) challengeWebAuthnFactor(w http.ResponseWriter, r *http.Request) er if err := retrieveRequestParams(r, params); err != nil { return err } - if params.WebAuthn == nil { - return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "web_authn config required") - } - webAuthn, err := params.WebAuthn.ToConfig() + webAuthn, err := a.getWebAuthnMFA() if err != nil { return err } @@ -917,7 +885,7 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param case params.WebAuthn.CredentialResponse == nil: return apierrors.NewBadRequestError(apierrors.ErrorCodeValidationFailed, "credential_response required") default: - webAuthn, err = params.WebAuthn.ToConfig() + webAuthn, err = a.getWebAuthnMFA() if err != nil { return err } diff --git a/internal/api/mfa_test.go b/internal/api/mfa_test.go index 5cc6ae384d..73d26cf5dc 100644 --- a/internal/api/mfa_test.go +++ b/internal/api/mfa_test.go @@ -94,6 +94,12 @@ func (ts *MFATestSuite) SetupTest() { ts.Config.MFA.WebAuthn.EnrollEnabled = true ts.Config.MFA.WebAuthn.VerifyEnabled = true + ts.Config.WebAuthn = conf.WebAuthnConfiguration{ + RPID: "localhost", + RPDisplayName: "Test App", + RPOrigins: []string{"http://localhost:3000"}, + ChallengeExpiryDuration: 5 * time.Minute, + } key, err := totp.Generate(totp.GenerateOpts{ Issuer: ts.TestDomain, @@ -721,13 +727,9 @@ func (ts *MFATestSuite) TestMFAFollowedByPasswordSignIn() { func (ts *MFATestSuite) TestChallengeWebAuthnFactor() { factor := models.NewWebAuthnFactor(ts.TestUser, "WebAuthnfactor") - validWebAuthnConfiguration := &WebAuthnParams{ - RPID: "localhost", - RPOrigins: []string{"http://localhost:3000"}, - } require.NoError(ts.T(), ts.API.db.Create(factor), "Error saving new test factor") token := ts.generateAAL1Token(ts.TestUser, &ts.TestSession.ID) - w := performChallengeWebAuthnFlow(ts, factor.ID, token, validWebAuthnConfiguration) + w := performChallengeWebAuthnFlow(ts, factor.ID, token, &WebAuthnParams{}) require.Equal(ts.T(), http.StatusOK, w.Code) } diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index ad7b59a022..be3c7315c1 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -1333,7 +1333,7 @@ func (c *GlobalConfiguration) Validate() error { } } - if c.Passkey.Enabled { + if c.Passkey.Enabled || c.MFA.WebAuthn.EnrollEnabled || c.MFA.WebAuthn.VerifyEnabled { if err := c.WebAuthn.Validate(); err != nil { return err } diff --git a/openapi.yaml b/openapi.yaml index 6ac282f56f..0c6f22ceff 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1040,21 +1040,6 @@ paths: enum: - sms - whatsapp - webauthn: - type: object - required: - - rpId - - rpOrigins - properties: - rpId: - type: string - description: The relying party identifier (usually the domain) - rpOrigins: - type: array - items: - type: string - minItems: 1 - description: List of allowed origins for WebAuthn responses: 200: @@ -1103,20 +1088,9 @@ paths: webauthn: type: object required: - - rpId - - rpOrigins - type - credential_response properties: - rpId: - type: string - description: The relying party identifier - rpOrigins: - type: array - items: - type: string - minItems: 1 - description: List of allowed origins for WebAuthn type: type: string enum: [create, request]