From ae7851fce399f4bdf66c02392c2796e96d27058d Mon Sep 17 00:00:00 2001 From: John Schaeffer Date: Tue, 5 Mar 2024 17:13:27 -0500 Subject: [PATCH] Update to IAM runtime v0.3.0 (#14) * Update iam-runtime dependency to v0.3.0 This commit updates iam-runtime-static to use v0.3.0 of the IAM runtime spec. Signed-off-by: John Schaeffer * Update runtime implementation to match v0.3.0 interface This commit updates iam-runtime-static to use the v0.3.0 IAM runtime interface, returning authentication and authorization errors as result fields in the response rather than riding on gRPC errors. Signed-off-by: John Schaeffer * Add proper linting, fix Makefile This commit adds a proper linter to the project and fixes the Makefile to be generally better at being a Makefile. Signed-off-by: John Schaeffer * Remove reference to args in RunE to satisfy linter This commit makes some fixes to keep the linter happy. Signed-off-by: John Schaeffer * Add tests This commit adds tests to iam-runtime-static. Signed-off-by: John Schaeffer * Fix whitespace linting error This commit fixes a whitespace linting error in code. Signed-off-by: John Schaeffer --------- Signed-off-by: John Schaeffer --- .gitignore | 4 +- .golangci.yml | 61 +++++++++++++ Makefile | 40 +++++---- cmd/serve.go | 2 +- go.mod | 5 +- go.sum | 4 +- internal/server/server.go | 29 ++++-- internal/server/server_test.go | 159 +++++++++++++++++++++++++++++++++ 8 files changed, 275 insertions(+), 29 deletions(-) create mode 100644 .golangci.yml create mode 100644 internal/server/server_test.go diff --git a/.gitignore b/.gitignore index c866c75..71553f0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ *~ -bin/* \ No newline at end of file +bin/* +coverage.out +.tools/* \ No newline at end of file diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..309afab --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,61 @@ +linters-settings: + goimports: + local-prefixes: github.com/metal-toolbox/iam-runtime-static + +run: + # default timeout is 1m + timeout: 3m + +linters: + enable: + # default linters + - errcheck + - gosimple + - govet + - ineffassign + - staticcheck + - typecheck + - unused + + # additional linters + - bodyclose + - gocritic + - gocyclo + - goerr113 + - gofmt + - goimports + - gomnd + - govet + - misspell + - noctx + - revive + - stylecheck + - whitespace + - wsl + - paralleltest + + # - bod +issues: + exclude: + # Default excludes from `golangci-lint run --help` with EXC0002 removed + # EXC0001 errcheck: Almost all programs ignore errors on these functions and in most cases it's ok + - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked + # EXC0002 golint: Annoying issue about not having a comment. The rare codebase has such comments + # - (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form) + # EXC0003 golint: False positive when tests are defined in package 'test' + - func name will be used as test\.Test.* by other packages, and that stutters; consider calling this + # EXC0004 govet: Common false positives + - (possible misuse of unsafe.Pointer|should have signature) + # EXC0005 staticcheck: Developers tend to write in C-style with an explicit 'break' in a 'switch', so it's ok to ignore + - ineffective break statement. Did you mean to break out of the outer loop + # EXC0006 gosec: Too many false-positives on 'unsafe' usage + - Use of unsafe calls should be audited + # EXC0007 gosec: Too many false-positives for parametrized shell calls + - Subprocess launch(ed with variable|ing should be audited) + # EXC0008 gosec: Duplicated errcheck checks + - (G104|G307) + # EXC0009 gosec: Too many issues in popular repos + - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less) + # EXC0010 gosec: False positive is triggered by 'src, err := ioutil.ReadFile(filename)' + - Potential file inclusion via variable + exclude-use-default: false diff --git a/Makefile b/Makefile index 1a3477b..6fdc3ac 100644 --- a/Makefile +++ b/Makefile @@ -1,26 +1,36 @@ -all: lint test -PHONY: test coverage lint golint clean vendor docker-up docker-down unit-test +ROOT_DIR := $(shell dirname $(realpath $(firstword $(MAKEFILE_LIST)))) +TOOLS_DIR := .tools +GOOS ?= linux +GOARCH ?= amd64 + +GOLANGCI_LINT_REPO = github.com/golangci/golangci-lint +GOLANGCI_LINT_VERSION = v1.56.1 + # use the working dir as the app name, this should be the repo name APP_NAME=$(shell basename $(CURDIR)) -test: | unit-test +PHONY: all test lint build go-dependencies -unit-test: | lint - @echo Running unit tests... - @go test -cover -short -tags testtools ./... +all: go-dependencies test build -coverage: - @echo Generating coverage report... +test: lint + @echo Running unit tests... @go test ./... -race -coverprofile=coverage.out -covermode=atomic -tags testtools -p 1 - @go tool cover -func=coverage.out - @go tool cover -html=coverage.out - -lint: golint -golint: | vendor +lint: $(TOOLS_DIR)/golangci-lint @echo Linting Go files... - @golangci-lint run --build-tags "-tags testtools" + @$(TOOLS_DIR)/golangci-lint run --modules-download-mode=readonly build: - @go mod download @CGO_ENABLED=0 go build -mod=readonly -v -o bin/${APP_NAME} + +go-dependencies: + @go mod download + @go mod tidy + +$(TOOLS_DIR): + mkdir -p $(TOOLS_DIR) + +$(TOOLS_DIR)/golangci-lint: | $(TOOLS_DIR) + @echo "Installing $(GOLANGCI_LINT_REPO)/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)" + @GOBIN=$(ROOT_DIR)/$(TOOLS_DIR) go install $(GOLANGCI_LINT_REPO)/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) diff --git a/cmd/serve.go b/cmd/serve.go index 1c19259..02d24fc 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -20,7 +20,7 @@ import ( var serveCmd = &cobra.Command{ Use: "serve", Short: "starts the iam-runtime-static service", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { return serve(cmd.Context(), viper.GetViper()) }, } diff --git a/go.mod b/go.mod index 14901d9..8ad50b7 100644 --- a/go.mod +++ b/go.mod @@ -3,17 +3,19 @@ module github.com/metal-toolbox/iam-runtime-static go 1.21.6 require ( - github.com/metal-toolbox/iam-runtime v0.1.0 + github.com/metal-toolbox/iam-runtime v0.3.0 github.com/mitchellh/go-homedir v1.1.0 github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.17.0 + github.com/stretchr/testify v1.8.4 go.uber.org/zap v1.26.0 google.golang.org/grpc v1.58.3 gopkg.in/yaml.v3 v3.0.1 ) require ( + github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/hashicorp/hcl v1.0.0 // indirect @@ -21,6 +23,7 @@ require ( github.com/magiconair/properties v1.8.7 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/pelletier/go-toml/v2 v2.1.0 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/sagikazarmark/locafero v0.3.0 // indirect github.com/sagikazarmark/slog-shim v0.1.0 // indirect github.com/sourcegraph/conc v0.3.0 // indirect diff --git a/go.sum b/go.sum index 02f1c6f..db65266 100644 --- a/go.sum +++ b/go.sum @@ -146,8 +146,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/magiconair/properties v1.8.7 h1:IeQXZAiQcpL9mgcAe1Nu6cX9LLw6ExEHKjN0VQdvPDY= github.com/magiconair/properties v1.8.7/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0= -github.com/metal-toolbox/iam-runtime v0.1.0 h1:3Bx9AgCzqi6AbLG2ghbagqKtppep5Tpyl/Hn9ttqybA= -github.com/metal-toolbox/iam-runtime v0.1.0/go.mod h1:O0Tay8IBHlW4KSv3GpqhVwa9MLKRg8sBHSiPTPN45Ik= +github.com/metal-toolbox/iam-runtime v0.3.0 h1:O8L0U2FFZgRVMCqa4rg5euGN9TpiwC8dTrHEerLLfVM= +github.com/metal-toolbox/iam-runtime v0.3.0/go.mod h1:O0Tay8IBHlW4KSv3GpqhVwa9MLKRg8sBHSiPTPN45Ik= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY= diff --git a/internal/server/server.go b/internal/server/server.go index f99531e..2b64a29 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -99,17 +99,22 @@ func newFromPolicy(c policy, logger *zap.SugaredLogger) (*server, error) { return out, nil } -func (s *server) AuthenticateSubject(_ context.Context, req *authentication.AuthenticateSubjectRequest) (*authentication.AuthenticateSubjectResponse, error) { - s.logger.Info("received AuthenticateSubject request") +func (s *server) ValidateCredential(_ context.Context, req *authentication.ValidateCredentialRequest) (*authentication.ValidateCredentialResponse, error) { + s.logger.Info("received ValidateCredential request") sub, ok := s.tokens[req.Credential] if !ok { - return nil, status.Errorf(codes.Unauthenticated, "invalid credential") + out := &authentication.ValidateCredentialResponse{ + Result: authentication.ValidateCredentialResponse_RESULT_INVALID, + } + + return out, nil } - resp := &authentication.AuthenticateSubjectResponse{ - SubjectClaims: map[string]string{ - "sub": sub.ID, + resp := &authentication.ValidateCredentialResponse{ + Result: authentication.ValidateCredentialResponse_RESULT_VALID, + Subject: &authentication.Subject{ + SubjectId: sub.ID, }, } @@ -121,14 +126,20 @@ func (s *server) CheckAccess(_ context.Context, req *authorization.CheckAccessRe sub, ok := s.tokens[req.Credential] if !ok { - return nil, status.Errorf(codes.Unauthenticated, "invalid credential") + return nil, status.Errorf(codes.InvalidArgument, "invalid credential") } + result := authorization.CheckAccessResponse_RESULT_ALLOWED + for _, action := range req.Actions { if ok := checkAccess(sub, action.Action, action.ResourceId); !ok { - return nil, status.Errorf(codes.PermissionDenied, "subject does not have permission to perform '%s' on resource '%s'", action.Action, action.ResourceId) + result = authorization.CheckAccessResponse_RESULT_DENIED } } - return &authorization.CheckAccessResponse{}, nil + out := &authorization.CheckAccessResponse{ + Result: result, + } + + return out, nil } diff --git a/internal/server/server_test.go b/internal/server/server_test.go new file mode 100644 index 0000000..af3de48 --- /dev/null +++ b/internal/server/server_test.go @@ -0,0 +1,159 @@ +package server + +import ( + "context" + "os" + "testing" + + "github.com/metal-toolbox/iam-runtime/pkg/iam/runtime/authentication" + "github.com/metal-toolbox/iam-runtime/pkg/iam/runtime/authorization" + + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestServer(t *testing.T) { + // Run everything in parallel + t.Parallel() + + // Set up a server with a simple policy. + subjectAlice := "alice" + subjectBob := "bob" + + envVarAlice := "IAM_ALICE_TOKEN" + envVarBob := "IAM_BOB_TOKEN" + + tokenAlice := "alic3!" + tokenBob := "b0b" + tokenDNE := "doesnotexist" + + actionGreet := "greet" + + resourceWorld := "resourc-world" + + // In this policy, Alice has the ability to greet the world, but Bob does not. + authPolicy := policy{ + Subjects: []policySubject{ + { + ID: subjectAlice, + Tokens: []policyToken{ + { + EnvVar: envVarAlice, + }, + }, + Resources: []policyResource{ + { + ID: resourceWorld, + Actions: []string{ + actionGreet, + }, + }, + }, + }, + { + ID: subjectBob, + Tokens: []policyToken{ + { + EnvVar: envVarBob, + }, + }, + Resources: []policyResource{}, + }, + }, + } + + // Set the environment variables for the test + os.Setenv(envVarAlice, tokenAlice) + os.Setenv(envVarBob, tokenBob) + + logger := zap.NewNop().Sugar() + + srv, err := newFromPolicy(authPolicy, logger) + + require.NoError(t, err) + + t.Run("ValidateCredentialSuccess", func(t *testing.T) { + t.Parallel() + + req := &authentication.ValidateCredentialRequest{ + Credential: tokenAlice, + } + resp, err := srv.ValidateCredential(context.Background(), req) + + require.NoError(t, err) + require.Equal(t, authentication.ValidateCredentialResponse_RESULT_VALID, resp.Result) + require.Equal(t, subjectAlice, resp.Subject.SubjectId) + }) + + t.Run("ValidateCredentialFail", func(t *testing.T) { + t.Parallel() + + req := &authentication.ValidateCredentialRequest{ + Credential: tokenDNE, + } + resp, err := srv.ValidateCredential(context.Background(), req) + + require.NoError(t, err) + require.Equal(t, authentication.ValidateCredentialResponse_RESULT_INVALID, resp.Result) + }) + + t.Run("CheckAccessSuccess", func(t *testing.T) { + t.Parallel() + + req := &authorization.CheckAccessRequest{ + Credential: tokenAlice, + Actions: []*authorization.AccessRequestAction{ + { + ResourceId: resourceWorld, + Action: actionGreet, + }, + }, + } + resp, err := srv.CheckAccess(context.Background(), req) + + require.NoError(t, err) + require.Equal(t, authorization.CheckAccessResponse_RESULT_ALLOWED, resp.Result) + }) + + t.Run("CheckAccessUnauthenticated", func(t *testing.T) { + t.Parallel() + + req := &authorization.CheckAccessRequest{ + Credential: tokenDNE, + Actions: []*authorization.AccessRequestAction{ + { + ResourceId: resourceWorld, + Action: actionGreet, + }, + }, + } + + _, err := srv.CheckAccess(context.Background(), req) + + errStatus, ok := status.FromError(err) + + require.Equal(t, true, ok) + require.Equal(t, codes.InvalidArgument, errStatus.Code()) + }) + + t.Run("CheckAccessUnauthorized", func(t *testing.T) { + t.Parallel() + + req := &authorization.CheckAccessRequest{ + Credential: tokenBob, + Actions: []*authorization.AccessRequestAction{ + { + ResourceId: resourceWorld, + Action: actionGreet, + }, + }, + } + + resp, err := srv.CheckAccess(context.Background(), req) + + require.NoError(t, err) + require.Equal(t, authorization.CheckAccessResponse_RESULT_DENIED, resp.Result) + }) +}