trustification / trustify

Apache License 2.0
8 stars 15 forks source link

feat: add an embedded OIDC server #332

Closed ctron closed 2 months ago

bobmcwhirter commented 2 months ago

I'm super excited for this.

ctron commented 2 months ago

It works. I just not sure how to expose this to the user. It should be disabled by default, that's why I put it behind a feature flag, which is off by default. Having the feature flag present during compilation, you also need `--embedded-oidc during startup.

The full command to start is:

cargo run -p trustify-trustd --features garage-door -- api --devmode --embedded-oidc

However, for the PM mode we want this on I guess, by default. Maybe it makes sense to compile two binaries? One insecure, one not.

ctron commented 2 months ago

The PR now also has two binaries (trustd and trustd-pm, the latter includes the embedded OIDC server).

Also, I added a container build, which uses the default trustd only.

ctron commented 2 months ago

Also-- is there a way to communicate to the UI that the embedded OIDC is being used, so we can perhaps slap a Hold up, you're insecure! NOT FOR PRODUCTION USE banner on every page?

@carlosthe19916 I am sure we can come up with one. I could put a marker/claim into the access token, which the UI could use as a trigger.

ctron commented 2 months ago

I want to see the release CI to succeed before merging: https://github.com/ctron/trustify/actions/runs/9271416164

ctron commented 2 months ago

How is this going to impact my life as a developer?

Answer in the form of some doc (e.g. README.md) in the PR, please.

By default that feature is not enabled and the information in the readme still applies.

I agree it might make sense to add this to some installation/getting started document. But I would avoid spamming the main readme with a lot of options.

jcrossley3 commented 2 months ago

Sure, alternate doc is fine. Some summary of what this new ~1000 lines of code does.

ctron commented 2 months ago

@jcrossley3 I added some docs. Linking them from the main readme.

ctron commented 2 months ago

The outcome of the release build would look like this: https://github.com/ctron/trustify/releases/tag/v0.1.0-alpha.1

ctron commented 2 months ago

@jcrossley3 I updated the scope to use a unified /api scope.

bobmcwhirter commented 2 months ago

fwiw, I likewise had to add attachment size config in the tests, since the local configure() wouldn't allow big SBOMs, even though the root configuration does that for me.

value of the above datapoint: no idea.