-
Notifications
You must be signed in to change notification settings - Fork 352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support custom json and base64 encoders for Token and Parser #301
base: main
Are you sure you want to change the base?
Changes from all commits
90d315c
8089d9e
7a5e5d6
f0fa303
7684d3e
3ae2a4a
f64f460
5e2ab08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package jwt | ||
|
||
import "io" | ||
|
||
// Base64Encoding represents an object that can encode and decode base64. A | ||
// common example is [encoding/base64.Encoding]. | ||
type Base64Encoding interface { | ||
EncodeToString(src []byte) string | ||
DecodeString(s string) ([]byte, error) | ||
} | ||
|
||
type StrictFunc[T Base64Encoding] func() T | ||
|
||
type Stricter[T Base64Encoding] interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This at least needs a comment if we want to keep It exported, it could probably also be private I guess |
||
Strict() T | ||
} | ||
|
||
func DoStrict[S Base64Encoding, T Stricter[S]](x T) Base64Encoding { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Double-check if this is actually still needed, which I think it is not? |
||
return x.Strict() | ||
} | ||
|
||
// JSONMarshalFunc is a function type that allows to implement custom JSON | ||
// encoding algorithms. | ||
type JSONMarshalFunc func(v any) ([]byte, error) | ||
|
||
// JSONUnmarshalFunc is a function type that allows to implement custom JSON | ||
// unmarshal algorithms. | ||
type JSONUnmarshalFunc func(data []byte, v any) error | ||
|
||
type JSONDecoder interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs a comment because its a public interface |
||
UseNumber() | ||
Decode(v any) error | ||
} | ||
|
||
type JSONNewDecoderFunc[T JSONDecoder] func(r io.Reader) T | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs a comment |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,16 +12,26 @@ type Parser struct { | |
// If populated, only these methods will be considered valid. | ||
validMethods []string | ||
|
||
// Use JSON Number format in JSON decoder. | ||
useJSONNumber bool | ||
|
||
// Skip claims validation during token parsing. | ||
skipClaimsValidation bool | ||
|
||
validator *Validator | ||
|
||
decodeStrict bool | ||
decoding | ||
} | ||
|
||
type decoding struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its not private, but still we probably could use some comments here, especially for this outer struct |
||
jsonUnmarshal JSONUnmarshalFunc | ||
jsonNewDecoder JSONNewDecoderFunc[JSONDecoder] | ||
|
||
rawUrlBase64Encoding Base64Encoding | ||
urlBase64Encoding Base64Encoding | ||
strict StrictFunc[Base64Encoding] | ||
|
||
// Use JSON Number format in JSON decoder. | ||
useJSONNumber bool | ||
|
||
decodeStrict bool | ||
decodePaddingAllowed bool | ||
} | ||
|
||
|
@@ -148,7 +158,18 @@ func (p *Parser) ParseUnverified(tokenString string, claims Claims) (token *Toke | |
if headerBytes, err = p.DecodeSegment(parts[0]); err != nil { | ||
return token, parts, newError("could not base64 decode header", ErrTokenMalformed, err) | ||
} | ||
if err = json.Unmarshal(headerBytes, &token.Header); err != nil { | ||
|
||
// Choose our JSON decoder. If no custom function is supplied, we use the standard library. | ||
var unmarshal JSONUnmarshalFunc | ||
if p.jsonUnmarshal != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, why are we determining whether a custom JSON marshaller is present in the Parse() method instead of during the Parser creation? The same applies to other places. Parser.jsonUnmarshal should be initialized during Parser construction, as we're not planning to replace JSON marshallers or base64 encoders on the fly. |
||
unmarshal = p.jsonUnmarshal | ||
} else { | ||
unmarshal = json.Unmarshal | ||
} | ||
|
||
// JSON Unmarshal the header | ||
err = unmarshal(headerBytes, &token.Header) | ||
if err != nil { | ||
return token, parts, newError("could not JSON decode header", ErrTokenMalformed, err) | ||
} | ||
|
||
|
@@ -160,25 +181,31 @@ func (p *Parser) ParseUnverified(tokenString string, claims Claims) (token *Toke | |
return token, parts, newError("could not base64 decode claim", ErrTokenMalformed, err) | ||
} | ||
|
||
// If `useJSONNumber` is enabled then we must use *json.Decoder to decode | ||
// the claims. However, this comes with a performance penalty so only use | ||
// it if we must and, otherwise, simple use json.Unmarshal. | ||
if !p.useJSONNumber { | ||
// JSON Unmarshal. Special case for map type to avoid weird pointer behavior. | ||
if c, ok := token.Claims.(MapClaims); ok { | ||
err = json.Unmarshal(claimBytes, &c) | ||
} else { | ||
err = json.Unmarshal(claimBytes, &claims) | ||
// If `useJSONNumber` is enabled, then we must use a dedicated JSONDecoder | ||
// to decode the claims. However, this comes with a performance penalty so | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. small nit: we only know definitly that the performance penalty occurs when using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any evidence? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. #303 |
||
// only use it if we must and, otherwise, simple use our existing unmarshal | ||
// function. | ||
if p.useJSONNumber { | ||
unmarshal = func(data []byte, v any) error { | ||
buffer := bytes.NewBuffer(claimBytes) | ||
|
||
var decoder JSONDecoder | ||
if p.jsonNewDecoder != nil { | ||
decoder = p.jsonNewDecoder(buffer) | ||
} else { | ||
decoder = json.NewDecoder(buffer) | ||
} | ||
decoder.UseNumber() | ||
return decoder.Decode(v) | ||
} | ||
} | ||
|
||
// JSON Unmarshal the claims. Special case for map type to avoid weird | ||
// pointer behavior. | ||
if c, ok := token.Claims.(MapClaims); ok { | ||
err = unmarshal(claimBytes, &c) | ||
} else { | ||
dec := json.NewDecoder(bytes.NewBuffer(claimBytes)) | ||
dec.UseNumber() | ||
// JSON Decode. Special case for map type to avoid weird pointer behavior. | ||
if c, ok := token.Claims.(MapClaims); ok { | ||
err = dec.Decode(&c) | ||
} else { | ||
err = dec.Decode(&claims) | ||
} | ||
err = unmarshal(claimBytes, &claims) | ||
} | ||
if err != nil { | ||
return token, parts, newError("could not JSON decode claim", ErrTokenMalformed, err) | ||
|
@@ -200,18 +227,37 @@ func (p *Parser) ParseUnverified(tokenString string, claims Claims) (token *Toke | |
// take into account whether the [Parser] is configured with additional options, | ||
// such as [WithStrictDecoding] or [WithPaddingAllowed]. | ||
func (p *Parser) DecodeSegment(seg string) ([]byte, error) { | ||
encoding := base64.RawURLEncoding | ||
var encoding Base64Encoding | ||
if p.rawUrlBase64Encoding != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same comment about stage where custom base64 encoder function set. We can set p.rawUrlBase64Encoding to base64.RawURLEncoding or custom function on Parser creation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this as a safe guard if people ever came across the crazy idea to directly create a Parser struct, which is unfortunately possible because we did not hide it behind an interface. But yes, if we assume that in the case you are doing that you REALLY know what you are doing we can do it in the init There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that sounds reasonable. |
||
encoding = p.rawUrlBase64Encoding | ||
} else { | ||
encoding = base64.RawURLEncoding | ||
} | ||
|
||
if p.decodePaddingAllowed { | ||
if l := len(seg) % 4; l > 0 { | ||
seg += strings.Repeat("=", 4-l) | ||
} | ||
encoding = base64.URLEncoding | ||
|
||
if p.urlBase64Encoding != nil { | ||
encoding = p.urlBase64Encoding | ||
} else { | ||
encoding = base64.URLEncoding | ||
} | ||
} | ||
|
||
if p.decodeStrict { | ||
encoding = encoding.Strict() | ||
if p.strict != nil { | ||
encoding = p.strict() | ||
} else { | ||
stricter, ok := encoding.(Stricter[*base64.Encoding]) | ||
if !ok { | ||
return nil, newError("WithStrictDecoding() was enabled but supplied base64 encoder does not support strict mode", ErrUnsupported) | ||
} | ||
encoding = stricter.Strict() | ||
} | ||
} | ||
|
||
return encoding.DecodeString(seg) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,9 @@ | ||
package jwt | ||
|
||
import "time" | ||
import ( | ||
"io" | ||
"time" | ||
) | ||
|
||
// ParserOption is used to implement functional-style options that modify the | ||
// behavior of the parser. To add new options, just create a function (ideally | ||
|
@@ -121,8 +124,80 @@ func WithPaddingAllowed() ParserOption { | |
// WithStrictDecoding will switch the codec used for decoding JWTs into strict | ||
// mode. In this mode, the decoder requires that trailing padding bits are zero, | ||
// as described in RFC 4648 section 3.5. | ||
// | ||
// Note: This is only supported when using [encoding/base64.Encoding], but not | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this note is not true anymore, since we are checking whether the library supports a |
||
// by any other decoder specified with [WithBase64Decoder]. | ||
func WithStrictDecoding() ParserOption { | ||
return func(p *Parser) { | ||
p.decodeStrict = true | ||
} | ||
} | ||
|
||
// WithJSONDecoder supports a custom JSON decoder to use in parsing the JWT. | ||
// There are two functions that can be supplied: | ||
// - jsonUnmarshal is a [JSONUnmarshalFunc] that is used for the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo: double "the" |
||
// un-marshalling the header and claims when no other options are specified | ||
// - jsonNewDecoder is a [JSONNewDecoderFunc] that is used to create an object | ||
// satisfying the [JSONDecoder] interface. | ||
// | ||
// The latter is used when the [WithJSONNumber] option is used. | ||
// | ||
// If any of the supplied functions is set to nil, the defaults from the Go | ||
// standard library, [encoding/json.Unmarshal] and [encoding/json.NewDecoder] | ||
// are used. | ||
// | ||
// Example using the https://github.com/bytedance/sonic library. | ||
// | ||
// import ( | ||
// "github.com/bytedance/sonic" | ||
// ) | ||
// | ||
// var parser = jwt.NewParser(jwt.WithJSONDecoder(sonic.Unmarshal, sonic.ConfigDefault.NewDecoder)) | ||
func WithJSONDecoder[T JSONDecoder](jsonUnmarshal JSONUnmarshalFunc, jsonNewDecoder JSONNewDecoderFunc[T]) ParserOption { | ||
return func(p *Parser) { | ||
p.jsonUnmarshal = jsonUnmarshal | ||
// This seems to be necessary, since we don't want to store the specific | ||
// JSONDecoder type in our parser, but need it in the function | ||
// interface. | ||
p.jsonNewDecoder = func(r io.Reader) JSONDecoder { | ||
return jsonNewDecoder(r) | ||
} | ||
} | ||
} | ||
|
||
// WithBase64Decoder supports a custom Base64 when decoding a base64 encoded | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. grammar: "a custom Base64 [something missing here]" |
||
// token. Two encoding can be specified: | ||
// - rawURL needs to contain a [Base64Encoding] that is based on base64url | ||
// without padding. This is used for parsing tokens with the default | ||
// options. | ||
// - url needs to contain a [Base64Encoding] based on base64url with padding. | ||
// The sole use of this to decode tokens when [WithPaddingAllowed] is | ||
// enabled. | ||
// | ||
// If any of the supplied encodings are set to nil, the defaults from the Go | ||
// standard library, [encoding/base64.RawURLEncoding] and | ||
// [encoding/base64.URLEncoding] are used. | ||
// | ||
// Example using the https://github.com/segmentio/asm library. | ||
// | ||
// import ( | ||
// asmbase64 "github.com/segmentio/asm/base64" | ||
// ) | ||
// | ||
// var parser = jwt.NewParser(jwt.WithBase64Decoder(asmbase64.RawURLEncoding, asmbase64.URLEncoding)) | ||
func WithBase64Decoder[T Base64Encoding](rawURL Base64Encoding, url T) ParserOption { | ||
return func(p *Parser) { | ||
p.rawUrlBase64Encoding = rawURL | ||
p.urlBase64Encoding = url | ||
|
||
// Check, whether the library supports the Strict() function | ||
stricter, ok := rawURL.(Stricter[T]) | ||
if ok { | ||
// We need to get rid of the type parameter T, so we need to wrap it | ||
// here | ||
p.strict = func() Base64Encoding { | ||
return stricter.Strict() | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this type really needs to be exported at all