lxc / incus

Powerful system container and virtual machine manager
https://linuxcontainers.org/incus
Apache License 2.0
2.48k stars 198 forks source link

Add OIDC testing (and re-enable OpenFGA tests with it) #209

Closed stgraber closed 10 months ago

stgraber commented 10 months ago

Currently the OIDC logic isn't being tested, mostly due to lack of an OIDC provider that can be used self-contained within the testsuite. This also makes it currently impossible to automatically test OpenFGA support.

jdstrand commented 10 months ago

While not self-contained in unit tests, you might consider keycloak if you haven't already. At work people have used it in e2e tests (eg in GitHub actions/circleci/etc). You should also be able to run unit tests against it conditionally if an environment variable is set for local dev of the OIDC feature.

stgraber commented 10 months ago

Yeah, we used keycloak for manual testing when we first added OIDC logic. It's unfortunately not the most self-contained or light thing to run :)

I've spent some time this evening looking at the zytadel OIDC provider example code and turned that into the simplest OIDC provider I could come up with. It implements the device code flow that we're using with the CLI and allows for specifying what user we'd like it to authenticate the next request as.

This allows for easily running tests as multiple different users which will be very convenient to enable the OpenFGA tests.

I should have a PR introducing that tiny provider tomorrow alongside some initial OIDC tests and hopefully re-enabling the OpenFGA tests in the same PR.

stgraber commented 10 months ago

image

stgraber commented 10 months ago

Here we can see how it all works.

Basically starting the daemon on a given port. Then configuring the local Incus daemon to use the mini-oidc daemon as the OIDC issuer.

And then removing/adding the same remote (localhost) twice, the first time with no user data provided (shows unknown), the second after telling mini-oidc that the user is to be reported as foo@bar.

stgraber commented 10 months ago
stgraber@dakara:~/data/code/lxc/incus (stgraber/main)$ git show ce6316b98c7af7a8204e1fa3759c03c6660d742d | diffstat
 .gitignore                                |    2 
 Makefile                                  |    3 
 test/mini-oidc/README.md                  |    7 
 test/mini-oidc/go.mod                     |   33 +++
 test/mini-oidc/go.sum                     |  102 ++++++++++++
 test/mini-oidc/main.go                    |  104 ++++++++++++
 test/mini-oidc/storage/client.go          |  235 +++++++++++++++++++++++++++
 test/mini-oidc/storage/local_client.go    |   19 ++
 test/mini-oidc/storage/oidc.go            |  219 ++++++++++++++++++++++++++
 test/mini-oidc/storage/storage.go         |  916 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/mini-oidc/storage/storage_dynamic.go |  281 +++++++++++++++++++++++++++++++++
 test/mini-oidc/storage/token.go           |   25 ++
 test/mini-oidc/storage/user.go            |   88 ++++++++++
 13 files changed, 2034 insertions(+)

The storage sub-package is basically a local copy of https://github.com/zitadel/oidc/tree/main/example/server/storage but with an added client type that matches what Incus needs.

So the actual logic for the OIDC provider is just main.go which clocks at a tiny 104 lines :)