🌱 Adds Custom Factor #484

gowthamidommety-okta · 2018-06-14T23:36:48Z

Resolves: OKTA-169502

Enroll Flow using another okta org as an idp:

Verify Flow using another okta org as an idp:

mauriciocastillosilva-okta

left some comments

mauriciocastillosilva-okta · 2018-06-20T22:31:46Z

package.json

@@ -89,6 +89,7 @@
    "webpack": "1.13.1"
  },
  "dependencies": {
+    "@okta/i18n": "45.3.0",


this should be removed, same with the changes to package-lock.json. We don't need i18n as a dependency, the contents we need are in packages folder

mauriciocastillosilva-okta · 2018-06-20T22:42:29Z

src/VerifyCustomFactorController.js

+ * See the License for the specific language governing permissions and limitations under the License.
+ */
+
+/* eslint camelcase: 0 */


why do we need this? Couldn't find it...

mauriciocastillosilva-okta · 2018-06-20T22:43:24Z

src/util/FactorUtil.js

+      label: '',
+      description: 'factor.customFactor.description',
+      iconClassName: 'mfa-custom-factor',
+      sortOrder: 15


can we confirm with Alex about the order of custom factor?

confirmed, should be last

mauriciocastillosilva-okta · 2018-06-20T22:44:57Z

src/models/AppState.js

+          if (!res._links || !res._links.next || !res._links.next.href) {
+            return null;
+          }
+          return res._links.next.href;


we should check that the name of the link is redirect, so we are sure it is a url redirect and not any other next link

mauriciocastillosilva-okta

This is looking good.
Can you add a screenshot please?

Missing is one comment to confirm about the order of this custom factor, and the tests.

mauriciocastillosilva-okta · 2018-06-26T04:07:28Z

src/models/AppState.js

+      'enrollCustomFactorRedirectUrl': {
+        deps: ['lastAuthResponse'],
+        fn: function (res) {
+          if (!res._links || !res._links.next || res._links.next.name !== 'activate' || !res._links.next.href) {


I think this link should also be named redirect in our APIs, since this factor is very different to the other ones we currently support

@AjayThomas-okta thoughts?

as discussed, we will revisit this link name later.

mauriciocastillosilva-okta · 2018-06-26T04:10:02Z

test/unit/helpers/xhr/MFA_CHALLENGE_customFactor.js

+  "status": 200,
+  "responseType": "json",
+  "response": {
+    "stateToken": "00M2MpKONl8k_a3oRjwDcznNy6NKeNfb0lkKLUjDVG",


nit: should be a string with testStateToken or something similar, no need for this token here

👍 sorry, forgot to change

mauriciocastillosilva-okta · 2018-06-26T04:12:10Z

test/unit/helpers/xhr/MFA_ENROLL_ACTIVATE_CustomFactor.js

+    },
+    "_links":{  
+     "next":{  
+       "name":"activate",


why is this not redirect?

I'm not sure.. This is how it was designed - https://oktawiki.atlassian.net/wiki/spaces/~ajay.thomas/pages/494863610/Changes+to+authn+response+for+generic+SAML+OIDC+factors#ChangestoauthnresponseforgenericSAML/OIDCfactors-UserenrollsforSAMLfactor
@AjayThomas-okta?

mauriciocastillosilva-okta · 2018-06-26T04:15:39Z

test/unit/helpers/xhr/customFactor_error.js

@@ -0,0 +1,13 @@
+define({


can we rename this file to CUSTOM_FACTOR_error?
And also, do we need this file? it seems to be a generic error, so we might have already another file that we can use

This error is in specific files like SMS verify and resend.. Maybe we can use RECOVERY_error.js? If not, i'll rename this file to something more generic

mauriciocastillosilva-okta · 2018-06-26T04:18:12Z

test/unit/spec/EnrollCustomFactor_spec.js

+        return setup().then(function (test) {
+          test.setNextResponse(responseSuccess);
+          expect(test.form.titleText())
+            .toBe('GENERIC_SAML');


can we change the vendorName in the test xhr files to be something different than the provider? So we can test we are displaying the correct name. Maybe something like Third Party Factor?

srinivasanagandla-okta · 2018-06-26T13:29:22Z

@gowthamidommety-okta Can you add video?

mauriciocastillosilva-okta

looks good, just some nits. After fixing those this is great ⛵️

mauriciocastillosilva-okta · 2018-06-27T18:26:53Z

src/models/AppState.js

+      'enrollCustomFactorRedirectUrl': {
+        deps: ['lastAuthResponse'],
+        fn: function (res) {
+          if (!res._links || !res._links.next || res._links.next.name !== 'activate' || !res._links.next.href) {


as discussed, we will revisit this link name later.

mauriciocastillosilva-okta · 2018-06-27T18:31:48Z

test/unit/helpers/xhr/MFA_CHALLENGE_customFactor.js

+    "_links": {
+      "next": {
+        "name": "redirect",
+        "href": "http://rain.okta1.com:1802/policy/mfa-idp-redirect?okta_key=mfa.redirect.00opis8DrVkQCV2LL0g3",


let's change this to http://rain.okta1.com:1802/policy/mfa-idp-redirect?okta_key=mfa.redirect.id, same in ENROLL_ACTIVATE

mauriciocastillosilva-okta · 2018-06-27T18:32:19Z

test/unit/helpers/xhr/MFA_REQUIRED_allFactors.js

+        "profile": {"user":"administrator1@clouditude.net"},
+        "_links": {
+          "verify": {
+            "href":"http://rain.okta1.com:1802/api/v1/authn/factors/smlqwtL7uk8z7ikG80g3/verify",


also here, we should have http://rain.okta1.com:1802/api/v1/authn/factors/customFactorId/verify. Same comment for the other MFA_REQUIRED file

mauriciocastillosilva-okta · 2018-06-27T18:38:50Z

test/unit/spec/EnrollCustomFactor_spec.js

+        return setup().then(function (test) {
+          test.setNextResponse(responseSuccess);
+          expect(test.form.titleText())
+            .toBe('Third Party Factor');


nit: no need here for a new line I think. It could be in the same line 88

mauriciocastillosilva-okta · 2018-06-27T22:58:15Z

src/models/AppState.js

@@ -187,6 +187,12 @@ function (Okta, Q, Factor, BrowserFeatures, Errors) {
          return res.status === 'MFA_ENROLL';
        }
      },
+      'isMfaEnrollActivate': {


this method already exists, look below right after isMfaTimeout

Sorry, will remove

mauriciocastillosilva-okta

looks good

Resolves: OKTA-169502

gowthamidommety-okta requested a review from mauriciocastillosilva-okta June 20, 2018 05:18

gowthamidommety-okta force-pushed the gd-OKTA-169502-customFactor branch from 12fd476 to e6d035c Compare June 20, 2018 20:01

mauriciocastillosilva-okta reviewed Jun 20, 2018

View reviewed changes

mauriciocastillosilva-okta reviewed Jun 21, 2018

View reviewed changes

gowthamidommety-okta force-pushed the gd-OKTA-169502-customFactor branch from 9edfd4f to 26552d0 Compare June 22, 2018 23:05

gowthamidommety-okta requested a review from AjayThomas-okta June 25, 2018 20:13

mauriciocastillosilva-okta reviewed Jun 26, 2018

View reviewed changes

gowthamidommety-okta force-pushed the gd-OKTA-169502-customFactor branch from 4aeb4a7 to 4fc4cf4 Compare June 27, 2018 16:08

mauriciocastillosilva-okta approved these changes Jun 27, 2018

View reviewed changes

mauriciocastillosilva-okta requested changes Jun 27, 2018

View reviewed changes

mauriciocastillosilva-okta approved these changes Jun 27, 2018

View reviewed changes

gowthamidommety-okta force-pushed the gd-OKTA-169502-customFactor branch 2 times, most recently from c1fc688 to 813e611 Compare June 28, 2018 00:14

🌱 Adds Custom Factor

67ac268

Resolves: OKTA-169502

gowthamidommety-okta force-pushed the gd-OKTA-169502-customFactor branch from 813e611 to 67ac268 Compare June 28, 2018 19:01

gowthamidommety-okta removed the request for review from AjayThomas-okta June 28, 2018 19:02

gowthamidommety-okta merged commit 975ceb8 into okta:master Jun 28, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🌱 Adds Custom Factor #484

🌱 Adds Custom Factor #484

gowthamidommety-okta commented Jun 14, 2018 •

edited

Loading

mauriciocastillosilva-okta left a comment

mauriciocastillosilva-okta Jun 20, 2018

gowthamidommety-okta Jun 20, 2018

mauriciocastillosilva-okta Jun 20, 2018

mauriciocastillosilva-okta Jun 20, 2018

gowthamidommety-okta Jun 28, 2018

mauriciocastillosilva-okta Jun 20, 2018

gowthamidommety-okta Jun 20, 2018

mauriciocastillosilva-okta left a comment

mauriciocastillosilva-okta Jun 26, 2018

gowthamidommety-okta Jun 26, 2018

mauriciocastillosilva-okta Jun 27, 2018

mauriciocastillosilva-okta Jun 26, 2018

gowthamidommety-okta Jun 26, 2018

mauriciocastillosilva-okta Jun 26, 2018

gowthamidommety-okta Jun 26, 2018 •

edited

Loading

mauriciocastillosilva-okta Jun 26, 2018

gowthamidommety-okta Jun 26, 2018

mauriciocastillosilva-okta Jun 26, 2018

gowthamidommety-okta Jun 26, 2018

srinivasanagandla-okta commented Jun 26, 2018

mauriciocastillosilva-okta left a comment

mauriciocastillosilva-okta Jun 27, 2018

mauriciocastillosilva-okta Jun 27, 2018

mauriciocastillosilva-okta Jun 27, 2018

mauriciocastillosilva-okta Jun 27, 2018

mauriciocastillosilva-okta Jun 27, 2018

gowthamidommety-okta Jun 27, 2018

mauriciocastillosilva-okta left a comment

🌱 Adds Custom Factor #484

🌱 Adds Custom Factor #484

Conversation

gowthamidommety-okta commented Jun 14, 2018 • edited Loading

mauriciocastillosilva-okta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mauriciocastillosilva-okta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gowthamidommety-okta Jun 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srinivasanagandla-okta commented Jun 26, 2018

mauriciocastillosilva-okta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mauriciocastillosilva-okta left a comment

Choose a reason for hiding this comment

gowthamidommety-okta commented Jun 14, 2018 •

edited

Loading

gowthamidommety-okta Jun 26, 2018 •

edited

Loading