From 049e67b863e5dcf1eb81b53a63f89664d517a19c Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 12 Jul 2022 23:26:48 +0530 Subject: [PATCH] MM-45535: Batch optimize auth checks in GraphQL (#20634) We create two new auth checks which take multiple channels and teams. They can be used to check whenever a user needs access to multiple entities. These are then access in dataloaders to ease the load in the database. ```release-note NONE ``` --- api4/resolver_channel_member.go | 44 +++++++------ api4/resolver_team.go | 18 +++--- app/app_iface.go | 4 ++ app/authorization.go | 93 ++++++++++++++++++++++++++++ app/opentracing/opentracing_layer.go | 34 ++++++++++ model/session.go | 6 +- 6 files changed, 171 insertions(+), 28 deletions(-) diff --git a/api4/resolver_channel_member.go b/api4/resolver_channel_member.go index 32329f7f4b..f2fcd597f4 100644 --- a/api4/resolver_channel_member.go +++ b/api4/resolver_channel_member.go @@ -26,11 +26,6 @@ func (cm *channelMember) User(ctx context.Context) (*user, error) { // match with api4.Channel func (cm *channelMember) Channel(ctx context.Context) (*channel, error) { - c, err := getCtx(ctx) - if err != nil { - return nil, err - } - loader, err := getChannelsLoader(ctx) if err != nil { return nil, err @@ -43,19 +38,6 @@ func (cm *channelMember) Channel(ctx context.Context) (*channel, error) { } channel := result.(*channel) - if channel.Type == model.ChannelTypeOpen { - if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionReadPublicChannel) && - !c.App.SessionHasPermissionToChannel(*c.AppContext.Session(), cm.ChannelId, model.PermissionReadChannel) { - c.SetPermissionError(model.PermissionReadPublicChannel) - return nil, c.Err - } - } else { - if !c.App.SessionHasPermissionToChannel(*c.AppContext.Session(), cm.ChannelId, model.PermissionReadChannel) { - c.SetPermissionError(model.PermissionReadChannel) - return nil, c.Err - } - } - return channel, nil } @@ -95,6 +77,32 @@ func getGraphQLChannels(c *web.Context, channelIDs []string) ([]*channel, error) return nil, fmt.Errorf("all channels were not found. Requested %d; Found %d", len(channelIDs), len(channels)) } + var openChannels, nonOpenChannels, teamsForOpenChannels []string + uniqueTeams := make(map[string]bool) + for _, ch := range channels { + if ch.Type == model.ChannelTypeOpen { + openChannels = append(openChannels, ch.Id) + uniqueTeams[ch.TeamId] = true + } else { + nonOpenChannels = append(nonOpenChannels, ch.Id) + } + } + + for teamID := range uniqueTeams { + teamsForOpenChannels = append(teamsForOpenChannels, teamID) + } + + if len(openChannels) > 0 && !c.App.SessionHasPermissionToChannels(*c.AppContext.Session(), openChannels, model.PermissionReadChannel) && + !c.App.SessionHasPermissionToTeams(*c.AppContext.Session(), teamsForOpenChannels, model.PermissionReadPublicChannel) { + c.SetPermissionError(model.PermissionReadPublicChannel) + return nil, c.Err + } + + if len(nonOpenChannels) > 0 && !c.App.SessionHasPermissionToChannels(*c.AppContext.Session(), nonOpenChannels, model.PermissionReadChannel) { + c.SetPermissionError(model.PermissionReadChannel) + return nil, c.Err + } + appErr = c.App.FillInChannelsProps(model.ChannelList(channels)) if appErr != nil { return nil, appErr diff --git a/api4/resolver_team.go b/api4/resolver_team.go index 9dd864f6f0..7f2435b64c 100644 --- a/api4/resolver_team.go +++ b/api4/resolver_team.go @@ -64,15 +64,19 @@ func getGraphQLTeams(c *web.Context, teamIDs []string) ([]*model.Team, error) { return nil, fmt.Errorf("all teams were not found. Requested %d; Found %d", len(teamIDs), len(teams)) } - // We pre-calculate this so that it's not computed in separate goroutines outside - // the dataloader. - for i, team := range teams { - if (!team.AllowOpenInvite || team.Type != model.TeamOpen) && - !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), team.Id, model.PermissionViewTeam) { - c.SetPermissionError(model.PermissionViewTeam) - return nil, c.Err + var teamsToCheck []string + for _, team := range teams { + if !team.AllowOpenInvite || team.Type != model.TeamOpen { + teamsToCheck = append(teamsToCheck, team.Id) } + } + if !c.App.SessionHasPermissionToTeams(*c.AppContext.Session(), teamsToCheck, model.PermissionViewMembers) { + c.SetPermissionError(model.PermissionViewTeam) + return nil, c.Err + } + + for i, team := range teams { teams[i] = c.App.SanitizeTeam(*c.AppContext.Session(), team) } diff --git a/app/app_iface.go b/app/app_iface.go index 066374144a..c193d08007 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -298,10 +298,14 @@ type AppIface interface { SearchAllTeams(searchOpts *model.TeamSearch) ([]*model.Team, int64, *model.AppError) // SendNoCardPaymentFailedEmail SendNoCardPaymentFailedEmail() *model.AppError + // SessionHasPermissionToChannels returns true only if user has access to all channels. + SessionHasPermissionToChannels(session model.Session, channelIDs []string, permission *model.Permission) bool // SessionHasPermissionToManageBot returns nil if the session has access to manage the given bot. // This function deviates from other authorization checks in returning an error instead of just // a boolean, allowing the permission failure to be exposed with more granularity. SessionHasPermissionToManageBot(session model.Session, botUserId string) *model.AppError + // SessionHasPermissionToTeams returns true only if user has access to all teams. + SessionHasPermissionToTeams(session model.Session, teamIDs []string, permission *model.Permission) bool // SessionIsRegistered determines if a specific session has been registered SessionIsRegistered(session model.Session) bool // SetSessionExpireInHours sets the session's expiry the specified number of hours diff --git a/app/authorization.go b/app/authorization.go index a5064f93a6..96607a7b74 100644 --- a/app/authorization.go +++ b/app/authorization.go @@ -57,6 +57,40 @@ func (a *App) SessionHasPermissionToTeam(session model.Session, teamID string, p return a.RolesGrantPermission(session.GetUserRoles(), permission.Id) } +// SessionHasPermissionToTeams returns true only if user has access to all teams. +func (a *App) SessionHasPermissionToTeams(session model.Session, teamIDs []string, permission *model.Permission) bool { + for _, teamID := range teamIDs { + if teamID == "" { + return false + } + } + if session.IsUnrestricted() { + return true + } + + // Getting the list of unique roles from all teams. + var roles []string + uniqueRoles := make(map[string]bool) + for _, teamID := range teamIDs { + tm := session.GetTeamByTeamId(teamID) + if tm != nil { + for _, role := range tm.GetRoles() { + uniqueRoles[role] = true + } + } + } + + for role := range uniqueRoles { + roles = append(roles, role) + } + + if a.RolesGrantPermission(roles, permission.Id) { + return true + } + + return a.RolesGrantPermission(session.GetUserRoles(), permission.Id) +} + func (a *App) SessionHasPermissionToChannel(session model.Session, channelID string, permission *model.Permission) bool { if channelID == "" { return false @@ -90,6 +124,65 @@ func (a *App) SessionHasPermissionToChannel(session model.Session, channelID str return a.SessionHasPermissionTo(session, permission) } +// SessionHasPermissionToChannels returns true only if user has access to all channels. +func (a *App) SessionHasPermissionToChannels(session model.Session, channelIDs []string, permission *model.Permission) bool { + for _, channelID := range channelIDs { + if channelID == "" { + return false + } + } + + if session.IsUnrestricted() { + return true + } + + ids, err := a.Srv().Store.Channel().GetAllChannelMembersForUser(session.UserId, true, true) + + var channelRoles []string + uniqueRoles := make(map[string]bool) + if err == nil { + for _, channelID := range channelIDs { + if roles, ok := ids[channelID]; ok { + for _, role := range strings.Fields(roles) { + uniqueRoles[role] = true + } + } + } + } + + for role := range uniqueRoles { + channelRoles = append(channelRoles, role) + } + + if a.RolesGrantPermission(channelRoles, permission.Id) { + return true + } + + channels, appErr := a.GetChannels(channelIDs) + if appErr != nil && appErr.StatusCode == http.StatusNotFound { + return false + } + + // Get TeamIDs from channels + uniqueTeamIDs := make(map[string]bool) + for _, ch := range channels { + if ch.TeamId != "" { + uniqueTeamIDs[ch.TeamId] = true + } + } + + var teamIDs []string + for teamID := range uniqueTeamIDs { + teamIDs = append(teamIDs, teamID) + } + + if appErr == nil && len(teamIDs) > 0 { + return a.SessionHasPermissionToTeams(session, teamIDs, permission) + } + + return a.SessionHasPermissionTo(session, permission) +} + func (a *App) SessionHasPermissionToGroup(session model.Session, groupID string, permission *model.Permission) bool { groupMember, err := a.Srv().Store.Group().GetMember(groupID, session.UserId) // don't reject immediately on ErrNoRows error because there's further authz logic below for non-groupmembers diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 7f1e362607..38ee875a61 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -15233,6 +15233,23 @@ func (a *OpenTracingAppLayer) SessionHasPermissionToChannelByPost(session model. return resultVar0 } +func (a *OpenTracingAppLayer) SessionHasPermissionToChannels(session model.Session, channelIDs []string, permission *model.Permission) bool { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SessionHasPermissionToChannels") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0 := a.app.SessionHasPermissionToChannels(session, channelIDs, permission) + + return resultVar0 +} + func (a *OpenTracingAppLayer) SessionHasPermissionToCreateJob(session model.Session, job *model.Job) (bool, *model.Permission) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SessionHasPermissionToCreateJob") @@ -15323,6 +15340,23 @@ func (a *OpenTracingAppLayer) SessionHasPermissionToTeam(session model.Session, return resultVar0 } +func (a *OpenTracingAppLayer) SessionHasPermissionToTeams(session model.Session, teamIDs []string, permission *model.Permission) bool { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SessionHasPermissionToTeams") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0 := a.app.SessionHasPermissionToTeams(session, teamIDs, permission) + + return resultVar0 +} + func (a *OpenTracingAppLayer) SessionHasPermissionToUser(session model.Session, userID string) bool { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SessionHasPermissionToUser") diff --git a/model/session.go b/model/session.go index 676a644a91..c880b9884f 100644 --- a/model/session.go +++ b/model/session.go @@ -147,9 +147,9 @@ func (s *Session) AddProp(key string, value string) { } func (s *Session) GetTeamByTeamId(teamId string) *TeamMember { - for _, team := range s.TeamMembers { - if team.TeamId == teamId { - return team + for _, tm := range s.TeamMembers { + if tm.TeamId == teamId { + return tm } }