IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2026-08-27 05:37:15 -05:00
Code Issues Packages Projects Releases Wiki Activity

Tighten authorization on OAuth deauthorize and access token endpoints (#37153)

Browse Source 
Restrict OAuth deauthorization and personal access token lifecycle
actions to direct user sessions, denying delegated OAuth app tokens,
consistent with the existing OAuth authorize and access token creation
handlers. Adds regression tests covering the OAuth-session case.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Maria A Nunez
2026-06-25 12:56:44 -04:00
committed by GitHub
co-authored by Cursor
parent c9b42e932f
commit 1fd45ab33e
4 changed files with 143 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/channels/api4/user.go
+18
View File
@@ -3135,6 +3135,12 @@ func revokeUserAccessToken(c *Context, w http.ResponseWriter, r *http.Request) {
model.AddEventParameterToAuditRec(auditRec, "token_id", tokenId)
c.LogAudit("")
if c.AppContext.Session().IsOAuth {
c.SetPermissionError(model.PermissionRevokeUserAccessToken)
c.Err.DetailedError += ", attempted access by oauth app"
return
}
if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionRevokeUserAccessToken) {
c.SetPermissionError(model.PermissionRevokeUserAccessToken)
return
@@ -3179,6 +3185,12 @@ func disableUserAccessToken(c *Context, w http.ResponseWriter, r *http.Request)
defer c.LogAuditRec(auditRec)
c.LogAudit("")
if c.AppContext.Session().IsOAuth {
c.SetPermissionError(model.PermissionRevokeUserAccessToken)
c.Err.DetailedError += ", attempted access by oauth app"
return
}
// No separate permission for this action for now
if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionRevokeUserAccessToken) {
c.SetPermissionError(model.PermissionRevokeUserAccessToken)
@@ -3224,6 +3236,12 @@ func enableUserAccessToken(c *Context, w http.ResponseWriter, r *http.Request) {
model.AddEventParameterToAuditRec(auditRec, "token_id", tokenId)
c.LogAudit("")
if c.AppContext.Session().IsOAuth {
c.SetPermissionError(model.PermissionCreateUserAccessToken)
c.Err.DetailedError += ", attempted access by oauth app"
return
}
// No separate permission for this action for now
if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionCreateUserAccessToken) {
c.SetPermissionError(model.PermissionCreateUserAccessToken)
server/channels/api4/user_test.go
+70
View File
@@ -6986,6 +6986,76 @@ func TestEnableUserAccessToken(t *testing.T) {
})
}
func TestRevokeUserAccessTokenDeniesOAuthSession(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true })
_, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false)
require.Nil(t, appErr)
token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0)
require.NoError(t, err)
assertToken(t, th, token, th.BasicUser.Id)
session, _ := th.App.GetSession(th.Client.AuthToken)
session.IsOAuth = true
th.App.AddSessionToCache(session)
resp, err := th.Client.RevokeUserAccessToken(context.Background(), token.Id)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
}
func TestDisableUserAccessTokenDeniesOAuthSession(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true })
_, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false)
require.Nil(t, appErr)
token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0)
require.NoError(t, err)
assertToken(t, th, token, th.BasicUser.Id)
session, _ := th.App.GetSession(th.Client.AuthToken)
session.IsOAuth = true
th.App.AddSessionToCache(session)
resp, err := th.Client.DisableUserAccessToken(context.Background(), token.Id)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
}
func TestEnableUserAccessTokenDeniesOAuthSession(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true })
_, appErr := th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserRoleId+" "+model.SystemUserAccessTokenRoleId, false)
require.Nil(t, appErr)
token, _, err := th.Client.CreateUserAccessToken(context.Background(), th.BasicUser.Id, "test token", 0)
require.NoError(t, err)
assertToken(t, th, token, th.BasicUser.Id)
_, err = th.Client.DisableUserAccessToken(context.Background(), token.Id)
require.NoError(t, err)
assertInvalidToken(t, th, token)
session, _ := th.App.GetSession(th.Client.AuthToken)
session.IsOAuth = true
th.App.AddSessionToCache(session)
resp, err := th.Client.EnableUserAccessToken(context.Background(), token.Id)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
}
func TestUserAccessTokenInactiveUser(t *testing.T) {
mainHelper.Parallel(t)
server/channels/web/oauth.go
+6
View File
@@ -105,6 +105,12 @@ func deauthorizeOAuthApp(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
if c.AppContext.Session().IsOAuth {
c.SetPermissionError(model.PermissionEditOtherUsers)
c.Err.DetailedError += ", attempted access by oauth app"
return
}
auditRec := c.MakeAuditRecord(model.AuditEventDeauthorizeOAuthApp, model.AuditStatusFail)
auditRec.AddMeta("client_id", clientId)
defer c.LogAuditRec(auditRec)
server/channels/web/oauth_test.go
+49
View File
@@ -227,6 +227,55 @@ func TestDeauthorizeOAuthApp(t *testing.T) {
CheckUnauthorizedStatus(t, resp)
}
func TestDeauthorizeOAuthAppDeniesOAuthSession(t *testing.T) {
th := Setup(t).InitBasic(t)
th.Login(t, apiClient, th.SystemAdminUser)
enableOAuth := *th.App.Config().ServiceSettings.EnableOAuthServiceProvider
defer func() {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = enableOAuth })
}()
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOAuthServiceProvider = true })
oapp := &model.OAuthApp{
Name: GenerateTestAppName(),
Homepage: "https://nowhere.com",
Description: "test",
CallbackUrls: []string{"https://nowhere.com"},
CreatorId: th.SystemAdminUser.Id,
ClientSecret: model.NewId(),
}
rapp, appErr := th.App.CreateOAuthApp(oapp)
require.Nil(t, appErr)
authRequest := &model.AuthorizeRequest{
ResponseType: model.ImplicitResponseType,
ClientId: rapp.Id,
RedirectURI: rapp.CallbackUrls[0],
Scope: "",
State: "123",
}
ruri, _, err := apiClient.AuthorizeOAuthApp(context.Background(), authRequest)
require.NoError(t, err)
ru, err := url.Parse(ruri)
require.NoError(t, err)
values, err := url.ParseQuery(ru.Fragment)
require.NoError(t, err)
oauthToken := values.Get("access_token")
require.NotEmpty(t, oauthToken)
oldToken := apiClient.AuthToken
apiClient.AuthToken = oauthToken
defer func() { apiClient.AuthToken = oldToken }()
resp, err := apiClient.DeauthorizeOAuthApp(context.Background(), rapp.Id)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
}
func TestOAuthAccessToken(t *testing.T) {
if testing.Short() {
t.SkipNow()