-
-
Notifications
You must be signed in to change notification settings - Fork 109
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
chore: bump dependencies #795
Conversation
zepatrik
commented
Jul 9, 2024
github.com/auth0/go-jwt-middleware v1.0.1 => v2.2.1 github.com/docker/docker v20.10.27+incompatible => v27.0.3+incompatible github.com/form3tech-oss/jwt-go v3.2.5+incompatible => github.com/golang-jwt/jwt/v5 v5.2.1 v3.10.1-0.20240619125955-3328cf9343b8 github.com/ory/dockertest/v3 v3.10.1-0.20240619125955-3328cf9343b8 => v3.10.1-0.20240704115616-d229e74b748d github.com/rs/cors v1.8.2 => v1.11.0
}, | ||
// token without kid | ||
{ | ||
token: "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwiZXhwIjo5OTk5OTk5OTk5LCJzZXNzaW9uIjp7ImlkZW50aXR5Ijp7ImlkIjoiMTIzNDU2Nzg5MCJ9fX0.j0SgjC21nhkNP2QX0uE-I4wDYYRYlZq9wqGeDhrbplkKGW4BOjW5Sk0XFFbqrx68hQYz23QvYOYW5avUBzTjPxHwVqB1HPv6M5P2wHvRn7ZvAyhz83fmJMnBRNBOz1MfjxnEgkwfcVbNqsW2y37kRdZfveBlAzSfuPJV8Rkb4wlBbEGUwoCk78j8zcD_dcYFfXbt7uXz_tscScoIOg959Rmwr2E1XqRNy2qWLKSImwo8athdEEE-byLYytg6mgM02bmEQk2dyd5W2MmqG_4UaiBru6Bf9-drqExHDGUyndnAKi_uvF_131_LkPxy6H5Hu_YfZgSE5hXUbRsBzU-gbY5aV5FSn855PnRDyS_lFnBEn-0vcCIMmxbdfhqyKtFPmFHdSO1YsGruhqYaOLOlTVzThP-1XJSpgMKXHXW35c52zB9AaTV-0ETICvZ_OjZM_uzdWeb6PQmFsztcwdO-9C70yR3_HdcjljvnQ4XHs9ho_3_V57fcbW3uQCTq0TRbwD0AXpkVOvKJqaP1yEXYLKSNpGL2MMkuY-i3k6wTZMTV1280TqbJcSpY5n6WoWJnjoZ08BwBQDfX8AUsKk-D71wJbONqmLo5YnmrS-1gHR3bKCfuUzDdvensLXYJwSHg3ae_qE5VxscRhT_p2odeE8JgQBhd0d6765YBAP93F1c", | ||
expectedStatusCode: 401, | ||
expectedResponse: "jwt from authorization HTTP header is missing value for \"kid\" in token header", |
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 previous errors exposed here were not really something a legitimate user would run into, and they could not fix it either. The generic version is fine IMO.
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 kind of disagree, it makes debugging so much more difficult!
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.
But should this really be exposed to the client, or should we have debug logs with the details?
require.NoError(t, err) | ||
|
||
assert.Equal(t, tc.expectedStatusCode, res.StatusCode, string(body)) | ||
assert.Equal(t, tc.expectedResponse, strings.TrimSpace(string(body))) | ||
assert.Equal(t, tc.expectedErrorReason, gjson.GetBytes(body, "error.reason").String()) |
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.
We now always return JSON errors.
return k.Key, nil | ||
} | ||
jm: jwtmiddleware.New( | ||
func(ctx context.Context, rawToken string) (any, error) { |
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.
Is debug missing now?
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.
Yes, the debug option does not exist anymore. Instead, the errors returned should have more context.