Skip to content

Commit d4b501c

Browse files
authored
fix: properly set casbin authz policy (#1776)
### Proposed Changes When calling the SetPolicy function it was overwrittng what ever was defined within the yaml configuration. This pr addresses this by changing the current functionality to overwrite the default policy while still allowing the policy in configuration to take precedence. The pull request also introduces two new fields `UsernameClaim` and `GroupsClaim` aligning more with openid based claim names and not keycloak realm roles. The authz policy will pull the username and list of groups and run them through the casbin enforce function now. A user can now scope a policy to a username. ``` p, strantalis, policy.attributes.*, read, allow ``` ### Checklist - [ ] I have added or updated unit tests - [ ] I have added or updated integration tests (if appropriate) - [ ] I have added or updated documentation ### Testing Instructions
1 parent 5f26568 commit d4b501c

File tree

15 files changed

+454
-523
lines changed

15 files changed

+454
-523
lines changed

docs/configuration.md

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -243,11 +243,12 @@ OpenTDF uses Casbin to manage authorization policies. This document provides an
243243

244244
#### Key Aspects of Authorization Configuration
245245

246-
1. **Default Role**: The default role assigned to an authorized user if no specific role is found.
247-
2. **Claim**: The claim in the OIDC token that should be used to map roles.
248-
3. **Map**: Mapping between policy roles and IdP roles.
249-
4. **CSV**: The authorization policy in CSV format.
250-
5. **Model**: The Casbin policy model.
246+
2. **Username Claim**: The claim in the OIDC token that should be used to extract a username.
247+
3. **Group Claim**: The claim in the OIDC token that should be used to find the group claims.
248+
4. **Map (Deprecated)**: Mapping between policy roles and IdP roles.
249+
4. **Extension**: Policy that will extend the builtin policy
250+
4. **CSV**: The authorization policy in CSV format. This will override the builtin policy.
251+
5. **Model**: The Casbin policy model. This should only be set if you have a deep understanding of how casbin works.
251252

252253
#### Configuration in opentdf-example.yaml
253254

@@ -263,19 +264,34 @@ server:
263264
audience: 'http://localhost:8080'
264265
issuer: http://keycloak:8888/auth/realms/opentdf
265266
policy:
266-
## Default role for all requests
267-
default: "role:standard"
268267
268+
## Deprecated
269269
## Dot notation is used to access nested claims (i.e. realm_access.roles)
270270
claim: "realm_access.roles"
271+
272+
## Dot notation is used to access the username claim
273+
username_claim: "email"
274+
275+
## Dot notation is used to access the groups claim
276+
group_claim: "realm_access.roles"
271277
278+
## Deprecated: Use standard casbin policy groupings (g, <user/group>, <role>)
272279
## Maps the external role to the OpenTDF role
273280
## Note: left side is used in the policy, right side is the external role
274281
map:
275282
standard: opentdf-standard
276283
admin: opentdf-admin
277284
285+
## Policy that will extend the builtin policy.
286+
extension: |
287+
p, role:admin, *, *, allow
288+
p, role:standard, policy:attributes, read, allow
289+
p, role:standard, policy:subject-mappings, read, allow
290+
g, opentdf-admin, role:admin
291+
g, [email protected], role:standard
292+
278293
## Custom policy (see examples https://github.com/casbin/casbin/tree/master/examples)
294+
## This will overwrite the builtin policy. Use with caution.
279295
csv: |
280296
p, role:admin, *, *, allow
281297
p, role:standard, policy:attributes, read, allow
@@ -286,6 +302,7 @@ server:
286302
p, role:unknown, kas.AccessService/Rewrap, *, allow
287303
288304
## Custom model (see https://casbin.org/docs/syntax-for-models/)
305+
## Avoid setting this unless you have a deep understanding of how casbin works.
289306
model: |
290307
[request_definition]
291308
r = sub, res, act, obj
@@ -305,8 +322,7 @@ server:
305322

306323
#### Role Permissions
307324

308-
- **Org Admin**: Can read, write, and perform unsafe mutations.
309-
- **Admin**: Can read and write.
325+
- **Admin**: Can perform all operations.
310326
- **Standard User**: Can read.
311327
- **Public Endpoints**: Accessible without specific roles.
312328

opentdf-dev.yaml

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,18 @@ server:
4949
audience: 'http://localhost:8080'
5050
issuer: http://localhost:8888/auth/realms/opentdf
5151
policy:
52-
## Default policy for all requests
53-
default: #"role:standard"
5452
## Dot notation is used to access nested claims (i.e. realm_access.roles)
55-
claim: # realm_access.roles
56-
## Maps the external role to the opentdf role
57-
## Note: left side is used in the policy, right side is the external role
58-
map:
59-
# standard: opentdf-standard
60-
# admin: opentdf-admin
61-
62-
## Custom policy (see examples https://github.com/casbin/casbin/tree/master/examples)
53+
# Claim that represents the user (i.e. email)
54+
username_claim: # preferred_username
55+
# That claim to access groups (i.e. realm_access.roles)
56+
groups_claim: # realm_access.roles
57+
## Extends the builtin policy
58+
extension: |
59+
g, opentdf-admin, role:admin
60+
g, opentdf-standard, role:standard
61+
## Custom policy that overrides builtin policy (see examples https://github.com/casbin/casbin/tree/master/examples)
6362
csv: #|
6463
# p, role:admin, *, *, allow
65-
6664
## Custom model (see https://casbin.org/docs/syntax-for-models/)
6765
model: #|
6866
# [request_definition]

opentdf-example.yaml

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,16 @@ server:
3636
audience: 'http://localhost:8080'
3737
issuer: http://keycloak:8888/auth/realms/opentdf
3838
policy:
39-
## Default policy for all requests
40-
default: #"role:standard"
4139
## Dot notation is used to access nested claims (i.e. realm_access.roles)
42-
claim: # realm_access.roles
43-
## Maps the external role to the opentdf role
44-
## Note: left side is used in the policy, right side is the external role
45-
map:
46-
# standard: opentdf-standard
47-
# admin: opentdf-admin
48-
49-
## Custom policy (see examples https://github.com/casbin/casbin/tree/master/examples)
40+
# Claim that represents the user (i.e. email)
41+
username_claim: # preferred_username
42+
# That claim to access groups (i.e. realm_access.roles)
43+
groups_claim: # realm_access.roles
44+
## Extends the builtin policy
45+
extension: |
46+
g, opentdf-admin, role:admin
47+
g, opentdf-standard, role:standard
48+
## Custom policy that overrides builtin policy (see examples https://github.com/casbin/casbin/tree/master/examples)
5049
csv: #|
5150
# p, role:admin, *, *, allow
5251
## Custom model (see https://casbin.org/docs/syntax-for-models/)

opentdf-with-hsm.yaml

Lines changed: 0 additions & 114 deletions
This file was deleted.

service/go.mod

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ require (
1010
github.com/Nerzal/gocloak/v13 v13.9.0
1111
github.com/bmatcuk/doublestar v1.3.4
1212
github.com/bufbuild/protovalidate-go v0.6.0
13-
github.com/casbin/casbin/v2 v2.84.0
13+
github.com/casbin/casbin/v2 v2.101.0
1414
github.com/creasty/defaults v1.7.0
1515
github.com/go-chi/cors v1.2.1
1616
github.com/go-playground/validator/v10 v10.22.0
@@ -43,7 +43,8 @@ require (
4343
github.com/andybalholm/brotli v1.1.0 // indirect
4444
github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
4545
github.com/beorn7/perks v1.0.1 // indirect
46-
github.com/casbin/govaluate v1.1.0 // indirect
46+
github.com/bmatcuk/doublestar/v4 v4.6.1 // indirect
47+
github.com/casbin/govaluate v1.2.0 // indirect
4748
github.com/cenkalti/backoff/v4 v4.3.0 // indirect
4849
github.com/cespare/xxhash/v2 v2.3.0 // indirect
4950
github.com/containerd/containerd v1.7.21 // indirect

service/go.sum

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,20 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
4444
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
4545
github.com/bmatcuk/doublestar v1.3.4 h1:gPypJ5xD31uhX6Tf54sDPUOBXTqKH4c9aPY66CyQrS0=
4646
github.com/bmatcuk/doublestar v1.3.4/go.mod h1:wiQtGV+rzVYxB7WIlirSN++5HPtPlXEo9MEoZQC/PmE=
47+
github.com/bmatcuk/doublestar/v4 v4.6.1 h1:FH9SifrbvJhnlQpztAx++wlkk70QBf0iBWDwNy7PA4I=
48+
github.com/bmatcuk/doublestar/v4 v4.6.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
4749
github.com/bufbuild/protovalidate-go v0.6.0 h1:Jgs1kFuZ2LHvvdj8SpCLA1W/+pXS8QSM3F/E2l3InPY=
4850
github.com/bufbuild/protovalidate-go v0.6.0/go.mod h1:1LamgoYHZ2NdIQH0XGczGTc6Z8YrTHjcJVmiBaar4t4=
4951
github.com/bytecodealliance/wasmtime-go/v3 v3.0.2 h1:3uZCA/BLTIu+DqCfguByNMJa2HVHpXvjfy0Dy7g6fuA=
5052
github.com/bytecodealliance/wasmtime-go/v3 v3.0.2/go.mod h1:RnUjnIXxEJcL6BgCvNyzCCRzZcxCgsZCi+RNlvYor5Q=
5153
github.com/casbin/casbin/v2 v2.84.0 h1:WioxdvRbG/1lR7GMIhEHk4GwPcVOxQqhCGWpEUKJoGI=
5254
github.com/casbin/casbin/v2 v2.84.0/go.mod h1:jX8uoN4veP85O/n2674r2qtfSXI6myvxW85f6TH50fw=
55+
github.com/casbin/casbin/v2 v2.101.0 h1:y8qZRXcgv5omd3k/7kpaP03Hov82sXzCC5FAfm17lkw=
56+
github.com/casbin/casbin/v2 v2.101.0/go.mod h1:LO7YPez4dX3LgoTCqSQAleQDo0S0BeZBDxYnPUl95Ng=
5357
github.com/casbin/govaluate v1.1.0 h1:6xdCWIpE9CwHdZhlVQW+froUrCsjb6/ZYNcXODfLT+E=
5458
github.com/casbin/govaluate v1.1.0/go.mod h1:G/UnbIjZk/0uMNaLwZZmFQrR72tYRZWQkO70si/iR7A=
59+
github.com/casbin/govaluate v1.2.0 h1:wXCXFmqyY+1RwiKfYo3jMKyrtZmOL3kHwaqDyCPOYak=
60+
github.com/casbin/govaluate v1.2.0/go.mod h1:G/UnbIjZk/0uMNaLwZZmFQrR72tYRZWQkO70si/iR7A=
5561
github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
5662
github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
5763
github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko=

service/internal/auth/authn.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -197,16 +197,6 @@ func normalizeURL(o string, u *url.URL) string {
197197
return ou.String()
198198
}
199199

200-
// deprecated
201-
func (a *Authentication) ExtendAuthzDefaultPolicy(policies [][]string) error {
202-
return a.enforcer.ExtendDefaultPolicy(policies)
203-
}
204-
205-
// SetAuthzPolicy sets the policy for the casbin enforcer
206-
func (a *Authentication) SetAuthzPolicy(policy string) error {
207-
return a.enforcer.SetPolicy(policy)
208-
}
209-
210200
// verifyTokenHandler is a http handler that verifies the token
211201
func (a Authentication) MuxHandler(handler http.Handler) http.Handler {
212202
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

service/internal/auth/authn_test.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"time"
2222

2323
"connectrpc.com/connect"
24+
"github.com/creasty/defaults"
2425
"github.com/lestrrat-go/jwx/v2/jwa"
2526
"github.com/lestrrat-go/jwx/v2/jwk"
2627
"github.com/lestrrat-go/jwx/v2/jws"
@@ -145,6 +146,10 @@ func (s *AuthSuite) SetupTest() {
145146
}
146147
}))
147148

149+
policyCfg := PolicyConfig{}
150+
err = defaults.Set(&policyCfg)
151+
s.Require().NoError(err)
152+
148153
auth, err := NewAuthenticator(
149154
context.Background(),
150155
Config{
@@ -154,6 +159,7 @@ func (s *AuthSuite) SetupTest() {
154159
Audience: "test",
155160
DPoPSkew: time.Hour,
156161
TokenSkew: time.Minute,
162+
Policy: policyCfg,
157163
},
158164
PublicRoutes: []string{
159165
"/public",
@@ -549,15 +555,6 @@ func (s *AuthSuite) TestDPoPEndToEnd_HTTP() {
549555
s.Equal(dpopJWK.N(), dpopJWKFromRequest.N())
550556
}
551557

552-
func (s *AuthSuite) Test_AddAuthzPolicies() {
553-
err := s.auth.ExtendAuthzDefaultPolicy([][]string{
554-
{"p", "role:admin", "/path", "*", "allow"},
555-
{"p", "role:standard", "/path2", "read", "deny"},
556-
})
557-
s.Require().NoError(err)
558-
s.False(s.auth.enforcer.isDefaultPolicy)
559-
}
560-
561558
func makeDPoPToken(t *testing.T, tc dpopTestCase) string {
562559
jtiBytes := make([]byte, sdkauth.JTILength)
563560
_, err := rand.Read(jtiBytes)

0 commit comments

Comments
 (0)