Closed cfryanr closed 4 months ago
The format check seems to have failed complaining about almost every copyright date in the repo. Please advise if I should update any copyright dates in this PR.
Nice! I recently found the same issue on our prod system. I think we can also delete the PKCE entry if it's not already being deleted
Small aside, this makes me think there is an issue with the tests since there is only one mock EXPECT added for DeleteOpenIDConnectSession
.
Thanks for accepting the PR.
I think we can also delete the PKCE entry if it's not already being deleted
I believe those are already correctly deleted here https://github.com/ory/fosite/blob/v0.46.0/handler/pkce/handler.go#L133.
Small aside, this makes me think there is an issue with the tests since there is only one mock EXPECT added for DeleteOpenIDConnectSession.
I added two EXPECTs in the PR. One for each pre-existing happy path unit test.
You're right - those are indeed already covered!
I added two EXPECTs in the PR. One for each pre-existing happy path unit test.
Oh your right my bad. I swore when I looked at the "should pass" test it didn't have one. Was really confusing why it was passing without it. But it's there!
Fixes https://github.com/ory/fosite/issues/790. Please see description and comments in https://github.com/ory/fosite/issues/790 for more information about why this is a necessary and safe change.
Related Issue or Design Document
Fixes #790.
Checklist
Further comments