Merge ~roadmr/canonical-identity-provider:add-2fa-dev-creation-metrics into canonical-identity-provider:master

Proposed by Daniel Manrique
Status: Merged
Approved by: Daniel Manrique
Approved revision: c6d56679be600adde7bfb2946c58328f8cf1c4f0
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~roadmr/canonical-identity-provider:add-2fa-dev-creation-metrics
Merge into: canonical-identity-provider:master
Diff against target: 113 lines (+36/-4)
2 files modified
src/webui/tests/test_views_devices.py (+22/-0)
src/webui/views/devices.py (+14/-4)
Reviewer Review Type Date Requested Status
Maximiliano Bertacchini Approve
Review via email: mp+384389@code.launchpad.net

Commit message

Emit metrics when 2fa devices are added.

The metric includes the device type (automatically-added backup devices
have the fake "paper_auto" type) and subtype (for OATH devices which
can be TOTP or HOTP)

Description of the change

Emitted metrics look like flows.device.$DEVICE_TYPE.add.

Device types can be e.g. google_totp, google_hotp (and so on for oath devices), paper, paper_auto, for now.

To post a comment you must log in.
Revision history for this message
Maximiliano Bertacchini (maxiberta) wrote :

LGTM!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/webui/tests/test_views_devices.py b/src/webui/tests/test_views_devices.py
index 4c1586b..aadca90 100644
--- a/src/webui/tests/test_views_devices.py
+++ b/src/webui/tests/test_views_devices.py
@@ -50,6 +50,8 @@ class DeviceViewsTestCaseBase(AuthenticatedTestCase):
50 self.switch_twofactor = switches(TWOFACTOR=True)50 self.switch_twofactor = switches(TWOFACTOR=True)
51 self.switch_twofactor.patch()51 self.switch_twofactor.patch()
52 self.addCleanup(self.switch_twofactor.unpatch)52 self.addCleanup(self.switch_twofactor.unpatch)
53 self.mock_increment = self.patch(
54 'webui.views.ui.stats.increment')
5355
54 def do_2fa_login(self, device):56 def do_2fa_login(self, device):
55 oath_token = self.factory.make_next_otp(device)57 oath_token = self.factory.make_next_otp(device)
@@ -509,6 +511,11 @@ class DeviceAdditionViewTestCase(DeviceViewsTestCaseBase):
509 )511 )
510 self.assertRedirects(response, redirect_to, status_code=303)512 self.assertRedirects(response, redirect_to, status_code=303)
511513
514 args_list = [
515 ((u'flows.device.generic_hotp',), {u'key': u'new'}),
516 ((u'flows.device.paper_auto',), {u'key': u'new'})]
517 self.assertEqual(self.mock_increment.call_args_list, args_list)
518
512 # When the auto backup deviceflag is unset...519 # When the auto backup deviceflag is unset...
513 @switches(TWOFACTOR_AUTO_BACKUP_DEVICE=False)520 @switches(TWOFACTOR_AUTO_BACKUP_DEVICE=False)
514 def test_successful_post_generic_no_paper_no_auto_add(self):521 def test_successful_post_generic_no_paper_no_auto_add(self):
@@ -521,6 +528,10 @@ class DeviceAdditionViewTestCase(DeviceViewsTestCaseBase):
521 # Does not add a paper backup device528 # Does not add a paper backup device
522 self.assertEqual(devices.filter(device_type="paper").count(), 0)529 self.assertEqual(devices.filter(device_type="paper").count(), 0)
523530
531 args_list = [
532 ((u'flows.device.generic_hotp',), {u'key': u'new'})]
533 self.assertEqual(self.mock_increment.call_args_list, args_list)
534
524 # When the auto backup deviceflag is set...535 # When the auto backup deviceflag is set...
525 @switches(TWOFACTOR_AUTO_BACKUP_DEVICE=True)536 @switches(TWOFACTOR_AUTO_BACKUP_DEVICE=True)
526 def test_successful_post_generic_paper_no_add(self):537 def test_successful_post_generic_paper_no_add(self):
@@ -538,6 +549,13 @@ class DeviceAdditionViewTestCase(DeviceViewsTestCaseBase):
538 # Does not add additional paper devices.549 # Does not add additional paper devices.
539 self.assertEqual(devices.filter(device_type="paper").count(), 1)550 self.assertEqual(devices.filter(device_type="paper").count(), 1)
540551
552 # Args list changes a bit because we did an actual login
553 args_list = [
554 ((u'flows.login',), {u'rpconfig': None, u'key': u'success'}),
555 ((u'flows.2fa',), {u'rpconfig': None, u'key': u'success'}),
556 ((u'flows.device.generic_hotp',), {u'key': u'new'})]
557 self.assertEqual(self.mock_increment.call_args_list, args_list)
558
541 # When the auto backup deviceflag is set...559 # When the auto backup deviceflag is set...
542 @switches(TWOFACTOR_AUTO_BACKUP_DEVICE=True)560 @switches(TWOFACTOR_AUTO_BACKUP_DEVICE=True)
543 def test_successful_post_paper_main_no_add(self):561 def test_successful_post_paper_main_no_add(self):
@@ -553,6 +571,10 @@ class DeviceAdditionViewTestCase(DeviceViewsTestCaseBase):
553 # Does not add additional paper devices.571 # Does not add additional paper devices.
554 self.assertEqual(devices.filter(device_type="paper").count(), 1)572 self.assertEqual(devices.filter(device_type="paper").count(), 1)
555573
574 args_list = [
575 ((u'flows.device.paper',), {u'key': u'new'})]
576 self.assertEqual(self.mock_increment.call_args_list, args_list)
577
556 def test_unrecognised_type(self):578 def test_unrecognised_type(self):
557 data = {'type': 'foobar', 'hex_key': KEY, 'name': 'Some device',579 data = {'type': 'foobar', 'hex_key': KEY, 'name': 'Some device',
558 'otp': 'some otp key'}580 'otp': 'some otp key'}
diff --git a/src/webui/views/devices.py b/src/webui/views/devices.py
index 3f7716b..c66a4a5 100644
--- a/src/webui/views/devices.py
+++ b/src/webui/views/devices.py
@@ -24,6 +24,7 @@ from identityprovider.forms import DeviceRenameForm, OATHDeviceForm
24from identityprovider.models import AuthenticationDevice24from identityprovider.models import AuthenticationDevice
25from identityprovider.models import twofactor25from identityprovider.models import twofactor
26from identityprovider.models.twofactor import get_otp_type26from identityprovider.models.twofactor import get_otp_type
27from identityprovider.stats import stats
2728
28from webui.decorators import require_twofactor_enabled, sso_login_required29from webui.decorators import require_twofactor_enabled, sso_login_required
29from webui.views.const import (30from webui.views.const import (
@@ -187,7 +188,9 @@ def _device_addition_standard(request, device_type):
187 device_types['paper'],188 device_types['paper'],
188 request.user)189 request.user)
189 backup_device = _create_device(190 backup_device = _create_device(
190 request, device_name, backup_hex_key, 0, 'paper')191 request, device_name, backup_hex_key, 0, 'paper',
192 auto=True)
193
191 # If a device was auto-added, send to a nice page.194 # If a device was auto-added, send to a nice page.
192 # Very similar to device-print view with some tweaks195 # Very similar to device-print view with some tweaks
193 # for auto-generated device.196 # for auto-generated device.
@@ -222,20 +225,27 @@ def _device_addition_standard(request, device_type):
222225
223226
224def _create_device(227def _create_device(
225 request, device_name, hex_key, counter, device_type, oath_type=None):228 request, device_name, hex_key, counter, device_type, oath_type=None,
229 auto=False):
230
231 full_device_type = device_type + (
232 ':{}'.format(oath_type) if oath_type else '')
233 stats_device_type = (
234 "paper_auto" if device_type == 'paper' and auto else device_type) + (
235 '_{}'.format(oath_type) if oath_type else '')
226 device = AuthenticationDevice.objects.create(236 device = AuthenticationDevice.objects.create(
227 account=request.user,237 account=request.user,
228 name=device_name,238 name=device_name,
229 key=hex_key,239 key=hex_key,
230 counter=counter,240 counter=counter,
231 device_type=device_type + (241 device_type=full_device_type,
232 ':{}'.format(oath_type) if oath_type else ''),
233 )242 )
234 twofactor.login(request)243 twofactor.login(request)
235 added_type = oath_type.upper() if oath_type else device_types[device_type]244 added_type = oath_type.upper() if oath_type else device_types[device_type]
236 messages.success(245 messages.success(
237 request, DEVICE_ADDED.format(246 request, DEVICE_ADDED.format(
238 name=device_name, type=added_type), 'temporary')247 name=device_name, type=added_type), 'temporary')
248 stats.increment('flows.device.{}'.format(stats_device_type), key='new')
239 return device249 return device
240250
241251

Subscribers

People subscribed via source and target branches