Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add discoverable webauthn #3567

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/clidoc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ func init() {
"NewInfoSelfServiceLoginCode": text.NewInfoSelfServiceLoginCode(),
"NewErrorValidationRegistrationRetrySuccessful": text.NewErrorValidationRegistrationRetrySuccessful(),
"NewInfoSelfServiceRegistrationRegisterCode": text.NewInfoSelfServiceRegistrationRegisterCode(),
"NewErrorValidationWebauthnVerifierWrong": text.NewErrorValidationWebauthnVerifierWrong(),
}
}

Expand Down
1 change: 1 addition & 0 deletions contrib/quickstart/kratos/webauthn/kratos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ selfservice:
webauthn:
config:
passwordless: true
enable_discoverable_credentials: true
rp:
display_name: Your Application name
# Set 'id' to the top-level domain.
Expand Down
5 changes: 5 additions & 0 deletions driver/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ const (
ViperKeyWebAuthnRPOrigin = "selfservice.methods.webauthn.config.rp.origin"
ViperKeyWebAuthnRPOrigins = "selfservice.methods.webauthn.config.rp.origins"
ViperKeyWebAuthnPasswordless = "selfservice.methods.webauthn.config.passwordless"
ViperKeyWebAuthnEnableDiscoverableCredentials = "selfservice.methods.webauthn.config.enable_discoverable_credentials"
ViperKeyOAuth2ProviderURL = "oauth2_provider.url"
ViperKeyOAuth2ProviderHeader = "oauth2_provider.headers"
ViperKeyOAuth2ProviderOverrideReturnTo = "oauth2_provider.override_return_to"
Expand Down Expand Up @@ -1373,6 +1374,10 @@ func (p *Config) WebAuthnForPasswordless(ctx context.Context) bool {
return p.GetProvider(ctx).BoolF(ViperKeyWebAuthnPasswordless, false)
}

func (p *Config) WebAuthnEnableDiscoverableCredentials(ctx context.Context) bool {
return p.GetProvider(ctx).BoolF(ViperKeyWebAuthnEnableDiscoverableCredentials, false)
jonas-jonas marked this conversation as resolved.
Show resolved Hide resolved
}

func (p *Config) WebAuthnConfig(ctx context.Context) *webauthn.Config {
scheme := p.SelfPublicURL(ctx).Scheme
id := p.GetProvider(ctx).String(ViperKeyWebAuthnRPID)
Expand Down
5 changes: 5 additions & 0 deletions embedx/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1673,6 +1673,11 @@
"title": "Use For Passwordless Flows",
"description": "If enabled will have the effect that WebAuthn is used for passwordless flows (as a first factor) and not for multi-factor set ups. With this set to true, users will see an option to sign up with WebAuthn on the registration screen."
},
"enable_discoverable_credentials": {
"type": "boolean",
"title": "Enable discoverable webauthn credentials",
"description": "If enabled will have the effect that WebAuthn credentials are discoverable by relying parties."
},
"rp": {
"title": "Relying Party (RP) Config",
"properties": {
Expand Down
17 changes: 10 additions & 7 deletions identity/credentials_webauthn.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package identity
import (
"time"

"github.com/go-webauthn/webauthn/protocol"
"github.com/go-webauthn/webauthn/webauthn"
)

Expand All @@ -24,6 +25,7 @@ func CredentialFromWebAuthn(credential *webauthn.Credential, isPasswordless bool
PublicKey: credential.PublicKey,
IsPasswordless: isPasswordless,
AttestationType: credential.AttestationType,
Transport: credential.Transport,
Authenticator: AuthenticatorWebAuthn{
AAGUID: credential.Authenticator.AAGUID,
SignCount: credential.Authenticator.SignCount,
Expand Down Expand Up @@ -66,13 +68,14 @@ func (c *CredentialWebAuthn) ToWebAuthn() *webauthn.Credential {
}

type CredentialWebAuthn struct {
ID []byte `json:"id"`
PublicKey []byte `json:"public_key"`
AttestationType string `json:"attestation_type"`
Authenticator AuthenticatorWebAuthn `json:"authenticator"`
DisplayName string `json:"display_name"`
AddedAt time.Time `json:"added_at"`
IsPasswordless bool `json:"is_passwordless"`
ID []byte `json:"id"`
PublicKey []byte `json:"public_key"`
AttestationType string `json:"attestation_type"`
Authenticator AuthenticatorWebAuthn `json:"authenticator"`
DisplayName string `json:"display_name"`
AddedAt time.Time `json:"added_at"`
IsPasswordless bool `json:"is_passwordless"`
Transport []protocol.AuthenticatorTransport `json:"transport,omitempty"`
}

type AuthenticatorWebAuthn struct {
Expand Down
41 changes: 33 additions & 8 deletions identity/extension_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"strings"
"sync"

"golang.org/x/exp/slices"

"github.com/ory/jsonschema/v3"
"github.com/ory/x/sqlxx"
"github.com/ory/x/stringslice"
Expand All @@ -17,9 +19,9 @@ import (
)

type SchemaExtensionCredentials struct {
i *Identity
v map[CredentialsType][]string
l sync.Mutex
i *Identity
credentialIdentifiers map[CredentialsType][]string
l sync.Mutex
}

func NewSchemaExtensionCredentials(i *Identity) *SchemaExtensionCredentials {
Expand All @@ -35,15 +37,38 @@ func (r *SchemaExtensionCredentials) setIdentifier(ct CredentialsType, value int
Config: sqlxx.JSONRawMessage{},
}
}
if r.v == nil {
r.v = make(map[CredentialsType][]string)
if r.credentialIdentifiers == nil {
r.credentialIdentifiers = make(map[CredentialsType][]string)
}

r.v[ct] = stringslice.Unique(append(r.v[ct], strings.ToLower(fmt.Sprintf("%s", value))))
cred.Identifiers = r.v[ct]
r.credentialIdentifiers[ct] = stringslice.Unique(append(r.credentialIdentifiers[ct], strings.ToLower(fmt.Sprintf("%s", value))))
cred.Identifiers = r.credentialIdentifiers[ct]
r.i.SetCredentials(ct, *cred)
}

func (r *SchemaExtensionCredentials) addIdentifierForWebAuthn(value any) {
cred, ok := r.i.GetCredentials(CredentialsTypeWebAuthn)
if !ok {
cred = &Credentials{
Type: CredentialsTypeWebAuthn,
Identifiers: []string{},
Config: sqlxx.JSONRawMessage{},
}
}
if r.credentialIdentifiers == nil {
r.credentialIdentifiers = make(map[CredentialsType][]string)
}

r.credentialIdentifiers[CredentialsTypeWebAuthn] = cred.Identifiers
normalizedAddress := strings.ToLower(fmt.Sprintf("%s", value))
if !slices.Contains(r.credentialIdentifiers[CredentialsTypeWebAuthn], normalizedAddress) {
r.credentialIdentifiers[CredentialsTypeWebAuthn] = append(r.credentialIdentifiers[CredentialsTypeWebAuthn], normalizedAddress)
}

cred.Identifiers = r.credentialIdentifiers[CredentialsTypeWebAuthn]
r.i.SetCredentials(CredentialsTypeWebAuthn, *cred)
}

func (r *SchemaExtensionCredentials) Run(ctx jsonschema.ValidationContext, s schema.ExtensionConfig, value interface{}) error {
r.l.Lock()
defer r.l.Unlock()
Expand All @@ -53,7 +78,7 @@ func (r *SchemaExtensionCredentials) Run(ctx jsonschema.ValidationContext, s sch
}

if s.Credentials.WebAuthn.Identifier {
r.setIdentifier(CredentialsTypeWebAuthn, value, CredentialsIdentifierAddressTypeNone)
r.addIdentifierForWebAuthn(value)
}

if s.Credentials.Code.Identifier {
Expand Down
4 changes: 2 additions & 2 deletions identity/extension_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ func TestSchemaExtensionCredentials(t *testing.T) {
{
doc: `{"email":"FOO@ory.sh"}`,
schema: "file://./stub/extension/credentials/webauthn.schema.json",
expect: []string{"foo@ory.sh"},
expect: []string{"foo@ory.sh", "642fd313-9d6d-400d-b64d-04156ef3dce4"},
existing: &identity.Credentials{
Identifiers: []string{"not-foo@ory.sh"},
Identifiers: []string{"642fd313-9d6d-400d-b64d-04156ef3dce4"},
},
ct: identity.CredentialsTypeWebAuthn,
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
DELETE FROM
identity_credential_types
WHERE
name = 'webauthn_key';
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
INSERT INTO
identity_credential_types (id, name)
SELECT
'92a3a4d1-f045-4fb2-b6c4-9a0ce104682f',
'webauthn_key'
WHERE
NOT EXISTS (
SELECT
*
FROM
identity_credential_types
WHERE
name = 'webauthn_key'
);
2 changes: 1 addition & 1 deletion schema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func NewTOTPVerifierWrongError(instancePtr string) error {
}

func NewWebAuthnVerifierWrongError(instancePtr string) error {
t := text.NewErrorValidationTOTPVerifierWrong()
t := text.NewErrorValidationWebauthnVerifierWrong()
return errors.WithStack(&ValidationError{
ValidationError: &jsonschema.ValidationError{
Message: t.Text,
Expand Down
2 changes: 1 addition & 1 deletion selfservice/flow/login/flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func (f Flow) GetID() uuid.UUID {
return f.ID
}

func (f *Flow) IsForced() bool {
func (f *Flow) IsRefresh() bool {
return f.Refresh
}

Expand Down
4 changes: 2 additions & 2 deletions selfservice/flow/login/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ func TestFlowLifecycle(t *testing.T) {

testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/password.schema.json")

assertion := func(body []byte, isForced, isApi bool) {
assertion := func(body []byte, isRefresh, isApi bool) {
r := gjson.GetBytes(body, "refresh")
assert.True(t, r.Exists(), "%s", body)
assert.Equal(t, isForced, r.Bool(), "%s", body)
assert.Equal(t, isRefresh, r.Bool(), "%s", body)
if isApi {
assert.Equal(t, "api", gjson.GetBytes(body, "type").String())
} else {
Expand Down
8 changes: 4 additions & 4 deletions selfservice/flow/recovery/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestInitFlow(t *testing.T) {
conf.MustSet(ctx, config.ViperKeySelfServiceBrowserDefaultReturnTo, "https://www.ory.sh")
testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/identity.schema.json")

assertion := func(body []byte, isForced, isApi bool) {
assertion := func(body []byte, isApi bool) {
if isApi {
assert.Equal(t, "api", gjson.GetBytes(body, "type").String())
} else {
Expand Down Expand Up @@ -134,7 +134,7 @@ func TestInitFlow(t *testing.T) {
t.Run("case=creates a new flow on unauthenticated request", func(t *testing.T) {
res, body := initFlow(t, true)
assert.Contains(t, res.Request.URL.String(), recovery.RouteInitAPIFlow)
assertion(body, false, true)
assertion(body, true)
})

t.Run("case=fails on authenticated request", func(t *testing.T) {
Expand All @@ -148,7 +148,7 @@ func TestInitFlow(t *testing.T) {
t.Run("case=creates a new flow on unauthenticated request", func(t *testing.T) {
res, body := initSPAFlow(t, new(http.Client), true)
assert.Contains(t, res.Request.URL.String(), recovery.RouteInitBrowserFlow)
assertion(body, false, false)
assertion(body, false)
})

t.Run("case=fails on authenticated request", func(t *testing.T) {
Expand All @@ -161,7 +161,7 @@ func TestInitFlow(t *testing.T) {
t.Run("flow=browser", func(t *testing.T) {
t.Run("case=does not set forced flag on unauthenticated request", func(t *testing.T) {
res, body := initFlow(t, false)
assertion(body, false, false)
assertion(body, false)
assert.Contains(t, res.Request.URL.String(), recoveryTS.URL)
})

Expand Down
12 changes: 6 additions & 6 deletions selfservice/flow/registration/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TestInitFlow(t *testing.T) {
conf.MustSet(ctx, config.ViperKeySelfServiceBrowserDefaultReturnTo, "https://www.ory.sh")
testhelpers.SetDefaultIdentitySchema(conf, "file://./stub/login.schema.json")

assertion := func(body []byte, isForced, isApi bool) {
assertion := func(body []byte, isApi bool) {
if isApi {
assert.Equal(t, "api", gjson.GetBytes(body, "type").String())
} else {
Expand Down Expand Up @@ -146,14 +146,14 @@ func TestInitFlow(t *testing.T) {
t.Run("case=creates a new flow on unauthenticated request", func(t *testing.T) {
res, body := initFlow(t, url.Values{}, true)
assert.Contains(t, res.Request.URL.String(), registration.RouteInitAPIFlow)
assertion(body, false, true)
assertion(body, true)
assert.Empty(t, gjson.GetBytes(body, "session_token_exchange_code").String())
})

t.Run("case=returns a session token exchange code", func(t *testing.T) {
res, body := initFlow(t, urlx.ParseOrPanic("/?return_session_token_exchange_code=true").Query(), true)
assert.Contains(t, res.Request.URL.String(), registration.RouteInitAPIFlow)
assertion(body, false, true)
assertion(body, true)
assert.NotEmpty(t, gjson.GetBytes(body, "session_token_exchange_code").String())
})

Expand All @@ -167,20 +167,20 @@ func TestInitFlow(t *testing.T) {
t.Run("flow=browser", func(t *testing.T) {
t.Run("case=does not set forced flag on unauthenticated request", func(t *testing.T) {
res, body := initFlow(t, url.Values{}, false)
assertion(body, false, false)
assertion(body, false)
assert.Contains(t, res.Request.URL.String(), registrationTS.URL)
assert.Empty(t, gjson.GetBytes(body, "session_token_exchange_code").String())
})

t.Run("case=never returns a session token exchange code", func(t *testing.T) {
_, body := initFlow(t, urlx.ParseOrPanic("/?return_session_token_exchange_code=true").Query(), false)
assertion(body, false, false)
assertion(body, false)
assert.Empty(t, gjson.GetBytes(body, "session_token_exchange_code").String())
})

t.Run("case=makes request with JSON", func(t *testing.T) {
res, body := initSPAFlow(t)
assertion(body, false, false)
assertion(body, false)
assert.NotContains(t, res.Request.URL.String(), registrationTS.URL)
})

Expand Down
4 changes: 2 additions & 2 deletions selfservice/flowhelpers/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ func GuessForcedLoginIdentifier(r *http.Request, d interface {
session.ManagementProvider
identity.PrivilegedPoolProvider
}, f interface {
IsForced() bool
IsRefresh() bool
}, ct identity.CredentialsType) (identifier string, id *identity.Identity, creds *identity.Credentials) {
var ok bool
// This block adds the identifier to the method when the request is forced - as a hint for the user.
if !f.IsForced() {
if !f.IsRefresh() {
// do nothing
} else if sess, err := d.SessionManager().FetchFromRequest(r.Context(), r); err != nil {
// do nothing
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/oidc/provider_apple.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (a *ProviderApple) OAuth2(ctx context.Context) (*oauth2.Config, error) {
func (a *ProviderApple) AuthCodeURLOptions(r ider) []oauth2.AuthCodeOption {
var options []oauth2.AuthCodeOption

if isForced(r) {
if isRefresh(r) {
options = append(options, oauth2.SetAuthURLParam("prompt", "login"))
}
if len(a.config.RequestedClaims) != 0 {
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/oidc/provider_discord.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (d *ProviderDiscord) OAuth2(ctx context.Context) (*oauth2.Config, error) {
}

func (d *ProviderDiscord) AuthCodeURLOptions(r ider) []oauth2.AuthCodeOption {
if isForced(r) {
if isRefresh(r) {
return []oauth2.AuthCodeOption{
oauth2.SetAuthURLParam("prompt", "consent"),
}
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/oidc/provider_generic_oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (g *ProviderGenericOIDC) OAuth2(ctx context.Context) (*oauth2.Config, error
func (g *ProviderGenericOIDC) AuthCodeURLOptions(r ider) []oauth2.AuthCodeOption {
var options []oauth2.AuthCodeOption

if isForced(r) {
if isRefresh(r) {
options = append(options, oauth2.SetAuthURLParam("prompt", "login"))
}
if len(g.config.RequestedClaims) != 0 {
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/oidc/provider_patreon.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (d *ProviderPatreon) OAuth2(ctx context.Context) (*oauth2.Config, error) {
}

func (d *ProviderPatreon) AuthCodeURLOptions(r ider) []oauth2.AuthCodeOption {
if isForced(r) {
if isRefresh(r) {
return []oauth2.AuthCodeOption{
oauth2.SetAuthURLParam("prompt", "consent"),
}
Expand Down
8 changes: 4 additions & 4 deletions selfservice/strategy/oidc/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ type Dependencies interface {
jsonnetsecure.VMProvider
}

func isForced(req interface{}) bool {
func isRefresh(req interface{}) bool {
f, ok := req.(interface {
IsForced() bool
IsRefresh() bool
})
return ok && f.IsForced()
return ok && f.IsRefresh()
}

// Strategy implements selfservice.LoginStrategy, selfservice.RegistrationStrategy and selfservice.SettingsStrategy.
Expand Down Expand Up @@ -352,7 +352,7 @@ func (s *Strategy) alreadyAuthenticated(w http.ResponseWriter, r *http.Request,
if sess, _ := s.d.SessionManager().FetchFromRequest(ctx, r); sess != nil {
if _, ok := f.(*settings.Flow); ok {
// ignore this if it's a settings flow
} else if !isForced(f) {
} else if !isRefresh(f) {
if flowID, ok := registrationOrLoginFlowID(f); ok {
if _, hasCode, _ := s.d.SessionTokenExchangePersister().CodeForFlow(ctx, flowID); hasCode {
err := s.d.SessionTokenExchangePersister().UpdateSessionOnExchanger(ctx, flowID, sess.ID)
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/password/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (s *Strategy) PopulateLoginMethod(r *http.Request, requestedAAL identity.Au
return nil
}

if sr.IsForced() {
if sr.IsRefresh() {
// We only show this method on a refresh request if the user has indeed a password set.
identifier, id, _ := flowhelpers.GuessForcedLoginIdentifier(r, s.d, sr, s.ID())
if identifier == "" {
Expand Down
Loading
Loading