New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabling datasource level authorization in Druid #2424

Merged
merged 1 commit into from Apr 28, 2016

Conversation

Projects
None yet
5 participants
@pjain1
Contributor

pjain1 commented Feb 9, 2016

Fixes #2355. This PR is meant to put necessary abstractions inside Druid for enabling authorization as discussed in #2355

  • Introduce AuthorizationInfo interface, specific implementations of which would be provided by extensions
  • If the druid.auth.enabled is set to true then the isAuthorized method of AuthorizationInfo will be called to perform authorization checks
  • AuthorizationInfo object will be created in the servlet filters of specific extension and will be passed as a request attribute with attribute name as AuthConfig.DRUID_AUTH_TOKEN
  • As per the scope of this PR, all resources that needs to be secured are divided into 3 types - DATASOURCE, CONFIG and STATE. For any type of resource, possible actions are - READ or WRITE
  • Specific ResourceFilters are used to perform auth checks for all endpoints that corresponds to a specific resource type. This prevents duplication of logic and need to inject HttpServletRequest inside each endpoint. For example
    • DatasourceResourceFilter is used for endpoints where the datasource information is present after "datasources" segment in the request Path such as /druid/coordinator/v1/datasources/, /druid/coordinator/v1/metadata/datasources/, /druid/v2/datasources/
    • RulesResourceFilter is used where the datasource information is present after "rules" segment in the request Path such as /druid/coordinator/v1/rules/
    • TaskResourceFilter is used for endpoints is used where the datasource information is present after "task" segment in the request Path such as druid/indexer/v1/task
    • ConfigResourceFilter is used for endpoints like /druid/coordinator/v1/config, /druid/indexer/v1/worker, /druid/worker/v1 etc
    • StateResourceFilter is used for endpoints like /druid/broker/v1/loadstatus, /druid/coordinator/v1/leader, /druid/coordinator/v1/loadqueue, /druid/coordinator/v1/rules etc
  • For endpoints where a list of resources is returned like /druid/coordinator/v1/datasources, /druid/indexer/v1/completeTasks etc. the list is filtered to return only the resources to which the requested user has access. In these cases, HttpServletRequest instance needs to be injected in the endpoint method.

Note -
JAX-RS specification provides an interface called SecurityContext. However, we did not use this but provided our own interface AuthorizationInfo mainly because it provides more flexibility. For example, SecurityContext has a method called isUserInRole(String role) which would be used for auth checks and if used then the mapping of what roles can access what resource needs to be modeled inside Druid either using some convention or some other means which is not very flexible as Druid has dynamic resources like datasources.

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Feb 9, 2016

Contributor

There is an interesting mix of standardized and non standardized auth methods here. On one hand the authorization tries to provide a standard resource related framework, but on the other hand it completely relies on the endpoint to do all the auth requesting and logic.

I suspect this is because the resource of interest is contained within the body rather than as part of the http request metadata.

IMHO a "cleaner" solution would be to architect the requests such that any endpoint that touches a sensitive resource must have the appropriate annotations on what resources it touches. Then the auth layer transparently handles the auth based on request metadata, and the duty within the endpoint itself is simply to wire up the authorized resource to the resource actually being used.

Another way to say it is that it would be awesome if by the time the method is called, the auth system has already performed its function, and it is simply the job of the endpoint method to ensure compliance with the auth system's expectations on resource usage.

Contributor

drcrallen commented Feb 9, 2016

There is an interesting mix of standardized and non standardized auth methods here. On one hand the authorization tries to provide a standard resource related framework, but on the other hand it completely relies on the endpoint to do all the auth requesting and logic.

I suspect this is because the resource of interest is contained within the body rather than as part of the http request metadata.

IMHO a "cleaner" solution would be to architect the requests such that any endpoint that touches a sensitive resource must have the appropriate annotations on what resources it touches. Then the auth layer transparently handles the auth based on request metadata, and the duty within the endpoint itself is simply to wire up the authorized resource to the resource actually being used.

