mirrors/mostlymatter
1
0
Форкнуть 0
Код Задачи Запросы на слияние Действия Пакеты Проекты Релизы Вики Активность

[MM-37557] Move error out of client4 response (#18101)

Просмотр исходного кода 
* Return an error seperately from Response

* Remove BuildErrorResponse

* Drop Response.Error from model/client4.go

* Migrate require.Nil checks

* Migrate require.NotNil checks

* More manual fixes

* Move error check out of CheckOKStatus and CheckCreatedStatus

* Move error check out of CheckForbiddenStatus

* Move error check out of CheckUnauthorizedStatus

* Move error check out of CheckNotFoundStatus

* Move error check out of CheckBadRequestStatus

* Move error check out of CheckNotImplementedStatus and CheckRequestEntityTooLargeStatus

* Move error check out of CheckInternalErrorStatus

* Move error check out of CheckServiceUnavailableStatus

* Remove error check from checkHTTPStatus

* Remove remaining references to Response.Error

* Check previously unchecked errors

* Manually fix compile and linter errors

* Return error in CreateWebSocket methods

* Return error instead of *AppError in DoApi methods

* Manually fix bad replacments

* Conistently return Response and error

* Use err instead of seperate bool return value to indicate success

* Reduce ussage of model.AppError in web/oauth_test.go

* Remove client4.Must

* Check error in buf.ReadFrom

* Fix failing tests
Этот коммит содержится в:
Ben Schumacher
2021-08-13 13:12:16 +02:00
коммит произвёл GitHub
родитель 96593580ae
Коммит a8ca5c423f
60 изменённых файлов: 9790 добавлений и 8706 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

180
web/oauth_test.go
Просмотреть файл

@@ -82,8 +82,8 @@ func TestAuthorizeOAuthApp(t *testing.T) {
}
// Test auth code flow
ruri, resp := ApiClient.AuthorizeOAuthApp(authRequest)
require.Nil(t, resp.Error)
ruri, _, err := ApiClient.AuthorizeOAuthApp(authRequest)
require.NoError(t, err)
require.NotEmpty(t, ruri, "redirect url should be set")
@@ -94,8 +94,8 @@ func TestAuthorizeOAuthApp(t *testing.T) {
// Test implicit flow
authRequest.ResponseType = model.ImplicitResponseType
ruri, resp = ApiClient.AuthorizeOAuthApp(authRequest)
require.Nil(t, resp.Error)
ruri, _, err = ApiClient.AuthorizeOAuthApp(authRequest)
require.NoError(t, err)
require.False(t, ruri == "", "redirect url should be set")
ru, _ = url.Parse(ruri)
@@ -107,44 +107,40 @@ func TestAuthorizeOAuthApp(t *testing.T) {
oldToken := ApiClient.AuthToken
ApiClient.AuthToken = values.Get("access_token")
_, resp = ApiClient.AuthorizeOAuthApp(authRequest)
_, resp, err := ApiClient.AuthorizeOAuthApp(authRequest)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
ApiClient.AuthToken = oldToken
authRequest.RedirectUri = ""
_, resp = ApiClient.AuthorizeOAuthApp(authRequest)
_, resp, err = ApiClient.AuthorizeOAuthApp(authRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
authRequest.RedirectUri = "http://somewhereelse.com"
_, resp = ApiClient.AuthorizeOAuthApp(authRequest)
_, resp, err = ApiClient.AuthorizeOAuthApp(authRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
authRequest.RedirectUri = rapp.CallbackUrls[0]
authRequest.ResponseType = ""
_, resp = ApiClient.AuthorizeOAuthApp(authRequest)
_, resp, err = ApiClient.AuthorizeOAuthApp(authRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
authRequest.ResponseType = model.AuthCodeResponseType
authRequest.ClientId = ""
_, resp = ApiClient.AuthorizeOAuthApp(authRequest)
_, resp, err = ApiClient.AuthorizeOAuthApp(authRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
authRequest.ClientId = model.NewId()
_, resp = ApiClient.AuthorizeOAuthApp(authRequest)
_, resp, err = ApiClient.AuthorizeOAuthApp(authRequest)
require.Error(t, err)
CheckNotFoundStatus(t, resp)
}
func TestNilAuthorizeOAuthApp(t *testing.T) {
th := Setup(t).InitBasic()
th.Login(ApiClient, th.SystemAdminUser)
defer th.TearDown()
_, resp := ApiClient.AuthorizeOAuthApp(nil)
require.NotNil(t, resp.Error)
assert.Equal(t, "api.context.invalid_body_param.app_error", resp.Error.Id)
}
func TestDeauthorizeOAuthApp(t *testing.T) {
th := Setup(t).InitBasic()
th.Login(ApiClient, th.SystemAdminUser)
@@ -175,22 +171,22 @@ func TestDeauthorizeOAuthApp(t *testing.T) {
State: "123",
}
_, resp := ApiClient.AuthorizeOAuthApp(authRequest)
require.Nil(t, resp.Error)
_, _, err := ApiClient.AuthorizeOAuthApp(authRequest)
require.NoError(t, err)
pass, resp := ApiClient.DeauthorizeOAuthApp(rapp.Id)
require.Nil(t, resp.Error)
_, err = ApiClient.DeauthorizeOAuthApp(rapp.Id)
require.NoError(t, err)
require.True(t, pass, "should have passed")
_, resp = ApiClient.DeauthorizeOAuthApp("junk")
resp, err := ApiClient.DeauthorizeOAuthApp("junk")
require.Error(t, err)
CheckBadRequestStatus(t, resp)
_, resp = ApiClient.DeauthorizeOAuthApp(model.NewId())
require.Nil(t, resp.Error)
_, err = ApiClient.DeauthorizeOAuthApp(model.NewId())
require.NoError(t, err)
th.Logout(ApiClient)
_, resp = ApiClient.DeauthorizeOAuthApp(rapp.Id)
resp, err = ApiClient.DeauthorizeOAuthApp(rapp.Id)
require.Error(t, err)
CheckUnauthorizedStatus(t, resp)
}
@@ -229,8 +225,8 @@ func TestOAuthAccessToken(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = false })
data := url.Values{"grant_type": []string{"junk"}, "client_id": []string{"12345678901234567890123456"}, "client_secret": []string{"12345678901234567890123456"}, "code": []string{"junk"}, "redirect_uri": []string{oauthApp.CallbackUrls[0]}}
_, resp := ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "should have failed - oauth providing turned off - response status code: %v", resp.StatusCode)
_, _, err := ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "should have failed - oauth providing turned off")
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
authRequest := &model.AuthorizeRequest{
@@ -241,48 +237,48 @@ func TestOAuthAccessToken(t *testing.T) {
State: "123",
}
redirect, resp := ApiClient.AuthorizeOAuthApp(authRequest)
require.Nil(t, resp.Error)
redirect, _, err := ApiClient.AuthorizeOAuthApp(authRequest)
require.NoError(t, err)
rurl, _ := url.Parse(redirect)
ApiClient.Logout()
data = url.Values{"grant_type": []string{"junk"}, "client_id": []string{oauthApp.Id}, "client_secret": []string{oauthApp.ClientSecret}, "code": []string{rurl.Query().Get("code")}, "redirect_uri": []string{oauthApp.CallbackUrls[0]}}
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "should have failed - bad grant type")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "should have failed - bad grant type")
data.Set("grant_type", model.AccessTokenGrantType)
data.Set("client_id", "")
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "should have failed - missing client id")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "should have failed - missing client id")
data.Set("client_id", "junk")
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "should have failed - bad client id")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "should have failed - bad client id")
data.Set("client_id", oauthApp.Id)
data.Set("client_secret", "")
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "should have failed - missing client secret")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "should have failed - missing client secret")
data.Set("client_secret", "junk")
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "should have failed - bad client secret")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "should have failed - bad client secret")
data.Set("client_secret", oauthApp.ClientSecret)
data.Set("code", "")
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "should have failed - missing code")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "should have failed - missing code")
data.Set("code", "junk")
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "should have failed - bad code")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "should have failed - bad code")
data.Set("code", rurl.Query().Get("code"))
data.Set("redirect_uri", "junk")
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "should have failed - non-matching redirect uri")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "should have failed - non-matching redirect uri")
// reset data for successful request
data.Set("grant_type", model.AccessTokenGrantType)
@@ -293,30 +289,30 @@ func TestOAuthAccessToken(t *testing.T) {
token := ""
refreshToken := ""
rsp, resp := ApiClient.GetOAuthAccessToken(data)
require.Nil(t, resp.Error)
rsp, _, err := ApiClient.GetOAuthAccessToken(data)
require.NoError(t, err)
require.NotEmpty(t, rsp.AccessToken, "access token not returned")
require.NotEmpty(t, rsp.RefreshToken, "refresh token not returned")
token, refreshToken = rsp.AccessToken, rsp.RefreshToken
require.Equal(t, rsp.TokenType, model.AccessTokenType, "access token type incorrect")
_, err := ApiClient.DoApiGet("/oauth_test", "")
require.Nil(t, err)
_, err = ApiClient.DoApiGet("/oauth_test", "")
require.NoError(t, err)
ApiClient.SetOAuthToken("")
_, err = ApiClient.DoApiGet("/oauth_test", "")
require.NotNil(t, err, "should have failed - no access token provided")
require.Error(t, err, "should have failed - no access token provided")
ApiClient.SetOAuthToken("badtoken")
_, err = ApiClient.DoApiGet("/oauth_test", "")
require.NotNil(t, err, "should have failed - bad token provided")
require.Error(t, err, "should have failed - bad token provided")
ApiClient.SetOAuthToken(token)
_, err = ApiClient.DoApiGet("/oauth_test", "")
require.Nil(t, err)
require.NoError(t, err)
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "should have failed - tried to reuse auth code")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "should have failed - tried to reuse auth code")
data.Set("grant_type", model.RefreshTokenGrantType)
data.Set("client_id", oauthApp.Id)
@@ -324,12 +320,12 @@ func TestOAuthAccessToken(t *testing.T) {
data.Set("refresh_token", "")
data.Set("redirect_uri", oauthApp.CallbackUrls[0])
data.Del("code")
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "Should have failed - refresh token empty")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "Should have failed - refresh token empty")
data.Set("refresh_token", refreshToken)
rsp, resp = ApiClient.GetOAuthAccessToken(data)
require.Nil(t, resp.Error)
rsp, _, err = ApiClient.GetOAuthAccessToken(data)
require.NoError(t, err)
require.NotEmpty(t, rsp.AccessToken, "access token not returned")
require.NotEmpty(t, rsp.RefreshToken, "refresh token not returned")
require.NotEqual(t, rsp.RefreshToken, refreshToken, "refresh token did not update")
@@ -337,11 +333,11 @@ func TestOAuthAccessToken(t *testing.T) {
ApiClient.SetOAuthToken(rsp.AccessToken)
_, err = ApiClient.DoApiGet("/oauth_test", "")
require.Nil(t, err)
require.NoError(t, err)
data.Set("refresh_token", rsp.RefreshToken)
rsp, resp = ApiClient.GetOAuthAccessToken(data)
require.Nil(t, resp.Error)
rsp, _, err = ApiClient.GetOAuthAccessToken(data)
require.NoError(t, err)
require.NotEmpty(t, rsp.AccessToken, "access token not returned")
require.NotEmpty(t, rsp.RefreshToken, "refresh token not returned")
require.NotEqual(t, rsp.RefreshToken, refreshToken, "refresh token did not update")
@@ -349,11 +345,11 @@ func TestOAuthAccessToken(t *testing.T) {
ApiClient.SetOAuthToken(rsp.AccessToken)
_, err = ApiClient.DoApiGet("/oauth_test", "")
require.Nil(t, err)
require.NoError(t, err)
authData := &model.AuthData{ClientId: oauthApp.Id, RedirectUri: oauthApp.CallbackUrls[0], UserId: th.BasicUser.Id, Code: model.NewId(), ExpiresIn: -1}
_, nErr := th.App.Srv().Store.OAuth().SaveAuthData(authData)
require.NoError(t, nErr)
_, err = th.App.Srv().Store.OAuth().SaveAuthData(authData)
require.NoError(t, err)
data.Set("grant_type", model.AccessTokenGrantType)
data.Set("client_id", oauthApp.Id)
@@ -361,8 +357,8 @@ func TestOAuthAccessToken(t *testing.T) {
data.Set("redirect_uri", oauthApp.CallbackUrls[0])
data.Set("code", authData.Code)
data.Del("refresh_token")
_, resp = ApiClient.GetOAuthAccessToken(data)
require.NotNil(t, resp.Error, "Should have failed - code is expired")
_, _, err = ApiClient.GetOAuthAccessToken(data)
require.Error(t, err, "Should have failed - code is expired")
ApiClient.ClearOAuthToken()
}
@@ -440,12 +436,12 @@ func TestOAuthComplete(t *testing.T) {
}()
r, err := HTTPGet(ApiClient.Url+"/login/gitlab/complete?code=123", ApiClient.HTTPClient, "", true)
assert.NotNil(t, err)
assert.Error(t, err)
closeBody(r)
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GitLabSettings.Enable = true })
r, err = HTTPGet(ApiClient.Url+"/login/gitlab/complete?code=123&state=!#$#F@#Yˆ&~ñ", ApiClient.HTTPClient, "", true)
assert.NotNil(t, err)
assert.Error(t, err)
closeBody(r)
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GitLabSettings.AuthEndpoint = ApiClient.Url + "/oauth/authorize" })
@@ -458,13 +454,13 @@ func TestOAuthComplete(t *testing.T) {
state := base64.StdEncoding.EncodeToString([]byte(model.MapToJson(stateProps)))
r, err = HTTPGet(ApiClient.Url+"/login/gitlab/complete?code=123&state="+url.QueryEscape(state), ApiClient.HTTPClient, "", true)
assert.NotNil(t, err)
assert.Error(t, err)
closeBody(r)
stateProps["hash"] = utils.HashSha256(*th.App.Config().GitLabSettings.Id)
state = base64.StdEncoding.EncodeToString([]byte(model.MapToJson(stateProps)))
r, err = HTTPGet(ApiClient.Url+"/login/gitlab/complete?code=123&state="+url.QueryEscape(state), ApiClient.HTTPClient, "", true)
assert.NotNil(t, err)
assert.Error(t, err)
closeBody(r)
// We are going to use mattermost as the provider emulating gitlab
@@ -507,8 +503,8 @@ func TestOAuthComplete(t *testing.T) {
State: "123",
}
redirect, resp := ApiClient.AuthorizeOAuthApp(authRequest)
require.Nil(t, resp.Error)
redirect, _, err := ApiClient.AuthorizeOAuthApp(authRequest)
require.NoError(t, err)
rurl, _ := url.Parse(redirect)
code := rurl.Query().Get("code")
@@ -525,8 +521,8 @@ func TestOAuthComplete(t *testing.T) {
einterfaces.RegisterOAuthProvider(model.ServiceGitlab, provider)
redirect, resp = ApiClient.AuthorizeOAuthApp(authRequest)
require.Nil(t, resp.Error)
redirect, _, err = ApiClient.AuthorizeOAuthApp(authRequest)
require.NoError(t, err)
rurl, _ = url.Parse(redirect)
code = rurl.Query().Get("code")
@@ -539,30 +535,30 @@ func TestOAuthComplete(t *testing.T) {
th.BasicUser.Id, model.ServiceGitlab, &th.BasicUser.Email, th.BasicUser.Email, true)
require.NoError(t, nErr)
redirect, resp = ApiClient.AuthorizeOAuthApp(authRequest)
require.Nil(t, resp.Error)
redirect, _, err = ApiClient.AuthorizeOAuthApp(authRequest)
require.NoError(t, err)
rurl, _ = url.Parse(redirect)
code = rurl.Query().Get("code")
stateProps["action"] = model.OAuthActionLogin
state = base64.StdEncoding.EncodeToString([]byte(model.MapToJson(stateProps)))
if r, err := HTTPGet(ApiClient.Url+"/login/"+model.ServiceGitlab+"/complete?code="+url.QueryEscape(code)+"&state="+url.QueryEscape(state), ApiClient.HTTPClient, "", false); err == nil {
if r, err = HTTPGet(ApiClient.Url+"/login/"+model.ServiceGitlab+"/complete?code="+url.QueryEscape(code)+"&state="+url.QueryEscape(state), ApiClient.HTTPClient, "", false); err == nil {
closeBody(r)
}
redirect, resp = ApiClient.AuthorizeOAuthApp(authRequest)
require.Nil(t, resp.Error)
redirect, _, err = ApiClient.AuthorizeOAuthApp(authRequest)
require.NoError(t, err)
rurl, _ = url.Parse(redirect)
code = rurl.Query().Get("code")
delete(stateProps, "action")
state = base64.StdEncoding.EncodeToString([]byte(model.MapToJson(stateProps)))
if r, err := HTTPGet(ApiClient.Url+"/login/"+model.ServiceGitlab+"/complete?code="+url.QueryEscape(code)+"&state="+url.QueryEscape(state), ApiClient.HTTPClient, "", false); err == nil {
if r, err = HTTPGet(ApiClient.Url+"/login/"+model.ServiceGitlab+"/complete?code="+url.QueryEscape(code)+"&state="+url.QueryEscape(state), ApiClient.HTTPClient, "", false); err == nil {
closeBody(r)
}
redirect, resp = ApiClient.AuthorizeOAuthApp(authRequest)
require.Nil(t, resp.Error)
redirect, _, err = ApiClient.AuthorizeOAuthApp(authRequest)
require.NoError(t, err)
rurl, _ = url.Parse(redirect)
code = rurl.Query().Get("code")
@@ -612,7 +608,7 @@ func TestOAuthComplete_ErrorMessages(t *testing.T) {
assert.Contains(t, responseWriter.Body.String(), "<!-- mobile app message -->")
}
func HTTPGet(url string, httpClient *http.Client, authToken string, followRedirect bool) (*http.Response, *model.AppError) {
func HTTPGet(url string, httpClient *http.Client, authToken string, followRedirect bool) (*http.Response, error) {
rq, _ := http.NewRequest("GET", url, nil)
rq.Close = true
@@ -627,7 +623,7 @@ func HTTPGet(url string, httpClient *http.Client, authToken string, followRedire
}
if rp, err := httpClient.Do(rq); err != nil {
return nil, model.NewAppError(url, "model.client.connecting.app_error", nil, err.Error(), 0)
return nil, err
} else if rp.StatusCode == 304 {
return rp, nil
} else if rp.StatusCode == 307 {
@@ -675,11 +671,9 @@ func GenerateTestAppName() string {
func checkHTTPStatus(t *testing.T, resp *model.Response, expectedStatus int) {
t.Helper()
require.NotNil(t, resp, "Unexpected nil response, expected http:%v, expectError:%v)", expectedStatus, true)
require.NotNilf(t, resp, "Unexpected nil response, expected http status:%v", expectedStatus)
require.NotNil(t, resp.Error, "Expected a non-nil error and http status:%v, got nil, %v", expectedStatus, resp.StatusCode)
require.Equal(t, resp.StatusCode, expectedStatus, "Expected http status:%v, got %v (err: %q)", expectedStatus, resp.StatusCode, resp.Error)
require.Equalf(t, expectedStatus, resp.StatusCode, "Expected http status:%v, got %v", expectedStatus, resp.StatusCode)
}
func CheckForbiddenStatus(t *testing.T, resp *model.Response) {