code review 6884049: jwt: Add support for external signers, ID tokens.

jwt: Add support for external signers, ID tokens.

This allows an external agent to be responsible for signing
the token generated by the package.  Extract expiry from
returned ID token if given.  Added and updated relevant
testcases.

[sledbetter, 2012-12-05 22:55:43]

Hello adg@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/goauth2/

[adg, 2012-12-07 03:39:24]

Hey, thanks!

Can you please sign the CLA, before I review this?

http://golang.org/doc/contribute.html#Copyright

[adg, 2012-12-12 02:44:38]

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go
File oauth/jwt/jwt.go (right):

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go#newcode38
oauth/jwt/jwt.go:38: package jwt
Can I get an example of this new functionality in action?

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go#newcode76
oauth/jwt/jwt.go:76: func urlEncode(b []byte) string {
let's rename these to base64Encode and base64Decode, as that's what they do

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go#newcode205
oauth/jwt/jwt.go:205: if len(t.keyID) != 0 {
header := struct{
  Algorithm string `json:"alg"`
  Type string `json:"typ"`
  KeyId string `json:"kid,omitempty"`
}{stdAlgorithm, stdType, t.keyId}
b, err := json.Marshal(header)
if err != nil {
  panic(err)
}
return urlEncode(b)

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go#newcode211
oauth/jwt/jwt.go:211: func (t *Token) SetKeyID(kid string) {
Why not just add a KeyID field if you want it to be mutable?

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go#newcode349
oauth/jwt/jwt.go:349: d, err := urlDecode(s[1])
must check if len(s) >=1 before using s[1]

[sledbetter, 2012-12-13 00:17:34]

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go
File oauth/jwt/jwt.go (right):

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go#newcode38
oauth/jwt/jwt.go:38: package jwt
On 2012/12/12 02:44:38, adg wrote:
> Can I get an example of this new functionality in action?

I have a simple pass through example in jwt_test.go; I also added another
example here in the comments.

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go#newcode76
oauth/jwt/jwt.go:76: func urlEncode(b []byte) string {
On 2012/12/12 02:44:38, adg wrote:
> let's rename these to base64Encode and base64Decode, as that's what they do

Done.

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go#newcode205
oauth/jwt/jwt.go:205: if len(t.keyID) != 0 {
On 2012/12/12 02:44:38, adg wrote:
> header := struct{
>   Algorithm string `json:"alg"`
>   Type string `json:"typ"`
>   KeyId string `json:"kid,omitempty"`
> }{stdAlgorithm, stdType, t.keyId}
> b, err := json.Marshal(header)
> if err != nil {
>   panic(err)
> }
> return urlEncode(b)

I pulled out the Header struct and did more json.Marshal elsewhere to clean this
up.

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go#newcode211
oauth/jwt/jwt.go:211: func (t *Token) SetKeyID(kid string) {
On 2012/12/12 02:44:38, adg wrote:
> Why not just add a KeyID field if you want it to be mutable?
Good point. Fixed.

https://codereview.appspot.com/6884049/diff/4001/oauth/jwt/jwt.go#newcode349
oauth/jwt/jwt.go:349: d, err := urlDecode(s[1])
On 2012/12/12 02:44:38, adg wrote:
> must check if len(s) >=1 before using s[1]

Done.

[adg, 2012-12-13 04:00:32]

LGTM

https://codereview.appspot.com/6884049/diff/12001/oauth/jwt/jwt.go
File oauth/jwt/jwt.go (right):

https://codereview.appspot.com/6884049/diff/12001/oauth/jwt/jwt.go#newcode154
oauth/jwt/jwt.go:154: fmt.Printf("header=%q", string(b))
errant debug print

https://codereview.appspot.com/6884049/diff/12001/oauth/jwt/jwt.go#newcode226
oauth/jwt/jwt.go:226: 
delete this line so the comment appears in godoc

also take a look at this package in godoc on your machine (godoc
-http=localhost:6006) to sanity check your formatting etc

[sledbetter, 2012-12-17 21:17:08]

Updated changes. I tried to submit but am it complained about me not being in
CONTRIBUTORS.

[adg, 2012-12-18 23:58:30]

*** Submitted as https://code.google.com/p/goauth2/source/detail?r=1f6bde75551d
***

jwt: Add support for external signers, ID tokens.

This allows an external agent to be responsible for signing
the token generated by the package.  Extract expiry from
returned ID token if given.  Added and updated relevant
testcases.

R=adg
CC=golang-dev
https://codereview.appspot.com/6884049

Committer: Andrew Gerrand <adg@golang.org>

[adg, 2012-12-18 23:58:43]

A snafu, sorry: https://codereview.appspot.com/6955043/