istio and keycloak integration #32

GuentherJulian · 2021-06-17T13:37:11Z

Integration of Istio and Keycloak features

hohwille

Seems as if only k8s configs have been added.
Assuming they have been tested and work, I approve this.
Anybody else active here giving feedback???

…tion

hohwille · 2021-06-22T07:51:01Z

@GuentherJulian is this PR incomplete as you prefixed it with WIP:?
BTW: This is github and not gitlab, next time please use this github feature: https://github.blog/2019-02-14-introducing-draft-pull-requests/

GuentherJulian · 2021-06-22T08:09:52Z

@GuentherJulian is this PR incomplete as you prefixed it with WIP:?
BTW: This is github and not gitlab, next time please use this github feature: https://github.blog/2019-02-14-introducing-draft-pull-requests/

@hohwille It's not finished yet. I am still working on it and would like to add some additional features to be able to check permissions in code.
I will convert it in draft and let you know when it is ready.

GuentherJulian · 2021-06-23T13:03:06Z

@hohwille Can you check this PR again? I have added an example showing the validation of JWTs in Istio and within the application.

documentation/istio_keycloak_integration.asciidoc

lilliCao · 2021-06-24T09:51:07Z

documentation/istio_keycloak_integration.asciidoc

+```
+You will get an error message 401 Unauthorized. This is because this operation is secured with the role `demo-quarkus.SaveObject`. You can only access this operation if you pass a valid JWT token in the request header. So add the role to the user in Keycloak and run the following command to get the token.
+```
+TOKEN=$(curl -d 'client_id=demo-quarkus-cli' -d 'username=demo' -d 'password=demo' -d 'grant_type=password' 'http://keycloak-demo-quarkus.localhost/auth/realms/demo-quarkus/protocol/openid-connect/token' | jq ".access_token" -r)


request token with client secret too. 'client_secret=7745de1c-a85e-4bfe-97fe-42dc2b0b3f65'. Without that, istio will return Jwks doesn't have key to match kid or alg from Jwt if authorization_policy.yaml is deployed.

You only need to pass the client secret if 'Access Type' of the Keycloak client is set to confidential. In the json file of the realm, the client 'demo-quarkus-cli' is specified as public client.

lilliCao · 2021-06-24T13:43:16Z

Also, I personally think we should put the code in a separated branch instead of merging to main in case we want to test other stuffs with the ref application @GuentherJulian @hohwille

hohwille · 2021-07-05T08:24:27Z

Also, I personally think we should put the code in a separated branch instead of merging to main in case we want to test other stuffs with the ref application @GuentherJulian @hohwille

Maintaining different branches in parallel does not work on the long run. In such case it is better to use a feature toggle but keep a single main branch maintained.

documentation/istio_keycloak_integration.asciidoc

hohwille

@GuentherJulian thanks for this PR. 👍
You got the integration working and extended our sample app with some scenarios to test everything together in action.
If that was the task and goal of this PR all is fine.
With my background in devon4j I am always thinking further and already would like to shape what we need on the long run.
However, I am also happy that we can be agile, create some issues from the review comments and merge this PR as is.
First, have a look at my comments and try to see what makes sense for you and what you could address easily.
IMHO it is really important to have the OCX team on board and get their POV and review feedback as well.

hohwille · 2021-07-16T16:01:48Z

documentation/istio_keycloak_integration.asciidoc

+=== Introducing Keycloak
+https://www.keycloak.org/[Keycloak] is an opensource IAM (Identity and Access Management) solution with a broad set of features like SSO, authentication and authorization, social login, multifactor authentication etc.


from our experience in devon4j structuring documentation like this does not work out and is not maintainable.
Therefore my suggestion is:

we create separate asciidoc files for keycloak and istio where we can document general aspects for each of them

in file like this where we want to document the integration between those two we can link to each of them but shall IMHO not explain anything more but focus on the integration aspects.

hohwille · 2021-07-16T16:04:55Z

documentation/istio_keycloak_integration.asciidoc

+To create the cluster, we use k3d. First create a local Docker registry.
+```
+k3d registry create registry --port 5000
+```
+The next step is to create the cluster. Navigate to the folder with the reference application and run the following command in your WSL environment.
+```
+k3d cluster create -c k8s/cluster-setup.yaml --k3s-server-arg '--no-deploy=traefik'
+```
+This will create a local Kubernetes cluster with one master and two worker nodes within your WSL environment. By default, k3d creates a Traefik proxy as an ingress controller. Since we will use the Istio Ingress Gateway later, we do not need it here.


Shall we really commit ourselves to k3d?
In the PR for devonfw-ide I suggested that we go for docker desktop.
In real environments we will always use kubernetes natively.
So if we locally work with k3d the guide only works locally and only if k3d is present...

I am happy to be told otherwise but why dont we describe this directly with kubectl?

hohwille · 2021-07-16T16:07:08Z

documentation/istio_keycloak_integration.asciidoc

+=== Install Istio
+The first step is to install Istio. You can find a https://istio.io/latest/docs/setup/getting-started/[Getting Started] guide and some sample applications on the Istio homepage. +
+Navigate in your home directory and execute the following commands:
+```
+curl -L https://istio.io/downloadIstio | sh -
+cd istio-1.10.0
+export PATH=$PWD/bin:$PATH
+```


do we need to "install" istio natively on our machine?
Dont we run this as container as well?

Further such instruction also does not seem stable and maintainable.
When I was testing the version had meanwhile already changed and I had to do cd istio-1.10.2.

