Merge pull request #316 from AzureAD/release-0.5.1

Release 0.5.1

apps/confidential/confidential_test.go

@@ -5,6 +5,9 @@ package confidential
 
 import (
 	"context"
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -15,55 +18,15 @@ import (
 )
 
 func TestCertFromPEM(t *testing.T) {
-	// pem is generated from: openssl req -newkey rsa:2048 -new -nodes -x509 -keyout key.pem -out cert.pem
-	// This cert is not used anywhere.
-	var pemData = []byte(`
------BEGIN CERTIFICATE-----
-MIICljCCAX4CCQDNgteZ+lJH4zANBgkqhkiG9w0BAQsFADANMQswCQYDVQQGEwJ1
-czAeFw0yMTAxMDQyMzQzNDVaFw0yMTAyMDMyMzQzNDVaMA0xCzAJBgNVBAYTAnVz
-MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1r58wq7JQxM12viLNbdG
-fFizeVQwWRwrx/4CH3kU8jjGovbhkvC/uLWqVGchgATThhGkvNrA92WvdkVwsZMk
-Qf7ZnTA7kemo4VFtgo5XCGEej9gOTW13Evdc/0Flip+RXl3h3Q6BbbB9IFE0c6cS
-3i/v/t8KGpVYQHQzBwTcYehM6eDO8ZjUyUUcJOMXdMCctamig7fMGlziKFahn4dX
-JoiiK4oNKE9okXIAXCTbVkAxxH0hD+5XH1nn5LJnHe0e5DflI3YIiPgmRL5uC89K
-XqmYCKWrq5z2D5k+5fQLmbOcxErBcFCh8hA+Xu0RLT4BHPEgc6iVIqxL4CZi/cke
-uwIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQAAyDbm0Fda0/vY6ZVDML2IbGWbro1w
-nWYNw6wclNU6sx1oeG/k/y2ni7NImPpbFN+594WS6rYHgFdROfeuNgGnjgQCJogk
-+8ouf1R6vFMUAScWeSaFnZmBEgwofWsnIcUKkbDIXbpRhMrkNEcY09VgjmCKhspQ
-iX2bJQTj49XBac9tBaJJYDZ4HgkO4nU7QeEPpvwlELZFoZZXtd3fan+VUyFS2a9n
-gkAMDYoQPGN4tyGFabWws/GlMxelWvqUzpQKmeRPVz+cij75l8eKThEiu0zbjOTD
-Gq81BcY61SPqN02zoPCtqZ/zU6HhaL3x7zUuzhLhNoh83A43UVYEoOOf
------END CERTIFICATE-----
------BEGIN PRIVATE KEY-----
-MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDWvnzCrslDEzXa
-+Is1t0Z8WLN5VDBZHCvH/gIfeRTyOMai9uGS8L+4tapUZyGABNOGEaS82sD3Za92
-RXCxkyRB/tmdMDuR6ajhUW2CjlcIYR6P2A5NbXcS91z/QWWKn5FeXeHdDoFtsH0g
-UTRzpxLeL+/+3woalVhAdDMHBNxh6Ezp4M7xmNTJRRwk4xd0wJy1qaKDt8waXOIo
-VqGfh1cmiKIrig0oT2iRcgBcJNtWQDHEfSEP7lcfWefksmcd7R7kN+UjdgiI+CZE
-vm4Lz0peqZgIpaurnPYPmT7l9AuZs5zESsFwUKHyED5e7REtPgEc8SBzqJUirEvg
-JmL9yR67AgMBAAECggEAAQ/IBh5fGFnL9l0sMwPI8Wxu1ra31njxLnfvAsDSfbAS
-K1QVIWjXSc58HRa1b7CWax9DNTvPoGl8SJVnTTlxAHKGGOTYJoyFLTf91ptlisEQ
-KZ3j1DYqVImsiAaGvfyz90d3imQ795Lby4EbRUcaLMcH5LatkhwS556rcelwPXuq
-M43XaZu5Es4pG0EmzfXplO/awt5HdUDPEAY3yw7QH8D1/l/toLPyiFv37RezkVK9
-ffcUQpH7uH000Gja+JSEHgpWZhE96ac6H0zBtlM1VkMtfBuczz5tkKN/p70fhr8T
-ZXARZqIaF4vx7RkBBzCfhvrgGqxXMuvTaW6N4RDWYQKBgQD1iZ7/xr9qy4cPFSOt
-yBnG5cE6wC7wP8qgr0N7MgAii5OZgx6rtfGIVJDY58CFijnT8jZ5pjNS3p7j/Rzp
-lQJMIwC5kIe/7FU7nmE3ko7Wg+bpd8iWLLIi/QWVFLbS7qVmulTc+CEXWyhAiI2u
-RL/1APjIDFKp9gqtKmwb9erxDwKBgQDf5PbGHuPv5RBLJz9du+M/BIBY+HDltG89
-p3huHHTjkJ5R38oximf2HnV4ygT/p2+ZUD6TJZZw6qou3/GiU5gZbRpg+4LXtQUR
-vV+S2n/t86NG1YcGmM29r8LWqrK9gxLW0X62Fpps16rHSP7kVc4SvmrYwqNzqKlC
-D9QbFYYflQKBgQCKEVzrDuNMNi43+PcbHU4BXeiOFMtQJU7XlDYp7C/PPRU+WVDB
-1Yl/062vioHjlZp259hiB2cMzkoigY3kevnTvksGDZOIBGjZIXIhQbQ4Q+twlP6i
-E3gH3Kdq8T7s1W0EmvplVtGkxImZ4C9rMxWNu4IpW2SQVd4jCZvJDTuTWQKBgQCn
-LGjuCYacSubdlpKDxJSrKwtCY0641P7yhCcx4GGOwR7Vd0mbsAJsDNYduIn+8eAs
-E3SFnl00NqOXmHLth4lcAtDddS5/LZR5aHMCTc+TtoVFkI3faRzF84SBkLchNctN
-RuNbxojLmETVxDU9/Kt/51oUO1CcPWUUBImVJ38b+QKBgQCTbi0nS0n8kC7nlXWN
-QtPcf4UraJAxv1DGq4lnJ8AHSZqqkP5fyjfknSw5ExOPDg4mEHhnnpsvwJuSX00d
-UYUN2ZJXPZeaO0HmbYZ3/vC9bo6KW95PhidEUQpGlKrFY342khjQHJtH67YUThwU
-lQFhpxvPgPNBuxVRnsxoH/sLOA==
------END PRIVATE KEY-----	
-`)
-
+	f, err := os.Open(filepath.Clean("../testdata/test-cert.pem"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+	pemData, err := ioutil.ReadAll(f)
+	if err != nil {
+		t.Fatal(err)
+	}
 	certs, key, err := CertFromPEM(pemData, "")
 	if err != nil {
 		t.Fatalf("TestCertFromPEM: got err == %s, want err == nil", err)

apps/internal/oauth/oauth.go

@@ -99,11 +99,8 @@ func (t *Client) Credential(ctx context.Context, authParams authority.AuthParams
 	if cred.Secret != "" {
 		return t.AccessTokens.FromClientSecret(ctx, authParams, cred.Secret)
 	}
-	var jwt string
-	var err error
-	if cred.Assertion != "" {
-		jwt = cred.Assertion
-	} else if jwt, err = cred.JWT(authParams); err != nil {
+	jwt, err := cred.JWT(authParams)
+	if err != nil {
 		return accesstokens.TokenResponse{}, err
 	}
 	return t.AccessTokens.FromAssertion(ctx, authParams, jwt)
@@ -119,11 +116,8 @@ func (t *Client) OnBehalfOf(ctx context.Context, authParams authority.AuthParams
 		return t.AccessTokens.FromUserAssertionClientSecret(ctx, authParams, authParams.UserAssertion, cred.Secret)
 
 	}
-	var jwt string
-	var err error
-	if cred.Assertion != "" {
-		jwt = cred.Assertion
-	} else if jwt, err = cred.JWT(authParams); err != nil {
+	jwt, err := cred.JWT(authParams)
+	if err != nil {
 		return accesstokens.TokenResponse{}, err
 	}
 	return t.AccessTokens.FromUserAssertionClientCertificate(ctx, authParams, authParams.UserAssertion, jwt)

apps/internal/oauth/ops/accesstokens/accesstokens.go

@@ -43,6 +43,9 @@ const (
 	password      = "password"
 )
 
+// assertionLifetime allows tests to control the expiration time of JWT assertions created by Credential.
+var assertionLifetime = 10 * time.Minute
+
 //go:generate stringer -type=AppType
 
 // AppType is whether the authorization code flow is for a public or confidential client.
@@ -106,14 +109,18 @@ func (c *Credential) JWT(authParams authority.AuthParams) (string, error) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	if c.Expires.Before(time.Now()) && c.Assertion != "" {
+	if c.Expires.After(time.Now()) {
+		return c.Assertion, nil
+	} else if c.Cert == nil || c.Key == nil {
+		// The assertion has expired and this Credential can't generate a new one. The assertion
+		// was presumably provided by the application via confidential.NewCredFromAssertion(). We
+		// return it despite its expiration to maintain the behavior of previous versions, and
+		// because there's no API enabling the application to replace the assertion
+		// (see https://github.com/AzureAD/microsoft-authentication-library-for-go/issues/292).
 		return c.Assertion, nil
 	}
-	// There is no hard requirement on what the expiry time should be.
-	// https://tools.ietf.org/html/rfc7519#section-4.1.4
-	// This just sets a default of 10 mins which means a cached JWT assertion
-	// can be used if it is less that 10 mins old after which we regenerate the assertion.
-	expires := time.Now().Add(10 * time.Minute)
+
+	expires := time.Now().Add(assertionLifetime)
 
 	token := jwt.NewWithClaims(jwt.SigningMethodRS256, jwt.MapClaims{
 		"aud": authParams.Endpoints.TokenEndpoint,

apps/internal/oauth/ops/accesstokens/accesstokens_test.go

@@ -5,10 +5,15 @@ package accesstokens
 
 import (
 	"context"
+	"crypto/x509"
 	"encoding/base64"
+	"encoding/pem"
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"net/url"
+	"os"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
@@ -450,6 +455,56 @@ func TestAccessTokenWithAssertion(t *testing.T) {
 	}
 }
 
+func TestCertAssertionExpiration(t *testing.T) {
+	f, err := os.Open(filepath.Clean("../../../../testdata/test-cert.pem"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+	pemData, err := ioutil.ReadAll(f)
+	if err != nil {
+		t.Fatal(err)
+	}
+	certBlock, rest := pem.Decode(pemData)
+	keyBlock, _ := pem.Decode(rest)
+	cert, err := x509.ParseCertificate(certBlock.Bytes)
+	if err != nil {
+		t.Fatal(err)
+	}
+	key, err := x509.ParsePKCS8PrivateKey(keyBlock.Bytes)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, shouldRegen := range []bool{true, false} {
+		name := "expiredJWT"
+		if !shouldRegen {
+			name = "validJWT"
+		}
+		t.Run(name, func(t *testing.T) {
+			if shouldRegen {
+				defaultLifetime := assertionLifetime
+				defer func() { assertionLifetime = defaultLifetime }()
+				assertionLifetime = time.Nanosecond
+			}
+			cred := Credential{Cert: cert, Key: key}
+			authParams := authority.NewAuthParams("clientID", authority.Info{Host: "host", Tenant: "tenant"})
+			jwt, err := cred.JWT(authParams)
+			if err != nil {
+				t.Fatal(err)
+			}
+			newJWT, err := cred.JWT(authParams)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if shouldRegen && newJWT == jwt {
+				t.Fatal("expected a new JWT")
+			} else if !shouldRegen && newJWT != jwt {
+				t.Fatal("expected the same JWT")
+			}
+		})
+	}
+}
+
 func TestDeviceCodeResult(t *testing.T) {
 	authParams := authority.AuthParams{
 		Endpoints:   testAuthorityEndpoints,
@@ -747,8 +802,8 @@ func TestTokenResponseUnmarshal(t *testing.T) {
 			desc: "Error: decodeJWT is going to error",
 			payload: `
 				{
-					"access_token": "secret", 
-					"expires_in": 86399, 
+					"access_token": "secret",
+					"expires_in": 86399,
 					"ext_expires_in": 86399,
 					"client_info": error,
 					"scope": "openid profile"
@@ -760,8 +815,8 @@ func TestTokenResponseUnmarshal(t *testing.T) {
 			desc: "Success",
 			payload: `
 				{
-					"access_token": "secret", 
-					"expires_in": 86399, 
+					"access_token": "secret",
+					"expires_in": 86399,
 					"ext_expires_in": 86399,
 					"client_info": {"uid":  "uid","utid": "utid"},
 					"scope": "openid profile"

apps/internal/version/version.go

@@ -5,4 +5,4 @@
 package version
 
 // Version is the version of this client package that is communicated to the server.
-const Version = "0.5.0"
+const Version = "0.5.1"

apps/testdata/test-cert.pem

@@ -0,0 +1,44 @@
+-----BEGIN CERTIFICATE-----
+MIICljCCAX4CCQDNgteZ+lJH4zANBgkqhkiG9w0BAQsFADANMQswCQYDVQQGEwJ1
+czAeFw0yMTAxMDQyMzQzNDVaFw0yMTAyMDMyMzQzNDVaMA0xCzAJBgNVBAYTAnVz
+MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA1r58wq7JQxM12viLNbdG
+fFizeVQwWRwrx/4CH3kU8jjGovbhkvC/uLWqVGchgATThhGkvNrA92WvdkVwsZMk
+Qf7ZnTA7kemo4VFtgo5XCGEej9gOTW13Evdc/0Flip+RXl3h3Q6BbbB9IFE0c6cS
+3i/v/t8KGpVYQHQzBwTcYehM6eDO8ZjUyUUcJOMXdMCctamig7fMGlziKFahn4dX
+JoiiK4oNKE9okXIAXCTbVkAxxH0hD+5XH1nn5LJnHe0e5DflI3YIiPgmRL5uC89K
+XqmYCKWrq5z2D5k+5fQLmbOcxErBcFCh8hA+Xu0RLT4BHPEgc6iVIqxL4CZi/cke
+uwIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQAAyDbm0Fda0/vY6ZVDML2IbGWbro1w
+nWYNw6wclNU6sx1oeG/k/y2ni7NImPpbFN+594WS6rYHgFdROfeuNgGnjgQCJogk
++8ouf1R6vFMUAScWeSaFnZmBEgwofWsnIcUKkbDIXbpRhMrkNEcY09VgjmCKhspQ
+iX2bJQTj49XBac9tBaJJYDZ4HgkO4nU7QeEPpvwlELZFoZZXtd3fan+VUyFS2a9n
+gkAMDYoQPGN4tyGFabWws/GlMxelWvqUzpQKmeRPVz+cij75l8eKThEiu0zbjOTD
+Gq81BcY61SPqN02zoPCtqZ/zU6HhaL3x7zUuzhLhNoh83A43UVYEoOOf
+-----END CERTIFICATE-----
+-----BEGIN PRIVATE KEY-----
+MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQDWvnzCrslDEzXa
++Is1t0Z8WLN5VDBZHCvH/gIfeRTyOMai9uGS8L+4tapUZyGABNOGEaS82sD3Za92
+RXCxkyRB/tmdMDuR6ajhUW2CjlcIYR6P2A5NbXcS91z/QWWKn5FeXeHdDoFtsH0g
+UTRzpxLeL+/+3woalVhAdDMHBNxh6Ezp4M7xmNTJRRwk4xd0wJy1qaKDt8waXOIo
+VqGfh1cmiKIrig0oT2iRcgBcJNtWQDHEfSEP7lcfWefksmcd7R7kN+UjdgiI+CZE
+vm4Lz0peqZgIpaurnPYPmT7l9AuZs5zESsFwUKHyED5e7REtPgEc8SBzqJUirEvg
+JmL9yR67AgMBAAECggEAAQ/IBh5fGFnL9l0sMwPI8Wxu1ra31njxLnfvAsDSfbAS
+K1QVIWjXSc58HRa1b7CWax9DNTvPoGl8SJVnTTlxAHKGGOTYJoyFLTf91ptlisEQ
+KZ3j1DYqVImsiAaGvfyz90d3imQ795Lby4EbRUcaLMcH5LatkhwS556rcelwPXuq
+M43XaZu5Es4pG0EmzfXplO/awt5HdUDPEAY3yw7QH8D1/l/toLPyiFv37RezkVK9
+ffcUQpH7uH000Gja+JSEHgpWZhE96ac6H0zBtlM1VkMtfBuczz5tkKN/p70fhr8T
+ZXARZqIaF4vx7RkBBzCfhvrgGqxXMuvTaW6N4RDWYQKBgQD1iZ7/xr9qy4cPFSOt
+yBnG5cE6wC7wP8qgr0N7MgAii5OZgx6rtfGIVJDY58CFijnT8jZ5pjNS3p7j/Rzp
+lQJMIwC5kIe/7FU7nmE3ko7Wg+bpd8iWLLIi/QWVFLbS7qVmulTc+CEXWyhAiI2u
+RL/1APjIDFKp9gqtKmwb9erxDwKBgQDf5PbGHuPv5RBLJz9du+M/BIBY+HDltG89
+p3huHHTjkJ5R38oximf2HnV4ygT/p2+ZUD6TJZZw6qou3/GiU5gZbRpg+4LXtQUR
+vV+S2n/t86NG1YcGmM29r8LWqrK9gxLW0X62Fpps16rHSP7kVc4SvmrYwqNzqKlC
+D9QbFYYflQKBgQCKEVzrDuNMNi43+PcbHU4BXeiOFMtQJU7XlDYp7C/PPRU+WVDB
+1Yl/062vioHjlZp259hiB2cMzkoigY3kevnTvksGDZOIBGjZIXIhQbQ4Q+twlP6i
+E3gH3Kdq8T7s1W0EmvplVtGkxImZ4C9rMxWNu4IpW2SQVd4jCZvJDTuTWQKBgQCn
+LGjuCYacSubdlpKDxJSrKwtCY0641P7yhCcx4GGOwR7Vd0mbsAJsDNYduIn+8eAs
+E3SFnl00NqOXmHLth4lcAtDddS5/LZR5aHMCTc+TtoVFkI3faRzF84SBkLchNctN
+RuNbxojLmETVxDU9/Kt/51oUO1CcPWUUBImVJ38b+QKBgQCTbi0nS0n8kC7nlXWN
+QtPcf4UraJAxv1DGq4lnJ8AHSZqqkP5fyjfknSw5ExOPDg4mEHhnnpsvwJuSX00d
+UYUN2ZJXPZeaO0HmbYZ3/vC9bo6KW95PhidEUQpGlKrFY342khjQHJtH67YUThwU
+lQFhpxvPgPNBuxVRnsxoH/sLOA==
+-----END PRIVATE KEY-----