Conditions #206

guicassolato · 2022-01-03T19:27:56Z

Conditions (new feature), named when in the AuthConfig API, are sets of expressions (JSON patterns) that whenever included must evaluate to true against the Authorization JSON, so the scope where the expressions are defined is enforced; otherwise Authorino will skip that scope in the pipeline.

The scope for a set of conditions can be the entire AuthConfig (top level) or a particular evaluator of any phase of the auth pipeline.

Each expression is a tuple composed of a selector, an operator and a fixed comparable value, just like the pattern matching expressions used in the existing conditions field of JSON pattern-matching authz and K8s SubjectAccessReview-based authz – both now superseded by this new implementation.

Literal expressions and references to expression sets (patterns, defined at the top level of the AuthConfig spec) can be listed, mixed and combined in the when conditions in the AuthConfig.

E.g., when conditions can be used:

i. to skip an entire AuthConfig based on the context:

spec:
  when:
  - selector: context.request.http.path
    operator: neq
    value: /status

ii. to skip parts of an AuthConfig (i.e. a specific evaluator):

spec:
  metadata:
  - name: metadata-source
    http:
      endpoint: https://my-metadata-source.io
    when:
    - selector: context.request.http.method
      operator: neq
      value: OPTIONS

iii. to enforce a particular evaluator only in certain contexts (really the same as the above, though to a different use case):

spec:
  identity:
  - name: authn-meth-1
    apiKey: {...} # this authn method only valid for POST requests to /foo[/*]
    when:
    - selector: context.request.http.path
      operator: matches
      value: ^/foo(/.*)?$
    - selector: context.request.http.method
      operator: eq
      value: POST

  - name: authn-meth-2
    oidc: {...}

iv. to avoid repetition while defining patterns for conditions:

spec:
  patterns:
    pets:
    - selector: context.request.http.path
      operator: matches
      value: ^/pets/\d+(/.*)$

  metadata:
  - name: pets-info
    when:
    - patternRef: pets
    http:
      endpoint: https://pets-info.io?petId={context.request.http.path.@extract:{"sep":"/","pos":2}}

  authorization:
  - name: pets-owners-only
    when:
    - patternRef: pets
    opa:
      inlineRego: |
        allow { input.metadata["pets-info"].ownerid == input.auth.identity.userid }

v. to avoid repetition while defining JSON pattern-matching authorizaton rules:

spec:
  patterns:
    admin:
    - selector: auth.identity.roles
      operator: incl
      value: admin

  authorization:
  - name: secured-endpoints
    when:
    - selector: context.request.http.path
      operator: matches
      value: ^/secure(/.*)?$
    json:
      rules:
      - patternRef: admin

  - name: only-admins-can-delete
    when:
    - selector: context.request.http.method
      operator: eq
      value: DELETE
    json:
     rules:
      - patternRef: admin

vi. mixing and combining literal expressions and refs:

spec:
  patterns:
    foo:
    - selector: context.request.http.path
      operator: eq
      value: /foo

  when: # unauthenticated access to /foo always granted
  - patternRef: foo
  - selector: context.request.http.headers.authorization
    operator: eq
    value: ""

  authorization:
  - name: my-policy-1
    when: # authenticated access to /foo controlled by policy
    - patternRef: foo
    json: {...}

vii. to avoid evaluating unnecessary identity checks when the user can indicate the preferred authentication method (again the pattern of skipping based upon the context):

spec:
  identity:
  - name: jwt
    when:
    - selector: selector: context.request.http.headers.authorization
      operator: matches
      value: JWT .+
    oidc: {...}

  - name: api-key
    when:
    - selector: context.request.http.headers.authorization
      operator: matches
      value: APIKEY .+
    apiKey: {...}

Verification steps

Try this PR locally with an AuthConfig that combines most of the use case patterns above into one same spec:

make local-setup DEPLOY_KEYCLOAK=1
kubectl -n authorino port-forward deployment/envoy 8000:8000 &

kubectl -n authorino apply -f -<<EOF
apiVersion: authorino.kuadrant.io/v1beta1
kind: AuthConfig
metadata:
  name: talker-api-protection
spec:
  hosts:
  - talker-api-authorino.127.0.0.1.nip.io

  patterns: # these are named sets of expressions that can be reused
    a-pet:
    - selector: context.request.http.path
      operator: matches
      value: ^/pets/\d+(/.*)?$

  when: # top-level conditions for the entire AuthConfig to be enforced
  - selector: context.request.http.path
    operator: neq
    value: /status

  identity:
  - name: idp-users
    oidc:
      endpoint: http://keycloak:8080/auth/realms/kuadrant
    when:
    - selector: context.request.http.headers.authorization
      operator: matches
      value: JWT .+
    credentials:
      in: authorization_header
      keySelector: JWT
    extendedProperties:
    - name: username
      valueFrom:
        authJSON: auth.identity.preferred_username
    - name: roles
      valueFrom:
        authJSON: auth.identity.realm_access.roles

  - name: admins
    apiKey:
      labelSelectors:
        authorino.kuadrant.io/managed-by: authorino
    when:
    - selector: context.request.http.headers.authorization
      operator: matches
      value: APIKEY .+
    credentials:
      in: authorization_header
      keySelector: APIKEY
    extendedProperties:
    - name: username
      valueFrom:
        authJSON: auth.identity.metadata.annotations.username
    - name: roles
      value: ["admin"]

  metadata:
  - name: resource-metadata
    http:
      endpoint: http://talker-api:3000/pets/{context.request.http.path.@extract:{"sep":"/","pos":2}}/meta
      method: GET
    when: # skip unnecessary out-of-context metadata fetching
    - patternRef: a-pet

  authorization:
  - name: owner-policy
    opa:
      inlineRego: |
        allow { input.auth.identity.username == input.metadata["resource-metadata"].owner }
    when:
    - patternRef: a-pet
    - selector: auth.identity.roles
      operator: excl
      value: admin

  - name: admin-role-required
    json:
      rules:
      - selector: auth.identity.roles
        operator: incl
        value: admin
    when:
    - selector: context.request.http.path
      operator: matches
      value: ^/admin(/.*)?$
EOF

kubectl -n authorino apply -f -<<EOF
apiVersion: v1
kind: Secret
metadata:
  name: api-key-1
  labels:
    authorino.kuadrant.io/managed-by: authorino
  annotations:
    username: alice
stringData:
  api_key: ndyBzreUzF4zqDQsqSPMHkRhriEOtcRx
type: Opaque
EOF

Try the /status endpoint, skipped according to the top-level when conditional:

curl http://talker-api-authorino.127.0.0.1.nip.io:8000/status -i
# HTTP/1.1 200 OK

Try with missing authentication on a case where the AuthConfig is enforced, according to the conditions:

curl http://talker-api-authorino.127.0.0.1.nip.io:8000/pets -i
# HTTP/1.1 401 Unauthorized

Try protected endpoints as a user bound to the member role in Keycloak:

ACCESS_TOKEN=$(kubectl -n authorino run token --attach --rm --restart=Never -q --image=curlimages/curl -- http://keycloak:8080/auth/realms/kuadrant/protocol/openid-connect/token -s -d 'grant_type=password' -d 'client_id=demo' -d 'username=john' -d 'password=p' | jq -r .access_token)

curl -H "Authorization: JWT $ACCESS_TOKEN" http://talker-api-authorino.127.0.0.1.nip.io:8000/pets -i
# HTTP/1.1 200 OK

curl -H "Authorization: JWT $ACCESS_TOKEN" http://talker-api-authorino.127.0.0.1.nip.io:8000/pets/123 -i
# HTTP/1.1 403 Forbidden

curl -H "Authorization: JWT $ACCESS_TOKEN" http://talker-api-authorino.127.0.0.1.nip.io:8000/admin -i
# HTTP/1.1 403 Forbidden

Try protected endpoints as a user bound to the admin role in Keycloak:

ACCESS_TOKEN=$(kubectl -n authorino run token --attach --rm --restart=Never -q --image=curlimages/curl -- http://keycloak:8080/auth/realms/kuadrant/protocol/openid-connect/token -s -d 'grant_type=password' -d 'client_id=demo' -d 'username=jane' -d 'password=p' | jq -r .access_token)

curl -H "Authorization: JWT $ACCESS_TOKEN" http://talker-api-authorino.127.0.0.1.nip.io:8000/pets -i
# HTTP/1.1 200 OK

curl -H "Authorization: JWT $ACCESS_TOKEN" http://talker-api-authorino.127.0.0.1.nip.io:8000/pets/123 -i
# HTTP/1.1 200 OK

curl -H "Authorization: JWT $ACCESS_TOKEN" http://talker-api-authorino.127.0.0.1.nip.io:8000/admin -i
# HTTP/1.1 200 OK

Try protected endpoints as an API-key user (admin by definition, according to the AuthConfig):

curl -H "Authorization: APIKEY ndyBzreUzF4zqDQsqSPMHkRhriEOtcRx" http://talker-api-authorino.127.0.0.1.nip.io:8000/pets -i
# HTTP/1.1 200 OK

curl -H "Authorization: APIKEY ndyBzreUzF4zqDQsqSPMHkRhriEOtcRx" http://talker-api-authorino.127.0.0.1.nip.io:8000/pets/123 -i
# HTTP/1.1 200 OK

curl -H "Authorization: APIKEY ndyBzreUzF4zqDQsqSPMHkRhriEOtcRx" http://talker-api-authorino.127.0.0.1.nip.io:8000/admin -i
# HTTP/1.1 200 OK

Closes #205
Closes #24

Conditions
Tests
Docs

eguzki · 2022-01-12T13:01:38Z

First thing in my mind after reading the descr of the PR: s/conditions/when/

Minor thing, but wdyt?

eguzki · 2022-01-12T13:10:37Z

patterns should not be condition-patterns or something similar? To be more meaningful at top level

guicassolato · 2022-01-12T13:12:50Z

First thing in my mind after reading the descr of the PR: s/conditions/when/

Minor thing, but wdyt?

Totally valid suggestion, @eguzki.

I thought about when myself. Ended up going with conditions for 2 reasons:

because it's the same semantics and syntax of the already existing spec.authorization.json.conditions and spec.authorization.kubernetes.conditions
So very "creative" users don't end up thinking there's some cron thing involved here – "when" means "if" in this case

if would also make sense.

eguzki · 2022-01-12T13:13:03Z

I cannot find it. What is the semantic meaning as a boolean operation of a list of conditions? AND, OR?

guicassolato · 2022-01-12T13:13:51Z

I cannot find it. What is the semantic meaning as a boolean operation of a list of conditions? AND, OR?

It's AND.

guicassolato · 2022-01-12T13:14:38Z

patterns should not be condition-patterns or something similar? To be more meaningful at top level

The patterns can also be used in spec.authorization.json.rules, not only in conditions fields.

eguzki · 2022-01-12T13:28:02Z

implementation looks good to me

eguzki · 2022-01-12T14:18:12Z

Influenced by OAS design (sorry I cannot avoid it), I miss some expression in the form of

(cond11 AND cond12 AND ... AND cond1N) OR (cond21 AND cond22 AND ... AND cond2N) OR ... OR (condM1 AND condM2 AND ... AND condMN)

I wonder if this is even a need. But I can think two example:

GET /path1 -> AUTH type: apikey OR http basic  (OAS allows this flexibility)

This use case is already covered having two different identity evaluators that are evaluated with OR semantics. So maybe does not apply but I think it is worth mentioning.

GET /path1 -> AUTH type: apikey
GET /path2 -> AUTH type: apikey

This use case requires defining two different identity evaluators which are exactly the same (except for the conditions)

maleck13 · 2022-01-12T15:19:39Z

api/v1beta1/auth_config_types.go

@@ -175,6 +212,9 @@ type Metadata struct {
 	// +kubebuilder:default:=0
 	Priority int `json:"priority,omitempty"`

+	// Conditions that must match for Authorino to enforce this metadata config; otherwise, the config will be skipped.
+	Conditions []JSONPattern `json:"conditions,omitempty"`


If there are no conditions set, is the default not to skip?

Correct. No conditions means always enforce it.

Perhaps add to the description:

// If omitted, the config will be enforced every time.

?

Yeah sounds good, it is a minor thing , it just caught my attention as I read it

guicassolato · 2022-01-12T15:20:18Z

Influenced by OAS design (sorry I cannot avoid it), I miss some expression in the form of
(cond11 AND cond12 AND ... AND cond1N) OR (cond21 AND cond22 AND ... AND cond2N) OR ... OR (condM1 AND condM2 AND ... AND condMN)
I wonder if this is even a need. But I can think two example:
GET /path1 -> AUTH type: apikey OR http basic  (OAS allows this flexibility)
This use case is already covered having two different identity evaluators that are evaluated with OR semantics. So maybe does not apply but I think it is worth mentioning.
GET /path1 -> AUTH type: apikey
GET /path2 -> AUTH type: apikey 
This use case requires defining two different identity evaluators which are exactly the same (except for the conditions)

No problem, @eguzki.

OR logics can be achieved by repeating the evaluator spec as many times as needed, varying only the conditions block.

For authorization based on JSON pattern-matching, because of #206 (comment), repeating the rules can get slightly cleaner.

E.g.:

spec:
  patterns:
    my-authz-rules:
    - selector: some.json.path
      operator: eq
      value: some-expected-value
    - selector: some.other.json.path
      operation: incl
      value: some-value-that-must-be-present-in-the-selected-array
  authorization:
  - name: use-case-1
    conditions:
    - selector: context.request.http.path
      operator: eq
      value: /path1
    json:
      rules:
      - patternRef: my-authz-rules
  - name: use-case-2
    conditions:
    - selector: context.request.http.path
      operator: eq
      value: /path2
    json:
      rules:
      - patternRef: my-authz-rules

As for the examples you shared...

First one is simpler:

spec:
  patterns:
    get-path1:
    - selector: context.request.http.method
      operation: eq
      value: GET
    - selector: context.request.http.path
      operator: eq
      value: /path1
  identity:
  - name: api-key 
    conditions:
    - patternRef: get-path1
    apikey: {...}
    credentials:
      in: custom_header
      keySelector: API-KEY
  - name: http-basic-auth
    conditions:
    - patternRef: get-path1
    apikey: {...} # RFC 7235 happens to be mimicked in Authorino using API key authn as well, though the label selectors here would probably differ from the ones in the `apiKey` above
    credentials:
      in: authorization_header
      keySelector: Basic

Second one is a bit more annoying because it involves really duplicating the apiKey authn spec:

spec:
  patterns:
    get:
    - selector: context.request.http.method
      operation: eq
      value: GET
  identity:
  - name: path1-authn 
    conditions:
    - patternRef: get
    - selector: context.request.http.path
      operator: eq
      value: /path1
    apikey: {...}
  - name: path2-authn
    conditions:
    - patternRef: get
    - selector: context.request.http.path
      operator: eq
      value: /path2
    apikey: {...} # identical to the `apiKey` above

maleck13 · 2022-01-12T15:43:02Z

docs/user-guides/json-pattern-matching-authorization.md

@@ -124,19 +124,19 @@ spec:
      endpoint: http://keycloak.keycloak.svc.cluster.local:8080/auth/realms/kuadrant
  authorization:
  - name: email-verified-only
+    conditions:


maybe not a good idea but will throw it out there: What if this were named: applyWhen do you think it would me more intuative? Perhaps not just a thought

If you say it's an improvement and not subject to the possible misunderstanding I mention in my 2nd point in #206 (comment), sounds good to me too

To sum up a few options we've got so far:

conditions

when

applyWhen

if

I personally prefer single-word names whenever possible, but only when they are clear enough.

Let's start a quick poll?

Yes as an improvement. Single word is fine also. When gets my vote

Two votes for when. Sounds like a crushing majority to me 🙂
I'll make the changes.

Thanks!

maleck13

I have given it a first review and it looks pretty good to me. As you say it is a breaking change. I am going to cast my eye over it once more as there is quite a bit to take in.

maleck13 · 2022-01-13T11:52:32Z

Other than the suggestion to change conditions to when and a broad conversation about whether to focus config in the CR and Policy in rego. This looks good to me 👍

jjaferson

Code looks good and all the tests described in the verification steps passed.

guicassolato self-assigned this Jan 3, 2022

guicassolato force-pushed the conditions branch 2 times, most recently from 0c0f769 to 1eba442 Compare January 4, 2022 09:12

guicassolato mentioned this pull request Jan 4, 2022

Kuadrant api design v2 Kuadrant/architecture#2

Closed

guicassolato marked this pull request as ready for review January 4, 2022 10:35

guicassolato requested a review from a team January 5, 2022 09:25

guicassolato force-pushed the conditions branch from 2909ab9 to a4d85f7 Compare January 5, 2022 11:17

maleck13 reviewed Jan 12, 2022

View reviewed changes

guicassolato force-pushed the conditions branch 4 times, most recently from 60c689b to 3c4731d Compare January 13, 2022 20:29

guicassolato added 4 commits January 14, 2022 16:14

Conditions

4146191

[docs] Conditions

d1b2b99

conditions -> when

68d062e

[docs] conditions -> when

f6be3b2

guicassolato force-pushed the conditions branch from 3c4731d to f6be3b2 Compare January 14, 2022 15:15

jjaferson approved these changes Jan 17, 2022

View reviewed changes

guicassolato merged commit e791592 into main Jan 17, 2022

guicassolato deleted the conditions branch January 17, 2022 18:33

guicassolato mentioned this pull request Mar 25, 2022

Fix examples using conditions -> when Kuadrant/authorino-examples#13

Merged

Conditions #206

Conditions #206

Uh oh!

Conversation

guicassolato commented Jan 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Verification steps

Uh oh!

eguzki commented Jan 12, 2022

Uh oh!

eguzki commented Jan 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guicassolato commented Jan 12, 2022

Uh oh!

eguzki commented Jan 12, 2022

Uh oh!

guicassolato commented Jan 12, 2022

Uh oh!

guicassolato commented Jan 12, 2022

Uh oh!

eguzki commented Jan 12, 2022

Uh oh!

eguzki commented Jan 12, 2022

Uh oh!

maleck13 Jan 12, 2022

Choose a reason for hiding this comment

Uh oh!

guicassolato Jan 12, 2022

Choose a reason for hiding this comment

Uh oh!

guicassolato Jan 12, 2022

Choose a reason for hiding this comment

Uh oh!

maleck13 Jan 12, 2022

Choose a reason for hiding this comment

Uh oh!

guicassolato commented Jan 12, 2022

Uh oh!

maleck13 Jan 12, 2022

Choose a reason for hiding this comment

Uh oh!

guicassolato Jan 12, 2022

Choose a reason for hiding this comment

Uh oh!

maleck13 Jan 13, 2022

Choose a reason for hiding this comment

Uh oh!

guicassolato Jan 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maleck13 left a comment

Choose a reason for hiding this comment

Uh oh!

maleck13 commented Jan 13, 2022

Uh oh!

jjaferson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guicassolato commented Jan 3, 2022 •

edited

Loading

eguzki commented Jan 12, 2022 •

edited

Loading

guicassolato Jan 13, 2022 •

edited

Loading