diff options
author | Joram Wilander <jwawilander@gmail.com> | 2018-07-27 17:35:43 -0400 |
---|---|---|
committer | Elias Nahum <nahumhbl@gmail.com> | 2018-07-27 17:35:43 -0400 |
commit | 6ac82d5171769bf8d543cb6c017d29c0a4c81621 (patch) | |
tree | 945a5d1511b1eb4048bfaa4ea59777886713d797 | |
parent | 441c8741c1738e93258b861d92e4f7293203918a (diff) | |
download | chat-6ac82d5171769bf8d543cb6c017d29c0a4c81621.tar.gz chat-6ac82d5171769bf8d543cb6c017d29c0a4c81621.tar.bz2 chat-6ac82d5171769bf8d543cb6c017d29c0a4c81621.zip |
Implement OAuth2 implicit grant flow (#9178)
-rw-r--r-- | api4/oauth.go | 9 | ||||
-rw-r--r-- | api4/oauth_test.go | 22 | ||||
-rw-r--r-- | app/oauth.go | 73 | ||||
-rw-r--r-- | app/oauth_test.go | 51 | ||||
-rw-r--r-- | model/authorize.go | 3 | ||||
-rw-r--r-- | model/oauth.go | 2 |
6 files changed, 148 insertions, 12 deletions
diff --git a/api4/oauth.go b/api4/oauth.go index b858267ee..ab4b1bfcf 100644 --- a/api4/oauth.go +++ b/api4/oauth.go @@ -278,6 +278,12 @@ func authorizeOAuthApp(c *Context, w http.ResponseWriter, r *http.Request) { return } + if c.Session.IsOAuth { + c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) + c.Err.DetailedError += ", attempted access by oauth app" + return + } + c.LogAudit("attempt") redirectUrl, err := c.App.AllowOAuthAppAccessToUser(c.Session.UserId, authRequest) @@ -358,7 +364,6 @@ func authorizeOAuthPage(c *Context, w http.ResponseWriter, r *http.Request) { // Automatically allow if the app is trusted if oauthApp.IsTrusted || isAuthorized { - authRequest.ResponseType = model.AUTHCODE_RESPONSE_TYPE redirectUrl, err := c.App.AllowOAuthAppAccessToUser(c.Session.UserId, authRequest) if err != nil { @@ -418,7 +423,7 @@ func getAccessToken(c *Context, w http.ResponseWriter, r *http.Request) { c.LogAudit("attempt") - accessRsp, err := c.App.GetOAuthAccessToken(clientId, grantType, redirectUri, code, secret, refreshToken) + accessRsp, err := c.App.GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, code, secret, refreshToken) if err != nil { c.Err = err return diff --git a/api4/oauth_test.go b/api4/oauth_test.go index 5415e485e..cac40e442 100644 --- a/api4/oauth_test.go +++ b/api4/oauth_test.go @@ -13,6 +13,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/einterfaces" "github.com/mattermost/mattermost-server/model" @@ -665,6 +666,7 @@ func TestAuthorizeOAuthApp(t *testing.T) { State: "123", } + // Test auth code flow ruri, resp := Client.AuthorizeOAuthApp(authRequest) CheckNoError(t, resp) @@ -684,6 +686,26 @@ func TestAuthorizeOAuthApp(t *testing.T) { } } + // Test implicit flow + authRequest.ResponseType = model.IMPLICIT_RESPONSE_TYPE + ruri, resp = Client.AuthorizeOAuthApp(authRequest) + CheckNoError(t, resp) + require.False(t, len(ruri) == 0, "redirect url should be set") + + ru, _ = url.Parse(ruri) + require.NotNil(t, ru, "redirect url unparseable") + values, err := url.ParseQuery(ru.Fragment) + require.Nil(t, err) + assert.False(t, len(values.Get("access_token")) == 0, "access_token not returned") + assert.Equal(t, authRequest.State, values.Get("state"), "returned state doesn't match") + + oldToken := Client.AuthToken + Client.AuthToken = values.Get("access_token") + _, resp = Client.AuthorizeOAuthApp(authRequest) + CheckForbiddenStatus(t, resp) + + Client.AuthToken = oldToken + authRequest.RedirectUri = "" _, resp = Client.AuthorizeOAuthApp(authRequest) CheckBadRequestStatus(t, resp) diff --git a/app/oauth.go b/app/oauth.go index 80fe4342e..af8bca050 100644 --- a/app/oauth.go +++ b/app/oauth.go @@ -11,6 +11,7 @@ import ( "io/ioutil" "net/http" "net/url" + "strconv" "strings" "time" @@ -108,6 +109,33 @@ func (a *App) GetOAuthAppsByCreator(userId string, page, perPage int) ([]*model. } } +func (a *App) GetOAuthImplicitRedirect(userId string, authRequest *model.AuthorizeRequest) (string, *model.AppError) { + session, err := a.GetOAuthAccessTokenForImplicitFlow(userId, authRequest) + if err != nil { + return "", err + } + + values := &url.Values{} + values.Add("access_token", session.Token) + values.Add("token_type", "bearer") + values.Add("expires_in", strconv.FormatInt((session.ExpiresAt-model.GetMillis())/1000, 10)) + values.Add("scope", authRequest.Scope) + values.Add("state", authRequest.State) + + return fmt.Sprintf("%s#%s", authRequest.RedirectUri, values.Encode()), nil +} + +func (a *App) GetOAuthCodeRedirect(userId string, authRequest *model.AuthorizeRequest) (string, *model.AppError) { + authData := &model.AuthData{UserId: userId, ClientId: authRequest.ClientId, CreateAt: model.GetMillis(), RedirectUri: authRequest.RedirectUri, State: authRequest.State, Scope: authRequest.Scope} + authData.Code = model.NewId() + model.NewId() + + if result := <-a.Srv.Store.OAuth().SaveAuthData(authData); result.Err != nil { + return authRequest.RedirectUri + "?error=server_error&state=" + authRequest.State, nil + } + + return authRequest.RedirectUri + "?code=" + url.QueryEscape(authData.Code) + "&state=" + url.QueryEscape(authData.State), nil +} + func (a *App) AllowOAuthAppAccessToUser(userId string, authRequest *model.AuthorizeRequest) (string, *model.AppError) { if !a.Config().ServiceSettings.EnableOAuthServiceProvider { return "", model.NewAppError("AllowOAuthAppAccessToUser", "api.oauth.allow_oauth.turn_off.app_error", nil, "", http.StatusNotImplemented) @@ -128,12 +156,22 @@ func (a *App) AllowOAuthAppAccessToUser(userId string, authRequest *model.Author return "", model.NewAppError("AllowOAuthAppAccessToUser", "api.oauth.allow_oauth.redirect_callback.app_error", nil, "", http.StatusBadRequest) } - if authRequest.ResponseType != model.AUTHCODE_RESPONSE_TYPE { + var redirectURI string + var err *model.AppError + + switch authRequest.ResponseType { + case model.AUTHCODE_RESPONSE_TYPE: + redirectURI, err = a.GetOAuthCodeRedirect(userId, authRequest) + case model.IMPLICIT_RESPONSE_TYPE: + redirectURI, err = a.GetOAuthImplicitRedirect(userId, authRequest) + default: return authRequest.RedirectUri + "?error=unsupported_response_type&state=" + authRequest.State, nil } - authData := &model.AuthData{UserId: userId, ClientId: authRequest.ClientId, CreateAt: model.GetMillis(), RedirectUri: authRequest.RedirectUri, State: authRequest.State, Scope: authRequest.Scope} - authData.Code = model.NewId() + model.NewId() + if err != nil { + mlog.Error(err.Error()) + return authRequest.RedirectUri + "?error=server_error&state=" + authRequest.State, nil + } // this saves the OAuth2 app as authorized authorizedApp := model.Preference{ @@ -144,17 +182,38 @@ func (a *App) AllowOAuthAppAccessToUser(userId string, authRequest *model.Author } if result := <-a.Srv.Store.Preference().Save(&model.Preferences{authorizedApp}); result.Err != nil { + mlog.Error(result.Err.Error()) return authRequest.RedirectUri + "?error=server_error&state=" + authRequest.State, nil } - if result := <-a.Srv.Store.OAuth().SaveAuthData(authData); result.Err != nil { - return authRequest.RedirectUri + "?error=server_error&state=" + authRequest.State, nil + return redirectURI, nil +} + +func (a *App) GetOAuthAccessTokenForImplicitFlow(userId string, authRequest *model.AuthorizeRequest) (*model.Session, *model.AppError) { + if !a.Config().ServiceSettings.EnableOAuthServiceProvider { + return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.disabled.app_error", nil, "", http.StatusNotImplemented) } - return authRequest.RedirectUri + "?code=" + url.QueryEscape(authData.Code) + "&state=" + url.QueryEscape(authData.State), nil + var oauthApp *model.OAuthApp + oauthApp, err := a.GetOAuthApp(authRequest.ClientId) + if err != nil { + return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.credentials.app_error", nil, "", http.StatusNotFound) + } + + user, err := a.GetUser(userId) + if err != nil { + return nil, err + } + + session, err := a.newSession(oauthApp.Name, user) + if err != nil { + return nil, err + } + + return session, nil } -func (a *App) GetOAuthAccessToken(clientId, grantType, redirectUri, code, secret, refreshToken string) (*model.AccessResponse, *model.AppError) { +func (a *App) GetOAuthAccessTokenForCodeFlow(clientId, grantType, redirectUri, code, secret, refreshToken string) (*model.AccessResponse, *model.AppError) { if !a.Config().ServiceSettings.EnableOAuthServiceProvider { return nil, model.NewAppError("GetOAuthAccessToken", "api.oauth.get_access_token.disabled.app_error", nil, "", http.StatusNotImplemented) } diff --git a/app/oauth_test.go b/app/oauth_test.go index 60854a354..70cd5460a 100644 --- a/app/oauth_test.go +++ b/app/oauth_test.go @@ -7,8 +7,59 @@ import ( "testing" "github.com/mattermost/mattermost-server/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestGetOAuthAccessTokenForImplicitFlow(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableOAuthServiceProvider = true }) + + oapp := &model.OAuthApp{ + Name: "fakeoauthapp" + model.NewRandomString(10), + CreatorId: th.BasicUser2.Id, + Homepage: "https://nowhere.com", + Description: "test", + CallbackUrls: []string{"https://nowhere.com"}, + } + + oapp, err := th.App.CreateOAuthApp(oapp) + require.Nil(t, err) + + authRequest := &model.AuthorizeRequest{ + ResponseType: model.IMPLICIT_RESPONSE_TYPE, + ClientId: oapp.Id, + RedirectUri: oapp.CallbackUrls[0], + Scope: "", + State: "123", + } + + session, err := th.App.GetOAuthAccessTokenForImplicitFlow(th.BasicUser.Id, authRequest) + assert.Nil(t, err) + assert.NotNil(t, session) + + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableOAuthServiceProvider = false }) + + session, err = th.App.GetOAuthAccessTokenForImplicitFlow(th.BasicUser.Id, authRequest) + assert.NotNil(t, err, "should fail - oauth2 disabled") + assert.Nil(t, session) + + th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableOAuthServiceProvider = true }) + authRequest.ClientId = "junk" + + session, err = th.App.GetOAuthAccessTokenForImplicitFlow(th.BasicUser.Id, authRequest) + assert.NotNil(t, err, "should fail - bad client id") + assert.Nil(t, session) + + authRequest.ClientId = oapp.Id + + session, err = th.App.GetOAuthAccessTokenForImplicitFlow("junk", authRequest) + assert.NotNil(t, err, "should fail - bad user id") + assert.Nil(t, session) +} + func TestOAuthRevokeAccessToken(t *testing.T) { th := Setup() defer th.TearDown() diff --git a/model/authorize.go b/model/authorize.go index 9fd5afa70..22325b181 100644 --- a/model/authorize.go +++ b/model/authorize.go @@ -12,6 +12,7 @@ import ( const ( AUTHCODE_EXPIRE_TIME = 60 * 10 // 10 minutes AUTHCODE_RESPONSE_TYPE = "code" + IMPLICIT_RESPONSE_TYPE = "token" DEFAULT_SCOPE = "user" ) @@ -58,7 +59,7 @@ func (ad *AuthData) IsValid() *AppError { return NewAppError("AuthData.IsValid", "model.authorize.is_valid.create_at.app_error", nil, "client_id="+ad.ClientId, http.StatusBadRequest) } - if len(ad.RedirectUri) == 0 || len(ad.RedirectUri) > 256 || !IsValidHttpUrl(ad.RedirectUri) { + if len(ad.RedirectUri) > 256 || !IsValidHttpUrl(ad.RedirectUri) { return NewAppError("AuthData.IsValid", "model.authorize.is_valid.redirect_uri.app_error", nil, "client_id="+ad.ClientId, http.StatusBadRequest) } diff --git a/model/oauth.go b/model/oauth.go index 0ea1aa4e2..6f662a5ab 100644 --- a/model/oauth.go +++ b/model/oauth.go @@ -109,7 +109,6 @@ func (a *OAuthApp) PreUpdate() { a.UpdateAt = GetMillis() } -// ToJson convert a User to a json string func (a *OAuthApp) ToJson() string { b, _ := json.Marshal(a) return string(b) @@ -135,7 +134,6 @@ func (a *OAuthApp) IsValidRedirectURL(url string) bool { return false } -// OAuthAppFromJson will decode the input and return a User func OAuthAppFromJson(data io.Reader) *OAuthApp { var app *OAuthApp json.NewDecoder(data).Decode(&app) |