Skip to content

Commit b1941a0

Browse files
fadymakfadymak
committed
Merge remote-tracking branch 'origin/master' into fm/auth-1111-passkeys-captcha-on-auth
2 parents 8bd82c2 + 30b3aeb commit b1941a0

12 files changed

+547
-87
lines changed

‎internal/api/api.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,9 @@ func NewAPIWithVersion(globalConfig *conf.GlobalConfiguration, db *storage.Conne
313313
r.Use(api.requirePasskeyEnabled)
314314

315315
r.Route("/authentication", func(r *router) {
316-
r.With(api.verifyCaptcha).Post("/options", api.PasskeyAuthenticationOptions)
316+
r.With(api.limitHandler(api.limiterOpts.PasskeyAuthentication)).
317+
With(api.verifyCaptcha).
318+
Post("/options", api.PasskeyAuthenticationOptions)
317319
r.Post("/verify", api.PasskeyAuthenticationVerify)
318320
})
319321

‎internal/api/external_oauth.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func (a *API) oAuthCallback(ctx context.Context, r *http.Request, providerType s
118118
}
119119
token, err := oauthProvider.GetOAuthToken(ctx, oauthCode, tokenOpts...)
120120
if err != nil {
121-
return nil, apierrors.NewInternalServerError("Unable to exchange external code: %s", oauthCode).WithInternalError(err)
121+
return nil, apierrors.NewInternalServerError("Unable to exchange external code: %s", oauthCode[:min(4, len(oauthCode))]).WithInternalError(err)
122122
}
123123

124124
userData, err := oauthProvider.GetUserData(ctx, token)

‎internal/api/options.go‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,22 @@ type LimiterOptions struct {
4242
Email ratelimit.Limiter
4343
Phone ratelimit.Limiter
4444

45-
Signups *limiter.Limiter
46-
AnonymousSignIns *limiter.Limiter
47-
Recover *limiter.Limiter
48-
Resend *limiter.Limiter
49-
MagicLink *limiter.Limiter
50-
Otp *limiter.Limiter
51-
Token *limiter.Limiter
52-
Verify *limiter.Limiter
53-
User *limiter.Limiter
54-
FactorVerify *limiter.Limiter
55-
FactorChallenge *limiter.Limiter
56-
SSO *limiter.Limiter
57-
SAMLAssertion *limiter.Limiter
58-
Web3 *limiter.Limiter
59-
OAuthClientRegister *limiter.Limiter
45+
Signups *limiter.Limiter
46+
AnonymousSignIns *limiter.Limiter
47+
Recover *limiter.Limiter
48+
Resend *limiter.Limiter
49+
MagicLink *limiter.Limiter
50+
Otp *limiter.Limiter
51+
Token *limiter.Limiter
52+
Verify *limiter.Limiter
53+
User *limiter.Limiter
54+
FactorVerify *limiter.Limiter
55+
FactorChallenge *limiter.Limiter
56+
SSO *limiter.Limiter
57+
SAMLAssertion *limiter.Limiter
58+
Web3 *limiter.Limiter
59+
OAuthClientRegister *limiter.Limiter
60+
PasskeyAuthentication *limiter.Limiter
6061
}
6162

6263
func (lo *LimiterOptions) apply(a *API) { a.limiterOpts = lo }
@@ -115,6 +116,7 @@ func NewLimiterOptions(gc *conf.GlobalConfiguration) *LimiterOptions {
115116
o.User = newLimiterPer5mOver1h(gc.RateLimitOtp)
116117
o.Signups = newLimiterPer5mOver1h(gc.RateLimitOtp)
117118
o.OAuthClientRegister = newLimiterPer5mOver1h(gc.RateLimitOAuthDynamicClientRegister)
119+
o.PasskeyAuthentication = newLimiterPer5mOver1h(gc.RateLimitPasskey)
118120

119121
return o
120122
}

‎internal/api/passkey_authentication_test.go‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,20 @@ func (ts *PasskeyTestSuite) TestAuthenticationOptionsCaptchaDisabled() {
274274
ts.NotEmpty(optionsResp.ChallengeID)
275275
}
276276

277+
// TestAuthenticationOptionsRateLimited tests that the passkey authentication options endpoint is rate limited.
278+
func (ts *PasskeyTestSuite) TestAuthenticationOptionsRateLimited() {
279+
// The passkey authentication limiter has a burst of 30 (from newLimiterPer5mOver1h).
280+
// Send 30 requests that consume the burst, then verify the 31st is rejected.
281+
for i := 0; i < 30; i++ {
282+
w := ts.makeRequest(http.MethodPost, "http://localhost/passkeys/authentication/options", nil, withHeader(ts.Config.RateLimitHeader, "1.2.3.4"))
283+
require.Equal(ts.T(), http.StatusOK, w.Code)
284+
}
285+
286+
// 31st request should be rate limited
287+
w := ts.makeRequest(http.MethodPost, "http://localhost/passkeys/authentication/options", nil, withHeader(ts.Config.RateLimitHeader, "1.2.3.4"))
288+
require.Equal(ts.T(), http.StatusTooManyRequests, w.Code)
289+
}
290+
277291
// registerPasskey is a test helper that registers a passkey for the test user
278292
// and returns the authenticator (with stored credential) for later assertion.
279293
func (ts *PasskeyTestSuite) registerPasskey() (*virtualAuthenticator, *PasskeyMetadataResponse) {
@@ -283,7 +297,7 @@ func (ts *PasskeyTestSuite) registerPasskey() (*virtualAuthenticator, *PasskeyMe
283297
origin: ts.Config.WebAuthn.RPOrigins[0],
284298
}
285299

286-
w := ts.makeAuthenticatedRequest(http.MethodPost, "http://localhost/passkeys/registration/options", token, nil)
300+
w := ts.makeRequest(http.MethodPost, "http://localhost/passkeys/registration/options", nil, withBearerToken(token))
287301
ts.Require().Equal(http.StatusOK, w.Code)
288302

289303
var optionsResp PasskeyRegistrationOptionsResponse
@@ -292,10 +306,10 @@ func (ts *PasskeyTestSuite) registerPasskey() (*virtualAuthenticator, *PasskeyMe
292306
credResp, err := authenticator.createCredential(optionsResp.Options)
293307
require.NoError(ts.T(), err)
294308

295-
w = ts.makeAuthenticatedRequest(http.MethodPost, "http://localhost/passkeys/registration/verify", token, map[string]any{
309+
w = ts.makeRequest(http.MethodPost, "http://localhost/passkeys/registration/verify", map[string]any{
296310
"challenge_id": optionsResp.ChallengeID,
297311
"credential_response": json.RawMessage(credResp.JSON),
298-
})
312+
}, withBearerToken(token))
299313
ts.Require().Equal(http.StatusOK, w.Code)
300314

301315
var passkeyResp PasskeyMetadataResponse

‎internal/api/passkey_manage_test.go‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func (ts *PasskeyTestSuite) createTestPasskey(userID uuid.UUID, friendlyName str
3131

3232
func (ts *PasskeyTestSuite) TestPasskeyListEmpty() {
3333
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
34-
w := ts.makeAuthenticatedRequest(http.MethodGet, "http://localhost/passkeys", token, nil)
34+
w := ts.makeRequest(http.MethodGet, "http://localhost/passkeys", nil, withBearerToken(token))
3535

3636
ts.Equal(http.StatusOK, w.Code)
3737

@@ -45,7 +45,7 @@ func (ts *PasskeyTestSuite) TestPasskeyListWithPasskeys() {
4545
pk2 := ts.createTestPasskey(ts.TestUser.ID, "My MacBook")
4646

4747
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
48-
w := ts.makeAuthenticatedRequest(http.MethodGet, "http://localhost/passkeys", token, nil)
48+
w := ts.makeRequest(http.MethodGet, "http://localhost/passkeys", nil, withBearerToken(token))
4949

5050
ts.Equal(http.StatusOK, w.Code)
5151

@@ -74,7 +74,7 @@ func (ts *PasskeyTestSuite) TestPasskeyListDoesNotReturnOtherUsersPasskeys() {
7474
ts.createTestPasskey(otherUser.ID, "Other User Passkey")
7575

7676
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
77-
w := ts.makeAuthenticatedRequest(http.MethodGet, "http://localhost/passkeys", token, nil)
77+
w := ts.makeRequest(http.MethodGet, "http://localhost/passkeys", nil, withBearerToken(token))
7878

7979
ts.Equal(http.StatusOK, w.Code)
8080

@@ -93,9 +93,9 @@ func (ts *PasskeyTestSuite) TestPasskeyUpdateFriendlyName() {
9393
cred := ts.createTestPasskey(ts.TestUser.ID, "Old Name")
9494

9595
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
96-
w := ts.makeAuthenticatedRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", cred.ID), token, map[string]any{
96+
w := ts.makeRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", cred.ID), map[string]any{
9797
"friendly_name": "New Name",
98-
})
98+
}, withBearerToken(token))
9999

100100
ts.Equal(http.StatusOK, w.Code)
101101

@@ -113,7 +113,7 @@ func (ts *PasskeyTestSuite) TestPasskeyUpdateMissingFriendlyName() {
113113
cred := ts.createTestPasskey(ts.TestUser.ID, "Name")
114114

115115
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
116-
w := ts.makeAuthenticatedRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", cred.ID), token, map[string]any{})
116+
w := ts.makeRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", cred.ID), map[string]any{}, withBearerToken(token))
117117

118118
ts.Equal(http.StatusBadRequest, w.Code)
119119
}
@@ -122,9 +122,9 @@ func (ts *PasskeyTestSuite) TestPasskeyUpdateFriendlyNameTooLong() {
122122
cred := ts.createTestPasskey(ts.TestUser.ID, "Name")
123123

124124
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
125-
w := ts.makeAuthenticatedRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", cred.ID), token, map[string]any{
125+
w := ts.makeRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", cred.ID), map[string]any{
126126
"friendly_name": strings.Repeat("a", 121),
127-
})
127+
}, withBearerToken(token))
128128

129129
ts.Equal(http.StatusBadRequest, w.Code)
130130

@@ -139,9 +139,9 @@ func (ts *PasskeyTestSuite) TestPasskeyUpdateFriendlyNameAtMaxLength() {
139139

140140
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
141141
longName := strings.Repeat("a", 120)
142-
w := ts.makeAuthenticatedRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", cred.ID), token, map[string]any{
142+
w := ts.makeRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", cred.ID), map[string]any{
143143
"friendly_name": longName,
144-
})
144+
}, withBearerToken(token))
145145

