Refactor OAuth 2.0 code into app layer (#6037)

5 files changed, 24 insertions, 22 deletions

diff --git a/store/sql_oauth_store.go b/store/sql_oauth_store.go
index bc97ee33a..6311b56dd 100644
--- a/store/sql_oauth_store.go
+++ b/store/sql_oauth_store.go
@@ -4,6 +4,7 @@
 package store
 
 import (
+	"net/http"
 	"strings"
 
 	"github.com/go-gorp/gorp"
@@ -42,6 +43,7 @@ func NewSqlOAuthStore(sqlStore *SqlStore) OAuthStore {
 		tableAccess.ColMap("Token").SetMaxSize(26)
 		tableAccess.ColMap("RefreshToken").SetMaxSize(26)
 		tableAccess.ColMap("RedirectUri").SetMaxSize(256)
+		tableAccess.ColMap("Scope").SetMaxSize(128)
 		tableAccess.SetUniqueTogether("ClientId", "UserId")
 	}
 
@@ -138,9 +140,9 @@ func (as SqlOAuthStore) GetApp(id string) StoreChannel {
 		result := StoreResult{}
 
 		if obj, err := as.GetReplica().Get(model.OAuthApp{}, id); err != nil {
-			result.Err = model.NewLocAppError("SqlOAuthStore.GetApp", "store.sql_oauth.get_app.finding.app_error", nil, "app_id="+id+", "+err.Error())
+			result.Err = model.NewAppError("SqlOAuthStore.GetApp", "store.sql_oauth.get_app.finding.app_error", nil, "app_id="+id+", "+err.Error(), http.StatusInternalServerError)
 		} else if obj == nil {
-			result.Err = model.NewLocAppError("SqlOAuthStore.GetApp", "store.sql_oauth.get_app.find.app_error", nil, "app_id="+id)
+			result.Err = model.NewAppError("SqlOAuthStore.GetApp", "store.sql_oauth.get_app.find.app_error", nil, "app_id="+id, http.StatusNotFound)
 		} else {
 			result.Data = obj.(*model.OAuthApp)
 		}
@@ -153,7 +155,7 @@ func (as SqlOAuthStore) GetApp(id string) StoreChannel {
 	return storeChannel
 }
 
-func (as SqlOAuthStore) GetAppByUser(userId string) StoreChannel {
+func (as SqlOAuthStore) GetAppByUser(userId string, offset, limit int) StoreChannel {
 
 	storeChannel := make(StoreChannel, 1)
 
@@ -162,8 +164,8 @@ func (as SqlOAuthStore) GetAppByUser(userId string) StoreChannel {
 
 		var apps []*model.OAuthApp
 
-		if _, err := as.GetReplica().Select(&apps, "SELECT * FROM OAuthApps WHERE CreatorId = :UserId", map[string]interface{}{"UserId": userId}); err != nil {
-			result.Err = model.NewLocAppError("SqlOAuthStore.GetAppByUser", "store.sql_oauth.get_app_by_user.find.app_error", nil, "user_id="+userId+", "+err.Error())
+		if _, err := as.GetReplica().Select(&apps, "SELECT * FROM OAuthApps WHERE CreatorId = :UserId LIMIT :Limit OFFSET :Offset", map[string]interface{}{"UserId": userId, "Offset": offset, "Limit": limit}); err != nil {
+			result.Err = model.NewAppError("SqlOAuthStore.GetAppByUser", "store.sql_oauth.get_app_by_user.find.app_error", nil, "user_id="+userId+", "+err.Error(), http.StatusInternalServerError)
 		}
 
 		result.Data = apps
@@ -175,7 +177,7 @@ func (as SqlOAuthStore) GetAppByUser(userId string) StoreChannel {
 	return storeChannel
 }
 
-func (as SqlOAuthStore) GetApps() StoreChannel {
+func (as SqlOAuthStore) GetApps(offset, limit int) StoreChannel {
 
 	storeChannel := make(StoreChannel, 1)
 
@@ -184,8 +186,8 @@ func (as SqlOAuthStore) GetApps() StoreChannel {
 
 		var apps []*model.OAuthApp
 
-		if _, err := as.GetReplica().Select(&apps, "SELECT * FROM OAuthApps"); err != nil {
-			result.Err = model.NewLocAppError("SqlOAuthStore.GetAppByUser", "store.sql_oauth.get_apps.find.app_error", nil, "err="+err.Error())
+		if _, err := as.GetReplica().Select(&apps, "SELECT * FROM OAuthApps LIMIT :Limit OFFSET :Offset", map[string]interface{}{"Offset": offset, "Limit": limit}); err != nil {
+			result.Err = model.NewAppError("SqlOAuthStore.GetAppByUser", "store.sql_oauth.get_apps.find.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
 		}
 
 		result.Data = apps
@@ -197,7 +199,7 @@ func (as SqlOAuthStore) GetApps() StoreChannel {
 	return storeChannel
 }
 
-func (as SqlOAuthStore) GetAuthorizedApps(userId string) StoreChannel {
+func (as SqlOAuthStore) GetAuthorizedApps(userId string, offset, limit int) StoreChannel {
 	storeChannel := make(StoreChannel, 1)
 
 	go func() {
@@ -207,8 +209,8 @@ func (as SqlOAuthStore) GetAuthorizedApps(userId string) StoreChannel {
 
 		if _, err := as.GetReplica().Select(&apps,
 			`SELECT o.* FROM OAuthApps AS o INNER JOIN
-			Preferences AS p ON p.Name=o.Id AND p.UserId=:UserId`, map[string]interface{}{"UserId": userId}); err != nil {
-			result.Err = model.NewLocAppError("SqlOAuthStore.GetAuthorizedApps", "store.sql_oauth.get_apps.find.app_error", nil, "err="+err.Error())
+			Preferences AS p ON p.Name=o.Id AND p.UserId=:UserId LIMIT :Limit OFFSET :Offset`, map[string]interface{}{"UserId": userId, "Offset": offset, "Limit": limit}); err != nil {
+			result.Err = model.NewAppError("SqlOAuthStore.GetAuthorizedApps", "store.sql_oauth.get_apps.find.app_error", nil, "err="+err.Error(), http.StatusInternalServerError)
 		}
 
 		result.Data = apps
diff --git a/store/sql_oauth_store_test.go b/store/sql_oauth_store_test.go
index d0c04c53f..8c707562f 100644
--- a/store/sql_oauth_store_test.go
+++ b/store/sql_oauth_store_test.go
@@ -56,7 +56,7 @@ func TestOAuthStoreGetApp(t *testing.T) {
 	}
 
 	// Lets try and get the app from a user that hasn't created any apps
-	if result := (<-store.OAuth().GetAppByUser("fake0123456789abcderfgret1")); result.Err == nil {
+	if result := (<-store.OAuth().GetAppByUser("fake0123456789abcderfgret1", 0, 1000)); result.Err == nil {
 		if len(result.Data.([]*model.OAuthApp)) > 0 {
 			t.Fatal("Should have failed. Fake user hasn't created any apps")
 		}
@@ -64,11 +64,11 @@ func TestOAuthStoreGetApp(t *testing.T) {
 		t.Fatal(result.Err)
 	}
 
-	if err := (<-store.OAuth().GetAppByUser(a1.CreatorId)).Err; err != nil {
+	if err := (<-store.OAuth().GetAppByUser(a1.CreatorId, 0, 1000)).Err; err != nil {
 		t.Fatal(err)
 	}
 
-	if err := (<-store.OAuth().GetApps()).Err; err != nil {
+	if err := (<-store.OAuth().GetApps(0, 1000)).Err; err != nil {
 		t.Fatal(err)
 	}
 }
@@ -324,7 +324,7 @@ func TestOAuthGetAuthorizedApps(t *testing.T) {
 	Must(store.OAuth().SaveApp(&a1))
 
 	// Lets try and get an Authorized app for a user who hasn't authorized it
-	if result := <-store.OAuth().GetAuthorizedApps("fake0123456789abcderfgret1"); result.Err == nil {
+	if result := <-store.OAuth().GetAuthorizedApps("fake0123456789abcderfgret1", 0, 1000); result.Err == nil {
 		if len(result.Data.([]*model.OAuthApp)) > 0 {
 			t.Fatal("Should have failed. Fake user hasn't authorized the app")
 		}
@@ -340,7 +340,7 @@ func TestOAuthGetAuthorizedApps(t *testing.T) {
 	p.Value = "true"
 	Must(store.Preference().Save(&model.Preferences{p}))
 
-	if result := <-store.OAuth().GetAuthorizedApps(a1.CreatorId); result.Err != nil {
+	if result := <-store.OAuth().GetAuthorizedApps(a1.CreatorId, 0, 1000); result.Err != nil {
 		t.Fatal(result.Err)
 	} else {
 		apps := result.Data.([]*model.OAuthApp)
@@ -368,7 +368,7 @@ func TestOAuthGetAccessDataByUserForApp(t *testing.T) {
 	p.Value = "true"
 	Must(store.Preference().Save(&model.Preferences{p}))
 
-	if result := <-store.OAuth().GetAuthorizedApps(a1.CreatorId); result.Err != nil {
+	if result := <-store.OAuth().GetAuthorizedApps(a1.CreatorId, 0, 1000); result.Err != nil {
 		t.Fatal(result.Err)
 	} else {
 		apps := result.Data.([]*model.OAuthApp)
diff --git a/store/sql_upgrade.go b/store/sql_upgrade.go
index dbdf12605..b25e7e73d 100644
--- a/store/sql_upgrade.go
+++ b/store/sql_upgrade.go
@@ -257,6 +257,7 @@ func UpgradeDatabaseToVersion38(sqlStore *SqlStore) {
 func UpgradeDatabaseToVersion39(sqlStore *SqlStore) {
 	// TODO: Uncomment following condition when version 3.9.0 is released
 	//if shouldPerformUpgrade(sqlStore, VERSION_3_8_0, VERSION_3_9_0) {
+	sqlStore.CreateColumnIfNotExists("OAuthAccessData", "Scope", "varchar(128)", "varchar(128)", model.DEFAULT_SCOPE)
 
 	//	saveSchemaVersion(sqlStore, VERSION_3_9_0)
 	//}
diff --git a/store/sql_user_store.go b/store/sql_user_store.go
index 52e45ed7d..5ea04155d 100644
--- a/store/sql_user_store.go
+++ b/store/sql_user_store.go
@@ -389,8 +389,7 @@ func (us SqlUserStore) Get(id string) StoreChannel {
 		if obj, err := us.GetReplica().Get(model.User{}, id); err != nil {
 			result.Err = model.NewLocAppError("SqlUserStore.Get", "store.sql_user.get.app_error", nil, "user_id="+id+", "+err.Error())
 		} else if obj == nil {
-			result.Err = model.NewLocAppError("SqlUserStore.Get", MISSING_ACCOUNT_ERROR, nil, "user_id="+id)
-			result.Err.StatusCode = http.StatusNotFound
+			result.Err = model.NewAppError("SqlUserStore.Get", MISSING_ACCOUNT_ERROR, nil, "user_id="+id, http.StatusNotFound)
 		} else {
 			result.Data = obj.(*model.User)
 		}
diff --git a/store/store.go b/store/store.go
index b78d4a458..18f7374dc 100644
--- a/store/store.go
+++ b/store/store.go
@@ -246,9 +246,9 @@ type OAuthStore interface {
 	SaveApp(app *model.OAuthApp) StoreChannel
 	UpdateApp(app *model.OAuthApp) StoreChannel
 	GetApp(id string) StoreChannel
-	GetAppByUser(userId string) StoreChannel
-	GetApps() StoreChannel
-	GetAuthorizedApps(userId string) StoreChannel
+	GetAppByUser(userId string, offset, limit int) StoreChannel
+	GetApps(offset, limit int) StoreChannel
+	GetAuthorizedApps(userId string, offset, limit int) StoreChannel
 	DeleteApp(id string) StoreChannel
 	SaveAuthData(authData *model.AuthData) StoreChannel
 	GetAuthData(code string) StoreChannel