2 Factor Auth #950

Merged
merged 21 commits into from Feb 7, 2014

Conversation

Projects
None yet
5 participants
@tarunc
Contributor

tarunc commented Feb 4, 2014

Finally! Fixes #684

Pictures for ease:
screenshot 2014-02-04 14 22 35
screenshot 2014-02-04 14 22 49
screenshot 2014-02-04 14 22 57
screenshot 2014-02-04 14 22 19
screenshot 2014-02-04 14 23 08
screenshot 2014-02-04 14 27 28

CC @dmdj03 how do people go back to marketplaces after they have enabled security?

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Feb 5, 2014

Contributor

I restarted the build, since it errored out.

Contributor

steveklabnik commented Feb 5, 2014

I restarted the build, since it errored out.

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Feb 5, 2014

Contributor

Woot, passing!

Contributor

steveklabnik commented Feb 5, 2014

Woot, passing!

@mjallday

This comment has been minimized.

Show comment
Hide comment
@mjallday

mjallday Feb 5, 2014

Contributor

@dmdj03 how do people go back to marketplaces after they have enabled security?

great question @dmdj03

Contributor

mjallday commented Feb 5, 2014

@dmdj03 how do people go back to marketplaces after they have enabled security?

great question @dmdj03

@tarunc

This comment has been minimized.

Show comment
Hide comment
@tarunc

tarunc Feb 5, 2014

Contributor

@dmdj03 Also, to disable multi factor auth, you should need to enter an otp. We should make it in that disable confirmation window.

Credit belongs to @mjallday for this.

Contributor

tarunc commented Feb 5, 2014

@dmdj03 Also, to disable multi factor auth, you should need to enter an otp. We should make it in that disable confirmation window.

Credit belongs to @mjallday for this.

@dmdj03

This comment has been minimized.

Show comment
Hide comment
@dmdj03

dmdj03 Feb 6, 2014

When you successfully activate, you will be taken to the activities page with a confirmation message at the top. Same if you deactivate.

screen shot 2014-02-05 at 4 25 52 pm

dmdj03 commented Feb 6, 2014

When you successfully activate, you will be taken to the activities page with a confirmation message at the top. Same if you deactivate.

screen shot 2014-02-05 at 4 25 52 pm

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Feb 6, 2014

Contributor

That didn't actually happen when I just tried it out.

Contributor

steveklabnik commented Feb 6, 2014

That didn't actually happen when I just tried it out.

@tarunc

This comment has been minimized.

Show comment
Hide comment
@tarunc

tarunc Feb 6, 2014

Contributor

Yup, Damon just clarified that right now. Working on it.

Contributor

tarunc commented Feb 6, 2014

Yup, Damon just clarified that right now. Working on it.

@tarunc

This comment has been minimized.

Show comment
Hide comment
@tarunc

tarunc Feb 6, 2014

Contributor

@dmdj03 I'm going to make it go back to the marketplaces page instead of the activity page because 2-factor auth is not marketplace specific-- they can do it from anywhere after they are logged in.

Contributor

tarunc commented Feb 6, 2014

@dmdj03 I'm going to make it go back to the marketplaces page instead of the activity page because 2-factor auth is not marketplace specific-- they can do it from anywhere after they are logged in.

@tarunc

This comment has been minimized.

Show comment
Hide comment
@tarunc

tarunc Feb 6, 2014

Contributor

@mjallday Just added the redirects/confirmation messages after you enable or disable multi-factor auth.

Contributor

tarunc commented Feb 6, 2014

@mjallday Just added the redirects/confirmation messages after you enable or disable multi-factor auth.

@Knele83

This comment has been minimized.

Show comment
Hide comment
@Knele83

Knele83 Feb 6, 2014

As for payment I did not even know about it and have no money I am disabled at work I fell 3 tons per foot, and now I'm half a foot amputated when I was 37 years old and now I have about 41.tako that I really have a participated'd as for paying, but i have only a month disability assistance 200 dollars and I have two sons and barely survive here in Srbiji.Hvala your understanding ..
dejan vasic knele

Knele83 commented Feb 6, 2014

