mirror of
https://github.com/VictoriaMetrics/VictoriaMetrics.git
synced 2026-05-17 00:26:36 +03:00
lib/jwt: address code review comments (#10428)
### Describe Your Changes Addressing code revoew comments from https://github.com/VictoriaMetrics/VictoriaMetrics/pull/10426, kept them separate to isolate copy-paste change from follow up changes ### Checklist The following checks are **mandatory**: - [ ] My change adheres to [VictoriaMetrics contributing guidelines](https://docs.victoriametrics.com/victoriametrics/contributing/#pull-request-checklist). - [ ] My change adheres to [VictoriaMetrics development goals](https://docs.victoriametrics.com/victoriametrics/goals/).
This commit is contained in:
@@ -5,7 +5,7 @@ import (
|
||||
"crypto/rsa"
|
||||
)
|
||||
|
||||
// newVerifierPS returns a new RSA-PSS-based signer.
|
||||
// newVerifierPS returns a new RSA-PSS-based verifier.
|
||||
func newVerifierPS(alg Algorithm, key *rsa.PublicKey) (*psAlg, error) {
|
||||
if key == nil {
|
||||
return nil, ErrNilKey
|
||||
|
||||
@@ -98,8 +98,15 @@ func NewToken(auth string, enforceAuthPrefix bool) (*Token, error) {
|
||||
if enforceAuthPrefix && (len(auth) < len(prefix) || !strings.EqualFold(auth[:len(prefix)], prefix)) {
|
||||
return nil, fmt.Errorf("wrong format, prefix: %s is missing", prefix)
|
||||
}
|
||||
// remove prefix if it is present
|
||||
auth = strings.TrimPrefix(auth, prefix)
|
||||
|
||||
// While https://datatracker.ietf.org/doc/html/rfc6750#section-2.1 states that only Bearer prefix is allowed,
|
||||
// it claims to be conformant to the generic syntax defined in https://datatracker.ietf.org/doc/html/rfc2617#section-1.2
|
||||
// which permits case-insensitive auth scheme.
|
||||
// So we should be tolerant to different cases of "Bearer" prefix.
|
||||
if len(auth) >= len(prefix) && strings.EqualFold(auth[:len(prefix)], prefix) {
|
||||
auth = auth[len(prefix):]
|
||||
}
|
||||
|
||||
jwt := strings.SplitN(auth, ".", 3)
|
||||
if len(jwt) != 3 {
|
||||
return nil, ErrBadTokenFormat
|
||||
@@ -203,9 +210,9 @@ func parseJWTBody(data string) (*body, error) {
|
||||
// expired at time unix_ts
|
||||
Exp int64 `json:"exp"`
|
||||
// issued at time unix_ts
|
||||
Iat int64 `json:"iat"`
|
||||
Jti string `json:"jti,omitempty"`
|
||||
Scope *json.RawMessage `json:"scope,omitempty"`
|
||||
Iat int64 `json:"iat"`
|
||||
Jti string `json:"jti,omitempty"`
|
||||
Scope json.RawMessage `json:"scope,omitempty"`
|
||||
// store as raw message to support different types
|
||||
VMAccess *json.RawMessage `json:"vm_access"`
|
||||
}
|
||||
@@ -240,9 +247,9 @@ func parseJWTBody(data string) (*body, error) {
|
||||
// some IDPs encode scope as a string and some as an array
|
||||
var scope string
|
||||
if tb.Scope != nil {
|
||||
if err := json.Unmarshal(*tb.Scope, &scope); err != nil {
|
||||
if err := json.Unmarshal(tb.Scope, &scope); err != nil {
|
||||
var scopeSlice []string
|
||||
if err := json.Unmarshal(*tb.Scope, &scopeSlice); err != nil {
|
||||
if err := json.Unmarshal(tb.Scope, &scopeSlice); err != nil {
|
||||
return nil, fmt.Errorf("cannot parse jwt body scope: %w", err)
|
||||
}
|
||||
scope = strings.Join(scopeSlice, " ")
|
||||
@@ -262,7 +269,7 @@ func parseJWTBody(data string) (*body, error) {
|
||||
func decodeB64(data []byte) ([]byte, error) {
|
||||
idx := bytes.IndexAny(data, "+/")
|
||||
// slow path, std base64 encoding convert it to url encoding
|
||||
if idx > 0 {
|
||||
if idx >= 0 {
|
||||
for idx, c := range data {
|
||||
switch c {
|
||||
case '+':
|
||||
|
||||
@@ -21,7 +21,8 @@ func TestParseJWTHeader_Failure(t *testing.T) {
|
||||
if err.Error() != expectedErr {
|
||||
t.Errorf("unexpected error message: \ngot\n%s\nwant\n%s", err.Error(), expectedErr)
|
||||
}
|
||||
return
|
||||
} else {
|
||||
t.Errorf("expecting non-nil error")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -97,7 +98,8 @@ func TestParseJWTBody_Failure(t *testing.T) {
|
||||
if err.Error() != expectedErr {
|
||||
t.Errorf("unexpected error message: \ngot\n%s\nwant\n%s", err.Error(), expectedErr)
|
||||
}
|
||||
return
|
||||
} else {
|
||||
t.Errorf("expecting non-nil error")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -404,10 +406,10 @@ func TestNewTokenFromRequest_Success(t *testing.T) {
|
||||
t.Fatalf("NewTokenFromRequest() error: %s", err)
|
||||
}
|
||||
if !reflect.DeepEqual(result.body.VMAccess, resultExpected.body.VMAccess) {
|
||||
t.Fatalf("unxpected token body VMAccess;\ngot\n%v\nwant\n%v", result.body.VMAccess, resultExpected.body.VMAccess)
|
||||
t.Fatalf("unexpected token body VMAccess;\ngot\n%v\nwant\n%v", result.body.VMAccess, resultExpected.body.VMAccess)
|
||||
}
|
||||
if !reflect.DeepEqual(result.header, resultExpected.header) {
|
||||
t.Fatalf("unxpected token header\ngot\n%v\nwant\n%v", result.header, resultExpected.header)
|
||||
t.Fatalf("unexpected token header\ngot\n%v\nwant\n%v", result.header, resultExpected.header)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -444,6 +446,39 @@ func TestNewTokenFromRequest_Success(t *testing.T) {
|
||||
}
|
||||
f(r, resultExpected, true)
|
||||
|
||||
// parse ok with non-standard "BEARER" prefix
|
||||
r = &http.Request{
|
||||
Header: map[string][]string{
|
||||
"Authorization": {
|
||||
"BEARER eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJhQVpvQ0d2dUdiRm9mdFdIeFFaeVJTUWVuM3lYNFUwR1BsUDVvWk9RU3djIn0.eyJleHAiOjE2MTA5NzYxODksImlhdCI6MTYxMDk3NTg4OSwiYXV0aF90aW1lIjoxNjEwOTc1ODg5LCJqdGkiOiI5YjE5NDE4Ny02YmI3LTQyNDQtOWQxYi01NTllYWIyZWY3ZjMiLCJpc3MiOiJodHRwczovL2xvY2FsaG9zdDo4NDQzL2F1dGgvcmVhbG1zL3Rlc3QiLCJhdWQiOiJhY2NvdW50Iiwic3ViIjoiNDYwODU5NDEtYjkyYi00NzFhLWIwNWEtOTU5OWNhMjlkYTFlIiwidHlwIjoiQmVhcmVyIiwiYXpwIjoiZ3JhZmFuYSIsInNlc3Npb25fc3RhdGUiOiIxMzc3ZDEwMi03NTJiLTQ0ODYtOTlkYS1jMjA4MjRiODJkMzEiLCJhY3IiOiIxIiwiYWxsb3dlZC1vcmlnaW5zIjpbImh0dHA6Ly9sb2NhbGhvc3Q6MzAwMCJdLCJyZWFsbV9hY2Nlc3MiOnsicm9sZXMiOlsib2ZmbGluZV9hY2Nlc3MiLCJ1bWFfYXV0aG9yaXphdGlvbiJdfSwicmVzb3VyY2VfYWNjZXNzIjp7ImFjY291bnQiOnsicm9sZXMiOlsibWFuYWdlLWFjY291bnQiLCJtYW5hZ2UtYWNjb3VudC1saW5rcyIsInZpZXctcHJvZmlsZSJdfX0sInNjb3BlIjoib3BlbmlkIGVtYWlsIHByb2ZpbGUiLCJ2bV9hY2Nlc3MiOnsiZXh0cmFfbGFiZWxzIjp7InByb2plY3QiOiJkZXYiLCJ0ZWFtIjoibW9iaWxlIn0sInRlbmFudF9pZCI6eyJhY2NvdW50X2lkIjoxLCJwcm9qZWN0X2lkIjo1fX0sImVtYWlsX3ZlcmlmaWVkIjpmYWxzZSwibmFtZSI6InRnIHRnIiwicHJvamVjdCI6Im1vYmlsZSIsInByZWZlcnJlZF91c2VybmFtZSI6InRnIiwidGVhbSI6ImRldiIsImdpdmVuX25hbWUiOiJ0ZyIsImZhbWlseV9uYW1lIjoidGciLCJlbWFpbCI6InRnQGZnaHQubmV0In0.XErPkz-qL-EV8BBAR17MoFytc5ajYRz71f9_GOuG1AVcMnUsD6D3x4z5jL1dLyoGGm8OUW_RIVrjMpZf_xOfgQKRVHAMaJi64UtpwS8EF50mlOCDAdKl6wlzAS4laV3dW9W9QrTH7TMetG33WVsJGaD-MIwSYJ5peh6u__oniezsRavw8Qw3nLpZCQPb-NatT3Q1raj1ymLJErJPtUBSk3ieWCVpTMo4ZYKFIQt2wjHeOVOF_3suhPfhgEgXlN6aUq3xeYJ1aAtl_5Ao3pB2pto46kDSXIulQQuGdttsw7bSDOYqZ-tx3y7DBWNdIcghsO_iMvrA805j5hG4Nu84Sw",
|
||||
},
|
||||
},
|
||||
}
|
||||
resultExpected = &Token{
|
||||
body: &body{
|
||||
Exp: 1610889266,
|
||||
Iat: 1610888966,
|
||||
Jti: "09a058a2-0752-4ecd-a4e9-b65e85af423f",
|
||||
Scope: "openid email profile",
|
||||
VMAccess: &access{
|
||||
Tenant: TenantID{
|
||||
ProjectID: 5,
|
||||
AccountID: 1,
|
||||
},
|
||||
Labels: map[string]string{
|
||||
"project": "dev",
|
||||
"team": "mobile",
|
||||
},
|
||||
},
|
||||
},
|
||||
header: &header{
|
||||
Alg: "RS256",
|
||||
Kid: "aAZoCGvuGbFoftWHxQZyRSQen3yX4U0GPlP5oZOQSwc",
|
||||
Typ: "JWT",
|
||||
},
|
||||
}
|
||||
f(r, resultExpected, true)
|
||||
|
||||
// go-jwt
|
||||
r = &http.Request{
|
||||
Header: map[string][]string{
|
||||
|
||||
@@ -10,7 +10,7 @@ func TestTokenVerify_Failure(t *testing.T) {
|
||||
|
||||
token, err := NewToken(auth, true)
|
||||
if err != nil {
|
||||
return
|
||||
t.Fatalf("unexpected error parsing token: %s", err)
|
||||
}
|
||||
|
||||
k, err := ParseKey([]byte(key))
|
||||
|
||||
Reference in New Issue
Block a user