Merge branch 'master' into tob-travis-friendly-tokens

Author: woodruffw

tests/unit/accounts/test_forms.py

@@ -615,15 +615,13 @@ def test_creation(self):
         user_service = pretend.stub()
         challenge = pretend.stub()
         origin = pretend.stub()
-        icon_url = pretend.stub()
         rp_id = pretend.stub()
 
         form = forms.WebAuthnAuthenticationForm(
             user_id=user_id,
             user_service=user_service,
             challenge=challenge,
             origin=origin,
-            icon_url=icon_url,
             rp_id=rp_id,
         )
 
@@ -636,7 +634,6 @@ def test_credential_bad_payload(self):
             user_service=pretend.stub(),
             challenge=pretend.stub(),
             origin=pretend.stub(),
-            icon_url=pretend.stub(),
             rp_id=pretend.stub(),
         )
         assert not form.validate()
@@ -653,7 +650,6 @@ def test_credential_invalid(self):
             ),
             challenge=pretend.stub(),
             origin=pretend.stub(),
-            icon_url=pretend.stub(),
             rp_id=pretend.stub(),
         )
         assert not form.validate()
@@ -670,7 +666,6 @@ def test_credential_valid(self):
             ),
             challenge=pretend.stub(),
             origin=pretend.stub(),
-            icon_url=pretend.stub(),
             rp_id=pretend.stub(),
         )
         assert form.validate()

tests/unit/accounts/test_services.py

@@ -426,22 +426,15 @@ def test_check_totp_value_user_rate_limited(self, user_service, metrics):
         ]
 
     @pytest.mark.parametrize(
-        ("challenge", "rp_name", "rp_id", "icon_url"),
-        (
-            ["fake_challenge", "fake_rp_name", "fake_rp_id", "fake_icon_url"],
-            [None, None, None, None],
-        ),
+        ("challenge", "rp_name", "rp_id"),
+        (["fake_challenge", "fake_rp_name", "fake_rp_id"], [None, None, None]),
     )
     def test_get_webauthn_credential_options(
-        self, user_service, challenge, rp_name, rp_id, icon_url
+        self, user_service, challenge, rp_name, rp_id
     ):
         user = UserFactory.create()
         options = user_service.get_webauthn_credential_options(
-            user.id,
-            challenge=challenge,
-            rp_name=rp_name,
-            rp_id=rp_id,
-            icon_url=icon_url,
+            user.id, challenge=challenge, rp_name=rp_name, rp_id=rp_id
         )
 
         assert options["user"]["id"] == str(user.id)
@@ -450,11 +443,7 @@ def test_get_webauthn_credential_options(
         assert options["challenge"] == challenge
         assert options["rp"]["name"] == rp_name
         assert options["rp"]["id"] == rp_id
-
-        if icon_url:
-            assert options["user"]["icon"] == icon_url
-        else:
-            assert "icon" not in options["user"]
+        assert "icon" not in options["user"]
 
     def test_get_webauthn_assertion_options(self, user_service):
         user = UserFactory.create()
@@ -467,10 +456,7 @@ def test_get_webauthn_assertion_options(self, user_service):
         )
 
         options = user_service.get_webauthn_assertion_options(
-            user.id,
-            challenge="fake_challenge",
-            icon_url="fake_icon_url",
-            rp_id="fake_rp_id",
+            user.id, challenge="fake_challenge", rp_id="fake_rp_id"
         )
 
         assert options["challenge"] == "fake_challenge"
@@ -550,7 +536,6 @@ def test_verify_webauthn_assertion(self, user_service, monkeypatch):
             pretend.stub(),
             challenge=pretend.stub(),
             origin=pretend.stub(),
-            icon_url=pretend.stub(),
             rp_id=pretend.stub(),
         )
         assert updated_sign_count == 2

tests/unit/manage/test_views.py

@@ -1138,12 +1138,7 @@ def test_get_webauthn_options(self):
                 get_webauthn_challenge=pretend.call_recorder(lambda: "fake_challenge")
             ),
             find_service=lambda *a, **kw: user_service,
-            registry=pretend.stub(
-                settings={
-                    "site.name": "fake_site_name",
-                    "warehouse.domain": "fake_domain",
-                }
-            ),
+            registry=pretend.stub(settings={"site.name": "fake_site_name"}),
             domain="fake_domain",
         )
 
@@ -1157,7 +1152,6 @@ def test_get_webauthn_options(self):
                 challenge="fake_challenge",
                 rp_name=request.registry.settings["site.name"],
                 rp_id=request.domain,
-                icon_url=request.registry.settings["warehouse.domain"],
             )
         ]
 

