Skip to content

Commit c2456f7

Browse files
dmihalcik-virtrudmihalcik-virtru
committed
chore(auth): DPoP and public fixes
- expose legacy public key endpoint as public - DPoP `htu` should include origin part of url - clarify error messages for dpop
1 parent dd65db1 commit c2456f7

File tree

6 files changed

+69
-37
lines changed

6 files changed

+69
-37
lines changed

‎docs/configuration.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ server:
5656
key: /path/to/key
5757
auth:
5858
enabled: true
59+
allowedHosts:
60+
- "https://example.com"
5961
audience: https://example.com
6062
issuer: https://example.com
6163
```

‎opentdf-example.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ services:
2323
server:
2424
auth:
2525
enabled: true
26+
allowedHosts:
27+
- "http://localhost:8080"
2628
audience: "http://localhost:8080"
2729
issuer: http://localhost:8888/auth/realms/opentdf
2830
policy:

‎opentdf-with-hsm.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ services:
2323
server:
2424
auth:
2525
enabled: true
26+
allowedHosts:
27+
- "http://localhost:8080"
2628
audience: "http://localhost:8080"
2729
issuer: http://localhost:8888/auth/realms/opentdf
2830
clients:

‎service/internal/auth/authn.go‎

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"log/slog"
1010
"net/http"
11+
"net/url"
1112
"path/filepath"
1213
"slices"
1314
"strings"
@@ -38,6 +39,7 @@ var (
3839
"/kas.AccessService/PublicKey",
3940
"/healthz",
4041
"/.well-known/opentdf-configuration",
42+
"/kas/kas_public_key",
4143
"/kas/v2/kas_public_key",
4244
}
4345
// only asymmetric algorithms and no 'none'
@@ -67,6 +69,8 @@ type Authentication struct {
6769
enforcer *Enforcer
6870
// Public Routes HTTP & gRPC
6971
publicRoutes []string
72+
// Allowed origins for the DPoP htu field
73+
allowedHosts []*url.URL
7074
}
7175

7276
// Creates new authN which is used to verify tokens for a set of given issuers
@@ -126,13 +130,33 @@ func NewAuthenticator(ctx context.Context, cfg Config, d *db.Client) (*Authentic
126130

127131
a.oidcConfigurations[cfg.Issuer] = cfg.AuthNConfig
128132

133+
a.allowedHosts = make([]*url.URL, len(cfg.AllowedHosts))
134+
for i, h := range cfg.AllowedHosts {
135+
a.allowedHosts[i], err = url.Parse(h)
136+
if err != nil {
137+
return nil, err
138+
}
139+
}
140+
129141
return a, nil
130142
}
131143

132144
type dpopInfo struct {
133-
headers []string
134-
path string
135-
method string
145+
u []string
146+
m string
147+
}
148+
149+
func (a Authentication) normalizeURL(u *url.URL) []string {
150+
n := *u
151+
n.RawQuery = ""
152+
n.Fragment = ""
153+
us := make([]string, len(a.allowedHosts))
154+
for i, v := range a.allowedHosts {
155+
v2 := *v
156+
v2.Path = u.Path
157+
us[i] = v2.String()
158+
}
159+
return us
136160
}
137161

138162
// verifyTokenHandler is a http handler that verifies the token
@@ -150,10 +174,9 @@ func (a Authentication) MuxHandler(handler http.Handler) http.Handler {
150174
return
151175
}
152176
tok, newCtx, err := a.checkToken(r.Context(), header, dpopInfo{
153-
headers: r.Header["Dpop"],
154-
path: r.URL.Path,
155-
method: r.Method,
156-
})
177+
u: a.normalizeURL(r.URL),
178+
m: r.Method,
179+
}, r.Header["Dpop"])
157180

158181
if err != nil {
159182
slog.WarnContext(r.Context(), "failed to validate token", slog.String("error", err.Error()))
@@ -232,10 +255,10 @@ func (a Authentication) UnaryServerInterceptor(ctx context.Context, req any, inf
232255
ctx,
233256
header,
234257
dpopInfo{
235-
headers: md["dpop"],
236-
path: info.FullMethod,
237-
method: http.MethodPost,
258+
u: []string{info.FullMethod},
259+
m: http.MethodPost,
238260
},
261+
md["dpop"],
239262
)
240263
if err != nil {
241264
slog.Warn("failed to validate token", slog.String("error", err.Error()))
@@ -258,7 +281,7 @@ func (a Authentication) UnaryServerInterceptor(ctx context.Context, req any, inf
258281
}
259282

260283
// checkToken is a helper function to verify the token.
261-
func (a Authentication) checkToken(ctx context.Context, authHeader []string, dpopInfo dpopInfo) (jwt.Token, context.Context, error) {
284+
func (a Authentication) checkToken(ctx context.Context, authHeader []string, dpopInfo dpopInfo, dpopHeader []string) (jwt.Token, context.Context, error) {
262285
var (
263286
tokenRaw string
264287
)
@@ -317,7 +340,7 @@ func (a Authentication) checkToken(ctx context.Context, authHeader []string, dpo
317340
// come from token introspection
318341
return accessToken, ctx, nil
319342
}
320-
key, err := validateDPoP(accessToken, tokenRaw, dpopInfo)
343+
key, err := validateDPoP(accessToken, tokenRaw, dpopInfo, dpopHeader)
321344
if err != nil {
322345
return nil, nil, err
323346
}
@@ -340,11 +363,11 @@ func GetJWKFromContext(ctx context.Context) jwk.Key {
340363
panic("got something that is not a jwk.Key from the JWK context")
341364
}
342365

343-
func validateDPoP(accessToken jwt.Token, acessTokenRaw string, dpopInfo dpopInfo) (jwk.Key, error) {
344-
if len(dpopInfo.headers) != 1 {
345-
return nil, fmt.Errorf("got %d dpop headers, should have 1", len(dpopInfo.headers))
366+
func validateDPoP(accessToken jwt.Token, acessTokenRaw string, dpopInfo dpopInfo, headers []string) (jwk.Key, error) {
367+
if len(headers) != 1 {
368+
return nil, fmt.Errorf("got %d dpop headers, should have 1", len(headers))
346369
}
347-
dpopHeader := dpopInfo.headers[0]
370+
dpopHeader := headers[0]
348371

349372
cnf, ok := accessToken.Get("cnf")
350373
if !ok {
@@ -405,8 +428,9 @@ func validateDPoP(accessToken jwt.Token, acessTokenRaw string, dpopInfo dpopInfo
405428
return nil, fmt.Errorf("couldn't compute thumbprint for key in `jwk` in DPoP JWT")
406429
}
407430

408-
if base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(thumbprint) != jkt {
409-
return nil, fmt.Errorf("the `jkt` from the DPoP JWT didn't match the thumbprint from the access token")
431+
thumbprintStr := base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(thumbprint)
432+
if thumbprintStr != jkt {
433+
return nil, fmt.Errorf("the `jkt` from the DPoP JWT didn't match the thumbprint from the access token; cnf.jkt=[%v], computed=[%v]", jkt, thumbprintStr)
410434
}
411435

412436
// at this point we have the right key because its thumbprint matches the `jkt` claim
@@ -432,17 +456,17 @@ func validateDPoP(accessToken jwt.Token, acessTokenRaw string, dpopInfo dpopInfo
432456
return nil, fmt.Errorf("`htm` claim missing in DPoP JWT")
433457
}
434458

435-
if htm != dpopInfo.method {
436-
return nil, fmt.Errorf("incorrect `htm` claim in DPoP JWT")
459+
if htm != dpopInfo.m {
460+
return nil, fmt.Errorf("incorrect `htm` claim in DPoP JWT; should match [%v]", dpopInfo.m)
437461
}
438462

439463
htu, ok := dpopToken.Get("htu")
440464
if !ok {
441465
return nil, fmt.Errorf("`htu` claim missing in DPoP JWT")
442466
}
443467

444-
if htu != dpopInfo.path {
445-
return nil, fmt.Errorf("incorrect `htu` claim in DPoP JWT")
468+
if !slices.Contains(dpopInfo.u, htu.(string)) {
469+
return nil, fmt.Errorf("incorrect `htu` claim in DPoP JWT; should match %v", dpopInfo.u)
446470
}
447471

448472
ath, ok := dpopToken.Get("ath")

‎service/internal/auth/authn_test.go‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,10 @@ func (s *AuthSuite) SetupTest() {
144144
context.Background(),
145145
Config{
146146
AuthNConfig: AuthNConfig{
147-
EnforceDPoP: true,
148-
Issuer: s.server.URL,
149-
Audience: "test",
147+
AllowedHosts: []string{""},
148+
EnforceDPoP: true,
149+
Issuer: s.server.URL,
150+
Audience: "test",
150151
},
151152
PublicRoutes: []string{"/public", "/public2/*", "/public3/static", "/static/*", "/static/*/*"},
152153
},
@@ -174,7 +175,7 @@ func (s *AuthSuite) Test_CheckToken_When_JWT_Expired_Expect_Error() {
174175
s.NotNil(signedTok)
175176
s.Require().NoError(err)
176177

177-
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{})
178+
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{}, nil)
178179
s.Require().Error(err)
179180
s.Equal("\"exp\" not satisfied", err.Error())
180181
}
@@ -198,7 +199,7 @@ func (s *AuthSuite) Test_UnaryServerInterceptor_When_Authorization_Header_Missin
198199
}
199200

200201
func (s *AuthSuite) Test_CheckToken_When_Authorization_Header_Invalid_Expect_Error() {
201-
_, _, err := s.auth.checkToken(context.Background(), []string{"BPOP "}, dpopInfo{})
202+
_, _, err := s.auth.checkToken(context.Background(), []string{"BPOP "}, dpopInfo{}, nil)
202203
s.Require().Error(err)
203204
s.Equal("not of type bearer or dpop", err.Error())
204205
}
@@ -212,7 +213,7 @@ func (s *AuthSuite) Test_CheckToken_When_Missing_Issuer_Expect_Error() {
212213
s.NotNil(signedTok)
213214
s.Require().NoError(err)
214215

215-
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{})
216+
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{}, nil)
216217
s.Require().Error(err)
217218
s.Equal("missing issuer", err.Error())
218219
}
@@ -227,7 +228,7 @@ func (s *AuthSuite) Test_CheckToken_When_Invalid_Issuer_Value_Expect_Error() {
227228
s.NotNil(signedTok)
228229
s.Require().NoError(err)
229230

230-
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{})
231+
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{}, nil)
231232
s.Require().Error(err)
232233
s.Equal("invalid issuer", err.Error())
233234
}
@@ -241,7 +242,7 @@ func (s *AuthSuite) Test_CheckToken_When_Audience_Missing_Expect_Error() {
241242
s.NotNil(signedTok)
242243
s.Require().NoError(err)
243244

244-
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{})
245+
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{}, nil)
245246
s.Require().Error(err)
246247
s.Equal("claim \"aud\" not found", err.Error())
247248
}
@@ -256,7 +257,7 @@ func (s *AuthSuite) Test_CheckToken_When_Audience_Invalid_Expect_Error() {
256257
s.NotNil(signedTok)
257258
s.Require().NoError(err)
258259

259-
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{})
260+
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{}, nil)
260261
s.Require().Error(err)
261262
s.Equal("\"aud\" not satisfied", err.Error())
262263
}
@@ -272,7 +273,7 @@ func (s *AuthSuite) Test_CheckToken_When_Valid_No_DPoP_Expect_Error() {
272273
s.NotNil(signedTok)
273274
s.Require().NoError(err)
274275

275-
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{})
276+
_, _, err = s.auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{}, nil)
276277
s.Require().Error(err)
277278
s.Require().Contains(err.Error(), "dpop")
278279
}
@@ -350,14 +351,14 @@ func (s *AuthSuite) TestInvalid_DPoP_Cases() {
350351
context.Background(),
351352
[]string{fmt.Sprintf("DPoP %s", string(testCase.accessToken))},
352353
dpopInfo{
353-
headers: []string{dpopToken},
354-
path: "/a/path",
355-
method: http.MethodPost,
354+
u: []string{"/a/path"},
355+
m: http.MethodPost,
356356
},
357+
[]string{dpopToken},
357358
)
358359

359360
s.Require().Error(err)
360-
s.Equal(testCase.errorMessage, err.Error())
361+
s.Contains(err.Error(), testCase.errorMessage)
361362
}
362363
}
363364

@@ -568,7 +569,7 @@ func (s *AuthSuite) Test_Allowing_Auth_With_No_DPoP() {
568569
s.NotNil(signedTok)
569570
s.Require().NoError(err)
570571

571-
_, ctx, err := auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{})
572+
_, ctx, err := auth.checkToken(context.Background(), []string{fmt.Sprintf("Bearer %s", string(signedTok))}, dpopInfo{}, nil)
572573
s.Require().NoError(err)
573574
s.Require().Nil(GetJWKFromContext(ctx))
574575
}

‎service/internal/auth/config.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ type AuthNConfig struct {
1717
OIDCConfiguration `yaml:"-" json:"-"`
1818
Policy PolicyConfig `yaml:"policy" json:"policy" mapstructure:"policy"`
1919
CacheRefresh string `mapstructure:"cache_refresh_interval"`
20+
AllowedHosts []string
2021
}
2122

2223
type PolicyConfig struct {

0 commit comments

Comments
 (0)