Skip to content

Commit

Permalink
Merge pull request #3 from spiretechnology/cdd/challenge-tokens
Browse files Browse the repository at this point in the history
feat: signed tokens instead of server-side challenge storage
  • Loading branch information
connerdouglass committed Aug 12, 2023
2 parents 6c9fda2 + c8431ef commit 64243e4
Show file tree
Hide file tree
Showing 21 changed files with 337 additions and 401 deletions.
2 changes: 1 addition & 1 deletion .mockery.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ packages:
config:
dir: "internal/mocks"
interfaces:
Challenges:
Credentials:
Tokener:
16 changes: 10 additions & 6 deletions authenticate_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

// AuthenticationChallenge is the challenge that is sent to the client to initiate an authentication ceremony.
type AuthenticationChallenge struct {
Token string `json:"token"`
Challenge string `json:"challenge"`
RPID string `json:"rpId"`
AllowCredentials []AllowedCredential `json:"allowCredentials"`
Expand Down Expand Up @@ -36,15 +37,18 @@ func (w *webauthn) CreateAuthentication(ctx context.Context, user User) (*Authen
return nil, errutil.Wrapf(err, "generating challenge")
}

// Store the challenge in the challenge store
if err := w.options.Challenges.StoreChallenge(ctx, user, challengeBytes); err != nil {
return nil, errutil.Wrapf(err, "storing challenge")
// Create the token for the challenge
token, err := w.options.Tokener.CreateToken(challengeBytes, user)
if err != nil {
return nil, errutil.Wrapf(err, "creating token")
}

// Format the response
var res AuthenticationChallenge
res.Challenge = w.options.Codec.EncodeToString(challengeBytes[:])
res.RPID = w.options.RP.ID
res := AuthenticationChallenge{
Token: token,
Challenge: w.options.Codec.EncodeToString(challengeBytes[:]),
RPID: w.options.RP.ID,
}
for _, cred := range credentials {
res.AllowCredentials = append(res.AllowCredentials, AllowedCredential{
Type: cred.Type,
Expand Down
25 changes: 10 additions & 15 deletions authenticate_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ import (

"github.com/spiretechnology/go-webauthn"
"github.com/spiretechnology/go-webauthn/internal/testutil"
"github.com/spiretechnology/go-webauthn/pkg/challenge"
"github.com/spiretechnology/go-webauthn/pkg/errs"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

Expand All @@ -22,49 +20,46 @@ func TestCreateAuthentication(t *testing.T) {

t.Run(tc.Name, func(t *testing.T) {
t.Run("user has no credentials", func(t *testing.T) {
w, credentials, challenges := setupMocks(tc, nil)
w, credentials, tokener := setupMocks(tc, tc.AuthenticationChallenge)
credentials.On("GetCredentials", ctx, tc.User).Return([]webauthn.Credential{}, nil).Once()

challenge, err := w.CreateAuthentication(ctx, tc.User)
require.Nil(t, challenge, "challenge should be nil")
require.ErrorIs(t, err, errs.ErrNoCredentials, "error should be ErrNoCredentials")

credentials.AssertExpectations(t)
challenges.AssertExpectations(t)
tokener.AssertExpectations(t)
})

t.Run("storing challenge fails", func(t *testing.T) {
w, credentials, challenges := setupMocks(tc, nil)
t.Run("creating challenge token fails", func(t *testing.T) {
w, credentials, tokener := setupMocks(tc, tc.AuthenticationChallenge)
credentials.On("GetCredentials", ctx, tc.User).Return([]webauthn.Credential{testCred}, nil).Once()
challenges.On("StoreChallenge", mock.Anything, tc.User, mock.Anything).Return(errors.New("test error")).Once()
tokener.On("CreateToken", tcChallenge, tc.User).Return("", errors.New("token creation failed")).Once()

challenge, err := w.CreateAuthentication(ctx, tc.User)
require.Nil(t, challenge, "challenge should be nil")
require.Error(t, err, "error should not be nil")

credentials.AssertExpectations(t)
challenges.AssertExpectations(t)
tokener.AssertExpectations(t)
})

t.Run("creates authentication successfully", func(t *testing.T) {
w, credentials, challenges := setupMocks(tc, &webauthn.Options{
ChallengeFunc: func() (challenge.Challenge, error) {
return tcChallenge, nil
},
})
w, credentials, tokener := setupMocks(tc, tc.AuthenticationChallenge)
credentials.On("GetCredentials", ctx, tc.User).Return([]webauthn.Credential{testCred}, nil).Once()
challenges.On("StoreChallenge", mock.Anything, tc.User, mock.Anything).Return(nil).Once()
tokener.On("CreateToken", tcChallenge, tc.User).Return(tc.Authentication.Token, nil).Once()

challenge, err := w.CreateAuthentication(ctx, tc.User)
require.NotNil(t, challenge, "challenge should not be nil")
require.Nil(t, err, "error should be nil")

require.Equal(t, tc.Authentication.Token, challenge.Token, "token should match")
require.Equal(t, testutil.Encode(tcChallenge[:]), challenge.Challenge, "challenge should match")
require.Equal(t, tc.RelyingParty.ID, challenge.RPID, "relying party should match")
require.Equal(t, 1, len(challenge.AllowCredentials), "allow credentials should match")

credentials.AssertExpectations(t)
challenges.AssertExpectations(t)
tokener.AssertExpectations(t)
})
})
}
Expand Down
14 changes: 4 additions & 10 deletions authenticate_verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

// AuthenticationResponse is the response sent back by the client after an authentication ceremony.
type AuthenticationResponse struct {
Token string `json:"token"`
Challenge string `json:"challenge"`
CredentialID string `json:"credentialId"`
Response AuthenticatorAssertionResponse `json:"response"`
Expand All @@ -32,17 +33,10 @@ func (w *webauthn) VerifyAuthentication(ctx context.Context, user User, res *Aut
return nil, errutil.Wrap(errs.ErrInvalidChallenge)
}
challengeBytes := challenge.Challenge(challengeBytesSlice)
ok, err := w.options.Challenges.HasChallenge(ctx, user, challengeBytes)
if err != nil {
return nil, errutil.Wrapf(err, "checking challenge")
}
if !ok {
return nil, errutil.Wrap(errs.ErrUnrecognizedChallenge)
}

// Remove the challenge from the store. It's no longer needed.
if err := w.options.Challenges.RemoveChallenge(ctx, user, challengeBytes); err != nil {
return nil, errutil.Wrapf(err, "removing challenge")
// Verify the challenge token
if err := w.options.Tokener.VerifyToken(res.Token, challengeBytes, user); err != nil {
return nil, errutil.Wrapf(err, "verifying token")
}

// Decode the received credential ID
Expand Down
32 changes: 13 additions & 19 deletions authenticate_verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,19 @@ package webauthn_test

import (
"context"
"errors"
"testing"

"github.com/spiretechnology/go-webauthn"
"github.com/spiretechnology/go-webauthn/internal/mocks"
"github.com/spiretechnology/go-webauthn/internal/testutil"
"github.com/spiretechnology/go-webauthn/pkg/challenge"
"github.com/spiretechnology/go-webauthn/pkg/errs"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

func seedMockWithCredential(t *testing.T, tc testutil.TestCase, w webauthn.WebAuthn, credentials *mocks.MockCredentials, challenges *mocks.MockChallenges) webauthn.Credential {
func seedMockWithCredential(t *testing.T, tc testutil.TestCase, w webauthn.WebAuthn, credentials *mocks.MockCredentials, tokener *mocks.MockTokener) webauthn.Credential {
// Seed the store with a valid credential
challenges.On("HasChallenge", mock.Anything, tc.User, tc.RegistrationChallenge()).Return(true, nil).Once()
challenges.On("RemoveChallenge", mock.Anything, tc.User, tc.RegistrationChallenge()).Return(nil).Once()
tokener.On("VerifyToken", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
credentials.On("StoreCredential", mock.Anything, tc.User, mock.Anything, mock.Anything).Return(nil).Once()
reg, err := w.VerifyRegistration(context.Background(), tc.User, &tc.Registration)
require.NoError(t, err, "seeding credential should not error")
Expand All @@ -29,38 +27,34 @@ func TestVerifyAuthentication(t *testing.T) {
tcChallenge := tc.AuthenticationChallenge()

t.Run(tc.Name, func(t *testing.T) {
t.Run("challenge doesn't exist", func(t *testing.T) {
w, credentials, challenges := setupMocks(tc, nil)
challenges.On("HasChallenge", mock.Anything, tc.User, tcChallenge).Return(false, nil).Once()
t.Run("challenge token is invalid", func(t *testing.T) {
w, credentials, tokener := setupMocks(tc, tc.AuthenticationChallenge)
tokener.On("VerifyToken", tc.Authentication.Token, tcChallenge, tc.User).Return(errors.New("invalid token")).Once()

result, err := w.VerifyAuthentication(ctx, tc.User, &tc.Authentication)
require.Nil(t, result, "result should be nil")
require.ErrorIs(t, err, errs.ErrUnrecognizedChallenge, "error should be errTest")
require.Error(t, err, "verify authentication should error")

credentials.AssertExpectations(t)
challenges.AssertExpectations(t)
tokener.AssertExpectations(t)
})

t.Run("verifies registration successfully", func(t *testing.T) {
w, credentials, challenges := setupMocks(tc, &webauthn.Options{
ChallengeFunc: func() (challenge.Challenge, error) {
return tcChallenge, nil
},
})
w, credentials, tokener := setupMocks(tc, tc.AuthenticationChallenge)

// Seed the store with a valid credential
credential := seedMockWithCredential(t, tc, w, credentials, challenges)
credential := seedMockWithCredential(t, tc, w, credentials, tokener)

challenges.On("HasChallenge", mock.Anything, tc.User, tcChallenge).Return(true, nil).Once()
challenges.On("RemoveChallenge", mock.Anything, tc.User, tcChallenge).Return(nil).Once()
tokener.On("VerifyToken", tc.Authentication.Token, tcChallenge, tc.User).Return(nil).Once()
credentials.On("GetCredential", mock.Anything, tc.User, mock.Anything).Return(&credential, nil).Once()

result, err := w.VerifyAuthentication(ctx, tc.User, &tc.Authentication)
require.Nil(t, err, "error should be nil")
require.NotNil(t, result, "result should not be nil")
require.Equal(t, credential, result.Credential, "credential should match")

credentials.AssertExpectations(t)
challenges.AssertExpectations(t)
tokener.AssertExpectations(t)
})
})
}
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.20

require (
github.com/fxamacker/cbor/v2 v2.4.0
github.com/spiretechnology/go-jwt/v2 v2.1.0
github.com/stretchr/testify v1.8.4
golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb
)
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ github.com/fxamacker/cbor/v2 v2.4.0 h1:ri0ArlOR+5XunOP8CRUowT0pSJOwhW098ZCUyskZD
github.com/fxamacker/cbor/v2 v2.4.0/go.mod h1:TA1xS00nchWmaBnEIxPSE5oHLuJBAVvqrtAnWBwBCVo=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/spiretechnology/go-jwt/v2 v2.1.0 h1:hBSS6rdg+Zg0AdxJqheG9rnFz+0tkyJkmyNfHSznl9k=
github.com/spiretechnology/go-jwt/v2 v2.1.0/go.mod h1:G4t9Mf53sTy+zvYFzcfdnMijb34fywdym5vtm+uSQB8=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
Expand Down

0 comments on commit 64243e4

Please sign in to comment.