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

Refactor FxA authentication configuration and fake auth handling #23077

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Feb 13, 2025

Important

Contains #23073 that should land first

Fixes: mozilla/addons#15331

Description

  • Update FxA authentication to use a more flexible configuration approach
  • Replace USE_FAKE_FXA_AUTH and DEV_MODE with ENV and client_id checks
  • Modify utils, views, and tests to support new authentication configuration
  • Add FXA_CONFIG to docker-compose and settings files
  • Simplify fake authentication logic across multiple files

Context

Relying on the arbitrary USE_FAKE_FXA and the DEV_MODE settinng means that by default any production image, even running locally, must use a real FXA_CONFIG.

This is super annoying if you want to run a production image locally but don't want/need real auth.

On the other hand, you should NOT be allowed to use fake auth outside of a local context.

Solution, do that instead. Now we check the ENV setting which reliably tells us the environment we are in, and the existence of valid FXA_CONFIG client id. If we are on a local environmnet with a fake config client ID, then we use fake auth, otherwiswe we use real auth redirects.

Testing

Fake auth

make up DOCKER_TARGET=production

Go through standard auth on http://olympia.test/developers and expect fake auth page.

Real auth with credentials

make up FXA_CLIENT_ID=<real_id> FXA_CLIENT_SECRET=<real_secret>

Real auth without credentials

override the ENV=local to something else in the docker-compose.yml

make up

Expect to be redirected to an erroring auth page because olympia expects auth credentials but they are invalid.

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 kevinmind/addons/153310-fake-fxa-prod branch 7 times, most recently from dd7bf87 to 0c71611 Compare February 14, 2025 12:45
@KevinMind KevinMind requested review from a team and eviljeff and removed request for a team February 14, 2025 13:42
@eviljeff eviljeff removed their request for review February 14, 2025 14:02
@KevinMind KevinMind requested review from a team and eviljeff and removed request for a team February 14, 2025 15:38
Copy link
Member

@eviljeff eviljeff left a comment

Choose a reason for hiding this comment

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

When testing the first scenario I hit the default value of client_id being '' (the possibility I mentioned in a code comment - I guess because once FXA_CLIENT_ID is defined in the docker variables, etc, it's now defined in settings.ENV, but as an empty string, so the default of '.' in settings_base.py is unused.)

Also, I wasn't sure how to interpret "Expect to be redirected to an erroring auth page because olympia expects auth credentials but they are invalid." but you only get an error after you're logged into FxA with your username, password, 2fa, and it tries to redirect you back to olympia.test

- Update FxA authentication to use a more flexible configuration approach
- Replace USE_FAKE_FXA_AUTH and DEV_MODE with ENV and client_id checks
- Modify utils, views, and tests to support new authentication configuration
- Add FXA_CONFIG to docker-compose and settings files
- Simplify fake authentication logic across multiple files
@KevinMind KevinMind force-pushed the kevinmind/addons/153310-fake-fxa-prod branch from 0c71611 to 3acd4f3 Compare February 19, 2025 08:53
@KevinMind KevinMind requested a review from eviljeff March 3, 2025 17:51
@KevinMind
Copy link
Contributor Author

@eviljeff I think I resolved the issues, lmk if it passes verification now.

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.

[Task]: Enable fake FXA in local production images
2 participants