Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework auth request code to provide useful error messages #172

Merged
merged 5 commits into from
Aug 9, 2018

Conversation

randomsamples
Copy link

Problem Statement

Was getting json responses from the authn call that were of 'status' : 'MFA_ENROLL' , and had no 'sessionToken', but did have a stateToken. The app crashed with a JSON parse error with very little useful information.

→ okta sts get-caller-identity
Username: XXXXXXX
Password:
Exception in thread "main" org.json.JSONException: JSONObject["sessionToken"] not found.
    at org.json.JSONObject.get(JSONObject.java:520)
    at org.json.JSONObject.getString(JSONObject.java:806)
    at com.okta.tools.authentication.OktaAuthentication.getOktaSessionToken(OktaAuthentication.java:40)
    at com.okta.tools.saml.OktaSaml.getSamlResponse(OktaSaml.java:46)
    at com.okta.tools.OktaAwsCliAssumeRole.doRequest(OktaAwsCliAssumeRole.java:106)
    at com.okta.tools.OktaAwsCliAssumeRole.run(OktaAwsCliAssumeRole.java:90)
    at com.okta.tools.awscli.main(awscli.java:40)

PROBLEM: Valid responses from the authn endpoint should not crash outright but rather provide the user with information to help diagnose the problem.

Solution

This PR restructures the authn response handling code to be more defensive in nature. It parses the incoming response json based on the status code, which looks sane based on the docks on okta. It adds a number of sanity checks and, if something goes awry the user will get a helpful error message and a copy of the response json in their command line output. The enum+switch structure makes it easy to expand coverage of the various various features of the okta API. Also added some doc strings and pointers to the Okta documentation where relevant.

In the same missing "sessionToken" scenario, our user will see this much more helpful message:

> okta sts get-caller-identity
Username: myuser
Password:
Exception in thread "main" java.lang.IllegalStateException: Could not find the expected property "sessionToken" in the response message.

Message:
{
   "_embedded":{
      "user":{
         "profile":{
            "firstName":"dhjfkc",
            "lastName":"fdsf",
            "timeZone":"America/Los_Angeles",
            "login":"[email protected]",
            "locale":"en"
         },
         "id":"00ulble6ayd6xOn48g0x7"
      },
      "factors":[
         {
            "provider":"OKTA",
            "_links":{
               "enroll":{
                  "hints":{
                     "allow":[
                        "POST"
                     ]
                  },
                  "href":"https://myco.okta.com/api/v1/authn/factors"
               }
            },
            "factorType":"push",
            "vendorName":"OKTA",
            "status":"NOT_SETUP",
            "enrollment":"REQUIRED"
         }
      ]
   },
   "_links":{
      "cancel":{
         "hints":{
            "allow":[
               "POST"
            ]
         },
         "href":"https://myco.okta.com/api/v1/authn/cancel"
      }
   },
   "stateToken":"00rAYJMd49Da10B0H4aJmGoCzrFj4AZ4lQS5W3f1bd",
   "expiresAt":"2018-08-08T23:50:34.000Z",
   "status":"MFA_ENROLL"
}

	at com.okta.tools.authentication.OktaAuthentication.makeException(OktaAuthentication.java:222)
	at com.okta.tools.authentication.OktaAuthentication.makeException(OktaAuthentication.java:209)
	at com.okta.tools.authentication.OktaAuthentication.getOktaSessionToken(OktaAuthentication.java:80)
	at com.okta.tools.saml.OktaSaml.getSamlResponse(OktaSaml.java:46)
	at com.okta.tools.OktaAwsCliAssumeRole.doRequest(OktaAwsCliAssumeRole.java:106)
	at com.okta.tools.OktaAwsCliAssumeRole.run(OktaAwsCliAssumeRole.java:90)
	at com.okta.tools.awscli.main(awscli.java:40)```

Copy link
Collaborator

@AlainODea AlainODea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat implementation. My manual tests with and without browser auth and with and without MFA worked.

I like this change a lot.

Thank you very much for your contribution, @randomsamples 🎉

@AlainODea AlainODea merged commit c7b373f into oktadev:master Aug 9, 2018
mraible pushed a commit that referenced this pull request Aug 17, 2018
Rework auth request code to provide useful error messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants