From 1cbe6e797517089140ee2db12d73c0781f5e3e6b Mon Sep 17 00:00:00 2001 From: enahum Date: Mon, 3 Apr 2017 14:37:58 -0300 Subject: Add more OAuth unit tests (#5946) --- api/oauth.go | 10 - api/oauth_test.go | 620 ++++++++++++++++++++++++++++++++++++++---- app/oauth_test.go | 44 +++ model/access.go | 2 +- model/access_test.go | 54 +++- model/authorize.go | 2 +- model/authorize_test.go | 72 ++++- store/sql_oauth_store.go | 6 + store/sql_oauth_store_test.go | 135 ++++++++- web/web_test.go | 139 ---------- 10 files changed, 873 insertions(+), 211 deletions(-) create mode 100644 app/oauth_test.go diff --git a/api/oauth.go b/api/oauth.go index 25d4a89f9..754c49564 100644 --- a/api/oauth.go +++ b/api/oauth.go @@ -901,11 +901,6 @@ func deauthorizeOAuthApp(c *Context, w http.ResponseWriter, r *http.Request) { params := mux.Vars(r) id := params["id"] - if len(id) == 0 { - c.SetInvalidParam("deauthorizeOAuthApp", "id") - return - } - // revoke app sessions if result := <-app.Srv.Store.OAuth().GetAccessDataByUserForApp(c.Session.UserId, id); result.Err != nil { c.Err = result.Err @@ -946,11 +941,6 @@ func regenerateOAuthSecret(c *Context, w http.ResponseWriter, r *http.Request) { params := mux.Vars(r) id := params["id"] - if len(id) == 0 { - c.SetInvalidParam("regenerateOAuthSecret", "id") - return - } - var oauthApp *model.OAuthApp if result := <-app.Srv.Store.OAuth().GetApp(id); result.Err != nil { c.Err = model.NewLocAppError("regenerateOAuthSecret", "api.oauth.allow_oauth.database.app_error", nil, "") diff --git a/api/oauth_test.go b/api/oauth_test.go index 1dcdd55e3..87a8f847c 100644 --- a/api/oauth_test.go +++ b/api/oauth_test.go @@ -4,20 +4,28 @@ package api import ( + "encoding/base64" + "github.com/mattermost/platform/app" + "github.com/mattermost/platform/einterfaces" "github.com/mattermost/platform/model" "github.com/mattermost/platform/utils" + "io" + "io/ioutil" + "net/http" "net/url" + "strings" "testing" ) -func TestRegisterApp(t *testing.T) { +func TestOAuthRegisterApp(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() - Client := th.SystemAdminClient + Client := th.BasicClient - app := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { - if _, err := Client.RegisterApp(app); err == nil { + if _, err := Client.RegisterApp(oauthApp); err == nil { t.Fatal("should have failed - oauth providing turned off") } @@ -25,15 +33,21 @@ func TestRegisterApp(t *testing.T) { utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true + // calling the endpoint without an app + if _, err := Client.DoApiPost("/oauth/register", ""); err == nil { + t.Fatal("should have failed") + } + Client.Logout() - if _, err := Client.RegisterApp(app); err == nil { + if _, err := Client.RegisterApp(oauthApp); err == nil { t.Fatal("not logged in - should have failed") } th.LoginSystemAdmin() + Client = th.SystemAdminClient - if result, err := Client.RegisterApp(app); err != nil { + if result, err := Client.RegisterApp(oauthApp); err != nil { t.Fatal(err) } else { rapp := result.Data.(*model.OAuthApp) @@ -45,41 +59,54 @@ func TestRegisterApp(t *testing.T) { } } - app = &model.OAuthApp{Name: "", Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} - if _, err := Client.RegisterApp(app); err == nil { + oauthApp = &model.OAuthApp{Name: "", Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + if _, err := Client.RegisterApp(oauthApp); err == nil { t.Fatal("missing name - should have failed") } - app = &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} - if _, err := Client.RegisterApp(app); err == nil { + oauthApp = &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + if _, err := Client.RegisterApp(oauthApp); err == nil { t.Fatal("missing homepage - should have failed") } - app = &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{}} - if _, err := Client.RegisterApp(app); err == nil { + oauthApp = &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{}} + if _, err := Client.RegisterApp(oauthApp); err == nil { t.Fatal("missing callback url - should have failed") } + + user := &model.User{Email: strings.ToLower("test+"+model.NewId()) + "@simulator.amazonses.com", Password: "hello1", Username: "n" + model.NewId(), EmailVerified: true} + + ruser := Client.Must(Client.CreateUser(user, "")).Data.(*model.User) + app.UpdateUserRoles(ruser.Id, "") + + Client.Logout() + Client.Login(user.Email, user.Password) + + oauthApp = &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + if _, err := Client.RegisterApp(oauthApp); err == nil { + t.Fatal("should have failed. not enough permissions") + } } -func TestAllowOAuth(t *testing.T) { +func TestOAuthAllow(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() Client := th.BasicClient AdminClient := th.SystemAdminClient utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true - app := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} - app = AdminClient.Must(AdminClient.RegisterApp(app)).Data.(*model.OAuthApp) + oauthApp := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp = AdminClient.Must(AdminClient.RegisterApp(oauthApp)).Data.(*model.OAuthApp) state := "123" utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false - if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, app.Id, app.CallbackUrls[0], "all", state); err == nil { + if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, oauthApp.CallbackUrls[0], "all", state); err == nil { t.Fatal("should have failed - oauth providing turned off") } utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true - if result, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, app.Id, app.CallbackUrls[0], "all", state); err != nil { + if result, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, oauthApp.CallbackUrls[0], "all", state); err != nil { t.Fatal(err) } else { redirect := result.Data.(map[string]string)["redirect"] @@ -100,15 +127,19 @@ func TestAllowOAuth(t *testing.T) { } } - if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, app.Id, "", "all", state); err == nil { + if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, "", "all", state); err == nil { t.Fatal("should have failed - no redirect_url given") } - if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, app.Id, "", "", state); err == nil { + if _, err := Client.AllowOAuth("", oauthApp.Id, "", "", state); err == nil { + t.Fatal("should have failed - no response type given") + } + + if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, "", "", state); err == nil { t.Fatal("should have failed - no redirect_url given") } - if result, err := Client.AllowOAuth("junk", app.Id, app.CallbackUrls[0], "all", state); err != nil { + if result, err := Client.AllowOAuth("junk", oauthApp.Id, oauthApp.CallbackUrls[0], "all", state); err != nil { t.Fatal(err) } else { redirect := result.Data.(map[string]string)["redirect"] @@ -129,24 +160,25 @@ func TestAllowOAuth(t *testing.T) { } } - if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, "", app.CallbackUrls[0], "all", state); err == nil { + if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, "", oauthApp.CallbackUrls[0], "all", state); err == nil { t.Fatal("should have failed - empty client id") } - if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, "junk", app.CallbackUrls[0], "all", state); err == nil { + if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, "junk", oauthApp.CallbackUrls[0], "all", state); err == nil { t.Fatal("should have failed - bad client id") } - if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, app.Id, "https://somewhereelse.com", "all", state); err == nil { + if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, "https://somewhereelse.com", "", state); err == nil { t.Fatal("should have failed - redirect uri host does not match app host") } } -func TestGetOAuthAppsByUser(t *testing.T) { +func TestOAuthGetAppsByUser(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() Client := th.BasicClient AdminClient := th.SystemAdminClient + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { if _, err := Client.GetOAuthAppsByUser(); err == nil { t.Fatal("should have failed - oauth providing turned off") @@ -173,8 +205,8 @@ func TestGetOAuthAppsByUser(t *testing.T) { } } - app := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} - app = Client.Must(Client.RegisterApp(app)).Data.(*model.OAuthApp) + oauthApp := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp = Client.Must(Client.RegisterApp(oauthApp)).Data.(*model.OAuthApp) if result, err := Client.GetOAuthAppsByUser(); err != nil { t.Fatal(err) @@ -186,8 +218,8 @@ func TestGetOAuthAppsByUser(t *testing.T) { } } - app = &model.OAuthApp{Name: "TestApp4" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} - app = AdminClient.Must(Client.RegisterApp(app)).Data.(*model.OAuthApp) + oauthApp = &model.OAuthApp{Name: "TestApp4" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp = AdminClient.Must(Client.RegisterApp(oauthApp)).Data.(*model.OAuthApp) if result, err := AdminClient.GetOAuthAppsByUser(); err != nil { t.Fatal(err) @@ -198,13 +230,25 @@ func TestGetOAuthAppsByUser(t *testing.T) { t.Fatal("incorrect number of apps should have been 4 or more") } } + + user := &model.User{Email: strings.ToLower("test+"+model.NewId()) + "@simulator.amazonses.com", Password: "hello1", Username: "n" + model.NewId(), EmailVerified: true} + ruser := Client.Must(AdminClient.CreateUser(user, "")).Data.(*model.User) + app.UpdateUserRoles(ruser.Id, "") + + Client.Logout() + Client.Login(user.Email, user.Password) + + if _, err := Client.GetOAuthAppsByUser(); err == nil { + t.Fatal("should have failed. not enough permissions") + } } -func TestGetOAuthAppInfo(t *testing.T) { +func TestOAuthGetAppInfo(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() Client := th.BasicClient AdminClient := th.SystemAdminClient + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { if _, err := Client.GetOAuthAppInfo("fakeId"); err == nil { t.Fatal("should have failed - oauth providing turned off") @@ -214,27 +258,38 @@ func TestGetOAuthAppInfo(t *testing.T) { utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true - app := &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp := &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + + oauthApp = AdminClient.Must(AdminClient.RegisterApp(oauthApp)).Data.(*model.OAuthApp) - app = AdminClient.Must(AdminClient.RegisterApp(app)).Data.(*model.OAuthApp) + if _, err := Client.GetOAuthAppInfo(model.NewId()); err == nil { + t.Fatal("Should have failed") + } - if _, err := Client.GetOAuthAppInfo(app.Id); err != nil { + if _, err := Client.GetOAuthAppInfo(oauthApp.Id); err != nil { t.Fatal(err) } } -func TestGetAuthorizedApps(t *testing.T) { +func TestOAuthGetAuthorizedApps(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() Client := th.BasicClient AdminClient := th.SystemAdminClient - utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false + if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { + if _, err := Client.GetOAuthAuthorizedApps(); err == nil { + t.Fatal("should have failed - oauth providing turned off") + } - app := &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + } - app = AdminClient.Must(AdminClient.RegisterApp(app)).Data.(*model.OAuthApp) + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true + + oauthApp := &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp = AdminClient.Must(AdminClient.RegisterApp(oauthApp)).Data.(*model.OAuthApp) - if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, app.Id, "https://nowhere.com", "user", ""); err != nil { + if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, "https://nowhere.com", "user", ""); err != nil { t.Fatal(err) } @@ -249,22 +304,43 @@ func TestGetAuthorizedApps(t *testing.T) { } } -func TestDeauthorizeApp(t *testing.T) { +func TestOAuthDeauthorizeApp(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() Client := th.BasicClient AdminClient := th.SystemAdminClient + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false + if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { + if err := Client.OAuthDeauthorizeApp(model.NewId()); err == nil { + t.Fatal("should have failed - oauth providing turned off") + } + + } + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true - app := &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp := &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} - app = AdminClient.Must(AdminClient.RegisterApp(app)).Data.(*model.OAuthApp) + oauthApp = AdminClient.Must(AdminClient.RegisterApp(oauthApp)).Data.(*model.OAuthApp) - if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, app.Id, "https://nowhere.com", "user", ""); err != nil { + if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, "https://nowhere.com", "user", ""); err != nil { t.Fatal(err) } - if err := Client.OAuthDeauthorizeApp(app.Id); err != nil { + if err := Client.OAuthDeauthorizeApp(""); err == nil { + t.Fatal("Should have failed - no id provided") + } + + a1 := model.AccessData{} + a1.ClientId = oauthApp.Id + a1.UserId = th.BasicUser.Id + a1.Token = model.NewId() + a1.RefreshToken = model.NewId() + a1.ExpiresAt = model.GetMillis() + a1.RedirectUri = "http://example.com" + <-app.Srv.Store.OAuth().SaveAccessData(&a1) + + if err := Client.OAuthDeauthorizeApp(oauthApp.Id); err != nil { t.Fatal(err) } @@ -279,25 +355,42 @@ func TestDeauthorizeApp(t *testing.T) { } } -func TestRegenerateOAuthAppSecret(t *testing.T) { - th := Setup().InitSystemAdmin() +func TestOAuthRegenerateAppSecret(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + Client := th.BasicClient AdminClient := th.SystemAdminClient + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false + if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { + if _, err := AdminClient.RegenerateOAuthAppSecret(model.NewId()); err == nil { + t.Fatal("should have failed - oauth providing turned off") + } + + } + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true - app := &model.OAuthApp{Name: "TestApp6" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp := &model.OAuthApp{Name: "TestApp6" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + + oauthApp = AdminClient.Must(AdminClient.RegisterApp(oauthApp)).Data.(*model.OAuthApp) + + if _, err := AdminClient.RegenerateOAuthAppSecret(model.NewId()); err == nil { + t.Fatal("Should have failed - invalid app id") + } - app = AdminClient.Must(AdminClient.RegisterApp(app)).Data.(*model.OAuthApp) + if _, err := Client.RegenerateOAuthAppSecret(oauthApp.Id); err == nil { + t.Fatal("Should have failed - only admin or the user who registered the app are allowed to perform this action") + } - if regenApp, err := AdminClient.RegenerateOAuthAppSecret(app.Id); err != nil { + if regenApp, err := AdminClient.RegenerateOAuthAppSecret(oauthApp.Id); err != nil { t.Fatal(err) } else { app2 := regenApp.Data.(*model.OAuthApp) - if app2.Id != app.Id { + if app2.Id != oauthApp.Id { t.Fatal("Should have been the same app Id") } - if app2.ClientSecret == app.ClientSecret { + if app2.ClientSecret == oauthApp.ClientSecret { t.Fatal("Should have been diferent client Secrets") } } @@ -308,6 +401,7 @@ func TestOAuthDeleteApp(t *testing.T) { Client := th.BasicClient AdminClient := th.SystemAdminClient + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { if _, err := Client.DeleteOAuthApp("fakeId"); err == nil { t.Fatal("should have failed - oauth providing turned off") @@ -319,19 +413,435 @@ func TestOAuthDeleteApp(t *testing.T) { *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = false utils.SetDefaultRolesBasedOnConfig() - app := &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp := &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + + oauthApp = Client.Must(Client.RegisterApp(oauthApp)).Data.(*model.OAuthApp) + + if _, err := Client.DeleteOAuthApp(oauthApp.Id); err != nil { + t.Fatal(err) + } + + oauthApp = &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + + oauthApp = Client.Must(Client.RegisterApp(oauthApp)).Data.(*model.OAuthApp) + + if _, err := AdminClient.DeleteOAuthApp(""); err == nil { + t.Fatal("Should have failed - id not provided") + } + + if _, err := AdminClient.DeleteOAuthApp(model.NewId()); err == nil { + t.Fatal("Should have failed - invalid id") + } + + if _, err := AdminClient.DeleteOAuthApp(oauthApp.Id); err != nil { + t.Fatal(err) + } + + oauthApp = &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp = AdminClient.Must(AdminClient.RegisterApp(oauthApp)).Data.(*model.OAuthApp) + + if _, err := Client.DeleteOAuthApp(oauthApp.Id); err == nil { + t.Fatal("Should have failed - only admin or the user who registered the app are allowed to perform this action") + } + + user := &model.User{Email: strings.ToLower("test+"+model.NewId()) + "@simulator.amazonses.com", Password: "hello1", Username: "n" + model.NewId(), EmailVerified: true} + ruser := Client.Must(AdminClient.CreateUser(user, "")).Data.(*model.User) + app.UpdateUserRoles(ruser.Id, "") + + Client.Logout() + Client.Login(user.Email, user.Password) + if _, err := Client.DeleteOAuthApp(oauthApp.Id); err == nil { + t.Fatal("Should have failed - not enough permissions") + } +} + +func TestOAuthAuthorize(t *testing.T) { + if testing.Short() { + t.SkipNow() + } + + th := Setup().InitBasic() + Client := th.BasicClient + + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = false + if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { + if r, err := HttpGet(Client.Url+"/oauth/authorize", Client.HttpClient, "", true); err == nil { + t.Fatal("should have failed - oauth providing turned off") + closeBody(r) + } + } + + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true + if r, err := HttpGet(Client.Url+"/oauth/authorize", Client.HttpClient, "", true); err == nil { + t.Fatal("should have failed - scope not provided") + closeBody(r) + } + + if r, err := HttpGet(Client.Url+"/oauth/authorize?client_id=bad&&redirect_uri=http://example.com&response_type="+model.AUTHCODE_RESPONSE_TYPE, Client.HttpClient, "", true); err == nil { + t.Fatal("should have failed - scope not provided") + closeBody(r) + } + + // register an app to authorize it + oauthApp := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp = Client.Must(Client.RegisterApp(oauthApp)).Data.(*model.OAuthApp) + if r, err := HttpGet(Client.Url+"/oauth/authorize?client_id="+oauthApp.Id+"&&redirect_uri=http://example.com&response_type="+model.AUTHCODE_RESPONSE_TYPE, Client.HttpClient, "", true); err == nil { + t.Fatal("should have failed - user not logged") + closeBody(r) + } + + authToken := Client.AuthType + " " + Client.AuthToken + if r, err := HttpGet(Client.Url+"/oauth/authorize?client_id="+oauthApp.Id+"&&redirect_uri=http://example.com&response_type="+model.AUTHCODE_RESPONSE_TYPE, Client.HttpClient, authToken, true); err != nil { + t.Fatal(err) + closeBody(r) + } + + // lets authorize the app + if _, err := Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, oauthApp.CallbackUrls[0], "user", ""); err != nil { + t.Fatal(err) + } + + if r, err := HttpGet(Client.Url+"/oauth/authorize?client_id="+oauthApp.Id+"&&redirect_uri="+oauthApp.CallbackUrls[0]+"&response_type="+model.AUTHCODE_RESPONSE_TYPE, + Client.HttpClient, authToken, true); err != nil { + // it will return an error as there is no connection to https://nowhere.com + if r != nil { + closeBody(r) + } + } +} + +func TestOAuthAccessToken(t *testing.T) { + if testing.Short() { + t.SkipNow() + } + + th := Setup().InitBasic() + Client := th.BasicClient + + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true + oauthApp := &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + oauthApp = Client.Must(Client.RegisterApp(oauthApp)).Data.(*model.OAuthApp) + + utils.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]}} + + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("should have failed - oauth providing turned off") + } + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true + + redirect := Client.Must(Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, oauthApp.CallbackUrls[0], "all", "123")).Data.(map[string]string)["redirect"] + rurl, _ := url.Parse(redirect) + + Client.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]}} + + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("should have failed - bad grant type") + } + + data.Set("grant_type", model.ACCESS_TOKEN_GRANT_TYPE) + data.Set("client_id", "") + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("should have failed - missing client id") + } + data.Set("client_id", "junk") + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("should have failed - bad client id") + } + + data.Set("client_id", oauthApp.Id) + data.Set("client_secret", "") + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("should have failed - missing client secret") + } + + data.Set("client_secret", "junk") + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("should have failed - bad client secret") + } + + data.Set("client_secret", oauthApp.ClientSecret) + data.Set("code", "") + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("should have failed - missing code") + } + + data.Set("code", "junk") + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("should have failed - bad code") + } + + data.Set("code", rurl.Query().Get("code")) + data.Set("redirect_uri", "junk") + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("should have failed - non-matching redirect uri") + } - app = Client.Must(Client.RegisterApp(app)).Data.(*model.OAuthApp) + // reset data for successful request + data.Set("grant_type", model.ACCESS_TOKEN_GRANT_TYPE) + data.Set("client_id", oauthApp.Id) + data.Set("client_secret", oauthApp.ClientSecret) + data.Set("code", rurl.Query().Get("code")) + data.Set("redirect_uri", oauthApp.CallbackUrls[0]) - if _, err := Client.DeleteOAuthApp(app.Id); err != nil { + token := "" + refreshToken := "" + if result, err := Client.GetAccessToken(data); err != nil { t.Fatal(err) + } else { + rsp := result.Data.(*model.AccessResponse) + if len(rsp.AccessToken) == 0 { + t.Fatal("access token not returned") + } else { + token = rsp.AccessToken + refreshToken = rsp.RefreshToken + } + if rsp.TokenType != model.ACCESS_TOKEN_TYPE { + t.Fatal("access token type incorrect") + } + } + + if result, err := Client.DoApiGet("/teams/"+th.BasicTeam.Id+"/users/0/100?access_token="+token, "", ""); err != nil { + t.Fatal(err) + } else { + userMap := model.UserMapFromJson(result.Body) + if len(userMap) == 0 { + t.Fatal("user map empty - did not get results correctly") + } + } + + if _, err := Client.DoApiGet("/teams/"+th.BasicTeam.Id+"/users/0/100", "", ""); err == nil { + t.Fatal("should have failed - no access token provided") + } + + if _, err := Client.DoApiGet("/teams/"+th.BasicTeam.Id+"/users/0/100?access_token=junk", "", ""); err == nil { + t.Fatal("should have failed - bad access token provided") } - app = &model.OAuthApp{Name: "TestApp5" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + Client.SetOAuthToken(token) + if result, err := Client.DoApiGet("/teams/"+th.BasicTeam.Id+"/users/0/100", "", ""); err != nil { + t.Fatal(err) + } else { + userMap := model.UserMapFromJson(result.Body) + if len(userMap) == 0 { + t.Fatal("user map empty - did not get results correctly") + } + } - app = Client.Must(Client.RegisterApp(app)).Data.(*model.OAuthApp) + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("should have failed - tried to reuse auth code") + } + + data.Set("grant_type", model.REFRESH_TOKEN_GRANT_TYPE) + data.Set("client_id", oauthApp.Id) + data.Set("client_secret", oauthApp.ClientSecret) + data.Set("refresh_token", "") + data.Set("redirect_uri", oauthApp.CallbackUrls[0]) + data.Del("code") + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("Should have failed - refresh token empty") + } - if _, err := AdminClient.DeleteOAuthApp(app.Id); err != nil { + data.Set("refresh_token", refreshToken) + if _, err := Client.GetAccessToken(data); err != nil { t.Fatal(err) } + + authData := &model.AuthData{ClientId: oauthApp.Id, RedirectUri: oauthApp.CallbackUrls[0], UserId: th.BasicUser.Id, Code: model.NewId(), ExpiresIn: -1} + <-app.Srv.Store.OAuth().SaveAuthData(authData) + + data.Set("grant_type", model.ACCESS_TOKEN_GRANT_TYPE) + data.Set("client_id", oauthApp.Id) + data.Set("client_secret", oauthApp.ClientSecret) + data.Set("redirect_uri", oauthApp.CallbackUrls[0]) + data.Set("code", authData.Code) + data.Del("refresh_token") + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("Should have failed - code is expired") + } + + authData = &model.AuthData{ClientId: oauthApp.Id, RedirectUri: oauthApp.CallbackUrls[0], UserId: th.BasicUser.Id, Code: model.NewId(), ExpiresIn: model.AUTHCODE_EXPIRE_TIME} + <-app.Srv.Store.OAuth().SaveAuthData(authData) + + data.Set("code", authData.Code) + if _, err := Client.GetAccessToken(data); err == nil { + t.Fatal("Should have failed - code with invalid hash comparission") + } + + Client.ClearOAuthToken() +} + +func TestOAuthComplete(t *testing.T) { + if testing.Short() { + t.SkipNow() + } + + th := Setup().InitBasic() + Client := th.BasicClient + + if r, err := HttpGet(Client.Url+"/login/gitlab/complete", Client.HttpClient, "", true); err == nil { + t.Fatal("should have failed - no code provided") + closeBody(r) + } + + if r, err := HttpGet(Client.Url+"/login/gitlab/complete?code=123", Client.HttpClient, "", true); err == nil { + t.Fatal("should have failed - gitlab disabled") + closeBody(r) + } + + utils.Cfg.GitLabSettings.Enable = true + if r, err := HttpGet(Client.Url+"/login/gitlab/complete?code=123&state=!#$#F@#Yˆ&~ñ", Client.HttpClient, "", true); err == nil { + t.Fatal("should have failed - gitlab disabled") + closeBody(r) + } + + utils.Cfg.GitLabSettings.AuthEndpoint = Client.Url + "/oauth/authorize" + utils.Cfg.GitLabSettings.Id = model.NewId() + + stateProps := map[string]string{} + stateProps["action"] = model.OAUTH_ACTION_LOGIN + stateProps["team_id"] = th.BasicTeam.Id + stateProps["redirect_to"] = utils.Cfg.GitLabSettings.AuthEndpoint + + state := base64.StdEncoding.EncodeToString([]byte(model.MapToJson(stateProps))) + if r, err := HttpGet(Client.Url+"/login/gitlab/complete?code=123&state="+url.QueryEscape(state), Client.HttpClient, "", true); err == nil { + t.Fatal("should have failed - bad state") + closeBody(r) + } + + stateProps["hash"] = model.HashPassword(utils.Cfg.GitLabSettings.Id) + state = base64.StdEncoding.EncodeToString([]byte(model.MapToJson(stateProps))) + if r, err := HttpGet(Client.Url+"/login/gitlab/complete?code=123&state="+url.QueryEscape(state), Client.HttpClient, "", true); err == nil { + t.Fatal("should have failed - no connection") + closeBody(r) + } + + // We are going to use mattermost as the provider emulating gitlab + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true + + oauthApp := &model.OAuthApp{ + Name: "TestApp5" + model.NewId(), + Homepage: "https://nowhere.com", + Description: "test", + CallbackUrls: []string{ + Client.Url + "/signup/" + model.SERVICE_GITLAB + "/complete", + Client.Url + "/login/" + model.SERVICE_GITLAB + "/complete", + }, + IsTrusted: true, + } + oauthApp = Client.Must(Client.RegisterApp(oauthApp)).Data.(*model.OAuthApp) + + utils.Cfg.GitLabSettings.Id = oauthApp.Id + utils.Cfg.GitLabSettings.Secret = oauthApp.ClientSecret + utils.Cfg.GitLabSettings.AuthEndpoint = Client.Url + "/oauth/authorize" + utils.Cfg.GitLabSettings.TokenEndpoint = Client.Url + "/oauth/access_token" + utils.Cfg.GitLabSettings.UserApiEndpoint = Client.ApiUrl + "/users/me" + + provider := &MattermostTestProvider{} + + redirect := Client.Must(Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, oauthApp.CallbackUrls[0], "all", "123")).Data.(map[string]string)["redirect"] + rurl, _ := url.Parse(redirect) + code := rurl.Query().Get("code") + stateProps["action"] = model.OAUTH_ACTION_EMAIL_TO_SSO + delete(stateProps, "team_id") + stateProps["redirect_to"] = utils.Cfg.GitLabSettings.AuthEndpoint + stateProps["hash"] = model.HashPassword(utils.Cfg.GitLabSettings.Id) + stateProps["redirect_to"] = "/oauth/authorize" + state = base64.StdEncoding.EncodeToString([]byte(model.MapToJson(stateProps))) + if r, err := HttpGet(Client.Url+"/login/"+model.SERVICE_GITLAB+"/complete?code="+url.QueryEscape(code)+"&state="+url.QueryEscape(state), Client.HttpClient, "", false); err == nil { + closeBody(r) + } + + einterfaces.RegisterOauthProvider(model.SERVICE_GITLAB, provider) + redirect = Client.Must(Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, oauthApp.CallbackUrls[0], "all", "123")).Data.(map[string]string)["redirect"] + rurl, _ = url.Parse(redirect) + code = rurl.Query().Get("code") + if r, err := HttpGet(Client.Url+"/login/"+model.SERVICE_GITLAB+"/complete?code="+url.QueryEscape(code)+"&state="+url.QueryEscape(state), Client.HttpClient, "", false); err == nil { + closeBody(r) + } + + if result := <-app.Srv.Store.User().UpdateAuthData( + th.BasicUser.Id, model.SERVICE_GITLAB, &th.BasicUser.Email, th.BasicUser.Email, true); result.Err != nil { + t.Fatal(result.Err) + } + + redirect = Client.Must(Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, oauthApp.CallbackUrls[0], "all", "123")).Data.(map[string]string)["redirect"] + rurl, _ = url.Parse(redirect) + code = rurl.Query().Get("code") + stateProps["action"] = model.OAUTH_ACTION_LOGIN + state = base64.StdEncoding.EncodeToString([]byte(model.MapToJson(stateProps))) + if r, err := HttpGet(Client.Url+"/login/"+model.SERVICE_GITLAB+"/complete?code="+url.QueryEscape(code)+"&state="+url.QueryEscape(state), Client.HttpClient, "", false); err == nil { + closeBody(r) + } + + redirect = Client.Must(Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, oauthApp.CallbackUrls[0], "all", "123")).Data.(map[string]string)["redirect"] + rurl, _ = url.Parse(redirect) + code = rurl.Query().Get("code") + delete(stateProps, "action") + state = base64.StdEncoding.EncodeToString([]byte(model.MapToJson(stateProps))) + if r, err := HttpGet(Client.Url+"/login/"+model.SERVICE_GITLAB+"/complete?code="+url.QueryEscape(code)+"&state="+url.QueryEscape(state), Client.HttpClient, "", false); err == nil { + closeBody(r) + } + + redirect = Client.Must(Client.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, oauthApp.CallbackUrls[0], "all", "123")).Data.(map[string]string)["redirect"] + rurl, _ = url.Parse(redirect) + code = rurl.Query().Get("code") + stateProps["action"] = model.OAUTH_ACTION_SIGNUP + state = base64.StdEncoding.EncodeToString([]byte(model.MapToJson(stateProps))) + if r, err := HttpGet(Client.Url+"/login/"+model.SERVICE_GITLAB+"/complete?code="+url.QueryEscape(code)+"&state="+url.QueryEscape(state), Client.HttpClient, "", false); err == nil { + closeBody(r) + } +} + +func HttpGet(url string, httpClient *http.Client, authToken string, followRedirect bool) (*http.Response, *model.AppError) { + rq, _ := http.NewRequest("GET", url, nil) + rq.Close = true + + if len(authToken) > 0 { + rq.Header.Set(model.HEADER_AUTH, authToken) + } + + if !followRedirect { + httpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + } + } + + if rp, err := httpClient.Do(rq); err != nil { + return nil, model.NewLocAppError(url, "model.client.connecting.app_error", nil, err.Error()) + } else if rp.StatusCode == 304 { + return rp, nil + } else if rp.StatusCode == 307 { + return rp, nil + } else if rp.StatusCode >= 300 { + defer closeBody(rp) + return rp, model.AppErrorFromJson(rp.Body) + } else { + return rp, nil + } +} + +func closeBody(r *http.Response) { + if r.Body != nil { + ioutil.ReadAll(r.Body) + r.Body.Close() + } +} + +type MattermostTestProvider struct { +} + +func (m *MattermostTestProvider) GetIdentifier() string { + return model.SERVICE_GITLAB +} + +func (m *MattermostTestProvider) GetUserFromJson(data io.Reader) *model.User { + return model.UserFromJson(data) +} + +func (m *MattermostTestProvider) GetAuthDataFromJson(data io.Reader) string { + authData := model.UserFromJson(data) + return authData.Email } diff --git a/app/oauth_test.go b/app/oauth_test.go new file mode 100644 index 000000000..3ca3a2d4a --- /dev/null +++ b/app/oauth_test.go @@ -0,0 +1,44 @@ +// Copyright (c) 2017 Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +import ( + "testing" + + "github.com/mattermost/platform/model" +) + +func TestOAuthRevokeAccessToken(t *testing.T) { + Setup() + if err := RevokeAccessToken(model.NewRandomString(16)); err == nil { + t.Fatal("Should have failed bad token") + } + + session := &model.Session{} + session.CreateAt = model.GetMillis() + session.UserId = model.NewId() + session.Token = model.NewId() + session.Roles = model.ROLE_SYSTEM_USER.Id + session.SetExpireInDays(1) + + session, _ = CreateSession(session) + if err := RevokeAccessToken(session.Token); err == nil { + t.Fatal("Should have failed does not have an access token") + } + + accessData := &model.AccessData{} + accessData.Token = session.Token + accessData.UserId = session.UserId + accessData.RedirectUri = "http://example.com" + accessData.ClientId = model.NewId() + accessData.ExpiresAt = session.ExpiresAt + + if result := <-Srv.Store.OAuth().SaveAccessData(accessData); result.Err != nil { + t.Fatal(result.Err) + } + + if err := RevokeAccessToken(accessData.Token); err != nil { + t.Fatal(err) + } +} diff --git a/model/access.go b/model/access.go index 85417fce9..520417f4e 100644 --- a/model/access.go +++ b/model/access.go @@ -51,7 +51,7 @@ func (ad *AccessData) IsValid() *AppError { return NewLocAppError("AccessData.IsValid", "model.access.is_valid.refresh_token.app_error", nil, "") } - if len(ad.RedirectUri) > 256 { + if len(ad.RedirectUri) == 0 || len(ad.RedirectUri) > 256 || !IsValidHttpUrl(ad.RedirectUri) { return NewLocAppError("AccessData.IsValid", "model.access.is_valid.redirect_uri.app_error", nil, "") } diff --git a/model/access_test.go b/model/access_test.go index 0eca302ba..77b4cf15b 100644 --- a/model/access_test.go +++ b/model/access_test.go @@ -27,12 +27,32 @@ func TestAccessIsValid(t *testing.T) { ad := AccessData{} if err := ad.IsValid(); err == nil { - t.Fatal("should have failed") + t.Fatal() + } + + ad.ClientId = NewRandomString(28) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed Client Id") + } + + ad.ClientId = "" + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed Client Id") } ad.ClientId = NewId() if err := ad.IsValid(); err == nil { - t.Fatal("should have failed") + t.Fatal() + } + + ad.UserId = NewRandomString(28) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed User Id") + } + + ad.UserId = "" + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed User Id") } ad.UserId = NewId() @@ -40,7 +60,37 @@ func TestAccessIsValid(t *testing.T) { t.Fatal("should have failed") } + ad.Token = NewRandomString(22) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed Token") + } + ad.Token = NewId() + if err := ad.IsValid(); err == nil { + t.Fatal() + } + + ad.RefreshToken = NewRandomString(28) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed Refresh Token") + } + + ad.RefreshToken = NewId() + if err := ad.IsValid(); err == nil { + t.Fatal() + } + + ad.RedirectUri = "" + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed Redirect URI not set") + } + + ad.RedirectUri = NewRandomString(28) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed invalid URL") + } + + ad.RedirectUri = "http://example.com" if err := ad.IsValid(); err != nil { t.Fatal(err) } diff --git a/model/authorize.go b/model/authorize.go index 2b4017e9c..3f259718b 100644 --- a/model/authorize.go +++ b/model/authorize.go @@ -49,7 +49,7 @@ func (ad *AuthData) IsValid() *AppError { return NewLocAppError("AuthData.IsValid", "model.authorize.is_valid.create_at.app_error", nil, "client_id="+ad.ClientId) } - if len(ad.RedirectUri) > 256 { + if len(ad.RedirectUri) == 0 || len(ad.RedirectUri) > 256 || !IsValidHttpUrl(ad.RedirectUri) { return NewLocAppError("AuthData.IsValid", "model.authorize.is_valid.redirect_uri.app_error", nil, "client_id="+ad.ClientId) } diff --git a/model/authorize_test.go b/model/authorize_test.go index 3fedc37e4..82a48332c 100644 --- a/model/authorize_test.go +++ b/model/authorize_test.go @@ -39,28 +39,98 @@ func TestAuthIsValid(t *testing.T) { t.Fatal() } + ad.ClientId = NewRandomString(28) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed Client Id") + } + ad.ClientId = NewId() if err := ad.IsValid(); err == nil { t.Fatal() } + ad.UserId = NewRandomString(28) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed User Id") + } + ad.UserId = NewId() if err := ad.IsValid(); err == nil { t.Fatal() } + ad.Code = NewRandomString(129) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed Code to long") + } + + ad.Code = "" + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed Code not set") + } + ad.Code = NewId() if err := ad.IsValid(); err == nil { t.Fatal() } + ad.ExpiresIn = 0 + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed invalid ExpiresIn") + } + ad.ExpiresIn = 1 if err := ad.IsValid(); err == nil { t.Fatal() } + ad.CreateAt = 0 + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed Invalid Create At") + } + ad.CreateAt = 1 - if err := ad.IsValid(); err != nil { + if err := ad.IsValid(); err == nil { t.Fatal() } + + ad.State = NewRandomString(129) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed invalid State") + } + + ad.State = NewRandomString(128) + if err := ad.IsValid(); err == nil { + t.Fatal(err) + } + + ad.Scope = NewRandomString(129) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed invalid Scope") + } + + ad.Scope = NewRandomString(128) + if err := ad.IsValid(); err == nil { + t.Fatal() + } + + ad.RedirectUri = "" + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed Redirect URI not set") + } + + ad.RedirectUri = NewRandomString(28) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed invalid URL") + } + + ad.RedirectUri = NewRandomString(257) + if err := ad.IsValid(); err == nil { + t.Fatal("Should have failed invalid URL") + } + + ad.RedirectUri = "http://example.com" + if err := ad.IsValid(); err != nil { + t.Fatal(err) + } } diff --git a/store/sql_oauth_store.go b/store/sql_oauth_store.go index caa04c008..72bc574d9 100644 --- a/store/sql_oauth_store.go +++ b/store/sql_oauth_store.go @@ -387,6 +387,12 @@ func (as SqlOAuthStore) UpdateAccessData(accessData *model.AccessData) StoreChan go func() { result := StoreResult{} + if result.Err = accessData.IsValid(); result.Err != nil { + storeChannel <- result + close(storeChannel) + return + } + if _, err := as.GetMaster().Exec("UPDATE OAuthAccessData SET Token = :Token, ExpiresAt = :ExpiresAt WHERE ClientId = :ClientId AND UserID = :UserId", map[string]interface{}{"Token": accessData.Token, "ExpiresAt": accessData.ExpiresAt, "ClientId": accessData.ClientId, "UserId": accessData.UserId}); err != nil { result.Err = model.NewLocAppError("SqlOAuthStore.Update", "store.sql_oauth.update_access_data.app_error", nil, diff --git a/store/sql_oauth_store_test.go b/store/sql_oauth_store_test.go index b9bde5be3..dd6fe906e 100644 --- a/store/sql_oauth_store_test.go +++ b/store/sql_oauth_store_test.go @@ -13,10 +13,24 @@ func TestOAuthStoreSaveApp(t *testing.T) { a1 := model.OAuthApp{} a1.CreatorId = model.NewId() - a1.Name = "TestApp" + model.NewId() a1.CallbackUrls = []string{"https://nowhere.com"} a1.Homepage = "https://nowhere.com" + // Try to save an app that already has an Id + a1.Id = model.NewId() + if err := (<-store.OAuth().SaveApp(&a1)).Err; err == nil { + t.Fatal("Should have failed, cannot add an OAuth app cannot be save with an Id, it has to be updated") + } + + // Try to save an Invalid App + a1.Id = "" + if err := (<-store.OAuth().SaveApp(&a1)).Err; err == nil { + t.Fatal("Should have failed, app should be invalid cause it doesn' have a name set") + } + + // Save the app + a1.Id = "" + a1.Name = "TestApp" + model.NewId() if err := (<-store.OAuth().SaveApp(&a1)).Err; err != nil { t.Fatal(err) } @@ -32,10 +46,24 @@ func TestOAuthStoreGetApp(t *testing.T) { a1.Homepage = "https://nowhere.com" Must(store.OAuth().SaveApp(&a1)) + // Lets try to get and app that does not exists + if err := (<-store.OAuth().GetApp("fake0123456789abcderfgret1")).Err; err == nil { + t.Fatal("Should have failed. App does not exists") + } + if err := (<-store.OAuth().GetApp(a1.Id)).Err; err != nil { t.Fatal(err) } + // 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 len(result.Data.([]*model.OAuthApp)) > 0 { + t.Fatal("Should have failed. Fake user hasn't created any apps") + } + } else { + t.Fatal(result.Err) + } + if err := (<-store.OAuth().GetAppByUser(a1.CreatorId)).Err; err != nil { t.Fatal(err) } @@ -55,10 +83,27 @@ func TestOAuthStoreUpdateApp(t *testing.T) { a1.Homepage = "https://nowhere.com" Must(store.OAuth().SaveApp(&a1)) + // temporarily save the created app id + id := a1.Id + a1.CreateAt = 1 a1.ClientSecret = "pwd" a1.CreatorId = "12345678901234567890123456" + + // Lets update the app by removing the name + a1.Name = "" + if result := <-store.OAuth().UpdateApp(&a1); result.Err == nil { + t.Fatal("Should have failed. App name is not set") + } + + // Lets not find the app that we are trying to update + a1.Id = "fake0123456789abcderfgret1" a1.Name = "NewName" + if result := <-store.OAuth().UpdateApp(&a1); result.Err == nil { + t.Fatal("Should have failed. Not able to find the app") + } + + a1.Id = id if result := <-store.OAuth().UpdateApp(&a1); result.Err != nil { t.Fatal(result.Err) } else { @@ -81,14 +126,59 @@ func TestOAuthStoreSaveAccessData(t *testing.T) { a1 := model.AccessData{} a1.ClientId = model.NewId() a1.UserId = model.NewId() + + // Lets try and save an incomplete access data + if err := (<-store.OAuth().SaveAccessData(&a1)).Err; err == nil { + t.Fatal("Should have failed. Access data needs the token") + } + a1.Token = model.NewId() a1.RefreshToken = model.NewId() + a1.RedirectUri = "http://example.com" if err := (<-store.OAuth().SaveAccessData(&a1)).Err; err != nil { t.Fatal(err) } } +func TestOAuthUpdateAccessData(t *testing.T) { + Setup() + + a1 := model.AccessData{} + a1.ClientId = model.NewId() + a1.UserId = model.NewId() + a1.Token = model.NewId() + a1.RefreshToken = model.NewId() + a1.ExpiresAt = model.GetMillis() + a1.RedirectUri = "http://example.com" + Must(store.OAuth().SaveAccessData(&a1)) + + //Try to update to invalid Refresh Token + refreshToken := a1.RefreshToken + a1.RefreshToken = model.NewId() + "123" + if err := (<-store.OAuth().UpdateAccessData(&a1)).Err; err == nil { + t.Fatal("Should have failed with invalid token") + } + + //Try to update to invalid RedirectUri + a1.RefreshToken = model.NewId() + a1.RedirectUri = "" + if err := (<-store.OAuth().UpdateAccessData(&a1)).Err; err == nil { + t.Fatal("Should have failed with invalid Redirect URI") + } + + // Should update fine + a1.RedirectUri = "http://example.com" + if result := <-store.OAuth().UpdateAccessData(&a1); result.Err != nil { + t.Fatal(result.Err) + } else { + ra1 := result.Data.(*model.AccessData) + if ra1.RefreshToken == refreshToken { + t.Fatal("refresh tokens didn't match") + } + } +} + func TestOAuthStoreGetAccessData(t *testing.T) { Setup() @@ -98,8 +188,13 @@ func TestOAuthStoreGetAccessData(t *testing.T) { a1.Token = model.NewId() a1.RefreshToken = model.NewId() a1.ExpiresAt = model.GetMillis() + a1.RedirectUri = "http://example.com" Must(store.OAuth().SaveAccessData(&a1)) + if err := (<-store.OAuth().GetAccessData("invalidToken")).Err; err == nil { + t.Fatal("Should have failed. There is no data with an invalid token") + } + if result := <-store.OAuth().GetAccessData(a1.Token); result.Err != nil { t.Fatal(result.Err) } else { @@ -116,6 +211,21 @@ func TestOAuthStoreGetAccessData(t *testing.T) { if err := (<-store.OAuth().GetPreviousAccessData("user", "junk")).Err; err != nil { t.Fatal(err) } + + // Try to get the Access data using an invalid refresh token + if err := (<-store.OAuth().GetAccessDataByRefreshToken(a1.Token)).Err; err == nil { + t.Fatal("Should have failed. There is no data with an invalid token") + } + + // Get the Access Data using the refresh token + if result := <-store.OAuth().GetAccessDataByRefreshToken(a1.RefreshToken); result.Err != nil { + t.Fatal(result.Err) + } else { + ra1 := result.Data.(*model.AccessData) + if a1.RefreshToken != ra1.RefreshToken { + t.Fatal("tokens didn't match") + } + } } func TestOAuthStoreRemoveAccessData(t *testing.T) { @@ -126,6 +236,7 @@ func TestOAuthStoreRemoveAccessData(t *testing.T) { a1.UserId = model.NewId() a1.Token = model.NewId() a1.RefreshToken = model.NewId() + a1.RedirectUri = "http://example.com" Must(store.OAuth().SaveAccessData(&a1)) if err := (<-store.OAuth().RemoveAccessData(a1.Token)).Err; err != nil { @@ -147,7 +258,7 @@ func TestOAuthStoreSaveAuthData(t *testing.T) { a1.ClientId = model.NewId() a1.UserId = model.NewId() a1.Code = model.NewId() - + a1.RedirectUri = "http://example.com" if err := (<-store.OAuth().SaveAuthData(&a1)).Err; err != nil { t.Fatal(err) } @@ -160,6 +271,7 @@ func TestOAuthStoreGetAuthData(t *testing.T) { a1.ClientId = model.NewId() a1.UserId = model.NewId() a1.Code = model.NewId() + a1.RedirectUri = "http://example.com" Must(store.OAuth().SaveAuthData(&a1)) if err := (<-store.OAuth().GetAuthData(a1.Code)).Err; err != nil { @@ -174,6 +286,7 @@ func TestOAuthStoreRemoveAuthData(t *testing.T) { a1.ClientId = model.NewId() a1.UserId = model.NewId() a1.Code = model.NewId() + a1.RedirectUri = "http://example.com" Must(store.OAuth().SaveAuthData(&a1)) if err := (<-store.OAuth().RemoveAuthData(a1.Code)).Err; err != nil { @@ -192,6 +305,7 @@ func TestOAuthStoreRemoveAuthDataByUser(t *testing.T) { a1.ClientId = model.NewId() a1.UserId = model.NewId() a1.Code = model.NewId() + a1.RedirectUri = "http://example.com" Must(store.OAuth().SaveAuthData(&a1)) if err := (<-store.OAuth().PermanentDeleteAuthDataByUser(a1.UserId)).Err; err != nil { @@ -209,6 +323,15 @@ func TestOAuthGetAuthorizedApps(t *testing.T) { a1.Homepage = "https://nowhere.com" 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 len(result.Data.([]*model.OAuthApp)) > 0 { + t.Fatal("Should have failed. Fake user hasn't authorized the app") + } + } else { + t.Fatal(result.Err) + } + // allow the app p := model.Preference{} p.UserId = a1.CreatorId @@ -260,6 +383,7 @@ func TestOAuthGetAccessDataByUserForApp(t *testing.T) { ad1.UserId = a1.CreatorId ad1.Token = model.NewId() ad1.RefreshToken = model.NewId() + ad1.RedirectUri = "http://example.com" if err := (<-store.OAuth().SaveAccessData(&ad1)).Err; err != nil { t.Fatal(err) @@ -276,6 +400,8 @@ func TestOAuthGetAccessDataByUserForApp(t *testing.T) { } func TestOAuthStoreDeleteApp(t *testing.T) { + Setup() + a1 := model.OAuthApp{} a1.CreatorId = model.NewId() a1.Name = "TestApp" + model.NewId() @@ -283,6 +409,11 @@ func TestOAuthStoreDeleteApp(t *testing.T) { a1.Homepage = "https://nowhere.com" Must(store.OAuth().SaveApp(&a1)) + // delete a non-existent app + if err := (<-store.OAuth().DeleteApp("fakeclientId")).Err; err != nil { + t.Fatal(err) + } + if err := (<-store.OAuth().DeleteApp(a1.Id)).Err; err != nil { t.Fatal(err) } diff --git a/web/web_test.go b/web/web_test.go index 8db0eb91c..03cacdddf 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -4,8 +4,6 @@ package web import ( - "net/url" - "strings" "testing" "time" @@ -62,143 +60,6 @@ func TestStatic(t *testing.T) { } */ -func TestGetAccessToken(t *testing.T) { - Setup() - - user := model.User{Email: strings.ToLower(model.NewId()) + "success+test@simulator.amazonses.com", Password: "passwd1"} - ruser := ApiClient.Must(ApiClient.CreateUser(&user, "")).Data.(*model.User) - store.Must(app.Srv.Store.User().VerifyEmail(ruser.Id)) - - ApiClient.Must(ApiClient.LoginById(ruser.Id, "passwd1")) - - team := model.Team{DisplayName: "Name", Name: "z-z-" + model.NewId() + "a", Email: "test@nowhere.com", Type: model.TEAM_OPEN} - rteam, _ := ApiClient.CreateTeam(&team) - - oauthApp := &model.OAuthApp{Name: "TestApp" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} - - utils.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]}} - - if _, err := ApiClient.GetAccessToken(data); err == nil { - t.Fatal("should have failed - oauth providing turned off") - } - utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true - - ApiClient.Must(ApiClient.LoginById(ruser.Id, "passwd1")) - ApiClient.SetTeamId(rteam.Data.(*model.Team).Id) - *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = false - utils.SetDefaultRolesBasedOnConfig() - oauthApp = ApiClient.Must(ApiClient.RegisterApp(oauthApp)).Data.(*model.OAuthApp) - *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = true - utils.SetDefaultRolesBasedOnConfig() - - redirect := ApiClient.Must(ApiClient.AllowOAuth(model.AUTHCODE_RESPONSE_TYPE, oauthApp.Id, oauthApp.CallbackUrls[0], "all", "123")).Data.(map[string]string)["redirect"] - rurl, _ := url.Parse(redirect) - - teamId := rteam.Data.(*model.Team).Id - - 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]}} - - if _, err := ApiClient.GetAccessToken(data); err == nil { - t.Fatal("should have failed - bad grant type") - } - - data.Set("grant_type", model.ACCESS_TOKEN_GRANT_TYPE) - data.Set("client_id", "") - if _, err := ApiClient.GetAccessToken(data); err == nil { - t.Fatal("should have failed - missing client id") - } - data.Set("client_id", "junk") - if _, err := ApiClient.GetAccessToken(data); err == nil { - t.Fatal("should have failed - bad client id") - } - - data.Set("client_id", oauthApp.Id) - data.Set("client_secret", "") - if _, err := ApiClient.GetAccessToken(data); err == nil { - t.Fatal("should have failed - missing client secret") - } - - data.Set("client_secret", "junk") - if _, err := ApiClient.GetAccessToken(data); err == nil { - t.Fatal("should have failed - bad client secret") - } - - data.Set("client_secret", oauthApp.ClientSecret) - data.Set("code", "") - if _, err := ApiClient.GetAccessToken(data); err == nil { - t.Fatal("should have failed - missing code") - } - - data.Set("code", "junk") - if _, err := ApiClient.GetAccessToken(data); err == nil { - t.Fatal("should have failed - bad code") - } - - data.Set("code", rurl.Query().Get("code")) - data.Set("redirect_uri", "junk") - if _, err := ApiClient.GetAccessToken(data); err == nil { - t.Fatal("should have failed - non-matching redirect uri") - } - - // reset data for successful request - data.Set("grant_type", model.ACCESS_TOKEN_GRANT_TYPE) - data.Set("client_id", oauthApp.Id) - data.Set("client_secret", oauthApp.ClientSecret) - data.Set("code", rurl.Query().Get("code")) - data.Set("redirect_uri", oauthApp.CallbackUrls[0]) - - token := "" - if result, err := ApiClient.GetAccessToken(data); err != nil { - t.Fatal(err) - } else { - rsp := result.Data.(*model.AccessResponse) - if len(rsp.AccessToken) == 0 { - t.Fatal("access token not returned") - } else { - token = rsp.AccessToken - } - if rsp.TokenType != model.ACCESS_TOKEN_TYPE { - t.Fatal("access token type incorrect") - } - } - - if result, err := ApiClient.DoApiGet("/teams/"+teamId+"/users/0/100?access_token="+token, "", ""); err != nil { - t.Fatal(err) - } else { - userMap := model.UserMapFromJson(result.Body) - if len(userMap) == 0 { - t.Fatal("user map empty - did not get results correctly") - } - } - - if _, err := ApiClient.DoApiGet("/teams/"+teamId+"/users/0/100", "", ""); err == nil { - t.Fatal("should have failed - no access token provided") - } - - if _, err := ApiClient.DoApiGet("/teams/"+teamId+"/users/0/100?access_token=junk", "", ""); err == nil { - t.Fatal("should have failed - bad access token provided") - } - - ApiClient.SetOAuthToken(token) - if result, err := ApiClient.DoApiGet("/teams/"+teamId+"/users/0/100", "", ""); err != nil { - t.Fatal(err) - } else { - userMap := model.UserMapFromJson(result.Body) - if len(userMap) == 0 { - t.Fatal("user map empty - did not get results correctly") - } - } - - if _, err := ApiClient.GetAccessToken(data); err == nil { - t.Fatal("should have failed - tried to reuse auth code") - } - - ApiClient.ClearOAuthToken() -} - func TestIncomingWebhook(t *testing.T) { Setup() -- cgit v1.2.3-1-g7c22