Skip to content
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

Dynamic reconfig of discovery endpoint not updating endpoints in all cases #16994

Closed
c00crane opened this issue Apr 30, 2021 · 0 comments · Fixed by #17735
Closed

Dynamic reconfig of discovery endpoint not updating endpoints in all cases #16994

c00crane opened this issue Apr 30, 2021 · 0 comments · Fixed by #17735
Assignees
Labels
bug This bug is not present in a released version of Open Liberty release bug This bug is present in a released version of Open Liberty release:21008 team:Security SSO

Comments

@c00crane
Copy link
Member

c00crane commented Apr 30, 2021

If Social client is reconfigured in the following steps, it will not set the authorization, token, ... endpoints.

  1. configure server with a valid discoveryEndpointUrl attribute and none of the discoverable config attributes set
  2. start the server
  3. Update the server config - replace the discoveryEndpointUrl value with ""
  4. Note the message in the server's messages.log: CWWKS5500E: A required configuration parameter authorizationEndpoint is missing or has an invalid value null
  5. Update the server config again - replace the discoveryEndpointUrl value with the original good discovery endpoint.
  6. Note the message in the server's messages.log: CWWKS5500E: A required configuration parameter authorizationEndpoint is missing or has an invalid value null
  7. Attempt to access a social login protected app, you'll receive a 403 in the response and find this message in the server log: CWWKS5500E: A required configuration parameter authorizationEndpoint is missing or has an invalid value null

It looks like the initial config sets up the discoveryDocumentHash which is used to populate the config.
The second config finds that the discovery endpoint is not set, so, it skips using the discoveryDocumentHash and tries to init the config from the values found in server.xml.
The hash instance is still set to the values from the first config.
The third config comes along and runs discovery. If then compares the current discoveryDocumentHash to the still existing discoveryDocumentHash - they're the same, so, it skips setting the values into the config object.

We need to do one of two things:

  1. when we find that the discovery endpoint is not set, we should discard the discoveryDocumentHash (I'm not sure if we can get to the hash at that point to do this)
  2. stop checking if the discoveryDocumentHash values are the same and if set, always set the config values from the hash. (I don't know why that check was put in place - could we be breaking something else if we make this change)
@c00crane c00crane added bug This bug is not present in a released version of Open Liberty team:Security SSO release bug This bug is present in a released version of Open Liberty labels Apr 30, 2021
@ayoho ayoho self-assigned this Apr 30, 2021
@ayoho ayoho added this to Current Iteration in Security SSO May 10, 2021
ayoho added a commit to ayoho/open-liberty that referenced this issue Jun 30, 2021
…overy data

Resolves OpenLiberty#16994

This now sets the `discoveryDocumentHash` in `OidcLoginConfigImpl` back to null if the `discoveryEndpointUrl` is dynamically removed from the configuration or set to an empty string after discovery has already been performed. This should ensure that the various endpoint information in the discovery data gets re-populated if the `discoveryEndpointUrl` is dynamically re-added.
Security SSO automation moved this from Current Iteration to Done Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This bug is not present in a released version of Open Liberty release bug This bug is present in a released version of Open Liberty release:21008 team:Security SSO
Projects
Status: Done
Security SSO
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants