diff --git a/web/params.go b/web/params.go index 4facaf8a1f..20c944355b 100644 --- a/web/params.go +++ b/web/params.go @@ -101,29 +101,12 @@ func ParamsFromRequest(r *http.Request) *Params { props := mux.Vars(r) query := r.URL.Query() - if val, ok := props["user_id"]; ok { - params.UserId = val - } - - if val, ok := props["team_id"]; ok { - params.TeamId = val - } - - if val, ok := props["category_id"]; ok { - params.CategoryId = val - } - - if val, ok := props["invite_id"]; ok { - params.InviteId = val - } - - if val, ok := props["token_id"]; ok { - params.TokenId = val - } - - if val, ok := props["thread_id"]; ok { - params.ThreadId = val - } + params.UserId = props["user_id"] + params.TeamId = props["team_id"] + params.CategoryId = props["category_id"] + params.InviteId = props["invite_id"] + params.TokenId = props["token_id"] + params.ThreadId = props["thread_id"] if val, ok := props["channel_id"]; ok { params.ChannelId = val @@ -131,116 +114,34 @@ func ParamsFromRequest(r *http.Request) *Params { params.ChannelId = query.Get("channel_id") } - if val, ok := props["post_id"]; ok { - params.PostId = val - } - - if val, ok := props["policy_id"]; ok { - params.PolicyId = val - } - - if val, ok := props["file_id"]; ok { - params.FileId = val - } - + params.PostId = props["post_id"] + params.PolicyId = props["policy_id"] + params.FileId = props["file_id"] params.Filename = query.Get("filename") - - if val, ok := props["upload_id"]; ok { - params.UploadId = val - } - - if val, ok := props["plugin_id"]; ok { - params.PluginId = val - } - - if val, ok := props["command_id"]; ok { - params.CommandId = val - } - - if val, ok := props["hook_id"]; ok { - params.HookId = val - } - - if val, ok := props["report_id"]; ok { - params.ReportId = val - } - - if val, ok := props["emoji_id"]; ok { - params.EmojiId = val - } - - if val, ok := props["app_id"]; ok { - params.AppId = val - } - - if val, ok := props["email"]; ok { - params.Email = val - } - - if val, ok := props["username"]; ok { - params.Username = val - } - - if val, ok := props["team_name"]; ok { - params.TeamName = strings.ToLower(val) - } - - if val, ok := props["channel_name"]; ok { - params.ChannelName = strings.ToLower(val) - } - - if val, ok := props["category"]; ok { - params.Category = val - } - - if val, ok := props["service"]; ok { - params.Service = val - } - - if val, ok := props["preference_name"]; ok { - params.PreferenceName = val - } - - if val, ok := props["emoji_name"]; ok { - params.EmojiName = val - } - - if val, ok := props["job_id"]; ok { - params.JobId = val - } - - if val, ok := props["job_type"]; ok { - params.JobType = val - } - - if val, ok := props["action_id"]; ok { - params.ActionId = val - } - - if val, ok := props["role_id"]; ok { - params.RoleId = val - } - - if val, ok := props["role_name"]; ok { - params.RoleName = val - } - - if val, ok := props["scheme_id"]; ok { - params.SchemeId = val - } - - if val, ok := props["group_id"]; ok { - params.GroupId = val - } - - if val, ok := props["remote_id"]; ok { - params.RemoteId = val - } - - if val, ok := props["invoice_id"]; ok { - params.InvoiceId = val - } - + params.UploadId = props["upload_id"] + params.PluginId = props["plugin_id"] + params.CommandId = props["command_id"] + params.HookId = props["hook_id"] + params.ReportId = props["report_id"] + params.EmojiId = props["emoji_id"] + params.AppId = props["app_id"] + params.Email = props["email"] + params.Username = props["username"] + params.TeamName = strings.ToLower(props["team_name"]) + params.ChannelName = strings.ToLower(props["channel_name"]) + params.Category = props["category"] + params.Service = props["service"] + params.PreferenceName = props["preference_name"] + params.EmojiName = props["emoji_name"] + params.JobId = props["job_id"] + params.JobType = props["job_type"] + params.ActionId = props["action_id"] + params.RoleId = props["role_id"] + params.RoleName = props["role_name"] + params.SchemeId = props["scheme_id"] + params.GroupId = props["group_id"] + params.RemoteId = props["remote_id"] + params.InvoiceId = props["invoice_id"] params.Scope = query.Get("scope") if val, err := strconv.Atoi(query.Get("page")); err != nil || val < 0 { @@ -256,12 +157,9 @@ func ParamsFromRequest(r *http.Request) *Params { } params.TimeRange = query.Get("time_range") - - if val, err := strconv.ParseBool(query.Get("permanent")); err == nil { - params.Permanent = val - } - + params.Permanent, _ = strconv.ParseBool(query.Get("permanent")) params.PerPage = getPerPageFromQuery(query) + if val, err := strconv.Atoi(query.Get("logs_per_page")); err != nil || val < 0 { params.LogsPerPage = LogsPerPageDefault } else if val > LogsPerPageMaximum { @@ -286,23 +184,16 @@ func ParamsFromRequest(r *http.Request) *Params { params.LimitBefore = val } - if val, ok := props["syncable_id"]; ok { - params.SyncableId = val - } - - if val, ok := props["syncable_type"]; ok { - switch val { - case "teams": - params.SyncableType = model.GroupSyncableTypeTeam - case "channels": - params.SyncableType = model.GroupSyncableTypeChannel - } - } - - if val, ok := props["bot_user_id"]; ok { - params.BotUserId = val + params.SyncableId = props["syncable_id"] + + switch props["syncable_type"] { + case "teams": + params.SyncableType = model.GroupSyncableTypeTeam + case "channels": + params.SyncableType = model.GroupSyncableTypeChannel } + params.BotUserId = props["bot_user_id"] params.Q = query.Get("q") if val, err := strconv.ParseBool(query.Get("is_linked")); err == nil { @@ -315,57 +206,27 @@ func ParamsFromRequest(r *http.Request) *Params { params.NotAssociatedToTeam = query.Get("not_associated_to_team") params.NotAssociatedToChannel = query.Get("not_associated_to_channel") - - if val, err := strconv.ParseBool(query.Get("filter_allow_reference")); err == nil { - params.FilterAllowReference = val - } - - if val, err := strconv.ParseBool(query.Get("filter_parent_team_permitted")); err == nil { - params.FilterParentTeamPermitted = val - } + params.FilterAllowReference, _ = strconv.ParseBool(query.Get("filter_allow_reference")) + params.FilterParentTeamPermitted, _ = strconv.ParseBool(query.Get("filter_parent_team_permitted")) if val, err := strconv.ParseBool(query.Get("paginate")); err == nil { params.Paginate = &val } - if val, err := strconv.ParseBool(query.Get("include_member_count")); err == nil { - params.IncludeMemberCount = val - } - + params.IncludeMemberCount, _ = strconv.ParseBool(query.Get("include_member_count")) params.NotAssociatedToGroup = query.Get("not_associated_to_group") - - if val, err := strconv.ParseBool(query.Get("exclude_default_channels")); err == nil { - params.ExcludeDefaultChannels = val - } - + params.ExcludeDefaultChannels, _ = strconv.ParseBool(query.Get("exclude_default_channels")) params.GroupIDs = query.Get("group_ids") - - if val, err := strconv.ParseBool(query.Get("include_total_count")); err == nil { - params.IncludeTotalCount = val - } - - if val, err := strconv.ParseBool(query.Get("include_deleted")); err == nil { - params.IncludeDeleted = val - } - - if val, ok := props["warn_metric_id"]; ok { - params.WarnMetricId = val - } - - if val, ok := props["export_name"]; ok { - params.ExportName = val - } - - if val, err := strconv.ParseBool(query.Get("exclude_policy_constrained")); err == nil { - params.ExcludePolicyConstrained = val - } + params.IncludeTotalCount, _ = strconv.ParseBool(query.Get("include_total_count")) + params.IncludeDeleted, _ = strconv.ParseBool(query.Get("include_deleted")) + params.WarnMetricId = props["warn_metric_id"] + params.ExportName = props["export_name"] + params.ExcludePolicyConstrained, _ = strconv.ParseBool(query.Get("exclude_policy_constrained")) if val := query.Get("group_source"); val != "" { switch val { case "custom": params.GroupSource = model.GroupSourceCustom - case "ldap": - params.GroupSource = model.GroupSourceLdap default: params.GroupSource = model.GroupSourceLdap } diff --git a/web/params_test.go b/web/params_test.go index 54d8d82ddc..82810acf8d 100644 --- a/web/params_test.go +++ b/web/params_test.go @@ -3,9 +3,12 @@ package web import ( + "net/http" "net/url" "testing" + "github.com/gorilla/mux" + "github.com/mattermost/mattermost-server/v6/model" "github.com/stretchr/testify/require" ) @@ -32,3 +35,452 @@ func TestGetPerPageFromQuery(t *testing.T) { require.Equal(t, 100, perPage) }) } + +func TestParamsFromRequest(t *testing.T) { + testCases := []struct { + Description string + URL *url.URL + Vars map[string]string + Params *Params + }{ + { + "empty params", + mustURL("/"), + nil, + &Params{ + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitAfter: LimitDefault, + LimitBefore: LimitDefault, + }, + }, + { + "query params", + mustURL("/page?" + + "channel_id=abc123&" + + "filename=file.ext&" + + "page=42&" + + "time_range=then-till-now&" + + "permanent=1&" + + "logs_per_page=5&" + + "limit_after=6&" + + "limit_before=7&" + + "q=picard&" + + "is_linked=t&" + + "is_configured=TRUE&" + + "not_associated_to_team=this_team&" + + "not_associated_to_channel=this_channel&" + + "filter_allow_reference=true&" + + "filter_parent_team_permitted=True&" + + "paginate=T&" + + "include_member_count=1&" + + "not_associated_to_group=test&" + + "exclude_default_channels=1&" + + "group_ids=hello,world&" + + "include_total_count=T&" + + "include_deleted=True&" + + "exclude_policy_constrained=TRUE&" + + "filter_has_member=xyz"), + nil, + &Params{ + ChannelId: "abc123", + Filename: "file.ext", + Page: 42, + TimeRange: "then-till-now", + PerPage: PerPageDefault, + Permanent: true, + LogsPerPage: 5, + LimitAfter: 6, + LimitBefore: 7, + Q: "picard", + IsLinked: boolPtr(true), + IsConfigured: boolPtr(true), + NotAssociatedToTeam: "this_team", + NotAssociatedToChannel: "this_channel", + FilterAllowReference: true, + FilterParentTeamPermitted: true, + Paginate: boolPtr(true), + IncludeMemberCount: true, + NotAssociatedToGroup: "test", + ExcludeDefaultChannels: true, + GroupIDs: "hello,world", + IncludeTotalCount: true, + IncludeDeleted: true, + ExcludePolicyConstrained: true, + FilterHasMember: "xyz", + }, + }, + { + "page invalid", + mustURL("?page=hello"), + nil, + &Params{ + Page: PageDefault, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitAfter: LimitDefault, + LimitBefore: LimitDefault, + }, + }, + { + "page negative", + mustURL("?page=-1"), + nil, + &Params{ + Page: PageDefault, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitAfter: LimitDefault, + LimitBefore: LimitDefault, + }, + }, + { + "per page valid", + mustURL("?per_page=123"), + nil, + &Params{ + PerPage: 123, + + LogsPerPage: LogsPerPageDefault, + LimitAfter: LimitDefault, + LimitBefore: LimitDefault, + }, + }, + { + "per page too small", + mustURL("?per_page=-100"), + nil, + &Params{ + PerPage: PerPageDefault, + + LogsPerPage: LogsPerPageDefault, + LimitAfter: LimitDefault, + LimitBefore: LimitDefault, + }, + }, + { + "per page too big", + mustURL("?per_page=100000"), + nil, + &Params{ + PerPage: PerPageMaximum, + + LogsPerPage: LogsPerPageDefault, + LimitAfter: LimitDefault, + LimitBefore: LimitDefault, + }, + }, + { + "logs per page valid", + mustURL("?logs_per_page=512"), + nil, + &Params{ + LogsPerPage: 512, + + PerPage: PerPageDefault, + LimitAfter: LimitDefault, + LimitBefore: LimitDefault, + }, + }, + { + "logs per page invalid", + mustURL("?logs_per_page=logs"), + nil, + &Params{ + LogsPerPage: LogsPerPageDefault, + + PerPage: PerPageDefault, + LimitAfter: LimitDefault, + LimitBefore: LimitDefault, + }, + }, + { + "logs per page too small", + mustURL("?logs_per_page=-512"), + nil, + &Params{ + LogsPerPage: LogsPerPageDefault, + + PerPage: PerPageDefault, + LimitAfter: LimitDefault, + LimitBefore: LimitDefault, + }, + }, + { + "logs per page too big", + mustURL("?logs_per_page=99999999"), + nil, + &Params{ + LogsPerPage: LogsPerPageMaximum, + + PerPage: PerPageDefault, + LimitAfter: LimitDefault, + LimitBefore: LimitDefault, + }, + }, + { + "limit before valid", + mustURL("?limit_before=100"), + nil, + &Params{ + LimitBefore: 100, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitAfter: LimitDefault, + }, + }, + { + "limit before invalid", + mustURL("?limit_before=limit"), + nil, + &Params{ + LimitBefore: LimitDefault, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitAfter: LimitDefault, + }, + }, + { + "limit before too small", + mustURL("?limit_before=-100"), + nil, + &Params{ + LimitBefore: LimitDefault, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitAfter: LimitDefault, + }, + }, + { + "limit before too big", + mustURL("?limit_before=99999"), + nil, + &Params{ + LimitBefore: LimitMaximum, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitAfter: LimitDefault, + }, + }, + { + "limit after valid", + mustURL("?limit_after=100"), + nil, + &Params{ + LimitAfter: 100, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + }, + }, + { + "limit after invalid", + mustURL("?limit_after=limit"), + nil, + &Params{ + LimitAfter: LimitDefault, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + }, + }, + { + "limit after too small", + mustURL("?limit_aftere=-100"), + nil, + &Params{ + LimitAfter: LimitDefault, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + }, + }, + { + "limit after too big", + mustURL("?limit_after=99999"), + nil, + &Params{ + LimitAfter: LimitMaximum, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + }, + }, + { + "group source custom", + mustURL("?group_source=custom"), + nil, + &Params{ + GroupSource: model.GroupSourceCustom, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + LimitAfter: LimitDefault, + }, + }, + { + "group source LDAP", + mustURL("?group_source=ldap"), + nil, + &Params{ + GroupSource: model.GroupSourceLdap, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + LimitAfter: LimitDefault, + }, + }, + { + "group source other", + mustURL("?group_source=aabbcc"), + nil, + &Params{ + GroupSource: model.GroupSourceLdap, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + LimitAfter: LimitDefault, + }, + }, + { + "group source empty", + mustURL("?group_souce="), + nil, + &Params{ + GroupSource: "", + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + LimitAfter: LimitDefault, + }, + }, + { + "timestamp valid", + mustURL("/"), + map[string]string{ + "timestamp": "1234567", + }, + &Params{ + Timestamp: 1234567, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + LimitAfter: LimitDefault, + }, + }, + { + "timestamp valid", + mustURL("/"), + map[string]string{ + "timestamp": "yes", + }, + &Params{ + Timestamp: 0, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + LimitAfter: LimitDefault, + }, + }, + { + "timestamp too small", + mustURL("/"), + map[string]string{ + "timestamp": "-1234567", + }, + &Params{ + Timestamp: 0, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + LimitAfter: LimitDefault, + }, + }, + { + "syncable type teams", + mustURL("/"), + map[string]string{ + "syncable_type": "teams", + }, + &Params{ + SyncableType: model.GroupSyncableTypeTeam, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + LimitAfter: LimitDefault, + }, + }, + { + "syncable type channels", + mustURL("/"), + map[string]string{ + "syncable_type": "channels", + }, + &Params{ + SyncableType: model.GroupSyncableTypeChannel, + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + LimitAfter: LimitDefault, + }, + }, + { + "syncable type other", + mustURL("/"), + map[string]string{ + "syncable_type": "unknownvalue", + }, + &Params{ + SyncableType: "", + + PerPage: PerPageDefault, + LogsPerPage: LogsPerPageDefault, + LimitBefore: LimitDefault, + LimitAfter: LimitDefault, + }, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.Description, func(t *testing.T) { + t.Parallel() + + r := &http.Request{URL: testCase.URL} + r = mux.SetURLVars(r, testCase.Vars) + params := ParamsFromRequest(r) + require.Equal(t, testCase.Params, params) + }) + } +} + +func mustURL(u string) *url.URL { + parsed, err := url.Parse(u) + if err != nil { + panic(err) + } + return parsed +} + +func boolPtr(b bool) *bool { + return &b +}