tests/unit/utils/test_webauthn.py

@@ -80,12 +80,11 @@ def test_verify_assertion_response(monkeypatch):
         challenge="not_a_real_challenge",
         user=not_a_real_user,
         origin="fake_origin",
-        icon_url="fake_icon_url",
         rp_id="fake_rp_id",
     )
 
     assert get_webauthn_users.calls == [
-        pretend.call(not_a_real_user, icon_url="fake_icon_url", rp_id="fake_rp_id")
+        pretend.call(not_a_real_user, rp_id="fake_rp_id")
     ]
     assert assertion_cls.calls == [
         pretend.call(
@@ -117,6 +116,5 @@ def test_verify_assertion_response_failure(monkeypatch):
             challenge="not_a_real_challenge",
             user=pretend.stub(),
             origin="fake_origin",
-            icon_url="fake_icon_url",
             rp_id="fake_rp_id",
         )

warehouse/accounts/forms.py

@@ -268,11 +268,10 @@ def validate_totp_value(self, field):
 class WebAuthnAuthenticationForm(WebAuthnCredentialMixin, _TwoFactorAuthenticationForm):
     __params__ = ["credential"]
 
-    def __init__(self, *args, challenge, origin, icon_url, rp_id, **kwargs):
+    def __init__(self, *args, challenge, origin, rp_id, **kwargs):
         super().__init__(*args, **kwargs)
         self.challenge = challenge
         self.origin = origin
-        self.icon_url = icon_url
         self.rp_id = rp_id
 
     def validate_credential(self, field):
@@ -289,7 +288,6 @@ def validate_credential(self, field):
                 assertion_dict,
                 challenge=self.challenge,
                 origin=self.origin,
-                icon_url=self.icon_url,
                 rp_id=self.rp_id,
             )
 

warehouse/accounts/interfaces.py

@@ -137,15 +137,13 @@ def add_webauthn(user_id, **kwargs):
         Returns None if the user already has this credential.
         """
 
-    def get_webauthn_credential_options(
-        user_id, *, challenge, rp_name, rp_id, icon_url
-    ):
+    def get_webauthn_credential_options(user_id, *, challenge, rp_name, rp_id):
         """
         Returns a dictionary of credential options suitable for beginning the WebAuthn
         provisioning process for the given user.
         """
 
-    def get_webauthn_assertion_options(user_id, *, challenge, icon_url, rp_id):
+    def get_webauthn_assertion_options(user_id, *, challenge, rp_id):
         """
         Returns a dictionary of assertion options suitable for beginning the WebAuthn
         authentication process for the given user.
@@ -160,9 +158,7 @@ def verify_webauthn_credential(credential, *, challenge, rp_id, origin):
         webauthn.RegistrationRejectedException on failure.
         """
 
-    def verify_webauthn_assertion(
-        user_id, assertion, *, challenge, origin, icon_url, rp_id
-    ):
+    def verify_webauthn_assertion(user_id, assertion, *, challenge, origin, rp_id):
         """
         Checks whether the given assertion was produced by the given user's WebAuthn
         device.

warehouse/accounts/services.py

@@ -327,29 +327,25 @@ def check_totp_value(self, user_id, totp_value, *, tags=None):
 
         return valid
 
-    def get_webauthn_credential_options(
-        self, user_id, *, challenge, rp_name, rp_id, icon_url
-    ):
+    def get_webauthn_credential_options(self, user_id, *, challenge, rp_name, rp_id):
         """
         Returns a dictionary of credential options suitable for beginning the WebAuthn
         provisioning process for the given user.
         """
         user = self.get_user(user_id)
 
         return webauthn.get_credential_options(
-            user, challenge=challenge, rp_name=rp_name, rp_id=rp_id, icon_url=icon_url
+            user, challenge=challenge, rp_name=rp_name, rp_id=rp_id
         )
 
-    def get_webauthn_assertion_options(self, user_id, *, challenge, icon_url, rp_id):
+    def get_webauthn_assertion_options(self, user_id, *, challenge, rp_id):
         """
         Returns a dictionary of assertion options suitable for beginning the WebAuthn
         authentication process for the given user.
         """
         user = self.get_user(user_id)
 
-        return webauthn.get_assertion_options(
-            user, challenge=challenge, icon_url=icon_url, rp_id=rp_id
-        )
+        return webauthn.get_assertion_options(user, challenge=challenge, rp_id=rp_id)
 
     def verify_webauthn_credential(self, credential, *, challenge, rp_id, origin):
         """
@@ -375,7 +371,7 @@ def verify_webauthn_credential(self, credential, *, challenge, rp_id, origin):
         return validated_credential
 
     def verify_webauthn_assertion(
-        self, user_id, assertion, *, challenge, origin, icon_url, rp_id
+        self, user_id, assertion, *, challenge, origin, rp_id
     ):
         """
         Checks whether the given assertion was produced by the given user's WebAuthn