Another way to say it is that it would be awesome if by the time the method is called, the auth system has already performed its function, and it is simply the job of the endpoint method to ensure compliance with the auth system's expectations on resource usage.

@pjain1

This comment has been minimized.

Show comment
Hide comment
@pjain1

pjain1 Feb 9, 2016

Contributor

Yes the auth checks are performed inside endpoints as many of the Druid endpoints does not follow REST conventions and resource information is inside the body.
So the cleaner approach that you are suggesting is to change the endpoints to follow REST conventions ? Is that what you meant when you said the requests must have appropriate annotations ?

Contributor

pjain1 commented Feb 9, 2016

Yes the auth checks are performed inside endpoints as many of the Druid endpoints does not follow REST conventions and resource information is inside the body.
So the cleaner approach that you are suggesting is to change the endpoints to follow REST conventions ? Is that what you meant when you said the requests must have appropriate annotations ?

@himanshug

This comment has been minimized.

Show comment
Hide comment
@himanshug

himanshug Feb 9, 2016

Contributor

@drcrallen your suggestion is valid, however something that can be done independently of this PR and would probably require druid client API changes.

Contributor

himanshug commented Feb 9, 2016

@drcrallen your suggestion is valid, however something that can be done independently of this PR and would probably require druid client API changes.

@himanshug himanshug added this to the 0.9.1 milestone Feb 9, 2016

@@ -0,0 +1,6 @@
package io.druid.server.security;
public enum ResourceType

This comment has been minimized.

@drcrallen

drcrallen Feb 9, 2016

Contributor

why does this need to be enum?

@drcrallen

drcrallen Feb 9, 2016

Contributor

why does this need to be enum?

This comment has been minimized.

@himanshug

himanshug Feb 9, 2016

Contributor

@drcrallen i think, the set of valid resource types would be fixed by druid-core because druid-core calls in to the action and provides those as arguments, so it makes sense to use enum to represent resource types.
in any case, since we are calling the api experimental and potentially changeable in near future. I wouldn't worry too much about it either ways.

@himanshug

himanshug Feb 9, 2016

Contributor

@drcrallen i think, the set of valid resource types would be fixed by druid-core because druid-core calls in to the action and provides those as arguments, so it makes sense to use enum to represent resource types.
in any case, since we are calling the api experimental and potentially changeable in near future. I wouldn't worry too much about it either ways.

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Feb 9, 2016

Contributor

@himanshug / @pjain1 would you guys feel comfortable calling auth an experimental feature that may change significantly in the near-ish future?

If that's the case then a stop-gap that solves your main pain points should be ok as long as its not intrusive in other scenarios (which I don't think this PR is)

Contributor

drcrallen commented Feb 9, 2016

@himanshug / @pjain1 would you guys feel comfortable calling auth an experimental feature that may change significantly in the near-ish future?

If that's the case then a stop-gap that solves your main pain points should be ok as long as its not intrusive in other scenarios (which I don't think this PR is)

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Feb 9, 2016

Contributor

General PR comments:

  • Headers missing on some of the files
  • Why are enums needed instead of just strings?
Contributor

drcrallen commented Feb 9, 2016

General PR comments:

  • Headers missing on some of the files
  • Why are enums needed instead of just strings?
@himanshug

This comment has been minimized.

Show comment
Hide comment
@himanshug

himanshug Feb 9, 2016

Contributor

@drcrallen I'm fine calling it experimental.
sounds like high level approach is ok.
@pjain1 can you make necessary add/updates and remove the "Discuss" when done?

Contributor

himanshug commented Feb 9, 2016

@drcrallen I'm fine calling it experimental.
sounds like high level approach is ok.
@pjain1 can you make necessary add/updates and remove the "Discuss" when done?

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Feb 9, 2016

Contributor

@pjain1 please comment in the code where you're adding the auth checks that auth is experimental and reference this PR. ((so future developers know why it is there))

Contributor

drcrallen commented Feb 9, 2016

@pjain1 please comment in the code where you're adding the auth checks that auth is experimental and reference this PR. ((so future developers know why it is there))

@pjain1

This comment has been minimized.

Show comment
Hide comment
@pjain1

pjain1 Feb 9, 2016

Contributor

@drcrallen sure I will put the necessary comments. I will put the headers.
About Enum vs String - I chose enums as it gives more control on what values can be passed in and all the options available to the developer but I also see the point that anyways AuthorizationInfo will be implemented by extensions they can pass in whatever they want. Personally I would prefer enum but I am OK in changing it to String if you have a strong opinion against using enum. BTW what is your reason of not having enums ?

Contributor

pjain1 commented Feb 9, 2016

@drcrallen sure I will put the necessary comments. I will put the headers.
About Enum vs String - I chose enums as it gives more control on what values can be passed in and all the options available to the developer but I also see the point that anyways AuthorizationInfo will be implemented by extensions they can pass in whatever they want. Personally I would prefer enum but I am OK in changing it to String if you have a strong opinion against using enum. BTW what is your reason of not having enums ?

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Feb 9, 2016

Contributor

@pjain1 good points. My reasons for not favoring enums are:

  • Enums are not very extensible, and IMHO make more sense when either A) there is an explicit requirement for ordering or B) There is an explicit need to limit the options available
  • I'm not sure how well enums play with classloaders. I'm guessing they don't any more so than other classes. Getting an error message like "DATASOURCE cannot be assigned to type ResourceType" (or similar) is pretty irritating. This can occur if the two enum classes were loaded through different classLoaders.

So basically, if you're trying to force any extensions to ONLY use the values presented, and not intending the security set to be extendable, then there can be a good argument for enums.

Contributor

drcrallen commented Feb 9, 2016

@pjain1 good points. My reasons for not favoring enums are:

  • Enums are not very extensible, and IMHO make more sense when either A) there is an explicit requirement for ordering or B) There is an explicit need to limit the options available
  • I'm not sure how well enums play with classloaders. I'm guessing they don't any more so than other classes. Getting an error message like "DATASOURCE cannot be assigned to type ResourceType" (or similar) is pretty irritating. This can occur if the two enum classes were loaded through different classLoaders.

So basically, if you're trying to force any extensions to ONLY use the values presented, and not intending the security set to be extendable, then there can be a good argument for enums.

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Feb 9, 2016

Contributor

@himanshug I think from #2424 (comment) you're supporting B) from my list above, where you are purposefully limiting the options available. Is that correct?

Contributor

drcrallen commented Feb 9, 2016

@himanshug I think from #2424 (comment) you're supporting B) from my list above, where you are purposefully limiting the options available. Is that correct?

@himanshug

This comment has been minimized.

Show comment
Hide comment
@himanshug

himanshug Feb 9, 2016

Contributor
Contributor

himanshug commented Feb 9, 2016

@fjy

This comment has been minimized.

Show comment
Hide comment
@fjy

fjy Feb 27, 2016

Contributor

@pjain1 merge conflicts

Contributor

fjy commented Feb 27, 2016

@pjain1 merge conflicts

@pjain1

This comment has been minimized.

Show comment
Hide comment
@pjain1

pjain1 Feb 29, 2016

Contributor

@fjy I am still working on it, I will resolve the conflicts when it is reviewable

Contributor

pjain1 commented Feb 29, 2016

@fjy I am still working on it, I will resolve the conflicts when it is reviewable

@pjain1

This comment has been minimized.

Show comment
Hide comment
@pjain1

pjain1 Mar 7, 2016

Contributor

@drcrallen @fjy @himanshug the PR is reviewable. Updated the top level comment to reflect the state of latest changes. @drcrallen using annotation based resource filtering for performing access checks.

Contributor

pjain1 commented Mar 7, 2016

@drcrallen @fjy @himanshug the PR is reviewable. Updated the top level comment to reflect the state of latest changes. @drcrallen using annotation based resource filtering for performing access checks.

@pjain1 pjain1 changed the title from [Discuss] [WIP] Enabling datasource level authorization in Druid to Enabling datasource level authorization in Druid Mar 7, 2016

@pjain1 pjain1 removed the Discuss label Mar 7, 2016

@@ -372,7 +454,36 @@ public Response getRunningTasks()
@Override
public Collection<? extends TaskRunnerWorkItem> apply(TaskRunner taskRunner)
{
return taskRunner.getRunningTasks();
if (authConfig.isEnabled()) {

This comment has been minimized.

@himanshug

himanshug Mar 14, 2016

Contributor

this and the change in getPendingTasks() are same, can you refactor them into a private helper method?

@himanshug

himanshug Mar 14, 2016

Contributor

this and the change in getPendingTasks() are same, can you refactor them into a private helper method?

This comment has been minimized.

@pjain1

pjain1 Mar 16, 2016

Contributor

done

@pjain1

pjain1 Mar 16, 2016

Contributor

done

{
return getSegmentsForDatasources().keySet();
if (authConfig.isEnabled()) {
// This is an experimental feature, see - https://github.com/druid-io/druid/pull/2424

This comment has been minimized.

@himanshug

himanshug Mar 14, 2016

Contributor

instead of repeating this line everywhere, can you just put it in one central place e.g. DruidAuthModule or AuthorizationInfo?

@himanshug

himanshug Mar 14, 2016

Contributor

instead of repeating this line everywhere, can you just put it in one central place e.g. DruidAuthModule or AuthorizationInfo?

This comment has been minimized.

@pjain1

pjain1 Mar 16, 2016

Contributor

I can but I thought as per this comment we want it to be everywhere auth check is added

@pjain1

pjain1 Mar 16, 2016

Contributor

I can but I thought as per this comment we want it to be everywhere auth check is added

This comment has been minimized.

@himanshug

himanshug Apr 12, 2016

Contributor

ok

@himanshug

himanshug Apr 12, 2016

Contributor

ok

@fjy

This comment has been minimized.

Show comment
Hide comment
@fjy

fjy Mar 14, 2016

Contributor

@pjain1 can we finish this up? there's merge conflicts

Contributor

fjy commented Mar 14, 2016

@pjain1 can we finish this up? there's merge conflicts

@pjain1 pjain1 closed this Mar 15, 2016

@pjain1 pjain1 reopened this Mar 15, 2016

@pjain1

This comment has been minimized.

Show comment
Hide comment
@pjain1

pjain1 Apr 21, 2016

Contributor

@drcrallen I think I addressed your comments, can you have a look again ?

Contributor

pjain1 commented Apr 21, 2016

@drcrallen I think I addressed your comments, can you have a look again ?

@@ -0,0 +1,115 @@
package io.druid.indexing.overlord.http.security;

This comment has been minimized.

@drcrallen

drcrallen Apr 26, 2016

Contributor

header

@drcrallen

drcrallen Apr 26, 2016

Contributor

header

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Apr 26, 2016

Contributor

@pjain1 Please check out pjain1#1 and see if that works for you

Contributor

drcrallen commented Apr 26, 2016

@pjain1 Please check out pjain1#1 and see if that works for you

@pjain1

This comment has been minimized.

Show comment
Hide comment
@pjain1

pjain1 Apr 27, 2016

Contributor

@drcrallen fixed UTs. have a look now

Contributor

pjain1 commented Apr 27, 2016

@drcrallen fixed UTs. have a look now

@Override
public void configure(Binder binder)
{
JsonConfigProvider.bind(binder, "druid.auth", AuthConfig.class);

This comment has been minimized.

@drcrallen

drcrallen Apr 28, 2016

Contributor

(optional) would this be more appropriate as druid.request.auth or something else a little more descriptive?

@drcrallen

drcrallen Apr 28, 2016

Contributor

(optional) would this be more appropriate as druid.request.auth or something else a little more descriptive?

This comment has been minimized.

@pjain1

pjain1 Apr 28, 2016

Contributor

Not sure if request should be included in the property name as the scope is much more than just requests to Druid. It can be called druid.security or something like that or can be kept as it is..

@pjain1

pjain1 Apr 28, 2016

Contributor

Not sure if request should be included in the property name as the scope is much more than just requests to Druid. It can be called druid.security or something like that or can be kept as it is..

This comment has been minimized.

@drcrallen

drcrallen Apr 28, 2016

Contributor

keep as is then.

@drcrallen

drcrallen Apr 28, 2016

Contributor

keep as is then.

return input.getPath().equals("datasources");
}
}
) + 1

This comment has been minimized.

@drcrallen

drcrallen Apr 28, 2016

Contributor

same question here about if the path exactly matches one of the strings in isApplicable

@drcrallen

drcrallen Apr 28, 2016

Contributor

same question here about if the path exactly matches one of the strings in isApplicable

This comment has been minimized.

@pjain1

pjain1 Apr 28, 2016

Contributor

same as this

@pjain1

pjain1 Apr 28, 2016

Contributor

same as this

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Apr 28, 2016

Contributor

few minor comments but looking very good overall.

Contributor

drcrallen commented Apr 28, 2016

few minor comments but looking very good overall.

authorizationInfo = EasyMock.createStrictMock(AuthorizationInfo.class);
// Memory barrier
synchronized (this) {

This comment has been minimized.

@pjain1

pjain1 Apr 28, 2016

Contributor

@drcrallen btw can you please explain what it this for ?

@pjain1

pjain1 Apr 28, 2016

Contributor

@drcrallen btw can you please explain what it this for ?

This comment has been minimized.

@drcrallen

drcrallen Apr 28, 2016

Contributor

I thought I was having trouble with threading, but it might not be right now.

Sometimes if a mocked object is accessed by multiple threads, the threads see an inconsistent rule set if the expectations are set in a different thread than the object is used. Memory barriers make the memory consistent at least to the point you pass the barrier.

You can tell you are hitting this kind of mocking race condition if you get an error like "expected 1 actual 1"

@drcrallen

drcrallen Apr 28, 2016

Contributor

I thought I was having trouble with threading, but it might not be right now.

Sometimes if a mocked object is accessed by multiple threads, the threads see an inconsistent rule set if the expectations are set in a different thread than the object is used. Memory barriers make the memory consistent at least to the point you pass the barrier.

You can tell you are hitting this kind of mocking race condition if you get an error like "expected 1 actual 1"

This comment has been minimized.

@pjain1

pjain1 Apr 28, 2016

Contributor

ok

@pjain1

pjain1 Apr 28, 2016

Contributor

ok

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Apr 28, 2016

Contributor

@pjain1 can you comment in the master comment why you opted for this route instead of going through
https://jersey.java.net/documentation/latest/security.html ?

Contributor

drcrallen commented Apr 28, 2016

@pjain1 can you comment in the master comment why you opted for this route instead of going through
https://jersey.java.net/documentation/latest/security.html ?

@pjain1

This comment has been minimized.

Show comment
Hide comment
@pjain1

pjain1 Apr 28, 2016

Contributor

@drcrallen updated the master comment with the info you asked for. Please, see if it makes sense.

Contributor

pjain1 commented Apr 28, 2016

@drcrallen updated the master comment with the info you asked for. Please, see if it makes sense.

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Apr 28, 2016

Contributor

@pjain1 yes thanks!

Contributor

drcrallen commented Apr 28, 2016

@pjain1 yes thanks!

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Apr 28, 2016

Contributor

Please ping me when #2424 (comment) is resolved and I think this should be ready to go

Contributor

drcrallen commented Apr 28, 2016

Please ping me when #2424 (comment) is resolved and I think this should be ready to go

@pjain1

This comment has been minimized.

Show comment
Hide comment
@pjain1

pjain1 Apr 28, 2016

Contributor

@drcrallen done

Contributor

pjain1 commented Apr 28, 2016

@drcrallen done

@drcrallen

This comment has been minimized.

Show comment
Hide comment
@drcrallen

drcrallen Apr 28, 2016

Contributor

Cool 👍

Contributor

drcrallen commented Apr 28, 2016

Cool 👍

pjain1 added a commit to pjain1/druid that referenced this pull request Apr 28, 2016

Basic authorization in Druid
- Introduce `AuthorizationInfo` interface, specific implementations of which would be provided by extensions
- If the `druid.auth.enabled` is set to `true` then the `isAuthorized` method of `AuthorizationInfo` will be called to perform authorization checks
-  `AuthorizationInfo` object will be created in the servlet filters of specific extension and will be passed as a request attribute with attribute name as `AuthConfig.DRUID_AUTH_TOKEN`
- As per the scope of this PR, all resources that needs to be secured are divided into 3 types - `DATASOURCE`, `CONFIG` and `STATE`. For any type of resource, possible actions are  - `READ` or `WRITE`
- Specific ResourceFilters are used to perform auth checks for all endpoints that corresponds to a specific resource type. This prevents duplication of logic and need to inject HttpServletRequest inside each endpoint. For example
 - `DatasourceResourceFilter` is used for endpoints where the datasource information is present after "datasources" segment in the request Path such as `/druid/coordinator/v1/datasources/`, `/druid/coordinator/v1/metadata/datasources/`, `/druid/v2/datasources/`
 - `RulesResourceFilter` is used where the datasource information is present after "rules" segment in the request Path such as `/druid/coordinator/v1/rules/`
 - `TaskResourceFilter` is used for endpoints is used where the datasource information is present after "task" segment in the request Path such as `druid/indexer/v1/task`
 - `ConfigResourceFilter` is used for endpoints like `/druid/coordinator/v1/config`, `/druid/indexer/v1/worker`, `/druid/worker/v1` etc
 - `StateResourceFilter` is used for endpoints like `/druid/broker/v1/loadstatus`, `/druid/coordinator/v1/leader`, `/druid/coordinator/v1/loadqueue`, `/druid/coordinator/v1/rules` etc
- For endpoints where a list of resources is returned like `/druid/coordinator/v1/datasources`, `/druid/indexer/v1/completeTasks` etc. the list is filtered to return only the resources to which the requested user has access. In these cases, `HttpServletRequest` instance needs to be injected in the endpoint method.

Note -
JAX-RS specification provides an interface called `SecurityContext`. However, we did not use this but provided our own interface `AuthorizationInfo` mainly because it provides more flexibility. For example, `SecurityContext` has a method called `isUserInRole(String role)` which would be used for auth checks and if used then the mapping of what roles can access what resource needs to be modeled inside Druid either using some convention or some other means which is not very flexible as Druid has dynamic resources like datasources. Fixes apache#2355 with PR apache#2424
@pjain1

This comment has been minimized.

Show comment
Hide comment
@pjain1

pjain1 Apr 28, 2016

Contributor

@drcrallen squashed the commits

Contributor

pjain1 commented Apr 28, 2016

@drcrallen squashed the commits

Basic authorization support in Druid
- Introduce `AuthorizationInfo` interface, specific implementations of which would be provided by extensions
- If the `druid.auth.enabled` is set to `true` then the `isAuthorized` method of `AuthorizationInfo` will be called to perform authorization checks
-  `AuthorizationInfo` object will be created in the servlet filters of specific extension and will be passed as a request attribute with attribute name as `AuthConfig.DRUID_AUTH_TOKEN`
- As per the scope of this PR, all resources that needs to be secured are divided into 3 types - `DATASOURCE`, `CONFIG` and `STATE`. For any type of resource, possible actions are  - `READ` or `WRITE`
- Specific ResourceFilters are used to perform auth checks for all endpoints that corresponds to a specific resource type. This prevents duplication of logic and need to inject HttpServletRequest inside each endpoint. For example
 - `DatasourceResourceFilter` is used for endpoints where the datasource information is present after "datasources" segment in the request Path such as `/druid/coordinator/v1/datasources/`, `/druid/coordinator/v1/metadata/datasources/`, `/druid/v2/datasources/`
 - `RulesResourceFilter` is used where the datasource information is present after "rules" segment in the request Path such as `/druid/coordinator/v1/rules/`
 - `TaskResourceFilter` is used for endpoints is used where the datasource information is present after "task" segment in the request Path such as `druid/indexer/v1/task`
 - `ConfigResourceFilter` is used for endpoints like `/druid/coordinator/v1/config`, `/druid/indexer/v1/worker`, `/druid/worker/v1` etc
 - `StateResourceFilter` is used for endpoints like `/druid/broker/v1/loadstatus`, `/druid/coordinator/v1/leader`, `/druid/coordinator/v1/loadqueue`, `/druid/coordinator/v1/rules` etc
- For endpoints where a list of resources is returned like `/druid/coordinator/v1/datasources`, `/druid/indexer/v1/completeTasks` etc. the list is filtered to return only the resources to which the requested user has access. In these cases, `HttpServletRequest` instance needs to be injected in the endpoint method.

Note -
JAX-RS specification provides an interface called `SecurityContext`. However, we did not use this but provided our own interface `AuthorizationInfo` mainly because it provides more flexibility. For example, `SecurityContext` has a method called `isUserInRole(String role)` which would be used for auth checks and if used then the mapping of what roles can access what resource needs to be modeled inside Druid either using some convention or some other means which is not very flexible as Druid has dynamic resources like datasources. Fixes #2355 with PR #2424

@fjy fjy merged commit 0d745ee into apache:master Apr 28, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

drcrallen added a commit to metamx/druid that referenced this pull request May 6, 2016

Lookups-DEV (#29)
* support LookupReferencesManager registration of namespaced lookup and eliminate static configurations for lookup from namespecd lookup extensions

- druid-namespace-lookup and druid-kafka-extraction-namespace are modified
- However, druid-namespace-lookup still has configuration about ON/OFF
  HEAP cache manager selection, which is not namespace wide
  configuration but node wide configuration as multiple namespace shares
  the same cache manager

* update KafkaExtractionNamespaceTest to reflect argument signature changes

* Add more synchronization functionality to NamespaceLookupExtractorFactory

* Remove old way of using extraction namespaces

* resolve compile error by supporting LookupIntrospectHandler

* Sketch cache key should include size, isInputThetaSketch. (apache#2893)

* Adjust colliding aggregator cache IDs. (apache#2891)

- Renumbered ApproximateHistogramAggregatorFactory from 8 to 12,
  8 was taken by CardinalityAggregatorFactory
- Renumbered ApproximateHistogramFoldingAggregatorFactory from 9 to 13,
  9 was taken by FilteredAggregatorFactory

* fix misleading error log due to race in RTR and concurrency test (apache#2878)

* doc fixes (apache#2897)

* Basic authorization support in Druid (apache#2424)

- Introduce `AuthorizationInfo` interface, specific implementations of which would be provided by extensions
- If the `druid.auth.enabled` is set to `true` then the `isAuthorized` method of `AuthorizationInfo` will be called to perform authorization checks
-  `AuthorizationInfo` object will be created in the servlet filters of specific extension and will be passed as a request attribute with attribute name as `AuthConfig.DRUID_AUTH_TOKEN`
- As per the scope of this PR, all resources that needs to be secured are divided into 3 types - `DATASOURCE`, `CONFIG` and `STATE`. For any type of resource, possible actions are  - `READ` or `WRITE`
- Specific ResourceFilters are used to perform auth checks for all endpoints that corresponds to a specific resource type. This prevents duplication of logic and need to inject HttpServletRequest inside each endpoint. For example
 - `DatasourceResourceFilter` is used for endpoints where the datasource information is present after "datasources" segment in the request Path such as `/druid/coordinator/v1/datasources/`, `/druid/coordinator/v1/metadata/datasources/`, `/druid/v2/datasources/`
 - `RulesResourceFilter` is used where the datasource information is present after "rules" segment in the request Path such as `/druid/coordinator/v1/rules/`
 - `TaskResourceFilter` is used for endpoints is used where the datasource information is present after "task" segment in the request Path such as `druid/indexer/v1/task`
 - `ConfigResourceFilter` is used for endpoints like `/druid/coordinator/v1/config`, `/druid/indexer/v1/worker`, `/druid/worker/v1` etc
 - `StateResourceFilter` is used for endpoints like `/druid/broker/v1/loadstatus`, `/druid/coordinator/v1/leader`, `/druid/coordinator/v1/loadqueue`, `/druid/coordinator/v1/rules` etc
- For endpoints where a list of resources is returned like `/druid/coordinator/v1/datasources`, `/druid/indexer/v1/completeTasks` etc. the list is filtered to return only the resources to which the requested user has access. In these cases, `HttpServletRequest` instance needs to be injected in the endpoint method.

Note -
JAX-RS specification provides an interface called `SecurityContext`. However, we did not use this but provided our own interface `AuthorizationInfo` mainly because it provides more flexibility. For example, `SecurityContext` has a method called `isUserInRole(String role)` which would be used for auth checks and if used then the mapping of what roles can access what resource needs to be modeled inside Druid either using some convention or some other means which is not very flexible as Druid has dynamic resources like datasources. Fixes apache#2355 with PR apache#2424

* Datasketches: Remove isInputThetaSketch from cache key. (apache#2899)

* Fix missing log arguments in PendingTaskBasedWorkerResourceManagementStrategy (apache#2898)

* fix ClassCastException in FiniteAppenderatorDriver (apache#2896)

* statsd-emitter (apache#2410)

* CombiningSequence: Delay making next yielder on creation until it is actually asked for. (apache#2892)

This fixes the behavior of limited combining sequences (otherwise limit = 1 would
actually yield 2 elements).

* Merge pull request apache#2905 from javasoze/eclipse_formatting

eclipse formatting fixes and added import order specs

* [QTL] Move kafka-extraction-namespace to the Lookup framework. (apache#2800)

* Move kafka-extraction-namespace to the Lookup framework.

* Address comments

* Fix missing kafka introspection

* Fix tests to be less racy

* Make testing a bit more leniant

* Make tests even more forgiving

* Add comments to kafka lookup cache method

* Move startStopLock to just use started

* Make start() and stop() idempotent

* Forgot to update test after last change, test now accounts for idempotency

* Add extra idempotency on stop check

* Add more descriptive docs of behavior

* [QTL] Make URI Exctraction Namespace take more sane arguments (apache#2738)

* Make URI Exctraction Namespace take more sane arguments
* Fixes apache#2669

* Update docs

* Rename error message

* Undo overzealous deletion of docs

* Explain caching mechanism a bit more in docs

* publish metrics numJettyConns to see how number of active jetty connections change over time (apache#2839)

this can be compared with numer of active queries to see if requests are waiting in jetty queue

* Remove kafka lookups

* Remove unused stuff

* Fix start and stop behavior to be consistent with new javadocs

* Remove unused strings

* Add timeout option

* Address comments on configurations and improve docs

* Add more options and update hash key and replaces

* Move monitoring to the overriding classes

* Add better start/stop logging

@pjain1 pjain1 deleted the pjain1:security branch May 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment