diff --git a/internal/authn/multitenant/authn.go b/internal/authn/multitenant/authn.go deleted file mode 100644 index 3cb57b05d..000000000 --- a/internal/authn/multitenant/authn.go +++ /dev/null @@ -1,57 +0,0 @@ -package multitenant - -import ( - "github.com/golang-jwt/jwt/v4" - grpcAuth "github.com/grpc-ecosystem/go-grpc-middleware/auth" - "golang.org/x/net/context" - - "github.com/Permify/permify/internal/authn" -) - -// TenantAuthenticator - Interface for key authenticator -type TenantAuthenticator interface { - Authenticate(ctx context.Context, reqTenantID string) error -} - -// TenantAuthn - Authentication Keys Structure -type TenantAuthn struct { - secret string - algorithms []string -} - -// NewTenantAuthn - Create New NewTenantAuthn Keys -func NewTenantAuthn(secret string, algorithms []string) (*TenantAuthn, error) { - return &TenantAuthn{ - secret: secret, - algorithms: algorithms, - }, nil -} - -// Authenticate - Checking whether any API request contain keys -func (t *TenantAuthn) Authenticate(ctx context.Context, reqTenantID string) error { - header, err := grpcAuth.AuthFromMD(ctx, "Bearer") - if err != nil { - return authn.MissingBearerTokenError - } - jwtParser := jwt.NewParser(jwt.WithValidMethods(t.algorithms)) - var token *jwt.Token - token, err = jwtParser.Parse(header, func(token *jwt.Token) (any, error) { - return []byte(t.secret), nil - }) - if err != nil { - return authn.Unauthenticated - } - claims, ok := token.Claims.(jwt.MapClaims) - if ok && token.Valid { - var tenantID string - tenantID, ok = claims["tenant_id"].(string) - if !ok { - return authn.MissingTenantIDError - } - if reqTenantID != tenantID { - return authn.Unauthenticated - } - return nil - } - return authn.Unauthenticated -} diff --git a/internal/authn/multitenant/interceptors.go b/internal/authn/multitenant/interceptors.go deleted file mode 100644 index 096dcaff5..000000000 --- a/internal/authn/multitenant/interceptors.go +++ /dev/null @@ -1,80 +0,0 @@ -package multitenant - -import ( - "context" - - "google.golang.org/grpc" - - basev1 "github.com/Permify/permify/pkg/pb/base/v1" -) - -// UnaryServerInterceptor - -func UnaryServerInterceptor(t TenantAuthenticator) grpc.UnaryServerInterceptor { - return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { - var reqTenantID string - switch r := req.(type) { - case *basev1.PermissionCheckRequest: - reqTenantID = r.TenantId - case *basev1.PermissionLookupEntityRequest: - reqTenantID = r.TenantId - case *basev1.PermissionLookupSchemaRequest: - reqTenantID = r.TenantId - case *basev1.PermissionExpandRequest: - reqTenantID = r.TenantId - case *basev1.RelationshipWriteRequest: - reqTenantID = r.TenantId - case *basev1.RelationshipDeleteRequest: - reqTenantID = r.TenantId - case *basev1.RelationshipReadRequest: - reqTenantID = r.TenantId - case *basev1.SchemaReadRequest: - reqTenantID = r.TenantId - case *basev1.SchemaWriteRequest: - reqTenantID = r.TenantId - case *basev1.TenantDeleteRequest: - reqTenantID = r.Id - default: - return handler(ctx, req) - } - - err := t.Authenticate(ctx, reqTenantID) - if err != nil { - return nil, err - } - - return handler(ctx, req) - } -} - -// StreamServerInterceptor - -func StreamServerInterceptor(t TenantAuthenticator) grpc.StreamServerInterceptor { - return func(srv interface{}, stream grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error { - wrapper := &authnWrapper{ServerStream: stream, authenticator: t} - return handler(srv, wrapper) - } -} - -// authnWrapper - -type authnWrapper struct { - grpc.ServerStream - authenticator TenantAuthenticator -} - -// RecvMsg - -func (s *authnWrapper) RecvMsg(req interface{}) error { - if err := s.ServerStream.RecvMsg(req); err != nil { - return err - } - var reqTenantID string - switch r := req.(type) { - case *basev1.PermissionLookupEntityRequest: - reqTenantID = r.TenantId - default: - return nil - } - err := s.authenticator.Authenticate(s.ServerStream.Context(), reqTenantID) - if err != nil { - return err - } - return nil -} diff --git a/internal/authn/oidc/authn.go b/internal/authn/oidc/authn.go index d3e70e896..da5cc0a37 100644 --- a/internal/authn/oidc/authn.go +++ b/internal/authn/oidc/authn.go @@ -54,7 +54,6 @@ func (t *OidcAuthn) Authenticate(ctx context.Context) error { return authn.Unauthenticated } return nil - } // validateOtherClaims - Validate claims that are not validated by the oidc client library diff --git a/internal/authn/oidc/authn_test.go b/internal/authn/oidc/authn_test.go index 3d14c66e5..298398d64 100644 --- a/internal/authn/oidc/authn_test.go +++ b/internal/authn/oidc/authn_test.go @@ -32,20 +32,24 @@ func Test_AuthenticateWithSigningMethods(t *testing.T) { method jwt.SigningMethod wantErr bool }{ - {"Should pass with RS256", + { + "Should pass with RS256", jwt.SigningMethodRS256, false, }, - {"Should fail with HS256, zitadel/oidc does not support HSXXX algorithms by default", + { + "Should fail with HS256, zitadel/oidc does not support HSXXX algorithms by default", // see https://github.com/zitadel/oidc/blob/v1.13.0/pkg/oidc/keyset.go#L94 jwt.SigningMethodHS256, true, }, - {"Should pass with ES256", + { + "Should pass with ES256", jwt.SigningMethodES256, false, }, - {"Should pass with PS256", + { + "Should pass with PS256", jwt.SigningMethodPS256, false, }, @@ -104,35 +108,41 @@ func Test_AuthenticateClaims(t *testing.T) { claimOverride *jwt.RegisteredClaims wantErr bool }{ - {"With correct values there should be no errors", + { + "With correct values there should be no errors", &jwt.RegisteredClaims{}, false, }, - {"Wrong issuer in the token, it should fail", + { + "Wrong issuer in the token, it should fail", &jwt.RegisteredClaims{ Issuer: "http://wrong-issuer", }, true, }, - {"Wrong clientID in the token, it should fail", + { + "Wrong clientID in the token, it should fail", &jwt.RegisteredClaims{ Audience: []string{"wrong-clientid"}, }, true, }, - {"Expired Token, it should fail", + { + "Expired Token, it should fail", &jwt.RegisteredClaims{ ExpiresAt: &jwt.NumericDate{Time: time.Date(1999, 1, 0, 0, 0, 0, 0, time.UTC)}, }, true, }, - {"Issued at the future, it should fail", + { + "Issued at the future, it should fail", &jwt.RegisteredClaims{ IssuedAt: &jwt.NumericDate{Time: time.Date(3999, 1, 0, 0, 0, 0, 0, time.UTC)}, }, true, }, - {"Token used before its NotBefore date, it should fail", + { + "Token used before its NotBefore date, it should fail", &jwt.RegisteredClaims{ NotBefore: &jwt.NumericDate{Time: time.Date(3999, 1, 0, 0, 0, 0, 0, time.UTC)}, }, @@ -196,27 +206,32 @@ func Test_AuthenticateKeyIds(t *testing.T) { keyId string wantErr bool }{ - {"With no keyid using RS256 it should fail, multiple public RSA keys matching for RS256 and PS256", + { + "With no keyid using RS256 it should fail, multiple public RSA keys matching for RS256 and PS256", jwt.SigningMethodRS256, false, "", true, }, - {"With no keyid using ES256 it should pass, unique public ecdsa key in keyset", + { + "With no keyid using ES256 it should pass, unique public ecdsa key in keyset", jwt.SigningMethodES256, true, "", false, }, - {"With right keyid using RS256 it should pass", + { + "With right keyid using RS256 it should pass", jwt.SigningMethodRS256, true, fakeOidcProvider.keyIds[jwt.SigningMethodRS256], false, }, - {"With wrong keyid using RS256 it should fail", + { + "With wrong keyid using RS256 it should fail", jwt.SigningMethodRS256, true, "wrongkeyid", true, }, - {"With keyid belonging to a different key it should fail", + { + "With keyid belonging to a different key it should fail", jwt.SigningMethodES256, true, fakeOidcProvider.keyIds[jwt.SigningMethodRS256], true, diff --git a/internal/authn/oidc/fakes_test.go b/internal/authn/oidc/fakes_test.go index 9d3c76348..dc8fd0f52 100644 --- a/internal/authn/oidc/fakes_test.go +++ b/internal/authn/oidc/fakes_test.go @@ -149,7 +149,7 @@ func httpError(w http.ResponseWriter, code int) { } func (s *fakeOidcProvider) SignIDToken(unsignedToken *jwt.Token) (string, error) { - var signedToken = "" + signedToken := "" var err error switch unsignedToken.Method { diff --git a/internal/authn/preshared/authn.go b/internal/authn/preshared/authn.go index 57cfa6a0d..bd5623f59 100644 --- a/internal/authn/preshared/authn.go +++ b/internal/authn/preshared/authn.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "github.com/Permify/permify/internal/authn" + "github.com/Permify/permify/internal/config" ) // KeyAuthenticator - Interface for key authenticator @@ -20,12 +21,12 @@ type KeyAuthn struct { } // NewKeyAuthn - Create New Authenticated Keys -func NewKeyAuthn(keys ...string) (*KeyAuthn, error) { - if len(keys) < 1 { +func NewKeyAuthn(ctx context.Context, cfg config.Preshared) (*KeyAuthn, error) { + if len(cfg.Keys) < 1 { return nil, errors.New("pre shared key authn must have at least one key") } mapKeys := make(map[string]struct{}) - for _, k := range keys { + for _, k := range cfg.Keys { mapKeys[k] = struct{}{} } return &KeyAuthn{ diff --git a/internal/config/config.go b/internal/config/config.go index 689028a75..e59b0cfbc 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -49,12 +49,14 @@ type ( // Authn -. Authn struct { - Enabled bool `mapstructure:"enabled"` - Method string `mapstructure:"method"` - Keys []string `mapstructure:"keys"` - PrivateToken string `mapstructure:"private_token"` - Algorithms []string `mapstructure:"algorithms"` - Oidc Oidc `mapstructure:"oidc"` + Enabled bool `mapstructure:"enabled"` + Method string `mapstructure:"method"` + Preshared Preshared `mapstructure:"preshared"` + Oidc Oidc `mapstructure:"oidc"` + } + + Preshared struct { + Keys []string `mapstructure:"keys"` } Oidc struct { @@ -201,8 +203,9 @@ func DefaultConfig() *Config { Relationship: Relationship{}, }, Authn: Authn{ - Enabled: false, - Keys: []string{}, + Enabled: false, + Preshared: Preshared{}, + Oidc: Oidc{}, }, Database: Database{ Engine: "memory", diff --git a/internal/repositories/postgres/relationshipWriter_test.go b/internal/repositories/postgres/relationshipWriter_test.go index e592ec6cb..da3128568 100644 --- a/internal/repositories/postgres/relationshipWriter_test.go +++ b/internal/repositories/postgres/relationshipWriter_test.go @@ -92,7 +92,6 @@ var _ = Describe("RelationshipWriter", func() { Expect(err).ShouldNot(HaveOccurred()) // TODO: can we write a helper function to fetch the recently inserted record? as we are just creating a mock! any comments? Will think about it! - }) }) }) diff --git a/internal/servers/server.go b/internal/servers/server.go index 5cf3936d0..e8c30370f 100644 --- a/internal/servers/server.go +++ b/internal/servers/server.go @@ -25,7 +25,6 @@ import ( health "google.golang.org/grpc/health/grpc_health_v1" - "github.com/Permify/permify/internal/authn/multitenant" "github.com/Permify/permify/internal/authn/oidc" "github.com/Permify/permify/internal/authn/preshared" "github.com/Permify/permify/internal/config" @@ -63,20 +62,12 @@ func (s *ServiceContainer) Run(ctx context.Context, cfg *config.Server, authenti switch authentication.Method { case "preshared": var authenticator *preshared.KeyAuthn - authenticator, err = preshared.NewKeyAuthn(authentication.Keys...) + authenticator, err = preshared.NewKeyAuthn(ctx, authentication.Preshared) if err != nil { return err } unaryInterceptors = append(unaryInterceptors, grpcAuth.UnaryServerInterceptor(middleware.KeyAuthFunc(authenticator))) streamingInterceptors = append(streamingInterceptors, grpcAuth.StreamServerInterceptor(middleware.KeyAuthFunc(authenticator))) - case "multitenant": - var authenticator *multitenant.TenantAuthn - authenticator, err = multitenant.NewTenantAuthn(authentication.PrivateToken, authentication.Algorithms) - if err != nil { - return err - } - unaryInterceptors = append(unaryInterceptors, multitenant.UnaryServerInterceptor(authenticator)) - streamingInterceptors = append(streamingInterceptors, multitenant.StreamServerInterceptor(authenticator)) case "oidc": var authenticator *oidc.OidcAuthn authenticator, err = oidc.NewOidcAuthn(ctx, authentication.Oidc) @@ -86,7 +77,7 @@ func (s *ServiceContainer) Run(ctx context.Context, cfg *config.Server, authenti unaryInterceptors = append(unaryInterceptors, oidc.UnaryServerInterceptor(authenticator)) streamingInterceptors = append(streamingInterceptors, oidc.StreamServerInterceptor(authenticator)) default: - return fmt.Errorf("Unkown authentication method: '%s'", authentication.Method) + return fmt.Errorf("unkown authentication method: '%s'", authentication.Method) } } diff --git a/pkg/cmd/flags/serve.go b/pkg/cmd/flags/serve.go index c582624e9..63a86dd8b 100644 --- a/pkg/cmd/flags/serve.go +++ b/pkg/cmd/flags/serve.go @@ -1,9 +1,10 @@ package flags import ( - "github.com/Permify/permify/internal/config" "github.com/spf13/cobra" "github.com/spf13/viper" + + "github.com/Permify/permify/internal/config" ) // RegisterServeFlags - Define and registers permify CLI flags @@ -131,11 +132,11 @@ func RegisterServeFlags(cmd *cobra.Command) { panic(err) } - flags.StringSlice("authn-keys", conf.Authn.Keys, "preshared key/keys for server authentication") - if err = viper.BindPFlag("authn.keys", flags.Lookup("authn-keys")); err != nil { + flags.StringSlice("authn-preshared-keys", conf.Authn.Preshared.Keys, "preshared key/keys for server authentication") + if err = viper.BindPFlag("authn.preshared.keys", flags.Lookup("authn-preshared-keys")); err != nil { panic(err) } - if err = viper.BindEnv("authn.keys", "PERMIFY_AUTHN_KEYS"); err != nil { + if err = viper.BindEnv("authn.preshared.keys", "PERMIFY_AUTHN_PRESHARED_KEYS"); err != nil { panic(err) } diff --git a/pkg/cmd/migrate.go b/pkg/cmd/migrate.go index 927805238..1900ec305 100644 --- a/pkg/cmd/migrate.go +++ b/pkg/cmd/migrate.go @@ -1,14 +1,15 @@ package cmd import ( - "github.com/gookit/color" - "github.com/hashicorp/go-multierror" - "github.com/pressly/goose/v3" - "github.com/spf13/cobra" "os" "path/filepath" "strconv" "strings" + + "github.com/gookit/color" + "github.com/hashicorp/go-multierror" + "github.com/pressly/goose/v3" + "github.com/spf13/cobra" ) const ( @@ -115,15 +116,15 @@ func migrateUp() func(cmd *cobra.Command, args []string) error { color.Success.Println("migration successfully up: ✓ ✅ ") return nil - } else { - if err := goose.UpTo(db, "internal/repositories/postgres/migrations", p); err != nil { - color.Warn.Println("migration failed: Goose Up Error") - return nil - } + } - color.Success.Println("migration successfully up: ✓ ✅ ") + if err := goose.UpTo(db, "internal/repositories/postgres/migrations", p); err != nil { + color.Warn.Println("migration failed: Goose Up Error") return nil } + + color.Success.Println("migration successfully up: ✓ ✅ ") + return nil } } @@ -174,16 +175,16 @@ func migrateDown() func(cmd *cobra.Command, args []string) error { color.Success.Println("migration successfully down: ✓ ✅ ") return nil - } else { - if err := goose.DownTo(db, "internal/repositories/postgres/migrations", p); err != nil { - color.Warn.Println("migration failed: down error " + err.Error()) + } - return nil - } + if err := goose.DownTo(db, "internal/repositories/postgres/migrations", p); err != nil { + color.Warn.Println("migration failed: down error " + err.Error()) - color.Success.Println("migration successfully down: ✓ ✅ ") return nil } + + color.Success.Println("migration successfully down: ✓ ✅ ") + return nil } } diff --git a/pkg/dsl/compiler/compiler_test.go b/pkg/dsl/compiler/compiler_test.go index 64ea79bae..00d5984f9 100644 --- a/pkg/dsl/compiler/compiler_test.go +++ b/pkg/dsl/compiler/compiler_test.go @@ -835,7 +835,6 @@ var _ = Describe("compiler", func() { _, err = c.Compile() Expect(err.Error()).Should(Equal(base.ErrorCode_ERROR_CODE_RELATION_REFERENCE_MUST_HAVE_ONE_ENTITY_REFERENCE.String())) - }) It("Case 10", func() { @@ -866,7 +865,6 @@ var _ = Describe("compiler", func() { _, err = c.Compile() Expect(err.Error()).Should(Equal(base.ErrorCode_ERROR_CODE_RELATION_REFERENCE_NOT_FOUND_IN_ENTITY_REFERENCES.String())) - }) }) }) diff --git a/pkg/dsl/parser/parser_test.go b/pkg/dsl/parser/parser_test.go index 715d52e80..57b8b0d30 100644 --- a/pkg/dsl/parser/parser_test.go +++ b/pkg/dsl/parser/parser_test.go @@ -289,7 +289,6 @@ var _ = Describe("parser", func() { Expect(res2.Expression.(*ast.InfixExpression).Left.(*ast.Identifier).String()).Should(Equal("owner")) Expect(res2.Expression.(*ast.InfixExpression).Right.(*ast.Identifier).String()).Should(Equal("parent.create_repository")) - }) }) })