@@ -387,12 +383,7 @@ def verify_webauthn_assertion(
         user = self.get_user(user_id)
 
         return webauthn.verify_assertion_response(
-            assertion,
-            challenge=challenge,
-            user=user,
-            origin=origin,
-            icon_url=icon_url,
-            rp_id=rp_id,
+            assertion, challenge=challenge, user=user, origin=origin, rp_id=rp_id
         )
 
     def add_webauthn(self, user_id, **kwargs):

warehouse/accounts/views.py

@@ -252,10 +252,7 @@ def webauthn_authentication_options(request):
     userid = two_factor_data.get("userid")
     user_service = request.find_service(IUserService, context=None)
     return user_service.get_webauthn_assertion_options(
-        userid,
-        challenge=request.session.get_webauthn_challenge(),
-        icon_url=request.registry.settings.get("warehouse.domain", request.domain),
-        rp_id=request.domain,
+        userid, challenge=request.session.get_webauthn_challenge(), rp_id=request.domain
     )
 
 
@@ -288,7 +285,6 @@ def webauthn_authentication_validate(request):
         user_service=user_service,
         challenge=request.session.get_webauthn_challenge(),
         origin=request.host_url,
-        icon_url=request.registry.settings.get("warehouse.domain", request.domain),
         rp_id=request.domain,
     )
 

warehouse/manage/views.py

@@ -474,9 +474,6 @@ def webauthn_provision_options(self):
             challenge=self.request.session.get_webauthn_challenge(),
             rp_name=self.request.registry.settings["site.name"],
             rp_id=self.request.domain,
-            icon_url=self.request.registry.settings.get(
-                "warehouse.domain", self.request.domain
-            ),
         )
 
     @view_config(

warehouse/utils/webauthn.py

@@ -32,7 +32,7 @@ class RegistrationRejectedException(Exception):
 WebAuthnCredential = pywebauthn.WebAuthnCredential
 
 
-def _get_webauthn_users(user, *, icon_url, rp_id):
+def _get_webauthn_users(user, *, rp_id):
     """
     Returns a webauthn.WebAuthnUser instance corresponding
     to the given user model, with properties suitable for
@@ -43,7 +43,7 @@ def _get_webauthn_users(user, *, icon_url, rp_id):
             str(user.id),
             user.username,
             user.name,
-            icon_url,
+            None,
             credential.credential_id,
             credential.public_key,
             credential.sign_count,
@@ -74,25 +74,25 @@ def generate_webauthn_challenge():
     return _webauthn_b64encode(os.urandom(32)).decode()
 
 
-def get_credential_options(user, *, challenge, rp_name, rp_id, icon_url):
+def get_credential_options(user, *, challenge, rp_name, rp_id):
     """
     Returns a dictionary of options for credential creation
     on the client side.
     """
     options = pywebauthn.WebAuthnMakeCredentialOptions(
-        challenge, rp_name, rp_id, str(user.id), user.username, user.name, icon_url
+        challenge, rp_name, rp_id, str(user.id), user.username, user.name, None
     )
 
     return options.registration_dict
 
 
-def get_assertion_options(user, *, challenge, icon_url, rp_id):
+def get_assertion_options(user, *, challenge, rp_id):
     """
     Returns a dictionary of options for assertion retrieval
     on the client side.
     """
     options = pywebauthn.WebAuthnAssertionOptions(
-        _get_webauthn_users(user, icon_url=icon_url, rp_id=rp_id), challenge
+        _get_webauthn_users(user, rp_id=rp_id), challenge
     )
 
     return options.assertion_dict
@@ -120,15 +120,15 @@ def verify_registration_response(response, challenge, *, rp_id, origin):
         raise RegistrationRejectedException(str(e))
 
 
-def verify_assertion_response(assertion, *, challenge, user, origin, icon_url, rp_id):
+def verify_assertion_response(assertion, *, challenge, user, origin, rp_id):
     """
     Validates the challenge and assertion information
     sent from the client during authentication.
 
     Returns an updated signage count on success.
     Raises AuthenticationRejectedException on failure.
     """
-    webauthn_users = _get_webauthn_users(user, icon_url=icon_url, rp_id=rp_id)
+    webauthn_users = _get_webauthn_users(user, rp_id=rp_id)
     cred_ids = [cred.credential_id for cred in webauthn_users]
     encoded_challenge = _webauthn_b64encode(challenge.encode()).decode()