146146
ts.Equal(http.StatusOK, w.Code)
147147

@@ -157,9 +157,9 @@ func (ts *PasskeyTestSuite) TestPasskeyUpdateOtherUsersPasskey() {
157157
otherCred := ts.createTestPasskey(otherUser.ID, "Other Passkey")
158158

159159
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
160-
w := ts.makeAuthenticatedRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", otherCred.ID), token, map[string]any{
160+
w := ts.makeRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", otherCred.ID), map[string]any{
161161
"friendly_name": "Stolen Passkey",
162-
})
162+
}, withBearerToken(token))
163163

164164
ts.Equal(http.StatusNotFound, w.Code)
165165

@@ -171,18 +171,18 @@ func (ts *PasskeyTestSuite) TestPasskeyUpdateOtherUsersPasskey() {
171171

172172
func (ts *PasskeyTestSuite) TestPasskeyUpdateNonExistent() {
173173
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
174-
w := ts.makeAuthenticatedRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", uuid.Must(uuid.NewV4())), token, map[string]any{
174+
w := ts.makeRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", uuid.Must(uuid.NewV4())), map[string]any{
175175
"friendly_name": "New Name",
176-
})
176+
}, withBearerToken(token))
177177

178178
ts.Equal(http.StatusNotFound, w.Code)
179179
}
180180

