Skip to content

Commit 6d2fdc2

Browse files
timedoutuktimedoutuk
authored
Perform UIA correctly when registering users on a deployment (#896)
* Perform UIA correctly when registering users on a deployment Fixes #893 Signed-off-by: timedout <git@nexy7574.co.uk> * Properly initiate UIA sessions in `TestRegistration` Signed-off-by: timedout <git@nexy7574.co.uk> * `session` is optional and may be omitted if not provided Signed-off-by: timedout <git@nexy7574.co.uk> * Add a regression test to ensure `session` optionality is respected during registration Signed-off-by: timedout <git@nexy7574.co.uk> * Request validation is performed before authentication Also fixes the maps.Clone param order being inverted Signed-off-by: timedout <git@nexy7574.co.uk> * Skip sessionless UIA test on some homeservers Signed-off-by: timedout <git@nexy7574.co.uk> * Start a new session in pre-existing user register test Synapse validates the request body before auth, but Dendrite does not. This breaks the stalemate by just starting a "new" UIA session Signed-off-by: timedout <git@nexy7574.co.uk> * Skip existing user registration test on Dendrite and revert to previous behaviour Signed-off-by: timedout <git@nexy7574.co.uk> --------- Signed-off-by: timedout <git@nexy7574.co.uk>
1 parent 13f0b93 commit 6d2fdc2

3 files changed

Lines changed: 98 additions & 63 deletions

File tree

‎client/auth.go‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,22 +99,39 @@ func (c *CSAPI) ConsumeRefreshToken(t ct.TestLike, refreshToken string) (newAcce
9999
}
100100

101101
// RegisterUser will register the user with given parameters and
102-
// return user ID, access token and device ID. It fails the test on network error.
102+
// return user ID, access token and device ID. It fails the test on network error,
103+
// or if registration fails for another reason (e.g. server has non-dummy requirements).
103104
func (c *CSAPI) RegisterUser(t ct.TestLike, localpart, password string) (userID, accessToken, deviceID string) {
104105
t.Helper()
105-
reqBody := map[string]interface{}{
106-
"auth": map[string]string{
107-
"type": "m.login.dummy",
108-
},
106+
reqBody := map[string]any{
109107
"username": localpart,
110108
"password": password,
111109
}
112-
res := c.MustDo(t, "POST", []string{"_matrix", "client", "v3", "register"}, WithJSONBody(t, reqBody))
110+
// First request is expected to receive a UIA challenge
111+
res := c.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, WithJSONBody(t, reqBody))
112+
if res.StatusCode != 401 {
113+
ct.Fatalf(t, "Expected 401 Unauthorized, got %d", res.StatusCode)
114+
}
113115

114116
body, err := io.ReadAll(res.Body)
115117
if err != nil {
116118
ct.Fatalf(t, "unable to read response body: %v", err)
117119
}
120+
session := GetJSONFieldStr(t, body, "session")
121+
122+
// Now actually register the user
123+
reqBody["auth"] = map[string]any{
124+
"session": session,
125+
"type": "m.login.dummy",
126+
}
127+
if session == "" {
128+
delete(reqBody["auth"].(map[string]any), "session")
129+
}
130+
res = c.MustDo(t, "POST", []string{"_matrix", "client", "v3", "register"}, WithJSONBody(t, reqBody))
131+
body, err = io.ReadAll(res.Body)
132+
if err != nil {
133+
ct.Fatalf(t, "unable to read response body: %v", err)
134+
}
118135

119136
userID = GetJSONFieldStr(t, body, "user_id")
120137
accessToken = GetJSONFieldStr(t, body, "access_token")

‎tests/csapi/apidoc_register_test.go‎

Lines changed: 74 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import (
66
"encoding/hex"
77
"encoding/json"
88
"fmt"
9+
"io"
910
"io/ioutil"
11+
"maps"
1012
"net/http"
1113
"net/url"
1214
"testing"
@@ -63,17 +65,11 @@ func TestRegistration(t *testing.T) {
6365
})
6466
// sytest: POST /register can create a user
6567
t.Run("POST /register can create a user", func(t *testing.T) {
66-
// Venator: https://github.com/matrix-org/complement/issues/893
67-
runtime.SkipIf(t, runtime.Venator)
6868
t.Parallel()
69-
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithRawBody(json.RawMessage(`{
70-
"auth": {
71-
"type": "m.login.dummy"
72-
},
73-
"username": "post-can-create-a-user",
74-
"password": "sUp3rs3kr1t"
75-
}`)))
69+
reqBody, _ := startUIASession(t, unauthedClient, "post-can-create-a-user", "sUp3rs3kr1t", nil)
70+
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithJSONBody(t, reqBody))
7671
must.MatchResponse(t, res, match.HTTPResponse{
72+
StatusCode: 200,
7773
JSON: []match.JSON{
7874
match.JSONKeyTypeEqual("access_token", gjson.String),
7975
match.JSONKeyTypeEqual("user_id", gjson.String),
@@ -82,17 +78,11 @@ func TestRegistration(t *testing.T) {
8278
})
8379
// sytest: POST /register downcases capitals in usernames
8480
t.Run("POST /register downcases capitals in usernames", func(t *testing.T) {
85-
// Venator: https://github.com/matrix-org/complement/issues/893
86-
runtime.SkipIf(t, runtime.Venator)
8781
t.Parallel()
88-
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithRawBody(json.RawMessage(`{
89-
"auth": {
90-
"type": "m.login.dummy"
91-
},
92-
"username": "user-UPPER",
93-
"password": "sUp3rs3kr1t"
94-
}`)))
82+
reqBody, _ := startUIASession(t, unauthedClient, "user-UPPER", "sUp3rs3kr1t", nil)
83+
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithJSONBody(t, reqBody))
9584
must.MatchResponse(t, res, match.HTTPResponse{
85+
StatusCode: 200,
9686
JSON: []match.JSON{
9787
match.JSONKeyTypeEqual("access_token", gjson.String),
9888
match.JSONKeyEqual("user_id", "@user-upper:hs1"),
@@ -101,19 +91,12 @@ func TestRegistration(t *testing.T) {
10191
})
10292
// sytest: POST /register returns the same device_id as that in the request
10393
t.Run("POST /register returns the same device_id as that in the request", func(t *testing.T) {
104-
// Venator: https://github.com/matrix-org/complement/issues/893
105-
runtime.SkipIf(t, runtime.Venator)
10694
t.Parallel()
10795
deviceID := "my_device_id"
108-
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithRawBody(json.RawMessage(`{
109-
"auth": {
110-
"type": "m.login.dummy"
111-
},
112-
"username": "user-device",
113-
"password": "sUp3rs3kr1t",
114-
"device_id": "`+deviceID+`"
115-
}`)))
96+
reqBody, _ := startUIASession(t, unauthedClient, "user-device", deviceID, map[string]any{"device_id": deviceID})
97+
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithJSONBody(t, reqBody))
11698
must.MatchResponse(t, res, match.HTTPResponse{
99+
StatusCode: 200,
117100
JSON: []match.JSON{
118101
match.JSONKeyTypeEqual("access_token", gjson.String),
119102
match.JSONKeyEqual("device_id", deviceID),
@@ -122,8 +105,6 @@ func TestRegistration(t *testing.T) {
122105
})
123106
// sytest: POST /register rejects registration of usernames with '$q'
124107
t.Run("POST /register rejects usernames with special characters", func(t *testing.T) {
125-
// Venator: https://github.com/matrix-org/complement/issues/893
126-
runtime.SkipIf(t, runtime.Venator)
127108
t.Parallel()
128109
specialChars := []string{
129110
`!`,
@@ -143,14 +124,13 @@ func TestRegistration(t *testing.T) {
143124
`'`,
144125
}
145126
for _, ch := range specialChars {
146-
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"},
147-
client.WithJSONBody(t, map[string]interface{}{
148-
"auth": map[string]string{
149-
"type": "m.login.dummy",
150-
},
151-
"username": "user-" + ch + "-reject-please",
152-
"password": "sUp3rs3kr1t",
153-
}))
127+
reqBody := map[string]any{
128+
"username": "user-" + ch + "-reject-please",
129+
"password": "sUp3rs3kr1t",
130+
}
131+
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithJSONBody(t, reqBody))
132+
// N.B. servers are expected to validate request bodies before handling UIA,
133+
// so 400 is expected here, not 401.
154134
must.MatchResponse(t, res, match.HTTPResponse{
155135
StatusCode: 400,
156136
JSON: []match.JSON{
@@ -160,37 +140,30 @@ func TestRegistration(t *testing.T) {
160140
}
161141
})
162142
t.Run("POST /register rejects if user already exists", func(t *testing.T) {
163-
// Venator: https://github.com/matrix-org/complement/issues/893
164-
runtime.SkipIf(t, runtime.Venator)
143+
// Dendrite: auth is validated before input, meaning the second register request needs to start a fresh
144+
// auth session. This conflicts with Synapse, which forbids a second session being started, as it
145+
// validates the input before auth. Skip on Dendrite for now.
146+
runtime.SkipIf(t, runtime.Dendrite)
165147
t.Parallel()
166-
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithRawBody(json.RawMessage(`{
167-
"auth": {
168-
"type": "m.login.dummy"
169-
},
170-
"username": "post-can-create-a-user-once",
171-
"password": "sUp3rs3kr1t"
172-
}`)))
148+
reqBody, _ := startUIASession(t, unauthedClient, "post-can-create-a-user-once", "sUp3rs3kr1t", nil)
149+
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithJSONBody(t, reqBody))
173150
must.MatchResponse(t, res, match.HTTPResponse{
174151
JSON: []match.JSON{
175152
match.JSONKeyTypeEqual("access_token", gjson.String),
176153
match.JSONKeyTypeEqual("user_id", gjson.String),
177154
},
178155
})
179-
res = unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithRawBody(json.RawMessage(`{
180-
"auth": {
181-
"type": "m.login.dummy"
182-
},
183-
"username": "post-can-create-a-user-once",
184-
"password": "anotherSuperSecret"
185-
}`)))
156+
delete(reqBody, "auth")
157+
res = unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithJSONBody(t, reqBody))
186158
must.MatchResponse(t, res, match.HTTPResponse{
187159
StatusCode: 400,
160+
JSON: []match.JSON{
161+
match.JSONKeyEqual("errcode", "M_USER_IN_USE"),
162+
},
188163
})
189164
})
190165
// sytest: POST /register allows registration of usernames with '$chr'
191166
t.Run("POST /register allows registration of usernames with ", func(t *testing.T) {
192-
// Venator: https://github.com/matrix-org/complement/issues/893
193-
runtime.SkipIf(t, runtime.Venator)
194167
testChars := []rune("q3._=-/")
195168
for x := range testChars {
196169
localpart := fmt.Sprintf("chrtestuser%s", string(testChars[x]))
@@ -321,6 +294,25 @@ func TestRegistration(t *testing.T) {
321294
},
322295
})
323296
})
297+
// Test that subsequent calls to /_matrix/client/v3/register after receiving a UIA
298+
// challenge fail if the session is not provided.
299+
t.Run("Registration without a session fails", func(t *testing.T) {
300+
// Many implementations historically did not enforce this requirement strictly
301+
runtime.SkipIf(t, runtime.Synapse, runtime.Dendrite, runtime.Conduit)
302+
t.Parallel()
303+
reqBody, session := startUIASession(t, unauthedClient, "auth-requires-session", "sUp3rs3kr1t", nil)
304+
if session == "" {
305+
t.Skip("Homeserver does not require a session for UIA")
306+
}
307+
delete(reqBody["auth"].(map[string]any), "session")
308+
// Re-send the same request without the session.
309+
// Since session is required if it is provided by the homeserver, this should
310+
// return an error
311+
res := unauthedClient.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithJSONBody(t, reqBody))
312+
must.MatchResponse(t, res, match.HTTPResponse{
313+
StatusCode: 401,
314+
})
315+
})
324316
})
325317
}
326318

@@ -359,3 +351,29 @@ func registerSharedSecret(t *testing.T, c *client.CSAPI, user, pass string, isAd
359351
resp = c.Do(t, "POST", []string{"_synapse", "admin", "v1", "register"}, client.WithJSONBody(t, reqBody))
360352
return resp
361353
}
354+
355+
// startUIASession starts a UIA session and returns the updated request body,
356+
// and associated session token, failing the test if the response is not a UIA challenge.
357+
func startUIASession(t *testing.T, c *client.CSAPI, user, pass string, extra map[string]any) (map[string]any, string) {
358+
reqBody := map[string]any{
359+
"username": user,
360+
"password": pass,
361+
}
362+
if extra != nil {
363+
maps.Copy(reqBody, extra)
364+
}
365+
res := c.Do(t, "POST", []string{"_matrix", "client", "v3", "register"}, client.WithJSONBody(t, reqBody))
366+
if res.StatusCode != 401 {
367+
t.Fatalf("expected status code 401 (UIA challenge), got %d", res.StatusCode)
368+
}
369+
body, err := io.ReadAll(res.Body)
370+
if err != nil {
371+
t.Fatal(err)
372+
}
373+
session := client.GetJSONFieldStr(t, body, "session")
374+
reqBody["auth"] = map[string]any{"session": session, "type": "m.login.dummy"}
375+
if session == "" {
376+
delete(reqBody["auth"].(map[string]any), "session")
377+
}
378+
return reqBody, session
379+
}

‎tests/csapi/power_levels_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func TestPowerLevels(t *testing.T) {
129129
func(body gjson.Result) error {
130130
// This key should be missing for room v12+
131131
if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() {
132-
must.MatchGJSON(t, body, match.JSONKeyMissing("users." + client.GjsonEscape(alice.UserID)))
132+
must.MatchGJSON(t, body, match.JSONKeyMissing("users."+client.GjsonEscape(alice.UserID)))
133133
return nil
134134
} else {
135135
userDefault := int(body.Get("users_default").Num)

0 commit comments

Comments
 (0)