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

[v11] Add rate limiter to saml/oidc routes #19950

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

sfreiberg
Copy link
Contributor

@sfreiberg sfreiberg commented Jan 6, 2023

This is a backport of adding the rate limiter to routes that are currently in the e submodule but in v11 were in the main repo. It is an extension of #19869.

@sfreiberg sfreiberg marked this pull request as ready for review January 6, 2023 17:58
@sfreiberg sfreiberg enabled auto-merge (squash) January 6, 2023 17:58
@sfreiberg sfreiberg force-pushed the sfreiberg/add-ent-rate-limits-v11 branch from 609599c to 04a89dd Compare January 6, 2023 17:58
@avatus avatus changed the title Add rate limiter to saml/oidc routes [v11] Add rate limiter to saml/oidc routes Jan 6, 2023
@sfreiberg sfreiberg force-pushed the sfreiberg/add-ent-rate-limits-v11 branch from 04a89dd to 9212282 Compare January 6, 2023 18:55
@@ -598,13 +598,13 @@ func (h *Handler) bindDefaultEndpoints() {
// OIDC related callback handlers
h.GET("/webapi/oidc/login/web", h.WithRedirect(h.oidcLoginWeb))
h.GET("/webapi/oidc/callback", h.WithMetaRedirect(h.oidcCallback))
h.POST("/webapi/oidc/login/console", httplib.MakeHandler(h.oidcLoginConsole))
h.POST("/webapi/oidc/login/console", h.WithLimiter(h.oidcLoginConsole))
Copy link
Contributor

Choose a reason for hiding this comment

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

Not here, since it's a backport of sorts, but should we add a test to verify that certain routes are rate-limited, so we can prevent regressions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think I have enough context to give a meaningful answer. My optimistic side hopes that would get caught in a code review but I know these kinds of invisible things are easy to miss.

@sfreiberg sfreiberg force-pushed the sfreiberg/add-ent-rate-limits-v11 branch from 9212282 to 134235d Compare January 6, 2023 19:32
@sfreiberg sfreiberg merged commit 0c332bb into branch/v11 Jan 6, 2023
@zmb3 zmb3 deleted the sfreiberg/add-ent-rate-limits-v11 branch January 6, 2023 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants