[DONE] External auth feature


#1

Hi

I need to authenticate against an external system which is not ldap,

Would it be acceptable to add a new rule type
external_auth(“https://example.com”)

This will send an http get request to the specified url argument along with the Authorization header, and interpret return status 200 as a successful authentication

thanks


(Simone Scarduzio) #2

Hello @presto9292,

Yes it would be possible, but I think this is one of those esoteric configuration where proxy_auth rule takes care of bridging the external site’s identities to a ReadonlyREST user or group.

I suggest to use nginx as a reverse proxy with auth_request configured.

Once this is done, follow ReadonlyREST docs about proxy_auth rule.


#3

Hi @sscarduzio

Thanks for your quick reply and for your amazing plugin.

I have an application server that’s already configured to do all kinds of authentication schemes.
I’d rather use it for auth rather than adding ngninx into my architecture which would be difficult. As a wise developer said “no more proxies! Yay Ponies!”

Even X-Pack accept that there are cases for custom authentication
https://www.elastic.co/guide/en/x-pack/current/custom-realms.html

Having seen the proxy_auth implementation I think I can add external_auth myself (using Java’s URLConnection so no 3rd party dependencies)

I’m just hoping you would be willing to consider the pull request (even as an undocumented feature if that helps in anyway.)

Thanks


(Simone Scarduzio) #4

haha touchè :smiley:

You can certainly PR a new rule, you know I love PRs :unicorn: :christmas_tree:

But remember ES (and ReadonlyREST) are asynchronous, non-blocking applications based on Netty.

This means URLConnection cannot be used, as it’s a synchronous, blocking API. If you use it, you’ll block one of the few worker threads every time a request needs authentication (kinda always).

@coutoPL has done an excellent work setting up the framework for this kind of things defining a GeneralAuthKeyAsyncRule you could just extend:

https://github.com/sscarduzio/elasticsearch-readonlyrest-plugin/blob/master/src/main/java/org/elasticsearch/plugin/readonlyrest/acl/blocks/rules/impl/GeneralAuthKeyAsyncRule.java#L34

I wonder if @CoutoPL could refactor his code to include the caching capability into it, leaving you to implement just the outbound HTTP request using any async client (i.e. we got Netty4 in the classpath already because of SSL) and wrapping the response into a CompletableFuture.

So my idea is that we could have something like GeneralCachedAuthKeyAsyncRule


(Mateusz Kołodziejczyk) #5

Hi all :slight_smile:

Ok, I’m going to tell what I’m working on.
I’d like to add external authorization or/and external roles provider. I’d like to use proxy_auth together with new future to do authentication and authorization. So, external authentication could base on this feature.

I thought also about splitting ldap_auth into ldap_authentication and ldap_authorization rules. Then we could compose authentication with authorization rules (eg. authentication by reverse proxy with ldap authorization).

But there is more to do here … IMO it is high time to think about rules order. In my case I have to be sure that authentication rule will be checked before authorization rule - so second rule depends on first.

General cached auth rule is also great idea, because new async auth rule also should have some kind of caching (optional of course). So, ldap caching have to be generalised.

At the moment I’m working on external roles provider. When I finish (next week I’m going to deliver the feature), I can help with rest things I was writing about.

But IMO rules order & theirs dependencies should be discussed before starting development because it’s not so easy task to do it right.


(Simone Scarduzio) #6

Amazing! I got @coutoPL to reveal his plans :smiley:
As we discussed with @diegonc, rules ordering has become very sensitive nowadays.

See discussion:

We could either:

  • Define a hard coded well-reasoned ordering + dependency system (easier UX, similar to today)
  • Or let the users carefully pick what goes first (more powerful, more tricky, requires breaking changes to YAML structure)

(Simone Scarduzio) #7

Hey guys, to sum this up:

The feature proposal for a transparent Basic HTTP Auth delegation rule sounds interesting, and we can implement it.

It’s worth waiting for @coutoPL to release his PR which includes a generic cached auth async rule

Meanwhile we need to sort out rules ordering, which maybe is less complicated than it seems. As I see it:

  • Evaluate all the rules that mutate the request for last
  • Evaluate all the auth rules at the beginning
  • All the others are evaluated in between.

The evaluation order can be enforced in code a it’s done today (that horrible sequence of try catch in Block.java).

@coutoPL would this be satisfy your requirement?


(Mateusz Kołodziejczyk) #8

Yes, in my case I have to be sure if authentication rule is executed before authorization one. And your proposal is OK for that matter.

But IMO we have to think about 2 more things:

  1. one ordering for async and sync rule (as I wrote in other post, we could achieve it by async adapter)
  2. breaking early block rule checking … now, async rules are executed till first mismatch. Checking sync rules is different - all rules are executed.

This two points should be considered. What do you think?


(Simone Scarduzio) #9

yep valid points. So summing up the design:

In Block.java

  1. Collect all configured rules in the established order (see @diegonc PR)
  2. Transform all Sync rules (if any) into AsyncRule w/ some adapter
  3. Use the code that today handles the evaluation of AsyncRules (with early exit if NO_MATCH).
  4. Throw away the code that currently evaluates the SyncRules

Sounds OK?


(Mateusz Kołodziejczyk) #10

Yes, sounds good. And rules order should conform rules below:

  1. Sync rules should be checked as soon as possible.
  2. Any authentication rule should be checked before any authorization rule.

(Simone Scarduzio) #11

OK this is an amazing plan :thumbsup:

If I now merge @diegonc PR with the reordering of rules, could you include what we just discussed in your PR?


(Mateusz Kołodziejczyk) #12

Yes, of course. I almost finished.


(Simone Scarduzio) #13

ok will merge to master :thumbsup:


(Mateusz Kołodziejczyk) #14

Extrnal authentication feature waiting for review and merge. Check this out: https://github.com/sscarduzio/elasticsearch-readonlyrest-plugin/pull/209 :slight_smile:


(Simone Scarduzio) #15

aaaaand it’s merged :thumbsup: :rocket:


(Simone Scarduzio) #16

Now tomorrow I have to backport all we’ve done to 5.2.x, 5.1.x, 5.0.x, 2.4.x, 2.3.x
HALP :skull: :cry: :bomb: PLS


(Mateusz Kołodziejczyk) #17

A week ago, I had to build ROR plugin for ES 5.2.2 and backporting is very unpleasant. I thought about it and maybe it’s a good idea to abstract over ES and all ES dependent code move to separate module. Then there will be ror-module, es53x, es52x and so on. Each ES module will depend on ror-module. There will be only one branch and any new added feature will have take into consideration all supported ES versions at once.


(Simone Scarduzio) #18

I thought about this, sounds really nice, how do you propose this to be?
Two different repos?
Would one depend from the other via maven or as a git submodule?


(Mateusz Kołodziejczyk) #19

I rather thought about gradle modules and one git repo.


(Simone Scarduzio) #20

All right, but then how is this different from having a different java package that separates ES wiring and RR logic?