Skip to content

Remove unused OpenID Connect Logout prevent_logout_redirect parameter#7001

Merged
mitchellhenke merged 1 commit intomainfrom
mitchellhenke/drop-prevent-logout-redirect-param
Sep 22, 2022
Merged

Remove unused OpenID Connect Logout prevent_logout_redirect parameter#7001
mitchellhenke merged 1 commit intomainfrom
mitchellhenke/drop-prevent-logout-redirect-param

Conversation

@mitchellhenke
Copy link
Contributor

@mitchellhenke mitchellhenke commented Sep 21, 2022

It was introduced by #3479 / LG-2441, but it's not clear what the intention was since the default action is to try rendering index.html which doesn't exist so using prevent_logout_redirect will 404 currently.

The parameter does not appear in:

  • Production logs
  • The JIRA ticket referenced in the Pull Request that introduced it
  • Slack chat logs
  • Our OIDC Developer documentation

So it seems safe to drop?

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/drop-prevent-logout-redirect-param branch from 3a324e5 to af5cf94 Compare September 21, 2022 17:54
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@orenyk orenyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no idea what this was but if the spec pass I'm cool with it

@mitchellhenke mitchellhenke force-pushed the mitchellhenke/drop-prevent-logout-redirect-param branch from af5cf94 to 3b4ea03 Compare September 21, 2022 19:06
…ect Logout prevent_logout_redirect parameter
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/drop-prevent-logout-redirect-param branch from 3b4ea03 to a2bdc12 Compare September 21, 2022 19:08
@mitchellhenke
Copy link
Contributor Author

mitchellhenke commented Sep 21, 2022

After trying to remove it, it seems like it is only used intended for use within feature specs for testing that the CSP headers are correct, so I have written a controller spec to cover that behavior and dropped the parameter and the CSP checks from those feature tests.

@mitchellhenke mitchellhenke merged commit c52b51b into main Sep 22, 2022
@mitchellhenke mitchellhenke deleted the mitchellhenke/drop-prevent-logout-redirect-param branch September 22, 2022 13:57
@solipet solipet mentioned this pull request Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants