Skip to content

fix: close session in case of error to avoid blocked session#11089

Merged
shreemaan-abhishek merged 5 commits into
apache:masterfrom
markusmueller:fix-openid-uauth-deny-locking-storage
Apr 22, 2024
Merged

fix: close session in case of error to avoid blocked session#11089
shreemaan-abhishek merged 5 commits into
apache:masterfrom
markusmueller:fix-openid-uauth-deny-locking-storage

Conversation

@markusmueller

@markusmueller markusmueller commented Mar 25, 2024

Copy link
Copy Markdown
Contributor

…session on locking session storage (example unauth_action=deny)

Description

This PR is fixing a bug where sessions are blocked until ttl is expired in case of an authentication error. For example unauthenticated session and unauth_action='deny' and locking session storage is configured.

Basically the same problem as in #10788 but in case of error.

Fixes # (issue)

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

…session on locking session storage (example unauth_action=deny)
@markusmueller
markusmueller marked this pull request as ready for review March 25, 2024 16:03
@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

the solution looks good in first observation, let's wait for the CI to run then I will review this again.

@markusmueller

Copy link
Copy Markdown
Contributor Author

@shreemaan-abhishek Is it possible that ci is flaky? The errors don't seem related to this PR to me.

Comment thread t/plugin/openid-connect5.t
@shreemaan-abhishek shreemaan-abhishek changed the title fix(openid-connect): Close session in case of error to avoid blocked … fix: close session in case of error to avoid blocked session Mar 30, 2024
@shreemaan-abhishek

Copy link
Copy Markdown
Contributor

@starsz does this feature look good to you?

Comment thread t/plugin/openid-connect5.t
Comment thread t/plugin/openid-connect5.t
Comment thread t/plugin/openid-connect5.t
Comment thread t/plugin/openid-connect5.t Outdated
}
})
end
ngx.thread.spawn(firstRequest)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How can you ensure that the first request is arrived before the second one?
I think maybe you can give a return value of the firstRequest function , and use ngx.thread.wait to get the return value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@starsz Thank you for the suggestion, I have implemented a wait for the completion of the first request.

@shreemaan-abhishek
shreemaan-abhishek merged commit 07b3cc3 into apache:master Apr 22, 2024
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