-
Notifications
You must be signed in to change notification settings - Fork 548
Refactor FxA authentication configuration and fake auth handling #23077
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
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
3acd4f3
Refactor FxA authentication configuration and fake auth handling
KevinMind e712525
Updates from code review
KevinMind d06bccd
Fix tests
KevinMind edd7d62
Remove redundant config argument from login_redirect_url function
KevinMind 7e13d38
Remove invalid default values for fxa client id in test
KevinMind File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
# Authentication | ||
|
||
## Firefox Accounts | ||
|
||
Firefox Accounts (FXA) is the authentication system used by addons-server. | ||
In local development and in testing, we default to fake credentials which redirect | ||
to a local fake auth page. If you define real credentials, you will be redirected | ||
to fxa using the specified client id and secret. | ||
|
||
### Local development | ||
|
||
In local development, we default to fake credentials which redirect to a local | ||
fake auth page. Fake credentials of '' are defined by default on the environment and read | ||
into the FXA_CONFIG settings. | ||
|
||
### Production environments | ||
|
||
In production environments, we defined real FXA_CLIENT_ID and FXA_CLIENT_SECRET values | ||
to be used on the corrresponding FXA servers. | ||
|
||
### use_fake_fxa | ||
|
||
A utility method is used to determine if we should use the fake or real fxa redirect. | ||
This function only returns true if the environment is local/test and if the fake fxa client | ||
id and secret are defined. This forces us to use real auth redirection in production environments. | ||
|
||
### Getting real credentials for local development | ||
|
||
You must contact the FxA team to get your own credentials for FxA stage. | ||
|
||
To use these credentials, you can pass them to make up: | ||
|
||
```bash | ||
make up FXA_CLIENT_ID=<your-client-id> FXA_CLIENT_SECRET=<your-client-secret> | ||
``` | ||
|
||
or set them in your environment (on the host machine, not inside the container): | ||
|
||
```bash | ||
export FXA_CLIENT_ID=<your-client-id> | ||
export FXA_CLIENT_SECRET=<your-client-secret> | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,4 +28,5 @@ docs | |
waffle | ||
remote_settings | ||
../../../README.rst | ||
authentication | ||
``` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
from base64 import urlsafe_b64decode, urlsafe_b64encode | ||
from urllib.parse import parse_qs, urlparse | ||
|
||
from django.conf import settings | ||
from django.test import RequestFactory | ||
from django.test.utils import override_settings | ||
from django.urls import reverse | ||
|
@@ -10,24 +11,14 @@ | |
from olympia.amo.tests import TestCase | ||
|
||
|
||
FXA_CONFIG = { | ||
'default': { | ||
'client_id': 'foo', | ||
'client_secret': 'bar', | ||
}, | ||
'other': {'client_id': 'foo_other', 'client_secret': 'bar_other'}, | ||
} | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
@override_settings(FXA_OAUTH_HOST='https://accounts.firefox.com/oauth') | ||
def test_fxa_login_url_without_requiring_two_factor_auth(): | ||
path = '/en-US/addons/abp/?source=ddg' | ||
request = RequestFactory().get(path) | ||
request.session = {'fxa_state': 'myfxastate'} | ||
|
||
raw_url = utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path=path, | ||
enforce_2fa=False, | ||
|
@@ -41,22 +32,21 @@ def test_fxa_login_url_without_requiring_two_factor_auth(): | |
query = parse_qs(url.query) | ||
next_path = urlsafe_b64encode(path.encode('utf-8')).rstrip(b'=') | ||
assert query == { | ||
'client_id': ['foo'], | ||
'client_id': ['amodefault'], | ||
'scope': ['profile openid'], | ||
'state': [f'myfxastate:{force_str(next_path)}'], | ||
'access_type': ['offline'], | ||
} | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
@override_settings(FXA_OAUTH_HOST='https://accounts.firefox.com/oauth') | ||
def test_fxa_login_url_requiring_two_factor_auth(): | ||
path = '/en-US/addons/abp/?source=ddg' | ||
request = RequestFactory().get(path) | ||
request.session = {'fxa_state': 'myfxastate'} | ||
|
||
raw_url = utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path=path, | ||
enforce_2fa=True, | ||
|
@@ -71,22 +61,21 @@ def test_fxa_login_url_requiring_two_factor_auth(): | |
next_path = urlsafe_b64encode(path.encode('utf-8')).rstrip(b'=') | ||
assert query == { | ||
'acr_values': ['AAL2'], | ||
'client_id': ['foo'], | ||
'client_id': ['amodefault'], | ||
'scope': ['profile openid'], | ||
'state': [f'myfxastate:{force_str(next_path)}'], | ||
'access_type': ['offline'], | ||
} | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
@override_settings(FXA_OAUTH_HOST='https://accounts.firefox.com/oauth') | ||
def test_fxa_login_url_requiring_two_factor_auth_passing_token(): | ||
path = '/en-US/addons/abp/?source=ddg' | ||
request = RequestFactory().get(path) | ||
request.session = {'fxa_state': 'myfxastate'} | ||
|
||
raw_url = utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path=path, | ||
enforce_2fa=True, | ||
|
@@ -102,7 +91,7 @@ def test_fxa_login_url_requiring_two_factor_auth_passing_token(): | |
next_path = urlsafe_b64encode(path.encode('utf-8')).rstrip(b'=') | ||
assert query == { | ||
'acr_values': ['AAL2'], | ||
'client_id': ['foo'], | ||
'client_id': ['amodefault'], | ||
'id_token_hint': ['YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXo='], | ||
'prompt': ['none'], | ||
'scope': ['profile openid'], | ||
|
@@ -111,15 +100,14 @@ def test_fxa_login_url_requiring_two_factor_auth_passing_token(): | |
} | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
@override_settings(FXA_OAUTH_HOST='https://accounts.firefox.com/oauth') | ||
def test_fxa_login_url_requiring_two_factor_auth_passing_request(): | ||
path = '/en-US/addons/abp/?source=ddg' | ||
request = RequestFactory().get(path) | ||
request.session = {'fxa_state': 'myfxastate'} | ||
|
||
raw_url = utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path=path, | ||
enforce_2fa=True, | ||
|
@@ -135,22 +123,21 @@ def test_fxa_login_url_requiring_two_factor_auth_passing_request(): | |
next_path = urlsafe_b64encode(path.encode('utf-8')).rstrip(b'=') | ||
assert query == { | ||
'acr_values': ['AAL2'], | ||
'client_id': ['foo'], | ||
'client_id': ['amodefault'], | ||
'scope': ['profile openid'], | ||
'state': [f'myfxastate:{force_str(next_path)}'], | ||
'access_type': ['offline'], | ||
} | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
@override_settings(FXA_OAUTH_HOST='https://accounts.firefox.com/oauth') | ||
def test_fxa_login_url_requiring_two_factor_auth_passing_login_hint(): | ||
path = '/en-US/addons/abp/?source=ddg' | ||
request = RequestFactory().get(path) | ||
request.session = {'fxa_state': 'myfxastate'} | ||
|
||
raw_url = utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path=path, | ||
enforce_2fa=True, | ||
|
@@ -167,7 +154,7 @@ def test_fxa_login_url_requiring_two_factor_auth_passing_login_hint(): | |
next_path = urlsafe_b64encode(path.encode('utf-8')).rstrip(b'=') | ||
assert query == { | ||
'acr_values': ['AAL2'], | ||
'client_id': ['foo'], | ||
'client_id': ['amodefault'], | ||
'login_hint': ['[email protected]'], | ||
'prompt': ['none'], | ||
'scope': ['profile openid'], | ||
|
@@ -181,7 +168,7 @@ def test_unicode_next_path(): | |
request = RequestFactory().get(path) | ||
request.session = {'fxa_state': 'fake-state'} | ||
url = utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path=utils.path_with_query(request), | ||
) | ||
|
@@ -190,68 +177,51 @@ def test_unicode_next_path(): | |
assert next_path.decode('utf-8') == path | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
def test_redirect_for_login(): | ||
request = RequestFactory().get('/somewhere') | ||
request.session = {'fxa_state': 'fake-state'} | ||
response = utils.redirect_for_login(request) | ||
assert response['location'] == utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path='/somewhere', | ||
) | ||
assert request.session['enforce_2fa'] is False | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
def test_redirect_for_login_with_next_path(): | ||
request = RequestFactory().get('/somewhere') | ||
request.session = {'fxa_state': 'fake-state'} | ||
response = utils.redirect_for_login(request, next_path='/over/the/rainbow') | ||
assert response['location'] == utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path='/over/the/rainbow', | ||
) | ||
assert request.session['enforce_2fa'] is False | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
def test_redirect_for_login_with_config(): | ||
request = RequestFactory().get('/somewhere') | ||
request.session = {'fxa_state': 'fake-state'} | ||
response = utils.redirect_for_login(request, config=FXA_CONFIG['other']) | ||
assert response['location'] == utils.fxa_login_url( | ||
config=FXA_CONFIG['other'], | ||
state=request.session['fxa_state'], | ||
next_path='/somewhere', | ||
) | ||
assert request.session['enforce_2fa'] is False | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
def test_redirect_for_login_with_2fa_enforced(): | ||
request = RequestFactory().get('/somewhere') | ||
request.session = {'fxa_state': 'fake-state'} | ||
response = utils.redirect_for_login_with_2fa_enforced(request) | ||
assert response['location'] == utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path='/somewhere', | ||
enforce_2fa=True, | ||
) | ||
assert request.session['enforce_2fa'] is True | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
def test_redirect_for_login_with_2fa_enforced_id_token_hint(): | ||
request = RequestFactory().get('/somewhere') | ||
request.session = {'fxa_state': 'fake-state'} | ||
response = utils.redirect_for_login_with_2fa_enforced( | ||
request, id_token_hint='some_token_hint' | ||
) | ||
assert response['location'] == utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path='/somewhere', | ||
enforce_2fa=True, | ||
|
@@ -260,56 +230,56 @@ def test_redirect_for_login_with_2fa_enforced_id_token_hint(): | |
assert request.session['enforce_2fa'] is True | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
def test_redirect_for_login_with_2fa_enforced_and_next_path(): | ||
request = RequestFactory().get('/somewhere') | ||
request.session = {'fxa_state': 'fake-state'} | ||
response = utils.redirect_for_login_with_2fa_enforced( | ||
request, next_path='/over/the/rainbow' | ||
) | ||
assert response['location'] == utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path='/over/the/rainbow', | ||
enforce_2fa=True, | ||
) | ||
assert request.session['enforce_2fa'] is True | ||
|
||
|
||
@override_settings(FXA_CONFIG=FXA_CONFIG) | ||
def test_redirect_for_login_with_2fa_enforced_and_config(): | ||
request = RequestFactory().get('/somewhere') | ||
request.session = {'fxa_state': 'fake-state'} | ||
response = utils.redirect_for_login_with_2fa_enforced( | ||
request, config=FXA_CONFIG['other'] | ||
request, | ||
config={'client_id': 'foo_other', 'client_secret': 'bar_other'}, | ||
) | ||
assert response['location'] == utils.fxa_login_url( | ||
config=FXA_CONFIG['other'], | ||
config={'client_id': 'foo_other', 'client_secret': 'bar_other'}, | ||
state=request.session['fxa_state'], | ||
next_path='/somewhere', | ||
enforce_2fa=True, | ||
) | ||
assert request.session['enforce_2fa'] is True | ||
|
||
|
||
@override_settings(DEV_MODE=True, USE_FAKE_FXA_AUTH=True) | ||
@override_settings(FXA_CONFIG={'default': {'client_id': ''}}) | ||
def test_fxa_login_url_when_faking_fxa_auth(): | ||
path = '/en-US/addons/abp/?source=ddg' | ||
request = RequestFactory().get(path) | ||
request.session = {'fxa_state': 'myfxastate'} | ||
raw_url = utils.fxa_login_url( | ||
config=FXA_CONFIG['default'], | ||
config=settings.FXA_CONFIG['default'], | ||
state=request.session['fxa_state'], | ||
next_path=path, | ||
) | ||
url = urlparse(raw_url) | ||
assert url.scheme == '' | ||
assert url.netloc == '' | ||
assert url.path == reverse('fake-fxa-authorization') | ||
query = parse_qs(url.query) | ||
# client_id has a blank value, so we should inspect blank values | ||
query = parse_qs(url.query, keep_blank_values=True) | ||
next_path = urlsafe_b64encode(path.encode('utf-8')).rstrip(b'=') | ||
assert query == { | ||
'client_id': ['foo'], | ||
'client_id': [''], | ||
'scope': ['profile openid'], | ||
'state': [f'myfxastate:{force_str(next_path)}'], | ||
'access_type': ['offline'], | ||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.