Merge pull request #823 from krzysztofdrys/fail-early-on-no-token

Return more informative error message incase there is no sessionToken

Author: mapkon

pkg/provider/okta/okta.go

@@ -823,14 +823,7 @@ func verifyMfa(oc *Client, oktaOrgHost string, loginDetails *creds.LoginDetails,
 			return "", errors.Wrap(err, "error retrieving token post response")
 		}
 
-		body, err := io.ReadAll(res.Body)
-		if err != nil {
-			return "", errors.Wrap(err, "error retrieving body from response")
-		}
-
-		resp = string(body)
-
-		return gjson.Get(resp, "sessionToken").String(), nil
+		return extractSessionToken(res.Body)
 
 	case IdentifierPushMfa:
 
@@ -1151,6 +1144,25 @@ func verifyMfa(oc *Client, oktaOrgHost string, loginDetails *creds.LoginDetails,
 	return "", errors.New("no mfa options provided")
 }
 
+func extractSessionToken(r io.Reader) (string, error) {
+	bb, err := io.ReadAll(r)
+	if err != nil {
+		return "", errors.Wrap(err, "error retrieving body from response")
+	}
+
+	resp := string(bb)
+	sessionToken := gjson.Get(resp, "sessionToken").String()
+	if sessionToken == "" {
+		status := gjson.Get(resp, "status").String()
+		if status != "" {
+			return "", errors.Errorf("response does not contain session token, received status is: %q", status)
+		}
+		return "", errors.Errorf("response does not contain session token")
+	}
+
+	return gjson.Get(resp, "sessionToken").String(), nil
+}
+
 func fidoWebAuthn(oc *Client, oktaOrgHost string, challengeContext *mfaChallengeContext, mfaOption int, stateToken string, mfaOptions []string, resp string) (string, error) {
 
 	var signedAssertion *SignedAssertion

pkg/provider/okta/okta_test.go

@@ -4,10 +4,13 @@ import (
 	"crypto/tls"
 	"errors"
 	"fmt"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
+	"testing/iotest"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/versent/saml2aws/v2/pkg/cfg"
@@ -63,6 +66,65 @@ func TestGetStateTokenFromOktaPageBody(t *testing.T) {
 	}
 }
 
+func TestExtractSessionToken(t *testing.T) {
+	tests := []struct {
+		name          string
+		r             io.Reader
+		expectedToken string
+		expectedError string
+	}{
+		{
+			name:          "response with session token",
+			r:             strings.NewReader(`{"sessionToken": "xxxx"}`),
+			expectedToken: "xxxx",
+		},
+		{
+			name:          "response with no session token but with status",
+			r:             strings.NewReader(`{"status": "invalid password"}`),
+			expectedError: "response does not contain session token, received status is: \"invalid password\"",
+		},
+		{
+			name:          "response with no session token and no status",
+			r:             strings.NewReader(`{}`),
+			expectedError: "response does not contain session token",
+		},
+		{
+			name:          "response is not even json",
+			r:             strings.NewReader(`const x = {}`),
+			expectedError: "response does not contain session token",
+		},
+		{
+			name:          "reader returns an error",
+			r:             iotest.ErrReader(fmt.Errorf("failed to read")),
+			expectedError: "error retrieving body from response: failed to read",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			resp, err := extractSessionToken(tc.r)
+			if tc.expectedError != "" {
+				if err == nil {
+					t.Fatalf("Expected error, but got null")
+				}
+				if err.Error() != tc.expectedError {
+					t.Fatalf("Expected error %q, but got %q",
+						err.Error(), tc.expectedError,
+					)
+				}
+			}
+			if tc.expectedToken != "" {
+				if err != nil {
+					t.Fatalf("Expected token %q, but got error %v", tc.expectedToken, err)
+				}
+				if resp != tc.expectedToken {
+					t.Fatalf("Expected token %q, but got %q", tc.expectedToken, resp)
+				}
+			}
+		})
+	}
+}
+
 func TestGetMfaChallengeContext(t *testing.T) {
 	ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		_, _ = w.Write([]byte("OK"))