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

Implement APIKey as django form + recaptcha field #22759

Merged
merged 2 commits into from
Oct 17, 2024
Merged

Conversation

KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Oct 14, 2024

Fixes: mozilla/addons#15083

Description

This PR refactors the APIKeyPage view to utilize a django form, both to support using the recaptcha field as part of the flow, and to make the form logic more consolidated and easier to test/grok.

Context

Previously, the logic for managing the API key page was kind of murky and spread/duplicated between the view and the template layer.

Now, there is a Django form APIKeyForm that is managing the state machine of which actions a developer has available, what is the current state of the relevant entities (confirmation + credentials) and which fields + validations should be in scope.

This is a dynamic form that can model the full state of the API key lifecycle from requesting a confirmation, confirming + generating, revoking, and regenerating credentials.

The underlying logic is largely the same as before, but to support the re-captcha a few key changes have been made.

  1. the re-captcha field is now included in the "confirm" stage, when a user does not have a confirmation or credentials.

  2. if a user has credentials but no confirmation, they are shown the credentials view with ONLY the revoke action. Previously we handled creating the confirmation to allow them to regenerate but now, we don't know if they have re-captcha complete and it is simpler to remove that action and force the developer to reenter the flow from the beginning.

  3. We use GET parameters to prefill the confirmation code, but the API is exposed as a post endpoint so it is (unlikely but) possible to submit both a POST and GET token. this is explicitly handled to prefer the POST over the get and covered with a test.

Testing

I'm going to document the entire flow in the issue since there are really a lot of edge cases with this form.

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@KevinMind KevinMind force-pushed the recaptcha-api-keys branch 4 times, most recently from c14a88c to 830670b Compare October 16, 2024 09:09
@KevinMind KevinMind changed the title TMP: attempt at api key validation Implement APIKey as django form + recaptcha field Oct 16, 2024
@KevinMind KevinMind marked this pull request as ready for review October 17, 2024 10:38
@KevinMind KevinMind requested review from a team and diox and removed request for a team October 17, 2024 10:38
src/olympia/devhub/forms.py Outdated Show resolved Hide resolved
else:
if waffle.switch_is_active('developer-submit-addon-captcha'):
self.fields['recaptcha'] = ReCaptchaField(
label='', help_text=_("You don't have any API credentials.")
Copy link
Member

Choose a reason for hiding this comment

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

By making it a help_text on the recaptcha field that text is no longer displayed if the captcha isn't enabled... I guess this is fine because it's not a very useful text anyway ?

@KevinMind KevinMind merged commit f8e7fa2 into master Oct 17, 2024
31 checks passed
@KevinMind KevinMind deleted the recaptcha-api-keys branch October 17, 2024 14:44
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.

(P0) When a developer generates new API keys, ask them to solve a captcha.
2 participants