As for payment I did not even know about it and have no money I am disabled at work I fell 3 tons per foot, and now I'm half a foot amputated when I was 37 years old and now I have about 41.tako that I really have a participated'd as for paying, but i have only a month disability assistance 200 dollars and I have two sons and barely survive here in Srbiji.Hvala your understanding ..
dejan vasic knele

@mjallday

This comment has been minimized.

Show comment
Hide comment
@mjallday

mjallday Feb 6, 2014

Contributor
WARN [web-server]: 404: /images/balanced-logo.png
Firefox 19.0.0 (Linux) ERROR
    Error: Logs not working
    at /home/travis/build/balanced/balanced-dashboard/test/integration/logs.js:24
Firefox 19.0.0 (Linux): Executed 115 of 0 ERROR (0 secs / 7 mins 58.986 secs)
 ERROR (15 mins 2.763 secs / 7 mins 58.986 secs)

@tarunc related?

Contributor

mjallday commented Feb 6, 2014

WARN [web-server]: 404: /images/balanced-logo.png
Firefox 19.0.0 (Linux) ERROR
    Error: Logs not working
    at /home/travis/build/balanced/balanced-dashboard/test/integration/logs.js:24
Firefox 19.0.0 (Linux): Executed 115 of 0 ERROR (0 secs / 7 mins 58.986 secs)
 ERROR (15 mins 2.763 secs / 7 mins 58.986 secs)

@tarunc related?

+ loadQRCode: function() {
+ var self = this;
+
+ $.getScript('//cdnjs.cloudflare.com/ajax/libs/jquery.qrcode/1.0/jquery.qrcode.min.js', function() {

This comment has been minimized.

@mjallday

mjallday Feb 7, 2014

Contributor

why isn't this in bower?

@mjallday

mjallday Feb 7, 2014

Contributor

why isn't this in bower?

This comment has been minimized.

@tarunc

tarunc Feb 7, 2014

Contributor

I didn't want to have the app include the 5kb script for generating qr codes every time because it is only used on the account security page.

@tarunc

tarunc Feb 7, 2014

Contributor

I didn't want to have the app include the 5kb script for generating qr codes every time because it is only used on the account security page.

app/controllers/account_security.js
+ }.property('status'),
+
+ isNotEnabling: function() {
+ return this.get('status') !== 'enabling';

This comment has been minimized.

@mjallday

mjallday Feb 7, 2014

Contributor

isn't this exactly the same as isEnabling but in reverse?

@mjallday

mjallday Feb 7, 2014

Contributor

isn't this exactly the same as isEnabling but in reverse?

app/controllers/account_security.js
+ enable_label: function() {
+ var status = this.get('status');
+
+ if (status === 'disabled') {

This comment has been minimized.

@mjallday

mjallday Feb 7, 2014

Contributor

in the future you can simplify this as

return {
   'disabled': 'Enable',
   'enabling': 'Enabling',
}[status] || 'On'
@mjallday

mjallday Feb 7, 2014

Contributor

in the future you can simplify this as

return {
   'disabled': 'Enable',
   'enabling': 'Enabling',
}[status] || 'On'
app/auth.js
+ var self = this;
+
+ return this.request({
+ url: ENV.BALANCED.AUTH + '/users/' + this.get('userId') + '/otp',

This comment has been minimized.

@mjallday

mjallday Feb 7, 2014

Contributor

this should be returned by the user object. the app shouldn't need to know how to create uris

@mjallday

mjallday Feb 7, 2014

Contributor

this should be returned by the user object. the app shouldn't need to know how to create uris

This comment has been minimized.

@tarunc

tarunc Feb 7, 2014

Contributor

You are right-- we do that in every model.

@tarunc

tarunc Feb 7, 2014

Contributor

You are right-- we do that in every model.

@tarunc

This comment has been minimized.

Show comment
Hide comment
@tarunc

tarunc Feb 7, 2014

Contributor

@mjallday The logs test error isn't related to this.

Contributor

tarunc commented Feb 7, 2014

@mjallday The logs test error isn't related to this.

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Feb 7, 2014

Contributor

So is this ready to go?

Contributor

steveklabnik commented Feb 7, 2014

So is this ready to go?

mjallday added a commit that referenced this pull request Feb 7, 2014

@mjallday mjallday merged commit 10c1115 into balanced:master Feb 7, 2014

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment