Skip to content

Commit 7e5c22d

Browse files
roelarentsdavidcollom
authored andcommitted
fix acr auth JWT token verification (default no verification)
Signed-off-by: Roel Arents <[email protected]> fix acr basic auth also needs access token Signed-off-by: Roel Arents <[email protected]>
1 parent d7e6786 commit 7e5c22d

File tree

5 files changed

+78
-20
lines changed

5 files changed

+78
-20
lines changed

Diff for: cmd/app/options.go

+8
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const (
2424
envACRUsername = "ACR_USERNAME"
2525
envACRPassword = "ACR_PASSWORD" // #nosec G101
2626
envACRRefreshToken = "ACR_REFRESH_TOKEN" // #nosec G101
27+
envACRJWKSURI = "ACR_JWKS_URI"
2728

2829
envDockerUsername = "DOCKER_USERNAME"
2930
envDockerPassword = "DOCKER_PASSWORD" // #nosec G101
@@ -157,6 +158,12 @@ func (o *Options) addAuthFlags(fs *pflag.FlagSet) {
157158
"username/password (%s_%s).",
158159
envPrefix, envACRRefreshToken,
159160
))
161+
fs.StringVar(&o.Client.ACR.JWKSURI,
162+
"acr-jwks-uri", "",
163+
fmt.Sprintf(
164+
"JWKS URI to verify the JWT access token received. If left blank, JWT token will not be verified. (%s_%s)",
165+
envPrefix, envACRJWKSURI,
166+
))
160167
///
161168

