mirror of
https://github.com/zitadel/zitadel.git
synced 2025-08-11 21:37:32 +00:00
fix(webauthn): allow to use "old" passkeys/u2f credentials on session API (#10150)
# Which Problems Are Solved
To prevent presenting unusable WebAuthN credentials to the user /
browser, we filtered out all credentials, which do not match the
requested RP ID. Since credentials set up through Login V1 and Console
do not have an RP ID stored, they never matched. This was previously
intended, since the Login V2 could be served on a separate domain.
The problem is, that if it is hosted on the same domain, the credentials
would also be filtered out and user would not be able to login.
# How the Problems Are Solved
Change the filtering to return credentials, if no RP ID is stored and
the requested RP ID matches the instance domain.
# Additional Changes
None
# Additional Context
Noted internally when testing the login v2
(cherry picked from commit 71575e8d67)
This commit is contained in:
Livio Spring
@@ -1,16 +1,26 @@
|
||||
package webauthn
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
|
||||
"github.com/go-webauthn/webauthn/protocol"
|
||||
"github.com/go-webauthn/webauthn/webauthn"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/api/http"
|
||||
"github.com/zitadel/zitadel/internal/domain"
|
||||
)
|
||||
|
||||
func WebAuthNsToCredentials(webAuthNs []*domain.WebAuthNToken, rpID string) []webauthn.Credential {
|
||||
func WebAuthNsToCredentials(ctx context.Context, webAuthNs []*domain.WebAuthNToken, rpID string) []webauthn.Credential {
|
||||
creds := make([]webauthn.Credential, 0)
|
||||
for _, webAuthN := range webAuthNs {
|
||||
if webAuthN.State == domain.MFAStateReady && webAuthN.RPID == rpID {
|
||||
// only add credentials that are ready and
|
||||
// either match the rpID or
|
||||
// if they were added through Console / old login UI, there is no stored rpID set;
|
||||
// then we check if the requested rpID matches the instance domain
|
||||
if webAuthN.State == domain.MFAStateReady &&
|
||||
(webAuthN.RPID == rpID ||
|
||||
(webAuthN.RPID == "" && rpID == strings.Split(http.DomainContext(ctx).InstanceHost, ":")[0])) {
|
||||
creds = append(creds, webauthn.Credential{
|
||||
ID: webAuthN.KeyID,
|
||||
PublicKey: webAuthN.PublicKey,
|
||||
|
||||
153
internal/webauthn/converter_test.go
Normal file
153
internal/webauthn/converter_test.go
Normal file
@@ -0,0 +1,153 @@
|
||||
package webauthn
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/go-webauthn/webauthn/webauthn"
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"github.com/zitadel/zitadel/internal/api/http"
|
||||
"github.com/zitadel/zitadel/internal/domain"
|
||||
)
|
||||
|
||||
func TestWebAuthNsToCredentials(t *testing.T) {
|
||||
type args struct {
|
||||
ctx context.Context
|
||||
webAuthNs []*domain.WebAuthNToken
|
||||
rpID string
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
args args
|
||||
want []webauthn.Credential
|
||||
}{
|
||||
{
|
||||
name: "unready credential",
|
||||
args: args{
|
||||
ctx: context.Background(),
|
||||
webAuthNs: []*domain.WebAuthNToken{
|
||||
{
|
||||
KeyID: []byte("key1"),
|
||||
PublicKey: []byte("publicKey1"),
|
||||
AttestationType: "attestation1",
|
||||
AAGUID: []byte("aaguid1"),
|
||||
SignCount: 1,
|
||||
State: domain.MFAStateNotReady,
|
||||
},
|
||||
},
|
||||
rpID: "example.com",
|
||||
},
|
||||
want: []webauthn.Credential{},
|
||||
},
|
||||
{
|
||||
name: "not matching rpID",
|
||||
args: args{
|
||||
ctx: context.Background(),
|
||||
webAuthNs: []*domain.WebAuthNToken{
|
||||
{
|
||||
KeyID: []byte("key1"),
|
||||
PublicKey: []byte("publicKey1"),
|
||||
AttestationType: "attestation1",
|
||||
AAGUID: []byte("aaguid1"),
|
||||
SignCount: 1,
|
||||
State: domain.MFAStateReady,
|
||||
RPID: "other.com",
|
||||
},
|
||||
},
|
||||
rpID: "example.com",
|
||||
},
|
||||
want: []webauthn.Credential{},
|
||||
},
|
||||
{
|
||||
name: "matching rpID",
|
||||
args: args{
|
||||
ctx: context.Background(),
|
||||
webAuthNs: []*domain.WebAuthNToken{
|
||||
{
|
||||
KeyID: []byte("key1"),
|
||||
PublicKey: []byte("publicKey1"),
|
||||
AttestationType: "attestation1",
|
||||
AAGUID: []byte("aaguid1"),
|
||||
SignCount: 1,
|
||||
State: domain.MFAStateReady,
|
||||
RPID: "example.com",
|
||||
},
|
||||
},
|
||||
rpID: "example.com",
|
||||
},
|
||||
want: []webauthn.Credential{
|
||||
{
|
||||
ID: []byte("key1"),
|
||||
PublicKey: []byte("publicKey1"),
|
||||
AttestationType: "attestation1",
|
||||
Authenticator: webauthn.Authenticator{
|
||||
AAGUID: []byte("aaguid1"),
|
||||
SignCount: 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no rpID, different host",
|
||||
args: args{
|
||||
ctx: http.WithDomainContext(context.Background(), &http.DomainCtx{
|
||||
InstanceHost: "other.com:443",
|
||||
PublicHost: "other.com:443",
|
||||
Protocol: "https",
|
||||
}),
|
||||
webAuthNs: []*domain.WebAuthNToken{
|
||||
{
|
||||
KeyID: []byte("key1"),
|
||||
PublicKey: []byte("publicKey1"),
|
||||
AttestationType: "attestation1",
|
||||
AAGUID: []byte("aaguid1"),
|
||||
SignCount: 1,
|
||||
State: domain.MFAStateReady,
|
||||
RPID: "",
|
||||
},
|
||||
},
|
||||
rpID: "example.com",
|
||||
},
|
||||
want: []webauthn.Credential{},
|
||||
},
|
||||
{
|
||||
name: "no rpID, same host",
|
||||
args: args{
|
||||
ctx: http.WithDomainContext(context.Background(), &http.DomainCtx{
|
||||
InstanceHost: "example.com:443",
|
||||
PublicHost: "example.com:443",
|
||||
Protocol: "https",
|
||||
}),
|
||||
webAuthNs: []*domain.WebAuthNToken{
|
||||
{
|
||||
KeyID: []byte("key1"),
|
||||
PublicKey: []byte("publicKey1"),
|
||||
AttestationType: "attestation1",
|
||||
AAGUID: []byte("aaguid1"),
|
||||
SignCount: 1,
|
||||
State: domain.MFAStateReady,
|
||||
RPID: "",
|
||||
},
|
||||
},
|
||||
rpID: "example.com",
|
||||
},
|
||||
want: []webauthn.Credential{
|
||||
{
|
||||
ID: []byte("key1"),
|
||||
PublicKey: []byte("publicKey1"),
|
||||
AttestationType: "attestation1",
|
||||
Authenticator: webauthn.Authenticator{
|
||||
AAGUID: []byte("aaguid1"),
|
||||
SignCount: 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
assert.Equalf(t, tt.want, WebAuthNsToCredentials(tt.args.ctx, tt.args.webAuthNs, tt.args.rpID), "WebAuthNsToCredentials(%v, %v, %v)", tt.args.ctx, tt.args.webAuthNs, tt.args.rpID)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -57,7 +57,7 @@ func (w *Config) BeginRegistration(ctx context.Context, user *domain.Human, acco
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
creds := WebAuthNsToCredentials(webAuthNs, rpID)
|
||||
creds := WebAuthNsToCredentials(ctx, webAuthNs, rpID)
|
||||
existing := make([]protocol.CredentialDescriptor, len(creds))
|
||||
for i, cred := range creds {
|
||||
existing[i] = protocol.CredentialDescriptor{
|
||||
@@ -136,7 +136,7 @@ func (w *Config) BeginLogin(ctx context.Context, user *domain.Human, userVerific
|
||||
}
|
||||
assertion, sessionData, err := webAuthNServer.BeginLogin(&webUser{
|
||||
Human: user,
|
||||
credentials: WebAuthNsToCredentials(webAuthNs, rpID),
|
||||
credentials: WebAuthNsToCredentials(ctx, webAuthNs, rpID),
|
||||
}, webauthn.WithUserVerification(UserVerificationFromDomain(userVerification)))
|
||||
if err != nil {
|
||||
logging.WithFields("error", tryExtractProtocolErrMsg(err)).Debug("webauthn login could not be started")
|
||||
@@ -163,7 +163,7 @@ func (w *Config) FinishLogin(ctx context.Context, user *domain.Human, webAuthN *
|
||||
}
|
||||
webUser := &webUser{
|
||||
Human: user,
|
||||
credentials: WebAuthNsToCredentials(webAuthNs, webAuthN.RPID),
|
||||
credentials: WebAuthNsToCredentials(ctx, webAuthNs, webAuthN.RPID),
|
||||
}
|
||||
webAuthNServer, err := w.serverFromContext(ctx, webAuthN.RPID, assertionData.Response.CollectedClientData.Origin)
|
||||
if err != nil {
|
||||
|
||||
Reference in New Issue
Block a user