Skip to content

Commit cdeb8d1

Browse files
authored
Fix Broken SCIM integration (#171)
* Fix several bugs
1 parent b9a1270 commit cdeb8d1

File tree

8 files changed

+53
-27
lines changed

8 files changed

+53
-27
lines changed

build/package/helm/gateway/files/policies/policies.rego

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ allowed_paths := [
1313
"/domain.ClusterAccess/GetClusterAccess",
1414
]
1515

16-
scoped_paths := [{"path": "/scim/", "scope": "WRITE_SCIM"}]
16+
scoped_paths := [{"scope": "WRITE_SCIM", "paths": [
17+
"/scim/",
18+
"/eventsourcing.CommandHandler/Execute",
19+
"/domain.User/",
20+
]}]
1721

1822
command_path := "/eventsourcing.CommandHandler/Execute"
1923

@@ -25,6 +29,7 @@ role_admin = "admin"
2529

2630
# check if system admin
2731
is_system_admin {
32+
print("entering is_system_admin")
2833
some role in input.User.Roles
2934
role.Scope == scope_system
3035
role.Name == role_admin
@@ -33,6 +38,8 @@ is_system_admin {
3338

3439
# check if user is tenant admin and adjusts rolebindings of other users of the tenant
3540
tenant_admin_rolebindings {
41+
print("entering tenant_admin_rolebindings")
42+
3643
# check that it is a command
3744
startswith(input.Path, command_path)
3845
req := json.unmarshal(input.Request)
@@ -50,7 +57,7 @@ tenant_admin_rolebindings {
5057
role.Name == role_admin
5158
role.Resource == req.data.resource
5259

53-
print(input.User.Name, "is tenant admin and allowed to execute", req.type)
60+
print(input.User.Name, "is tenant admin and allowed to execute", req.type, "for tenant", req.data.resource)
5461
}
5562

5663
# authorized because system admin
@@ -65,16 +72,19 @@ authorized {
6572

6673
# authorized via allowed_paths
6774
authorized {
75+
print("entering allowed_paths")
6876
some path in allowed_paths
6977
startswith(input.Path, path)
7078
print(path, "is allowed to everyone")
7179
}
7280

7381
# authorized via scope
7482
authorized {
83+
print("entering scoped_paths")
7584
some scoped_path in scoped_paths
76-
startswith(input.Path, scoped_path.path)
7785
some scope in input.Authentication.Scopes
7886
scope == scoped_path.scope
79-
print("scope", scope, "allows access to path", scoped_path.path)
87+
some path in scoped_path.paths
88+
startswith(input.Path, path)
89+
print("scope", scope, "allows access to path", path)
8090
}

build/package/helm/gateway/files/policies/policies_test.rego

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package m8.authz
33
alice_admin = {"User": {
44
"Id": "1234", "Name": "alice",
55
"Roles": [{"Name": "admin", "Scope": "system"}, {"Name": "admin", "Scope": "tenant", "Resource": "1234"}],
6+
"Path": "/domain.User/GetAll",
67
}}
78

89
bob_tenant_admin = {

internal/gateway/auth/auth_token_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package auth
1717
import (
1818
"time"
1919

20+
"github.com/finleap-connect/monoskope/pkg/api/gateway"
2021
"github.com/finleap-connect/monoskope/pkg/jwt"
2122
. "github.com/onsi/ginkgo"
2223
. "github.com/onsi/gomega"
@@ -32,4 +33,11 @@ var _ = Describe("internal/gateway/auth/token", func() {
3233
t.Expiry = jose_jwt.NewNumericDate(time.Now().UTC().Add(time.Hour * -12))
3334
Expect(t.Validate(expectedIssuer)).To(HaveOccurred())
3435
})
36+
37+
It("validate api token", func() {
38+
t := NewApiToken(&jwt.StandardClaims{}, expectedIssuer, "me", expectedValidity, []gateway.AuthorizationScope{gateway.AuthorizationScope_WRITE_SCIM})
39+
t.Expiry = jose_jwt.NewNumericDate(time.Now().UTC().Add(time.Hour * 1))
40+
Expect(t.Validate(expectedIssuer)).ToNot(HaveOccurred())
41+
Expect(t.Scope).To(Equal(gateway.AuthorizationScope_WRITE_SCIM.String()))
42+
})
3543
})

internal/gateway/auth_server.go

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func NewInsecureAuthServerClient(ctx context.Context, gatewayAddr string) (*grpc
9292
}
9393

9494
func (s *authServer) Print(_ print.Context, msg string) error {
95-
s.log.V(logger.DebugLevel).Info(msg)
95+
s.log.V(logger.DebugLevel).WithValues("source", "rego").Info(msg)
9696
return nil
9797
}
9898

@@ -180,11 +180,6 @@ func (s *authServer) Check(ctx context.Context, req *gateway.CheckRequest) (*gat
180180

181181
// validatePolicies validates the configured policies using OPA
182182
func (s *authServer) validatePolicies(ctx context.Context, req *gateway.CheckRequest, authToken *jwt.AuthToken) (bool, error) {
183-
userId, err := uuid.Parse(authToken.Subject)
184-
if err != nil {
185-
return false, fmt.Errorf("failed to parse user id from token: %w", err)
186-
}
187-
188183
input := policyInput{
189184
User: policyUser{
190185
Id: authToken.Subject,
@@ -198,18 +193,21 @@ func (s *authServer) validatePolicies(ctx context.Context, req *gateway.CheckReq
198193
CommandTypes: commands.CommandTypes,
199194
}
200195

201-
roleBindings, err := s.roleBindingRepo.ByUserId(ctx, userId)
202-
if err != nil {
203-
return false, fmt.Errorf("failed get rolebindings for user: %w", err)
204-
}
196+
userId, err := uuid.Parse(authToken.Subject)
197+
if err == nil {
198+
roleBindings, err := s.roleBindingRepo.ByUserId(ctx, userId)
199+
if err != nil {
200+
return false, fmt.Errorf("failed get rolebindings for user: %w", err)
201+
}
205202

206-
input.User.Roles = make([]policyRoles, 0)
207-
for _, role := range roleBindings {
208-
input.User.Roles = append(input.User.Roles, policyRoles{
209-
Name: role.Role,
210-
Scope: role.Scope,
211-
Resource: role.Resource,
212-
})
203+
input.User.Roles = make([]policyRoles, 0)
204+
for _, role := range roleBindings {
205+
input.User.Roles = append(input.User.Roles, policyRoles{
206+
Name: role.Role,
207+
Scope: role.Scope,
208+
Resource: role.Resource,
209+
})
210+
}
213211
}
214212

215213
scopes := strings.Split(authToken.Scope, " ")

internal/gateway/testenv.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ func NewTestEnvWithParent(testeEnv *test.TestEnv) (*TestEnv, error) {
238238

239239
gatewayAuthServer, errAuthServer := NewAuthServer(context.Background(), localAddrAPIServer, authServer, env.PoliciesPath, userRoleBindingRepo)
240240
if errAuthServer != nil {
241-
return nil, err
241+
return nil, errAuthServer
242242
}
243243

244244
// Create gRPC server and register implementation

internal/scimserver/auth_handler.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package scimserver
1616

1717
import (
18-
"fmt"
1918
"net/http"
2019

2120
"github.com/elimity-com/scim"
@@ -46,7 +45,7 @@ func (h *authHandler) withAuthContext(r *http.Request) (*http.Request, error) {
4645
Detail: "Request unauthenticated with " + auth.AuthScheme,
4746
}
4847
}
49-
md := metadata.Pairs(auth.HeaderAuthorization, fmt.Sprintf("%s %v", auth.AuthScheme, token))
48+
md := metadata.Pairs(auth.HeaderAuthorization, token)
5049
nCtx := metautils.NiceMD(md).ToIncoming(r.Context())
5150
return r.WithContext(nCtx), nil
5251
}

internal/scimserver/server_test.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626

2727
"github.com/cenkalti/backoff"
2828
"github.com/finleap-connect/monoskope/internal/gateway/auth"
29+
"github.com/finleap-connect/monoskope/pkg/api/gateway"
2930
"github.com/finleap-connect/monoskope/pkg/domain/constants/roles"
3031
"github.com/finleap-connect/monoskope/pkg/jwt"
3132
"github.com/google/uuid"
@@ -36,17 +37,24 @@ import (
3637
var _ = Describe("internal/scimserver/Server", func() {
3738
var userId uuid.UUID
3839

39-
getAdminAuthToken := func() string {
40+
getAuthToken := func() string {
4041
signer := testEnv.gatewayTestEnv.JwtTestEnv.CreateSigner()
41-
token := auth.NewAuthToken(&jwt.StandardClaims{Name: testEnv.gatewayTestEnv.AdminUser.Name, Email: testEnv.gatewayTestEnv.AdminUser.Email}, testEnv.gatewayTestEnv.GetApiAddr(), testEnv.gatewayTestEnv.AdminUser.ID().String(), time.Minute*10)
42+
token := auth.NewApiToken(
43+
&jwt.StandardClaims{Name: testEnv.gatewayTestEnv.AdminUser.Name,
44+
Email: testEnv.gatewayTestEnv.AdminUser.Email},
45+
testEnv.gatewayTestEnv.GetApiAddr(),
46+
"test",
47+
time.Minute*10,
48+
[]gateway.AuthorizationScope{gateway.AuthorizationScope_WRITE_SCIM},
49+
)
4250
authToken, err := signer.GenerateSignedToken(token)
4351
Expect(err).ToNot(HaveOccurred())
4452
return authToken
4553
}
4654

4755
getRequest := func(method string, target string, body io.Reader) *http.Request {
4856
req := httptest.NewRequest(method, target, body)
49-
req.Header.Add("authorization", getAdminAuthToken())
57+
req.Header.Add("authorization", auth.AuthScheme+" "+getAuthToken())
5058
return req
5159
}
5260

internal/scimserver/suite_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"testing"
1919

2020
"github.com/finleap-connect/monoskope/internal/test"
21+
"github.com/onsi/ginkgo"
2122
. "github.com/onsi/ginkgo"
2223
. "github.com/onsi/gomega"
2324
)
@@ -36,6 +37,7 @@ var _ = BeforeSuite(func() {
3637
done := make(chan interface{})
3738

3839
go func() {
40+
defer ginkgo.GinkgoRecover()
3941
var err error
4042
By("bootstrapping test env")
4143
baseTestEnv = test.NewTestEnv("scimserver-testenv")

0 commit comments

Comments
 (0)