-
Notifications
You must be signed in to change notification settings - Fork 374
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
Add support for RFC7797 unencoded payloads #561
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -29,7 +29,7 @@ Metrics/AbcSize: | |
Max: 25 | ||
|
||
Metrics/ClassLength: | ||
Max: 121 | ||
Max: 140 | ||
|
||
Metrics/ModuleLength: | ||
Max: 100 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,16 @@ def encode(payload, key, algorithm = 'HS256', header_fields = {}) | |
Encode.new(payload: payload, | ||
key: key, | ||
algorithm: algorithm, | ||
headers: header_fields).segments | ||
headers: header_fields, | ||
detached: false).segments | ||
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. Maybe we can set detached as default false |
||
end | ||
|
||
def encode_detached(payload, key, algorithm = 'HS256', header_fields = {}) | ||
Encode.new(payload: payload, | ||
key: key, | ||
algorithm: algorithm, | ||
headers: header_fields, | ||
detached: true).segments | ||
end | ||
|
||
def decode(jwt, key = nil, verify = true, options = {}, &keyfinder) # rubocop:disable Style/OptionalBooleanParameter | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ def initialize(jwt, key, verify, options, &keyfinder) | |
|
||
@jwt = jwt | ||
@key = key | ||
@detached_payload = options[:payload] | ||
@options = options | ||
@segments = jwt.split('.') | ||
@verify = verify | ||
|
@@ -152,15 +153,35 @@ def header | |
end | ||
|
||
def payload | ||
@payload ||= parse_and_decode @segments[1] | ||
@payload ||= parse_and_decode(encoded_payload, decode: decode_payload?) | ||
end | ||
|
||
def encoded_payload | ||
payload = encoded_detached_payload if !@detached_payload.nil? && @segments[1].empty? | ||
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 it would make sense to raise an error if a payload is given and the token also contains a payload, instead of just ignoring the given payload. |
||
payload ||= @segments[1] | ||
payload | ||
end | ||
|
||
def encoded_detached_payload | ||
payload ||= ::JWT::Base64.url_encode(JWT::JSON.generate(@detached_payload)) if decode_payload? | ||
payload ||= @detached_payload.to_json | ||
payload | ||
end | ||
|
||
def decode_payload? | ||
header['b64'].nil? || !!header['b64'] | ||
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 wonder if this is a bit problematic to change behavior because of user-input. Would it be too much of an inconvenience to have the caller decide to decode or not? |
||
end | ||
|
||
def signing_input | ||
@segments.first(2).join('.') | ||
[@segments[0], encoded_payload].join('.') | ||
end | ||
|
||
def parse_and_decode(segment) | ||
JWT::JSON.parse(::JWT::Base64.url_decode(segment)) | ||
def parse_and_decode(segment, decode: true) | ||
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. Would it be possible to avoid the parameter if the caller would just call parse and decode in separate? |
||
if decode | ||
JWT::JSON.parse(::JWT::Base64.url_decode(segment)) | ||
else | ||
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. We can avoid else statement with a guard clause. |
||
JWT::JSON.parse(segment) | ||
end | ||
rescue ::JSON::ParserError | ||
raise JWT::DecodeError, 'Invalid segment encoding' | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,11 +15,24 @@ def initialize(options) | |
@algorithm = resolve_algorithm(options[:algorithm]) | ||
@headers = options[:headers].transform_keys(&:to_s) | ||
@headers[ALG_KEY] = @algorithm.alg | ||
@detached = options[:detached] | ||
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. Here we can set as default false
|
||
|
||
# add b64 claim to crit as per RFC7797 proposed standard | ||
unless encode_payload? | ||
@headers['crit'] ||= [] | ||
@headers['crit'] << 'b64' unless @headers['crit'].include?('b64') | ||
end | ||
end | ||
|
||
def segments | ||
validate_claims! | ||
combine(encoded_header_and_payload, encoded_signature) | ||
|
||
parts = [] | ||
parts << encoded_header | ||
parts << (@detached ? '' : encoded_payload) | ||
parts << encoded_signature | ||
|
||
combine(*parts) | ||
end | ||
|
||
private | ||
|
@@ -51,7 +64,21 @@ def encode_header | |
end | ||
|
||
def encode_payload | ||
encode_data(@payload) | ||
# if b64 header is present and false, do not encode payload as per RFC7797 proposed standard | ||
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. Would it make sense to not rely on header values but instead have an explicit parameter that controls the behavior of both header values and payload encoding? |
||
encode_payload? ? encode_data(@payload) : prepare_unencoded_payload | ||
end | ||
|
||
def encode_payload? | ||
# if b64 header is left out, default to true as per RFC7797 proposed standard | ||
@headers['b64'].nil? || !!@headers['b64'] | ||
end | ||
|
||
def prepare_unencoded_payload | ||
json = @payload.to_json | ||
|
||
raise(JWT::InvalidUnencodedPayload, 'An unencoded payload cannot contain period/dot characters (i.e. ".").') if json.include?('.') | ||
|
||
json | ||
end | ||
|
||
def signature | ||
|
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.
The example could be a more simple one than one that required the rbnacl dependency.