[oauth2] x/oauth2: Adding support for private_key_jwt clientAuthentication method.

70 views
Skip to first unread message

Gopher Robot (Gerrit)

unread,
Sep 28, 2022, 4:59:50 AM9/28/22
to Dileep Reddem, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

View Change

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 1
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Comment-Date: Wed, 28 Sep 2022 08:59:47 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Dileep Reddem (Gerrit)

    unread,
    Sep 28, 2022, 2:19:26 PM9/28/22
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

    Dileep Reddem has uploaded this change for review.

    View Change

    x/oauth2: Adding support for private_key_jwt clientAuthentication method.

    Details:
    OAuth2 spec describes a secure way of sending token API by using private_key_jwt. This method avoids sending client_secret, and hence is extremely secure.

    This is described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

    private_key_jwt is defined in https://datatracker.ietf.org/doc/html/rfc7523

    Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    ---
    M clientcredentials/clientcredentials.go
    M internal/token.go
    M internal/token_test.go
    M jws/jws.go
    M oauth2.go
    M token.go
    6 files changed, 200 insertions(+), 7 deletions(-)

    diff --git a/clientcredentials/clientcredentials.go b/clientcredentials/clientcredentials.go
    index 7a0b9ed..8b0d39a 100644
    --- a/clientcredentials/clientcredentials.go
    +++ b/clientcredentials/clientcredentials.go
    @@ -103,7 +103,8 @@
    v[k] = p
    }

    - tk, err := internal.RetrieveToken(c.ctx, c.conf.ClientID, c.conf.ClientSecret, c.conf.TokenURL, v, internal.AuthStyle(c.conf.AuthStyle))
    + // TODO: Add private_key_jwt support for client_credentials grant.
    + tk, err := internal.RetrieveToken(c.ctx, c.conf.ClientID, c.conf.ClientSecret, c.conf.TokenURL, "", nil, v, internal.AuthStyle(c.conf.AuthStyle))
    if err != nil {
    if rErr, ok := err.(*internal.RetrieveError); ok {
    return nil, (*oauth2.RetrieveError)(rErr)
    diff --git a/internal/token.go b/internal/token.go
    index 355c386..b4e134f 100644
    --- a/internal/token.go
    +++ b/internal/token.go
    @@ -6,12 +6,14 @@

    import (
    "context"
    + "crypto/rand"
    "encoding/json"
    "errors"
    "fmt"
    "io"
    "io/ioutil"
    "math"
    + "math/big"
    "mime"
    "net/http"
    "net/url"
    @@ -21,6 +23,13 @@
    "time"

    "golang.org/x/net/context/ctxhttp"
    + "golang.org/x/oauth2/jws"
    +)
    +
    +const (
    + // ClientAssertionType is used to denote private_key_jwt client authenticaiton method
    + // described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication
    + clientAssertionType = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"
    )

    // Token represents the credentials used to authorize
    @@ -109,6 +118,8 @@
    AuthStyleUnknown AuthStyle = 0
    AuthStyleInParams AuthStyle = 1
    AuthStyleInHeader AuthStyle = 2
    + // AuthStylePrivateKeyJWT denotes private_key_jwt ClientAuthentication method defined at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication
    + AuthStylePrivateKeyJWT AuthStyle = 3
    )

    // authStyleCache is the set of tokenURLs we've successfully used via
    @@ -148,6 +159,74 @@
    authStyleCache.m[tokenURL] = v
    }

    +// generateJTI generates random/unique bytes to be passed as 'jti' argument in the token request.
    +// Only used in private_key_jwt variant of Token API.
    +func generateJTI() (string, error) {
    + size := 32 // arbitrary power of 2
    + bytes := make([]byte, size/2)
    + if _, err := io.ReadFull(rand.Reader, bytes); err != nil {
    + return "", err
    + }
    + // convert binary to printable ascii
    + return fmt.Sprintf("%x", big.NewInt(int64(size)).SetBytes(bytes)), nil
    +}
    +
    +// clientAssertion prepares the client_assertion element of the private_key_jwt method as described at
    +// https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication
    +func clientAssertion(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (url.Values, error) {
    + pk, err := ParseKey(privateKey)
    + if err != nil {
    + return nil, err
    + }
    +
    + jti, err := generateJTI()
    + if err != nil {
    + return nil, err
    + }
    +
    + claimSet := &jws.ClaimSet{
    + Iss: clientID,
    + Sub: clientID,
    + Aud: tokenURL,
    + Jti: jti,
    + Exp: time.Now().Add(time.Hour).Unix(),
    + Iat: time.Now().Unix(),
    + }
    +
    + h := jws.Header{
    + Algorithm: "RS256",
    + Typ: "JWT",
    + KeyID: keyID,
    + }
    +
    + payload, err := jws.Encode(&h, claimSet, pk)
    + if err != nil {
    + return nil, err
    + }
    + v.Set("client_assertion", payload)
    + v.Set("client_assertion_type", clientAssertionType)
    + v.Set("client_id", clientID)
    +
    + return v, nil
    +}
    +
    +// newPrivateKeyJWTTokenRequest returns a new *http.Request to retrieve a new token
    +// from tokenURL using the provided clientID, privatekey, keyID and POST
    +// body parameters.
    +func newPrivateKeyJWTTokenRequest(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (*http.Request, error) {
    + body, err := clientAssertion(tokenURL, clientID, keyID, privateKey, v)
    + if err != nil {
    + return nil, fmt.Errorf("oauth2: cannot prepare client_assertion for token request : %v", err)
    + }
    +
    + req, err := http.NewRequest("POST", tokenURL, strings.NewReader(body.Encode()))
    + if err != nil {
    + return nil, err
    + }
    + req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
    + return req, nil
    +}
    +
    // newTokenRequest returns a new *http.Request to retrieve a new token
    // from tokenURL using the provided clientID, clientSecret, and POST
    // body parameters.
    @@ -185,7 +264,10 @@
    return v2
    }

    -func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string, v url.Values, authStyle AuthStyle) (*Token, error) {
    +// RetrieveToken sends API to the Token Endpoint of OAUTH Provider with necessary parameters based on the configuration.
    +// It also automatically tries different style of sending credentials (clientid/clientsecret)
    +// depending on the server response.
    +func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL, keyID string, privateKey []byte, v url.Values, authStyle AuthStyle) (*Token, error) {
    needsAuthStyleProbe := authStyle == 0
    if needsAuthStyleProbe {
    if style, ok := lookupAuthStyle(tokenURL); ok {
    @@ -195,7 +277,13 @@
    authStyle = AuthStyleInHeader // the first way we'll try
    }
    }
    - req, err := newTokenRequest(tokenURL, clientID, clientSecret, v, authStyle)
    + var req *http.Request
    + var err error
    + if authStyle == AuthStylePrivateKeyJWT {
    + req, err = newPrivateKeyJWTTokenRequest(tokenURL, clientID, keyID, privateKey, v)
    + } else {
    + req, err = newTokenRequest(tokenURL, clientID, clientSecret, v, authStyle)
    + }
    if err != nil {
    return nil, err
    }
    diff --git a/internal/token_test.go b/internal/token_test.go
    index c54095a..fde1413 100644
    --- a/internal/token_test.go
    +++ b/internal/token_test.go
    @@ -15,6 +15,34 @@
    "testing"
    )

    +var testPrivateKey = []byte(`-----BEGIN RSA PRIVATE KEY-----
    +MIIEowIBAAKCAQEAoZdD//51RbZp51aoyB0QFKfRDmLLRCL6lCXmz6uu78rVJEXB
    +9sCFwjx+sVzH2dCsqJs1DbQnnLJ0rM4Gk4UdjWWErGx0XVuGM4BEcce7GAG8q2RT
    +54/SB8Pt95oUVrfa5bKpjT9UKoLL0TGqZDbnCtf9RLK3sEbQjumHyxFwDjQiqgRL
    +k+9+C4tO/q1GrT3tHANHp6X8PseAugkteuT/DIWqjX3FQsA7scH0RxtOFAvHFJ1R
    +i/RPnCI+okfmmYravje6Ue5xsr8YxOH14iLr6zRQla2/Rqby8M3a4IR1ho2sIelh
    +jEKsooFKeA5/xoHrAHBhff6mIFYwWhUj5K+2cwIDAQABAoIBAETwzOW0ceMeqrs0
    +uUi7QYeWe3ZIRxGYXNEFBJ7YUAflQR87FcBJLigK+ECCZY9z3J4Irc9da8MKTgYF
    +1j9s/Qk85ShNEy5bZHunf0wN2zAoWY7D/JogPYrrmCTZm1DOGvmByp3FBYsnh36G
    +Unx0AgmZ0efT2dO+uq8mSjWkiGq+PNBoHthsVhnbZlS9jOo+F4d6KehN4ic6Xylf
    +0OATkH2r0lUp28MOoKm9WhVbznpr99C9bnaxeKZbqPzLV1i7j4AwtDJjQM+nlWkA
    +Klfe1ikrj9VcDt+cGjkVsBu60GSUcfCdfcFeHFnQxU2PoU6mKR04wigJsJE1rLOk
    +wQmYAw0CgYEA1Ekq8RjCPJyDmpZ25hJP4u9W6kxdxzyplpu4kDJlmNpIukXk2XJV
    +rl+WE84/PxGhIEpwKS4d9rUJEajhh1ywtq5ncLr3h9fP9zVhucRM5vPDhoPF3IpW
    +nZGl7jEvEZSt1gN1l1HL08vtoIDJH9A4DwfQcs4k+5nsb1sBQTkp6SUCgYEAwt2r
    +mFwsdXWp17OIkTvAPXW+ZxEKMNm8LqbWO0DZOv9FAmYqroax59FfVqcknp/zirmz
    +8UnybMW2q42epGGlO2p1XyVpFYwiyr17A0zRNS91SOfU2URZupUdxzc0uLNmw81i
    +FXjIxCyd+OUQuvj0AM9+W/ja8BPq/jasBB88ybcCgYBZGmz/zZiZwEgs9sLpRv5p
    +DnS421zxZ7D3Gl2ZiM3EHrswFG4+JxN3oX9oyMUbP9cVqqjxX/4Ls2kfFHe1TV6P
    +Dx0z4AZN6nPMG4ftZUuyFNcY+u5t51L7yEqRc+uBwZpFniYZYafgOlR/bg79X3Ro
    +OqtvxL8ZLD5lbxlTux6wUQKBgQCKR5c/FRmPeKG/qW0d8pKmimlE5jifFcOOL2IS
    +xh/g18h2vV1IX9jbMh4/dXhtAABozwK8FMdtHJhWALc02v3PvenwTLHXUoxV5kPm
    +wZor7bOCutC2JOSvnKV2+tv0vYoNJC+YIRAG9cpcFuDabVNpR9TZGyzx9JBrOQy0
    +GB6g3QKBgFutTJZrTFMNOwvQYUhG21/EOYz6pZPCofTNBUOQM7KvbKH/UJ5kPtYW
    +oK6yzHhpOGsZvly5CPau6djDhQZvzdDl2t3eRKuqTtHARhtyL5Qb7OICEsOCYVWh
    +IHJQ79Wak98HWO9Dkww1mRumvEtOTqjtzTLp2w1IaKGdI+fLD3/v
    +-----END RSA PRIVATE KEY-----`)
    +
    func TestRetrieveToken_InParams(t *testing.T) {
    ResetAuthCache()
    const clientID = "client-id"
    @@ -29,7 +57,7 @@
    io.WriteString(w, `{"access_token": "ACCESS_TOKEN", "token_type": "bearer"}`)
    }))
    defer ts.Close()
    - _, err := RetrieveToken(context.Background(), clientID, "", ts.URL, url.Values{}, AuthStyleInParams)
    + _, err := RetrieveToken(context.Background(), clientID, "", ts.URL, "", nil, url.Values{}, AuthStyleInParams)
    if err != nil {
    t.Errorf("RetrieveToken = %v; want no error", err)
    }
    @@ -45,7 +73,7 @@
    }))
    defer ts.Close()

    - _, err := RetrieveToken(context.Background(), clientID, "", ts.URL, url.Values{}, AuthStyleUnknown)
    + _, err := RetrieveToken(context.Background(), clientID, "", ts.URL, "", nil, url.Values{}, AuthStyleUnknown)
    if err != nil {
    t.Errorf("RetrieveToken (with background context) = %v; want no error", err)
    }
    @@ -58,7 +86,7 @@

    ctx, cancel := context.WithCancel(context.Background())
    cancel()
    - _, err = RetrieveToken(ctx, clientID, "", cancellingts.URL, url.Values{}, AuthStyleUnknown)
    + _, err = RetrieveToken(ctx, clientID, "", cancellingts.URL, "", nil, url.Values{}, AuthStyleUnknown)
    close(retrieved)
    if err == nil {
    t.Errorf("RetrieveToken (with cancelled context) = nil; want error")
    @@ -75,3 +103,48 @@
    t.Errorf("expiration time = %v; want %v", e, want)
    }
    }
    +
    +func TestRetrieveToken_PrivateKeyJWT(t *testing.T) {
    + ResetAuthCache()
    + clientID := "client-id"
    + clientAssertionType := "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"
    +
    + testCases := []struct {
    + name string
    + clientID string
    + privateKey []byte
    + wantErr bool
    + }{
    + {
    + name: "Success case",
    + clientID: clientID,
    + privateKey: testPrivateKey,
    + },
    + {
    + name: "invalid private key",
    + clientID: clientID,
    + privateKey: []byte(`invalid certificate data`),
    + wantErr: true,
    + },
    + }
    + for _, tc := range testCases {
    + t.Run(tc.name, func(t *testing.T) {
    + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    + if got, want := r.FormValue("client_id"), tc.clientID; got != want {
    + t.Errorf("client_id = %q; want %q", got, want)
    + }
    + if got, want := r.FormValue("client_assertion_type"), clientAssertionType; got != want {
    + t.Errorf("client_assertion_type = %q; want %q", got, want)
    + }
    + w.Header().Set("Content-Type", "application/json")
    + io.WriteString(w, `{"access_token": "ACCESS_TOKEN", "token_type": "bearer"}`)
    + }))
    + _, err := RetrieveToken(context.Background(), tc.clientID, "", ts.URL, "", tc.privateKey, url.Values{}, AuthStylePrivateKeyJWT)
    + if err != nil && !tc.wantErr {
    + t.Errorf("RetrieveToken = %v; want no error", err)
    + }
    + ts.Close()
    + })
    + }
    +
    +}
    diff --git a/jws/jws.go b/jws/jws.go
    index 9501564..6dbf1f4 100644
    --- a/jws/jws.go
    +++ b/jws/jws.go
    @@ -38,6 +38,7 @@
    Exp int64 `json:"exp"` // the expiration time of the assertion (seconds since Unix epoch)
    Iat int64 `json:"iat"` // the time the assertion was issued (seconds since Unix epoch)
    Typ string `json:"typ,omitempty"` // token type (Optional).
    + Jti string `json:"jti,omitempty"` // jti denotes unique id for the token request. Used in private_key_jwt

    // Email for which the application is requesting delegated access (Optional).
    Sub string `json:"sub,omitempty"`
    diff --git a/oauth2.go b/oauth2.go
    index 291df5c..098ac72 100644
    --- a/oauth2.go
    +++ b/oauth2.go
    @@ -57,6 +57,15 @@

    // Scope specifies optional requested permissions.
    Scopes []string
    +
    + // Private key contains the base64 encoded contents of the RSA private key.
    + // This key would be used in Token request (https://openid.net/specs/openid-connect-core-1_0.html#TokenEndpoint).
    + // Specifically, this certificate would be used in private_key_jwt variant of client authentication method
    + // as described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication
    + PrivateKey []byte
    +
    + // KeyID is a hint that uniquely identfies the 'PrivateKey' used to sign jwt.
    + KeyID string
    }

    // A TokenSource is anything that can return a token.
    @@ -97,6 +106,11 @@
    // using HTTP Basic Authorization. This is an optional style
    // described in the OAuth2 RFC 6749 section 2.3.1.
    AuthStyleInHeader AuthStyle = 2
    +
    + // AuthStylePrivateKeyJWT implies that the token request must contain a JWT instead of client_secret
    + // either in header or in post body. This is more secure way of sending the API.
    + // See https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication
    + AuthStylePrivateKeyJWT AuthStyle = 3
    )

    var (
    diff --git a/token.go b/token.go
    index 8227203..c35984a 100644
    --- a/token.go
    +++ b/token.go
    @@ -154,7 +154,7 @@
    // This token is then mapped from *internal.Token into an *oauth2.Token which is returned along
    // with an error..
    func retrieveToken(ctx context.Context, c *Config, v url.Values) (*Token, error) {
    - tk, err := internal.RetrieveToken(ctx, c.ClientID, c.ClientSecret, c.Endpoint.TokenURL, v, internal.AuthStyle(c.Endpoint.AuthStyle))
    + tk, err := internal.RetrieveToken(ctx, c.ClientID, c.ClientSecret, c.Endpoint.TokenURL, c.KeyID, c.PrivateKey, v, internal.AuthStyle(c.Endpoint.AuthStyle))
    if err != nil {
    if rErr, ok := err.(*internal.RetrieveError); ok {
    return nil, (*RetrieveError)(rErr)

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 1
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-MessageType: newchange

    Damien Neil (Gerrit)

    unread,
    Nov 18, 2022, 2:54:53 PM11/18/22
    to Dileep Reddem, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, Gopher Robot, golang-co...@googlegroups.com

    Attention is currently required from: Brad Fitzpatrick, Dileep Reddem.

    View Change

    10 comments:

    • Patchset:

    • File clientcredentials/clientcredentials.go:

      • Patch Set #1, Line 106: // TODO: Add private_key_jwt support for client_credentials grant.

        At a minimum this needs to return an error if AuthStylePrivateKeyJIT is used, but it should support this auth mode here unless there's a compelling reason not to.

    • File internal/token.go:

      • Patch Set #1, Line 165: size := 32 // arbitrary power of 2

        Why does this need to be a power of 2?

      • Patch Set #1, Line 166: bytes := make([]byte, size/2)

        Why size/2?

      • Patch Set #1, Line 171: return fmt.Sprintf("%x", big.NewInt(int64(size)).SetBytes(bytes)), nil

        Just `fmt.Sprintf("%x", bytes)` will do, and avoids pulling in math/big.

      • Patch Set #1, Line 176: func clientAssertion(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (url.Values, error) {

        Why does this return a url.Values?

      • Patch Set #1, Line 216: func newPrivateKeyJWTTokenRequest(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (*http.Request, error) {

        This function overlaps with newTokenRequest, and is inconsistent with it:

        • newTokenRequest clones the url.Values, newPrivateKeyJWTTokenRequest does not.
        • both post the request and set the Content-Type in identical ways

        Having separate functions might make sense, but the Values cloning behavior should be aligned.

    • File oauth2.go:

      • Patch Set #1, Line 61: // Private key contains the base64 encoded contents of the RSA private key.

        This seems like it should be an `*rsa.PrivateKey`, but I see that this matches jwt.Config.PrivateKey.

      • Patch Set #1, Line 68: KeyID string

        There probably should be an error for these fields mismatching the auth style: If you set PrivateKey or KeyID, then you must use AuthStylePrivateKeyJWT, and vice-versa.

      • Patch Set #1, Line 99: AuthStyleAutoDetect AuthStyle = 0

        Should AutoDetect select private_key_jwt if PrivateKey is set?

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 1
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-CC: Chris Broadfoot <cb...@golang.org>
    Gerrit-CC: Cody Oss <cod...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: JBD <j...@google.com>
    Gerrit-CC: Shin Fan <shi...@google.com>
    Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Attention: Dileep Reddem <dileep...@google.com>
    Gerrit-Comment-Date: Fri, 18 Nov 2022 19:54:48 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Dileep Reddem (Gerrit)

    unread,
    Nov 24, 2022, 1:40:00 AM11/24/22
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

    Attention is currently required from: Brad Fitzpatrick, Dileep Reddem.

    Dileep Reddem uploaded patch set #2 to this change.

    View Change

    x/oauth2: Adding support for private_key_jwt clientAuthentication method.

    Details:
    OAuth2 spec describes a secure way of sending token API by using private_key_jwt. This method avoids sending client_secret, and hence is extremely secure.

    This is described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

    private_key_jwt is defined in https://datatracker.ietf.org/doc/html/rfc7523

    Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    ---
    M clientcredentials/clientcredentials.go
    M clientcredentials/clientcredentials_test.go

    M internal/token.go
    M internal/token_test.go
    M jws/jws.go
    M oauth2.go
    M oauth2_test.go
    M token.go
    8 files changed, 287 insertions(+), 7 deletions(-)

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 2
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-CC: Chris Broadfoot <cb...@golang.org>
    Gerrit-CC: Cody Oss <cod...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: JBD <j...@google.com>
    Gerrit-CC: Shin Fan <shi...@google.com>
    Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Attention: Dileep Reddem <dileep...@google.com>
    Gerrit-MessageType: newpatchset

    Dileep Reddem (Gerrit)

    unread,
    Nov 24, 2022, 1:48:01 AM11/24/22
    to goph...@pubsubhelper.golang.org, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, Gopher Robot, golang-co...@googlegroups.com

    Attention is currently required from: Brad Fitzpatrick, Damien Neil.

    View Change

    8 comments:

    • File clientcredentials/clientcredentials.go:

      • At a minimum this needs to return an error if AuthStylePrivateKeyJIT is used, but it should support […]

        Done

    • File internal/token.go:

      • It's a random choice. 32 is chosen for sufficiently large size. Power of 2 as powers-of-2 are quite common.

      • Below I was converting single-byte potentially non-printable char to a printable ascii/hex.. ie 1 byte will be converted to 2 hex chars. Hence /2.

      • Patch Set #1, Line 171: return fmt.Sprintf("%x", big.NewInt(int64(size)).SetBytes(bytes)), nil

        Just `fmt.Sprintf("%x", bytes)` will do, and avoids pulling in math/big.

      • Done

      • Patch Set #1, Line 176: func clientAssertion(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (url.Values, error) {

      • Why does this return a url. […]

        This returns url.Values to keep the code similar to newTokenRequest(), specifically the line that posts the API:
        ```


      • req, err := http.NewRequest("POST", tokenURL, strings.NewReader(body.Encode()))

      • ```

        I was viewing this API as a helper function that gets the data required for 'http.NewRequest(...)' similar to the if-block in lines 239-247.

      • Patch Set #1, Line 216: func newPrivateKeyJWTTokenRequest(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (*http.Request, error) {

      • This function overlaps with newTokenRequest, and is inconsistent with it: […]

        Done

    • File oauth2.go:

      • Patch Set #1, Line 68: KeyID string

        There probably should be an error for these fields mismatching the auth style: If you set PrivateKey […]

        Done

      • Patch Set #1, Line 99: AuthStyleAutoDetect AuthStyle = 0

        Should AutoDetect select private_key_jwt if PrivateKey is set?

      • Good question. Looks like this is used only by google. From the comment, it feels like this is related to AuthStyleInHeader vs AuthStyleInParam. I don't see any use cases for this. We can do it as a follow up if required. LMK.

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 2
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-CC: Chris Broadfoot <cb...@golang.org>
    Gerrit-CC: Cody Oss <cod...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: JBD <j...@google.com>
    Gerrit-CC: Shin Fan <shi...@google.com>
    Gerrit-Attention: Damien Neil <dn...@google.com>
    Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Comment-Date: Thu, 24 Nov 2022 06:47:55 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Damien Neil <dn...@google.com>
    Gerrit-MessageType: comment

    Dileep Reddem (Gerrit)

    unread,
    Dec 9, 2022, 2:08:14 AM12/9/22
    to goph...@pubsubhelper.golang.org, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, Gopher Robot, golang-co...@googlegroups.com

    Attention is currently required from: Brad Fitzpatrick, Damien Neil.

    View Change

    2 comments:

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 2
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-CC: Chris Broadfoot <cb...@golang.org>
    Gerrit-CC: Cody Oss <cod...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: JBD <j...@google.com>
    Gerrit-CC: Shin Fan <shi...@google.com>
    Gerrit-Attention: Damien Neil <dn...@google.com>
    Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Comment-Date: Fri, 09 Dec 2022 07:08:08 +0000

    Damien Neil (Gerrit)

    unread,
    Dec 12, 2022, 1:25:41 PM12/12/22
    to Dileep Reddem, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, Gopher Robot, golang-co...@googlegroups.com

    Attention is currently required from: Brad Fitzpatrick, Dileep Reddem.

    View Change

    5 comments:

    • File clientcredentials/clientcredentials.go:

      • Done

        What's the reason for not supporting private_key_jwt here?

    • File internal/token.go:

      • It's a random choice. 32 is chosen for sufficiently large size. […]

        The comment here says "arbitrary power of 2", which implies that this needs to be a power of 2.

        If it doesn't need to be a power of 2, then drop or change the comment.

      • Below I was converting single-byte potentially non-printable char to a printable ascii/hex.. […]

        Defining 'size' as the size in hex-encoded bytes makes it more difficult to see how much entropy is present in the JWT ID. Start with the size in bytes (size = 16).

        This means that if the encoding here were changed to something more compact than hex (base64, for example), the definition of the JWT ID size would not need to change.

      • Patch Set #1, Line 176: func clientAssertion(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (url.Values, error) {

      • This returns url. […]

        But the url.Values is just the one passed in as a parameter. Why is it being returned? The caller already has it.

      • Patch Set #1, Line 216: func newPrivateKeyJWTTokenRequest(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (*http.Request, error) {

      • Done

        Doesn't seem to be done? newPrivateKeyJWTTokenRequest doesn't clone the provided url.Values, and newTokenRequest does. Both functions should behave the same.

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 2
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-CC: Chris Broadfoot <cb...@golang.org>
    Gerrit-CC: Cody Oss <cod...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: JBD <j...@google.com>
    Gerrit-CC: Shin Fan <shi...@google.com>
    Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Attention: Dileep Reddem <dileep...@google.com>
    Gerrit-Comment-Date: Mon, 12 Dec 2022 18:25:37 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Damien Neil <dn...@google.com>
    Comment-In-Reply-To: Dileep Reddem <dileep...@google.com>
    Gerrit-MessageType: comment

    Dileep Reddem (Gerrit)

    unread,
    Dec 13, 2022, 5:50:43 AM12/13/22
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

    Attention is currently required from: Brad Fitzpatrick, Dileep Reddem.

    Dileep Reddem uploaded patch set #3 to this change.

    View Change

    x/oauth2: Adding support for private_key_jwt clientAuthentication method.

    Details:
    OAuth2 spec describes a secure way of sending token API by using private_key_jwt. This method avoids sending client_secret, and hence is extremely secure.

    This is described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

    private_key_jwt is defined in https://datatracker.ietf.org/doc/html/rfc7523

    Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    ---
    M clientcredentials/clientcredentials.go
    M clientcredentials/clientcredentials_test.go
    M internal/token.go
    M internal/token_test.go
    M jws/jws.go
    M oauth2.go
    M oauth2_test.go
    M token.go
    8 files changed, 288 insertions(+), 7 deletions(-)

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 3
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-CC: Chris Broadfoot <cb...@golang.org>
    Gerrit-CC: Cody Oss <cod...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: JBD <j...@google.com>
    Gerrit-CC: Shin Fan <shi...@google.com>
    Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Attention: Dileep Reddem <dileep...@google.com>
    Gerrit-MessageType: newpatchset

    Dileep Reddem (Gerrit)

    unread,
    Jan 12, 2023, 9:33:59 PM1/12/23
    to goph...@pubsubhelper.golang.org, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, Gopher Robot, golang-co...@googlegroups.com

    Attention is currently required from: Brad Fitzpatrick, Damien Neil.

    View Change

    6 comments:

      • What's the reason for not supporting private_key_jwt here?

        No specific reason other than keeping the CL short. I will incorporate the calls (and add test cases) once current CL is done.

        For now, we have unit tests that fail if this method is specified (as per previous review comment).

    • File internal/token.go:

      • The comment here says "arbitrary power of 2", which implies that this needs to be a power of 2. […]

        I will change the comment. It can be any size

      • Defining 'size' as the size in hex-encoded bytes makes it more difficult to see how much entropy is […]

        got it. I made size as 32, so that current JTI will be 64. I did not want to make it 16 just in case we decide to go with other encoding like base64url

      • Patch Set #1, Line 176: func clientAssertion(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (url.Values, error) {

      • But the url.Values is just the one passed in as a parameter. […]

        3 new url-values are added to this. Hence we are also returning the result.
        Caller was closing the values before the call. I think that's a bit confusing. I now moved cloning of urlValues to this function before using 'v'. Let me know how does it look now.

      • Patch Set #1, Line 216: func newPrivateKeyJWTTokenRequest(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (*http.Request, error) {

      • Doesn't seem to be done? newPrivateKeyJWTTokenRequest doesn't clone the provided url. […]

        In the prev snapshot, cloning is done in the call to 'clientAssertion()'. I am viewing clientAssertion() as a helper/inline.
        That is, I don't see a node to clone in newPrivateKeyJWT..() where the actual use is within clientAssertion(). Now I am doing clone in clientAssertion() where it's being used.

        Please check and let me know if you have suggestions.

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 3
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-CC: Chris Broadfoot <cb...@golang.org>
    Gerrit-CC: Cody Oss <cod...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: JBD <j...@google.com>
    Gerrit-CC: Shin Fan <shi...@google.com>
    Gerrit-Attention: Damien Neil <dn...@google.com>
    Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Comment-Date: Fri, 13 Jan 2023 02:33:50 +0000

    Dileep Reddem (Gerrit)

    unread,
    Jan 12, 2023, 9:48:07 PM1/12/23
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

    Attention is currently required from: Brad Fitzpatrick, Damien Neil.

    Dileep Reddem uploaded patch set #4 to this change.

    View Change

    x/oauth2: Adding support for private_key_jwt clientAuthentication method.

    Details:
    OAuth2 spec describes a secure way of sending token API by using private_key_jwt. This method avoids sending client_secret, and hence is extremely secure.

    This is described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

    private_key_jwt is defined in https://datatracker.ietf.org/doc/html/rfc7523

    Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    ---
    M clientcredentials/clientcredentials.go
    M clientcredentials/clientcredentials_test.go
    M internal/token.go
    M internal/token_test.go
    M jws/jws.go
    M oauth2.go
    M oauth2_test.go
    M token.go
    8 files changed, 289 insertions(+), 7 deletions(-)

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 4
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-CC: Chris Broadfoot <cb...@golang.org>
    Gerrit-CC: Cody Oss <cod...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: JBD <j...@google.com>
    Gerrit-CC: Shin Fan <shi...@google.com>
    Gerrit-Attention: Damien Neil <dn...@google.com>
    Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-MessageType: newpatchset

    Damien Neil (Gerrit)

    unread,
    Jan 26, 2023, 1:55:46 PM1/26/23
    to Dileep Reddem, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, Gopher Robot, golang-co...@googlegroups.com

    Attention is currently required from: Brad Fitzpatrick, Dileep Reddem.

    View Change

    16 comments:

    • Patchset:

      • Patch Set #1:

        Done

        The proposal needs to include the actual API being proposed. Several people on that issue have asked for it, but you haven't responded to them yet.

    • File clientcredentials/clientcredentials.go:

      • No specific reason other than keeping the CL short. […]

        I believe that finishing support here will be a prerequisite for landing this feature; we don't want to add it half-finished and incomplete.

    • File internal/token.go:

      • Patch Set #1, Line 216: func newPrivateKeyJWTTokenRequest(tokenURL, clientID, keyID string, privateKey []byte, v url.Values) (*http.Request, error) {

        In the prev snapshot, cloning is done in the call to 'clientAssertion()'. […]

        Ack

    • File internal/token.go:

      • Patch Set #4, Line 119: // AuthStylePrivateKeyJWT denotes private_key_jwt ClientAuthentication method defined at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

        No need for this comment here; the documentation on oauth2.AuthStylePrivateKeyJWT (which this is a copy of) is sufficient.

      • Patch Set #4, Line 169: return fmt.Sprintf("%x", bytes), nil

        `base64.RawURLEncoding.EncodeToString(bytes)` would be more compact.

      • Patch Set #4, Line 246: }

        You can simplify this by dropping `newPivateKeyJWTTokenRequest` and changing this to:

        ```
        var body url.Values
        switch c.AuthStyle {
        case AuthStyleInHeader:
        body = v
        case AuthStyleInParams:
        body = cloneURLValues(v)
        if c.ClientID != "" {
        body.Set("client_id", c.ClientID)
        }
        if clientSecret != "" {
        body.Set("client_secret", c.ClientSecret)
        }
        case AuthStylePrivateKeyJWT:
        var err error
        body, err = clientAssertion(c.URL, c.ClientID, c.KeyID, c.PrivateKey, v)
        if err != nil {
        return err
        }
        default:
        return fmt.Errorf("oauth2: unsupported AuthStyle %v", c.AuthStyle)
        }
        ```

        This centralizes the decision of how to construct the request for various auth styles in `newTokenRequest`, rather than spreading it across two different functions.

        (I'm assuming here that we add a TokenConfig type, see next comment below.)

      • Patch Set #4, Line 269: func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL, keyID string, privateKey []byte, v url.Values, authStyle AuthStyle) (*Token, error) {

        This parameter list is getting very long, and too many of the parameters have the same type. Let's bundle up the token configuration into a struct so we can pass it around more easily:

        ```
        type TokenConfig struct {
        AuthStyle AuthStyle
        URL string
        ClientID string
        ClientSecret string
        KeyID string
        PrivateKey []byte
        }
        func RetrieveToken(ctx context.Context, v url.Values, c TokenConfig) (*Token, error) {
        // ...
        }
        ```
    • File jws/jws.go:

      • Patch Set #4, Line 41: jti denotes unique id for the token request. Used in private_key_jwt

        ```
        // unique token identifier, used by private_key_jwt
        ```

    • File oauth2.go:

      • Patch Set #1, Line 99: AuthStyleAutoDetect AuthStyle = 0

        Good question. Looks like this is used only by google. […]

        Leaving AutoDetect to apply only to InParams/InHeader seems fine.

        The doc comment here should be updated to indicate that it only applies to those styles.

    • File oauth2.go:

      • Patch Set #4, Line 64: // as described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

        ```
        // PrivateKey contains the contents of an RSA private key or the
        // contents of a PEM file that contains a private key.
        // The provided private key is used to sign JWTs when using AuthStylePrivateKeyJWT.
        // PEM containers with a passphrase are not supported.
        ```

        (This matches the language in jwt.Config.)

        However, see next comment.

      • Patch Set #4, Line 65: PrivateKey []byte

        As currently implemented, this private key is reparsed on every use.

        Using a `[]byte` here matches the `jwt.Config.PrivateKey` field, but the `jwt` package parses its key only once and retains the parsed `*rsa.PrivateKey` for future use.

        Either this needs to be a `*rsa.PrivateKey`, or the implementation needs to ensure somehow that it doesn't repeatedly reparse the key.

      • Patch Set #4, Line 67: // KeyID is a hint that uniquely identfies the 'PrivateKey' used to sign jwt.

        ```
        // PrivateKeyID is an optional hint indicating which key is being used.
        ```

        (Better matches the language in jwt.Config.)

        (Obsolete if changed: "identifies" is typoed, and we don't generally use 'quotes' around symbol names in doc comments.)

      • Patch Set #4, Line 68: KeyID string

        Name this PrivateKeyID, for consistency with the same field in jwt.Config.

      • Patch Set #4, Line 112: // See https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

        ```
        // AuthStylePrivateKeyJWT authenticates with a JWT signed with a private key.
        // The PrivateKey field must be set in the Config.
        //
        // This is the private_key_jwt authentication method described at:
        // https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication
        ```

      • Patch Set #4, Line 153: func (c *Config) Validate() error {

        This change doesn't seem to require adding an exported Validate method. Unless there's a compelling need for it, make it unexported to avoid complicating the proposal?

      • Patch Set #4, Line 157: return errors.New("PrivateKey and KeyID must both be specified or omitted.")

        When using private_key_jwt, PrivateKey must always be specified, no?

        And is PrivateKeyID required? jwt.Config.PrivateKeyID seems to be the same field, and documents it as being optional.

        This should also verify that the user isn't setting configuration settings that will be ignored: It should be an error to set ClientSecret when using private_key_jwt.

    To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: oauth2
    Gerrit-Branch: master
    Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
    Gerrit-Change-Number: 435795
    Gerrit-PatchSet: 4
    Gerrit-Owner: Dileep Reddem <dileep...@google.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-CC: Chris Broadfoot <cb...@golang.org>
    Gerrit-CC: Cody Oss <cod...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: JBD <j...@google.com>
    Gerrit-CC: Shin Fan <shi...@google.com>
    Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Attention: Dileep Reddem <dileep...@google.com>
    Gerrit-Comment-Date: Thu, 26 Jan 2023 18:55:40 +0000

    Matthew Hickford (Gerrit)

    unread,
    Feb 27, 2023, 3:06:58 AM2/27/23
    to Dileep Reddem, goph...@pubsubhelper.golang.org, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, Gopher Robot, golang-co...@googlegroups.com

    Attention is currently required from: Brad Fitzpatrick, Dileep Reddem.

    Patch set 4:Run-TryBot +1

    View Change

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 4
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: Gopher Robot <go...@golang.org>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: Dileep Reddem <dileep...@google.com>
      Gerrit-Comment-Date: Mon, 27 Feb 2023 08:06:55 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes
      Gerrit-MessageType: comment

      Dileep Reddem (Gerrit)

      unread,
      Mar 28, 2023, 10:54:46 PM3/28/23
      to goph...@pubsubhelper.golang.org, adam strickland, Gopher Robot, Matthew Hickford, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, adam strickland.

      View Change

      1 comment:

      • Patchset:

        • Patch Set #1:

          Heads up, I'm trying to move this along in the issue, and would be happy to take this on need be. […]

          Hi Adam! That would be awesome. Please go ahead!

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 4
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>
      Gerrit-Comment-Date: Wed, 29 Mar 2023 02:54:41 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Damien Neil <dn...@google.com>
      Comment-In-Reply-To: adam strickland <adamstri...@gmail.com>

      adam strickland (Gerrit)

      unread,
      Mar 29, 2023, 12:16:57 PM3/29/23
      to Dileep Reddem, goph...@pubsubhelper.golang.org, Gopher Robot, Matthew Hickford, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Dileep Reddem.

      View Change

      2 comments:

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 4
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: Dileep Reddem <dileep...@google.com>
      Gerrit-Comment-Date: Wed, 29 Mar 2023 02:22:50 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Damien Neil <dn...@google.com>

      Dileep Reddem (Gerrit)

      unread,
      Apr 24, 2023, 6:33:16 AM4/24/23
      to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Matthew Hickford, adam strickland.

      Dileep Reddem uploaded patch set #5 to this change.

      View Change

      The following approvals got outdated and were removed: Run-TryBot+1 by Matthew Hickford, TryBot-Result+1 by Gopher Robot

      x/oauth2: Adding support for private_key_jwt clientAuthentication method.

      Details:
      OAuth2 spec describes a secure way of sending token API by using private_key_jwt. This method avoids sending client_secret, and hence is extremely secure.

      This is described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

      private_key_jwt is defined in https://datatracker.ietf.org/doc/html/rfc7523

      Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      ---
      M clientcredentials/clientcredentials.go
      M clientcredentials/clientcredentials_test.go
      M internal/token.go
      M internal/token_test.go
      M jws/jws.go
      M oauth2.go
      M oauth2_test.go
      M token.go
      8 files changed, 352 insertions(+), 28 deletions(-)

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: newpatchset
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 5
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Matthew Hickford <hick...@google.com>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>

      Dileep Reddem (Gerrit)

      unread,
      May 3, 2023, 4:15:11 AM5/3/23
      to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Matthew Hickford, adam strickland.

      Dileep Reddem uploaded patch set #6 to this change.

      View Change

      x/oauth2: Adding support for private_key_jwt clientAuthentication method.


      Details:
      OAuth2 spec describes a secure way of sending token API by using private_key_jwt. This method avoids sending client_secret, and hence is extremely secure.

      This is described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

      private_key_jwt is defined in https://datatracker.ietf.org/doc/html/rfc7523

      Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      ---
      M clientcredentials/clientcredentials.go
      M clientcredentials/clientcredentials_test.go
      M internal/token.go
      M internal/token_test.go
      M jws/jws.go
      M oauth2.go
      M oauth2_test.go
      M token.go
      8 files changed, 375 insertions(+), 28 deletions(-)

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: newpatchset
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 6

      Dileep Reddem (Gerrit)

      unread,
      May 3, 2023, 4:16:05 AM5/3/23
      to goph...@pubsubhelper.golang.org, adam strickland, Gopher Robot, Matthew Hickford, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Matthew Hickford, adam strickland.

      View Change

      13 comments:

      • File internal/token.go:

        • No need for this comment here; the documentation on oauth2. […]

          Done

        • Patch Set #4, Line 169: return fmt.Sprintf("%x", bytes), nil

          `base64.RawURLEncoding.EncodeToString(bytes)` would be more compact.

        • Done. Changed to the suggested API. PTAL.

        • You can simplify this by dropping `newPivateKeyJWTTokenRequest` and changing this to: […]

          Done

        • Patch Set #4, Line 269: func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL, keyID string, privateKey []byte, v url.Values, authStyle AuthStyle) (*Token, error) {

        • This parameter list is getting very long, and too many of the parameters have the same type. […]

          Done

      • File jws/jws.go:

        • ``` […]

          Done

      • File oauth2.go:

        • Leaving AutoDetect to apply only to InParams/InHeader seems fine. […]

          Changed the comment. PTAL

      • File oauth2.go:

        • ``` […]

          Okay, will change this to match with jwt.Config

        • As currently implemented, this private key is reparsed on every use. […]

          May be I'm missing something. I cannot find where the `jwt` module parses and stores it. I see that it uses `ParseKey` from internal/oauth2.go but don't see where it's stored. I agree that storing it for subsequent uses is good. But, how do determine if the key is changed?

        • ``` […]

          Done

        • Done

        • ``` […]

          Done

        • This change doesn't seem to require adding an exported Validate method. […]

          Sure. We could make it internal.

        • When using private_key_jwt, PrivateKey must always be specified, no? […]

          updated validate() to handle these cases.

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 6
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Matthew Hickford <hick...@google.com>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>
      Gerrit-Comment-Date: Wed, 03 May 2023 08:15:58 +0000

      Dileep Reddem (Gerrit)

      unread,
      May 3, 2023, 7:45:24 AM5/3/23
      to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Matthew Hickford, adam strickland.

      Dileep Reddem uploaded patch set #7 to this change.

      View Change

      x/oauth2: Adding support for private_key_jwt clientAuthentication method.

      Details:
      OAuth2 spec describes a secure way of sending token API by using private_key_jwt. This method avoids sending client_secret, and hence is extremely secure.

      This is described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

      private_key_jwt is defined in https://datatracker.ietf.org/doc/html/rfc7523

      Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      ---
      M clientcredentials/clientcredentials.go
      M clientcredentials/clientcredentials_test.go
      M internal/token.go
      M internal/token_test.go
      M jws/jws.go
      M oauth2.go
      M oauth2_test.go
      M token.go
      8 files changed, 552 insertions(+), 28 deletions(-)

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: newpatchset
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 7

      Dileep Reddem (Gerrit)

      unread,
      May 3, 2023, 7:47:18 AM5/3/23
      to goph...@pubsubhelper.golang.org, adam strickland, Gopher Robot, Matthew Hickford, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Matthew Hickford, adam strickland.

      View Change

      1 comment:

      • File clientcredentials/clientcredentials.go:

        • I believe that finishing support here will be a prerequisite for landing this feature; we don't want […]

          Sure. Added support for that. PTAL.

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 7
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Matthew Hickford <hick...@google.com>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>
      Gerrit-Comment-Date: Wed, 03 May 2023 11:47:11 +0000

      Dileep Reddem (Gerrit)

      unread,
      Jun 4, 2023, 8:46:35 AM6/4/23
      to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Matthew Hickford, adam strickland.

      Dileep Reddem uploaded patch set #8 to this change.

      View Change

      x/oauth2: Adding support for private_key_jwt clientAuthentication method.

      Details:
      OAuth2 spec describes a secure way of sending token API by using private_key_jwt. This method avoids sending client_secret, and hence is extremely secure.

      This is described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

      private_key_jwt is defined in https://datatracker.ietf.org/doc/html/rfc7523

      Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      ---
      M clientcredentials/clientcredentials.go
      M clientcredentials/clientcredentials_test.go
      M internal/token.go
      M internal/token_test.go
      M jws/jws.go
      M oauth2.go
      M oauth2_test.go
      M token.go
      8 files changed, 631 insertions(+), 28 deletions(-)

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: newpatchset
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 8

      Dileep Reddem (Gerrit)

      unread,
      Jun 4, 2023, 8:52:05 AM6/4/23
      to goph...@pubsubhelper.golang.org, adam strickland, Gopher Robot, Matthew Hickford, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Matthew Hickford, adam strickland.

      View Change

      2 comments:

      • Patchset:

        • Patch Set #1:

          Hi Adam! That would be awesome. […]

          Any update on this one?

      • File oauth2.go:

        • Patch Set #4, Line 65: PrivateKey []byte

          May be I'm missing something. I cannot find where the `jwt` module parses and stores it. […]

          Added a cache. Right now it is a map. So, it can grow to arbitrary size. It's small (string -> pointer). But, might be a good thing to do pruning in the future. Or we could just hold one item only. Holding one item might not work if multiple callers are involved.
          PTAL and let me know if we should implement only one key or if we should restrict to any max items.

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 8
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Matthew Hickford <hick...@google.com>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>
      Gerrit-Comment-Date: Sun, 04 Jun 2023 12:52:01 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Damien Neil <dn...@google.com>

      Brad Fitzpatrick (Gerrit)

      unread,
      Jun 12, 2023, 2:24:28 PM6/12/23
      to Dileep Reddem, goph...@pubsubhelper.golang.org, adam strickland, Gopher Robot, Matthew Hickford, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com

      Attention is currently required from: Damien Neil, Dileep Reddem, Matthew Hickford, adam strickland.

      View Change

      2 comments:

        • // PrivateKey contains the contents of an RSA private key or the


        • // PEM containers with a passphrase are not supported.

        • 	PrivateKey []byte



        • // PrivateKeyID is an optional hint indicating which key is being used.

        • 	PrivateKeyID string

          The "Config" type docs above say this type "describes a 2-legged OAuth2 flow".

          These fields aren't part of a 2-legged OAuth2 flow, right?

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 8
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Matthew Hickford <hick...@google.com>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Dileep Reddem <dileep...@google.com>
      Gerrit-Comment-Date: Mon, 12 Jun 2023 18:24:25 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No

      Dileep Reddem (Gerrit)

      unread,
      Jun 12, 2023, 10:49:52 PM6/12/23
      to goph...@pubsubhelper.golang.org, adam strickland, Gopher Robot, Matthew Hickford, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Matthew Hickford, adam strickland.

      View Change

      2 comments:

      • File clientcredentials/clientcredentials.go:

        • Patch Set #8, Line 58: // PrivateKeyID is an optional hint indicating which key is being used.

          What form is this string in?

        • Patch Set #8, Line 52:

          // PrivateKey contains the contents of an RSA private key or the
          // contents of a PEM file that contains a private key. The provided
          // private key is used to sign the Token request (https://openid.net/specs/openid-connect-core-1_0.html#TokenEndpoint).
          // PEM containers with a passphrase are not supported.
          PrivateKey []byte

          // PrivateKeyID is an optional hint indicating which key is being used.
          PrivateKeyID string

          The "Config" type docs above say this type "describes a 2-legged OAuth2 flow". […]

        • These are applicable in the 2-Legged OAuth flow as well. This can be used in lieu of client_secret when sending request for client_credentials grant. This is a more secure option. Essentially, this is a way to 'authenticate'/verify the client's identity.

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 8
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Matthew Hickford <hick...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>
      Gerrit-Comment-Date: Tue, 13 Jun 2023 02:49:47 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Brad Fitzpatrick <brad...@golang.org>

      Damien Neil (Gerrit)

      unread,
      Jul 20, 2023, 7:59:40 PM7/20/23
      to Dileep Reddem, goph...@pubsubhelper.golang.org, adam strickland, Gopher Robot, Matthew Hickford, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Dileep Reddem, Matthew Hickford, adam strickland.

      View Change

      2 comments:

      • File oauth2.go:

        • Added a cache. Right now it is a map. So, it can grow to arbitrary size. […]

          As you point out, using a map has issues with size growth over time. We could attempt to address that in one way or another, but this seems like unnecessary complexity. I really think this should take the parsed key as input, or possibly a crypto.Signer.

          Commented on https://go.dev/issue/57186; I think that issue has more visibility, so let's work out the API there. I think the API seems fine, with the exception of the private key/signer.

      • File oauth2.go:

        • Patch Set #8, Line 68: PrivateKeyID string

          The doc comment says optional, but the validate function below requires this to be set. Which is it?

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 8
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Matthew Hickford <hick...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Dileep Reddem <dileep...@google.com>
      Gerrit-Comment-Date: Thu, 20 Jul 2023 23:59:35 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No

      Dileep Reddem (Gerrit)

      unread,
      Dec 21, 2023, 1:37:58 AM12/21/23
      to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Dileep Reddem, Matthew Hickford, adam strickland.

      Dileep Reddem uploaded patch set #9 to this change.

      View Change

      x/oauth2: Adding support for private_key_jwt clientAuthentication method.

      Details:
      OAuth2 spec describes a secure way of sending token API by using private_key_jwt. This method avoids sending client_secret, and hence is extremely secure.

      This is described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

      private_key_jwt is defined in https://datatracker.ietf.org/doc/html/rfc7523

      Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      ---
      M clientcredentials/clientcredentials.go
      M clientcredentials/clientcredentials_test.go
      M endpoints/endpoints.go

      M internal/token.go
      M internal/token_test.go
      M jws/jws.go
      M oauth2.go
      M oauth2_test.go
      M token.go
      9 files changed, 638 insertions(+), 30 deletions(-)

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: newpatchset
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 9
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>

      Dileep Reddem (Gerrit)

      unread,
      Dec 21, 2023, 1:44:03 AM12/21/23
      to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Dileep Reddem, Matthew Hickford, adam strickland.

      Dileep Reddem uploaded patch set #10 to this change.

      View Change

      x/oauth2: Adding support for private_key_jwt clientAuthentication method.

      Details:
      OAuth2 spec describes a secure way of sending token API by using private_key_jwt. This method avoids sending client_secret, and hence is extremely secure.

      This is described at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

      private_key_jwt is defined in https://datatracker.ietf.org/doc/html/rfc7523

      Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      ---
      M clientcredentials/clientcredentials.go
      M clientcredentials/clientcredentials_test.go
      M endpoints/endpoints.go
      M internal/token.go
      M internal/token_test.go
      M jws/jws.go
      M oauth2.go
      M oauth2_test.go
      M token.go
      9 files changed, 638 insertions(+), 30 deletions(-)

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: newpatchset
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 10

      Dileep Reddem (Gerrit)

      unread,
      Dec 21, 2023, 1:45:06 AM12/21/23
      to goph...@pubsubhelper.golang.org, triciu...@appspot.gserviceaccount.com, adam strickland, Gopher Robot, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Matthew Hickford, adam strickland.

      View Change

      1 comment:

      • File oauth2.go:

        • The doc comment says optional, but the validate function below requires this to be set. […]

          Done.

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 10
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Matthew Hickford <hick...@google.com>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>
      Gerrit-Comment-Date: Thu, 21 Dec 2023 06:45:00 +0000

      Dileep Reddem (Gerrit)

      unread,
      Dec 21, 2023, 1:46:05 AM12/21/23
      to goph...@pubsubhelper.golang.org, triciu...@appspot.gserviceaccount.com, adam strickland, Gopher Robot, Damien Neil, Brad Fitzpatrick, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com

      Attention is currently required from: Brad Fitzpatrick, Damien Neil, Matthew Hickford, adam strickland.

      View Change

      1 comment:

        • Any update on this one?

      To view, visit change 435795. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-MessageType: comment
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 10
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: JBD <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Matthew Hickford <hick...@google.com>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>
      Gerrit-Comment-Date: Thu, 21 Dec 2023 06:45:58 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Damien Neil <dn...@google.com>

      Sean Liao (Gerrit)

      unread,
      Apr 16, 2025, 4:11:42 PMApr 16
      to Dileep Reddem, goph...@pubsubhelper.golang.org, triciu...@appspot.gserviceaccount.com, adam strickland, Gopher Robot, Damien Neil, Brad Fitzpatrick, Jaana Dogan, Chris Broadfoot, Shin Fan, Cody Oss, golang-co...@googlegroups.com
      Attention needed from Brad Fitzpatrick, Damien Neil, Dileep Reddem, Matthew Hickford and adam strickland

      Sean Liao voted and added 1 comment

      Votes added by Sean Liao

      Hold+1

      1 comment

      Patchset-level comments
      Open in Gerrit

      Related details

      Attention is currently required from:
      • Brad Fitzpatrick
      • Damien Neil
      • Dileep Reddem
      • Matthew Hickford
      • adam strickland
      Submit Requirements:
      • requirement is not satisfiedCode-Review
      • requirement is not satisfiedNo-Holds
      • requirement is not satisfiedNo-Unresolved-Comments
      • requirement is not satisfiedReview-Enforcement
      • requirement is not satisfiedTryBots-Pass
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: oauth2
      Gerrit-Branch: master
      Gerrit-Change-Id: I4c289a28b624990c6558fb72314a193ff28f1071
      Gerrit-Change-Number: 435795
      Gerrit-PatchSet: 10
      Gerrit-Owner: Dileep Reddem <dileep...@google.com>
      Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Reviewer: Damien Neil <dn...@google.com>
      Gerrit-Reviewer: Gopher Robot <go...@golang.org>
      Gerrit-Reviewer: Matthew Hickford <hick...@google.com>
      Gerrit-Reviewer: Sean Liao <se...@liao.dev>
      Gerrit-CC: Chris Broadfoot <cb...@golang.org>
      Gerrit-CC: Cody Oss <cod...@google.com>
      Gerrit-CC: Jaana Dogan <j...@google.com>
      Gerrit-CC: Shin Fan <shi...@google.com>
      Gerrit-CC: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Matthew Hickford <hick...@google.com>
      Gerrit-Attention: Damien Neil <dn...@google.com>
      Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
      Gerrit-Attention: adam strickland <adamstri...@gmail.com>
      Gerrit-Attention: Dileep Reddem <dileep...@google.com>
      Gerrit-Comment-Date: Wed, 16 Apr 2025 20:11:35 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      unsatisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages