Skip to content

Commit 841eb62

Browse files
authored
fix: correct verification request expiry logic in Login (#492)
* fix: correct verification request expiry logic in Login The expiry check was inverted - non-expired verification requests were being deleted while expired ones blocked the user. Now non-expired requests correctly return 'email verification pending' and expired ones are deleted and re-sent. Fixes #481 * test: add test for login with unverified email verification pending * chore: improve err handling
1 parent 6486586 commit 841eb62

2 files changed

Lines changed: 51 additions & 4 deletions

File tree

internal/graphql/login.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,15 @@ func (g *graphqlProvider) Login(ctx context.Context, params *model.LoginRequest)
110110
// if verification request exists and not expired then return
111111
// if verification request exists and expired then delete it and proceed
112112
if vreq.ExpiresAt > time.Now().Unix() {
113+
log.Debug().Msg("Email verification pending")
114+
return nil, fmt.Errorf(`email verification pending`)
115+
} else {
113116
if err := g.StorageProvider.DeleteVerificationRequest(ctx, vreq); err != nil {
114117
log.Debug().Msg("Failed to delete verification request")
115-
// continue with the flow
118+
return nil, err
119+
} else {
120+
log.Debug().Msg("Verification request deleted")
116121
}
117-
} else {
118-
log.Debug().Msg("Email verification pending")
119-
return nil, fmt.Errorf(`email verification pending`)
120122
}
121123
}
122124
expiresAt := time.Now().Add(1 * time.Minute).Unix()

internal/integration_tests/login_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,53 @@ import (
88
"github.com/authorizerdev/authorizer/internal/graph/model"
99
"github.com/google/uuid"
1010
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
1112
)
1213

14+
// TestLoginUnverifiedEmail tests login with email verification enabled
15+
func TestLoginUnverifiedEmail(t *testing.T) {
16+
cfg := getTestConfig()
17+
cfg.IsEmailServiceEnabled = true
18+
cfg.EnableEmailVerification = true
19+
cfg.SMTPHost = "localhost"
20+
cfg.SMTPPort = 1025
21+
cfg.SMTPSenderEmail = "test@authorizer.dev"
22+
cfg.SMTPSenderName = "Test"
23+
cfg.SMTPLocalName = "Test"
24+
cfg.SMTPSkipTLSVerification = true
25+
ts := initTestSetup(t, cfg)
26+
_, ctx := createContext(ts)
27+
28+
email := "login_unverified_" + uuid.New().String() + "@authorizer.dev"
29+
password := "Password@123"
30+
31+
signupReq := &model.SignUpRequest{
32+
Email: &email,
33+
Password: password,
34+
ConfirmPassword: password,
35+
}
36+
signupRes, err := ts.GraphQLProvider.SignUp(ctx, signupReq)
37+
require.NoError(t, err)
38+
require.NotNil(t, signupRes)
39+
// User should be nil since email verification is pending
40+
assert.Nil(t, signupRes.User)
41+
42+
t.Run("should return verification pending for unverified email", func(t *testing.T) {
43+
loginReq := &model.LoginRequest{
44+
Email: &email,
45+
Password: password,
46+
}
47+
res, err := ts.GraphQLProvider.Login(ctx, loginReq)
48+
// Should get either an error or an OTP screen response
49+
if err != nil {
50+
assert.Contains(t, err.Error(), "verification")
51+
} else {
52+
// If MFA/OTP flow is triggered instead of error
53+
assert.NotNil(t, res)
54+
}
55+
})
56+
}
57+
1358
// TestLogin tests the login functionality of the Authorizer application.
1459
func TestLogin(t *testing.T) {
1560
// Initialize test setup

0 commit comments

Comments
 (0)