fix: Admins should not be allowed to add third-party login for their members (#932)

* feat: admin can unlink the other user

* feat: global admin can unlink other user

* fix

controllers/link.go

@@ -21,7 +21,8 @@ import (
 )
 
 type LinkForm struct {
-	ProviderType string `json:"providerType"`
+	ProviderType string      `json:"providerType"`
+	User         object.User `json:"user"`
 }
 
 // Unlink ...
@@ -40,16 +41,55 @@ func (c *ApiController) Unlink() {
 	}
 	providerType := form.ProviderType
 
+	// the user will be unlinked from the provider
+	unlinkedUser := form.User
 	user := object.GetUser(userId)
-	value := object.GetUserField(user, providerType)
+
+	if user.Id != unlinkedUser.Id && !user.IsGlobalAdmin {
+		// if the user is not the same as the one we are unlinking, we need to make sure the user is the global admin.
+		c.ResponseError("You are not the global admin, you can't unlink other users")
+		return
+	}
+
+	if user.Id == unlinkedUser.Id && !user.IsGlobalAdmin {
+		// if the user is unlinking themselves, should check the provider can be unlinked, if not, we should return an error.
+		application := object.GetApplicationByUser(user)
+		if application == nil {
+			c.ResponseError("You can't unlink yourself, you are not a member of any application")
+			return
+		}
+
+		if len(application.Providers) == 0 {
+			c.ResponseError("This application has no providers")
+			return
+		}
+
+		provider := application.GetProviderItemByType(providerType)
+		if provider == nil {
+			c.ResponseError("This application has no providers of type " + providerType)
+			return
+		}
+
+		if !provider.CanUnlink {
+			c.ResponseError("This provider can't be unlinked")
+			return
+		}
+
+	}
+
+	// only two situations can happen here
+	// 1. the user is the global admin
+	// 2. the user is unlinking themselves and provider can be unlinked
+
+	value := object.GetUserField(&unlinkedUser, providerType)
 
 	if value == "" {
 		c.ResponseError("Please link first", value)
 		return
 	}
 
-	object.ClearUserOAuthProperties(user, providerType)
+	object.ClearUserOAuthProperties(&unlinkedUser, providerType)
 
-	object.LinkUserAccount(user, providerType, "")
+	object.LinkUserAccount(&unlinkedUser, providerType, "")
 	c.ResponseOk()
 }

object/provider_item.go

@@ -33,6 +33,15 @@ func (application *Application) GetProviderItem(providerName string) *ProviderIt
 	return nil
 }
 
+func (application *Application) GetProviderItemByType(providerType string) *ProviderItem {
+	for _, item := range application.Providers {
+		if item.Provider.Type == providerType {
+			return item
+		}
+	}
+	return nil
+}
+
 func (pi *ProviderItem) IsProviderVisible() bool {
 	if pi.Provider == nil {
 		return false

object/user.go

@@ -73,7 +73,7 @@ type User struct {
 	LastSigninTime string `xorm:"varchar(100)" json:"lastSigninTime"`
 	LastSigninIp   string `xorm:"varchar(100)" json:"lastSigninIp"`
 
-	Github        string `xorm:"varchar(100)" json:"github"`
+	GitHub        string `xorm:"github varchar(100)" json:"github"`
 	Google        string `xorm:"varchar(100)" json:"google"`
 	QQ            string `xorm:"qq varchar(100)" json:"qq"`
 	WeChat        string `xorm:"wechat varchar(100)" json:"wechat"`

object/user_test.go

@@ -37,11 +37,11 @@ func TestSyncAvatarsFromGitHub(t *testing.T) {
 
 	users := GetGlobalUsers()
 	for _, user := range users {
-		if user.Github == "" {
+		if user.GitHub == "" {
 			continue
 		}
 
-		user.Avatar = fmt.Sprintf("https://avatars.githubusercontent.com/%s", user.Github)
+		user.Avatar = fmt.Sprintf("https://avatars.githubusercontent.com/%s", user.GitHub)
 		updateUserColumn("avatar", user)
 	}
 }

web/src/UserEditPage.js

@@ -466,7 +466,7 @@ class UserEditPage extends React.Component {
                   (this.state.application === null || this.state.user === null) ? null : (
                     this.state.application?.providers.filter(providerItem => Setting.isProviderVisible(providerItem)).map((providerItem, index) =>
                       (providerItem.provider.category === "OAuth") ? (
-                        <OAuthWidget key={providerItem.name} labelSpan={(Setting.isMobile()) ? 10 : 3} user={this.state.user} application={this.state.application} providerItem={providerItem} onUnlinked={() => {return this.unlinked();}} />
+                        <OAuthWidget key={providerItem.name} labelSpan={(Setting.isMobile()) ? 10 : 3} user={this.state.user} application={this.state.application} providerItem={providerItem} account={this.props.account} onUnlinked={() => {return this.unlinked();}} />
                       ) : (
                         <SamlWidget key={providerItem.name} labelSpan={(Setting.isMobile()) ? 10 : 3} user={this.state.user} application={this.state.application} providerItem={providerItem} onUnlinked={() => {return this.unlinked();}} />
                       )

web/src/common/OAuthWidget.js

@@ -91,6 +91,8 @@ class OAuthWidget extends React.Component {
   unlinkUser(providerType) {
     const body = {
       providerType: providerType,
+      // should add the unlink user's info, cause the user may not be logged in, but a admin want to unlink the user.
+      user: this.props.user,
     };
     AuthBackend.unlink(body)
       .then((res) => {
@@ -113,6 +115,8 @@ class OAuthWidget extends React.Component {
     const displayName = this.getUserProperty(user, provider.type, "displayName");
     const email = this.getUserProperty(user, provider.type, "email");
     let avatarUrl = this.getUserProperty(user, provider.type, "avatarUrl");
+    // the account user
+    const account = this.props.account;
 
     if (avatarUrl === "" || avatarUrl === undefined) {
       avatarUrl = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAB4AAAAeCAQAAACROWYpAAAAHElEQVR42mNkoAAwjmoe1TyqeVTzqOZRzcNZMwB18wAfEFQkPQAAAABJRU5ErkJggg==";
@@ -161,10 +165,10 @@ class OAuthWidget extends React.Component {
           {
             linkedValue === "" ? (
               <a key={provider.displayName} href={Provider.getAuthUrl(application, provider, "link")}>
-                <Button style={{marginLeft: "20px", width: "80px"}} type="primary">{i18next.t("user:Link")}</Button>
+                <Button style={{marginLeft: "20px", width: "80px"}} type="primary" disabled={user.id !== account.id}>{i18next.t("user:Link")}</Button>
               </a>
             ) : (
-              <Button disabled={!providerItem.canUnlink} style={{marginLeft: "20px", width: "80px"}} onClick={() => this.unlinkUser(provider.type)}>{i18next.t("user:Unlink")}</Button>
+              <Button disabled={!providerItem.canUnlink && !account.isGlobalAdmin} style={{marginLeft: "20px", width: "80px"}} onClick={() => this.unlinkUser(provider.type)}>{i18next.t("user:Unlink")}</Button>
             )
           }
         </Col>