181181
func (ts *PasskeyTestSuite) TestPasskeyUpdateInvalidID() {
182182
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
183-
w := ts.makeAuthenticatedRequest(http.MethodPatch, "http://localhost/passkeys/not-a-uuid", token, map[string]any{
183+
w := ts.makeRequest(http.MethodPatch, "http://localhost/passkeys/not-a-uuid", map[string]any{
184184
"friendly_name": "New Name",
185-
})
185+
}, withBearerToken(token))
186186

187187
ts.Equal(http.StatusNotFound, w.Code)
188188
}
@@ -199,7 +199,7 @@ func (ts *PasskeyTestSuite) TestPasskeyDelete() {
199199
cred := ts.createTestPasskey(ts.TestUser.ID, "To Delete")
200200

201201
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
202-
w := ts.makeAuthenticatedRequest(http.MethodDelete, fmt.Sprintf("http://localhost/passkeys/%s", cred.ID), token, nil)
202+
w := ts.makeRequest(http.MethodDelete, fmt.Sprintf("http://localhost/passkeys/%s", cred.ID), nil, withBearerToken(token))
203203

204204
ts.Equal(http.StatusOK, w.Code)
205205

@@ -219,7 +219,7 @@ func (ts *PasskeyTestSuite) TestPasskeyDeleteOtherUsersPasskey() {
219219
otherCred := ts.createTestPasskey(otherUser.ID, "Other Passkey")
220220

221221
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
222-
w := ts.makeAuthenticatedRequest(http.MethodDelete, fmt.Sprintf("http://localhost/passkeys/%s", otherCred.ID), token, nil)
222+
w := ts.makeRequest(http.MethodDelete, fmt.Sprintf("http://localhost/passkeys/%s", otherCred.ID), nil, withBearerToken(token))
223223

224224
ts.Equal(http.StatusNotFound, w.Code)
225225

@@ -230,7 +230,7 @@ func (ts *PasskeyTestSuite) TestPasskeyDeleteOtherUsersPasskey() {
230230

231231
func (ts *PasskeyTestSuite) TestPasskeyDeleteNonExistent() {
232232
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
233-
w := ts.makeAuthenticatedRequest(http.MethodDelete, fmt.Sprintf("http://localhost/passkeys/%s", uuid.Must(uuid.NewV4())), token, nil)
233+
w := ts.makeRequest(http.MethodDelete, fmt.Sprintf("http://localhost/passkeys/%s", uuid.Must(uuid.NewV4())), nil, withBearerToken(token))
234234

235235
ts.Equal(http.StatusNotFound, w.Code)
236236
}
@@ -246,16 +246,16 @@ func (ts *PasskeyTestSuite) TestPasskeyManageDisabled() {
246246
token := ts.generateToken(ts.TestUser, &ts.TestSession.ID)
247247

248248
// List
249-
w := ts.makeAuthenticatedRequest(http.MethodGet, "http://localhost/passkeys/", token, nil)
249+
w := ts.makeRequest(http.MethodGet, "http://localhost/passkeys/", nil, withBearerToken(token))
250250
ts.Equal(http.StatusNotFound, w.Code)
251251

252252
// Update
253-
w = ts.makeAuthenticatedRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", uuid.Must(uuid.NewV4())), token, map[string]any{
253+
w = ts.makeRequest(http.MethodPatch, fmt.Sprintf("http://localhost/passkeys/%s", uuid.Must(uuid.NewV4())), map[string]any{
254254
"friendly_name": "Name",
255-
})
255+
}, withBearerToken(token))
256256
ts.Equal(http.StatusNotFound, w.Code)
257257

258258
// Delete
259-
w = ts.makeAuthenticatedRequest(http.MethodDelete, fmt.Sprintf("http://localhost/passkeys/%s", uuid.Must(uuid.NewV4())), token, nil)
259+
w = ts.makeRequest(http.MethodDelete, fmt.Sprintf("http://localhost/passkeys/%s", uuid.Must(uuid.NewV4())), nil, withBearerToken(token))
260260
ts.Equal(http.StatusNotFound, w.Code)
261261
}

‎internal/api/passkey_registration.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,8 @@ func (a *API) PasskeyRegistrationVerify(w http.ResponseWriter, r *http.Request)
177177
return apierrors.NewBadRequestError(apierrors.ErrorCodeWebAuthnVerificationFailed, "Credential verification failed").WithInternalError(err)
178178
}
179179

180-
// TODO(fm): fallback to AAGUID -> UA -> "Passkey" for friendly name
181-
passkeyCredential := models.NewWebAuthnCredential(user.ID, credential, "")
180+
friendlyName := utilities.PasskeyFriendlyName(credential.Authenticator.AAGUID)
181+
passkeyCredential := models.NewWebAuthnCredential(user.ID, credential, friendlyName)
182182

183183
err = db.Transaction(func(tx *storage.Connection) error {
184184
count, terr := models.CountWebAuthnCredentialsByUserID(tx, user.ID)

0 commit comments

Comments
 (0)