Skip to content

Commit

Permalink
Don't use AuthenticateRequest method in tests
Browse files Browse the repository at this point in the history 
Instead of using AuthenticateRequest to retrieve the user from the
request and then use it for the expected values, allocate a variable for
the username in the request and use that in the expected values.
This ensures we don't hide potential errors of AuthenticateRequest.
  • Loading branch information
@elikatsis
elikatsis committed Mar 18, 2021
1 parent c1914f1 commit 5ef17ea
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 61 deletions.
7 changes: 3 additions & 4 deletions backend/src/apiserver/server/auth_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,18 +87,17 @@ func TestAuthorizeRequest_Unauthorized(t *testing.T) {
defer clients.Close()
authServer := AuthServer{resourceManager: manager}

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: "accounts.google.com:[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)
userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

request := &api.AuthorizeRequest{
Namespace: "ns1",
Resources: api.AuthorizeRequest_VIEWERS,
Verb: api.AuthorizeRequest_GET,
}

_, err = authServer.Authorize(ctx, request)
_, err := authServer.Authorize(ctx, request)
assert.Error(t, err)

resourceAttributes := &authorizationv1.ResourceAttributes{
Expand Down
24 changes: 9 additions & 15 deletions backend/src/apiserver/server/experiment_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,13 @@ func TestCreateExperiment_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clients, resourceManager, _ := initWithExperiment_SubjectAccessReview_Unauthorized(t)
defer clients.Close()

userIdentity, err := resourceManager.AuthenticateRequest(ctx)
assert.Nil(t, err)

server := ExperimentServer{resourceManager: resourceManager, options: &ExperimentServerOptions{CollectMetrics: false}}
experiment := &api.Experiment{
Name: "exp1",
Expand All @@ -94,7 +92,7 @@ func TestCreateExperiment_Unauthorized(t *testing.T) {
},
}}

_, err = server.CreateExperiment(ctx, &api.CreateExperimentRequest{Experiment: experiment})
_, err := server.CreateExperiment(ctx, &api.CreateExperimentRequest{Experiment: experiment})
assert.NotNil(t, err)
resourceAttributes := &authorizationv1.ResourceAttributes{
Namespace: "ns1",
Expand Down Expand Up @@ -182,18 +180,16 @@ func TestGetExperiment_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clients, manager, experiment := initWithExperiment_SubjectAccessReview_Unauthorized(t)
defer clients.Close()

userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

server := ExperimentServer{manager, &ExperimentServerOptions{CollectMetrics: false}}

_, err = server.GetExperiment(ctx, &api.GetExperimentRequest{Id: experiment.UUID})
_, err := server.GetExperiment(ctx, &api.GetExperimentRequest{Id: experiment.UUID})
assert.NotNil(t, err)
resourceAttributes := &authorizationv1.ResourceAttributes{
Namespace: "ns1",
Expand Down Expand Up @@ -300,18 +296,16 @@ func TestListExperiment_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clients, manager, _ := initWithExperiment_SubjectAccessReview_Unauthorized(t)
defer clients.Close()

userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

server := ExperimentServer{manager, &ExperimentServerOptions{CollectMetrics: false}}

_, err = server.ListExperiment(ctx, &api.ListExperimentsRequest{
_, err := server.ListExperiment(ctx, &api.ListExperimentsRequest{
ResourceReferenceKey: &api.ResourceKey{
Type: api.ResourceType_NAMESPACE,
Id: "ns1",
Expand Down
34 changes: 12 additions & 22 deletions backend/src/apiserver/server/job_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,17 +304,15 @@ func TestCreateJob_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clients, manager, _ := initWithExperiment_SubjectAccessReview_Unauthorized(t)
defer clients.Close()

userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

server := NewJobServer(manager, &JobServerOptions{CollectMetrics: false})
_, err = server.CreateJob(ctx, &api.CreateJobRequest{Job: commonApiJob})
_, err := server.CreateJob(ctx, &api.CreateJobRequest{Job: commonApiJob})
assert.NotNil(t, err)
resourceAttributes := &authorizationv1.ResourceAttributes{
Namespace: "ns1",
Expand All @@ -335,15 +333,13 @@ func TestGetJob_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clients, manager, _ := initWithExperiment(t)
defer clients.Close()

userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

server := NewJobServer(manager, &JobServerOptions{CollectMetrics: false})
job, err := server.CreateJob(ctx, &api.CreateJobRequest{Job: commonApiJob})
assert.Nil(t, err)
Expand Down Expand Up @@ -391,17 +387,15 @@ func TestListJobs_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clients, manager, experiment := initWithExperiment_SubjectAccessReview_Unauthorized(t)
defer clients.Close()

userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

server := NewJobServer(manager, &JobServerOptions{CollectMetrics: false})
_, err = server.ListJobs(ctx, &api.ListJobsRequest{
_, err := server.ListJobs(ctx, &api.ListJobsRequest{
ResourceReferenceKey: &api.ResourceKey{
Type: api.ResourceType_EXPERIMENT,
Id: experiment.UUID,
Expand Down Expand Up @@ -546,7 +540,8 @@ func TestEnableJob_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clients, manager, _ := initWithExperiment(t)
Expand All @@ -559,9 +554,6 @@ func TestEnableJob_Unauthorized(t *testing.T) {
manager = resource.NewResourceManager(clients)
server = NewJobServer(manager, &JobServerOptions{CollectMetrics: false})

userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

_, err = server.EnableJob(ctx, &api.EnableJobRequest{Id: job.Id})
assert.NotNil(t, err)
resourceAttributes := &authorizationv1.ResourceAttributes{
Expand Down Expand Up @@ -601,7 +593,8 @@ func TestDisableJob_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clients, manager, _ := initWithExperiment(t)
Expand All @@ -614,9 +607,6 @@ func TestDisableJob_Unauthorized(t *testing.T) {
manager = resource.NewResourceManager(clients)
server = NewJobServer(manager, &JobServerOptions{CollectMetrics: false})

userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

_, err = server.DisableJob(ctx, &api.DisableJobRequest{Id: job.Id})
assert.NotNil(t, err)
resourceAttributes := &authorizationv1.ResourceAttributes{
Expand Down
24 changes: 9 additions & 15 deletions backend/src/apiserver/server/run_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,13 @@ func TestCreateRun_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clients, manager, _ := initWithExperiment_SubjectAccessReview_Unauthorized(t)
defer clients.Close()

userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

server := NewRunServer(manager, &RunServerOptions{CollectMetrics: false})
run := &api.Run{
Name: "run1",
Expand All @@ -145,7 +143,7 @@ func TestCreateRun_Unauthorized(t *testing.T) {
Parameters: []*api.Parameter{{Name: "param1", Value: "world"}},
},
}
_, err = server.CreateRun(ctx, &api.CreateRunRequest{Run: run})
_, err := server.CreateRun(ctx, &api.CreateRunRequest{Run: run})
assert.NotNil(t, err)
resourceAttributes := &authorizationv1.ResourceAttributes{
Namespace: "ns1",
Expand Down Expand Up @@ -261,17 +259,15 @@ func TestListRuns_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clients, manager, _ := initWithExperiment_SubjectAccessReview_Unauthorized(t)
defer clients.Close()

userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

server := NewRunServer(manager, &RunServerOptions{CollectMetrics: false})
_, err = server.ListRuns(ctx, &api.ListRunsRequest{
_, err := server.ListRuns(ctx, &api.ListRunsRequest{
ResourceReferenceKey: &api.ResourceKey{
Type: api.ResourceType_NAMESPACE,
Id: "ns1",
Expand Down Expand Up @@ -683,12 +679,10 @@ func TestCanAccessRun_Unauthorized(t *testing.T) {
defer clients.Close()
runServer := RunServer{resourceManager: manager, options: &RunServerOptions{CollectMetrics: false}}

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

userIdentity, err := manager.AuthenticateRequest(ctx)
assert.Nil(t, err)

apiRun := &api.Run{
Name: "run1",
PipelineSpec: &api.PipelineSpec{
Expand All @@ -710,7 +704,7 @@ func TestCanAccessRun_Unauthorized(t *testing.T) {
}
runDetail, _ := manager.CreateRun(apiRun)

err = runServer.canAccessRun(ctx, runDetail.UUID, &authorizationv1.ResourceAttributes{Verb: common.RbacResourceVerbGet})
err := runServer.canAccessRun(ctx, runDetail.UUID, &authorizationv1.ResourceAttributes{Verb: common.RbacResourceVerbGet})
assert.NotNil(t, err)
resourceAttributes := &authorizationv1.ResourceAttributes{
Namespace: runDetail.Namespace,
Expand Down
8 changes: 3 additions & 5 deletions backend/src/apiserver/server/visualization_server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,17 +257,15 @@ func TestCreateVisualization_Unauthorized(t *testing.T) {
viper.Set(common.MultiUserMode, "true")
defer viper.Set(common.MultiUserMode, "false")

md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + "[email protected]"})
userIdentity := "[email protected]"
md := metadata.New(map[string]string{common.GoogleIAPUserIdentityHeader: common.GoogleIAPUserIdentityPrefix + userIdentity})
ctx := metadata.NewIncomingContext(context.Background(), md)

clientManager := resource.NewFakeClientManagerOrFatal(util.NewFakeTimeForEpoch())
clientManager.SubjectAccessReviewClientFake = client.NewFakeSubjectAccessReviewClientUnauthorized()
resourceManager := resource.NewResourceManager(clientManager)
defer clientManager.Close()

userIdentity, err := resourceManager.AuthenticateRequest(ctx)
assert.Nil(t, err)

server := &VisualizationServer{
resourceManager: resourceManager,
}
Expand All @@ -281,7 +279,7 @@ func TestCreateVisualization_Unauthorized(t *testing.T) {
Visualization: visualization,
Namespace: "ns1",
}
_, err = server.CreateVisualization(ctx, request)
_, err := server.CreateVisualization(ctx, request)
assert.NotNil(t, err)
resourceAttributes := &authorizationv1.ResourceAttributes{
Namespace: "ns1",
Expand Down

0 comments on commit 5ef17ea

Please sign in to comment.