[MM-52445] Fix double url encoding of oauth redirect URI params (#23176)

* [MM-52445] Fix double url encoding of oauth redirect URI params

* Additional test based on code review

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Pantelis Vratsalis 2023-11-09 20:18:24 +02:00 committed by GitHub
parent c9e71a2dde
commit d9f5cd2740
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 7 additions and 3 deletions

View File

@ -178,8 +178,8 @@ func (a *App) GetOAuthCodeRedirect(userID string, authRequest *model.AuthorizeRe
uri.RawQuery = queryParams.Encode() uri.RawQuery = queryParams.Encode()
return uri.String(), nil return uri.String(), nil
} }
queryParams.Set("code", url.QueryEscape(authData.Code)) queryParams.Set("code", authData.Code)
queryParams.Set("state", url.QueryEscape(authData.State)) queryParams.Set("state", authData.State)
uri.RawQuery = queryParams.Encode() uri.RawQuery = queryParams.Encode()
return uri.String(), nil return uri.String(), nil
} }

View File

@ -154,7 +154,7 @@ func TestAuthorizeOAuthApp(t *testing.T) {
ClientId: rapp.Id, ClientId: rapp.Id,
RedirectURI: rapp.CallbackUrls[0], RedirectURI: rapp.CallbackUrls[0],
Scope: "", Scope: "",
State: "123", State: "/oauthcallback?sesskey=abcd&other=123",
} }
uriResponse, _, err := apiClient.AuthorizeOAuthApp(context.Background(), authRequest) uriResponse, _, err := apiClient.AuthorizeOAuthApp(context.Background(), authRequest)
require.NoError(t, err) require.NoError(t, err)
@ -164,7 +164,11 @@ func TestAuthorizeOAuthApp(t *testing.T) {
// require no query parameter to have "?" // require no query parameter to have "?"
require.False(t, strings.Contains(ru.RawQuery, "?"), "should not malform query parameters") require.False(t, strings.Contains(ru.RawQuery, "?"), "should not malform query parameters")
require.NotEmpty(t, ru.Query().Get("code"), "authorization code not returned") require.NotEmpty(t, ru.Query().Get("code"), "authorization code not returned")
// test state is not encoded multiple times
require.Equal(t, ru.Query().Get("state"), authRequest.State, "returned state doesn't match") require.Equal(t, ru.Query().Get("state"), authRequest.State, "returned state doesn't match")
// test state is URL encoded at least once
require.Empty(t, ru.Query().Get("other"), "state's query parameters should not leak")
} }
func TestDeauthorizeOAuthApp(t *testing.T) { func TestDeauthorizeOAuthApp(t *testing.T) {