meta: apply authorization to SHOW MEASUREMENTS #8708
Conversation
|
General questions:
|
|
Just not implemented yet, I requested the reviews to get some early
feedback.
…On Aug 17, 2017 5:44 PM, "Edd Robinson" ***@***.***> wrote:
General question: what's the reason behind not implementing FGA on SHOW
MEASURMENTS for the tsi1 index?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#8708 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABU8UvM_kfnOeOuzRhZDIq04DYuI7bCXks5sZLQygaJpZM4O5I8d>
.
|
|
@joelegasse sorry, I edited my message as you replied 😄 |
|
The authorizer is the user. Ultimately we need to perform lower level operations (filtering series) per-user. The original code base assumes full access at the database level, so execution context (originating user) was dropped after the initial check was done in the query handler. The Authorizer interface gives us an abstraction of access rules that can be expanded later if needed. |
| @@ -532,7 +534,7 @@ func (i *Index) measurementNamesByTagFilters(filter *TagFilter) [][]byte { | |||
| // True | False | False | |||
| // False | True | False | |||
| // False | False | True | |||
| if tagMatch == (filter.Op == influxql.EQ || filter.Op == influxql.EQREGEX) { | |||
| if tagMatch == (filter.Op == influxql.EQ || filter.Op == influxql.EQREGEX) && m.AuthorizeRead(auth) { | |||
There was a problem hiding this comment.
I think by now the preceding comment is flat-out wrong isn't it? Can you either correct it, simplify it, or remove it?
| @@ -395,6 +395,20 @@ func (m *Measurement) TagSets(shardID uint64, opt influxql.IteratorOptions) ([]* | |||
| return sortedTagsSets, nil | |||
| } | |||
|
|
|||
| func (m *Measurement) AuthorizeRead(auth influxql.Authorizer) bool { | |||
There was a problem hiding this comment.
I'm concerned about how this is going to perform in some possibly unusual/extreme situations. Apologies if I'm not understanding how the auth methods work internally though...
If I understand this properly, this is relying on using unordered iteration of the series on the measurement to find at least one series that we're authorised to read from. So it's the case that a user can read a measurement if they can read at least one series on that measurement, right?
I'm assuming the worse case here would be if we have some tags that we can read, but none of the series on the measurement have those tags? In that case we have to iterate over all the series to determine that we can't read any, and therefore can't read this measurement?
When the number of series for a measurement is large, I'm wondering if we would be better off taking in turn the (likely) small set of tag key/value grants we're interested in and binary searching over the ordered list of series. I realise that's probably too significant a change to the API though...
I guess the type of scenarios I'm thinking of would be something like a user being allowed to read values for a certain container ID (or some other high cardinality data), and the measurement in question containing many container IDs, but not the one the user can read from.
There was a problem hiding this comment.
Currently the only primitive exposed from the Authorizer is read/write access to a specific series. Ideally, yes, there would be a way to invert the rules to more efficiently filter through available series. @jwilder and I discussed possibly trying to extract an influxql.Expr from the rules, but they might have to be on a per-measurement basis (potentially large tree checking _name for different values). Also, since the representation of a measurement and the relevant series is per-index, we would have to try to export primitives useful to both inmem and tsi, and MeasurementNames already takes an Expr for filtering.
I'm all for finding a better way, which is why I asked for some early feedback. This works with what we have now, and is a starting point for improvements. :-)
|
I'm going to close this in favor of a simpler version based on #8752 |
This change will allow limiting the results from
SHOW MEASUREMENTSaccording to enterprise authorization rules.Required for all non-trivial PRs