Skip to content

refactor: make captcha a real service #15137

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

Merged

Conversation

miketheman
Copy link
Member

Best reviewed commit by commit, since there's some that are not too interesting (example: mass rename).

Paves the way for a future captcha service via interface.

In preparation for a conversion to a proper service, move the recaptcha
code to a package named `captcha`.
Next refactoring will include mass rename of variables.

Signed-off-by: Mike Fiedler <[email protected]>
In anticipation of adding another recaptcha service, wrap the existing
implementation with an Interface that conforms to the existing behavior.

Signed-off-by: Mike Fiedler <[email protected]>
As a way to make the service interface more generic, rename applicable
variables, parameters, strings.

Signed-off-by: Mike Fiedler <[email protected]>
By making the specific bits for a given service part of the service
instance, we avoid leaking the implementation details to the code
referencing it, allowing us to swap the implementation.

Signed-off-by: Mike Fiedler <[email protected]>
We don't have any other backends (yet), but now we can configure one if
we wanted to.

Signed-off-by: Mike Fiedler <[email protected]>
Signed-off-by: Mike Fiedler <[email protected]>
@miketheman miketheman requested a review from a team as a code owner January 3, 2024 19:50
@@ -70,3 +70,6 @@ OIDC_AUDIENCE=pypi
# Default to the reCAPTCHA testing keys from https://developers.google.com/recaptcha/docs/faq
RECAPTCHA_SITE_KEY=6LeIxAcTAAAAAJcZVRqyHh71UMIEGNQ_MXjiZKhI
RECAPTCHA_SECRET_KEY=6LeIxAcTAAAAAGG-vFI1TnRWxMZNFuojJ4WifJWe

# Example of Captcha backend configuration
# CAPTCHA_BACKEND=warehouse.captcha.recaptcha.Service
Copy link
Member Author

Choose a reason for hiding this comment

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

A value isn't needed for deployment, since the default is currently recaptcha.Service

@ewdurbin
Copy link
Member

ewdurbin commented Jan 3, 2024

Looks great, my only concern was CSP but it appears we are ahead of that!

@miketheman miketheman added the security Security-related issues and pull requests label Jan 3, 2024
@miketheman miketheman merged commit 7a0409a into pypi:main Jan 3, 2024
@miketheman miketheman deleted the miketheman/refactor-recaptcha-to-service branch February 6, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Security-related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants