Skip to content
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

OPTIONS request against service to load extension can require credentials #121

Closed
dannylamb opened this issue Jun 7, 2017 · 21 comments
Closed

Comments

@dannylamb
Copy link

dannylamb commented Jun 7, 2017

Stems from Islandora/documentation#612 (comment)

When the Loader Service is making an OPTIONS request against a service to load an extension, it does not provide credentials. This causes the loader service to fail when the attempting to load a service that requires authentication for OPTIONS requests.

@dannylamb dannylamb changed the title Use provided http client when loading extensions OPTIONS request against service to load extension requires credentials Jun 7, 2017
@dannylamb dannylamb changed the title OPTIONS request against service to load extension requires credentials OPTIONS request against service to load extension can require credentials Jun 7, 2017
@ajs6f
Copy link
Contributor

ajs6f commented Jun 8, 2017

Yes, I think we are talking about a different set of credentials from those used against a registry inside a repo. It happens that for CLAW, for now, they are the same, but that might not be the case.

@birkland
Copy link
Contributor

birkland commented Jun 8, 2017

OK, let's think through the security considerations here.

The loader itself is an extension, so it can be considered to be an actor distinct from API-X whose role is to provide a convenient mechanism for updating API-X's registries (i.e. you tell it where your extension lives, and it populates the API-X registries). The workflow is:

  • The loader accepts a POST with the URI of an extension to load.
  • The loader performs an OPTIONS against that URI, to retrieve an extension definition document
  • The loader registers that extension in the API-X registry. It uses the extension definition document to create the appropriate entries in the extension and service registry, and registers the provided URI as an instance/endpoint of the service

So the relevant security questions are:

  1. Is the agent who issued the POST authorized to add/update extensions?
    • Right now, the loader extension has no built-in security and trusts all requests. The only way to secure the loader service right now is via the network, e.g route and trust requests based on network origin (trusted subnet, etc).
  2. Is the Loader extension authorized to receive an OPTIONS response from the provided service URI?
    • Right now, the service instance would have to trust OPTIONS requests from API-X. This is the motivation for this particular issue,
  3. Is the Loader extension authorized to update the registries?
    • It is presently given the credentials (via configuration or provided HttpClient) to update the various registries via API-X's config.

Does that sound about right so far?

@ajs6f
Copy link
Contributor

ajs6f commented Jun 8, 2017

Sounds pretty on-target to me. Of the three points, 1 seems to me to be a separate ticket. 2 is indeed the one we are talking about right now. And 3 was, as you implied, the purpose of the recent work to break out an injectable client.

Possible problem: every given service just might require different authN!

@dannylamb
Copy link
Author

@ajs6f We don't have that use case right now, but it's frightening possibility. Let's not go there unless we absolutely have to 🙏

@ajs6f
Copy link
Contributor

ajs6f commented Jun 8, 2017

Good point, @dannylamb . Let's only make our lives a little bit harder than necessary, not a lot. :)

@dannylamb
Copy link
Author

dannylamb commented Jun 8, 2017

For this ticket, it looks like the jetty component can be configured with a custom http client. The documentation makes it sound like a bad idea, though. It might still be worth trying.

@birkland
Copy link
Contributor

birkland commented Jun 8, 2017

OK. I'm thinking that we may be OK just injecting the same HttpClient as we did for solving (3). Here's why:

  • If an HttpClient has been provided, it can decide which credentials to provide based on the hostname/realm of the URI being accessed. Use Fedora's credentials if it's Fedora, use service X's credentials for service X, etc

  • The built-in client in API-X can actually be provided different credentials for different hosts. It accepts configuration params like auth.${scheme}.${port}.${host}.username, auth.${scheme}.${port}.${host}.password, so you can easily configure it to use different username/passwords for different hosts/services. The one thing it doesn't have is a way to say "use this username and password for everything", so you'd need to enumerate all hosts and ports of services it needs to authenticate with for OPTIONS. This can be fixed/softened if that'd be worthwhile

@dannylamb
Copy link
Author

@birkland That would work for CLAW because the services and Fedora use the same authentication methods.

@birkland
Copy link
Contributor

birkland commented Jun 8, 2017

@dannylamb We may want to refactor to use http4 instead of adding httpclient to Jetty. We've found that the way Camel jetty component wraps and invokes the jetty httpclient library really sucks.

@ajs6f
Copy link
Contributor

ajs6f commented Jun 8, 2017

http4 is my go-to for that kind of action in Camel.

@dannylamb
Copy link
Author

@birkland That's cool. The http4 component looks like it has an httpClientConfigurator option that should be useful.

@dannylamb
Copy link
Author

Also... there's a fourth component to this issue. The routing/execution engine is going to have to authenticate against the services as well.

@ajs6f
Copy link
Contributor

ajs6f commented Jun 8, 2017

Is it safe to assume that the authN against the services for execution will be the same as for getting the extension definition?

@birkland
Copy link
Contributor

birkland commented Jun 8, 2017

@dannylamb The way it's set up now for (4) is much like a reverse proxy: API-X will pass along any auth headers present in the original request to the service. So as far as auth* is concerned, it's a negotiation between the client and the service. If Danny authenticates as Danny, then the service sees Danny's auth header, rather than FedoraAdmin's (for example), and makes its own decisions about its trust in Danny.

That being said, there may be value in using the injected HttpClient there as well, if you want to control which credentials are being used.

@dannylamb
Copy link
Author

@birkland Ah, I forgot about that. That will work fine for us as is, since we share authentication schemes across sevices and Fedora. I was just being overly paranoid.

@birkland
Copy link
Contributor

birkland commented Jun 8, 2017

@dannylamb No worries! CLAW is pushing the envelope with security, and that's a good thing.

@ajs6f
Copy link
Contributor

ajs6f commented Jun 8, 2017

"CLAW is pushing the envelope with security". Let's not make that the motto. :)

@dannylamb
Copy link
Author

dannylamb commented Jun 8, 2017

So in looking through this issue, the plan of attack would be:

  1. Use the http4 component instead of jetty here
  2. Configure the http4 component to use a custom HttpClientConfigurer
  3. Provide a default HttpClientConfigurer that provides an HttpClient identical to the default provided by Api-X
  4. CLAW will need to provide an analagous HttpClientConfigurer which adds the same StaticTokenRequestInterceptor as is added to its custom HttpClient.

@ajs6f
Copy link
Contributor

ajs6f commented Jun 8, 2017

Sounds right. I will be happy to tackle the 4th part when the time comes!

One question-- do we want to use HttpClientConfigurer, or would we rather do what (we now know is fully-supported and) we did previously-- inject an actual HttpClient?

@birkland
Copy link
Contributor

birkland commented Jun 8, 2017

I'm working on a PR right now (now just adding an IT). The approach it takes is to inject an HttpClient as before

@dannylamb
Copy link
Author

@ajs6f @birkland awesome. that seems simpler than duplicating the technique with a configurer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants