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

[pr] Okta Challenge Preferences #93

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,16 @@ The `--url` flag is the app's **embed link**. This can be retrieved as an Okta u
the URL of an app on the Okta web UI. The same can also be retrieved as an administrator by
clicking an app in the **Applications** view. The embed link is on the **General** tab.

The `--challenges` flag gives you the option to select which challenges in what order to use.
This flag is optional.
Currently only 2 challenge types are supported:

- push
- token:software:totp

Clisso will use the challenges in the preference you specified.
You can make a list by specifiying the flag multiple times like `--challenges push --challenges token:software:totp`

>NOTE: An Okta embed link must not contain an HTTP query, only the base URL. For AWS apps, the link
should end with `/137`.

Expand Down
11 changes: 7 additions & 4 deletions cmd/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ var providerDuration int

// Okta
Copy link

Choose a reason for hiding this comment

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

Although not precisely the same, could the OktaProviderConfig from config/config.go be used here, and the OneLoginProviderConfig be used above?

var baseURL string
var challenges []string

func init() {
// OneLogin
Expand All @@ -43,6 +44,7 @@ func init() {

// Okta
cmdProvidersCreateOkta.Flags().StringVar(&baseURL, "base-url", "", "Okta base URL")
cmdProvidersCreateOkta.Flags().StringArrayVar(&challenges, "challenges", []string{"push", "token:software:totp"}, "Challenges to be used with preference")
Copy link

Choose a reason for hiding this comment

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

This line might be better if split to reduce line length.

cmdProvidersCreateOkta.Flags().StringVar(&username, "username", "",
"Don't ask for a username and use this instead")
cmdProvidersCreateOkta.Flags().IntVar(&providerDuration, "duration", 0, "(Optional) Default session duration in seconds")
Expand Down Expand Up @@ -176,10 +178,11 @@ var cmdProvidersCreateOkta = &cobra.Command{
log.Fatalf(color.RedString("Provider '%s' already exists"), name)
}

conf := map[string]string{
"base-url": baseURL,
"type": "okta",
"username": username,
conf := map[string]interface{}{
"base-url": baseURL,
"type": "okta",
"username": username,
"challenges": challenges,
}
if providerDuration != 0 {
// Duration specified - validate value
Expand Down
8 changes: 5 additions & 3 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,20 +77,22 @@ func GetOneLoginApp(app string) (*OneLoginAppConfig, error) {

// OktaProviderConfig represents an Okta provider configuration.
type OktaProviderConfig struct {
BaseURL string
Username string
BaseURL string
Username string
Challenges []string
}

// GetOktaProvider returns a OktaProviderConfig struct containing the configuration for provider p.
func GetOktaProvider(p string) (*OktaProviderConfig, error) {
baseURL := viper.GetString(fmt.Sprintf("providers.%s.base-url", p))
username := viper.GetString(fmt.Sprintf("providers.%s.username", p))
challenges := viper.GetStringSlice(fmt.Sprintf("providers.%s.challenges", p))

if baseURL == "" {
return nil, errors.New("base-url config value must bet set")
}

return &OktaProviderConfig{BaseURL: baseURL, Username: username}, nil
return &OktaProviderConfig{BaseURL: baseURL, Username: username, Challenges: challenges}, nil
}

// OktaAppConfig represents an Okta app configuration.
Expand Down
20 changes: 11 additions & 9 deletions okta/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,20 @@ type GetSessionTokenResponse struct {
StateToken string `json:"stateToken"`
Status string `json:"status"`
Embedded struct {
Factors []struct {
ID string `json:"id"`
Links struct {
Verify struct {
Href string `json:"href"`
} `json:"verify"`
} `json:"_links"`
FactorType string `json:"factorType"`
} `json:"factors"`
Factors []factor `json:"factors"`
} `json:"_embedded"`
}

type factor struct {
Copy link

Choose a reason for hiding this comment

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

This type of cleanup is always good.

ID string `json:"id"`
Links struct {
Verify struct {
Href string `json:"href"`
} `json:"verify"`
} `json:"_links"`
FactorType string `json:"factorType"`
}

// GetSessionToken performs a login operation against the Okta API and returns a session token upon
// successful login.
//
Expand Down
41 changes: 41 additions & 0 deletions okta/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package okta
import (
"fmt"
"log"
"strings"
"time"

"github.com/allcloud-io/clisso/aws"
Expand Down Expand Up @@ -77,6 +78,15 @@ func Get(app, provider string, duration int64) (*aws.Credentials, error) {
st = resp.SessionToken
case StatusMFARequired:
factor := resp.Embedded.Factors[0]

if len(p.Challenges) > 0 {
// use preferred list if available
factor, err = preferredFactor(resp.Embedded.Factors, p.Challenges)
if err != nil {
return nil, err
}
}

stateToken := resp.StateToken

var vfResp *VerifyFactorResponse
Expand Down Expand Up @@ -163,3 +173,34 @@ func Get(app, provider string, duration int64) (*aws.Credentials, error) {

return creds, err
}

func typeMapOf(factors []factor) (m map[string]factor) {
Copy link

Choose a reason for hiding this comment

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

This code all looks good, but I don't see any accompanying unit tests. I strongly recommend adding these to protect the code against future changes that could unintentionally break things; and it may also be a good clean place to start with that if you haven't already as there aren't many dependencies.

m = make(map[string]factor)

for _, v := range factors {
m[v.FactorType] = v
}
return
}

func keys(m map[string]factor) (keys []string) {
keys = make([]string, len(m))

i := 0
for k := range m {
keys[i] = k
i++
}
return
}

func preferredFactor(factors []factor, pref []string) (factor, error) {
fmap := typeMapOf(factors)
for _, p := range pref {
if f, ok := fmap[p]; ok {
return f, nil
}
}

return factor{}, fmt.Errorf("no supported MFA type found in: %v", strings.Join(keys(fmap), ","))
}