From 1524bfa7f9a8bea389db7ce93ac3429afe953b3e Mon Sep 17 00:00:00 2001 From: joel Date: Wed, 11 Sep 2024 17:03:05 +0300 Subject: [PATCH 01/13] fix: add initial webauthn configuration --- internal/conf/configuration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index f96f27413..75a351795 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -138,7 +138,7 @@ type MFAConfiguration struct { MaxEnrolledFactors float64 `split_words:"true" default:"10"` MaxVerifiedFactors int `split_words:"true" default:"10"` Phone PhoneFactorTypeConfiguration `split_words:"true"` - TOTP TOTPFactorTypeConfiguration `split_words:"true"` + TOTP MFAFactorTypeConfiguration `split_words:"true" default:"{\"enroll_enabled\":true,\"verify_enabled\":true}"` WebAuthn MFAFactorTypeConfiguration `split_words:"true"` } From acc0025b69602d11e2c9b5aaf57bcad85f797a0f Mon Sep 17 00:00:00 2001 From: joel Date: Wed, 11 Sep 2024 17:21:33 +0300 Subject: [PATCH 02/13] fix: default MFAFactorType to false --- internal/conf/configuration.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 75a351795..f96f27413 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -138,7 +138,7 @@ type MFAConfiguration struct { MaxEnrolledFactors float64 `split_words:"true" default:"10"` MaxVerifiedFactors int `split_words:"true" default:"10"` Phone PhoneFactorTypeConfiguration `split_words:"true"` - TOTP MFAFactorTypeConfiguration `split_words:"true" default:"{\"enroll_enabled\":true,\"verify_enabled\":true}"` + TOTP TOTPFactorTypeConfiguration `split_words:"true"` WebAuthn MFAFactorTypeConfiguration `split_words:"true"` } From 27bf5c29962589b0eb6b945a87d9252b5c717e10 Mon Sep 17 00:00:00 2001 From: joel Date: Thu, 12 Sep 2024 15:58:33 +0300 Subject: [PATCH 03/13] fix: add initial structure --- internal/api/errorcodes.go | 2 ++ internal/api/mfa.go | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/internal/api/errorcodes.go b/internal/api/errorcodes.go index d9a982224..2c85efdec 100644 --- a/internal/api/errorcodes.go +++ b/internal/api/errorcodes.go @@ -82,6 +82,8 @@ const ( ErrorCodeMFAPhoneVerifyDisabled ErrorCode = "mfa_phone_verify_not_enabled" ErrorCodeMFATOTPEnrollDisabled ErrorCode = "mfa_totp_enroll_not_enabled" ErrorCodeMFATOTPVerifyDisabled ErrorCode = "mfa_totp_verify_not_enabled" + ErrorCodeMFAWebAuthnEnrollDisabled ErrorCode = "mfa_webauthn_enroll_not_enabled" + ErrorCodeMFAWebAuthnVerifyDisabled ErrorCode = "mfa_webauthn_verify_not_enabled" ErrorCodeMFAVerifiedFactorExists ErrorCode = "mfa_verified_factor_exists" //#nosec G101 -- Not a secret value. ErrorCodeInvalidCredentials ErrorCode = "invalid_credentials" diff --git a/internal/api/mfa.go b/internal/api/mfa.go index 357eea34c..ab90cd341 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -156,6 +156,9 @@ func (a *API) enrollPhoneFactor(w http.ResponseWriter, r *http.Request, params * }) } +func (a *API) enrollWebAuthnFactor(w http.ResponseWriter, r *http.Request, params *EnrollFactorParams) error { +} + func (a *API) enrollTOTPFactor(w http.ResponseWriter, r *http.Request, params *EnrollFactorParams) error { ctx := r.Context() user := getUser(ctx) @@ -280,6 +283,11 @@ func (a *API) EnrollFactor(w http.ResponseWriter, r *http.Request) error { return unprocessableEntityError(ErrorCodeMFATOTPEnrollDisabled, "MFA enroll is disabled for TOTP") } return a.enrollTOTPFactor(w, r, params) + case models.WebAuthn: + if !config.MFA.WebAuthn.EnrollEnabled { + return unprocessableEntityError(ErrorCodeMFAWebAuthnEnrollDisabled, "MFA enroll is disabled for WebAuthn") + } + return a.enrollWebAuthnFactor(w, r, params) default: return badRequestError(ErrorCodeValidationFailed, "factor_type needs to be totp or phone") } From 928dae2c993ec084e417813f975d97ae83f3f387 Mon Sep 17 00:00:00 2001 From: joel Date: Fri, 13 Sep 2024 08:42:15 +0300 Subject: [PATCH 04/13] feat: add MFA for WebAuthn --- go.mod | 21 +- go.sum | 30 +- internal/api/mfa.go | 327 +++++++++++++++++- internal/api/mfa_test.go | 31 ++ internal/models/amr.go | 2 +- internal/models/challenge.go | 50 ++- internal/models/factor.go | 65 +++- internal/models/user.go | 31 ++ .../20240912193726_add_web_authn.up.sql | 2 + 9 files changed, 505 insertions(+), 54 deletions(-) create mode 100644 migrations/20240912193726_add_web_authn.up.sql diff --git a/go.mod b/go.mod index 4ef25e611..9444e412c 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( github.com/joho/godotenv v1.4.0 github.com/kelseyhightower/envconfig v1.4.0 github.com/microcosm-cc/bluemonday v1.0.26 // indirect - github.com/mitchellh/mapstructure v1.1.2 + github.com/mitchellh/mapstructure v1.5.0 github.com/mrjones/oauth v0.0.0-20190623134757-126b35219450 github.com/pkg/errors v0.9.1 github.com/pquerna/otp v1.4.0 @@ -28,7 +28,7 @@ require ( github.com/sirupsen/logrus v1.9.3 github.com/spf13/cobra v1.6.1 github.com/stretchr/testify v1.9.0 - golang.org/x/crypto v0.24.0 + golang.org/x/crypto v0.26.0 golang.org/x/oauth2 v0.17.0 gopkg.in/gomail.v2 v2.0.0-20160411212932-81ebce5c23df ) @@ -36,10 +36,12 @@ require ( require ( github.com/bits-and-blooms/bitset v1.10.0 // indirect github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0 // indirect - github.com/fsnotify/fsnotify v1.7.0 // indirect + github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/go-jose/go-jose/v3 v3.0.3 // indirect + github.com/go-webauthn/x v0.1.14 // indirect github.com/gobuffalo/nulls v0.4.2 // indirect github.com/goccy/go-json v0.10.3 // indirect + github.com/google/go-tpm v0.9.1 // indirect github.com/jackc/pgx/v4 v4.18.2 // indirect github.com/lestrrat-go/blackmagic v1.0.2 // indirect github.com/lestrrat-go/httpcc v1.0.1 // indirect @@ -47,6 +49,7 @@ require ( github.com/lestrrat-go/iter v1.0.2 // indirect github.com/lestrrat-go/option v1.0.1 // indirect github.com/segmentio/asm v1.2.0 // indirect + github.com/x448/float16 v0.8.4 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect golang.org/x/mod v0.17.0 // indirect @@ -74,7 +77,9 @@ require ( github.com/crewjam/saml v0.4.14 github.com/deepmap/oapi-codegen v1.12.4 github.com/fatih/structs v1.1.0 + github.com/fsnotify/fsnotify v1.7.0 github.com/go-chi/chi/v5 v5.0.12 + github.com/go-webauthn/webauthn v0.11.2 github.com/gobuffalo/pop/v6 v6.1.1 github.com/golang-jwt/jwt/v5 v5.2.1 github.com/lestrrat-go/jwx/v2 v2.1.0 @@ -144,9 +149,9 @@ require ( go.opentelemetry.io/proto/otlp v1.2.0 // indirect golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb golang.org/x/net v0.23.0 // indirect - golang.org/x/sync v0.7.0 // indirect - golang.org/x/sys v0.21.0 // indirect - golang.org/x/text v0.16.0 // indirect + golang.org/x/sync v0.8.0 // indirect + golang.org/x/sys v0.23.0 // indirect + golang.org/x/text v0.17.0 // indirect golang.org/x/time v0.0.0-20220411224347-583f2d630306 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/grpc v1.63.2 // indirect @@ -156,4 +161,6 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect ) -go 1.22.3 +go 1.23 + +toolchain go1.23.0 diff --git a/go.sum b/go.sum index 73f647b56..8ce7bc13a 100644 --- a/go.sum +++ b/go.sum @@ -63,6 +63,8 @@ github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2 github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= +github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv5E= +github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ= github.com/go-chi/chi/v5 v5.0.12 h1:9euLV5sTrTNTRUU9POmDUvfxyj6LAABLUcEWO+JJb4s= github.com/go-chi/chi/v5 v5.0.12/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8= github.com/go-jose/go-jose/v3 v3.0.3 h1:fFKWeig/irsp7XD2zBxvnmA/XaRWp5V3CBsZXJF7G7k= @@ -78,6 +80,10 @@ github.com/go-sql-driver/mysql v1.6.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LB github.com/go-sql-driver/mysql v1.7.0 h1:ueSltNNllEqE3qcWBTD0iQd3IpL/6U+mJxLkazJ7YPc= github.com/go-sql-driver/mysql v1.7.0/go.mod h1:OXbVy3sEdcQ2Doequ6Z5BW6fXNQTmx+9S1MCJN5yJMI= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= +github.com/go-webauthn/webauthn v0.11.2 h1:Fgx0/wlmkClTKlnOsdOQ+K5HcHDsDcYIvtYmfhEOSUc= +github.com/go-webauthn/webauthn v0.11.2/go.mod h1:aOtudaF94pM71g3jRwTYYwQTG1KyTILTcZqN1srkmD0= +github.com/go-webauthn/x v0.1.14 h1:1wrB8jzXAofojJPAaRxnZhRgagvLGnLjhCAwg3kTpT0= +github.com/go-webauthn/x v0.1.14/go.mod h1:UuVvFZ8/NbOnkDz3y1NaxtUN87pmtpC1PQ+/5BBQRdc= github.com/gobuffalo/attrs v1.0.3/go.mod h1:KvDJCE0avbufqS0Bw3UV7RQynESY0jjod+572ctX4t8= github.com/gobuffalo/envy v1.10.2 h1:EIi03p9c3yeuRCFPOKcSfajzkLb3hrRjEpHGI8I2Wo4= github.com/gobuffalo/envy v1.10.2/go.mod h1:qGAGwdvDsaEtPhfBzb3o0SfDea8ByGn9j8bKmVft9z8= @@ -125,6 +131,8 @@ github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-tpm v0.9.1 h1:0pGc4X//bAlmZzMKf8iz6IsDo1nYTbYJ6FZN/rg4zdM= +github.com/google/go-tpm v0.9.1/go.mod h1:h9jEsEECg7gtLis0upRBQU+GhYVH6jMjrFxI8u6bVUY= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= @@ -255,8 +263,8 @@ github.com/mattn/go-sqlite3 v2.0.3+incompatible/go.mod h1:FPy6KqzDD04eiIsT53CuJW github.com/microcosm-cc/bluemonday v1.0.20/go.mod h1:yfBmMi8mxvaZut3Yytv+jTXRY8mxyjJ0/kQBTElld50= github.com/microcosm-cc/bluemonday v1.0.26 h1:xbqSvqzQMeEHCqMi64VAs4d8uy6Mequs3rQ0k/Khz58= github.com/microcosm-cc/bluemonday v1.0.26/go.mod h1:JyzOCs9gkyQyjs+6h10UEVSe02CGwkhd72Xdqh78TWs= -github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQzvN1EDeE= -github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= +github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY= +github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/mrjones/oauth v0.0.0-20190623134757-126b35219450 h1:j2kD3MT1z4PXCiUllUJF9mWUESr9TWKS7iEKsQ/IipM= github.com/mrjones/oauth v0.0.0-20190623134757-126b35219450/go.mod h1:skjdDftzkFALcuGzYSklqYd8gvat6F1gZJ4YPVbkZpM= github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32 h1:W6apQkHrMkS0Muv8G/TipAy/FJl/rCYT0+EuS8+Z0z4= @@ -346,6 +354,8 @@ github.com/supabase/mailme v0.2.0 h1:39LHZ4+YOeqoN4MiuncPBC3JarExAa0flmokM24qHNU github.com/supabase/mailme v0.2.0/go.mod h1:kWsnmPfUBZTavlXYkfJrE9unzmmRAIi/kqsxXfEWEY8= github.com/twmb/murmur3 v1.1.6 h1:mqrRot1BRxm+Yct+vavLMou2/iJt0tNVTTC0QoIjaZg= github.com/twmb/murmur3 v1.1.6/go.mod h1:Qq/R7NUyOfr65zD+6Q5IHKsJLwP7exErjN6lyyq3OSQ= +github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= +github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f h1:J9EGpcZtP0E/raorCMxlFGSTBrsSlaDGf3jU/qvAE2c= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHovont7NscjpAxXsDA8S8BMYve8Y5+7cuRE7R0= @@ -408,8 +418,8 @@ golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= -golang.org/x/crypto v0.24.0 h1:mnl8DM0o513X8fdIkmyFE/5hTYxbwYOjDS/+rK6qpRI= -golang.org/x/crypto v0.24.0/go.mod h1:Z1PMYSOR5nyMcyAVAIQSKCDwalqy85Aqn1x3Ws4L5DM= +golang.org/x/crypto v0.26.0 h1:RrRspgV4mU+YwB4FYnuBoKsUapNIL5cohGAmSH3azsw= +golang.org/x/crypto v0.26.0/go.mod h1:GY7jblb9wI+FOo5y8/S2oY4zWP07AkOJ4+jxCqdqn54= golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb h1:PaBZQdo+iSDyHT053FjUCgZQ/9uqVwPOcl7KSWhKn6w= golang.org/x/exp v0.0.0-20230213192124-5e25df0256eb/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc= golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= @@ -442,8 +452,8 @@ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220929204114-8fcdb60fdcc0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M= -golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ= +golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -468,8 +478,8 @@ golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws= -golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.23.0 h1:YfKFowiIMvtgl1UERQoTPPToxltDeZfbj4H7dVUCwmM= +golang.org/x/sys v0.23.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -487,8 +497,8 @@ golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= -golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= +golang.org/x/text v0.17.0 h1:XtiM5bkSOt+ewxlOE/aE/AKEHibwj/6gvWMl9Rsh0Qc= +golang.org/x/text v0.17.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= golang.org/x/time v0.0.0-20160926182426-711ca1cb8763/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20220411224347-583f2d630306 h1:+gHMid33q6pen7kv9xvT+JRinntgeXO2AeZVd0AWD3w= golang.org/x/time v0.0.0-20220411224347-583f2d630306/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= diff --git a/internal/api/mfa.go b/internal/api/mfa.go index ab90cd341..595dcec62 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -3,14 +3,18 @@ package api import ( "bytes" "crypto/subtle" + "encoding/json" "fmt" "net/http" "net/url" + "strings" "time" "github.com/aaronarduino/goqrsvg" svg "github.com/ajstarks/svgo" "github.com/boombuler/barcode/qr" + wbnprotocol "github.com/go-webauthn/webauthn/protocol" + "github.com/go-webauthn/webauthn/webauthn" "github.com/gofrs/uuid" "github.com/pquerna/otp" "github.com/pquerna/otp/totp" @@ -47,24 +51,36 @@ type EnrollFactorResponse struct { } type ChallengeFactorParams struct { - Channel string `json:"channel"` + Channel string `json:"channel"` + WebAuthn *WebAuthnParams `json:"web_authn,omitempty"` } type VerifyFactorParams struct { - ChallengeID uuid.UUID `json:"challenge_id"` - Code string `json:"code"` + ChallengeID uuid.UUID `json:"challenge_id"` + Code string `json:"code"` + WebAuthn *WebAuthnParams `json:"web_authn,omitempty"` } type ChallengeFactorResponse struct { - ID uuid.UUID `json:"id"` - Type string `json:"type"` - ExpiresAt int64 `json:"expires_at"` + ID uuid.UUID `json:"id"` + Type string `json:"type"` + ExpiresAt int64 `json:"expires_at"` + CredentialRequestOptions *wbnprotocol.CredentialAssertion `json:"credential_request_options"` + CredentialCreationOptions *wbnprotocol.CredentialCreation `json:"credential_creation_options"` } type UnenrollFactorResponse struct { ID uuid.UUID `json:"id"` } +type WebAuthnParams struct { + RPID string `json:"rp_id,omitempty"` + // TODO: reconcile this later + RPOrigins string `json:"rp_origins,omitempty"` + AssertionResponse *wbnprotocol.CredentialAssertionResponse `json:"assertion_response,omitempty"` + CreationResponse *wbnprotocol.CredentialCreationResponse `json:"creation_response,omitempty"` +} + const ( QRCodeGenerationErrorMessage = "Error generating QR Code" ) @@ -157,6 +173,62 @@ 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 { + 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 { + return terr + } + if terr := models.NewAuditLogEntry(r, tx, user, models.EnrollFactorAction, r.RemoteAddr, map[string]interface{}{ + "factor_id": factor.ID, + "factor_type": factor.FactorType, + }); terr != nil { + return terr + } + return nil + }) + if err != nil { + return err + } + return sendJSON(w, http.StatusOK, &EnrollFactorResponse{ + ID: factor.ID, + Type: models.WebAuthn, + FriendlyName: factor.FriendlyName, + }) } func (a *API) enrollTOTPFactor(w http.ResponseWriter, r *http.Request, params *EnrollFactorParams) error { @@ -289,7 +361,7 @@ func (a *API) EnrollFactor(w http.ResponseWriter, r *http.Request) error { } return a.enrollWebAuthnFactor(w, r, params) default: - return badRequestError(ErrorCodeValidationFailed, "factor_type needs to be totp or phone") + return badRequestError(ErrorCodeValidationFailed, "factor_type needs to be totp, phone, or webauthn") } } @@ -410,6 +482,106 @@ func (a *API) challengeTOTPFactor(w http.ResponseWriter, r *http.Request) error }) } +func validateWebAuthnConfig(config *WebAuthnParams) (*webauthn.WebAuthn, error) { + if config.RPID == "" { + return nil, badRequestError(ErrorCodeValidationFailed, "WebAuthn RP ID cannot be empty") + } + + var invalidOrigins []string + + parsedURL, err := url.Parse(config.RPOrigins) + if err != nil || (parsedURL.Scheme != "https" && parsedURL.Scheme != "http") || parsedURL.Host == "" { + return nil, badRequestError(ErrorCodeValidationFailed, fmt.Sprintf("Invalid RP origins: %s", strings.Join(invalidOrigins, ", "))) + } + + // TODO: Find a sensible default and have client library pass it in, or make a case for storing on server + wconfig := &webauthn.Config{ + // Display Name is not optional + RPDisplayName: config.RPID, + RPID: config.RPID, + RPOrigins: []string{config.RPOrigins}, + } + webAuthn, err := webauthn.New(wconfig) + if err != nil { + return nil, badRequestError(ErrorCodeValidationFailed, fmt.Sprintf("invalid WebAuthn configuration: %v", err)) + } + + return webAuthn, nil +} + +func (a *API) challengeWebAuthnFactor(w http.ResponseWriter, r *http.Request) error { + ctx := r.Context() + db := a.db.WithContext(ctx) + config := a.config + + user := getUser(ctx) + factor := getFactor(ctx) + ipAddress := utilities.GetIPAddress(r) + + params := &ChallengeFactorParams{} + if err := retrieveRequestParams(r, params); err != nil { + return err + } + if params.WebAuthn == nil { + return badRequestError(ErrorCodeValidationFailed, "WebAuthn config required") + } + webAuthn, err := validateWebAuthnConfig(params.WebAuthn) + if err != nil { + return err + } + var response *ChallengeFactorResponse + var ws *models.WebAuthnSessionData + var challenge *models.Challenge + if factor.IsUnverified() { + options, session, err := webAuthn.BeginRegistration(user) + if err != nil { + return internalServerError("error generating WebAuthn registration data").WithInternalError(err) + } + ws = &models.WebAuthnSessionData{ + SessionData: session, + } + challenge = ws.ToChallenge(factor.ID, ipAddress) + + response = &ChallengeFactorResponse{ + CredentialCreationOptions: options, + Type: factor.FactorType, + ID: challenge.ID, + } + + } else if factor.IsVerified() { + options, session, err := webAuthn.BeginLogin(user) + if err != nil { + return err + } + ws = &models.WebAuthnSessionData{ + SessionData: session, + } + challenge = ws.ToChallenge(factor.ID, ipAddress) + response = &ChallengeFactorResponse{ + CredentialRequestOptions: options, + Type: factor.FactorType, + ID: challenge.ID, + } + + } + + err = db.Transaction(func(tx *storage.Connection) error { + if terr := factor.WriteChallengeToDatabase(tx, challenge); terr != nil { + return terr + } + return nil + + }) + if err != nil { + return err + } + response.ExpiresAt = challenge.GetExpiryTime(config.MFA.ChallengeExpiryDuration).Unix() + + // TODO: Fetch and return verified factors as well + return sendJSON(w, http.StatusOK, response) + +} + func (a *API) ChallengeFactor(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() config := a.config @@ -418,17 +590,22 @@ func (a *API) ChallengeFactor(w http.ResponseWriter, r *http.Request) error { switch factor.FactorType { case models.Phone: if !config.MFA.Phone.VerifyEnabled { - return unprocessableEntityError(ErrorCodeMFAPhoneEnrollDisabled, "MFA verification is disabled for Phone") + return unprocessableEntityError(ErrorCodeMFAPhoneVerifyDisabled, "MFA verification is disabled for Phone") } return a.challengePhoneFactor(w, r) case models.TOTP: if !config.MFA.TOTP.VerifyEnabled { - return unprocessableEntityError(ErrorCodeMFATOTPEnrollDisabled, "MFA verification is disabled for TOTP") + return unprocessableEntityError(ErrorCodeMFATOTPVerifyDisabled, "MFA verification is disabled for TOTP") } return a.challengeTOTPFactor(w, r) + case models.WebAuthn: + if !config.MFA.WebAuthn.VerifyEnabled { + return unprocessableEntityError(ErrorCodeMFAWebAuthnVerifyDisabled, "MFA verification is disabled for WebAuthn") + } + return a.challengeWebAuthnFactor(w, r) default: - return badRequestError(ErrorCodeValidationFailed, "factor_type needs to be TOTP or Phone") + return badRequestError(ErrorCodeValidationFailed, "factor_type needs to be totp, phone, or webauthn") } } @@ -684,6 +861,123 @@ func (a *API) verifyPhoneFactor(w http.ResponseWriter, r *http.Request, params * return sendJSON(w, http.StatusOK, token) } +func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, params *VerifyFactorParams) error { + ctx := r.Context() + user := getUser(ctx) + factor := getFactor(ctx) + db := a.db.WithContext(ctx) + var webAuthn *webauthn.WebAuthn + var credential *webauthn.Credential + var err error + // TODO: Need to validate that htere's a Challenge ID + + switch { + 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") + case factor.IsUnverified() && params.WebAuthn.CreationResponse == nil: + return badRequestError(ErrorCodeValidationFailed, "WebAuthn Creation Response required to login") + default: + webAuthn, err = validateWebAuthnConfig(params.WebAuthn) + if err != nil { + return err + } + } + + challenge, err := factor.FindChallengeByID(a.db, params.ChallengeID) + if err != nil { + return err + } + // TODO: change so we don't handle pointer + webAuthnSession := *challenge.WebAuthnSessionData.SessionData + + if factor.IsUnverified() { + creationResponseJSON, err := json.Marshal(params.WebAuthn.CreationResponse) + if err != nil { + return badRequestError(ErrorCodeValidationFailed, "Failed to marshal CreationResponse to JSON") + } + creationResponseReader := bytes.NewReader(creationResponseJSON) + parsedResponse, err := wbnprotocol.ParseCredentialCreationResponseBody(creationResponseReader) + if err != nil { + return badRequestError(ErrorCodeValidationFailed, "Invalid credential creation response") + } + + // Destroy right before use + // TODO: Consider wrapping into function + if err := db.Destroy(challenge); err != nil { + return internalServerError("Database error deleting challenge").WithInternalError(err) + } + credential, err = webAuthn.CreateCredential(user, webAuthnSession, parsedResponse) + if err != nil { + return err + } + + } else if factor.IsVerified() { + assertionResponseJSON, err := json.Marshal(params.WebAuthn.AssertionResponse) + if err != nil { + return badRequestError(ErrorCodeValidationFailed, "Failed to marshal AssertionResponse to JSON") + } + assertionResponseReader := bytes.NewReader(assertionResponseJSON) + parsedResponse, err := wbnprotocol.ParseCredentialRequestResponseBody(assertionResponseReader) + if err != nil { + return badRequestError(ErrorCodeValidationFailed, "Invalid credential request response") + } + if err := db.Destroy(challenge); err != nil { + return internalServerError("Database error deleting challenge").WithInternalError(err) + } + credential, err = webAuthn.ValidateLogin(user, webAuthnSession, parsedResponse) + if err != nil { + return internalServerError("error validating WebAuthn credentials") + } + } + var token *AccessTokenResponse + err = db.Transaction(func(tx *storage.Connection) error { + var terr error + if terr = models.NewAuditLogEntry(r, tx, user, models.VerifyFactorAction, r.RemoteAddr, map[string]interface{}{ + "factor_id": factor.ID, + "challenge_id": challenge.ID, + "factor_type": factor.FactorType, + }); terr != nil { + return terr + } + if terr = challenge.Verify(tx); terr != nil { + return terr + } + if !factor.IsVerified() { + if terr = factor.UpdateStatus(tx, models.FactorStateVerified); terr != nil { + return terr + } + if terr = factor.SaveWebAuthnCredential(tx, credential); terr != nil { + return terr + } + } + user, terr = models.FindUserByID(tx, user.ID) + if terr != nil { + return terr + } + token, terr = a.updateMFASessionAndClaims(r, tx, user, models.MFAWebAuthn, models.GrantParams{ + FactorID: &factor.ID, + }) + if terr != nil { + return terr + } + if terr = models.InvalidateSessionsWithAALLessThan(tx, user.ID, models.AAL2.String()); terr != nil { + return internalServerError("Failed to update sessions. %s", terr) + } + if terr = models.DeleteUnverifiedFactors(tx, user, models.WebAuthn); terr != nil { + return internalServerError("Error removing unverified factors. %s", terr) + } + return nil + }) + if err != nil { + return err + } + metering.RecordLogin(string(models.MFACodeLoginAction), user.ID) + + return sendJSON(w, http.StatusOK, token) +} + func (a *API) VerifyFactor(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() factor := getFactor(ctx) @@ -693,24 +987,29 @@ func (a *API) VerifyFactor(w http.ResponseWriter, r *http.Request) error { if err := retrieveRequestParams(r, params); err != nil { return err } - if params.Code == "" { + if params.Code == "" && factor.FactorType != models.WebAuthn { return badRequestError(ErrorCodeValidationFailed, "Code needs to be non-empty") } switch factor.FactorType { case models.Phone: if !config.MFA.Phone.VerifyEnabled { - return unprocessableEntityError(ErrorCodeMFAPhoneEnrollDisabled, "MFA verification is disabled for Phone") + return unprocessableEntityError(ErrorCodeMFAPhoneVerifyDisabled, "MFA verification is disabled for Phone") } return a.verifyPhoneFactor(w, r, params) case models.TOTP: if !config.MFA.TOTP.VerifyEnabled { - return unprocessableEntityError(ErrorCodeMFATOTPEnrollDisabled, "MFA verification is disabled for TOTP") + return unprocessableEntityError(ErrorCodeMFATOTPVerifyDisabled, "MFA verification is disabled for TOTP") } return a.verifyTOTPFactor(w, r, params) + case models.WebAuthn: + if !config.MFA.WebAuthn.VerifyEnabled { + return unprocessableEntityError(ErrorCodeMFAWebAuthnEnrollDisabled, "MFA verification is disabled for WebAuthn") + } + return a.verifyWebAuthnFactor(w, r, params) default: - return badRequestError(ErrorCodeValidationFailed, "factor_type needs to be TOTP or Phone") + return badRequestError(ErrorCodeValidationFailed, "factor_type needs to be totp, phone, or webauthn") } } diff --git a/internal/api/mfa_test.go b/internal/api/mfa_test.go index 557b0ab13..fdd62c5fd 100644 --- a/internal/api/mfa_test.go +++ b/internal/api/mfa_test.go @@ -86,6 +86,9 @@ func (ts *MFATestSuite) SetupTest() { ts.Config.MFA.Phone.EnrollEnabled = true ts.Config.MFA.Phone.VerifyEnabled = true + ts.Config.MFA.WebAuthn.EnrollEnabled = true + ts.Config.MFA.WebAuthn.VerifyEnabled = true + key, err := totp.Generate(totp.GenerateOpts{ Issuer: ts.TestDomain, AccountName: ts.TestEmail, @@ -170,6 +173,12 @@ func (ts *MFATestSuite) TestEnrollFactor() { phone: "", expectedCode: http.StatusBadRequest, }, + { + desc: "WebAuthn: Enroll with friendly name", + friendlyName: "webauthn_factor", + factorType: models.WebAuthn, + expectedCode: http.StatusOK, + }, } for _, c := range cases { ts.Run(c.desc, func() { @@ -706,6 +715,28 @@ func (ts *MFATestSuite) TestMFAFollowedByPasswordSignIn() { require.True(ts.T(), session.IsAAL2()) } +func (ts *MFATestSuite) TestChallengeWebAuthnFactor() { + factor := models.NewWebAuthnFactor(ts.TestUser, "WebAuthnfactor") + validWebAuthnConfiguration := &WebAuthnParams{ + RPDisplayName: "Authapp", + 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) + require.Equal(ts.T(), http.StatusOK, w.Code) +} + +func performChallengeWebAuthnFlow(ts *MFATestSuite, factorID uuid.UUID, token string, webauthn *WebAuthnParams) *httptest.ResponseRecorder { + var buffer bytes.Buffer + err := json.NewEncoder(&buffer).Encode(ChallengeFactorParams{WebAuthn: webauthn}) + require.NoError(ts.T(), err) + w := ServeAuthenticatedRequest(ts, http.MethodPost, fmt.Sprintf("http://localhost/factors/%s/challenge", factorID), token, buffer) + require.Equal(ts.T(), http.StatusOK, w.Code) + return w +} + func (ts *MFATestSuite) TestChallengeFactorNotOwnedByUser() { var buffer bytes.Buffer email := "nomfaenabled@test.com" diff --git a/internal/models/amr.go b/internal/models/amr.go index c527ec86e..fdfd88361 100644 --- a/internal/models/amr.go +++ b/internal/models/amr.go @@ -22,7 +22,7 @@ func (AMRClaim) TableName() string { } func (cl *AMRClaim) IsAAL2Claim() bool { - return *cl.AuthenticationMethod == TOTPSignIn.String() || *cl.AuthenticationMethod == MFAPhone.String() + return *cl.AuthenticationMethod == TOTPSignIn.String() || *cl.AuthenticationMethod == MFAPhone.String() || *cl.AuthenticationMethod == MFAWebAuthn.String() } func AddClaimToSession(tx *storage.Connection, sessionId uuid.UUID, authenticationMethod AuthenticationMethod) error { diff --git a/internal/models/challenge.go b/internal/models/challenge.go index 24d334586..556277e4d 100644 --- a/internal/models/challenge.go +++ b/internal/models/challenge.go @@ -1,6 +1,11 @@ package models import ( + "database/sql/driver" + "fmt" + + "encoding/json" + "github.com/go-webauthn/webauthn/webauthn" "github.com/gofrs/uuid" "github.com/supabase/auth/internal/crypto" "github.com/supabase/auth/internal/storage" @@ -8,13 +13,44 @@ import ( ) type Challenge struct { - ID uuid.UUID `json:"challenge_id" db:"id"` - FactorID uuid.UUID `json:"factor_id" db:"factor_id"` - CreatedAt time.Time `json:"created_at" db:"created_at"` - VerifiedAt *time.Time `json:"verified_at,omitempty" db:"verified_at"` - IPAddress string `json:"ip_address" db:"ip_address"` - Factor *Factor `json:"factor,omitempty" belongs_to:"factor"` - OtpCode string `json:"otp_code,omitempty" db:"otp_code"` + ID uuid.UUID `json:"challenge_id" db:"id"` + FactorID uuid.UUID `json:"factor_id" db:"factor_id"` + CreatedAt time.Time `json:"created_at" db:"created_at"` + VerifiedAt *time.Time `json:"verified_at,omitempty" db:"verified_at"` + IPAddress string `json:"ip_address" db:"ip_address"` + Factor *Factor `json:"factor,omitempty" belongs_to:"factor"` + OtpCode string `json:"otp_code,omitempty" db:"otp_code"` + WebAuthnSessionData *WebAuthnSessionData `json:"web_authn_session_data,omitempty" db:"web_authn_session_data"` +} + +type WebAuthnSessionData struct { + *webauthn.SessionData +} + +func (s *WebAuthnSessionData) Value() (driver.Value, error) { + return json.Marshal(s) +} + +func (s *WebAuthnSessionData) Scan(value interface{}) error { + bytes, ok := value.([]byte) + if !ok { + return fmt.Errorf("expected byte array but got %T", value) + } + + return json.Unmarshal(bytes, s) +} + +func (ws *WebAuthnSessionData) ToChallenge(factorID uuid.UUID, ipAddress string) *Challenge { + id := uuid.Must(uuid.NewV4()) + return &Challenge{ + ID: id, + FactorID: factorID, + IPAddress: ipAddress, + WebAuthnSessionData: &WebAuthnSessionData{ + ws.SessionData, + }, + } + } func (Challenge) TableName() string { diff --git a/internal/models/factor.go b/internal/models/factor.go index 24cda188f..e0149907e 100644 --- a/internal/models/factor.go +++ b/internal/models/factor.go @@ -2,10 +2,13 @@ package models import ( "database/sql" + "database/sql/driver" + "encoding/json" "fmt" "strings" "time" + "github.com/go-webauthn/webauthn/webauthn" "github.com/gobuffalo/pop/v6" "github.com/gofrs/uuid" "github.com/pkg/errors" @@ -32,6 +35,7 @@ func (factorState FactorState) String() string { const TOTP = "totp" const Phone = "phone" +const WebAuthn = "webauthn" type AuthenticationMethod int @@ -41,6 +45,7 @@ const ( OTP TOTPSignIn MFAPhone + MFAWebAuthn SSOSAML Recovery Invite @@ -79,6 +84,8 @@ func (authMethod AuthenticationMethod) String() string { return "anonymous" case MFAPhone: return "mfa/phone" + case MFAWebAuthn: + return "mfa/webauthn" } return "" } @@ -112,6 +119,8 @@ func ParseAuthenticationMethod(authMethod string) (AuthenticationMethod, error) return TokenRefresh, nil case "mfa/sms": return MFAPhone, nil + case "mfa/webauthn": + return MFAWebAuthn, nil } return 0, fmt.Errorf("unsupported authentication method %q", authMethod) } @@ -119,17 +128,35 @@ func ParseAuthenticationMethod(authMethod string) (AuthenticationMethod, error) type Factor struct { ID uuid.UUID `json:"id" db:"id"` // TODO: Consider removing this nested user field. We don't use it. - User User `json:"-" belongs_to:"user"` - UserID uuid.UUID `json:"-" db:"user_id"` - CreatedAt time.Time `json:"created_at" db:"created_at"` - UpdatedAt time.Time `json:"updated_at" db:"updated_at"` - Status string `json:"status" db:"status"` - FriendlyName string `json:"friendly_name,omitempty" db:"friendly_name"` - Secret string `json:"-" db:"secret"` - FactorType string `json:"factor_type" db:"factor_type"` - Challenge []Challenge `json:"-" has_many:"challenges"` - Phone storage.NullString `json:"phone" db:"phone"` - LastChallengedAt *time.Time `json:"last_challenged_at" db:"last_challenged_at"` + User User `json:"-" belongs_to:"user"` + UserID uuid.UUID `json:"-" db:"user_id"` + CreatedAt time.Time `json:"created_at" db:"created_at"` + UpdatedAt time.Time `json:"updated_at" db:"updated_at"` + Status string `json:"status" db:"status"` + FriendlyName string `json:"friendly_name,omitempty" db:"friendly_name"` + Secret string `json:"-" db:"secret"` + FactorType string `json:"factor_type" db:"factor_type"` + Challenge []Challenge `json:"-" has_many:"challenges"` + 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"` +} + +type WebAuthnCredential struct { + webauthn.Credential +} + +func (wc *WebAuthnCredential) Value() (driver.Value, error) { + return json.Marshal(wc) +} + +func (wc *WebAuthnCredential) Scan(value interface{}) error { + bytes, ok := value.([]byte) + if !ok { + return fmt.Errorf("expected byte array but got %T", value) + } + + return json.Unmarshal(bytes, wc) } func (Factor) TableName() string { @@ -160,6 +187,11 @@ func NewPhoneFactor(user *User, phone, friendlyName string) *Factor { return factor } +func NewWebAuthnFactor(user *User, friendlyName string) *Factor { + factor := NewFactor(user, friendlyName, WebAuthn, FactorStateUnverified) + return factor +} + func (f *Factor) SetSecret(secret string, encrypt bool, encryptionKeyID, encryptionKey string) error { f.Secret = secret if encrypt { @@ -187,6 +219,13 @@ func (f *Factor) GetSecret(decryptionKeys map[string]string, encrypt bool, encry return f.Secret, encrypt, nil } +func (f *Factor) SaveWebAuthnCredential(tx *storage.Connection, credential *webauthn.Credential) error { + f.WebAuthnCredential = &WebAuthnCredential{ + Credential: *credential, + } + return tx.UpdateOnly(f, "web_authn_credential", "updated_at") +} + func FindFactorByFactorID(conn *storage.Connection, factorID uuid.UUID) (*Factor, error) { var factor Factor err := conn.Find(&factor, factorID) @@ -269,10 +308,6 @@ func (f *Factor) DowngradeSessionsToAAL1(tx *storage.Connection) error { return updateFactorAssociatedSessions(tx, f.UserID, f.ID, AAL1.String()) } -func (f *Factor) IsOwnedBy(user *User) bool { - return f.UserID == user.ID -} - func (f *Factor) IsVerified() bool { return f.Status == FactorStateVerified.String() } diff --git a/internal/models/user.go b/internal/models/user.go index 520d31fe9..e800fc236 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/go-webauthn/webauthn/webauthn" "github.com/gobuffalo/pop/v6" "github.com/gofrs/uuid" "github.com/pkg/errors" @@ -932,6 +933,36 @@ func (u *User) FindOwnedFactorByID(tx *storage.Connection, factorID uuid.UUID) ( } return &factor, nil } +func (user *User) WebAuthnID() []byte { + // TODO: confirm this user ID + return []byte(user.ID.String()) +} + +func (user *User) WebAuthnName() string { + return string(user.Email) +} + +func (user *User) WebAuthnDisplayName() string { + return string(user.Email) +} + +func (user *User) WebAuthnIcon() string { + // TODO: Find a better default + return "" +} + +func (user *User) WebAuthnCredentials() []webauthn.Credential { + var credentials []webauthn.Credential + + for _, factor := range user.Factors { + if factor.IsVerified() && factor.FactorType == WebAuthn { + credential := factor.WebAuthnCredential.Credential + credentials = append(credentials, credential) + } + } + + return credentials +} func obfuscateValue(id uuid.UUID, value string) string { hash := sha256.Sum256([]byte(id.String() + value)) diff --git a/migrations/20240912193726_add_web_authn.up.sql b/migrations/20240912193726_add_web_authn.up.sql new file mode 100644 index 000000000..c0025c9fd --- /dev/null +++ b/migrations/20240912193726_add_web_authn.up.sql @@ -0,0 +1,2 @@ +alter table {{ index .Options "Namespace" }}.mfa_factors add column if not exists web_authn_credential jsonb null; +alter table {{ index .Options "Namespace" }}.mfa_challenges add column if not exists web_authn_session_data jsonb null; From c5df5d63a2e4eb13736fd9f16261e039a70fc62e Mon Sep 17 00:00:00 2001 From: joel Date: Sat, 28 Sep 2024 10:06:39 +0200 Subject: [PATCH 05/13] fix: temp changes --- internal/api/mfa.go | 41 ++++++++++++++++++++++++++++------------ internal/api/mfa_test.go | 3 +-- internal/models/user.go | 6 ++---- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/internal/api/mfa.go b/internal/api/mfa.go index 595dcec62..b5d1368ca 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -42,6 +42,11 @@ type TOTPObject struct { URI string `json:"uri,omitempty"` } + +type CredentialOptions[T wbnprotocol.CredentialCreation | wbnprotocol.CredentialAssertion] struct { + PublicKey T `json:"publicKey"` +} + type EnrollFactorResponse struct { ID uuid.UUID `json:"id"` Type string `json:"type"` @@ -65,8 +70,7 @@ type ChallengeFactorResponse struct { ID uuid.UUID `json:"id"` Type string `json:"type"` ExpiresAt int64 `json:"expires_at"` - CredentialRequestOptions *wbnprotocol.CredentialAssertion `json:"credential_request_options"` - CredentialCreationOptions *wbnprotocol.CredentialCreation `json:"credential_creation_options"` + Options CredentialOptions[T] `json:"options"` } type UnenrollFactorResponse struct { @@ -75,7 +79,7 @@ type UnenrollFactorResponse struct { type WebAuthnParams struct { RPID string `json:"rp_id,omitempty"` - // TODO: reconcile this later + // TODO: reconcile this later, revert it back to a string array RPOrigins string `json:"rp_origins,omitempty"` AssertionResponse *wbnprotocol.CredentialAssertionResponse `json:"assertion_response,omitempty"` CreationResponse *wbnprotocol.CredentialCreationResponse `json:"creation_response,omitempty"` @@ -543,9 +547,10 @@ func (a *API) challengeWebAuthnFactor(w http.ResponseWriter, r *http.Request) er challenge = ws.ToChallenge(factor.ID, ipAddress) response = &ChallengeFactorResponse{ - CredentialCreationOptions: options, Type: factor.FactorType, ID: challenge.ID, + Options: CredentialOptions[wbnprotocol.PublicKeyCredentialCreationOptions]{PublicKey: options}, + } } else if factor.IsVerified() { @@ -558,9 +563,9 @@ func (a *API) challengeWebAuthnFactor(w http.ResponseWriter, r *http.Request) er } challenge = ws.ToChallenge(factor.ID, ipAddress) response = &ChallengeFactorResponse{ - CredentialRequestOptions: options, Type: factor.FactorType, ID: challenge.ID, + Options: CredentialOptions[wbnprotocol.PublicKeyCredentialRequestOptions]{PublicKey: options}, } } @@ -577,7 +582,6 @@ func (a *API) challengeWebAuthnFactor(w http.ResponseWriter, r *http.Request) er } response.ExpiresAt = challenge.GetExpiryTime(config.MFA.ChallengeExpiryDuration).Unix() - // TODO: Fetch and return verified factors as well return sendJSON(w, http.StatusOK, response) } @@ -869,7 +873,7 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param var webAuthn *webauthn.WebAuthn var credential *webauthn.Credential var err error - // TODO: Need to validate that htere's a Challenge ID + switch { case params.WebAuthn == nil: @@ -885,10 +889,25 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param } } - challenge, err := factor.FindChallengeByID(a.db, params.ChallengeID) - if err != nil { - return err + challenge, err := factor.FindChallengeByID(db, params.ChallengeID) + if err != nil && models.IsNotFoundError(err) { + return notFoundError(ErrorCodeMFAFactorNotFound, "MFA factor with the provided challenge ID not found") + } else if err != nil { + return internalServerError("Database error finding Challenge").WithInternalError(err) } + + // Ambiguous so as not to leak whether there is a verified challenge + if challenge.VerifiedAt != nil || challenge.IPAddress != currentIP { + return unprocessableEntityError(ErrorCodeMFAIPAddressMismatch, "Challenge and verify IP addresses mismatch") + } + + if challenge.HasExpired(config.MFA.ChallengeExpiryDuration) { + if err := db.Destroy(challenge); err != nil { + return internalServerError("Database error deleting challenge").WithInternalError(err) + } + return unprocessableEntityError(ErrorCodeMFAChallengeExpired, "MFA challenge %v has expired, verify against another challenge or create a new challenge.", challenge.ID) + } + // TODO: change so we don't handle pointer webAuthnSession := *challenge.WebAuthnSessionData.SessionData @@ -903,8 +922,6 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param return badRequestError(ErrorCodeValidationFailed, "Invalid credential creation response") } - // Destroy right before use - // TODO: Consider wrapping into function if err := db.Destroy(challenge); err != nil { return internalServerError("Database error deleting challenge").WithInternalError(err) } diff --git a/internal/api/mfa_test.go b/internal/api/mfa_test.go index fdd62c5fd..bba3bf04f 100644 --- a/internal/api/mfa_test.go +++ b/internal/api/mfa_test.go @@ -718,9 +718,8 @@ func (ts *MFATestSuite) TestMFAFollowedByPasswordSignIn() { func (ts *MFATestSuite) TestChallengeWebAuthnFactor() { factor := models.NewWebAuthnFactor(ts.TestUser, "WebAuthnfactor") validWebAuthnConfiguration := &WebAuthnParams{ - RPDisplayName: "Authapp", RPID: "localhost", - RPOrigins: []string{"http://localhost:3000"}, + RPOrigins: "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) diff --git a/internal/models/user.go b/internal/models/user.go index e800fc236..cdcede319 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -939,17 +939,15 @@ func (user *User) WebAuthnID() []byte { } func (user *User) WebAuthnName() string { + // TODO: Allow for overrides on this return string(user.Email) } func (user *User) WebAuthnDisplayName() string { + // TODO: return string(user.Email) } -func (user *User) WebAuthnIcon() string { - // TODO: Find a better default - return "" -} func (user *User) WebAuthnCredentials() []webauthn.Credential { var credentials []webauthn.Credential From bee17da656d83358b23bd53f46248983e8126597 Mon Sep 17 00:00:00 2001 From: joel Date: Sat, 28 Sep 2024 10:20:56 +0200 Subject: [PATCH 06/13] fix: put challenge into function --- internal/api/mfa.go | 108 ++++++++++++++++----------------------- internal/api/mfa_test.go | 4 +- internal/models/user.go | 1 - 3 files changed, 46 insertions(+), 67 deletions(-) diff --git a/internal/api/mfa.go b/internal/api/mfa.go index b5d1368ca..d29abe40d 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -42,11 +42,6 @@ type TOTPObject struct { URI string `json:"uri,omitempty"` } - -type CredentialOptions[T wbnprotocol.CredentialCreation | wbnprotocol.CredentialAssertion] struct { - PublicKey T `json:"publicKey"` -} - type EnrollFactorResponse struct { ID uuid.UUID `json:"id"` Type string `json:"type"` @@ -70,7 +65,8 @@ type ChallengeFactorResponse struct { ID uuid.UUID `json:"id"` Type string `json:"type"` ExpiresAt int64 `json:"expires_at"` - Options CredentialOptions[T] `json:"options"` + CredentialRequestOptions *wbnprotocol.CredentialAssertion `json:"credential_request_options"` + CredentialCreationOptions *wbnprotocol.CredentialCreation `json:"credential_creation_options"` } type UnenrollFactorResponse struct { @@ -79,7 +75,7 @@ type UnenrollFactorResponse struct { type WebAuthnParams struct { RPID string `json:"rp_id,omitempty"` - // TODO: reconcile this later, revert it back to a string array + // TODO: reconcile this later RPOrigins string `json:"rp_origins,omitempty"` AssertionResponse *wbnprotocol.CredentialAssertionResponse `json:"assertion_response,omitempty"` CreationResponse *wbnprotocol.CredentialCreationResponse `json:"creation_response,omitempty"` @@ -547,10 +543,9 @@ func (a *API) challengeWebAuthnFactor(w http.ResponseWriter, r *http.Request) er challenge = ws.ToChallenge(factor.ID, ipAddress) response = &ChallengeFactorResponse{ + CredentialCreationOptions: options, Type: factor.FactorType, ID: challenge.ID, - Options: CredentialOptions[wbnprotocol.PublicKeyCredentialCreationOptions]{PublicKey: options}, - } } else if factor.IsVerified() { @@ -563,9 +558,9 @@ func (a *API) challengeWebAuthnFactor(w http.ResponseWriter, r *http.Request) er } challenge = ws.ToChallenge(factor.ID, ipAddress) response = &ChallengeFactorResponse{ + CredentialRequestOptions: options, Type: factor.FactorType, ID: challenge.ID, - Options: CredentialOptions[wbnprotocol.PublicKeyCredentialRequestOptions]{PublicKey: options}, } } @@ -586,6 +581,32 @@ func (a *API) challengeWebAuthnFactor(w http.ResponseWriter, r *http.Request) er } +func (a *API) validateChallenge(r *http.Request, db *storage.Connection, factor *models.Factor, challengeID uuid.UUID) (*models.Challenge, error) { + config := a.config + currentIP := utilities.GetIPAddress(r) + + challenge, err := factor.FindChallengeByID(db, challengeID) + if err != nil { + if models.IsNotFoundError(err) { + return nil, notFoundError(ErrorCodeMFAFactorNotFound, "MFA factor with the provided challenge ID not found") + } + return nil, internalServerError("Database error finding Challenge").WithInternalError(err) + } + + if challenge.VerifiedAt != nil || challenge.IPAddress != currentIP { + return nil, unprocessableEntityError(ErrorCodeMFAIPAddressMismatch, "Challenge and verify IP addresses mismatch") + } + + if challenge.HasExpired(config.MFA.ChallengeExpiryDuration) { + if err := db.Destroy(challenge); err != nil { + return nil, internalServerError("Database error deleting challenge").WithInternalError(err) + } + return nil, unprocessableEntityError(ErrorCodeMFAChallengeExpired, "MFA challenge %v has expired, verify against another challenge or create a new challenge.", challenge.ID) + } + + return challenge, nil +} + func (a *API) ChallengeFactor(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() config := a.config @@ -621,25 +642,10 @@ func (a *API) verifyTOTPFactor(w http.ResponseWriter, r *http.Request, params *V factor := getFactor(ctx) config := a.config db := a.db.WithContext(ctx) - currentIP := utilities.GetIPAddress(r) - challenge, err := factor.FindChallengeByID(db, params.ChallengeID) - if err != nil && models.IsNotFoundError(err) { - return notFoundError(ErrorCodeMFAFactorNotFound, "MFA factor with the provided challenge ID not found") - } else if err != nil { - return internalServerError("Database error finding Challenge").WithInternalError(err) - } - - // Ambiguous so as not to leak whether there is a verified challenge - if challenge.VerifiedAt != nil || challenge.IPAddress != currentIP { - return unprocessableEntityError(ErrorCodeMFAIPAddressMismatch, "Challenge and verify IP addresses mismatch") - } - - if challenge.HasExpired(config.MFA.ChallengeExpiryDuration) { - if err := db.Destroy(challenge); err != nil { - return internalServerError("Database error deleting challenge").WithInternalError(err) - } - return unprocessableEntityError(ErrorCodeMFAChallengeExpired, "MFA challenge %v has expired, verify against another challenge or create a new challenge.", challenge.ID) + challenge, err := a.validateChallenge(r, db, factor, params.ChallengeID) + if err != nil { + return err } secret, shouldReEncrypt, err := factor.GetSecret(config.Security.DBEncryption.DecryptionKeys, config.Security.DBEncryption.Encrypt, config.Security.DBEncryption.EncryptionKeyID) @@ -756,25 +762,12 @@ func (a *API) verifyPhoneFactor(w http.ResponseWriter, r *http.Request, params * user := getUser(ctx) factor := getFactor(ctx) db := a.db.WithContext(ctx) - currentIP := utilities.GetIPAddress(r) - challenge, err := factor.FindChallengeByID(db, params.ChallengeID) - if err != nil && models.IsNotFoundError(err) { - return notFoundError(ErrorCodeMFAFactorNotFound, "MFA factor with the provided challenge ID not found") - } else if err != nil { - return internalServerError("Database error finding Challenge").WithInternalError(err) - } - - if challenge.VerifiedAt != nil || challenge.IPAddress != currentIP { - return unprocessableEntityError(ErrorCodeMFAIPAddressMismatch, "Challenge and verify IP addresses mismatch") + challenge, err := a.validateChallenge(r, db, factor, params.ChallengeID) + if err != nil { + return err } - if challenge.HasExpired(config.MFA.ChallengeExpiryDuration) { - if err := db.Destroy(challenge); err != nil { - return internalServerError("Database error deleting challenge").WithInternalError(err) - } - return unprocessableEntityError(ErrorCodeMFAChallengeExpired, "MFA challenge %v has expired, verify against another challenge or create a new challenge.", challenge.ID) - } otpCode, shouldReEncrypt, err := challenge.GetOtpCode(config.Security.DBEncryption.DecryptionKeys, config.Security.DBEncryption.Encrypt, config.Security.DBEncryption.EncryptionKeyID) if err != nil { return internalServerError("Database error verifying MFA TOTP secret").WithInternalError(err) @@ -870,10 +863,14 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param user := getUser(ctx) factor := getFactor(ctx) db := a.db.WithContext(ctx) + var webAuthn *webauthn.WebAuthn var credential *webauthn.Credential - var err error + challenge, err := a.validateChallenge(r, db, factor, params.ChallengeID) + if err != nil { + return err + } switch { case params.WebAuthn == nil: @@ -889,25 +886,6 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param } } - challenge, err := factor.FindChallengeByID(db, params.ChallengeID) - if err != nil && models.IsNotFoundError(err) { - return notFoundError(ErrorCodeMFAFactorNotFound, "MFA factor with the provided challenge ID not found") - } else if err != nil { - return internalServerError("Database error finding Challenge").WithInternalError(err) - } - - // Ambiguous so as not to leak whether there is a verified challenge - if challenge.VerifiedAt != nil || challenge.IPAddress != currentIP { - return unprocessableEntityError(ErrorCodeMFAIPAddressMismatch, "Challenge and verify IP addresses mismatch") - } - - if challenge.HasExpired(config.MFA.ChallengeExpiryDuration) { - if err := db.Destroy(challenge); err != nil { - return internalServerError("Database error deleting challenge").WithInternalError(err) - } - return unprocessableEntityError(ErrorCodeMFAChallengeExpired, "MFA challenge %v has expired, verify against another challenge or create a new challenge.", challenge.ID) - } - // TODO: change so we don't handle pointer webAuthnSession := *challenge.WebAuthnSessionData.SessionData @@ -922,6 +900,8 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param return badRequestError(ErrorCodeValidationFailed, "Invalid credential creation response") } + // Destroy right before use + // TODO: Consider wrapping into function if err := db.Destroy(challenge); err != nil { return internalServerError("Database error deleting challenge").WithInternalError(err) } diff --git a/internal/api/mfa_test.go b/internal/api/mfa_test.go index bba3bf04f..ceabc37be 100644 --- a/internal/api/mfa_test.go +++ b/internal/api/mfa_test.go @@ -718,8 +718,8 @@ func (ts *MFATestSuite) TestMFAFollowedByPasswordSignIn() { func (ts *MFATestSuite) TestChallengeWebAuthnFactor() { factor := models.NewWebAuthnFactor(ts.TestUser, "WebAuthnfactor") validWebAuthnConfiguration := &WebAuthnParams{ - RPID: "localhost", - RPOrigins: "http://localhost:3000", + RPID: "localhost", + RPOrigins: "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) diff --git a/internal/models/user.go b/internal/models/user.go index cdcede319..a289977b0 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -948,7 +948,6 @@ func (user *User) WebAuthnDisplayName() string { return string(user.Email) } - func (user *User) WebAuthnCredentials() []webauthn.Credential { var credentials []webauthn.Credential From cb1ac2a6f6a390b2985650f1ae8a9653f553567c Mon Sep 17 00:00:00 2001 From: joel Date: Sat, 28 Sep 2024 11:30:40 +0200 Subject: [PATCH 07/13] fix: add option to override on user --- internal/api/mfa.go | 49 ++++++++++++++++++++++++++++++++++++++--- internal/models/user.go | 17 +++++++++----- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/internal/api/mfa.go b/internal/api/mfa.go index d29abe40d..e295895c3 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -75,12 +75,55 @@ type UnenrollFactorResponse struct { type WebAuthnParams struct { RPID string `json:"rp_id,omitempty"` - // TODO: reconcile this later + // Can encode multiple origins as comma separated values like: "origin1,origin2" RPOrigins string `json:"rp_origins,omitempty"` AssertionResponse *wbnprotocol.CredentialAssertionResponse `json:"assertion_response,omitempty"` CreationResponse *wbnprotocol.CredentialCreationResponse `json:"creation_response,omitempty"` } +func (w *WebAuthnParams) GetRPOrigins() []string { + if w.RPOrigins == "" { + return nil + } + return strings.Split(w.RPOrigins, ",") +} + +func (w *WebAuthnParams) ToConfig() (*webauthn.WebAuthn, error) { + if w.RPID == "" { + return nil, fmt.Errorf("WebAuthn RP ID cannot be empty") + } + + origins := w.GetRPOrigins() + if len(origins) == 0 { + return nil, fmt.Errorf("WebAuthn RP Origins cannot be empty") + } + + var validOrigins []string + var invalidOrigins []string + + for _, origin := range origins { + parsedURL, err := url.Parse(origin) + if err != nil || (parsedURL.Scheme != "https" && parsedURL.Scheme != "http") || parsedURL.Host == "" { + invalidOrigins = append(invalidOrigins, origin) + } else { + validOrigins = append(validOrigins, origin) + } + } + + if len(invalidOrigins) > 0 { + return nil, fmt.Errorf("Invalid RP origins: %s", strings.Join(invalidOrigins, ", ")) + } + + // TODO: Find a more sensible default + wconfig := &webauthn.Config{ + RPDisplayName: w.RPID, + RPID: w.RPID, + RPOrigins: validOrigins, + } + + return webauthn.New(wconfig) +} + const ( QRCodeGenerationErrorMessage = "Error generating QR Code" ) @@ -525,7 +568,7 @@ func (a *API) challengeWebAuthnFactor(w http.ResponseWriter, r *http.Request) er if params.WebAuthn == nil { return badRequestError(ErrorCodeValidationFailed, "WebAuthn config required") } - webAuthn, err := validateWebAuthnConfig(params.WebAuthn) + webAuthn, err := params.WebAuthn.ToConfig() if err != nil { return err } @@ -880,7 +923,7 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param case factor.IsUnverified() && params.WebAuthn.CreationResponse == nil: return badRequestError(ErrorCodeValidationFailed, "WebAuthn Creation Response required to login") default: - webAuthn, err = validateWebAuthnConfig(params.WebAuthn) + webAuthn, err = params.WebAuthn.ToConfig() if err != nil { return err } diff --git a/internal/models/user.go b/internal/models/user.go index a289977b0..1abee0c34 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -933,19 +933,26 @@ func (u *User) FindOwnedFactorByID(tx *storage.Connection, factorID uuid.UUID) ( } return &factor, nil } + func (user *User) WebAuthnID() []byte { - // TODO: confirm this user ID + if webAuthnID, ok := user.UserMetaData["web_authn_id"].(string); ok && webAuthnID != "" { + return []byte(webAuthnID) + } return []byte(user.ID.String()) } func (user *User) WebAuthnName() string { - // TODO: Allow for overrides on this - return string(user.Email) + if webAuthnName, ok := user.UserMetaData["web_authn_name"].(string); ok && webAuthnName != "" { + return webAuthnName + } + return user.Email.String() } func (user *User) WebAuthnDisplayName() string { - // TODO: - return string(user.Email) + if webAuthnDisplayName, ok := user.UserMetaData["web_authn_display_name"].(string); ok && webAuthnDisplayName != "" { + return webAuthnDisplayName + } + return user.Email.String() } func (user *User) WebAuthnCredentials() []webauthn.Credential { From 8cc0affe58c3a31043034bab25d960cdd657c7e5 Mon Sep 17 00:00:00 2001 From: joel Date: Sat, 28 Sep 2024 11:56:42 +0200 Subject: [PATCH 08/13] fix: readjust challenge logic --- internal/api/mfa.go | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/internal/api/mfa.go b/internal/api/mfa.go index e295895c3..5caf780b1 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -910,11 +910,6 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param var webAuthn *webauthn.WebAuthn var credential *webauthn.Credential - challenge, err := a.validateChallenge(r, db, factor, params.ChallengeID) - if err != nil { - return err - } - switch { case params.WebAuthn == nil: return badRequestError(ErrorCodeValidationFailed, "WebAuthn config required") @@ -929,8 +924,15 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param } } - // TODO: change so we don't handle pointer + challenge, err := a.validateChallenge(r, db, factor, params.ChallengeID) + if err != nil { + return err + } webAuthnSession := *challenge.WebAuthnSessionData.SessionData + // Once the challenge is validated, we consume the challenge + if err := db.Destroy(challenge); err != nil { + return internalServerError("Database error deleting challenge").WithInternalError(err) + } if factor.IsUnverified() { creationResponseJSON, err := json.Marshal(params.WebAuthn.CreationResponse) @@ -942,12 +944,6 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param if err != nil { return badRequestError(ErrorCodeValidationFailed, "Invalid credential creation response") } - - // Destroy right before use - // TODO: Consider wrapping into function - if err := db.Destroy(challenge); err != nil { - return internalServerError("Database error deleting challenge").WithInternalError(err) - } credential, err = webAuthn.CreateCredential(user, webAuthnSession, parsedResponse) if err != nil { return err @@ -963,9 +959,6 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param if err != nil { return badRequestError(ErrorCodeValidationFailed, "Invalid credential request response") } - if err := db.Destroy(challenge); err != nil { - return internalServerError("Database error deleting challenge").WithInternalError(err) - } credential, err = webAuthn.ValidateLogin(user, webAuthnSession, parsedResponse) if err != nil { return internalServerError("error validating WebAuthn credentials") @@ -981,9 +974,7 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param }); terr != nil { return terr } - if terr = challenge.Verify(tx); terr != nil { - return terr - } + // Challenge verification not needed as the challenge is destroyed on use if !factor.IsVerified() { if terr = factor.UpdateStatus(tx, models.FactorStateVerified); terr != nil { return terr From f0c8e124f323126e23c85baf9bcc67cfcff48391 Mon Sep 17 00:00:00 2001 From: joel Date: Mon, 30 Sep 2024 07:36:54 +0200 Subject: [PATCH 09/13] fix: change the version --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2ad87dee4..3d3e3762a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -76,7 +76,7 @@ jobs: - uses: actions/setup-go@v3 if: ${{ steps.release.outputs.release_created == 'true' || steps.release.outputs.prs_created == 'true' }} with: - go-version: "^1.21.0" # The Go version to download (if necessary) and use. + go-version: "^1.22.0" # The Go version to download (if necessary) and use. - name: Build release artifacts if: ${{ steps.release.outputs.release_created == 'true' || steps.release.outputs.prs_created == 'true' }} From 03708bcca1c40831999c1aa1c9c30d04bbdb1e5f Mon Sep 17 00:00:00 2001 From: joel Date: Mon, 30 Sep 2024 09:01:31 +0200 Subject: [PATCH 10/13] fix: remove default overrides --- internal/api/mfa.go | 1 + internal/models/user.go | 9 --------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/internal/api/mfa.go b/internal/api/mfa.go index 5caf780b1..fdfd19841 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -909,6 +909,7 @@ func (a *API) verifyWebAuthnFactor(w http.ResponseWriter, r *http.Request, param var webAuthn *webauthn.WebAuthn var credential *webauthn.Credential + var err error switch { case params.WebAuthn == nil: diff --git a/internal/models/user.go b/internal/models/user.go index 1abee0c34..228e6e962 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -935,23 +935,14 @@ func (u *User) FindOwnedFactorByID(tx *storage.Connection, factorID uuid.UUID) ( } func (user *User) WebAuthnID() []byte { - if webAuthnID, ok := user.UserMetaData["web_authn_id"].(string); ok && webAuthnID != "" { - return []byte(webAuthnID) - } return []byte(user.ID.String()) } func (user *User) WebAuthnName() string { - if webAuthnName, ok := user.UserMetaData["web_authn_name"].(string); ok && webAuthnName != "" { - return webAuthnName - } return user.Email.String() } func (user *User) WebAuthnDisplayName() string { - if webAuthnDisplayName, ok := user.UserMetaData["web_authn_display_name"].(string); ok && webAuthnDisplayName != "" { - return webAuthnDisplayName - } return user.Email.String() } From c2b4a8ec6d7383bae5e1f3ee712219e963c5f031 Mon Sep 17 00:00:00 2001 From: joel Date: Mon, 30 Sep 2024 10:25:41 +0200 Subject: [PATCH 11/13] fix: revert to go v1.22 --- go.mod | 8 +++----- go.sum | 8 ++++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 9444e412c..888f5bb2c 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,7 @@ require ( github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0 // indirect github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/go-jose/go-jose/v3 v3.0.3 // indirect - github.com/go-webauthn/x v0.1.14 // indirect + github.com/go-webauthn/x v0.1.12 // indirect github.com/gobuffalo/nulls v0.4.2 // indirect github.com/goccy/go-json v0.10.3 // indirect github.com/google/go-tpm v0.9.1 // indirect @@ -79,7 +79,7 @@ require ( github.com/fatih/structs v1.1.0 github.com/fsnotify/fsnotify v1.7.0 github.com/go-chi/chi/v5 v5.0.12 - github.com/go-webauthn/webauthn v0.11.2 + github.com/go-webauthn/webauthn v0.11.1 github.com/gobuffalo/pop/v6 v6.1.1 github.com/golang-jwt/jwt/v5 v5.2.1 github.com/lestrrat-go/jwx/v2 v2.1.0 @@ -161,6 +161,4 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect ) -go 1.23 - -toolchain go1.23.0 +go 1.22.3 diff --git a/go.sum b/go.sum index 8ce7bc13a..2a07c7ffe 100644 --- a/go.sum +++ b/go.sum @@ -80,10 +80,10 @@ github.com/go-sql-driver/mysql v1.6.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LB github.com/go-sql-driver/mysql v1.7.0 h1:ueSltNNllEqE3qcWBTD0iQd3IpL/6U+mJxLkazJ7YPc= github.com/go-sql-driver/mysql v1.7.0/go.mod h1:OXbVy3sEdcQ2Doequ6Z5BW6fXNQTmx+9S1MCJN5yJMI= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= -github.com/go-webauthn/webauthn v0.11.2 h1:Fgx0/wlmkClTKlnOsdOQ+K5HcHDsDcYIvtYmfhEOSUc= -github.com/go-webauthn/webauthn v0.11.2/go.mod h1:aOtudaF94pM71g3jRwTYYwQTG1KyTILTcZqN1srkmD0= -github.com/go-webauthn/x v0.1.14 h1:1wrB8jzXAofojJPAaRxnZhRgagvLGnLjhCAwg3kTpT0= -github.com/go-webauthn/x v0.1.14/go.mod h1:UuVvFZ8/NbOnkDz3y1NaxtUN87pmtpC1PQ+/5BBQRdc= +github.com/go-webauthn/webauthn v0.11.1 h1:5G/+dg91/VcaJHTtJUfwIlNJkLwbJCcnUc4W8VtkpzA= +github.com/go-webauthn/webauthn v0.11.1/go.mod h1:YXRm1WG0OtUyDFaVAgB5KG7kVqW+6dYCJ7FTQH4SxEE= +github.com/go-webauthn/x v0.1.12 h1:RjQ5cvApzyU/xLCiP+rub0PE4HBZsLggbxGR5ZpUf/A= +github.com/go-webauthn/x v0.1.12/go.mod h1:XlRcGkNH8PT45TfeJYc6gqpOtiOendHhVmnOxh+5yHs= github.com/gobuffalo/attrs v1.0.3/go.mod h1:KvDJCE0avbufqS0Bw3UV7RQynESY0jjod+572ctX4t8= github.com/gobuffalo/envy v1.10.2 h1:EIi03p9c3yeuRCFPOKcSfajzkLb3hrRjEpHGI8I2Wo4= github.com/gobuffalo/envy v1.10.2/go.mod h1:qGAGwdvDsaEtPhfBzb3o0SfDea8ByGn9j8bKmVft9z8= From 27e3470abdc9c7184a8a6cf8466ba95befdb45e5 Mon Sep 17 00:00:00 2001 From: joel Date: Mon, 30 Sep 2024 10:34:42 +0200 Subject: [PATCH 12/13] fix: update error messages --- internal/api/mfa.go | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/internal/api/mfa.go b/internal/api/mfa.go index fdfd19841..068c5d5eb 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -90,12 +90,12 @@ func (w *WebAuthnParams) GetRPOrigins() []string { func (w *WebAuthnParams) ToConfig() (*webauthn.WebAuthn, error) { if w.RPID == "" { - return nil, fmt.Errorf("WebAuthn RP ID cannot be empty") + return nil, fmt.Errorf("webAuthn RP ID cannot be empty") } origins := w.GetRPOrigins() if len(origins) == 0 { - return nil, fmt.Errorf("WebAuthn RP Origins cannot be empty") + return nil, fmt.Errorf("webAuthn RP Origins cannot be empty") } var validOrigins []string @@ -111,7 +111,7 @@ func (w *WebAuthnParams) ToConfig() (*webauthn.WebAuthn, error) { } if len(invalidOrigins) > 0 { - return nil, fmt.Errorf("Invalid RP origins: %s", strings.Join(invalidOrigins, ", ")) + return nil, fmt.Errorf("invalid RP origins: %s", strings.Join(invalidOrigins, ", ")) } // TODO: Find a more sensible default @@ -525,33 +525,6 @@ func (a *API) challengeTOTPFactor(w http.ResponseWriter, r *http.Request) error }) } -func validateWebAuthnConfig(config *WebAuthnParams) (*webauthn.WebAuthn, error) { - if config.RPID == "" { - return nil, badRequestError(ErrorCodeValidationFailed, "WebAuthn RP ID cannot be empty") - } - - var invalidOrigins []string - - parsedURL, err := url.Parse(config.RPOrigins) - if err != nil || (parsedURL.Scheme != "https" && parsedURL.Scheme != "http") || parsedURL.Host == "" { - return nil, badRequestError(ErrorCodeValidationFailed, fmt.Sprintf("Invalid RP origins: %s", strings.Join(invalidOrigins, ", "))) - } - - // TODO: Find a sensible default and have client library pass it in, or make a case for storing on server - wconfig := &webauthn.Config{ - // Display Name is not optional - RPDisplayName: config.RPID, - RPID: config.RPID, - RPOrigins: []string{config.RPOrigins}, - } - webAuthn, err := webauthn.New(wconfig) - if err != nil { - return nil, badRequestError(ErrorCodeValidationFailed, fmt.Sprintf("invalid WebAuthn configuration: %v", err)) - } - - return webAuthn, nil -} - func (a *API) challengeWebAuthnFactor(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() db := a.db.WithContext(ctx) From 9d3d4125dfe6788287d8acc8c4d76e17bad6fa23 Mon Sep 17 00:00:00 2001 From: joel Date: Mon, 30 Sep 2024 12:36:33 +0200 Subject: [PATCH 13/13] fix: update openapi for challenge --- openapi.yaml | 191 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 176 insertions(+), 15 deletions(-) diff --git a/openapi.yaml b/openapi.yaml index ef0738012..4b9593411 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -694,6 +694,7 @@ paths: enum: - totp - phone + - webauthn friendly_name: type: string issuer: @@ -718,6 +719,7 @@ paths: enum: - totp - phone + - webauthn totp: type: object properties: @@ -730,6 +732,7 @@ paths: phone: type: string format: phone + 400: $ref: "#/components/responses/BadRequestResponse" @@ -768,17 +771,9 @@ paths: content: application/json: schema: - type: object - properties: - id: - type: string - format: uuid - example: 14c1560e-2749-4522-bb62-d1458451830a - description: ID of the challenge. - expires_at: - type: integer - example: 1674840917 - description: UNIX seconds of the timestamp past which the challenge should not be verified. + oneOf: + - $ref: '#/components/schemas/TOTPPhoneChallengeResponse' + - $ref: '#/components/schemas/WebAuthnChallengeResponse' 400: $ref: "#/components/responses/BadRequestResponse" 429: @@ -1705,10 +1700,6 @@ paths: optional: true example: twilio description: Which SMS provider is being used to send messages to phone numbers. - mfa_enabled: - type: boolean - example: true - description: Whether MFA is enabled on this API server. Defaults to false. saml_enabled: type: boolean example: true @@ -1980,9 +1971,23 @@ components: Usually one of: - totp - phone + - webauthn + web_authn_credential: + type: jsonb phone: type: string format: phone + nullable: true + created_at: + type: string + format: date-time + updated_at: + type: string + format: date-time + last_challenged_at: + type: string + format: date-time + nullable: true IdentitySchema: @@ -2013,6 +2018,162 @@ components: email: type: string format: email + TOTPPhoneChallengeResponse: + type: object + required: + - id + - type + - expires_at + properties: + id: + type: string + format: uuid + example: 14c1560e-2749-4522-bb62-d1458451830a + description: ID of the challenge. + type: + type: string + enum: [totp, phone] + description: Type of the challenge. + expires_at: + type: integer + example: 1674840917 + description: UNIX seconds of the timestamp past which the challenge should not be verified. + + WebAuthnChallengeResponse: + type: object + required: + - id + - type + - expires_at + - credential_options + properties: + id: + type: string + format: uuid + example: 14c1560e-2749-4522-bb62-d1458451830a + description: ID of the challenge. + type: + type: string + enum: [webauthn] + description: Type of the challenge. + expires_at: + type: integer + example: 1674840917 + description: UNIX seconds of the timestamp past which the challenge should not be verified. + credential_request_options: + $ref: '#/components/schemas/CredentialRequestOptions' + credential_creation_options: + $ref: '#/components/schemas/CredentialCreationOptions' + + CredentialAssertion: + type: object + description: WebAuthn credential assertion options + required: + - challenge + - rpId + - allowCredentials + - timeout + properties: + challenge: + type: string + description: A random challenge generated by the server, base64url encoded + example: "Y2hhbGxlbmdlAyv-5P0kw1SG-OxhLbSHpRLdWaVR1w" + rpId: + type: string + description: The relying party's identifier (usually the domain name) + example: "example.com" + allowCredentials: + type: array + description: List of credentials acceptable for this authentication + items: + type: object + required: + - id + - type + properties: + id: + type: string + description: Credential ID, base64url encoded + example: "AXwyVxYT7BgNKwNq0YqUXaHHIdRK6OdFGCYgZF9K6zNu" + type: + type: string + enum: [public-key] + description: Type of the credential + timeout: + type: integer + description: Time (in milliseconds) that the user has to respond to the authentication prompt + example: 60000 + userVerification: + type: string + enum: [required, preferred, discouraged] + description: The relying party's requirements for user verification + default: preferred + extensions: + type: object + description: Additional parameters requesting additional processing by the client + status: + type: string + enum: [ok, failed] + description: Status of the credential assertion + errorMessage: + type: string + description: Error message if the assertion failed + userHandle: + type: string + description: User handle, base64url encoded + authenticatorAttachment: + type: string + enum: [platform, cross-platform] + description: Type of authenticator to use + + CredentialRequest: + type: object + description: WebAuthn credential request (for the response from the client) + required: + - id + - rawId + - type + - response + properties: + id: + type: string + description: Base64url encoding of the credential ID + example: "AXwyVxYT7BgNKwNq0YqUXaHHIdRK6OdFGCYgZF9K6zNu" + rawId: + type: string + description: Base64url encoding of the credential ID (same as id) + example: "AXwyVxYT7BgNKwNq0YqUXaHHIdRK6OdFGCYgZF9K6zNu" + type: + type: string + enum: [public-key] + description: Type of the credential + response: + type: object + required: + - clientDataJSON + - authenticatorData + - signature + - userHandle + properties: + clientDataJSON: + type: string + description: Base64url encoding of the client data + example: "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiY2hhbGxlbmdlIiwib3JpZ2luIjoiaHR0cHM6Ly9leGFtcGxlLmNvbSJ9" + authenticatorData: + type: string + description: Base64url encoding of the authenticator data + example: "SZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2MBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAXwyVxYT7BgNKwNq0YqUXaHHIdRK6OdFGCYgZF9K6zNu" + signature: + type: string + description: Base64url encoding of the signature + example: "MEUCIQCx5cJVAB3kGP6bqCIoAV6CkBpVAf8rcx0WSZ22fIxXvQIgCKFt9pEu1vK8U4JKYTfn6tGjvGNfx2F4uXrHSXlefvM" + userHandle: + type: string + description: Base64url encoding of the user handle + example: "MQ" + clientExtensionResults: + type: object + description: Client extension results responses: OAuthCallbackRedirectResponse: