From 1326ab66a141e73f1ef7d9d39bb86596f56179e0 Mon Sep 17 00:00:00 2001 From: enahum Date: Tue, 30 Aug 2016 21:15:40 -0300 Subject: PLT-3984 Add the ability to regenerate OAuth Client Secret (#3899) --- api/oauth.go | 52 +++++++++++++++++++++- api/oauth_test.go | 24 ++++++++++ i18n/en.json | 4 ++ model/client.go | 13 ++++++ store/sql_oauth_store.go | 1 - store/sql_oauth_store_test.go | 3 -- webapp/client/client.jsx | 10 +++++ .../components/installed_oauth_app.jsx | 51 +++++++++++++++++++-- webapp/i18n/en.json | 1 + 9 files changed, 150 insertions(+), 9 deletions(-) diff --git a/api/oauth.go b/api/oauth.go index d3495895f..7942b0e0c 100644 --- a/api/oauth.go +++ b/api/oauth.go @@ -32,7 +32,8 @@ func InitOAuth() { BaseRoutes.OAuth.Handle("/allow", ApiUserRequired(allowOAuth)).Methods("GET") BaseRoutes.OAuth.Handle("/authorized", ApiUserRequired(getAuthorizedApps)).Methods("GET") BaseRoutes.OAuth.Handle("/delete", ApiUserRequired(deleteOAuthApp)).Methods("POST") - BaseRoutes.OAuth.Handle("/{id:[A-Za-z0-9]+}/deauthorize", AppHandlerIndependent(deauthorizeOAuthApp)).Methods("POST") + BaseRoutes.OAuth.Handle("/{id:[A-Za-z0-9]+}/deauthorize", ApiUserRequired(deauthorizeOAuthApp)).Methods("POST") + BaseRoutes.OAuth.Handle("/{id:[A-Za-z0-9]+}/regen_secret", ApiUserRequired(regenerateOAuthSecret)).Methods("POST") BaseRoutes.OAuth.Handle("/{service:[A-Za-z0-9]+}/complete", AppHandlerIndependent(completeOAuth)).Methods("GET") BaseRoutes.OAuth.Handle("/{service:[A-Za-z0-9]+}/login", AppHandlerIndependent(loginWithOAuth)).Methods("GET") BaseRoutes.OAuth.Handle("/{service:[A-Za-z0-9]+}/signup", AppHandlerIndependent(signupWithOAuth)).Methods("GET") @@ -957,6 +958,55 @@ func deauthorizeOAuthApp(c *Context, w http.ResponseWriter, r *http.Request) { ReturnStatusOK(w) } +func regenerateOAuthSecret(c *Context, w http.ResponseWriter, r *http.Request) { + if !utils.Cfg.ServiceSettings.EnableOAuthServiceProvider { + c.Err = model.NewLocAppError("registerOAuthApp", "api.oauth.register_oauth_app.turn_off.app_error", nil, "") + c.Err.StatusCode = http.StatusNotImplemented + return + } + + isSystemAdmin := c.IsSystemAdmin() + + if *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations { + if !isSystemAdmin { + c.Err = model.NewLocAppError("registerOAuthApp", "api.command.admin_only.app_error", nil, "") + c.Err.StatusCode = http.StatusForbidden + return + } + } + + params := mux.Vars(r) + id := params["id"] + + if len(id) == 0 { + c.SetInvalidParam("regenerateOAuthSecret", "id") + return + } + + var app *model.OAuthApp + if result := <-Srv.Store.OAuth().GetApp(id); result.Err != nil { + c.Err = model.NewLocAppError("regenerateOAuthSecret", "api.oauth.allow_oauth.database.app_error", nil, "") + return + } else { + app = result.Data.(*model.OAuthApp) + + //validate that is a System Admin or the same user that registered the app + if !isSystemAdmin && app.CreatorId != c.Session.UserId { + c.Err = model.NewLocAppError("regenerateOAuthSecret", "api.oauth.regenerate_secret.app_error", nil, "") + return + } + + app.ClientSecret = model.NewId() + if update := <-Srv.Store.OAuth().UpdateApp(app); update.Err != nil { + c.Err = update.Err + return + } + + w.Write([]byte(app.ToJson())) + return + } +} + func newSession(appName string, user *model.User) (*model.Session, *model.AppError) { // set new token an session session := &model.Session{UserId: user.Id, Roles: user.Roles, IsOAuth: true} diff --git a/api/oauth_test.go b/api/oauth_test.go index 944b1a95b..a54fbc2c3 100644 --- a/api/oauth_test.go +++ b/api/oauth_test.go @@ -278,6 +278,30 @@ func TestDeauthorizeApp(t *testing.T) { } } +func TestRegenerateOAuthAppSecret(t *testing.T) { + th := Setup().InitSystemAdmin() + AdminClient := th.SystemAdminClient + + utils.Cfg.ServiceSettings.EnableOAuthServiceProvider = true + + app := &model.OAuthApp{Name: "TestApp6" + model.NewId(), Homepage: "https://nowhere.com", Description: "test", CallbackUrls: []string{"https://nowhere.com"}} + + app = AdminClient.Must(AdminClient.RegisterApp(app)).Data.(*model.OAuthApp) + + if regenApp, err := AdminClient.RegenerateOAuthAppSecret(app.Id); err != nil { + t.Fatal(err) + } else { + app2 := regenApp.Data.(*model.OAuthApp) + if app2.Id != app.Id { + t.Fatal("Should have been the same app Id") + } + + if app2.ClientSecret == app.ClientSecret { + t.Fatal("Should have been diferent client Secrets") + } + } +} + func TestOAuthDeleteApp(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() Client := th.BasicClient diff --git a/i18n/en.json b/i18n/en.json index d4324aeff..1d3c8b8a1 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1065,6 +1065,10 @@ "id": "api.oauth.init.debug", "translation": "Initializing oauth api routes" }, + { + "id": "api.oauth.regenerate_secret.app_error", + "translation": "Inappropriate permissions to regenerate the OAuth2 App Secret" + }, { "id": "api.oauth.register_oauth_app.turn_off.app_error", "translation": "The system admin has turned off OAuth2 Service Provider." diff --git a/model/client.go b/model/client.go index 86e4dccf5..e2e003fe8 100644 --- a/model/client.go +++ b/model/client.go @@ -1557,6 +1557,19 @@ func (c *Client) OAuthDeauthorizeApp(clientId string) *AppError { } } +// RegenerateOAuthAppSecret generates a new OAuth App Client Secret. On success +// it returns an OAuth2 App. Must be authenticated as a user and the same user who +// registered the app or a System Admin. +func (c *Client) RegenerateOAuthAppSecret(clientId string) (*Result, *AppError) { + if r, err := c.DoApiPost("/oauth/"+clientId+"/regen_secret", ""); err != nil { + return nil, err + } else { + defer closeBody(r) + return &Result{r.Header.Get(HEADER_REQUEST_ID), + r.Header.Get(HEADER_ETAG_SERVER), OAuthAppFromJson(r.Body)}, nil + } +} + func (c *Client) GetAccessToken(data url.Values) (*Result, *AppError) { if r, err := c.DoPost("/oauth/access_token", data.Encode(), "application/x-www-form-urlencoded"); err != nil { return nil, err diff --git a/store/sql_oauth_store.go b/store/sql_oauth_store.go index 4a15d4f80..c162f36b4 100644 --- a/store/sql_oauth_store.go +++ b/store/sql_oauth_store.go @@ -111,7 +111,6 @@ func (as SqlOAuthStore) UpdateApp(app *model.OAuthApp) StoreChannel { } else { oldApp := oldAppResult.(*model.OAuthApp) app.CreateAt = oldApp.CreateAt - app.ClientSecret = oldApp.ClientSecret app.CreatorId = oldApp.CreatorId if count, err := as.GetMaster().Update(app); err != nil { diff --git a/store/sql_oauth_store_test.go b/store/sql_oauth_store_test.go index ebf9ad59b..b9bde5be3 100644 --- a/store/sql_oauth_store_test.go +++ b/store/sql_oauth_store_test.go @@ -69,9 +69,6 @@ func TestOAuthStoreUpdateApp(t *testing.T) { if ua1.CreateAt == 1 { t.Fatal("create at should not have updated") } - if ua1.ClientSecret == "pwd" { - t.Fatal("client secret should not have updated") - } if ua1.CreatorId == "12345678901234567890123456" { t.Fatal("creator id should not have updated") } diff --git a/webapp/client/client.jsx b/webapp/client/client.jsx index 5dda975f6..b842d9939 100644 --- a/webapp/client/client.jsx +++ b/webapp/client/client.jsx @@ -1573,6 +1573,16 @@ export default class Client { end(this.handleResponse.bind(this, 'deauthorizeOAuthApp', success, error)); } + regenerateOAuthAppSecret(id, success, error) { + request. + post(`${this.getOAuthRoute()}/${id}/regen_secret`). + set(this.defaultHeaders). + type('application/json'). + accept('application/json'). + send(). + end(this.handleResponse.bind(this, 'regenerateOAuthAppSecret', success, error)); + } + // Routes for Hooks addIncomingHook(hook, success, error) { diff --git a/webapp/components/integrations/components/installed_oauth_app.jsx b/webapp/components/integrations/components/installed_oauth_app.jsx index 37fc061f7..15a79ed4c 100644 --- a/webapp/components/integrations/components/installed_oauth_app.jsx +++ b/webapp/components/integrations/components/installed_oauth_app.jsx @@ -3,6 +3,9 @@ import React from 'react'; +import FormError from 'components/form_error.jsx'; + +import Client from 'client/web_client.jsx'; import * as Utils from 'utils/utils.jsx'; import {FormattedMessage, FormattedHTMLMessage} from 'react-intl'; @@ -23,6 +26,7 @@ export default class InstalledOAuthApp extends React.Component { this.handleShowClientSecret = this.handleShowClientSecret.bind(this); this.handleHideClientScret = this.handleHideClientScret.bind(this); + this.handleRegenerate = this.handleRegenerate.bind(this); this.handleDelete = this.handleDelete.bind(this); this.matchesFilter = this.matchesFilter.bind(this); @@ -42,6 +46,21 @@ export default class InstalledOAuthApp extends React.Component { this.setState({clientSecret: FAKE_SECRET}); } + handleRegenerate(e) { + e.preventDefault(); + + Client.regenerateOAuthAppSecret( + this.props.oauthApp.id, + (data) => { + this.props.oauthApp.client_secret = data.client_secret; + this.handleShowClientSecret(e); + }, + (err) => { + this.setState({error: err.message}); + } + ); + } + handleDelete(e) { e.preventDefault(); @@ -58,6 +77,15 @@ export default class InstalledOAuthApp extends React.Component { render() { const oauthApp = this.props.oauthApp; + let error; + + if (this.state.error) { + error = ( + + ); + } if (!this.matchesFilter(oauthApp, this.props.filter)) { return null; @@ -107,9 +135,9 @@ export default class InstalledOAuthApp extends React.Component { isTrusted = Utils.localizeMessage('installed_oauth_apps.trusted.no', 'No'); } - let action; + let showHide; if (this.state.clientSecret === FAKE_SECRET) { - action = ( + showHide = ( ); } else { - action = ( + showHide = ( + + + ); + let icon; if (oauthApp.icon_url) { icon = ( @@ -152,6 +192,7 @@ export default class InstalledOAuthApp extends React.Component { {name} + {error} {description}
@@ -201,7 +242,9 @@ export default class InstalledOAuthApp extends React.Component {
- {action} + {showHide} + {' - '} + {regen} {' - '}