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

Upgrade okta lib #8143

Merged
merged 11 commits into from Feb 3, 2020
34 changes: 19 additions & 15 deletions builtin/credential/okta/backend.go
Expand Up @@ -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) {
Expand Down Expand Up @@ -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"`
Expand All @@ -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,
})
Expand All @@ -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(fmt.Sprintf("Okta auth failed: %v (code=%v)", err, oe.ErrorCode)), nil, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: fmt.Sprintf is redundant as ErrorResponse assumes a format string + parameters

}
return nil, logical.ErrorResponse(fmt.Sprintf("Okta auth failed: %v", err)), nil, nil
}
if rsp == nil {
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down
40 changes: 27 additions & 13 deletions builtin/credential/okta/backend_test.go
Expand Up @@ -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"
)

Expand All @@ -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"}),
Expand All @@ -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 {
Expand Down Expand Up @@ -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"])
}

Expand Down
64 changes: 57 additions & 7 deletions builtin/credential/okta/path_config.go
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 {
Expand All @@ -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(),
Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to ask about context handling improvements, but it looks like the Okta SDK doesn't even do anything with the context, so nvm! https://github.com/okta/okta-sdk-golang/blob/7334d45e99bc09e0dbe209ef961aa40827e9d012/okta/okta.go#L55

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
Expand Down
4 changes: 4 additions & 0 deletions go.mod
Expand Up @@ -28,6 +28,7 @@ require (
github.com/cockroachdb/apd v1.1.0 // indirect
github.com/cockroachdb/cockroach-go v0.0.0-20181001143604-e0a95dfd547c
github.com/coreos/go-semver v0.2.0
github.com/coreos/go-systemd v0.0.0-20181012123002-c6f51f82210d // indirect
github.com/denisenkom/go-mssqldb v0.0.0-20190412130859-3b1d194e553a
github.com/dnaeon/go-vcr v1.0.1 // indirect
github.com/dsnet/compress v0.0.1 // indirect
Expand All @@ -47,6 +48,7 @@ require (
github.com/golang/protobuf v1.3.2
github.com/google/go-github v17.0.0+incompatible
github.com/google/go-metrics-stackdriver v0.0.0-20190816035513-b52628e82e2a
github.com/google/go-querystring v1.0.0 // indirect
github.com/grpc-ecosystem/grpc-gateway v1.8.5 // indirect
github.com/hashicorp/consul-template v0.22.0
github.com/hashicorp/consul/api v1.1.0
Expand Down Expand Up @@ -95,6 +97,7 @@ require (
github.com/joyent/triton-go v0.0.0-20190112182421-51ffac552869
github.com/keybase/go-crypto v0.0.0-20190403132359-d65b6b94177f
github.com/kr/pretty v0.1.0
github.com/kr/pty v1.1.3 // indirect
github.com/kr/text v0.1.0
github.com/lib/pq v1.2.0
github.com/mattn/go-colorable v0.1.4
Expand All @@ -109,6 +112,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
Expand Down
7 changes: 7 additions & 0 deletions go.sum
Expand Up @@ -206,6 +206,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.0.0/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
Expand Down Expand Up @@ -438,6 +440,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=
Expand Down Expand Up @@ -515,6 +519,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=
Expand Down Expand Up @@ -545,6 +551,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=
Expand Down