Further if we just recommend to follow specific installation instructions from the web, we could simply link them:
https://istio.io/latest/docs/setup/getting-started/

This is already maintained and updated to the latest version.

So we can link instead of copying.

Otherwise we can think about full automation where it really makes sense or integration in devonfw-ide, etc. where suitable.

hohwille · 2021-07-16T16:15:34Z

documentation/istio_keycloak_integration.asciidoc

+=== Build the application and create a Docker image
+Now we need to build the application and the corresponding Docker image. Use Maven to build the application.
+```
+mvn clean package
+```
+If you want to create a native executable use:
+```
+mvn clean package -Pnative
+```
+If tests fail because of authorization configurations you can add `-Dmaven.test.skip=true` option to skip the tests.
+
+Then we create a Docker image and push it into your local Docker registry, which was started by k3d.
+
+```
+docker build -f src/main/docker/Dockerfile.jvm . -t demo-quarkus
+docker tag demo-quarkus k3d-registry:5000/demo-quarkus


all this has nothing to do with the integration of keycloak and istio what this documentation should be about.

hohwille · 2021-07-16T16:23:25Z

...e-project/src/main/java/com/devonfw/demoquarkus/rest/v1/controller/SecureRestController.java

+    @Inject
+    JsonWebToken jwt;


I am not familiar with this style so sorry for asking:
You are making the REST service @RequestScope so physically for each request an own instance is created and the JWT passed as HTTP parameter is injected into this stateful bean instead of passing it as method parameter?

I rahter find this a magical confusion but maybe this is quarkus best practice.
However, I do not get why then we receive SecurityContext as paramter than and the JWT as injection.

hohwille · 2021-07-16T16:24:49Z

...e-project/src/main/java/com/devonfw/demoquarkus/rest/v1/controller/SecureRestController.java

+        if (ctx.getUserPrincipal() == null) { 
+            name = "anonymous";
+        } else if (!ctx.getUserPrincipal().getName().equals(jwt.getName())) { 
+            throw new InternalServerErrorException("Principal and JsonWebToken names do not match");


it is always good to assert things but is there any real reason how this could ever happen?
Sounds like inconsistency in quarkus itself if this case ever occurs...

hohwille · 2021-07-16T16:31:29Z

...e-project/src/main/java/com/devonfw/demoquarkus/rest/v1/controller/SecureRestController.java

+
+    @GET
+    @Path("/roles-allowed/admin") 
+    @RolesAllowed({ "admin" }) 


OK. So like in spring we have the freedom to put @RolesAllowed annotation "everywhere" and it is just working via AOP. So next we could think about some best practices how to do it.
In devon4j we always annotated use-cases as one use-case may call another use-case.
Do we give up such principles in quarkus and let the users do whatever they like?
Will we then call functionality from one service method in another service method? What if @Context parameters are required that prevent such usage.
My experience was that the REST service is simply exposing functionality to the outside world but nothing more.
However, I am already discussing beyond the goal of this acutal PR...

istio and keycloak integration

a2aaee8

hohwille approved these changes Jun 18, 2021

View reviewed changes

Merge branch 'devonfw-forge:main' into feature/istio_keycloak_integra…

271c635

…tion

GuentherJulian self-assigned this Jun 22, 2021

GuentherJulian marked this pull request as draft June 22, 2021 08:10

GuentherJulian changed the title ~~WIP: istio and keycloak integration~~ istio and keycloak integration Jun 22, 2021

added authorization in code

18525e1

GuentherJulian marked this pull request as ready for review June 23, 2021 12:58

added documentation for istio and keycloak integration

194c419

GuentherJulian requested a review from hohwille June 23, 2021 13:03

lilliCao reviewed Jun 24, 2021

View reviewed changes

changed documentation

f0f2406

GuentherJulian added 4 commits June 28, 2021 11:11

added documentation

ea361a7

added documentation

a41db94

added secure gateway definition

0ea1b62

modified documentation

8560b18

hohwille reviewed Jul 5, 2021

View reviewed changes

documentation/istio_keycloak_integration.asciidoc Outdated Show resolved Hide resolved

documentation/istio_keycloak_integration.asciidoc Outdated Show resolved Hide resolved

changed titles in documentation

1ef0ac0

hohwille reviewed Jul 16, 2021

View reviewed changes

GuentherJulian mentioned this pull request Oct 19, 2021

Implement access control concepts devonfw-sample/devon4quarkus-reference#27

Open

		=== Introducing Keycloak
		https://www.keycloak.org/[Keycloak] is an opensource IAM (Identity and Access Management) solution with a broad set of features like SSO, authentication and authorization, social login, multifactor authentication etc.

istio and keycloak integration #32

Are you sure you want to change the base?

istio and keycloak integration #32

Uh oh!

Conversation

GuentherJulian commented Jun 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hohwille left a comment

Choose a reason for hiding this comment

Uh oh!

hohwille commented Jun 22, 2021

Uh oh!

GuentherJulian commented Jun 22, 2021

Uh oh!

GuentherJulian commented Jun 23, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lilliCao Jun 24, 2021 • 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!

Choose a reason for hiding this comment

Uh oh!

lilliCao commented Jun 24, 2021

Uh oh!

hohwille commented Jul 5, 2021

Uh oh!

Uh oh!

Uh oh!

hohwille left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GuentherJulian commented Jun 17, 2021 •

edited

Loading

lilliCao Jun 24, 2021 •

edited

Loading