Skip to content

Commit

Permalink
Merge pull request #172 from okta/dont-panic-on-config-errors
Browse files Browse the repository at this point in the history
Return error on configuration validation error
  • Loading branch information
bogdanprodan-okta authored Oct 20, 2020
2 parents 55609f2 + b2836f6 commit fe09919
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 71 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/vendor
.okta
.idea
9 changes: 7 additions & 2 deletions okta/okta.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
"fmt"
"io/ioutil"
"os/user"
"path"
"path/filepath"
"runtime"

"github.com/okta/okta-sdk-golang/v2/okta/cache"

Expand Down Expand Up @@ -92,7 +95,7 @@ func NewClient(ctx context.Context, conf ...ConfigSetter) (context.Context, *Cli

config, err := validateConfig(config)
if err != nil {
panic(err)
return nil, nil, err
}

c := &Client{}
Expand Down Expand Up @@ -188,8 +191,10 @@ func readConfigFromSystem(c config) *config {
return conf
}

// read config from the project's root directory
func readConfigFromApplication(c config) *config {
conf, err := readConfigFromFile(".okta.yaml", c)
_, b, _, _ := runtime.Caller(0)
conf, err := readConfigFromFile(filepath.Join(filepath.Dir(path.Join(path.Dir(b))), ".okta.yaml"), c)

if err != nil {
return &c
Expand Down
2 changes: 1 addition & 1 deletion openapi/generator/templates/okta.go.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func NewClient(ctx context.Context, conf ...ConfigSetter) (context.Context, *Cli

config, err := validateConfig(config)
if err != nil {
panic(err)
return nil, err
}

c := &Client{}
Expand Down
27 changes: 9 additions & 18 deletions tests/integration/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@ import (
"testing"
"time"

"github.com/okta/okta-sdk-golang/v2/okta/query"

"github.com/okta/okta-sdk-golang/v2/okta"
"github.com/okta/okta-sdk-golang/v2/okta/query"
"github.com/okta/okta-sdk-golang/v2/tests"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -77,13 +76,13 @@ func Test_can_get_a_user(t *testing.T) {
}

func Test_can_activate_a_user(t *testing.T) {
ctx, client, _ := tests.NewClient(context.TODO())
ctx, client, err := tests.NewClient(context.TODO())
require.NoError(t, err)
//Create user with credentials → POST /api/v1/users?activate=false
p := &okta.PasswordCredential{
Value: "Abcd1234",
}
uc := &okta.UserCredentials{
Password: p,
Password: &okta.PasswordCredential{
Value: "Abcd1234",
},
}
profile := okta.UserProfile{}
profile["firstName"] = "John"
Expand All @@ -105,17 +104,9 @@ func Test_can_activate_a_user(t *testing.T) {
assert.NotEmpty(t, token, "Token was not provided")
assert.IsType(t, &okta.UserActivationToken{}, token, "Activation did not return correct type")

// Verify that the user is in the list of ACTIVE users with query parameter → GET /api/v1/users?filter=status eq "ACTIVE"
filter := query.NewQueryParams(query.WithFilter("status eq \"ACTIVE\""))
users, _, err := client.User.ListUsers(ctx, filter)
require.NoError(t, err, "Could not get active users")
found := false
for _, u := range users {
if user.Id == u.Id {
found = true
}
}
assert.True(t, found, "The user was not found")
crUser, _, err := client.User.GetUser(ctx, user.Id)
require.NoError(t, err, "Could not get user by ID")
assert.NotNil(t, crUser.Activated, "users activation time is missing")

// Deactivate the user → POST /api/v1/users/{{userId}}/lifecycle/deactivate
_, err = client.User.DeactivateUser(ctx, user.Id, nil)
Expand Down
84 changes: 36 additions & 48 deletions tests/unit/client_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,74 +26,62 @@ import (
"github.com/stretchr/testify/assert"
)

func Test_panic_on_empty_url(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithOrgUrl(""))
}, "Does not panic when org url is missing")
func Test_error_on_empty_url(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithOrgUrl(""))
assert.Error(t, err, "Does not error when org url is missing")
}

func Test_panic_when_url_contains_yourOktaDomain(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithOrgUrl("https://{yourOktaDomain}"))
}, "Does not panic when org url contains {yourOktaDomain}")
func Test_error_when_url_contains_yourOktaDomain(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithOrgUrl("https://{yourOktaDomain}"))
assert.Error(t, err, "Does not error when org url contains {yourOktaDomain}")
}

func Test_panic_when_url_contains_admin_okta_com(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithOrgUrl("https://test-admin.okta.com"))
}, "Does not panic when org url contains test-admin.okta.com")
func Test_error_when_url_contains_admin_okta_com(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithOrgUrl("https://test-admin.okta.com"))
assert.Error(t, err, "Does not error when org url contains test-admin.okta.com")
}

