Add unit tests for ensureAccessTokenClaims

go.mod

@@ -4,7 +4,7 @@ go 1.18
 
 require (
 	github.com/Azure/azure-sdk-for-go v63.1.0+incompatible
-	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.7.1
+	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.1
 	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.3.1
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/cosmos/armcosmos/v2 v2.5.0
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v2 v2.2.1
@@ -75,7 +75,7 @@ require (
 	github.com/ugorji/go/codec v1.2.7
 	github.com/vincent-petithory/dataurl v1.0.0
 	golang.org/x/crypto v0.17.0
-	golang.org/x/net v0.17.0
+	golang.org/x/net v0.19.0
 	golang.org/x/oauth2 v0.10.0
 	golang.org/x/sync v0.3.0
 	golang.org/x/text v0.14.0
@@ -95,7 +95,7 @@ require (
 )
 
 require (
-	github.com/Azure/azure-sdk-for-go/sdk/internal v1.3.0 // indirect
+	github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.1 // indirect
 	github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
 	github.com/Azure/go-autorest v14.2.0+incompatible // indirect
 	github.com/Azure/go-autorest/logger v0.2.1 // indirect

go.sum

@@ -567,12 +567,12 @@ git.sr.ht/~sbinet/gg v0.3.1/go.mod h1:KGYtlADtqsqANL9ueOFkWymvzUvLMQllU5Ixo+8v3p
 github.com/14rcole/gopopulate v0.0.0-20180821133914-b175b219e774 h1:SCbEWT58NSt7d2mcFdvxC9uyrdcTfvBbPLThhkDmXzg=
 github.com/Azure/azure-sdk-for-go v63.1.0+incompatible h1:yNC7qlSUWVF8p0TzxdmWW1FJ3DdIA+0Pge41IU/2+9U=
 github.com/Azure/azure-sdk-for-go v63.1.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
-github.com/Azure/azure-sdk-for-go/sdk/azcore v1.7.1 h1:/iHxaJhsFr0+xVFfbMr5vxz848jyiWuIEDhYq3y5odY=
-github.com/Azure/azure-sdk-for-go/sdk/azcore v1.7.1/go.mod h1:bjGvMhVMb+EEm3VRNQawDMUyMMjo+S5ewNjflkep/0Q=
+github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.1 h1:lGlwhPtrX6EVml1hO0ivjkUxsSyl4dsiw9qcA1k/3IQ=
+github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.1/go.mod h1:RKUqNu35KJYcVG/fqTRqmuXJZYNhYkBrnC/hX7yGbTA=
 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.3.1 h1:LNHhpdK7hzUcx/k1LIcuh5k7k1LGIWLQfCjaneSj7Fc=
 github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.3.1/go.mod h1:uE9zaUfEQT/nbQjVi2IblCG9iaLtZsuYZ8ne+PuQ02M=
-github.com/Azure/azure-sdk-for-go/sdk/internal v1.3.0 h1:sXr+ck84g/ZlZUOZiNELInmMgOsuGwdjjVkEIde0OtY=
-github.com/Azure/azure-sdk-for-go/sdk/internal v1.3.0/go.mod h1:okt5dMMTOFjX/aovMlrjvvXoPMBVSPzk9185BT0+eZM=
+github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.1 h1:6oNBlSdi1QqM1PNW7FPA6xOGA5UNsXnkaYZz9vdPGhA=
+github.com/Azure/azure-sdk-for-go/sdk/internal v1.5.1/go.mod h1:s4kgfzA0covAXNicZHDMN58jExvcng2mC/DepXiF1EI=
 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/cosmos/armcosmos/v2 v2.5.0 h1:FTNvxTFH/08JBmhcbL5lmLaGYVXokZM6Ni92Mqr+gSg=
 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/cosmos/armcosmos/v2 v2.5.0/go.mod h1:T0ryqIz5h5qg4HOBni+VeRn24alSqOx1Se1IAwUByOk=
 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/internal v1.1.2 h1:mLY+pNLjCUeKhgnAJWAKhEUQM+RJQo2H1fuGSw1Ky1E=
@@ -1727,8 +1727,9 @@ golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
 golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc=
 golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns=
 golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
-golang.org/x/net v0.17.0 h1:pVaXccu2ozPjCXewfr1S7xza/zcXTity9cCdXQYSjIM=
 golang.org/x/net v0.17.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE=
+golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c=
+golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U=
 golang.org/x/oauth2 v0.0.0-20220223155221-ee480838109b/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
 golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
 golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=

pkg/validate/openshiftcluster_validatedynamic.go

@@ -55,6 +55,17 @@ type openShiftClusterDynamicValidator struct {
 	fpAuthorizer    autorest.Authorizer
 }
 
+// ensureAccessTokenClaims can detect an error when the service principal (fp, cluster sp) has accidentally deleted from
+// the tenant. Using the token with ARM will result in the error as follows
+//
+//	Lack of an altsecid, puid or oid claim in the token. Continuing would
+//	subsequently cause the ARM error `Code="InvalidAuthenticationToken"
+//	Message="The received access token is not valid: at least one of the
+//	claims 'puid' or 'altsecid' or 'oid' should be present. If you are
+//	accessing as an application please make sure service principal is
+//	properly created in the tenant."`.  I think this can be returned when
+//	the service principal associated with the application hasn't yet
+//	caught up with the application itself.
 func ensureAccessTokenClaims(ctx context.Context, spTokenCredential azcore.TokenCredential, scopes []string) error {
 	options := policy.TokenRequestOptions{Scopes: scopes}
 	token, err := spTokenCredential.GetToken(ctx, options)
@@ -73,19 +84,6 @@ func ensureAccessTokenClaims(ctx context.Context, spTokenCredential azcore.Token
 			"The provided service principal generated an invalid token.")
 	}
 
-	// XXX Unclear if this check is still required, as it was originally
-	//     implemented for ADAL authentication with the comment:
-	//
-	//     Lack of an altsecid, puid or oid claim in the token. Continuing would
-	//     subsequently cause the ARM error `Code="InvalidAuthenticationToken"
-	//     Message="The received access token is not valid: at least one of the
-	//     claims 'puid' or 'altsecid' or 'oid' should be present. If you are
-	//     accessing as an application please make sure service principal is
-	//     properly created in the tenant."`.  I think this can be returned when
-	//     the service principal associated with the application hasn't yet
-	//     caught up with the application itself.
-	//
-	//     (source: commit id 52dff30f31bad63cc4e46bbf701437756a6da83a)
 	for _, claim := range []string{"altsecid", "oid", "puid"} {
 		if _, found := claims[claim]; found {
 			return nil

pkg/validate/openshiftcluster_validatedynamic_test.go

@@ -0,0 +1,79 @@
+package validate
+
+// Copyright (c) Microsoft Corporation.
+// Licensed under the Apache License 2.0.
+//
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/fake"
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
+
+	utilerror "github.com/Azure/ARO-RP/test/util/error"
+)
+
+type token struct {
+	accesstoken string
+}
+
+func (t token) GetToken(context.Context, policy.TokenRequestOptions) (azcore.AccessToken, error) {
+	return azcore.AccessToken{Token: t.accesstoken, ExpiresOn: time.Now().Add(24 * time.Hour)}, nil
+}
+
+func TestEnsureAccessTokenClaims(t *testing.T) {
+	// generated, not real tokens
+	hasOid := "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJia0Biay5jb20iLCJuYW1lIjoiQmlsbHkgS2VpbGxvciIsICJvaWQiOiAiMTExMTEiLCAiaWF0IjoxNTQ2MzAwODAwLCJleHAiOjE4OTM0NTYwMDB9."
+	hasAltsecid := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwgImFsdHNlY2lkIjoiYmlsbHkga2VpbGxvciIsICJuYW1lIjoiSm9obiBEb2UiLCJpYXQiOjE1MTYyMzkwMjJ9."
+	hasPuid := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwgInB1aWQiOiJiaWxseSBrZWlsbG9yIiwgIm5hbWUiOiJKb2huIERvZSIsImlhdCI6MTUxNjIzOTAyMn0."
+	for _, tt := range []struct {
+		name         string
+		tokenFactory func() azcore.TokenCredential
+		wantErr      string
+	}{
+		{
+			name: "server error",
+			tokenFactory: func() azcore.TokenCredential {
+				cred := fake.TokenCredential{}
+				cred.SetError(errors.New("Unable to establish a connection to ARM"))
+				return &cred
+			},
+			wantErr: "Unable to establish a connection to ARM",
+		},
+		{
+			name: "invalid Token, no required claims at all",
+			tokenFactory: func() azcore.TokenCredential {
+				return &fake.TokenCredential{}
+			},
+			wantErr: "400: InvalidServicePrincipalToken: properties.servicePrincipalProfile: The provided service principal generated an invalid token.",
+		},
+		{
+			name: "valid token: has oid",
+			tokenFactory: func() azcore.TokenCredential {
+				return token{hasOid}
+			},
+		}, {
+			name: "valid token: has altsecid",
+			tokenFactory: func() azcore.TokenCredential {
+				return token{hasAltsecid}
+			},
+		}, {
+			name: "valid token: has puid",
+			tokenFactory: func() azcore.TokenCredential {
+				return token{hasPuid}
+			},
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
+			scopes := []string{}
+			tok := tt.tokenFactory()
+
+			err := ensureAccessTokenClaims(ctx, tok, scopes)
+			utilerror.AssertErrorMessage(t, err, tt.wantErr)
+		})
+	}
+}