-
Notifications
You must be signed in to change notification settings - Fork 20
Feature/1356 openid configuration #1357
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
base: develop
Are you sure you want to change the base?
Conversation
|
@krowvin Perhaps it would make more sense to just integrate the groundwater login dialog instead of relying on the Swagger UI system? I'm fairly certain it's not difficult to do; should be able to hook into the internal auth bits that fill in those details. |
d3c1058 to
c0d9242
Compare
|
Now includes client_id value within an extension field. Considering
|
krowvin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Up to you if you want to change these things. Tossing info out there!
(There's CI/CD you can setup to remove console logs... before prod)
| }, | ||
| onComplete: () => { | ||
| const spec = JSON.parse(ui.spec().get("spec")); | ||
| console.log(JSON.stringify(spec.components.securitySchemes)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the logs if you are ready for prime time!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still draft, just push that change up before lunch in case the computer crashed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log statements should be gone now.
| // https://vitejs.dev/config/ | ||
| export default defineConfig(({ mode }) => { | ||
| // const env = loadEnv(mode, process.cwd(), ""); | ||
| const env = loadEnv(mode, process.cwd(), ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works with Node, but you should also see
https://vite.dev/guide/env-and-mode
i.e.
const CUSTOM_VAR = import.meta.env.VITE_CUSTOM_VAR
Where you prefix with VITE_ to avoid accidental env var injection.. from your .env files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, was wondering why that that was comment out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, that doesn't actually work within the vite.config.js
|
Okay, now it's ready. Once it deploys to -dev you should be able to use the very first OIDC option ("Code") and not have to know the client id and get forwarded through the various identity provides and otherwise login as usual. |
Confirming the user still clicks the green Then later we might add a Login button somewhere that could also tie into that? I imagine SwaggerUI (constructor) has a callback we can use to setup a Login button. Let me know if you want me to look into that first before trying to setup the GWW login methods. (It only supports direct-login right now afaik) |
Correct, other than not needing to already know the client id, and being able to choose the first option, the user side flow is the same. (They will get redirected to the auth server, but people should be used to that by now in general.) |
9879386 to
bd274af
Compare
|
@jbkolze @oskarhurst @krowvin FYI, keycloak login in -dev is currently broken due to the cache buster code sending unknown parameters to the keycloak instance which then intentionally fails since it doesn't like unknown query parameters. Somebody give this a look when you have time. There will likely be improvements to the SecurityScheme extension in a follow up, like providing a link to a graphic. Work on opendcs/rest_api#574 is informing that. (Plan to share the extension design since... why not, same need.) |
bd274af to
25c1edf
Compare
Fixes #1356.
Definitely still needs work (should only try the initOAuth if an OAuth based security scheme is actually present, but it gets the idea across.
Unfortunately, at least using the default Swagger Authentication system, I don't think I can do what I wanted where one, in the auth button, could have a login that would automatically work and pass the additional required query parameter. The SwaggerUI code simply doesn't do anything with extension values provided in the scheme.
However, that really only affects the SwaggerUI not usages of the application itself. So I'm inclined not to worry about it and will strip of some of the extra elements I added to the code.
Good news is, does pre-populate the client id, and the login flow behaves correctly; I was redirected to the local docker-compose keycloak and saw the kc_idp_hint field on the request so in environment should behave the same.