162169
// Docker
@@ -301,6 +308,7 @@ func (o *Options) complete() {
301308
{envACRUsername, &o.Client.ACR.Username},
302309
{envACRPassword, &o.Client.ACR.Password},
303310
{envACRRefreshToken, &o.Client.ACR.RefreshToken},
311+
{envACRJWKSURI, &o.Client.ACR.JWKSURI},
304312

305313
{envDockerUsername, &o.Client.Docker.Username},
306314
{envDockerPassword, &o.Client.Docker.Password},

Diff for: cmd/app/options_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ func TestComplete(t *testing.T) {
3131
{"VERSION_CHECKER_ACR_USERNAME", "acr-username"},
3232
{"VERSION_CHECKER_ACR_PASSWORD", "acr-password"},
3333
{"VERSION_CHECKER_ACR_REFRESH_TOKEN", "acr-token"},
34+
{"VERSION_CHECKER_ACR_JWKS_URI", "acr-jwks-uri"},
3435
{"VERSION_CHECKER_DOCKER_USERNAME", "docker-username"},
3536
{"VERSION_CHECKER_DOCKER_PASSWORD", "docker-password"},
3637
{"VERSION_CHECKER_DOCKER_TOKEN", "docker-token"},
@@ -51,6 +52,7 @@ func TestComplete(t *testing.T) {
5152
Username: "acr-username",
5253
Password: "acr-password",
5354
RefreshToken: "acr-token",
55+
JWKSURI: "acr-jwks-uri",
5456
},
5557
Docker: docker.Options{
5658
Username: "docker-username",
@@ -92,6 +94,7 @@ func TestComplete(t *testing.T) {
9294
{"VERSION_CHECKER_ACR_USERNAME", "acr-username"},
9395
{"VERSION_CHECKER_ACR_PASSWORD", "acr-password"},
9496
{"VERSION_CHECKER_ACR_REFRESH_TOKEN", "acr-token"},
97+
{"VERSION_CHECKER_ACR_JWKS_URI", "acr-jwks-uri"},
9598
{"VERSION_CHECKER_DOCKER_USERNAME", "docker-username"},
9699
{"VERSION_CHECKER_DOCKER_PASSWORD", "docker-password"},
97100
{"VERSION_CHECKER_DOCKER_TOKEN", "docker-token"},
@@ -119,6 +122,7 @@ func TestComplete(t *testing.T) {
119122
Username: "acr-username",
120123
Password: "acr-password",
121124
RefreshToken: "acr-token",
125+
JWKSURI: "acr-jwks-uri",
122126
},
123127
Docker: docker.Options{
124128
Username: "docker-username",

Diff for: go.mod

+2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ require (
2828
)
2929

3030
require (
31+
github.com/MicahParks/keyfunc/v3 v3.3.10
3132
github.com/aws/aws-sdk-go-v2/config v1.29.12
3233
github.com/aws/aws-sdk-go-v2/credentials v1.17.65
3334
github.com/aws/aws-sdk-go-v2/service/ecr v1.43.0
@@ -50,6 +51,7 @@ require (
5051
github.com/Azure/go-autorest/autorest/date v0.3.1 // indirect
5152
github.com/Azure/go-autorest/logger v0.2.2 // indirect
5253
github.com/Azure/go-autorest/tracing v0.6.1 // indirect
54+
github.com/MicahParks/jwkset v0.8.0 // indirect
5355
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.30 // indirect
5456
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34 // indirect
5557
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34 // indirect

Diff for: go.sum

+8
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ github.com/Azure/go-autorest/logger v0.2.2/go.mod h1:I5fg9K52o+iuydlWfa9T5K6WFos
1919
github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU=
2020
github.com/Azure/go-autorest/tracing v0.6.1 h1:YUMSrC/CeD1ZnnXcNYU4a/fzsO35u2Fsful9L/2nyR0=
2121
github.com/Azure/go-autorest/tracing v0.6.1/go.mod h1:/3EgjbsjraOqiicERAeu3m7/z0x1TzjQGAwDrJrXGkc=
22+
github.com/MicahParks/jwkset v0.5.19 h1:XZCsgJv05DBCvxEHYEHlSafqiuVn5ESG0VRB331Fxhw=
23+
github.com/MicahParks/jwkset v0.5.19/go.mod h1:q8ptTGn/Z9c4MwbcfeCDssADeVQb3Pk7PnVxrvi+2QY=
24+
github.com/MicahParks/jwkset v0.8.0 h1:jHtclI38Gibmu17XMI6+6/UB59srp58pQVxePHRK5o8=
25+
github.com/MicahParks/jwkset v0.8.0/go.mod h1:fVrj6TmG1aKlJEeceAz7JsXGTXEn72zP1px3us53JrA=
26+
github.com/MicahParks/keyfunc/v3 v3.3.5 h1:7ceAJLUAldnoueHDNzF8Bx06oVcQ5CfJnYwNt1U3YYo=
27+
github.com/MicahParks/keyfunc/v3 v3.3.5/go.mod h1:SdCCyMJn/bYqWDvARspC6nCT8Sk74MjuAY22C7dCST8=
28+
github.com/MicahParks/keyfunc/v3 v3.3.10 h1:JtEGE8OcNeI297AMrR4gVXivV8fyAawFUMkbwNreJRk=
29+
github.com/MicahParks/keyfunc/v3 v3.3.10/go.mod h1:1TEt+Q3FO7Yz2zWeYO//fMxZMOiar808NqjWQQpBPtU=
2230
github.com/aws/aws-sdk-go-v2 v1.36.3 h1:mJoei2CxPutQVxaATCzDUjcZEjVRdpsiiXi2o38yqWM=
2331
github.com/aws/aws-sdk-go-v2 v1.36.3/go.mod h1:LLXuLpgzEbD766Z5ECcRmi8AzSwfZItDtmABVkRLGzg=
2432
github.com/aws/aws-sdk-go-v2/config v1.29.12 h1:Y/2a+jLPrPbHpFkpAAYkVEtJmxORlXoo5k2g1fa2sUo=

Diff for: pkg/client/acr/acr.go

+56-20
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,19 @@ import (
1010
"sync"
1111
"time"
1212

13+
"github.com/MicahParks/keyfunc/v3"
14+
1315
"github.com/Azure/go-autorest/autorest"
1416
"github.com/Azure/go-autorest/autorest/adal"
15-
jwt "github.com/golang-jwt/jwt/v5"
17+
"github.com/golang-jwt/jwt/v5"
1618

1719
"github.com/jetstack/version-checker/pkg/api"
1820
"github.com/jetstack/version-checker/pkg/client/util"
1921
)
2022

2123
const (
22-
userAgent = "jetstack/version-checker"
24+
userAgent = "jetstack/version-checker"
25+
requiredScope = "repository:*:metadata_read"
2326
)
2427

2528
type Client struct {
@@ -38,6 +41,7 @@ type Options struct {
3841
Username string
3942
Password string
4043
RefreshToken string
44+
JWKSURI string
4145
}
4246

4347
type AccessTokenResponse struct {
@@ -154,35 +158,52 @@ func (c *Client) getACRClient(ctx context.Context, host string) (*acrClient, err
154158
}
155159

156160
var (
157-
client *acrClient
158-
err error
161+
client *acrClient
162+
accessTokenClient *autorest.Client
163+
accessTokenReq *http.Request
164+
err error
159165
)
160-
161166
if len(c.RefreshToken) > 0 {
162-
client, err = c.getAccessTokenClient(ctx, host)
167+
accessTokenClient, accessTokenReq, err = c.getAccessTokenRequesterForRefreshToken(ctx, host)
163168
} else {
164-
client, err = c.getBasicAuthClient(host)
169+
accessTokenClient, accessTokenReq, err = c.getAccessTokenRequesterForBasicAuth(ctx, host)
165170
}
166171
if err != nil {
167172
return nil, err
168173
}
174+
if client, err = c.getAuthorizedClient(accessTokenClient, accessTokenReq, host); err != nil {
175+
return nil, err
176+
}
169177

170178
c.cachedACRClient[host] = client
171179

172180
return client, nil
173181
}
174182

175-
func (c *Client) getBasicAuthClient(_ string) (*acrClient, error) {
183+
func (c *Client) getAccessTokenRequesterForBasicAuth(ctx context.Context, host string) (*autorest.Client, *http.Request, error) {
176184
client := autorest.NewClientWithUserAgent(userAgent)
177185
client.Authorizer = autorest.NewBasicAuthorizer(c.Username, c.Password)
186+
urlParameters := map[string]interface{}{
187+
"url": "https://" + host,
188+
}
178189

179-
return &acrClient{
180-
Client: &client,
181-
tokenExpiry: time.Unix(1<<63-1, 0),
182-
}, nil
190+
preparer := autorest.CreatePreparer(
191+
autorest.WithCustomBaseURL("{url}", urlParameters),
192+
autorest.WithPath("/oauth2/token"),
193+
autorest.WithQueryParameters(map[string]interface{}{
194+
"scope": requiredScope,
195+
"service": host,
196+
}),
197+
)
198+
req, err := preparer.Prepare((&http.Request{}).WithContext(ctx))
199+
if err != nil {
200+
return nil, nil, err
201+
}
202+
203+
return &client, req, nil
183204
}
184205

185-
func (c *Client) getAccessTokenClient(ctx context.Context, host string) (*acrClient, error) {
206+
func (c *Client) getAccessTokenRequesterForRefreshToken(ctx context.Context, host string) (*autorest.Client, *http.Request, error) {
186207
client := autorest.NewClientWithUserAgent(userAgent)
187208
urlParameters := map[string]interface{}{
188209
"url": "https://" + host,
@@ -191,7 +212,7 @@ func (c *Client) getAccessTokenClient(ctx context.Context, host string) (*acrCli
191212
formDataParameters := map[string]interface{}{
192213
"grant_type": "refresh_token",
193214
"refresh_token": c.RefreshToken,
194-
"scope": "repository:*:*",
215+
"scope": requiredScope,
195216
"service": host,
196217
}
197218

@@ -202,9 +223,12 @@ func (c *Client) getAccessTokenClient(ctx context.Context, host string) (*acrCli
202223
autorest.WithFormData(autorest.MapToValues(formDataParameters)))
203224
req, err := preparer.Prepare((&http.Request{}).WithContext(ctx))
204225
if err != nil {
205-
return nil, err
226+
return nil, nil, err
206227
}
228+
return &client, req, nil
229+
}
207230

231+
func (c *Client) getAuthorizedClient(client *autorest.Client, req *http.Request, host string) (*acrClient, error) {
208232
resp, err := autorest.SendWithSender(client, req,
209233
autorest.DoRetryForStatusCodes(client.RetryAttempts, client.RetryDuration, autorest.StatusCodesForRetry...),
210234
)
@@ -220,26 +244,38 @@ func (c *Client) getAccessTokenClient(ctx context.Context, host string) (*acrCli
220244
host, err)
221245
}
222246

223-
exp, err := getTokenExpiration(respToken.AccessToken)
247+
exp, err := c.getTokenExpiration(respToken.AccessToken)
224248
if err != nil {
225249
return nil, fmt.Errorf("%s: %s", host, err)
226250
}
227251

228252
token := &adal.Token{
229-
RefreshToken: c.RefreshToken,
253+
RefreshToken: "", // empty if access_token was retrieved with basic auth. but client is not reused after expiry anyway (see cachedACRClient)
230254
AccessToken: respToken.AccessToken,
231255
}
232256

233257
client.Authorizer = autorest.NewBearerAuthorizer(token)
234258

235259
return &acrClient{
236260
tokenExpiry: exp,
237-
Client: &client,
261+
Client: client,
238262
}, nil
239263
}
240264

241-
func getTokenExpiration(tokenString string) (time.Time, error) {
242-
token, err := jwt.Parse(tokenString, nil, jwt.WithoutClaimsValidation())
265+
func (c *Client) getTokenExpiration(tokenString string) (time.Time, error) {
266+
jwtParser := jwt.NewParser(jwt.WithoutClaimsValidation())
267+
var token *jwt.Token
268+
var err error
269+
if c.JWKSURI != "" {
270+
var k keyfunc.Keyfunc
271+
k, err = keyfunc.NewDefaultCtx(context.TODO(), []string{c.JWKSURI})
272+
if err != nil {
273+
return time.Time{}, err
274+
}
275+
token, err = jwtParser.Parse(tokenString, k.Keyfunc)
276+
} else {
277+
token, _, err = jwtParser.ParseUnverified(tokenString, jwt.MapClaims{})
278+
}
243279
if err != nil {
244280
return time.Time{}, err
245281
}

0 commit comments

Comments
 (0)