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

fix: conversion of raw extension fields #431

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

guicassolato
Copy link
Collaborator

@guicassolato guicassolato commented Sep 26, 2023

Fix back and forth conversion of runtime.RawExtension fields.

Example of fields and values affected:

  • Extended properties of identity objects (new defaults and overrides of the authentication configs)
  • Custom headers of the former denyWith spec (new response.(unauthenticated|unauthorized).headers)
  • Custom message and body of the former denyWith spec, as well as all other fields typed in v1beta1 as StaticOrDynamicValue, when the field was set to a static string value containing line breaks or any other character that requires escaping when marshalled

Closes #430.

Steps to reproduce

make local-setup FF=1
kubectl apply -f <<EOF
apiVersion: authorino.kuadrant.io/v1beta1
kind: AuthConfig
metadata:
  labels:
    testRun: testrun-phala-kudrnt-te-rd8b-w2ycvae
  name: ac-phala-kudrnt-te-g5bv
spec:
  authorization:
  - json:
      rules:
      - operator: eq
        selector: auth.identity.typ
        value: Bearer
    metrics: false
    name: bearer-token
    priority: 0
  - metrics: false
    name: deny-list
    opa:
      allValues: false
      inlineRego: |
        list := [
          "denied-test-user1@example.com"
        ]
        denied { list[_] == input.auth.identity.email }
        allow { not denied }
    priority: 0
    when:
    - patternRef: acl-required
  - metrics: false
    name: allow-list
    opa:
      allValues: false
      inlineRego: |
        list := [
          "123"
        ]
        allow { list[_] == input.auth.identity.org_id }
    priority: 0
    when:
    - patternRef: acl-required
  - json:
      rules:
      - operator: eq
        selector: auth.metadata.terms-and-conditions.terms_required
        value: "false"
    metrics: false
    name: terms-and-conditions
    priority: 0
    when:
    - patternRef: create-dinosaur-route
  - json:
      rules:
      - patternRef: redhat-sso
      - patternRef: require-org-id
    metrics: false
    name: dinosaurs
    priority: 0
    when:
    - patternRef: dinosaurs-route
  - json:
      rules:
      - patternRef: redhat-sso
      - patternRef: require-org-id
    metrics: false
    name: metrics-federate
    priority: 0
    when:
    - patternRef: metrics-federate-route
  - json:
      rules:
      - patternRef: redhat-sso
      - patternRef: require-org-id
    metrics: false
    name: service-accounts
    priority: 0
    when:
    - patternRef: service-accounts-route
  - json:
      rules:
      - patternRef: redhat-sso
      - patternRef: require-org-id
    metrics: false
    name: supported-instance-types
    priority: 0
    when:
    - patternRef: supported-instance-types-route
  - json:
      rules:
      - patternRef: redhat-sso
    metrics: false
    name: agent-clusters
    priority: 0
    when:
    - patternRef: agent-clusters-route
  - metrics: false
    name: cluster-id
    opa:
      allValues: false
      inlineRego: |
        allow { input.auth.identity.azp == object.get(input.auth.metadata, "cluster-info", {}).client_id }
    priority: 0
    when:
    - patternRef: agent-clusters-route
  - metrics: false
    name: owner
    opa:
      allValues: false
      inlineRego: |
        org_id := input.auth.identity.org_id
        filter_by_org { org_id }
        is_org_admin := input.auth.identity.is_org_admin
        resource_data := object.get(input.auth.metadata, "resource-info", {})
        same_org { resource_data.org_id == org_id }
        is_owner { resource_data.owner == input.auth.identity.azp }
        has_permission { filter_by_org; same_org; is_org_admin }
        has_permission { filter_by_org; same_org; is_owner }
        has_permission { not filter_by_org; is_owner }
        method := input.context.request.http.method
        allow { method == "GET";    has_permission }
        allow { method == "DELETE"; has_permission }
        allow { method == "PATCH";  has_permission }
    priority: 0
    when:
    - patternRef: dinosaur-resource-route
  - metrics: false
    name: admin-rbac
    opa:
      allValues: false
      inlineRego: |
        method := input.context.request.http.method
        roles := input.auth.identity.realm_access.roles
        allow { method == "GET";    roles[_] == "admin-full" }
        allow { method == "GET";    roles[_] == "admin-read" }
        allow { method == "GET";    roles[_] == "admin-write" }
        allow { method == "PATCH";  roles[_] == "admin-full" }
        allow { method == "PATCH";  roles[_] == "admin-write" }
        allow { method == "DELETE"; roles[_] == "admin-full" }
    priority: 0
    when:
    - patternRef: admin-route
    - patternRef: admin-sso
  - metrics: false
    name: admin-redhat-rhsso
    opa:
      allValues: false
      inlineRego: |
        allow { false }
    priority: 0
    when:
    - patternRef: admin-route
    - patternRef: redhat-sso
  - json:
      rules:
      - operator: neq
        selector: context.request.http.path.@extract:{"sep":"/","pos":4}
        value: authz-metadata
    metrics: false
    name: internal-endpoints
    priority: 0
  denyWith:
    unauthorized:
      body:
        value: |
          {
            "kind": "Error",
            "id": "403",
            "href": "/api/dinosaurs_mgmt/v1/errors/403",
            "code": "DINOSAURS-MGMT-403",
            "reason": "Forbidden"
          }
      headers:
      - name: content-type
        value: application/json
  hosts:
  - hostname-phala-kudrnt-te-rlya-authorino.apps.ocp-wide.osp.api-qe.eng.rdu2.redhat.com
  identity:
  - extendedProperties:
    - name: org_id
      overwrite: false
      valueFrom:
        authJSON: auth.identity.middle_name
    metrics: false
    name: redhat-sso
    oidc:
      endpoint: http://no-ssl-sso-tools.apps.ocp-wide.osp.api-qe.eng.rdu2.redhat.com/auth/realms/realm-phala-kudrnt-te-mieu
      ttl: 3600
    priority: 0
  - metrics: false
    name: admin-sso
    oidc:
      endpoint: http://no-ssl-sso-tools.apps.ocp-wide.osp.api-qe.eng.rdu2.redhat.com/auth/realms/realm-phala-kudrnt-te-eeoz
      ttl: 3600
    priority: 0
    when:
    - patternRef: admin-route
  metadata:
  - http:
      contentType: application/json
      endpoint: http://mockserver-tools.apps.ocp-wide.osp.api-qe.eng.rdu2.redhat.com/testrun-phala-kudrnt-te-rd8b-w2ycvae-terms
      method: GET
    metrics: false
    name: terms-and-conditions
    priority: 0
    when:
    - patternRef: create-dinosaur-route
  - http:
      contentType: application/json
      endpoint: http://mockserver-tools.apps.ocp-wide.osp.api-qe.eng.rdu2.redhat.com/testrun-phala-kudrnt-te-rd8b-w2ycvae-cluster
      method: GET
    metrics: false
    name: cluster-info
    priority: 0
    when:
    - patternRef: agent-clusters-route
  - http:
      contentType: application/json
      endpoint: http://mockserver-tools.apps.ocp-wide.osp.api-qe.eng.rdu2.redhat.com/testrun-phala-kudrnt-te-rd8b-w2ycvae-resource
      method: GET
    metrics: false
    name: resource-info
    priority: 0
    when:
    - patternRef: dinosaur-resource-route
  patterns:
    acl-required:
    - operator: neq
      selector: context.request.http.path.@extract:{"sep":"/","pos":4}
      value: agent-clusters
    - operator: neq
      selector: context.request.http.path.@extract:{"sep":"/","pos":4}
      value: admin
    admin-route:
    - operator: eq
      selector: context.request.http.path.@extract:{"sep":"/","pos":4}
      value: admin
    admin-sso:
    - operator: eq
      selector: auth.identity.iss
      value: http://no-ssl-sso-tools.apps.ocp-wide.osp.api-qe.eng.rdu2.redhat.com/auth/realms/realm-phala-kudrnt-te-eeoz
    agent-clusters-route:
    - operator: eq
      selector: context.request.http.path.@extract:{"sep":"/","pos":4}
      value: agent-clusters
    api-route:
    - operator: matches
      selector: context.request.http.path
      value: ^/anything/dinosaurs_mgmt/.+
    create-dinosaur-route:
    - operator: matches
      selector: context.request.http.path
      value: /dinosaurs/?$
    - operator: eq
      selector: context.request.http.method
      value: POST
    dinosaur-resource-route:
    - operator: matches
      selector: context.request.http.path
      value: /dinosaurs/[^/]+$
    dinosaurs-route:
    - operator: eq
      selector: context.request.http.path.@extract:{"sep":"/","pos":4}
      value: dinosaurs
    metrics-federate-route:
    - operator: eq
      selector: context.request.http.path.@extract:{"sep":"/","pos":4}
      value: dinosaurs
    - operator: matches
      selector: context.request.http.path
      value: /metrics/federate$
    redhat-sso:
    - operator: eq
      selector: auth.identity.iss
      value: http://no-ssl-sso-tools.apps.ocp-wide.osp.api-qe.eng.rdu2.redhat.com/auth/realms/realm-phala-kudrnt-te-mieu
    require-org-id:
    - operator: neq
      selector: auth.identity.org_id
      value: ""
    service-accounts-route:
    - operator: eq
      selector: context.request.http.path.@extract:{"sep":"/","pos":4}
      value: service_accounts
    supported-instance-types-route:
    - operator: eq
      selector: context.request.http.path.@extract:{"sep":"/","pos":4}
      value: instance_types
    v1-route:
    - operator: eq
      selector: context.request.http.path.@extract:{"sep":"/","pos":3}
      value: v1
  response:
  - json:
      properties:
      - name: auth
        valueFrom:
          authJSON: auth
      - name: context
        valueFrom:
          authJSON: context
    metrics: false
    name: auth-json
    priority: 0
    wrapper: httpHeader
  when:
  - patternRef: api-route
  - patternRef: v1-route
EOF

Creating the AuthConfig should throw no error, as well as listing the resources afterwards:

kubectl get authconfig/ac-phala-kudrnt-te-g5bv -o yaml

Fix back and forth conversion of [`runtime.RawExtension`)(https://pkg.go.dev/k8s.io/apimachinery@v0.23.0/pkg/runtime#RawExtension) fields.

Example of fields and values affected:
- Extended properties of identity objects (new `defaults` and `overrides` of the authentication config)
- Custom `headers` of the former `denyWith` spec (new `response.(unauthenticated|unauthorized).headers`)
- Custom `message` and `body` of the former `denyWith` spec, as well as all other fields typed in v1beta1 as [`StaticOrDynamicValue`](https://pkg.go.dev/github.com/kuadrant/authorino/api/v1beta1#StaticOrDynamicValue), when the field was set to a static string value containing line breaks or any other character that require escaping when marshalled

Closes #430.
@guicassolato guicassolato self-assigned this Sep 26, 2023
@guicassolato guicassolato requested review from pehala and a team September 26, 2023 12:21
Copy link

@pehala pehala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@adam-cattermole adam-cattermole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, worked as expected in verification

@guicassolato guicassolato merged commit 991af36 into main Sep 26, 2023
9 checks passed
@guicassolato guicassolato deleted the fix/conversion-raw-extension branch September 26, 2023 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working size/small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conversion webhook contains a lot of errors
3 participants