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.

Sign up for GitHub

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
Merged

Upgrade okta lib #8143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7515806
Update test to use newer field names. Remove obsolete test invalidat…
ncabatoff Jan 10, 2020
0dbb81c
Upgrade to new official Okta sdk lib. Unfortunately it doesn't suppo…
ncabatoff Jan 13, 2020
7e97157
go mod vendor
ncabatoff Jan 13, 2020
6b420bb
Stop using my forked okta lib. Use old unofficial okta lib for no-ap…
ncabatoff Jan 17, 2020
f4fff5a
Drop references to removing no-api-token option in 1.5.
ncabatoff Jan 21, 2020
4167de8
Merge branch 'master' into upgrade-okta-lib
ncabatoff Jan 22, 2020
930ed10
go mod tidy and vendor
ncabatoff Jan 22, 2020
9d2b10c
Remove redundant Sprintf.
ncabatoff Feb 3, 2020
b128a2b
Merge branch 'master' into upgrade-okta-lib
ncabatoff Feb 3, 2020
97f22ee
Post-merge go mod vendor.
ncabatoff Feb 3, 2020
d104872
Merge branch 'master' into upgrade-okta-lib
ncabatoff Feb 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions builtin/credential/okta/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
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("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 {
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
Loading