feat: refactor LDAP backend code and improve frontend operation (#1640)

* refactor: simplify ldap backend code and improve frontend operation

* chore: add skipCi tag in sync_test.go

* fix: ui

controllers/ldap.go

@@ -21,14 +21,6 @@ import (
 	"github.com/casdoor/casdoor/util"
 )
 
-type LdapServer struct {
-	Host   string `json:"host"`
-	Port   int    `json:"port"`
-	Admin  string `json:"admin"`
-	Passwd string `json:"passwd"`
-	BaseDn string `json:"baseDn"`
-}
-
 type LdapResp struct {
 	// Groups []LdapRespGroup `json:"groups"`
 	Users []object.LdapRespUser `json:"users"`
@@ -44,19 +36,15 @@ type LdapSyncResp struct {
 	Failed []object.LdapRespUser `json:"failed"`
 }
 
-// GetLdapUser
+// GetLdapUsers
 // @Tag Account API
 // @Title GetLdapser
-// @router /get-ldap-user [post]
+// @router /get-ldap-users [get]
-func (c *ApiController) GetLdapUser() {
+func (c *ApiController) GetLdapUsers() {
-	ldapServer := LdapServer{}
+	id := c.Input().Get("id")
-	err := json.Unmarshal(c.Ctx.Input.RequestBody, &ldapServer)
-	if err != nil || util.IsStringsEmpty(ldapServer.Host, ldapServer.Admin, ldapServer.Passwd, ldapServer.BaseDn) {
-		c.ResponseError(c.T("general:Missing parameter"))
-		return
-	}
 
-	var resp LdapResp
+	_, ldapId := util.GetOwnerAndNameFromId(id)
+	ldapServer := object.GetLdap(ldapId)
 
 	conn, err := object.GetLdapConn(ldapServer.Host, ldapServer.Port, ldapServer.Admin, ldapServer.Passwd)
 	if err != nil {
@@ -83,6 +71,8 @@ func (c *ApiController) GetLdapUser() {
 		return
 	}
 
+	var resp LdapResp
+	uuids := make([]string, len(users))
 	for _, user := range users {
 		resp.Users = append(resp.Users, object.LdapRespUser{
 			UidNumber: user.UidNumber,
@@ -95,9 +85,12 @@ func (c *ApiController) GetLdapUser() {
 			Phone:   util.GetMaxLenStr(user.TelephoneNumber, user.Mobile, user.MobileTelephoneNumber),
 			Address: util.GetMaxLenStr(user.RegisteredAddress, user.PostalAddress),
 		})
+		uuids = append(uuids, user.Uuid)
 	}
 
-	c.ResponseOk(resp)
+	existUuids := object.CheckLdapUuidExist(ldapServer.Owner, uuids)
+
+	c.ResponseOk(resp, existUuids)
 }
 
 // GetLdaps
@@ -134,7 +127,7 @@ func (c *ApiController) AddLdap() {
 	var ldap object.Ldap
 	err := json.Unmarshal(c.Ctx.Input.RequestBody, &ldap)
 	if err != nil {
-		c.ResponseError(c.T("general:Missing parameter"))
+		c.ResponseError(err.Error())
 		return
 	}
 
@@ -150,14 +143,14 @@ func (c *ApiController) AddLdap() {
 
 	affected := object.AddLdap(&ldap)
 	resp := wrapActionResponse(affected)
-	if affected {
+	resp.Data2 = ldap
-		resp.Data2 = ldap
+
-	}
 	if ldap.AutoSync != 0 {
 		object.GetLdapAutoSynchronizer().StartAutoSync(ldap.Id)
 	}
 
-	c.ResponseOk(resp)
+	c.Data["json"] = resp
+	c.ServeJSON()
 }
 
 // UpdateLdap
@@ -174,17 +167,15 @@ func (c *ApiController) UpdateLdap() {
 
 	prevLdap := object.GetLdap(ldap.Id)
 	affected := object.UpdateLdap(&ldap)
-	resp := wrapActionResponse(affected)
+
-	if affected {
-		resp.Data2 = ldap
-	}
 	if ldap.AutoSync != 0 {
 		object.GetLdapAutoSynchronizer().StartAutoSync(ldap.Id)
 	} else if ldap.AutoSync == 0 && prevLdap.AutoSync != 0 {
 		object.GetLdapAutoSynchronizer().StopAutoSync(ldap.Id)
 	}
 
-	c.ResponseOk(resp)
+	c.Data["json"] = wrapActionResponse(affected)
+	c.ServeJSON()
 }
 
 // DeleteLdap
@@ -199,8 +190,12 @@ func (c *ApiController) DeleteLdap() {
 		return
 	}
 
+	affected := object.DeleteLdap(&ldap)
+
 	object.GetLdapAutoSynchronizer().StopAutoSync(ldap.Id)
-	c.ResponseOk(wrapActionResponse(object.DeleteLdap(&ldap)))
+
+	c.Data["json"] = wrapActionResponse(affected)
+	c.ServeJSON()
 }
 
 // SyncLdapUsers
@@ -226,20 +221,3 @@ func (c *ApiController) SyncLdapUsers() {
 		Failed: *failed,
 	})
 }
-
-// CheckLdapUsersExist
-// @Tag Account API
-// @Title CheckLdapUserExist
-// @router /check-ldap-users-exist [post]
-func (c *ApiController) CheckLdapUsersExist() {
-	owner := c.Input().Get("owner")
-	var uuids []string
-	err := json.Unmarshal(c.Ctx.Input.RequestBody, &uuids)
-	if err != nil {
-		c.ResponseError(err.Error())
-		return
-	}
-
-	exist := object.CheckLdapUuidExist(owner, uuids)
-	c.ResponseOk(exist)
-}

object/ldap.go

@@ -160,12 +160,12 @@ func GetLdapConn(host string, port int, adminUser string, adminPasswd string) (*
 
 	err = conn.Bind(adminUser, adminPasswd)
 	if err != nil {
-		return nil, fmt.Errorf("fail to login Ldap server with [%s]", adminUser)
+		return nil, err
 	}
 
 	isAD, err := isMicrosoftAD(conn)
 	if err != nil {
-		return nil, fmt.Errorf("fail to get Ldap server type [%s]", adminUser)
+		return nil, err
 	}
 	return &ldapConn{Conn: conn, IsAD: isAD}, nil
 }

routers/router.go

@@ -118,13 +118,12 @@ func initAPI() {
 	beego.Router("/api/reset-email-or-phone", &controllers.ApiController{}, "POST:ResetEmailOrPhone")
 	beego.Router("/api/get-captcha", &controllers.ApiController{}, "GET:GetCaptcha")
 
-	beego.Router("/api/get-ldap-user", &controllers.ApiController{}, "POST:GetLdapUser")
+	beego.Router("/api/get-ldap-users", &controllers.ApiController{}, "GET:GetLdapUsers")
 	beego.Router("/api/get-ldaps", &controllers.ApiController{}, "GET:GetLdaps")
 	beego.Router("/api/get-ldap", &controllers.ApiController{}, "GET:GetLdap")
 	beego.Router("/api/add-ldap", &controllers.ApiController{}, "POST:AddLdap")
 	beego.Router("/api/update-ldap", &controllers.ApiController{}, "POST:UpdateLdap")
 	beego.Router("/api/delete-ldap", &controllers.ApiController{}, "POST:DeleteLdap")
-	beego.Router("/api/check-ldap-users-exist", &controllers.ApiController{}, "POST:CheckLdapUsersExist")
 	beego.Router("/api/sync-ldap-users", &controllers.ApiController{}, "POST:SyncLdapUsers")
 
 	beego.Router("/api/get-providers", &controllers.ApiController{}, "GET:GetProviders")

swagger/swagger.json

@@ -636,14 +636,6 @@
                 }
             }
         },
-        "/api/check-ldap-users-exist": {
-            "post": {
-                "tags": [
-                    "Account API"
-                ],
-                "operationId": "ApiController.CheckLdapUserExist"
-            }
-        },
         "/api/check-user-password": {
             "post": {
                 "tags": [
@@ -1349,8 +1341,8 @@
                 "operationId": "ApiController.GetLdap"
             }
         },
-        "/api/get-ldap-user": {
+        "/api/get-ldap-users": {
-            "post": {
+            "get": {
                 "tags": [
                     "Account API"
                 ],
@@ -1835,20 +1827,6 @@
                 }
             }
         },
-        "/api/get-release": {
-            "get": {
-                "tags": [
-                    "System API"
-                ],
-                "description": "get local github repo's latest release version info",
-                "operationId": "ApiController.GitRepoVersion",
-                "responses": {
-                    "200": {
-                        "description": "{string} local latest version hash of casdoor"
-                    }
-                }
-            }
-        },
         "/api/get-resource": {
             "get": {
                 "tags": [
@@ -2340,6 +2318,20 @@
                 }
             }
         },
+        "/api/get-version-info": {
+            "get": {
+                "tags": [
+                    "System API"
+                ],
+                "description": "get local git repo's latest release version info",
+                "operationId": "ApiController.GetVersionInfo",
+                "responses": {
+                    "200": {
+                        "description": "{string} local latest version hash of Casdoor"
+                    }
+                }
+            }
+        },
         "/api/get-webhook": {
             "get": {
                 "tags": [
@@ -3635,11 +3627,11 @@
         }
     },
     "definitions": {
-        "2346.0xc000278ab0.false": {
+        "2268.0xc000528cf0.false": {
             "title": "false",
             "type": "object"
         },
-        "2381.0xc000278ae0.false": {
+        "2302.0xc000528d20.false": {
             "title": "false",
             "type": "object"
         },
@@ -3766,10 +3758,10 @@
             "type": "object",
             "properties": {
                 "data": {
-                    "$ref": "#/definitions/2346.0xc000278ab0.false"
+                    "$ref": "#/definitions/2268.0xc000528cf0.false"
                 },
                 "data2": {
-                    "$ref": "#/definitions/2381.0xc000278ae0.false"
+                    "$ref": "#/definitions/2302.0xc000528d20.false"
                 },
                 "msg": {
                     "type": "string"

swagger/swagger.yml

@@ -412,11 +412,6 @@ paths:
           description: The Response object
           schema:
             $ref: '#/definitions/controllers.Response'
-  /api/check-ldap-users-exist:
-    post:
-      tags:
-      - Account API
-      operationId: ApiController.CheckLdapUserExist
   /api/check-user-password:
     post:
       tags:
@@ -875,8 +870,8 @@ paths:
       tags:
       - Account API
       operationId: ApiController.GetLdap
-  /api/get-ldap-user:
+  /api/get-ldap-users:
-    post:
+    get:
       tags:
       - Account API
       operationId: ApiController.GetLdapser
@@ -1193,15 +1188,6 @@ paths:
           description: The Response object
           schema:
             $ref: '#/definitions/object.Record'
-  /api/get-release:
-    get:
-      tags:
-      - System API
-      description: get local github repo's latest release version info
-      operationId: ApiController.GitRepoVersion
-      responses:
-        "200":
-          description: '{string} local latest version hash of casdoor'
   /api/get-resource:
     get:
       tags:
@@ -1525,6 +1511,15 @@ paths:
             type: array
             items:
               $ref: '#/definitions/object.User'
+  /api/get-version-info:
+    get:
+      tags:
+      - System API
+      description: get local git repo's latest release version info
+      operationId: ApiController.GetVersionInfo
+      responses:
+        "200":
+          description: '{string} local latest version hash of Casdoor'
   /api/get-webhook:
     get:
       tags:
@@ -2379,10 +2374,10 @@ paths:
           schema:
             $ref: '#/definitions/Response'
 definitions:
-  2346.0xc000278ab0.false:
+  2268.0xc000528cf0.false:
     title: "false"
     type: object
-  2381.0xc000278ae0.false:
+  2302.0xc000528d20.false:
     title: "false"
     type: object
   Response:
@@ -2469,9 +2464,9 @@ definitions:
     type: object
     properties:
       data:
-        $ref: '#/definitions/2346.0xc000278ab0.false'
+        $ref: '#/definitions/2268.0xc000528cf0.false'
       data2:
-        $ref: '#/definitions/2381.0xc000278ae0.false'
+        $ref: '#/definitions/2302.0xc000528d20.false'
       msg:
         type: string
       name:

sync/sync_test.go

@@ -12,6 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+//go:build !skipCi
+// +build !skipCi
+
 package sync
 
 import (

web/src/LdapEditPage.js

@@ -83,7 +83,12 @@ class LdapEditPage extends React.Component {
       <Card size="small" title={
         <div>
           {i18next.t("ldap:Edit LDAP")}&nbsp;&nbsp;&nbsp;&nbsp;
-          <Button type="primary" onClick={() => this.submitLdapEdit()}>{i18next.t("general:Save")}</Button>
+          <Button onClick={() => this.submitLdapEdit()}>{i18next.t("general:Save")}</Button>
+          <Button style={{marginLeft: "20px"}} type="primary" onClick={() => this.submitLdapEdit(true)}>{i18next.t("general:Save & Exit")}</Button>
+          <Button style={{marginLeft: "20px"}}
+            onClick={() => Setting.goToLink(`/ldap/sync/${this.state.organizationName}/${this.state.ldapId}`)}>
+            {i18next.t("ldap:Sync")} LDAP
+          </Button>
         </div>
       } style={{marginLeft: "5px"}} type="inner">
         <Row style={{marginTop: "10px"}}>
@@ -190,14 +195,18 @@ class LdapEditPage extends React.Component {
     );
   }
 
-  submitLdapEdit() {
+  submitLdapEdit(willExist) {
     LddpBackend.updateLdap(this.state.ldap)
       .then((res) => {
         if (res.status === "ok") {
           Setting.showMessage("success", "Update LDAP server success");
-          this.setState((prevState) => {
+          this.setState({
-            prevState.ldap = res.data2;
+            organizationName: this.state.ldap.owner,
           });
+
+          if (willExist) {
+            this.props.history.push(`/organizations/${this.state.organizationName}`);
+          }
         } else {
           Setting.showMessage("error", res.msg);
         }
@@ -210,25 +219,13 @@ class LdapEditPage extends React.Component {
   render() {
     return (
       <div>
-        <Row style={{width: "100%"}}>
+        {
-          <Col span={1}>
+          this.state.ldap !== null ? this.renderLdap() : null
-          </Col>
+        }
-          <Col span={22}>
+        <div style={{marginTop: "20px", marginLeft: "40px"}}>
-            {
+          <Button size="large" onClick={() => this.submitLdapEdit()}>{i18next.t("general:Save")}</Button>
-              this.state.ldap !== null ? this.renderLdap() : null
+          <Button style={{marginLeft: "20px"}} type="primary" size="large" onClick={() => this.submitLdapEdit(true)}>{i18next.t("general:Save & Exit")}</Button>
-            }
+        </div>
-          </Col>
-          <Col span={1}>
-          </Col>
-        </Row>
-        <Row style={{margin: 10}}>
-          <Col span={2}>
-          </Col>
-          <Col span={18}>
-            <Button type="primary" size="large"
-              onClick={() => this.submitLdapEdit()}>{i18next.t("general:Save")}</Button>
-          </Col>
-        </Row>
       </div>
     );
   }

web/src/LdapSyncPage.js

@@ -81,44 +81,28 @@ class LdapSyncPage extends React.Component {
             prevState.ldap = res.data;
             return prevState;
           });
-          this.getLdapUser(res.data);
+          this.getLdapUser();
         } else {
           Setting.showMessage("error", res.msg);
         }
       });
   }
 
-  getLdapUser(ldap) {
+  getLdapUser() {
-    LdapBackend.getLdapUser(ldap)
+    LdapBackend.getLdapUser(this.state.organizationName, this.state.ldapId)
       .then((res) => {
         if (res.status === "ok") {
           this.setState((prevState) => {
             prevState.users = res.data.users;
+            prevState.existUuids = res.data2?.length > 0 ? res.data2 : [];
             return prevState;
           });
-          this.getExistUsers(ldap.owner, res.data.users);
         } else {
           Setting.showMessage("error", res.msg);
         }
       });
   }
 
-  getExistUsers(owner, users) {
-    const uuidArray = [];
-    users.forEach(elem => {
-      uuidArray.push(elem.uuid);
-    });
-    LdapBackend.checkLdapUsersExist(owner, uuidArray)
-      .then((res) => {
-        if (res.status === "ok") {
-          this.setState(prevState => {
-            prevState.existUuids = res.data?.length > 0 ? res.data : [];
-            return prevState;
-          });
-        }
-      });
-  }
-
   buildValArray(data, key) {
     const valTypesArray = [];
 
@@ -220,9 +204,14 @@ class LdapSyncPage extends React.Component {
                 title={"Please confirm to sync selected users"}
                 onConfirm={() => this.syncUsers()}
               >
-                <Button type="primary" size="small"
+                <Button type="primary" style={{marginLeft: "10px"}}>
-                  style={{marginLeft: "10px"}}>{i18next.t("ldap:Sync")}</Button>
+                  {i18next.t("ldap:Sync")}
+                </Button>
               </Popconfirm>
+              <Button style={{marginLeft: "20px"}}
+                onClick={() => Setting.goToLink(`/ldap/${this.state.organizationName}/${this.state.ldapId}`)}>
+                {i18next.t("general:Edit")} LDAP
+              </Button>
             </div>
           )}
           loading={users === null}
@@ -234,17 +223,20 @@ class LdapSyncPage extends React.Component {
   render() {
     return (
       <div>
-        <Row style={{width: "100%"}}>
+        <Row style={{width: "100%", justifyContent: "center"}}>
-          <Col span={1}>
-          </Col>
           <Col span={22}>
             {
               this.renderTable(this.state.users)
             }
           </Col>
-          <Col span={1}>
-          </Col>
         </Row>
+        <div style={{marginTop: "20px", marginLeft: "40px"}}>
+          <Button style={{marginLeft: "20px"}} type="primary" size="large" onClick={() => {
+            this.props.history.push(`/organizations/${this.state.organizationName}`);
+          }}>
+            {i18next.t("general:Save & Exit")}
+          </Button>
+        </div>
       </div>
     );
   }

web/src/LdapTable.js

@@ -58,14 +58,14 @@ class LdapTable extends React.Component {
     LdapBackend.addLdap(newLdap)
       .then((res) => {
         if (res.status === "ok") {
-          Setting.showMessage("success", "Add LDAP server success");
+          Setting.showMessage("success", i18next.t("general:Successfully added"));
           if (table === undefined) {
             table = [];
           }
           table = Setting.addRow(table, res.data2);
           this.updateTable(table);
         } else {
-          Setting.showMessage("error", res.msg);
+          Setting.showMessage("error", `${i18next.t("general:Failed to add")}: ${res.msg}`);
         }
       }
       )
@@ -78,14 +78,13 @@ class LdapTable extends React.Component {
     LdapBackend.deleteLdap(table[i])
       .then((res) => {
         if (res.status === "ok") {
-          Setting.showMessage("success", "Delete LDAP server success");
+          Setting.showMessage("success", i18next.t("general:Successfully deleted"));
           table = Setting.deleteRow(table, i);
           this.updateTable(table);
         } else {
-          Setting.showMessage("error", `${i18next.t("general:Failed to save")}: ${res.msg}`);
+          Setting.showMessage("error", `${i18next.t("general:Failed to delete")}: ${res.msg}`);
         }
-      }
+      })
-      )
       .catch(error => {
         Setting.showMessage("error", `Delete LDAP server failed: ${error}`);
       });
@@ -153,11 +152,14 @@ class LdapTable extends React.Component {
         render: (text, record, index) => {
           return (
             <div>
+              <Button style={{marginTop: "10px", marginBottom: "10px", marginRight: "10px"}} type="primary"
+                onClick={() => Setting.goToLink(`/ldap/sync/${record.owner}/${record.id}`)}>
+                {i18next.t("ldap:Sync")}
+              </Button>
               <Button style={{marginTop: "10px", marginBottom: "10px", marginRight: "10px"}}
-                type="primary"
+                onClick={() => Setting.goToLink(`/ldap/${record.owner}/${record.id}`)}>
-                onClick={() => Setting.goToLink(`/ldap/sync/${record.owner}/${record.id}`)}>{i18next.t("ldap:Sync")}</Button>
+                {i18next.t("general:Edit")}
-              <Button style={{marginTop: "10px", marginBottom: "10px", marginRight: "10px"}}
+              </Button>
-                onClick={() => Setting.goToLink(`/ldap/${record.owner}/${record.id}`)}>{i18next.t("general:Edit")}</Button>
               <PopconfirmModal
                 title={i18next.t("general:Sure to delete") + `: ${record.serverName} ?`}
                 onConfirm={() => this.deleteRow(table, index)}

web/src/backend/LdapBackend.js

@@ -67,11 +67,10 @@ export function updateLdap(body) {
   }).then(res => res.json());
 }
 
-export function getLdapUser(body) {
+export function getLdapUser(owner, name) {
-  return fetch(`${Setting.ServerUrl}/api/get-ldap-user`, {
+  return fetch(`${Setting.ServerUrl}/api/get-ldap-users?id=${owner}/${encodeURIComponent(name)}`, {
-    method: "POST",
+    method: "GET",
     credentials: "include",
-    body: JSON.stringify(body),
     headers: {
       "Accept-Language": Setting.getAcceptLanguage(),
     },
@@ -88,14 +87,3 @@ export function syncUsers(owner, ldapId, body) {
     },
   }).then(res => res.json());
 }
-
-export function checkLdapUsersExist(owner, body) {
-  return fetch(`${Setting.ServerUrl}/api/check-ldap-users-exist?owner=${owner}`, {
-    method: "POST",
-    credentials: "include",
-    body: JSON.stringify(body),
-    headers: {
-      "Accept-Language": Setting.getAcceptLanguage(),
-    },
-  }).then(res => res.json());
-}