Upgrade okta sdk lib (#8143)

Upgrade to new official Okta sdk lib.  Since it requires an API token, use old unofficial okta lib for no-apitoken case. 

Update test to use newer field names.  Remove obsolete test invalidated by #4798.  Properly handle case where an error was expected and didn't occur.

builtin/credential/okta/backend.go

@@ -5,11 +5,11 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/chrismalek/oktasdk-go/okta"
 	"github.com/hashicorp/vault/helper/mfa"
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/helper/cidrutil"
 	"github.com/hashicorp/vault/sdk/logical"
+	"github.com/okta/okta-sdk-golang/okta"
 )
 
 func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) {
@@ -77,7 +77,10 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
 		}
 	}
 
-	client := cfg.OktaClient()
+	shim, err := cfg.OktaClient()
+	if err != nil {
+		return nil, nil, nil, err
+	}
 
 	type mfaFactor struct {
 		Id       string `json:"id"`
@@ -97,7 +100,7 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
 		StateToken   string         `json:"stateToken"`
 	}
 
-	authReq, err := client.NewRequest("POST", "authn", map[string]interface{}{
+	authReq, err := shim.NewRequest("POST", "/api/v1/authn", map[string]interface{}{
 		"username": username,
 		"password": password,
 	})
@@ -106,8 +109,11 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
 	}
 
 	var result authResult
-	rsp, err := client.Do(authReq, &result)
+	rsp, err := shim.Do(authReq, &result)
 	if err != nil {
+		if oe, ok := err.(*okta.Error); ok {
+			return nil, logical.ErrorResponse("Okta auth failed: %v (code=%v)", err, oe.ErrorCode), nil, nil
+		}
 		return nil, logical.ErrorResponse(fmt.Sprintf("Okta auth failed: %v", err)), nil, nil
 	}
 	if rsp == nil {
@@ -178,12 +184,12 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
 		payload := map[string]interface{}{
 			"stateToken": result.StateToken,
 		}
-		verifyReq, err := client.NewRequest("POST", requestPath, payload)
+		verifyReq, err := shim.NewRequest("POST", requestPath, payload)
 		if err != nil {
 			return nil, nil, nil, err
 		}
 
-		rsp, err := client.Do(verifyReq, &result)
+		rsp, err := shim.Do(verifyReq, &result)
 		if err != nil {
 			return nil, logical.ErrorResponse(fmt.Sprintf("Okta auth failed: %v", err)), nil, nil
 		}
@@ -193,11 +199,11 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
 		for result.Status == "MFA_CHALLENGE" {
 			switch result.FactorResult {
 			case "WAITING":
-				verifyReq, err := client.NewRequest("POST", requestPath, payload)
+				verifyReq, err := shim.NewRequest("POST", requestPath, payload)
 				if err != nil {
 					return nil, logical.ErrorResponse(fmt.Sprintf("okta auth failed creating verify request: %v", err)), nil, nil
 				}
-				rsp, err := client.Do(verifyReq, &result)
+				rsp, err := shim.Do(verifyReq, &result)
 				if err != nil {
 					return nil, logical.ErrorResponse(fmt.Sprintf("Okta auth failed checking loop: %v", err)), nil, nil
 				}
@@ -252,7 +258,8 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
 
 	var allGroups []string
 	// Only query the Okta API for group membership if we have a token
-	if cfg.Token != "" {
+	client := shim.Client()
+	if client != nil {
 		oktaGroups, err := b.getOktaGroups(client, &result.Embedded.User)
 		if err != nil {
 			return nil, logical.ErrorResponse(fmt.Sprintf("okta failure retrieving groups: %v", err)), nil, nil
@@ -302,15 +309,12 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
 }
 
 func (b *backend) getOktaGroups(client *okta.Client, user *okta.User) ([]string, error) {
-	rsp, err := client.Users.PopulateGroups(user)
+	groups, _, err := client.User.ListUserGroups(user.Id, nil)
 	if err != nil {
 		return nil, err
 	}
-	if rsp == nil {
-		return nil, fmt.Errorf("okta auth method unexpected failure")
-	}
-	oktaGroups := make([]string, 0, len(user.Groups))
-	for _, group := range user.Groups {
+	oktaGroups := make([]string, 0, len(groups))
+	for _, group := range groups {
 		oktaGroups = append(oktaGroups, group.Profile.Name)
 	}
 	if b.Logger().IsDebug() {

builtin/credential/okta/backend_test.go

@@ -6,14 +6,12 @@ import (
 	"os"
 	"strings"
 	"testing"
+	"time"
 
 	log "github.com/hashicorp/go-hclog"
+	logicaltest "github.com/hashicorp/vault/helper/testhelpers/logical"
 	"github.com/hashicorp/vault/sdk/helper/logging"
 	"github.com/hashicorp/vault/sdk/helper/policyutil"
-
-	"time"
-
-	logicaltest "github.com/hashicorp/vault/helper/testhelpers/logical"
 	"github.com/hashicorp/vault/sdk/logical"
 )
 
@@ -36,31 +34,44 @@ func TestBackend_Config(t *testing.T) {
 	token := os.Getenv("OKTA_API_TOKEN")
 
 	configData := map[string]interface{}{
-		"organization": os.Getenv("OKTA_ORG"),
-		"base_url":     "oktapreview.com",
+		"org_name": os.Getenv("OKTA_ORG"),
+		"base_url": "oktapreview.com",
 	}
 
 	updatedDuration := time.Hour * 1
 	configDataToken := map[string]interface{}{
-		"token": token,
-		"ttl":   "1h",
+		"api_token": token,
+		"token_ttl": "1h",
 	}
 
 	logicaltest.Test(t, logicaltest.TestCase{
-		AcceptanceTest: true,
-		PreCheck:       func() { testAccPreCheck(t) },
-		LogicalBackend: b,
+		AcceptanceTest:    true,
+		PreCheck:          func() { testAccPreCheck(t) },
+		CredentialBackend: b,
 		Steps: []logicaltest.TestStep{
 			testConfigCreate(t, configData),
+			// 2. Login with bad password, expect failure (E0000004=okta auth failure).
 			testLoginWrite(t, username, "wrong", "E0000004", 0, nil),
-			testLoginWrite(t, username, password, "user is not a member of any authorized policy", 0, nil),
+			// 3. Make our user belong to two groups and have one user-specific policy.
 			testAccUserGroups(t, username, "local_grouP,lOcal_group2", []string{"user_policy"}),
+			// 4. Create the group local_group, assign it a single policy.
 			testAccGroups(t, "local_groUp", "loCal_group_policy"),
+			// 5. Login with good password, expect user to have their user-specific
+			// policy and the policy of the one valid group they belong to.
 			testLoginWrite(t, username, password, "", defaultLeaseTTLVal, []string{"local_group_policy", "user_policy"}),
+			// 6. Create the group everyone, assign it two policies.  This is a
+			// magic group name in okta that always exists and which every
+			// user automatically belongs to.
 			testAccGroups(t, "everyoNe", "everyone_grouP_policy,eveRy_group_policy2"),
+			// 7. Login as before, expect same result
 			testLoginWrite(t, username, password, "", defaultLeaseTTLVal, []string{"local_group_policy", "user_policy"}),
+			// 8. Add API token so we can lookup groups
 			testConfigUpdate(t, configDataToken),
 			testConfigRead(t, token, configData),
+			// 10. Login should now lookup okta groups; since all okta users are
+			// in the "everyone" group, that should be returned; since we
+			// defined policies attached to the everyone group, we should now
+			// see those policies attached to returned vault token.
 			testLoginWrite(t, username, password, "", updatedDuration, []string{"everyone_group_policy", "every_group_policy2", "local_group_policy", "user_policy"}),
 			testAccGroups(t, "locAl_group2", "testgroup_group_policy"),
 			testLoginWrite(t, username, password, "", updatedDuration, []string{"everyone_group_policy", "every_group_policy2", "local_group_policy", "testgroup_group_policy", "user_policy"}),
@@ -81,6 +92,9 @@ func testLoginWrite(t *testing.T, username, password, reason string, expectedTTL
 				if reason == "" || !strings.Contains(resp.Error().Error(), reason) {
 					return resp.Error()
 				}
+			} else if reason != "" {
+				return fmt.Errorf("expected error containing %q, got no error", reason)
+
 			}
 
 			if resp.Auth != nil {
@@ -124,7 +138,7 @@ func testConfigRead(t *testing.T, token string, d map[string]interface{}) logica
 				return resp.Error()
 			}
 
-			if resp.Data["organization"] != d["organization"] {
+			if resp.Data["org_name"] != d["org_name"] {
 				return fmt.Errorf("org mismatch expected %s but got %s", d["organization"], resp.Data["Org"])
 			}
 

builtin/credential/okta/path_config.go

@@ -3,15 +3,16 @@ package okta
 import (
 	"context"
 	"fmt"
+	"github.com/hashicorp/go-cleanhttp"
+	"net/http"
 	"net/url"
-
 	"time"
 
-	"github.com/chrismalek/oktasdk-go/okta"
-	cleanhttp "github.com/hashicorp/go-cleanhttp"
+	oktaold "github.com/chrismalek/oktasdk-go/okta"
 	"github.com/hashicorp/vault/sdk/framework"
 	"github.com/hashicorp/vault/sdk/helper/tokenutil"
 	"github.com/hashicorp/vault/sdk/logical"
+	oktanew "github.com/okta/okta-sdk-golang/okta"
 )
 
 const (
@@ -266,8 +267,46 @@ func (b *backend) pathConfigExistenceCheck(ctx context.Context, req *logical.Req
 	return cfg != nil, nil
 }
 
+type oktaShim interface {
+	Client() *oktanew.Client
+	NewRequest(method string, url string, body interface{}) (*http.Request, error)
+	Do(req *http.Request, v interface{}) (interface{}, error)
+}
+
+type oktaShimNew struct {
+	client *oktanew.Client
+}
+
+func (new *oktaShimNew) Client() *oktanew.Client {
+	return new.client
+}
+
+func (new *oktaShimNew) NewRequest(method string, url string, body interface{}) (*http.Request, error) {
+	return new.client.GetRequestExecutor().NewRequest(method, url, body)
+}
+
+func (new *oktaShimNew) Do(req *http.Request, v interface{}) (interface{}, error) {
+	return new.client.GetRequestExecutor().Do(req, v)
+}
+
+type oktaShimOld struct {
+	client *oktaold.Client
+}
+
+func (new *oktaShimOld) Client() *oktanew.Client {
+	return nil
+}
+
+func (new *oktaShimOld) NewRequest(method string, url string, body interface{}) (*http.Request, error) {
+	return new.client.NewRequest(method, url, body)
+}
+
+func (new *oktaShimOld) Do(req *http.Request, v interface{}) (interface{}, error) {
+	return new.client.Do(req, v)
+}
+
 // OktaClient creates a basic okta client connection
-func (c *ConfigEntry) OktaClient() *okta.Client {
+func (c *ConfigEntry) OktaClient() (oktaShim, error) {
 	baseURL := defaultBaseURL
 	if c.Production != nil {
 		if !*c.Production {
@@ -278,9 +317,20 @@ func (c *ConfigEntry) OktaClient() *okta.Client {
 		baseURL = c.BaseURL
 	}
 
-	// We validate config on input and errors are only returned when parsing URLs
-	client, _ := okta.NewClientWithDomain(cleanhttp.DefaultClient(), c.Org, baseURL, c.Token)
-	return client
+	if c.Token != "" {
+		client, err := oktanew.NewClient(context.Background(),
+			oktanew.WithOrgUrl("https://"+c.Org+"."+baseURL),
+			oktanew.WithToken(c.Token))
+		if err != nil {
+			return nil, err
+		}
+		return &oktaShimNew{client}, nil
+	}
+	client, err := oktaold.NewClientWithDomain(cleanhttp.DefaultClient(), c.Org, baseURL, "")
+	if err != nil {
+		return nil, err
+	}
+	return &oktaShimOld{client}, nil
 }
 
 // ConfigEntry for Okta

go.mod

@@ -109,6 +109,7 @@ require (
 	github.com/ncw/swift v1.0.47
 	github.com/nwaples/rardecode v1.0.0 // indirect
 	github.com/oklog/run v1.0.0
+	github.com/okta/okta-sdk-golang v1.0.1
 	github.com/oracle/oci-go-sdk v12.5.0+incompatible
 	github.com/ory/dockertest v3.3.5+incompatible
 	github.com/patrickmn/go-cache v2.1.0+incompatible

go.sum

@@ -214,6 +214,8 @@ github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/me
 github.com/go-test/deep v1.0.2-0.20181118220953-042da051cf31/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
 github.com/go-test/deep v1.0.2 h1:onZX1rnHT3Wv6cqNgYyFOOlgVKJrksuCMCRvJStbMYw=
 github.com/go-test/deep v1.0.2/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
+github.com/go-yaml/yaml v2.1.0+incompatible h1:RYi2hDdss1u4YE7GwixGzWwVo47T8UQwnTLB6vQiq+o=
+github.com/go-yaml/yaml v2.1.0+incompatible/go.mod h1:w2MrLa16VYP0jy6N7M5kHaCkaLENm+P+Tv+MfurjSw0=
 github.com/gocql/gocql v0.0.0-20190402132108-0e1d5de854df h1:fwXmhM0OqixzJDOGgTSyNH9eEDij9uGTXwsyWXvyR0A=
 github.com/gocql/gocql v0.0.0-20190402132108-0e1d5de854df/go.mod h1:4Fw1eo5iaEhDUs8XyuhSVCVy52Jq3L+/3GJgYkwc+/0=
 github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
@@ -453,6 +455,8 @@ github.com/jtolds/gls v4.2.1+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVY
 github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo=
 github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU=
 github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w=
+github.com/kelseyhightower/envconfig v1.3.0 h1:IvRS4f2VcIQy6j4ORGIf9145T/AsUB+oY8LyvN8BXNM=
+github.com/kelseyhightower/envconfig v1.3.0/go.mod h1:cccZRl6mQpaq41TPp5QxidR+Sa3axMbJDNb//FQX6Gg=
 github.com/keybase/go-crypto v0.0.0-20190403132359-d65b6b94177f h1:Gsc9mVHLRqBjMgdQCghN9NObCcRncDqxJvBvEaIIQEo=
 github.com/keybase/go-crypto v0.0.0-20190403132359-d65b6b94177f/go.mod h1:ghbZscTyKdM07+Fw3KSi0hcJm+AlEUWj8QLlPtijN/M=
 github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
@@ -529,6 +533,8 @@ github.com/nwaples/rardecode v1.0.0 h1:r7vGuS5akxOnR4JQSkko62RJ1ReCMXxQRPtxsiFMB
 github.com/nwaples/rardecode v1.0.0/go.mod h1:5DzqNKiOdpKKBH87u8VlvAnPZMXcGRhxWkRpHbbfGS0=
 github.com/oklog/run v1.0.0 h1:Ru7dDtJNOyC66gQ5dQmaCa0qIsAUFY3sFpK1Xk8igrw=
 github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA=
+github.com/okta/okta-sdk-golang v1.0.1 h1:1DGm5+h2JvfdHz07yVVM7+LgUVSwxnk+6RoLUOB6CwI=
+github.com/okta/okta-sdk-golang v1.0.1/go.mod h1:8k//sN2mFTq8Ayo90DqGbcumCkSmYjF0+2zkIbZysec=
 github.com/olekukonko/tablewriter v0.0.0-20170122224234-a0225b3f23b5/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo=
 github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
@@ -558,6 +564,7 @@ github.com/oxtoacart/bpool v0.0.0-20150712133111-4e1c5567d7c2/go.mod h1:L3UMQOTh
 github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
 github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY=
 github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
+github.com/patrickmn/go-cache v0.0.0-20180815053127-5633e0862627/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ=
 github.com/patrickmn/go-cache v2.1.0+incompatible h1:HRMgzkcYKYpi3C8ajMPV8OFXaaRUnok+kx1WdO15EQc=
 github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ=
 github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY=