Skip to content

Commit

Permalink
Fix internal/authz and internal/controlplane tests
Browse files Browse the repository at this point in the history
  • Loading branch information
evankanderson committed Jan 30, 2025
1 parent e10fca5 commit edf9294
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 24 deletions.
19 changes: 8 additions & 11 deletions internal/authz/authz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"testing"

"github.com/google/uuid"
"github.com/lestrrat-go/jwx/v2/jwt/openid"
fgasdk "github.com/openfga/go-sdk"
"github.com/openfga/openfga/cmd/run"
"github.com/openfga/openfga/pkg/logger"
Expand All @@ -20,7 +19,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/mindersec/minder/internal/auth/jwt"
"github.com/mindersec/minder/internal/auth"
"github.com/mindersec/minder/internal/authz"
srvconfig "github.com/mindersec/minder/pkg/config/server"
)
Expand Down Expand Up @@ -91,9 +90,9 @@ func TestVerifyOneProject(t *testing.T) {
prj := uuid.New()
assert.NoError(t, c.Write(ctx, "user-1", authz.RoleAdmin, prj), "failed to write project")

userJWT := openid.New()
assert.NoError(t, userJWT.Set("sub", "user-1"))
userctx := jwt.WithAuthTokenContext(ctx, userJWT)
userctx := auth.WithIdentityContext(ctx, &auth.Identity{
UserID: "user-1",
})

// verify the project
assert.NoError(t, c.Check(userctx, "get", prj), "failed to check project")
Expand Down Expand Up @@ -145,9 +144,9 @@ func TestVerifyMultipleProjects(t *testing.T) {
prj1 := uuid.New()
assert.NoError(t, c.Write(ctx, "user-1", authz.RoleAdmin, prj1), "failed to write project")

user1JWT := openid.New()
assert.NoError(t, user1JWT.Set("sub", "user-1"))
userctx := jwt.WithAuthTokenContext(ctx, user1JWT)
userctx := auth.WithIdentityContext(ctx, &auth.Identity{
UserID: "user-1",
})

// verify the project
assert.NoError(t, c.Check(userctx, "get", prj1), "failed to check project")
Expand All @@ -164,9 +163,7 @@ func TestVerifyMultipleProjects(t *testing.T) {
assert.NoError(t, c.Write(ctx, "user-2", authz.RoleAdmin, prj3), "failed to write project")

// verify the project
user2JWT := openid.New()
assert.NoError(t, user2JWT.Set("sub", "user-2"))
assert.NoError(t, c.Check(jwt.WithAuthTokenContext(ctx, user2JWT), "get", prj3), "failed to check project")
assert.NoError(t, c.Check(auth.WithIdentityContext(ctx, &auth.Identity{UserID: "user-2"}), "get", prj3), "failed to check project")

// verify user-1 cannot operate on project 3
assert.Error(t, c.Check(userctx, "get", prj3), "expected user-1 to not be able to operate on project 3")
Expand Down
2 changes: 1 addition & 1 deletion internal/controlplane/handlers_authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func getDefaultProjectID(

// Not sure if we still need to do this at all, but we only create database users
// for users registered in the primary ("") provider.
if userId.Provider.String() == "" {
if userId != nil && userId.String() == userId.UserID {
_, err := store.GetUserBySubject(ctx, userId.String())
if err != nil {
// Note that we're revealing that the user is not registered in minder
Expand Down
7 changes: 7 additions & 0 deletions internal/controlplane/handlers_authz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ func TestEntityContextProjectInterceptor(t *testing.T) {
tc.buildStubs(t, mockStore)
}
ctx := authjwt.WithAuthTokenContext(withRpcOptions(context.Background(), rpcOptions), userJWT)
ctx = auth.WithIdentityContext(ctx, &auth.Identity{
UserID: userJWT.Subject(),
})

authzClient := &mock.SimpleClient{}

Expand Down Expand Up @@ -278,6 +281,10 @@ func TestProjectAuthorizationInterceptor(t *testing.T) {
ctx := withRpcOptions(context.Background(), rpcOptions)
ctx = engcontext.WithEntityContext(ctx, tc.entityCtx)
ctx = authjwt.WithAuthTokenContext(ctx, userJWT)
ctx = auth.WithIdentityContext(ctx, &auth.Identity{
UserID: userJWT.Subject(),
HumanName: userJWT.Subject(),
})
_, err := ProjectAuthorizationInterceptor(ctx, request{}, &grpc.UnaryServerInfo{
Server: &server,
}, unaryHandler)
Expand Down
2 changes: 1 addition & 1 deletion internal/controlplane/handlers_projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (s *Server) ListProjects(

// Not sure if we still need to do this at all, but we only create database users
// for users registered in the primary ("") provider.
if id.Provider.String() == "" {
if id != nil && id.String() == id.UserID {
_, err := s.store.GetUserBySubject(ctx, id.String())
if err != nil {
return nil, status.Errorf(codes.Internal, "error getting user: %v", err)
Expand Down
21 changes: 11 additions & 10 deletions internal/controlplane/handlers_projects_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import (
"testing"

"github.com/google/uuid"
"github.com/lestrrat-go/jwx/v2/jwt/openid"
"github.com/stretchr/testify/assert"
"go.uber.org/mock/gomock"

mockdb "github.com/mindersec/minder/database/mock"
"github.com/mindersec/minder/internal/auth/jwt"
"github.com/mindersec/minder/internal/auth"
"github.com/mindersec/minder/internal/authz/mock"
"github.com/mindersec/minder/internal/db"
minder "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
Expand All @@ -23,8 +22,9 @@ import (
func TestListProjects(t *testing.T) {
t.Parallel()

user := openid.New()
assert.NoError(t, user.Set("sub", "testuser"))
user := &auth.Identity{
UserID: "testuser",
}

authzClient := &mock.SimpleClient{
Allowed: []uuid.UUID{uuid.New()},
Expand All @@ -34,7 +34,7 @@ func TestListProjects(t *testing.T) {
defer ctrl.Finish()

mockStore := mockdb.NewMockStore(ctrl)
mockStore.EXPECT().GetUserBySubject(gomock.Any(), user.Subject()).Return(db.User{ID: 1}, nil)
mockStore.EXPECT().GetUserBySubject(gomock.Any(), user.String()).Return(db.User{ID: 1}, nil)
mockStore.EXPECT().GetProjectByID(gomock.Any(), authzClient.Allowed[0]).Return(
db.Project{ID: authzClient.Allowed[0]}, nil)

Expand All @@ -44,7 +44,7 @@ func TestListProjects(t *testing.T) {
}

ctx := context.Background()
ctx = jwt.WithAuthTokenContext(ctx, user)
ctx = auth.WithIdentityContext(ctx, user)

resp, err := server.ListProjects(ctx, &minder.ListProjectsRequest{})
assert.NoError(t, err)
Expand All @@ -56,8 +56,9 @@ func TestListProjects(t *testing.T) {
func TestListProjectsWithOneDeletedWhileIterating(t *testing.T) {
t.Parallel()

user := openid.New()
assert.NoError(t, user.Set("sub", "testuser"))
user := &auth.Identity{
UserID: "testuser",
}

authzClient := &mock.SimpleClient{
Allowed: []uuid.UUID{uuid.New(), uuid.New(), uuid.New()},
Expand All @@ -67,7 +68,7 @@ func TestListProjectsWithOneDeletedWhileIterating(t *testing.T) {
defer ctrl.Finish()

mockStore := mockdb.NewMockStore(ctrl)
mockStore.EXPECT().GetUserBySubject(gomock.Any(), user.Subject()).Return(db.User{ID: 1}, nil)
mockStore.EXPECT().GetUserBySubject(gomock.Any(), user.String()).Return(db.User{ID: 1}, nil)
mockStore.EXPECT().GetProjectByID(gomock.Any(), authzClient.Allowed[0]).Return(
db.Project{ID: authzClient.Allowed[0]}, nil)
mockStore.EXPECT().GetProjectByID(gomock.Any(), authzClient.Allowed[1]).Return(
Expand All @@ -81,7 +82,7 @@ func TestListProjectsWithOneDeletedWhileIterating(t *testing.T) {
}

ctx := context.Background()
ctx = jwt.WithAuthTokenContext(ctx, user)
ctx = auth.WithIdentityContext(ctx, user)

resp, err := server.ListProjects(ctx, &minder.ListProjectsRequest{})
assert.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/controlplane/handlers_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ func ensureUser(ctx context.Context, s *Server, store db.ExtendQuerier) (db.User
// Only allow invitation flow for users from the primary provider / who can accept
// things like terms & conditions. Secondary providers are for machine identities,
// which cannot operate webpages, etc.
if id.Provider.String() != "" {
if id == nil || id.String() != id.UserID {
return db.User{}, util.UserVisibleError(codes.FailedPrecondition, "this type of user cannot accept invitations")
}
sub := id.String()
Expand Down
3 changes: 3 additions & 0 deletions internal/controlplane/handlers_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,9 @@ func TestResolveInvitation(t *testing.T) {

ctx := context.Background()
ctx = jwt.WithAuthTokenContext(ctx, user)
ctx = auth.WithIdentityContext(ctx, &auth.Identity{
UserID: userSubject,
})

mockStore := mockdb.NewMockStore(ctrl)
if tc.setup != nil {
Expand Down

0 comments on commit edf9294

Please sign in to comment.