Skip to content

Commit a4c1a8e

Browse files
authored
feat: support profile settings method and prevent email impersonation (#851)
2 parents 7bdce2c + c344cbf commit a4c1a8e

File tree

2 files changed

+191
-9
lines changed

2 files changed

+191
-9
lines changed

pkg/kratos/service.go

Lines changed: 67 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -989,21 +989,32 @@ func (s *Service) ParseRegistrationFlowMethodBody(r *http.Request) (*kClient.Upd
989989
return &ret, nil
990990
}
991991

992-
func parseProfileBody(body io.ReadCloser) (*kClient.UpdateRegistrationFlowWithProfileMethod, error) {
993-
var raw map[string]interface{}
994-
if err := json.NewDecoder(body).Decode(&raw); err != nil {
995-
return nil, err
992+
func extractProfileTraits(raw map[string]any) map[string]any {
993+
traits := make(map[string]any)
994+
995+
if nested, ok := raw["traits"].(map[string]any); ok {
996+
for k, v := range nested {
997+
traits[k] = v
998+
}
996999
}
9971000

998-
traits := make(map[string]interface{})
9991001
for k, v := range raw {
10001002
if strings.HasPrefix(k, "traits.") {
1001-
key := strings.TrimPrefix(k, "traits.")
1002-
traits[key] = v
1003-
delete(raw, k)
1003+
traits[strings.TrimPrefix(k, "traits.")] = v
10041004
}
10051005
}
10061006

1007+
return traits
1008+
}
1009+
1010+
func parseProfileBody(body io.ReadCloser) (*kClient.UpdateRegistrationFlowWithProfileMethod, error) {
1011+
var raw map[string]interface{}
1012+
if err := json.NewDecoder(body).Decode(&raw); err != nil {
1013+
return nil, err
1014+
}
1015+
1016+
traits := extractProfileTraits(raw)
1017+
10071018
profileBody := kClient.UpdateRegistrationFlowWithProfileMethod{
10081019
Method: "profile",
10091020
Traits: traits,
@@ -1348,13 +1359,60 @@ func (s *Service) ParseSettingsFlowMethodBody(r *http.Request) (*kClient.UpdateS
13481359
}
13491360
ret = kClient.UpdateSettingsFlowWithLookupMethodAsUpdateSettingsFlowBody(&body)
13501361

1362+
case "profile":
1363+
profileBody, err := s.parseSettingsProfileMethod(r, bodyBytes)
1364+
if err != nil {
1365+
return nil, err
1366+
}
1367+
1368+
ret = kClient.UpdateSettingsFlowWithProfileMethodAsUpdateSettingsFlowBody(profileBody)
1369+
13511370
default:
1352-
return nil, fmt.Errorf("upsupported method: %s", methodPayload.Method)
1371+
return nil, fmt.Errorf("unsupported method: %s", methodPayload.Method)
13531372
}
13541373

13551374
return &ret, nil
13561375
}
13571376

1377+
func (s *Service) parseSettingsProfileMethod(r *http.Request, bodyBytes []byte) (*kClient.UpdateSettingsFlowWithProfileMethod, error) {
1378+
var raw map[string]any
1379+
if err := json.Unmarshal(bodyBytes, &raw); err != nil {
1380+
return nil, fmt.Errorf("failed to parse profile body: %w", err)
1381+
}
1382+
1383+
traits := extractProfileTraits(raw)
1384+
1385+
// Remove the email trait if present in the request
1386+
delete(traits, "email")
1387+
1388+
session, _, err := s.CheckSession(r.Context(), r.Cookies())
1389+
if err != nil {
1390+
return nil, fmt.Errorf("failed to verify session for profile update: %w", err)
1391+
}
1392+
1393+
if session == nil || session.Identity == nil {
1394+
return nil, fmt.Errorf("invalid session state: identity is missing")
1395+
}
1396+
1397+
// Inject the current user email and preserve other existing traits
1398+
// so they are not unset when omitted in the request payload
1399+
identityTraits, ok := session.Identity.Traits.(map[string]any)
1400+
if !ok {
1401+
return nil, fmt.Errorf("internal error: could not parse existing user traits")
1402+
}
1403+
1404+
for k, v := range traits {
1405+
identityTraits[k] = v
1406+
}
1407+
profileBody := kClient.NewUpdateSettingsFlowWithProfileMethod("profile", identityTraits)
1408+
1409+
if csrf, ok := raw["csrf_token"].(string); ok {
1410+
profileBody.SetCsrfToken(csrf)
1411+
}
1412+
1413+
return profileBody, nil
1414+
}
1415+
13581416
func (s *Service) contains(str []string, e string) bool {
13591417
for _, a := range str {
13601418
if a == e {

pkg/kratos/service_test.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3576,6 +3576,130 @@ func TestUpdateSettingsFlowForbiddenStatus(t *testing.T) {
35763576
}
35773577
}
35783578

3579+
func TestParseSettingsFlowProfileMethodBodySuccess(t *testing.T) {
3580+
ctrl := gomock.NewController(t)
3581+
defer ctrl.Finish()
3582+
3583+
mockLogger := NewMockLoggerInterface(ctrl)
3584+
mockHydra := NewMockHydraClientInterface(ctrl)
3585+
mockKratos := NewMockKratosClientInterface(ctrl)
3586+
mockAdminKratos := NewMockKratosAdminClientInterface(ctrl)
3587+
mockAuthz := NewMockAuthorizerInterface(ctrl)
3588+
mockTracer := NewMockTracingInterface(ctrl)
3589+
mockMonitor := monitoring.NewMockMonitorInterface(ctrl)
3590+
mockKratosFrontendApi := NewMockFrontendAPI(ctrl)
3591+
3592+
ctx := context.Background()
3593+
3594+
session := kClient.NewSession("test-session")
3595+
session.Identity = kClient.NewIdentity("id", "schema", "url", map[string]any{
3596+
"email": "test@example.com",
3597+
"name": "Old Name",
3598+
})
3599+
sessionRequest := kClient.FrontendAPIToSessionRequest{
3600+
ApiService: mockKratosFrontendApi,
3601+
}
3602+
3603+
mockTracer.EXPECT().Start(gomock.Any(), "kratos.Service.ToSession").Times(1).Return(ctx, trace.SpanFromContext(ctx))
3604+
mockKratos.EXPECT().FrontendApi().Times(1).Return(mockKratosFrontendApi)
3605+
mockKratosFrontendApi.EXPECT().ToSession(gomock.Any()).Times(1).Return(sessionRequest)
3606+
mockKratosFrontendApi.EXPECT().ToSessionExecute(gomock.Any()).Times(1).Return(session, &http.Response{}, nil)
3607+
3608+
// User attempts to change email while updating profile
3609+
payload := map[string]any{
3610+
"method": "profile",
3611+
"csrf_token": "token123",
3612+
"traits": map[string]any{
3613+
"email": "impersonated@example.com",
3614+
"name": "New Name",
3615+
},
3616+
}
3617+
jsonBody, err := json.Marshal(payload)
3618+
if err != nil {
3619+
t.Fatalf("failed to marshal payload: %v", err)
3620+
}
3621+
3622+
req := httptest.NewRequest(http.MethodPost, "http://some/path", io.NopCloser(bytes.NewBuffer(jsonBody)))
3623+
3624+
svc := NewService(mockKratos, mockAdminKratos, mockHydra, mockAuthz, false, mockTracer, mockMonitor, mockLogger)
3625+
b, err := svc.ParseSettingsFlowMethodBody(req)
3626+
3627+
if err != nil {
3628+
t.Fatalf("expected error to be nil, got %v", err)
3629+
}
3630+
3631+
profileMethod := b.UpdateSettingsFlowWithProfileMethod
3632+
if profileMethod == nil {
3633+
t.Fatalf("expected profile method, got nil")
3634+
}
3635+
3636+
if *profileMethod.CsrfToken != "token123" {
3637+
t.Fatalf("expected csrf_token to be token123, got %s", *profileMethod.CsrfToken)
3638+
}
3639+
3640+
traits := profileMethod.Traits
3641+
if traits == nil {
3642+
t.Fatalf("expected traits to not be nil")
3643+
}
3644+
3645+
if traits["email"] != "test@example.com" {
3646+
t.Fatalf("expected email to be unchanged (test@example.com), got %v", traits["email"])
3647+
}
3648+
if traits["name"] != "New Name" {
3649+
t.Fatalf("expected name to be updated (New Name), got %v", traits["name"])
3650+
}
3651+
}
3652+
3653+
func TestParseSettingsFlowProfileMethodBodySessionFail(t *testing.T) {
3654+
ctrl := gomock.NewController(t)
3655+
defer ctrl.Finish()
3656+
3657+
mockLogger := NewMockLoggerInterface(ctrl)
3658+
mockHydra := NewMockHydraClientInterface(ctrl)
3659+
mockKratos := NewMockKratosClientInterface(ctrl)
3660+
mockAdminKratos := NewMockKratosAdminClientInterface(ctrl)
3661+
mockAuthz := NewMockAuthorizerInterface(ctrl)
3662+
mockTracer := NewMockTracingInterface(ctrl)
3663+
mockMonitor := monitoring.NewMockMonitorInterface(ctrl)
3664+
mockKratosFrontendApi := NewMockFrontendAPI(ctrl)
3665+
3666+
ctx := context.Background()
3667+
3668+
sessionRequest := kClient.FrontendAPIToSessionRequest{
3669+
ApiService: mockKratosFrontendApi,
3670+
}
3671+
3672+
mockTracer.EXPECT().Start(gomock.Any(), "kratos.Service.ToSession").Times(1).Return(ctx, trace.SpanFromContext(ctx))
3673+
mockKratos.EXPECT().FrontendApi().Times(1).Return(mockKratosFrontendApi)
3674+
mockKratosFrontendApi.EXPECT().ToSession(gomock.Any()).Times(1).Return(sessionRequest)
3675+
mockKratosFrontendApi.EXPECT().ToSessionExecute(gomock.Any()).Times(1).Return(nil, &http.Response{}, fmt.Errorf("session invalid or expired"))
3676+
3677+
payload := map[string]any{
3678+
"method": "profile",
3679+
"csrf_token": "token123",
3680+
"traits": map[string]any{
3681+
"name": "New Name",
3682+
},
3683+
}
3684+
jsonBody, err := json.Marshal(payload)
3685+
if err != nil {
3686+
t.Fatalf("failed to marshal payload: %v", err)
3687+
}
3688+
3689+
req := httptest.NewRequest(http.MethodPost, "http://some/path", io.NopCloser(bytes.NewBuffer(jsonBody)))
3690+
3691+
svc := NewService(mockKratos, mockAdminKratos, mockHydra, mockAuthz, false, mockTracer, mockMonitor, mockLogger)
3692+
3693+
_, err = svc.ParseSettingsFlowMethodBody(req)
3694+
3695+
if err == nil {
3696+
t.Fatalf("expected error, got nil")
3697+
}
3698+
if !strings.Contains(err.Error(), "session invalid or expired") {
3699+
t.Fatalf("expected session error, got %v", err)
3700+
}
3701+
}
3702+
35793703
func TestHasNotEnoughLookupSecretsLeftSuccess(t *testing.T) {
35803704
ctrl := gomock.NewController(t)
35813705
defer ctrl.Finish()

0 commit comments

Comments
 (0)