Adds 2 factor backend code by thatarchguy · Pull Request #169 · StackFocus/PostMaster

thatarchguy · 2016-10-19T15:08:46Z

Ripped from #165 because of decision to separate the merge into frontend and backend pieces #165 (comment)

Signed-off-by: Kevin <kevin@stealsyour.pw>

… into ft-enable_2_factor

Signed-off-by: Kevin <kevin@stealsyour.pw>

mprahl · 2016-10-25T23:21:01Z

postmaster/apiv1/admins.py

+            return dict(enabled=admin.otp_active)
+        elif status.lower() == "true":
+            raise GenericError("Cannot enable 2 factor from this route - see docs")
+    raise GenericError("An Invalid parameter was supplied")


Lower case I in "invalid"

mprahl · 2016-10-25T23:22:40Z

postmaster/apiv1/admins.py

+                    'error', current_user.username,
+                    'The following error occurred in twofactor_disable: {0}'.format(str(e)))
+                raise GenericError('The administrator could not be updated')
+            return dict(enabled=admin.otp_active)


I don't think we should return anything in this case. Just an HTTP 200.

@thatarchguy can you take a look at this?

mprahl · 2016-10-25T23:28:44Z

postmaster/apiv1/admins.py

+                        'error', current_user.username,
+                        'The following error occurred in verify_qrcode: {0}'.format(str(e)))
+                    raise GenericError('The administrator could not be updated')
+            return dict(status="Success")


I don't think we should be returning this here. We don't anywhere else in the app. A simple HTTP 200 should do here with no JSON.

mprahl · 2016-10-25T23:31:00Z

postmaster/forms.py


 from flask_wtf import Form
-from wtforms import StringField, PasswordField, SelectField
+from wtforms import StringField, PasswordField, SelectField, IntegerField, validators


Instead of importing validators, import Optional from it on the next line. Alternatively, you can remove the line below and do validators.DataRequired() on line 17, 18, and 20.

mprahl · 2016-10-25T23:32:12Z

postmaster/forms.py

    """
    username = StringField(label='Username', validators=[DataRequired()])
    password = PasswordField(label='Password', validators=[DataRequired(), validate_wtforms_password])
+    two_factor = IntegerField(label='2 Factor', validators=(validators.Optional(),))


Can the label be "Two Factor Code"

I'm also thinking that instead of this, we should pop up a modal if two factor authentication is enabled. This is fine for now though.

mprahl · 2016-10-25T23:41:35Z

postmaster/apiv1/admins.py

    return {}, 200
+
+
+@apiv1.route('/admins/<int:admin_id>/2factor', methods=['GET'])


Can we change the route to "twofactor"?

mprahl · 2016-10-25T23:41:39Z

postmaster/apiv1/admins.py

+    return dict(enabled=admin.otp_active)
+
+
+@apiv1.route('/admins/<int:admin_id>/2factor', methods=['PUT'])


Can we change the route to "twofactor"?

mprahl · 2016-10-25T23:41:52Z

postmaster/apiv1/admins.py

+    raise GenericError("An Invalid parameter was supplied")
+
+
+@apiv1.route('/admins/<int:admin_id>/2factor/qrcode', methods=['GET'])


Can we change the route to "twofactor"?

mprahl · 2016-10-25T23:41:57Z

postmaster/apiv1/admins.py

+        'Expires': '0'}
+
+
+@apiv1.route('/admins/<int:admin_id>/2factor/verify', methods=['POST'])


Can we change the route to "twofactor"?

mprahl · 2016-10-25T23:44:01Z

tests/api/test_api_functions.py

+            assert False, "Not json"
+        assert "Success" in rv.data
+        assert rv.status_code == 200
+        assert "Success" in rv.data


Duplicate line. Please remove.

mprahl · 2016-10-25T23:44:43Z

@thatarchguy nice work. I just have some simple comments you need to address and then we'll merge this.

mprahl · 2016-10-25T23:49:17Z

@thatarchguy also please add to the Change Log.

Signed-off-by: Kevin <kevin@stealsyour.pw>

mprahl · 2016-10-29T00:03:37Z

@thatarchguy LGTM!

Next time, don't merge master into your branch, do:

git checkout master
git pull
git checkout feature-branch-name
git rebase master

This stacks your new commits on top of the latest master.

thatarchguy added 7 commits August 31, 2016 16:48

Enables 2 factor option for login

d629ff2

Signed-off-by: Kevin <kevin@stealsyour.pw>

Enables 2 factor option for login

11091ec

Signed-off-by: Kevin <kevin@stealsyour.pw>

Fixes up Python code

f78dd30

Signed-off-by: Kevin <kevin@stealsyour.pw>

Merge branch 'ft-enable_2_factor' of github.com:StackFocus/PostMaster…

9451d0a

… into ft-enable_2_factor

Takes out jsonify

f417a11

Signed-off-by: Kevin <kevin@stealsyour.pw>

Only update database when enabling 2 factor on verify

0b165cc

Signed-off-by: Kevin <kevin@stealsyour.pw>

Reverts frontend code back to release v1.1.0

bcd7902

Signed-off-by: Kevin <kevin@stealsyour.pw>

thatarchguy assigned mprahl Oct 19, 2016

thatarchguy added in progress needs review labels Oct 19, 2016

mprahl requested changes Oct 25, 2016

View reviewed changes

thatarchguy added 6 commits October 26, 2016 10:43

Removes css changes from PR

c944f23

Signed-off-by: Kevin <kevin@stealsyour.pw>

Revises route to twofactor

5b4b788

Signed-off-by: Kevin <kevin@stealsyour.pw>

Changes return to empty json

36816f1

Signed-off-by: Kevin <kevin@stealsyour.pw>

Merge remote-tracking branch 'origin/master' into ft-enable_2_factor

4d76f31

Fixes merge conflict in requirements

ba0082d

Signed-off-by: Kevin <kevin@stealsyour.pw>

Adds to Changelog

9cb158a

Signed-off-by: Kevin <kevin@stealsyour.pw>

mprahl approved these changes Oct 29, 2016

View reviewed changes

mprahl merged commit b1e046c into master Oct 29, 2016

mprahl deleted the ft-enable_2_factor branch October 29, 2016 00:03

thatarchguy removed the in progress label Oct 29, 2016

mprahl mentioned this pull request Oct 29, 2016

2 factor authentication #137

Open

		return {}, 200


		@apiv1.route('/admins/<int:admin_id>/2factor', methods=['GET'])

		return dict(enabled=admin.otp_active)


		@apiv1.route('/admins/<int:admin_id>/2factor', methods=['PUT'])

		raise GenericError("An Invalid parameter was supplied")


		@apiv1.route('/admins/<int:admin_id>/2factor/qrcode', methods=['GET'])

		'Expires': '0'}


		@apiv1.route('/admins/<int:admin_id>/2factor/verify', methods=['POST'])

Conversation

thatarchguy commented Oct 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mprahl commented Oct 25, 2016

Uh oh!

mprahl commented Oct 25, 2016

Uh oh!

mprahl commented Oct 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants