GUACAMOLE-1094: Add overridable response_type to auth-openid extension#517
GUACAMOLE-1094: Add overridable response_type to auth-openid extension#517lapchynski wants to merge 1 commit intoapache:mainfrom
Conversation
|
@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: 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. |
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. |
|
Hi @lapchynski @necouchman can this code be merged, I'm trying to integrate Guacamole with Cognito and can use this. |
|
@ShaizanKhan This needs some work by @lapchynski to fix this up before it's ready to merge. |
|
Please merge this PR as soon as possible. Its been 6 years and this is imo a trivial change that would solve a serious problem for a lot of users. Does anyone have a build of |
|
Hi, I will side with @heathprovost on this one. Based on the docs at https://docs.aws.amazon.com/cognito/latest/developerguide/authorization-endpoint.html, Cognito only do So, I can't get my Guacamole authorizing with Cognito. Any way to make this work? |
|
Hi, I did a custom/personal build for 1.6.0. That one made my Cognito <-> Guacamole integration to work (on dev/dirty mode) Looks like OIDC Implicit mode is not recommended anymore? |
|
@dnoliver @heathprovost @ShaizanKhan I appreciate that you'd like to get this merged; unfortunately it isn't ready to merge - there are outstanding issues that needs to be fixed, including (now) conflicts in the merge branch. There doesn't seem to have been any activity from the original requestor in several years. I believe that the correct way to address this is:
|
|
I added OIDC code flow in #1198. I believe AWS Cognito supports code flow so Cognito should work directly with my PR. There is no “token” response_type in the OIDC standard, though there is a response type “id_token token” that allows both an id_token and access token to be returned in the response from the authorization endpoint in implicit flow. As implicit flow is deprecated, AWS Cognito doesn’t respect the standard and you could get around the issue using code flow, it seems to me that if my PR is accepted the response_type should not be made configurable |
The “well-known” endpoint could be used to automatically configure the following parameters
It could help with what the response_type to use with implicit flow is, as the well-known endpoint returns a list response_types_supported. So if “token” was a supported type but “id_token” wasn’t then we’d know the response_type to use with implicit flow Yes it would be nice to use the well-known endpoint to automatically configure things, though this poses several questions
If the well-known endpoint was used to configure the implicit flow response_type, you’d still have treat the case of a missing well-known endpoint. Frankly better off just forcing AWS Cognito users to use code flow rather than implicit flow. I’d be willing to propose a PR for use of the well-known endpoint, but only after #1198 is merged. |
|
Here is an example AWS Cognito well-known endpoint: https://cognito-idp.ap-southeast-2.amazonaws.com/ap-southeast-2_M6vgxvpnC/.well-known/openid-configuration As can be seen the returned response_types_supported is [“code”,”token”]. So even now they don’t support “id_token” as a valid response_type. However, this confirms that the well-known endpoint could be used to automatically configure the implicit flow response_type |

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.