Skip to content
This repository has been archived by the owner on Jan 24, 2023. It is now read-only.

Ensure CF Cells info is shown for non cf admins #3121

Merged
merged 3 commits into from
Oct 19, 2018
Merged

Conversation

richard-cox
Copy link
Contributor

  • Add new /metrics/cf/cells/:op endpoint which will allow non-admins to access specific cell queries
  • Integrate new cells endpoint into front end & test with non-admin user
  • Hide cell app instances tab if not cf admin

Also

  • Minor tidy of isConnectedUserAdmin
  • Fix console when navigating from app instances to app metrics (bug in v2-master)

- Add new `/metrics/cf/cells/:op` endpoint which will allow non-admins to access specific cell queries
- Integrate new `cells` endpoint into front end & test with non-admin user
- Hide cell app instances tab if not cf admin
@cfdreddbot
Copy link

Hey richard-cox!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@codecov
Copy link

codecov bot commented Oct 15, 2018

Codecov Report

Merging #3121 into v2-master will decrease coverage by 0.03%.
The diff coverage is 80%.

@@              Coverage Diff              @@
##           v2-master    #3121      +/-   ##
=============================================
- Coverage      71.16%   71.13%   -0.04%     
=============================================
  Files            627      627              
  Lines          27407    27412       +5     
  Branches        6229     6230       +1     
=============================================
- Hits           19504    19499       -5     
- Misses          7903     7913      +10

@richard-cox
Copy link
Contributor Author

Had a look at improving the white list check and came up with the following. Let me know if I should swap it out or there's a better way to do it.

var (
	cellQueryWhiteList = map[string]int{
		"firehose_value_metric_rep_unhealthy_cell":                0,
		"firehose_value_metric_rep_capacity_remaining_containers": 0,
		"firehose_value_metric_rep_capacity_remaining_disk":       0,
		"firehose_value_metric_rep_capacity_remaining_memory":     0,
		"firehose_value_metric_rep_capacity_total_containers":     0,
		"firehose_value_metric_rep_capacity_total_disk":           0,
		"firehose_value_metric_rep_capacity_total_memory":         0,
		"firehose_value_metric_rep_num_cpus":                      0,
	}
)

func isAllowedCellMetricsQuery(query string) bool {
	safeQuery := query
	extrasIndex := strings.IndexAny(query, "{[")

	if extrasIndex >= 0 {
		safeQuery = query[0 : extrasIndex-1]
	}

	if _, ok := cellQueryWhiteList[query]; ok {
		return true
	}

	return false
}

Rather than the current

var (
	cellQueryWhiteList = []string{
		"firehose_value_metric_rep_unhealthy_cell",
		"firehose_value_metric_rep_capacity_remaining_containers",
		"firehose_value_metric_rep_capacity_remaining_disk",
		"firehose_value_metric_rep_capacity_remaining_memory",
		"firehose_value_metric_rep_capacity_total_containers",
		"firehose_value_metric_rep_capacity_total_disk",
		"firehose_value_metric_rep_capacity_total_memory",
		"firehose_value_metric_rep_num_cpus",
	}
)
func isAllowedCellMetricsQuery(query string) bool {
	for _, whiteListQuery := range cellQueryWhiteList {
		if strings.Index(query, whiteListQuery) == 0 {
			return true
		}
	}
	return false
}

Copy link
Contributor

@nwmac nwmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nwmac
Copy link
Contributor

nwmac commented Oct 19, 2018

Tested locally - looks good.

@nwmac nwmac merged commit f230b22 into v2-master Oct 19, 2018
@nwmac nwmac deleted the cell-security branch October 19, 2018 14:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants