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.

Sign up for GitHub

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

Autpolicy doc fixes #76

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Autpolicy doc fixes #76

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4c5b309
Fix openapi yaml examples
azgabur May 7, 2024
0635010
Fix spacing
azgabur May 7, 2024
2efa953
Add missing gateway to guide and sso url refactor
azgabur May 7, 2024
a9db0d0
Fix oidc url example and typos
azgabur May 9, 2024
e7956c8
Move notes to better place
azgabur May 22, 2024
2c9bdd1
Minor text fix
azgabur Jun 13, 2024
dba0c3a
Fix word then
azgabur Jun 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 65 additions & 31 deletions doc/generate-kuadrant-auth-policy.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ OpenAPI [Security Scheme Object](https://spec.openapis.org/oas/latest.html#secur
The following OAS example has one protected endpoint `GET /dog` with `openIdConnect` security scheme type.

```yaml
openapi: "3.1.0"
info:
title: "Pet Store API"
version: "1.0.0"
paths:
/dog:
get:
Expand All @@ -41,21 +45,17 @@ components:
openIdConnectUrl: https://example.com/.well-known/openid-configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

This particular example is also broken and we should not be sharing it without at least a similar warning to the user as this one proposed further below.

Explanation:

Copying a well-known OpenId Connect Discovery URL to the AuthPolicy's jwt.issuerUrl field will not work.

That's why kuadrantctl actually expects in the openIdConnectUrl field of the OAS an issuer URL identifier instead. This is a URL without the /.well-known/openid-configuration part, which is automatically appended to the value obtained from jwt.issuerUrl.

Users who follow the OAS docs though may inadvertently use the full Discovery URL. We do not want that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good catch! Please look at my next commit a9db0d0
I removed the mention of the /.well-known/openid-configuration path so it should be less confusing.

I am just wondering if there is a reason why AuthPolicy expects issuer instead of openid-configuration path?
The response JSON from openid-configuration contains value of issuer so theoretically it could be used as well. Also if the current practice is to use openid-configuration path in OAS yaml-s as you mentioned.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am just wondering if there is a reason why AuthPolicy expects issuer instead of openid-configuration path?

It's because of the OIDC library used by Authorino. In its simplest form, it expects an issuer URL and not the (more specific) Discovery endpoint. Otherwise, Authorino itself would have to perform the request to fetch the OIDC config, parse its response, handle possible exceptions, and then start using the library that, among other things, let us validate signed JWTs.

I would advocate for more flexibility in Authorino's JWT verification method, which surfaces into the Kuadrant AuthPolicy. Along with issuerUrl, I'd love to add discoveryUrl, jwksURL, and possibly others. It's just that currently this is not the state of things... yet.

```

Running the command
Take this example and save it as `example.yaml` and than run the command:
azgabur marked this conversation as resolved.
Show resolved Hide resolved

```
kuadrantctl generate kuadrant authpolicy --oas ./petstore-openapi.yaml | yq -P
```bash
kuadrantctl generate kuadrant authpolicy --oas example.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

i dont really get this change, why change the name to be example and why use remove the pretty print?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The yp command has been removed from multiple guides as is now not needed. The pretty print is done as default.
I changed the name of the yaml file so it does not clash with the same name as the yaml file generated as part of User guide that follows later. I am opened to better file name suggestion though.

```

The generated authpolicy (only relevan fields shown here):

```yaml
kind: AuthPolicy
apiVersion: kuadrant.io/v1beta2
metadata:
name: petstore
namespace: petstore
creationTimestamp: null
spec:
routeSelectors:
- matches:
Expand All @@ -82,6 +82,10 @@ spec:
The following OAS example has one protected endpoint `GET /dog` with `apiKey` security scheme type.

```yaml
openapi: "3.1.0"
info:
title: "Pet Store API"
version: "1.0.0"
paths:
/dog:
get:
Expand All @@ -99,21 +103,17 @@ components:
in: query
```

Running the command
Take this example and save it as `example.yaml` and than run the command:
azgabur marked this conversation as resolved.
Show resolved Hide resolved

```
kuadrantctl generate kuadrant authpolicy --oas ./petstore-openapi.yaml | yq -P
```bash
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

kuadrantctl generate kuadrant authpolicy --oas example.yaml
```

The generated authpolicy (only relevan fields shown here):

```yaml
kind: AuthPolicy
apiVersion: kuadrant.io/v1beta2
metadata:
name: petstore
namespace: petstore
creationTimestamp: null
spec:
routeSelectors:
- matches:
Expand Down Expand Up @@ -305,7 +305,7 @@ components:
in: header
oidc:
type: openIdConnect
openIdConnectUrl: https://${KEYCLOAK_PUBLIC_DOMAIN}/auth/realms/petstore
openIdConnectUrl: ${KEYCLOAK_ISSUER}
snakes_api_key:
type: apiKey
name: snake_token
Expand All @@ -315,9 +315,39 @@ EOF

</details>

> Replace `${KEYCLOAK_PUBLIC_DOMAIN}` with your SSO instance domain
> Replace `${KEYCLOAK_ISSUER}` with your SSO instance issuer endpoint for your `petstore` realm.
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about moving this to be above the resource that uses it so they know to set it before they create the resource it falls more in line with other docs we have

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I moved it

> Otherwise remove the oidc from `components.securitySchemas` and `/dog`, `/snake` paths

* Create `istio-ingressgateway` Gateway object

```yaml
kubectl apply -f -<<EOF
apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
name: istio-ingressgateway
namespace: istio-system
spec:
gatewayClassName: istio
listeners:
- allowedRoutes:
namespaces:
from: All
hostname: 'example.com'
name: api
port: 80
protocol: HTTP
EOF
```
* Get the IP
azgabur marked this conversation as resolved.
Show resolved Hide resolved

```bash
export INGRESS_IP=$(kubectl get -n istio-system Service/istio-ingressgateway-istio -o=jsonpath='{.status.loadBalancer.ingress[0].ip}')
```


* Create an API key only valid for `POST /api/v1/cat` endpoint

```yaml
kubectl apply -f -<<EOF
apiVersion: v1
Expand All @@ -333,6 +363,7 @@ stringData:
type: Opaque
EOF
```

> **Note**: the label's value of `kuadrant.io/apikeys-by: cat_api_key` is the name of the sec scheme of the OpenAPI spec.

* Create an API key only valid for `GET /api/v1/snake` endpoint
Expand Down Expand Up @@ -372,15 +403,15 @@ Now, we are ready to test OpenAPI endpoints :exclamation:
- `GET /api/v1/cat` -> It's a public endpoint, hence should return 200 Ok

```bash
curl -H "Host: example.com" -i "http://127.0.0.1:9080/api/v1/cat"
curl -H "Host: example.com" -i "http://${INGRESS_IP}/api/v1/cat"
```

- `POST /api/v1/cat` -> It's a protected endpoint with apikey

Without any credentials, it should return `401 Unauthorized`

```bash
curl -H "Host: example.com" -X POST -i "http://127.0.0.1:9080/api/v1/cat"
curl -H "Host: example.com" -X POST -i "http://${INGRESS_IP}/api/v1/cat"
```

```
Expand All @@ -403,7 +434,7 @@ What if we try a wrong token? one token assigned to other endpoint,
i.e. `I_LIKE_SNAKES` instead of the valid one `I_LIKE_CATS`. It should return `401 Unauthorized`.

```bash
curl -H "Host: example.com" -H "api_key: I_LIKE_SNAKES" -X POST -i "http://127.0.0.1:9080/api/v1/cat"
curl -H "Host: example.com" -H "api_key: I_LIKE_SNAKES" -X POST -i "http://${INGRESS_IP}/api/v1/cat"
```

```
Expand All @@ -422,40 +453,43 @@ The *reason* headers tell that `the API Key provided is invalid`.
Using valid token (from the secret `cat-api-key-1` assigned to `POST /api/v1/cats`)
in the `api_key` header should return 200 Ok

```
curl -H "Host: example.com" -H "api_key: I_LIKE_CATS" -X POST -i "http://127.0.0.1:9080/api/v1/cat"
```bash
curl -H "Host: example.com" -H "api_key: I_LIKE_CATS" -X POST -i "http://${INGRESS_IP}/api/v1/cat"
```

- `GET /api/v1/dog` -> It's a protected endpoint with oidc (assigned to our keycloak instance and `petstore` realm)

without credentials, it should return `401 Unauthorized`

```bash
curl -H "Host: example.com" -i "http://127.0.0.1:9080/api/v1/dog"
curl -H "Host: example.com" -i "http://${INGRESS_IP}/api/v1/dog"
```

#### [Optional] SSO example

To get the authentication token, this example is using Direct Access Grants oauth2 grant type
(also known as Client Credentials grant type). When configuring the Keycloak (OIDC provider) client
(also known as Resource Owner Password Credentials Grant grant type). When configuring the Keycloak (OIDC provider) client
settings, we enabled Direct Access Grants to enable this procedure.
We will be authenticating as `bob` user with `p` password.
We previously created `bob` user in Keycloak in the `petstore` realm.
We will use Command-line JSON processor `jq` to extract the access token into `ACCESS_TOKEN` variable:

```
```bash
export ACCESS_TOKEN=$(curl -k -H "Content-Type: application/x-www-form-urlencoded" \
-d 'grant_type=password' \
-d 'client_id=petstore' \
-d 'scope=openid' \
-d 'username=bob' \
-d 'password=p' "https://${KEYCLOAK_PUBLIC_DOMAIN}/auth/realms/petstore/protocol/openid-connect/token" | jq -r '.access_token')
-d 'password=p' \
"${KEYCLOAK_TOKEN_ENDPOINT}" | jq -r '.access_token')
```

> Replace `${KEYCLOAK_PUBLIC_DOMAIN}` with your SSO instance domain

> Replace `${KEYCLOAK_TOKEN_ENDPOINT}` with your SSO instance token endpoint for your `petstore` realm.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same like the other var moving to be above where its needed so people can set it then run the bash command


With the access token in place, let's try to get those puppies

```bash
curl -H "Authorization: Bearer $ACCESS_TOKEN" -H 'Host: example.com' http://127.0.0.1:9080/api/v1/dog -i
curl -H "Authorization: Bearer ${ACCESS_TOKEN}" -H 'Host: example.com' "http://${INGRESS_IP}/api/v1/dog" -i
```

it should return 200 OK
Expand All @@ -468,20 +502,20 @@ for an OpenAPI operation.
Without credentials, it should return `401 Unauthorized`

```bash
curl -H "Host: example.com" -i "http://127.0.0.1:9080/api/v1/snake"
curl -H "Host: example.com" -i "http://${INGRESS_IP}/api/v1/snake"
```

With the access token in place, it should return 200 OK (unless the token has expired).

```bash
curl -H "Authorization: Bearer $ACCESS_TOKEN" -H 'Host: example.com' http://127.0.0.1:9080/api/v1/snake -i
curl -H "Authorization: Bearer ${ACCESS_TOKEN}" -H 'Host: example.com' "http://${INGRESS_IP}/api/v1/snake" -i
```

With apiKey it should also work. According to the OpenAPI spec security scheme,
it should be a query string named `snake_token` and the token needs to be valid token
(from the secret `snake-api-key-1` assigned to `GET /api/v1/snake`)

```bash
curl -H 'Host: example.com' -i "http://127.0.0.1:9080/api/v1/snake?snake_token=I_LIKE_SNAKES"
curl -H 'Host: example.com' -i "http://${INGRESS_IP}/api/v1/snake?snake_token=I_LIKE_SNAKES"
```

Loading