Token Introspection Policy to get client information from "oidc_issuer_endpoint". #755

y-tabata · 2018-06-08T06:36:56Z

Since it is troublesome to define the client_id and client_secret twice in the AUTHENTICATION SETTINGS and in the POLICIES of the Admin Portal, this PR adds "use_oidc_issuer_endpoint" property to get client information from oidc_issuer_endpoint.

mikz · 2018-06-11T07:51:10Z

gateway/src/apicast/policy/token_introspection/apicast-policy.json

@@ -21,6 +21,11 @@
        "description": "Client Secret for the Token Introspection Endpoint",
        "type": "string"
      },
+      "use_oidc_issuer_endpoint": {


When this is true we don't need ClientID + Secret anymore, right?

I wonder if it makes sense to use dynamic schema like in #741.
Using either https://github.com/mozilla-services/react-jsonschema-form#property-dependencies or https://github.com/mozilla-services/react-jsonschema-form#schema-dependencies could allow us to have two login methods: clientid+secret or automatic using oidc_issuer_endpoint.

@davidor wdyt?

When this is true we don't need ClientID + Secret anymore, right?

Yes. We don't need to define client_id and client_secret in the schema.

What I mean is having either client_id+client_secret or use_oidc_issuer_endpoint.

Try following schema:

{ "type": "object", "properties": { "auth_type": { "type": "string", "enum": [ "oidc_issuer_endpoint", "client_id+client_secret" ], "default": "client_id+client_secret" } }, "required": [ "auth_type" ], "dependencies": { "auth_type": { "oneOf": [ { "properties": { "auth_type": { "enum": [ "oidc_issuer_endpoint" ] } } }, { "properties": { "auth_type": { "enum": [ "client_id+client_secret" ] }, "client_id": { "type": "string" }, "client_secret": { "type": "string" } }, "required": [ "client_id", "client_secret" ] } ] } } }

In https://mozilla-services.github.io/react-jsonschema-form/

That would allow user to choose which auth method to use and then fill the credentials.
It should also be extensible to add more properties in the future.

mikz · 2018-06-11T07:54:13Z

gateway/src/apicast/policy/token_introspection/token_introspection.lua

+function _M:rewrite(context)
+  if self.use_oidc_issuer_endpoint then
+    local components = resty_url.parse(context.service.oidc.issuer_endpoint)
+    set_client(self, components.user, components.password)


It might be easier to pass the headers into the individual http call instead of initializing new client every time. Then we won't mutate self and initialize new http client on every request.

You can pass headers when making the request: https://github.com/3scale/apicast/blob/154dc5a4087c97ef9c46209ada6fc8d7d944fea6/spec/resty/http_ng_spec.lua#L78

mikz · 2018-06-11T09:37:58Z

gateway/src/apicast/policy/token_introspection/token_introspection.lua

@@ -55,7 +66,8 @@ local function introspect_token(self, token)

  --- Parameters for the token introspection endpoint.
  -- https://tools.ietf.org/html/rfc7662#section-2.1
-  local res, err = self.http_client.post(self.introspection_url , { token = token, token_type_hint = 'access_token'})
+  local res, err = self.http_client.post{self.introspection_url , { token = token, token_type_hint = 'access_token'},
+    headers = {['User-Agent'] = user_agent(), ['Authorization'] = self.credential}}


User-Agent does not change, so it can be passed when creating the HTTP Client.

mikz · 2018-06-11T09:38:37Z

gateway/src/apicast/policy/token_introspection/token_introspection.lua

 function _M.new(config)
  local self = new(config)
  self.config = config or {}
+  self.auth_type = config.auth_type or {}


I think the default value for auth_type is "client_id+client_secret", right?

mikz · 2018-06-12T06:02:13Z

gateway/src/apicast/policy/token_introspection/apicast-policy.json

@@ -13,13 +13,10 @@
        "description": "Introspection Endpoint URL",


When using the OIDC endpoint, this setting is also unnecessary, right?

@mikz Regarding keycloak, the endpoint is http://${host}:${port}/auth/realms/${realm_name}/protocol/openid-connect/token/introspect.
But this is not common to all.
I wonder whether it's good that this is dedicated to keycloak.

But this endpoint is discoverable through OIDC.

For example: https://keycloak.3sca.net/auth/realms/oidc-demo/.well-known/openid-configuration.

It should be possible to get it from context.proxy.oauth.config. You can use require('resty.repl').start() to start a REPL and see around the objects available.

mikz · 2018-06-15T07:18:46Z

gateway/src/apicast/policy/token_introspection/token_introspection.lua

+  if self.auth_type == "oidc_issuer_endpoint" then
+    local components = resty_url.parse(context.service.oidc.issuer_endpoint)
+    self.credential = create_credential(components.user, components.password)
+    self.introspection_url = context.proxy.oauth.config.openid.token_introspection_endpoint


We probably should test how this policy behaves on non OIDC service. I think this would crash.

mikz

Great change! Thank you so much @y-tabata ! 👍

Left few final comments regarding the schema naming.

mikz · 2018-06-15T07:59:51Z

gateway/src/apicast/policy/token_introspection/apicast-policy.json

+          "properties": {
+            "auth_type": {
+              "describe": "Get the client and the introspection end point from the oidc issuer endpoint",
+              "enum": ["oidc_issuer_endpoint"]


One final nitpick. oidc_issuer_endpoint would imply the value is an endpoint, but in fact it is just a boolean flag. IMO better name would be use_3scale_oidc_issuer_endpoint, use_service_oidc_issuer_endpoint or something along those lines.

mikz · 2018-06-15T08:01:28Z

gateway/src/apicast/policy/token_introspection/apicast-policy.json

+        "oneOf": [{
+          "properties": {
+            "auth_type": {
+              "describe": "Get the client and the introspection end point from the oidc issuer endpoint",


I think this could be improved a bit.

Use the Client credentials and the OIDC Issuer Endpoint from the OpenID Connect Issuer setting.

Still not perfect, but we can improve that later.

mikz · 2018-06-15T08:06:19Z

gateway/src/apicast/policy/token_introspection/apicast-policy.json

+        }, {
+          "properties": {
+            "auth_type": {
+              "describe": "Specify the client ID and the client secret",


This probably should say something like:

Specify the Token Introspection Endpoint, Client ID, and Client Secret.

gateway/src/apicast/policy/token_introspection/token_introspection.lua

mikz

👍

y-tabata requested a review from a team as a code owner June 8, 2018 06:36

mikz reviewed Jun 11, 2018

View reviewed changes

y-tabata force-pushed the liquid-templating-token-introspection-policy branch 2 times, most recently from 9be11ca to da3a079 Compare June 12, 2018 00:08

mikz reviewed Jun 12, 2018

View reviewed changes

mikz reviewed Jun 15, 2018

View reviewed changes

add auth_type to get properties from the oidc issuer setting

20823bc

y-tabata force-pushed the liquid-templating-token-introspection-policy branch from bc91d90 to 20823bc Compare June 15, 2018 08:24

mikz approved these changes Jun 15, 2018

View reviewed changes

mikz merged commit 56ae1fc into 3scale:master Jun 15, 2018

y-tabata deleted the liquid-templating-token-introspection-policy branch June 15, 2018 22:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Token Introspection Policy to get client information from "oidc_issuer_endpoint". #755

Token Introspection Policy to get client information from "oidc_issuer_endpoint". #755

y-tabata commented Jun 8, 2018

mikz Jun 11, 2018

y-tabata Jun 11, 2018

mikz Jun 11, 2018 •

edited

y-tabata Jun 11, 2018

mikz Jun 11, 2018

y-tabata Jun 11, 2018

mikz Jun 11, 2018

y-tabata Jun 12, 2018

mikz Jun 11, 2018

y-tabata Jun 12, 2018

mikz Jun 12, 2018

y-tabata Jun 12, 2018

mikz Jun 12, 2018

y-tabata Jun 15, 2018

mikz Jun 15, 2018

mikz left a comment

mikz Jun 15, 2018

y-tabata Jun 15, 2018

mikz Jun 15, 2018

y-tabata Jun 15, 2018

mikz Jun 15, 2018

y-tabata Jun 15, 2018

mikz left a comment

		@@ -13,13 +13,10 @@
		"description": "Introspection Endpoint URL",

Token Introspection Policy to get client information from "oidc_issuer_endpoint". #755

Token Introspection Policy to get client information from "oidc_issuer_endpoint". #755

Conversation

y-tabata commented Jun 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikz Jun 11, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikz left a comment

Choose a reason for hiding this comment

mikz Jun 11, 2018 •

edited