Skip to content

Commit 2a9d22f

Browse files
authored
fix: prevent OAuth account pre-hijacking via unverified email linking (#503)
When an OAuth login matches an existing account by email, the handler now checks EmailVerifiedAt. If the existing account's email was never verified, the unverified account is deleted and a fresh one is created for the OAuth user. This prevents attackers from pre-registering with a victim's email to retain password access after the victim's OAuth login.
1 parent 6d9bef1 commit 2a9d22f

3 files changed

Lines changed: 386 additions & 47 deletions

File tree

internal/http_handlers/oauth_callback.go

Lines changed: 76 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -161,66 +161,98 @@ func (h *httpProvider) OAuthCallbackHandler() gin.HandlerFunc {
161161
}
162162
isSignUp = true
163163
} else {
164-
user = existingUser
165-
if user.RevokedTimestamp != nil {
164+
if existingUser.RevokedTimestamp != nil {
166165
log.Debug().Msg("User access has been revoked")
167166
ctx.JSON(400, gin.H{"error": "user access has been revoked"})
168167
return
169168
}
170169

171-
// user exists in db, check if method was google
172-
// if not append google to existing signup method and save it
173-
signupMethod := existingUser.SignupMethods
174-
if !strings.Contains(signupMethod, provider) {
175-
signupMethod = signupMethod + "," + provider
176-
}
177-
user.SignupMethods = signupMethod
178-
179-
if user.EmailVerifiedAt == nil {
180-
now := time.Now().Unix()
181-
user.EmailVerifiedAt = &now
182-
}
183-
184-
// There multiple scenarios with roles here in social login
185-
// 1. user has access to protected roles + roles and trying to login
186-
// 2. user has not signed up for one of the available role but trying to signup.
187-
// Need to modify roles in this case
188-
189-
// find the unassigned roles
190-
existingRoles := strings.Split(existingUser.Roles, ",")
191-
unasignedRoles := []string{}
192-
for _, ir := range inputRoles {
193-
if !utils.StringSliceContains(existingRoles, ir) {
194-
unasignedRoles = append(unasignedRoles, ir)
170+
// Prevent account pre-hijacking: if the existing account's email
171+
// was never verified, do not link the OAuth identity to it.
172+
// Instead, delete the unverified account and treat as a new signup
173+
// for the OAuth user who actually controls the email address.
174+
if existingUser.EmailVerifiedAt == nil {
175+
log.Info().Msg("Removing unverified pre-existing account before OAuth signup")
176+
if err := h.StorageProvider.DeleteUser(ctx, existingUser); err != nil {
177+
log.Debug().Err(err).Msg("Failed to delete unverified user")
178+
ctx.JSON(500, gin.H{"error": "failed to process OAuth login"})
179+
return
195180
}
196-
}
197-
198-
if len(unasignedRoles) > 0 {
199-
// check if it contains protected unassigned role
181+
// make sure inputRoles don't include protected roles
200182
hasProtectedRole := false
201-
for _, ur := range unasignedRoles {
202-
protectedRoles := h.Config.ProtectedRoles
203-
if utils.StringSliceContains(protectedRoles, ur) {
183+
for _, ir := range inputRoles {
184+
if utils.StringSliceContains(h.Config.ProtectedRoles, ir) {
204185
hasProtectedRole = true
205186
}
206187
}
207-
208188
if hasProtectedRole {
209-
log.Debug().Err(err).Msg("Invalid role. User is using protected role")
189+
log.Debug().Msg("Invalid role. User is using protected role")
210190
ctx.JSON(400, gin.H{"error": "invalid role"})
211191
return
212-
} else {
213-
user.Roles = existingUser.Roles + "," + strings.Join(unasignedRoles, ",")
214192
}
193+
user.SignupMethods = provider
194+
user.Roles = strings.Join(inputRoles, ",")
195+
now := time.Now().Unix()
196+
user.EmailVerifiedAt = &now
197+
user, err = h.StorageProvider.AddUser(ctx, user)
198+
if err != nil {
199+
log.Debug().Err(err).Msg("Failed to add user after removing unverified account")
200+
ctx.JSON(500, gin.H{"error": err.Error()})
201+
return
202+
}
203+
isSignUp = true
215204
} else {
216-
user.Roles = existingUser.Roles
217-
}
205+
user = existingUser
218206

219-
user, err = h.StorageProvider.UpdateUser(ctx, user)
220-
if err != nil {
221-
log.Debug().Err(err).Msg("Failed to update user")
222-
ctx.JSON(500, gin.H{"error": err.Error()})
223-
return
207+
// user exists in db, check if method was google
208+
// if not append google to existing signup method and save it
209+
signupMethod := existingUser.SignupMethods
210+
if !strings.Contains(signupMethod, provider) {
211+
signupMethod = signupMethod + "," + provider
212+
}
213+
user.SignupMethods = signupMethod
214+
215+
// There multiple scenarios with roles here in social login
216+
// 1. user has access to protected roles + roles and trying to login
217+
// 2. user has not signed up for one of the available role but trying to signup.
218+
// Need to modify roles in this case
219+
220+
// find the unassigned roles
221+
existingRoles := strings.Split(existingUser.Roles, ",")
222+
unasignedRoles := []string{}
223+
for _, ir := range inputRoles {
224+
if !utils.StringSliceContains(existingRoles, ir) {
225+
unasignedRoles = append(unasignedRoles, ir)
226+
}
227+
}
228+
229+
if len(unasignedRoles) > 0 {
230+
// check if it contains protected unassigned role
231+
hasProtectedRole := false
232+
for _, ur := range unasignedRoles {
233+
protectedRoles := h.Config.ProtectedRoles
234+
if utils.StringSliceContains(protectedRoles, ur) {
235+
hasProtectedRole = true
236+
}
237+
}
238+
239+
if hasProtectedRole {
240+
log.Debug().Err(err).Msg("Invalid role. User is using protected role")
241+
ctx.JSON(400, gin.H{"error": "invalid role"})
242+
return
243+
} else {
244+
user.Roles = existingUser.Roles + "," + strings.Join(unasignedRoles, ",")
245+
}
246+
} else {
247+
user.Roles = existingUser.Roles
248+
}
249+
250+
user, err = h.StorageProvider.UpdateUser(ctx, user)
251+
if err != nil {
252+
log.Debug().Err(err).Msg("Failed to update user")
253+
ctx.JSON(500, gin.H{"error": err.Error()})
254+
return
255+
}
224256
}
225257
}
226258

0 commit comments

Comments
 (0)