func Test_panic_when_url_contains_admin_oktapreview_com(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithOrgUrl("https://test-admin.oktapreview.com"))
}, "Does not panic when org url contains test-admin.oktapreview.com")
func Test_error_when_url_contains_admin_oktapreview_com(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithOrgUrl("https://test-admin.oktapreview.com"))
assert.Error(t, err, "Does not error when org url contains test-admin.oktapreview.com")
}

func Test_panic_when_url_contains_admin_okta_emea_com(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithOrgUrl("https://test-admin.okta-emea.com"))
}, "Does not panic when org url contains test-admin.okta-emea.com")
func Test_error_when_url_contains_admin_okta_emea_com(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithOrgUrl("https://test-admin.okta-emea.com"))
assert.Error(t, err, "Does not error when org url contains test-admin.okta-emea.com")
}

func Test_panic_when_url_contains_com_com(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithOrgUrl("https://test.okta.com.com"))
}, "Does not panic when org url contains .com.com")
func Test_error_when_url_contains_com_com(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithOrgUrl("https://test.okta.com.com"))
assert.Error(t, err, "Does not error when org url contains .com.com")
}

func Test_panic_when_url_does_not_begin_with_https(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithTestingDisableHttpsCheck(false), okta.WithOrgUrl("http://test.okta.com"))
}, "Does not panic when url contains only http")
func Test_error_when_url_does_not_begin_with_https(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithTestingDisableHttpsCheck(false), okta.WithOrgUrl("http://test.okta.com"))
assert.Error(t, err, "Does not error when url contains only http")
}

func Test_panic_when_api_token_is_empty(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithToken(""))
}, "Does not panic when api token is empty")
func Test_error_when_api_token_is_empty(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithToken(""))
assert.Error(t, err, "Does not error when api token is empty")
}

func Test_panic_when_api_token_contains_placeholder(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithToken("{apiToken}"))
}, "Does not panic when api token contains {apiToken}")
func Test_error_when_api_token_contains_placeholder(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithToken("{apiToken}"))
assert.Error(t, err, "Does not error when api token contains {apiToken}")
}

func Test_panic_when_authorization_mode_is_not_valid(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithAuthorizationMode("invalid"))
}, "Does not panic when authorization mode is invalid")
func Test_error_when_authorization_mode_is_not_valid(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithAuthorizationMode("invalid"))
assert.Error(t, err, "Does not error when authorization mode is invalid")
}

func Test_does_not_panic_when_authorization_mode_is_valid(t *testing.T) {
assert.NotPanics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithAuthorizationMode("SSWS"))
}, "Should not panic when authorization mode is SSWS")
func Test_does_not_error_when_authorization_mode_is_valid(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithAuthorizationMode("SSWS"))
assert.NoError(t, err, "Should not error when authorization mode is SSWS")
}

func Test_will_panic_if_private_key_authorization_type_with_missing_properties(t *testing.T) {
assert.Panics(t, func() {
_, _, _ = tests.NewClient(context.TODO(), okta.WithAuthorizationMode("PrivateKey"), okta.WithClientId(""))
}, "Does not panic if private key selected with no other required options")
func Test_will_error_if_private_key_authorization_type_with_missing_properties(t *testing.T) {
_, _, err := tests.NewClient(context.TODO(), okta.WithAuthorizationMode("PrivateKey"), okta.WithClientId(""))
assert.Error(t, err, "Does not error if private key selected with no other required options")
}
5 changes: 3 additions & 2 deletions tests/unit/retry_logic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ func Test_Will_Stop_Retrying_Based_On_Max_Retry_Configuration(t *testing.T) {
httpmock.Activate()
defer httpmock.DeactivateAndReset()

ctx, client, _ := tests.NewClient(context.TODO(), okta.WithRequestTimeout(0), okta.WithRateLimitMaxRetries(1))
ctx, client, err := tests.NewClient(context.TODO(), okta.WithRequestTimeout(0), okta.WithRateLimitMaxRetries(1))
require.NoError(t, err)

httpmock.RegisterResponder("GET", "/api/v1/users",
tests.MockResponse(
Expand All @@ -63,7 +64,7 @@ func Test_Will_Stop_Retrying_Based_On_Max_Retry_Configuration(t *testing.T) {
),
)

_, _, err := client.User.ListUsers(ctx, nil)
_, _, err = client.User.ListUsers(ctx, nil)
require.NotNil(t, err, "error was nil, but should have told the user they reached their max retry limit")

httpmock.GetTotalCallCount()
Expand Down

0 comments on commit fe09919

Please sign in to comment.