Merge pull request #68 from allcloud-io/bugfix/okta_arn_ordering

Bugfix/okta arn ordering

saml/saml.go

@@ -5,6 +5,7 @@ import (
 	"encoding/xml"
 	"errors"
 	"fmt"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -62,9 +63,30 @@ func extractArns(attrs []saml.Attribute) (arns []ARN) {
 				if len(av.Value) == 0 {
 					return
 				}
+
+				// Verify we have one of the following formats:
+				// 1. arn:aws:iam::xxxxxxxxxxxx:role/MyRole,arn:aws:iam::xxxxxxxxxxxx:saml-provider/MyProvider
+				// 2. arn:aws:iam::xxxxxxxxxxxx:saml-provider/MyProvider,arn:aws:iam::xxxxxxxxxxxx:role/MyRole
+				// Error otherwise.
 				components := strings.Split(av.Value, ",")
+				if len(components) != 2 {
+					// Wrong number of components - move on
+					continue
+				}
 
-				arns = append(arns, ARN{components[0], components[1]})
+				// Prepare patterns
+				role := regexp.MustCompile(`^arn:aws:iam::\d+:role/\S+$`)
+				idp := regexp.MustCompile(`^arn:aws:iam::\d+:saml-provider/\S+$`)
+
+				if role.MatchString(components[0]) && idp.MatchString(components[1]) {
+					// First component is role
+					arns = append(arns, ARN{components[0], components[1]})
+				} else if role.MatchString(components[1]) && idp.MatchString(components[0]) {
+					// First component is IdP
+					arns = append(arns, ARN{components[1], components[0]})
+				} else {
+					// Malformed ARNs - move on
+				}
 			}
 
 			return

saml/saml_test.go

@@ -30,15 +30,31 @@ func TestDecode(t *testing.T) {
 
 func TestGet(t *testing.T) {
 	for _, test := range []struct {
-		name        string
-		path        string
-		expectRole  string
-		expectError bool
+		name           string
+		path           string
+		expectProvider string
+		expectRole     string
+		expectError    bool
 	}{
-		{"Single ARN", "testdata/single-arn-response", "arn:aws:iam::123456789012:role/OneLogin-MyRole", false},
-		//{"Many ARNs", "testdata/valid-response", "", false},         // will ask questions
-		{"No ARNs", "testdata/no-arns-resonse", "", true},
-		{"No ARN value", "testdata/no-arn-value-response", "", true},
+		{
+			"Single ARN",
+			"testdata/single-arn-response",
+			"arn:aws:iam::123456789012:saml-provider/OneLogin-MyProvider",
+			"arn:aws:iam::123456789012:role/OneLogin-MyRole",
+			false,
+		},
+		//{"Many ARNs", "testdata/valid-response", "", "", false},         // will ask questions
+		{"No ARNs", "testdata/no-arns-resonse", "", "", true},
+		{"No ARN value", "testdata/no-arn-value-response", "", "", true},
+		{
+			"IdP ARN before role ARN",
+			"testdata/idp-before-role",
+			"arn:aws:iam::123456789012:saml-provider/OneLogin-MyProvider",
+			"arn:aws:iam::123456789012:role/OneLogin-MyRole",
+			false,
+		},
+		{"Too many ARN components", "testdata/too-many-components", "", "", true},
+		{"Malformed ARN components", "testdata/malformed-components", "", "", true},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			b, _ := ioutil.ReadFile(test.path)
@@ -51,6 +67,9 @@ func TestGet(t *testing.T) {
 				t.Errorf("unexpected error %+v", err)
 			}
 
+			if test.expectProvider != arn.Provider {
+				t.Errorf("expected %q, received %q", test.expectProvider, arn.Provider)
+			}
 			if test.expectRole != arn.Role {
 				t.Errorf("expected %q, received %q", test.expectRole, arn.Role)
 			}

saml/testdata/idp-before-role

@@ -0,0 +1 @@
+PD94bWwgdmVyc2lvbj0iMS4wIj8+CjxzYW1scDpSZXNwb25zZSB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIj4KICAgIDxzYW1sOkFzc2VydGlvbj4KICAgICAgICA8c2FtbDpBdHRyaWJ1dGVTdGF0ZW1lbnQ+CiAgICAgICAgICAgIDxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJodHRwczovL2F3cy5hbWF6b24uY29tL1NBTUwvQXR0cmlidXRlcy9Sb2xlIiBOYW1lRm9ybWF0PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXR0cm5hbWUtZm9ybWF0OmJhc2ljIj4KICAgICAgICAgICAgICAgIDxzYW1sOkF0dHJpYnV0ZVZhbHVlIHhtbG5zOnhzaT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEtaW5zdGFuY2UiIHhzaTp0eXBlPSJ4czpzdHJpbmciPmFybjphd3M6aWFtOjoxMjM0NTY3ODkwMTI6c2FtbC1wcm92aWRlci9PbmVMb2dpbi1NeVByb3ZpZGVyLGFybjphd3M6aWFtOjoxMjM0NTY3ODkwMTI6cm9sZS9PbmVMb2dpbi1NeVJvbGU8L3NhbWw6QXR0cmlidXRlVmFsdWU+CiAgICAgICAgICAgIDwvc2FtbDpBdHRyaWJ1dGU+CiAgICAgICAgPC9zYW1sOkF0dHJpYnV0ZVN0YXRlbWVudD4KICAgIDwvc2FtbDpBc3NlcnRpb24+Cjwvc2FtbHA6UmVzcG9uc2U+Cg==

saml/testdata/malformed-components

@@ -0,0 +1 @@
+PD94bWwgdmVyc2lvbj0iMS4wIj8+CjxzYW1scDpSZXNwb25zZSB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIj4KICAgIDxzYW1sOkFzc2VydGlvbj4KICAgICAgICA8c2FtbDpBdHRyaWJ1dGVTdGF0ZW1lbnQ+CiAgICAgICAgICAgIDxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJodHRwczovL2F3cy5hbWF6b24uY29tL1NBTUwvQXR0cmlidXRlcy9Sb2xlIiBOYW1lRm9ybWF0PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXR0cm5hbWUtZm9ybWF0OmJhc2ljIj4KICAgICAgICAgICAgICAgIDxzYW1sOkF0dHJpYnV0ZVZhbHVlIHhtbG5zOnhzaT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEtaW5zdGFuY2UiIHhzaTp0eXBlPSJ4czpzdHJpbmciPm9vcHMsb2hubzwvc2FtbDpBdHRyaWJ1dGVWYWx1ZT4KICAgICAgICAgICAgPC9zYW1sOkF0dHJpYnV0ZT4KICAgICAgICA8L3NhbWw6QXR0cmlidXRlU3RhdGVtZW50PgogICAgPC9zYW1sOkFzc2VydGlvbj4KPC9zYW1scDpSZXNwb25zZT4K

saml/testdata/too-many-components

@@ -0,0 +1 @@
+PD94bWwgdmVyc2lvbj0iMS4wIj8+CjxzYW1scDpSZXNwb25zZSB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIj4KICAgIDxzYW1sOkFzc2VydGlvbj4KICAgICAgICA8c2FtbDpBdHRyaWJ1dGVTdGF0ZW1lbnQ+CiAgICAgICAgICAgIDxzYW1sOkF0dHJpYnV0ZSBOYW1lPSJodHRwczovL2F3cy5hbWF6b24uY29tL1NBTUwvQXR0cmlidXRlcy9Sb2xlIiBOYW1lRm9ybWF0PSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXR0cm5hbWUtZm9ybWF0OmJhc2ljIj4KICAgICAgICAgICAgICAgIDxzYW1sOkF0dHJpYnV0ZVZhbHVlIHhtbG5zOnhzaT0iaHR0cDovL3d3dy53My5vcmcvMjAwMS9YTUxTY2hlbWEtaW5zdGFuY2UiIHhzaTp0eXBlPSJ4czpzdHJpbmciPmFybjphd3M6aWFtOjoxMjM0NTY3ODkwMTI6cm9sZS9PbmVMb2dpbi1NeVJvbGUsYXJuOmF3czppYW06OjEyMzQ1Njc4OTAxMjpzYW1sLXByb3ZpZGVyL09uZUxvZ2luLU15UHJvdmlkZXIsb29wczwvc2FtbDpBdHRyaWJ1dGVWYWx1ZT4KICAgICAgICAgICAgPC9zYW1sOkF0dHJpYnV0ZT4KICAgICAgICA8L3NhbWw6QXR0cmlidXRlU3RhdGVtZW50PgogICAgPC9zYW1sOkFzc2VydGlvbj4KPC9zYW1scDpSZXNwb25zZT4K