guacamole-client
guacamole-client copied to clipboard
GUACAMOLE-1094: Add overridable response_type to auth-openid extension
Allows overriding the default value of the response_type parameter in the OpenID auth extension for compatibility with authentication servers that require specific values that are different than the default "id_token", like AWS Cognito. See the Jira issue for further explanation.
@lapchynski: Thanks for the contribution. Overall this looks good to me. My only question would be - are there a set of discreet values that are valid response types, and instead of making this a free-form field, should we instead try to make sure that the admin is putting something into the field that is valid per the OID specification?
Personally I think it'd be better as a free-form field to allow the admin more flexibility with OIDC implementations that may not follow the spec to the letter. If everything followed the spec to the letter then leaving "response_type=id_token" hard-coded wouldn't be an issue, but by limiting it to a discrete set of values we'd be excluding any IdP implementations that are out of spec in a way we didn't consider and include in the list of allowed values. And if an admin breaks something in their setup with an out-of-spec value then I think that's on them to debug in their particular setup.
But if you think a discrete set of valid values would better follow the conventions of the rest of the project or have other reasons for preferring it over free-form, I'd be open to changing it.
Or alternatively, it could log a warning on out-of-spec values.
From my experience most OIDC implementations have a public info endpoint that reveals all of the values it supports in that field.
For example Keycloak has:
https://<host>/auth/realms/default/.well-known/openid-configuration
Which returns:

I think long term Guacamole will want to consume that public info to at least validate its configuration.
@knacktim It looks like at least AWS Cognito and GCP Identity Platform have a provider metadata endpoint, although for Cognito it doesn't seem to be documented or I couldn't find documentation for it if it is.
I think using the provider metadata for validation would be a good idea, but since if we're already using that data for validation why not just set the rest of the options (authorization-endpoint, jwks-endpoint, issuer, etc) directly from it and make the equivalent guacamole.properties options optional overrides? If I'm understanding the spec correctly, that seems like it's kinda the point of the provider metadata endpoint to begin with.
I think using the provider metadata for validation would be a good idea, but since if we're already using that data for validation why not just set the rest of the options (authorization-endpoint, jwks-endpoint, issuer, etc) directly from it and make the equivalent guacamole.properties options optional overrides? If I'm understanding the spec correctly, that seems like it's kinda the point of the provider metadata endpoint to begin with.
Totally agree with this. That is actually how the oidc-client-js library here implements things. You give it the 'Authority' property and it uses that + the OIDC standard endpoints to configure itself using the metadata endpoint. Then provides an override in the local configuration.js file.
@lapchynski: Sounds like maybe there's an opportunity to go another route here and pull most of the values from the OID provider. Any progress on that?
Also, there are some merge conflicts, now.