From d6715c76012bf9faec9361a1f3aca35f2f4794ec Mon Sep 17 00:00:00 2001 From: Yang Luo Date: Sun, 28 Mar 2021 00:48:34 +0800 Subject: [PATCH] Improve API error handling. --- controllers/application.go | 6 +++--- controllers/base.go | 8 ++++++++ controllers/organization.go | 6 +++--- controllers/provider.go | 6 +++--- controllers/token.go | 6 +++--- controllers/type.go | 21 --------------------- controllers/user.go | 6 +++--- object/application.go | 5 ++--- object/organization.go | 5 ++--- object/provider.go | 5 ++--- object/token.go | 5 ++--- object/user.go | 5 ++--- routers/authz_filter.go | 5 ++++- web/src/ApplicationEditPage.js | 8 ++++---- web/src/OrganizationEditPage.js | 6 +++--- web/src/ProviderEditPage.js | 6 +++--- web/src/TokenEditPage.js | 6 +++--- web/src/UserEditPage.js | 8 ++++---- 18 files changed, 54 insertions(+), 69 deletions(-) delete mode 100644 controllers/type.go diff --git a/controllers/application.go b/controllers/application.go index 591015fc..cb15dcb0 100644 --- a/controllers/application.go +++ b/controllers/application.go @@ -43,7 +43,7 @@ func (c *ApiController) UpdateApplication() { panic(err) } - c.Data["json"] = object.UpdateApplication(id, &application) + c.Data["json"] = wrapActionResponse(object.UpdateApplication(id, &application)) c.ServeJSON() } @@ -54,7 +54,7 @@ func (c *ApiController) AddApplication() { panic(err) } - c.Data["json"] = object.AddApplication(&application) + c.Data["json"] = wrapActionResponse(object.AddApplication(&application)) c.ServeJSON() } @@ -65,6 +65,6 @@ func (c *ApiController) DeleteApplication() { panic(err) } - c.Data["json"] = object.DeleteApplication(&application) + c.Data["json"] = wrapActionResponse(object.DeleteApplication(&application)) c.ServeJSON() } diff --git a/controllers/base.go b/controllers/base.go index b53f98a7..d26f4fc4 100644 --- a/controllers/base.go +++ b/controllers/base.go @@ -32,3 +32,11 @@ func (c *ApiController) GetSessionUser() string { func (c *ApiController) SetSessionUser(user string) { c.SetSession("username", user) } + +func wrapActionResponse(affected bool) *Response { + if affected { + return &Response{Status: "ok", Msg: "", Data: "affected"} + } else { + return &Response{Status: "ok", Msg: "", Data: "unaffected"} + } +} diff --git a/controllers/organization.go b/controllers/organization.go index 7aad45fb..ef1cf971 100644 --- a/controllers/organization.go +++ b/controllers/organization.go @@ -43,7 +43,7 @@ func (c *ApiController) UpdateOrganization() { panic(err) } - c.Data["json"] = object.UpdateOrganization(id, &organization) + c.Data["json"] = wrapActionResponse(object.UpdateOrganization(id, &organization)) c.ServeJSON() } @@ -54,7 +54,7 @@ func (c *ApiController) AddOrganization() { panic(err) } - c.Data["json"] = object.AddOrganization(&organization) + c.Data["json"] = wrapActionResponse(object.AddOrganization(&organization)) c.ServeJSON() } @@ -65,6 +65,6 @@ func (c *ApiController) DeleteOrganization() { panic(err) } - c.Data["json"] = object.DeleteOrganization(&organization) + c.Data["json"] = wrapActionResponse(object.DeleteOrganization(&organization)) c.ServeJSON() } diff --git a/controllers/provider.go b/controllers/provider.go index cdad47d3..72f6f3d9 100644 --- a/controllers/provider.go +++ b/controllers/provider.go @@ -43,7 +43,7 @@ func (c *ApiController) UpdateProvider() { panic(err) } - c.Data["json"] = object.UpdateProvider(id, &provider) + c.Data["json"] = wrapActionResponse(object.UpdateProvider(id, &provider)) c.ServeJSON() } @@ -54,7 +54,7 @@ func (c *ApiController) AddProvider() { panic(err) } - c.Data["json"] = object.AddProvider(&provider) + c.Data["json"] = wrapActionResponse(object.AddProvider(&provider)) c.ServeJSON() } @@ -65,6 +65,6 @@ func (c *ApiController) DeleteProvider() { panic(err) } - c.Data["json"] = object.DeleteProvider(&provider) + c.Data["json"] = wrapActionResponse(object.DeleteProvider(&provider)) c.ServeJSON() } diff --git a/controllers/token.go b/controllers/token.go index 7950a7e6..84cc1ed7 100644 --- a/controllers/token.go +++ b/controllers/token.go @@ -43,7 +43,7 @@ func (c *ApiController) UpdateToken() { panic(err) } - c.Data["json"] = object.UpdateToken(id, &token) + c.Data["json"] = wrapActionResponse(object.UpdateToken(id, &token)) c.ServeJSON() } @@ -54,7 +54,7 @@ func (c *ApiController) AddToken() { panic(err) } - c.Data["json"] = object.AddToken(&token) + c.Data["json"] = wrapActionResponse(object.AddToken(&token)) c.ServeJSON() } @@ -65,7 +65,7 @@ func (c *ApiController) DeleteToken() { panic(err) } - c.Data["json"] = object.DeleteToken(&token) + c.Data["json"] = wrapActionResponse(object.DeleteToken(&token)) c.ServeJSON() } diff --git a/controllers/type.go b/controllers/type.go deleted file mode 100644 index 356c2b01..00000000 --- a/controllers/type.go +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2021 The casbin Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package controllers - -type authResponse struct { - Email string `json:"email"` - Avatar string `json:"avatar"` - Method string `json:"method"` -} diff --git a/controllers/user.go b/controllers/user.go index e63f415c..ad125f80 100644 --- a/controllers/user.go +++ b/controllers/user.go @@ -48,7 +48,7 @@ func (c *ApiController) UpdateUser() { panic(err) } - c.Data["json"] = object.UpdateUser(id, &user) + c.Data["json"] = wrapActionResponse(object.UpdateUser(id, &user)) c.ServeJSON() } @@ -59,7 +59,7 @@ func (c *ApiController) AddUser() { panic(err) } - c.Data["json"] = object.AddUser(&user) + c.Data["json"] = wrapActionResponse(object.AddUser(&user)) c.ServeJSON() } @@ -70,6 +70,6 @@ func (c *ApiController) DeleteUser() { panic(err) } - c.Data["json"] = object.DeleteUser(&user) + c.Data["json"] = wrapActionResponse(object.DeleteUser(&user)) c.ServeJSON() } diff --git a/object/application.go b/object/application.go index 1caf2ea5..d0fa8c62 100644 --- a/object/application.go +++ b/object/application.go @@ -106,13 +106,12 @@ func UpdateApplication(id string, application *Application) bool { return false } - _, err := adapter.engine.ID(core.PK{owner, name}).AllCols().Update(application) + affected, err := adapter.engine.ID(core.PK{owner, name}).AllCols().Update(application) if err != nil { panic(err) } - //return affected != 0 - return true + return affected != 0 } func AddApplication(application *Application) bool { diff --git a/object/organization.go b/object/organization.go index 6118a1b5..a22c7572 100644 --- a/object/organization.go +++ b/object/organization.go @@ -63,13 +63,12 @@ func UpdateOrganization(id string, organization *Organization) bool { return false } - _, err := adapter.engine.ID(core.PK{owner, name}).AllCols().Update(organization) + affected, err := adapter.engine.ID(core.PK{owner, name}).AllCols().Update(organization) if err != nil { panic(err) } - //return affected != 0 - return true + return affected != 0 } func AddOrganization(organization *Organization) bool { diff --git a/object/provider.go b/object/provider.go index e73f7779..1897bc47 100644 --- a/object/provider.go +++ b/object/provider.go @@ -66,13 +66,12 @@ func UpdateProvider(id string, provider *Provider) bool { return false } - _, err := adapter.engine.ID(core.PK{owner, name}).AllCols().Update(provider) + affected, err := adapter.engine.ID(core.PK{owner, name}).AllCols().Update(provider) if err != nil { panic(err) } - //return affected != 0 - return true + return affected != 0 } func AddProvider(provider *Provider) bool { diff --git a/object/token.go b/object/token.go index fbc2662e..9544575c 100644 --- a/object/token.go +++ b/object/token.go @@ -96,13 +96,12 @@ func UpdateToken(id string, token *Token) bool { return false } - _, err := adapter.engine.ID(core.PK{owner, name}).AllCols().Update(token) + affected, err := adapter.engine.ID(core.PK{owner, name}).AllCols().Update(token) if err != nil { panic(err) } - //return affected != 0 - return true + return affected != 0 } func AddToken(token *Token) bool { diff --git a/object/user.go b/object/user.go index 83ed2174..56ca9f7e 100644 --- a/object/user.go +++ b/object/user.go @@ -97,13 +97,12 @@ func UpdateUser(id string, user *User) bool { return false } - _, err := adapter.engine.ID(core.PK{owner, name}).AllCols().Update(user) + affected, err := adapter.engine.ID(core.PK{owner, name}).AllCols().Update(user) if err != nil { panic(err) } - //return affected != 0 - return true + return affected != 0 } func AddUser(user *User) bool { diff --git a/routers/authz_filter.go b/routers/authz_filter.go index e5526cb2..82e38947 100644 --- a/routers/authz_filter.go +++ b/routers/authz_filter.go @@ -22,6 +22,8 @@ import ( "github.com/astaxie/beego/context" "github.com/casdoor/casdoor/authz" + "github.com/casdoor/casdoor/controllers" + "github.com/casdoor/casdoor/util" ) type Object struct { @@ -91,7 +93,8 @@ func getObject(ctx *context.Context) (string, string) { func denyRequest(ctx *context.Context) { w := ctx.ResponseWriter w.WriteHeader(403) - _, err := w.Write([]byte("403 Forbidden\n")) + resp := &controllers.Response{Status: "error", Msg: "unauthorized operation"} + _, err := w.Write([]byte(util.StructToJson(resp))) if err != nil { panic(err) } diff --git a/web/src/ApplicationEditPage.js b/web/src/ApplicationEditPage.js index 38faf2a0..11bd410d 100644 --- a/web/src/ApplicationEditPage.js +++ b/web/src/ApplicationEditPage.js @@ -56,7 +56,7 @@ class ApplicationEditPage extends React.Component { OrganizationBackend.getOrganizations("admin") .then((res) => { this.setState({ - organizations: res, + organizations: (res.msg === undefined) ? res : [], }); }); } @@ -277,19 +277,19 @@ class ApplicationEditPage extends React.Component { let application = Setting.deepCopy(this.state.application); ApplicationBackend.updateApplication(this.state.application.owner, this.state.applicationName, application) .then((res) => { - if (res) { + if (res.msg === "") { Setting.showMessage("success", `Successfully saved`); this.setState({ applicationName: this.state.application.name, }); this.props.history.push(`/applications/${this.state.application.name}`); } else { - Setting.showMessage("error", `failed to save: server side failure`); + Setting.showMessage("error", res.msg); this.updateApplicationField('name', this.state.applicationName); } }) .catch(error => { - Setting.showMessage("error", `failed to save: ${error}`); + Setting.showMessage("error", `failed to connect to server: ${error}`); }); } diff --git a/web/src/OrganizationEditPage.js b/web/src/OrganizationEditPage.js index a914185f..9b9d8f89 100644 --- a/web/src/OrganizationEditPage.js +++ b/web/src/OrganizationEditPage.js @@ -104,19 +104,19 @@ class OrganizationEditPage extends React.Component { let organization = Setting.deepCopy(this.state.organization); OrganizationBackend.updateOrganization(this.state.organization.owner, this.state.organizationName, organization) .then((res) => { - if (res) { + if (res.msg === "") { Setting.showMessage("success", `Successfully saved`); this.setState({ organizationName: this.state.organization.name, }); this.props.history.push(`/organizations/${this.state.organization.name}`); } else { - Setting.showMessage("error", `failed to save: server side failure`); + Setting.showMessage("error", res.msg); this.updateOrganizationField('name', this.state.organizationName); } }) .catch(error => { - Setting.showMessage("error", `failed to save: ${error}`); + Setting.showMessage("error", `failed to connect to server: ${error}`); }); } diff --git a/web/src/ProviderEditPage.js b/web/src/ProviderEditPage.js index 851ed71b..20232276 100644 --- a/web/src/ProviderEditPage.js +++ b/web/src/ProviderEditPage.js @@ -143,19 +143,19 @@ class ProviderEditPage extends React.Component { let provider = Setting.deepCopy(this.state.provider); ProviderBackend.updateProvider(this.state.provider.owner, this.state.providerName, provider) .then((res) => { - if (res) { + if (res.msg === "") { Setting.showMessage("success", `Successfully saved`); this.setState({ providerName: this.state.provider.name, }); this.props.history.push(`/providers/${this.state.provider.name}`); } else { - Setting.showMessage("error", `failed to save: server side failure`); + Setting.showMessage("error", res.msg); this.updateProviderField('name', this.state.providerName); } }) .catch(error => { - Setting.showMessage("error", `failed to save: ${error}`); + Setting.showMessage("error", `failed to connect to server: ${error}`); }); } diff --git a/web/src/TokenEditPage.js b/web/src/TokenEditPage.js index 3d2c7956..5ed5c8a9 100644 --- a/web/src/TokenEditPage.js +++ b/web/src/TokenEditPage.js @@ -144,19 +144,19 @@ class TokenEditPage extends React.Component { let token = Setting.deepCopy(this.state.token); TokenBackend.updateToken(this.state.token.owner, this.state.tokenName, token) .then((res) => { - if (res) { + if (res.msg === "") { Setting.showMessage("success", `Successfully saved`); this.setState({ tokenName: this.state.token.name, }); this.props.history.push(`/tokens/${this.state.token.name}`); } else { - Setting.showMessage("error", `failed to save: server side failure`); + Setting.showMessage("error", res.msg); this.updateTokenField('name', this.state.tokenName); } }) .catch(error => { - Setting.showMessage("error", `failed to save: ${error}`); + Setting.showMessage("error", `failed to connect to server: ${error}`); }); } diff --git a/web/src/UserEditPage.js b/web/src/UserEditPage.js index 6b7ad37c..79751b0b 100644 --- a/web/src/UserEditPage.js +++ b/web/src/UserEditPage.js @@ -53,7 +53,7 @@ class UserEditPage extends React.Component { OrganizationBackend.getOrganizations("admin") .then((res) => { this.setState({ - organizations: res, + organizations: (res.msg === undefined) ? res : [], }); }); } @@ -266,7 +266,7 @@ class UserEditPage extends React.Component { let user = Setting.deepCopy(this.state.user); UserBackend.updateUser(this.state.organizationName, this.state.userName, user) .then((res) => { - if (res) { + if (res.msg === "") { Setting.showMessage("success", `Successfully saved`); this.setState({ organizationName: this.state.user.owner, @@ -277,13 +277,13 @@ class UserEditPage extends React.Component { this.props.history.push(`/users/${this.state.user.owner}/${this.state.user.name}`); } } else { - Setting.showMessage("error", `failed to save: server side failure`); + Setting.showMessage("error", res.msg); this.updateUserField('owner', this.state.organizationName); this.updateUserField('name', this.state.userName); } }) .catch(error => { - Setting.showMessage("error", `failed to save: ${error}`); + Setting.showMessage("error", `failed to connect to server: ${error}`); }); }