From acded5f420ca8245547627f684f1383bee18e0ee Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Thu, 9 Nov 2023 11:39:00 +0000 Subject: [PATCH 01/19] Install jwt library --- lambda/shared/go.mod | 1 + lambda/shared/go.sum | 2 ++ 2 files changed, 3 insertions(+) diff --git a/lambda/shared/go.mod b/lambda/shared/go.mod index 549ab776..75f7cb24 100644 --- a/lambda/shared/go.mod +++ b/lambda/shared/go.mod @@ -10,6 +10,7 @@ require ( require ( github.com/andybalholm/brotli v1.0.4 // indirect + github.com/golang-jwt/jwt/v5 v5.1.0 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/jmespath/go-jmespath v0.4.0 // indirect github.com/klauspost/compress v1.15.9 // indirect diff --git a/lambda/shared/go.sum b/lambda/shared/go.sum index bec06c7a..f46798ef 100644 --- a/lambda/shared/go.sum +++ b/lambda/shared/go.sum @@ -35,6 +35,8 @@ github.com/aws/aws-xray-sdk-go v1.8.2 h1:PVxNWnQG+rAYjxsmhEN97DTO57Dipg6VS0wsu6b github.com/aws/aws-xray-sdk-go v1.8.2/go.mod h1:wMmVYzej3sykAttNBkXQHK/+clAPWTOrPiajEk7Cp3A= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/golang-jwt/jwt/v5 v5.1.0 h1:UGKbA/IPjtS6zLcdB7i5TyACMgSbOTiR8qzXgw8HWQU= +github.com/golang-jwt/jwt/v5 v5.1.0/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= From c48118f1dc566fdd87a66e8760a6b000c6ceace3 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Fri, 10 Nov 2023 17:09:06 +0000 Subject: [PATCH 02/19] Add testify package --- lambda/shared/go.mod | 4 ++++ lambda/shared/go.sum | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/lambda/shared/go.mod b/lambda/shared/go.mod index 75f7cb24..119d4b41 100644 --- a/lambda/shared/go.mod +++ b/lambda/shared/go.mod @@ -10,11 +10,14 @@ require ( require ( github.com/andybalholm/brotli v1.0.4 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/golang-jwt/jwt/v5 v5.1.0 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/jmespath/go-jmespath v0.4.0 // indirect github.com/klauspost/compress v1.15.9 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/objx v0.5.0 // indirect github.com/stretchr/testify v1.8.4 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect github.com/valyala/fasthttp v1.34.0 // indirect @@ -24,4 +27,5 @@ require ( google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect google.golang.org/grpc v1.56.3 // indirect google.golang.org/protobuf v1.30.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/lambda/shared/go.sum b/lambda/shared/go.sum index f46798ef..7f47e3f1 100644 --- a/lambda/shared/go.sum +++ b/lambda/shared/go.sum @@ -35,6 +35,7 @@ github.com/aws/aws-xray-sdk-go v1.8.2 h1:PVxNWnQG+rAYjxsmhEN97DTO57Dipg6VS0wsu6b github.com/aws/aws-xray-sdk-go v1.8.2/go.mod h1:wMmVYzej3sykAttNBkXQHK/+clAPWTOrPiajEk7Cp3A= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/golang-jwt/jwt/v5 v5.1.0 h1:UGKbA/IPjtS6zLcdB7i5TyACMgSbOTiR8qzXgw8HWQU= github.com/golang-jwt/jwt/v5 v5.1.0/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= @@ -55,6 +56,11 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= @@ -114,4 +120,6 @@ google.golang.org/protobuf v1.30.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqw gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From e301da8569e75f333ae9168a931d51c8511be1d5 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Fri, 10 Nov 2023 17:09:18 +0000 Subject: [PATCH 03/19] Add tests for jwt verification --- lambda/shared/jwt_test.go | 85 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 lambda/shared/jwt_test.go diff --git a/lambda/shared/jwt_test.go b/lambda/shared/jwt_test.go new file mode 100644 index 00000000..a2bd09e7 --- /dev/null +++ b/lambda/shared/jwt_test.go @@ -0,0 +1,85 @@ +package shared + +import ( + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/assert" +) + +var secretKey = []byte("secret") + +func createToken(claims jwt.MapClaims) (string, error) { + token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + + tokenString, err := token.SignedString(secretKey) + + if err != nil { + return "", err + } + + return tokenString, nil +} + +func TestVerifyEmptyJwt(t *testing.T) { + err := VerifyToken(secretKey, "") + assert.NotNil(t, err) +} + +func TestVerifyExpInPast(t *testing.T) { + token, _ := createToken(jwt.MapClaims{ + "exp": time.Now().Add(time.Hour * -24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.makeregister", + }) + + err := VerifyToken(secretKey, token) + + assert.NotNil(t, err) + if err != nil { + assert.Containsf(t, err.Error(), "token is expired", "") + } +} + +func TestVerifyIatInFuture(t *testing.T) { + token, _ := createToken(jwt.MapClaims{ + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * 24).Unix(), + "iss": "opg.poas.sirius", + }) + + err := VerifyToken(secretKey, token) + + assert.NotNil(t, err) + if err != nil { + assert.Containsf(t, err.Error(), "IssuedAt must not be in the future", "") + } +} + +func TestVerifyIssuer(t *testing.T) { + token, _ := createToken(jwt.MapClaims{ + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "daadsdaadsadsads", + }) + + err := VerifyToken(secretKey, token) + + assert.NotNil(t, err) + if err != nil { + assert.Containsf(t, err.Error(), "Invalid Issuer", "") + } +} + +func TestVerifyGoodJwt(t *testing.T) { + token, _ := createToken(jwt.MapClaims{ + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.sirius", + }) + + err := VerifyToken(secretKey, token) + + assert.Nil(t, err) +} From d2e0eef6d62d531219275a03a1a6b0a714c9eecb Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Fri, 10 Nov 2023 17:09:33 +0000 Subject: [PATCH 04/19] Implement (most of the) jwt verification --- lambda/shared/jwt.go | 69 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 lambda/shared/jwt.go diff --git a/lambda/shared/jwt.go b/lambda/shared/jwt.go new file mode 100644 index 00000000..f60e217a --- /dev/null +++ b/lambda/shared/jwt.go @@ -0,0 +1,69 @@ +package shared + +import ( + "errors" + "fmt" + "time" + + "github.com/golang-jwt/jwt/v5" +) + +var validIssuers []string = []string{ + "opg.poas.sirius", + "opg.poas.makeregister", +} + +type lpaStoreClaims struct { + jwt.RegisteredClaims +} + +// note that default validation for RegisteredClaims checks exp is in the future +func (l lpaStoreClaims) Validate() error { + // validate issued at (iat) + iat, err := l.GetIssuedAt() + if err != nil { + return err + } + + if iat.Time.After(time.Now()) { + return errors.New("IssuedAt must not be in the future") + } + + // validate issuer (iss) + iss, err := l.GetIssuer() + if err != nil { + return err + } + + isValid := false + for _, validIssuer := range validIssuers { + if validIssuer == iss { + isValid = true + break + } + } + + if !isValid { + return errors.New("Invalid Issuer") + } + + return nil +} + +func VerifyToken(secretKey []byte, tokenStr string) error { + lsc := lpaStoreClaims{} + + parsedToken, err := jwt.ParseWithClaims(tokenStr, &lsc, func(token *jwt.Token) (interface{}, error) { + return secretKey, nil + }) + + if err != nil { + return err + } + + if !parsedToken.Valid { + return fmt.Errorf("invalid token") + } + + return nil +} From aae37072b90eea1b4a015f16c12d050c830fa3a3 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Mon, 13 Nov 2023 14:15:44 +0000 Subject: [PATCH 05/19] Add test to validate JWT subject based on issuer Assumption at present is that the UID for MRLPA will be at least a one character string. --- lambda/shared/jwt_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/lambda/shared/jwt_test.go b/lambda/shared/jwt_test.go index a2bd09e7..d1a65ad6 100644 --- a/lambda/shared/jwt_test.go +++ b/lambda/shared/jwt_test.go @@ -32,6 +32,7 @@ func TestVerifyExpInPast(t *testing.T) { "exp": time.Now().Add(time.Hour * -24).Unix(), "iat": time.Now().Add(time.Hour * -24).Unix(), "iss": "opg.poas.makeregister", + "sub": "M-3467-89QW-ERTY", }) err := VerifyToken(secretKey, token) @@ -47,6 +48,7 @@ func TestVerifyIatInFuture(t *testing.T) { "exp": time.Now().Add(time.Hour * 24).Unix(), "iat": time.Now().Add(time.Hour * 24).Unix(), "iss": "opg.poas.sirius", + "sub": "someone@someplace.somewhere.com", }) err := VerifyToken(secretKey, token) @@ -62,6 +64,7 @@ func TestVerifyIssuer(t *testing.T) { "exp": time.Now().Add(time.Hour * 24).Unix(), "iat": time.Now().Add(time.Hour * -24).Unix(), "iss": "daadsdaadsadsads", + "sub": "someone@someplace.somewhere.com", }) err := VerifyToken(secretKey, token) @@ -72,11 +75,44 @@ func TestVerifyIssuer(t *testing.T) { } } +func TestVerifyBadEmailForSiriusIssuer(t *testing.T) { + token, _ := createToken(jwt.MapClaims{ + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.sirius", + "sub": "", + }) + + err := VerifyToken(secretKey, token) + + assert.NotNil(t, err) + if err != nil { + assert.Containsf(t, err.Error(), "Subject is not a valid email", "") + } +} + +func TestVerifyBadUIDForMRLPAIssuer(t *testing.T) { + token, _ := createToken(jwt.MapClaims{ + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.makeregister", + "sub": "", + }) + + err := VerifyToken(secretKey, token) + + assert.NotNil(t, err) + if err != nil { + assert.Containsf(t, err.Error(), "Subject is not a valid UID", "") + } +} + func TestVerifyGoodJwt(t *testing.T) { token, _ := createToken(jwt.MapClaims{ "exp": time.Now().Add(time.Hour * 24).Unix(), "iat": time.Now().Add(time.Hour * -24).Unix(), "iss": "opg.poas.sirius", + "sub": "someone@someplace.somewhere.com", }) err := VerifyToken(secretKey, token) From 1c4c1a7f9b3044fc15e5e7047081bc519f54f1bd Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Mon, 13 Nov 2023 14:16:22 +0000 Subject: [PATCH 06/19] Implement JWT subject verification based on issuer --- lambda/shared/jwt.go | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/lambda/shared/jwt.go b/lambda/shared/jwt.go index f60e217a..4a126042 100644 --- a/lambda/shared/jwt.go +++ b/lambda/shared/jwt.go @@ -3,14 +3,20 @@ package shared import ( "errors" "fmt" + "regexp" "time" "github.com/golang-jwt/jwt/v5" ) +const ( + sirius string = "opg.poas.sirius" + mrlpa = "opg.poas.makeregister" +) + var validIssuers []string = []string{ - "opg.poas.sirius", - "opg.poas.makeregister", + sirius, + mrlpa, } type lpaStoreClaims struct { @@ -47,6 +53,26 @@ func (l lpaStoreClaims) Validate() error { return errors.New("Invalid Issuer") } + // validate subject (sub) depending on the issuer value + sub, err := l.GetSubject() + if err != nil { + return err + } + + if iss == sirius { + emailRegex := regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") + if !emailRegex.MatchString(sub) { + return errors.New("Subject is not a valid email") + } + } + + if iss == mrlpa { + uidRegex := regexp.MustCompile("^.+$") + if !uidRegex.MatchString(sub) { + return errors.New("Subject is not a valid UID") + } + } + return nil } From 0f5a2e0954ea97f9fdd42b079d4de75a8dfee60e Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Mon, 13 Nov 2023 14:43:55 +0000 Subject: [PATCH 07/19] Add tests for constructing a JWTVerifier from env --- lambda/shared/jwt_test.go | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/lambda/shared/jwt_test.go b/lambda/shared/jwt_test.go index d1a65ad6..6edb86e4 100644 --- a/lambda/shared/jwt_test.go +++ b/lambda/shared/jwt_test.go @@ -1,6 +1,8 @@ package shared import ( + "fmt" + "os" "testing" "time" @@ -10,6 +12,10 @@ import ( var secretKey = []byte("secret") +var verifier = JWTVerifier{ + secretKey: secretKey, +} + func createToken(claims jwt.MapClaims) (string, error) { token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) @@ -23,7 +29,7 @@ func createToken(claims jwt.MapClaims) (string, error) { } func TestVerifyEmptyJwt(t *testing.T) { - err := VerifyToken(secretKey, "") + err := verifier.VerifyToken("") assert.NotNil(t, err) } @@ -35,7 +41,7 @@ func TestVerifyExpInPast(t *testing.T) { "sub": "M-3467-89QW-ERTY", }) - err := VerifyToken(secretKey, token) + err := verifier.VerifyToken(token) assert.NotNil(t, err) if err != nil { @@ -51,7 +57,7 @@ func TestVerifyIatInFuture(t *testing.T) { "sub": "someone@someplace.somewhere.com", }) - err := VerifyToken(secretKey, token) + err := verifier.VerifyToken(token) assert.NotNil(t, err) if err != nil { @@ -67,7 +73,7 @@ func TestVerifyIssuer(t *testing.T) { "sub": "someone@someplace.somewhere.com", }) - err := VerifyToken(secretKey, token) + err := verifier.VerifyToken(token) assert.NotNil(t, err) if err != nil { @@ -83,7 +89,7 @@ func TestVerifyBadEmailForSiriusIssuer(t *testing.T) { "sub": "", }) - err := VerifyToken(secretKey, token) + err := verifier.VerifyToken(token) assert.NotNil(t, err) if err != nil { @@ -99,7 +105,7 @@ func TestVerifyBadUIDForMRLPAIssuer(t *testing.T) { "sub": "", }) - err := VerifyToken(secretKey, token) + err := verifier.VerifyToken(token) assert.NotNil(t, err) if err != nil { @@ -115,7 +121,26 @@ func TestVerifyGoodJwt(t *testing.T) { "sub": "someone@someplace.somewhere.com", }) - err := VerifyToken(secretKey, token) + err := verifier.VerifyToken(token) + assert.Nil(t, err) + err = verifier.VerifyToken(fmt.Sprintf("Bearer: %s", token)) assert.Nil(t, err) } + +func TestNewJWTVerifier(t *testing.T) { + token, _ := createToken(jwt.MapClaims{ + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.sirius", + "sub": "someone@someplace.somewhere.com", + }) + + os.Setenv("JWT_SECRET_KEY", string(secretKey)) + newVerifier := NewJWTVerifier() + os.Unsetenv("JWT_SECRET_KEY") + + err := newVerifier.VerifyToken(token) + assert.Nil(t, err) +} + From 240165191e470f8051edf0e18977762708e6dbc9 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Mon, 13 Nov 2023 14:44:08 +0000 Subject: [PATCH 08/19] Implement JWTVerifier which can be constructed from env --- lambda/shared/jwt.go | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/lambda/shared/jwt.go b/lambda/shared/jwt.go index 4a126042..a423dfe6 100644 --- a/lambda/shared/jwt.go +++ b/lambda/shared/jwt.go @@ -3,6 +3,7 @@ package shared import ( "errors" "fmt" + "os" "regexp" "time" @@ -76,11 +77,28 @@ func (l lpaStoreClaims) Validate() error { return nil } -func VerifyToken(secretKey []byte, tokenStr string) error { +type JWTVerifier struct { + secretKey []byte +} + +func NewJWTVerifier() JWTVerifier { + return JWTVerifier{ + secretKey: []byte(os.Getenv("JWT_SECRET_KEY")), + } +} + +var bearerRegexp = regexp.MustCompile("^Bearer:[ ]+") + +// tokenStr may be just the JWT token, or can be prefixed with "^Bearer:[ ]+" +// (i.e. it can be the raw value from the authentication header in the original request); +// any prefix is stripped before parsing the token +func (v JWTVerifier) VerifyToken(tokenStr string) error { lsc := lpaStoreClaims{} + tokenStr = bearerRegexp.ReplaceAllString(tokenStr, "") + parsedToken, err := jwt.ParseWithClaims(tokenStr, &lsc, func(token *jwt.Token) (interface{}, error) { - return secretKey, nil + return v.secretKey, nil }) if err != nil { @@ -88,7 +106,7 @@ func VerifyToken(secretKey []byte, tokenStr string) error { } if !parsedToken.Valid { - return fmt.Errorf("invalid token") + return fmt.Errorf("invalid JWT") } return nil From b0bb74c2b4ae1304acb8839f26435b912502dce9 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Mon, 13 Nov 2023 15:08:52 +0000 Subject: [PATCH 09/19] Incorporate verifier into create lambda --- lambda/create/main.go | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/lambda/create/main.go b/lambda/create/main.go index ce8c7413..bde0c0c1 100644 --- a/lambda/create/main.go +++ b/lambda/create/main.go @@ -20,8 +20,9 @@ type Logger interface { } type Lambda struct { - store shared.Client - logger Logger + store shared.Client + verifier shared.JWTVerifier + logger Logger } func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) { @@ -31,7 +32,21 @@ func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRe Body: "{\"code\":\"INTERNAL_SERVER_ERROR\",\"detail\":\"Internal server error\"}", } - err := json.Unmarshal([]byte(event.Body), &data) + // check JWT before touching anything else in the event; + // NB we just log any errors here and still accept the request (for now) + authHeader, ok := event.Headers["Authorization"] + if !ok { + authHeader, ok = event.Headers["authorization"] + } + + if ok { + err := l.verifier.VerifyToken(authHeader) + if err != nil { + l.logger.Print(err) + } + } + + err = json.Unmarshal([]byte(event.Body), &data) if err != nil { l.logger.Print(err) return shared.ProblemInternalServerError.Respond() @@ -86,8 +101,9 @@ func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRe func main() { l := &Lambda{ - store: shared.NewDynamoDB(os.Getenv("DDB_TABLE_NAME_DEEDS")), - logger: logging.New(os.Stdout, "opg-data-lpa-store"), + store: shared.NewDynamoDB(os.Getenv("DDB_TABLE_NAME_DEEDS")), + verifier: shared.NewJWTVerifier(), + logger: logging.New(os.Stdout, "opg-data-lpa-store"), } lambda.Start(l.HandleEvent) From c49701090fe39d8cc9bc26704e7f03211fe678fd Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Mon, 13 Nov 2023 18:46:50 +0000 Subject: [PATCH 10/19] Wire up JWT verification to create lambda * Add JWT_SECRET_KEY variable to Makefile * Modify api-test/tester to send JWT header * Use X-Jwt-Authorization header for now to avoid clashes with AWS Authorization header (added by signer) * Expose JWT_SECRET_KEY as env var to create lambda * Add method for parsing a value out of an incoming event header * Log incoming JWT, whether verified or error --- Makefile | 9 +++++---- api-test/main.go | 32 ++++++++++++++++++++++++++------ docker-compose.yml | 1 + lambda/create/main.go | 16 +++++++++------- lambda/shared/event.go | 20 ++++++++++++++++++++ lambda/shared/jwt.go | 2 +- 6 files changed, 62 insertions(+), 18 deletions(-) create mode 100644 lambda/shared/event.go diff --git a/Makefile b/Makefile index 1509d059..f365e22b 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,7 @@ SHELL = '/bin/bash' export AWS_ACCESS_KEY_ID ?= X export AWS_SECRET_ACCESS_KEY ?= X +export JWT_SECRET_KEY := secret help: @grep --no-filename -E '^[0-9a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' @@ -20,10 +21,10 @@ test-api: $(shell go build -o ./api-test/tester ./api-test && chmod +x ./api-test/tester) $(eval LPA_UID := "$(shell ./api-test/tester UID)") - ./api-test/tester -expectedStatus=201 REQUEST PUT $(URL)/lpas/$(LPA_UID) '{"version":"1"}' && \ - ./api-test/tester -expectedStatus=400 REQUEST PUT $(URL)/lpas/$(LPA_UID) '{"version":"2"}' && \ - ./api-test/tester -expectedStatus=201 REQUEST POST $(URL)/lpas/$(LPA_UID)/updates '{"type":"BUMP_VERSION","changes":[{"key":"/version","old":"1","new":"2"}]}' && \ - ./api-test/tester -expectedStatus=200 REQUEST GET $(URL)/lpas/$(LPA_UID) '' + ./api-test/tester -jwtSecret=$(JWT_SECRET_KEY) -expectedStatus=201 REQUEST PUT $(URL)/lpas/$(LPA_UID) '{"version":"1"}' && \ + ./api-test/tester -jwtSecret=$(JWT_SECRET_KEY) -expectedStatus=400 REQUEST PUT $(URL)/lpas/$(LPA_UID) '{"version":"2"}' && \ + ./api-test/tester -jwtSecret=$(JWT_SECRET_KEY) -expectedStatus=201 REQUEST POST $(URL)/lpas/$(LPA_UID)/updates '{"type":"BUMP_VERSION","changes":[{"key":"/version","old":"1","new":"2"}]}' && \ + ./api-test/tester -jwtSecret=$(JWT_SECRET_KEY) -expectedStatus=200 REQUEST GET $(URL)/lpas/$(LPA_UID) '' .PHONY: test-api create-tables: diff --git a/api-test/main.go b/api-test/main.go index fa0e2452..91a41aad 100644 --- a/api-test/main.go +++ b/api-test/main.go @@ -12,18 +12,22 @@ import ( "github.com/aws/aws-sdk-go/aws/session" v4 "github.com/aws/aws-sdk-go/aws/signer/v4" + "github.com/golang-jwt/jwt/v5" "github.com/google/uuid" ) -// call with UID to generate a UID, or with -// -expectedStatus=200 REQUEST to make a test request +// ./api-test/tester UID -> generate a UID +// ./api-test/tester -jwtSecret=secret -expectedStatus=200 REQUEST +// -> make a test request with a JWT generated using secret "secret" and expected status 200 +// note that the jwtSecret sends a boilerplate JWT for now with valid iat, exp, iss and sub fields func main() { expectedStatusCode := flag.Int("expectedStatus", 200, "Expected response status code") + jwtSecret := flag.String("jwtSecret", "", "Add JWT Authorization header signed with this secret") flag.Parse() args := flag.Args() - // early exit if we're just generating a UID + // early exit if we're just generating a UID or JWT if args[0] == "UID" { fmt.Print("M-" + strings.ToUpper(uuid.NewString()[9:23])) os.Exit(0) @@ -33,9 +37,6 @@ func main() { panic("Unrecognised command") } - sess := session.Must(session.NewSession()) - signer := v4.NewSigner(sess.Config.Credentials) - method := args[1] url := args[2] body := strings.NewReader(args[3]) @@ -47,6 +48,25 @@ func main() { req.Header.Add("Content-type", "application/json") + if *jwtSecret != "" { + secretKey := []byte(*jwtSecret) + + claims := jwt.MapClaims{ + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.sirius", + "sub": "someone@someplace.somewhere.com", + } + + token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + tokenString, _ := token.SignedString(secretKey) + + req.Header.Add("X-Jwt-Authorization", fmt.Sprintf("Bearer: %s", tokenString)) + } + + sess := session.Must(session.NewSession()) + signer := v4.NewSigner(sess.Config.Credentials) + _, err = signer.Sign(req, body, "execute-api", "eu-west-1", time.Now()) if err != nil { panic(err) diff --git a/docker-compose.yml b/docker-compose.yml index 3eeb97cc..1fb0bbd1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -17,6 +17,7 @@ services: AWS_ACCESS_KEY_ID: X AWS_SECRET_ACCESS_KEY: X DDB_TABLE_NAME_DEEDS: deeds + JWT_SECRET_KEY: ${JWT_SECRET_KEY} volumes: - "./lambda/.aws-lambda-rie:/aws-lambda" entrypoint: /aws-lambda/aws-lambda-rie /var/task/main diff --git a/lambda/create/main.go b/lambda/create/main.go index bde0c0c1..162e3ad1 100644 --- a/lambda/create/main.go +++ b/lambda/create/main.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "fmt" "os" "time" @@ -27,22 +28,23 @@ type Lambda struct { func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) { var data shared.Lpa + var err error + response := events.APIGatewayProxyResponse{ StatusCode: 500, Body: "{\"code\":\"INTERNAL_SERVER_ERROR\",\"detail\":\"Internal server error\"}", } // check JWT before touching anything else in the event; - // NB we just log any errors here and still accept the request (for now) - authHeader, ok := event.Headers["Authorization"] - if !ok { - authHeader, ok = event.Headers["authorization"] - } + // NB we just log and accept the request (for now) + jwtHeaders := shared.GetEventHeader("X-Jwt-Authorization", event) - if ok { - err := l.verifier.VerifyToken(authHeader) + if len(jwtHeaders) > 0 { + err = l.verifier.VerifyToken(jwtHeaders[0]) if err != nil { l.logger.Print(err) + } else { + l.logger.Print(fmt.Printf("Successfully parsed JWT; header was %s", jwtHeaders[0])) } } diff --git a/lambda/shared/event.go b/lambda/shared/event.go new file mode 100644 index 00000000..8132466c --- /dev/null +++ b/lambda/shared/event.go @@ -0,0 +1,20 @@ +package shared + +import ( + "strings" + + "github.com/aws/aws-lambda-go/events" +) + +func GetEventHeader(headerName string, event events.APIGatewayProxyRequest) []string { + headerValues, ok := event.MultiValueHeaders[strings.Title(headerName)] + if !ok { + headerValues, ok = event.MultiValueHeaders[strings.ToLower(headerName)] + } + + if !ok { + return []string{} + } + + return headerValues +} diff --git a/lambda/shared/jwt.go b/lambda/shared/jwt.go index a423dfe6..68f975c4 100644 --- a/lambda/shared/jwt.go +++ b/lambda/shared/jwt.go @@ -90,7 +90,7 @@ func NewJWTVerifier() JWTVerifier { var bearerRegexp = regexp.MustCompile("^Bearer:[ ]+") // tokenStr may be just the JWT token, or can be prefixed with "^Bearer:[ ]+" -// (i.e. it can be the raw value from the authentication header in the original request); +// (i.e. it can be the raw value from the Authorization header in the original request); // any prefix is stripped before parsing the token func (v JWTVerifier) VerifyToken(tokenStr string) error { lsc := lpaStoreClaims{} From 748d00a75416f4d6ec5322f861f778d2a4d4ccd9 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Tue, 14 Nov 2023 11:55:02 +0000 Subject: [PATCH 11/19] Move header parsing out of lambda Hopefully makes it slightly more reusable. --- lambda/create/main.go | 15 +++++---------- lambda/shared/event.go | 3 ++- lambda/shared/jwt.go | 14 +++++++++++++- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lambda/create/main.go b/lambda/create/main.go index 162e3ad1..b450ec76 100644 --- a/lambda/create/main.go +++ b/lambda/create/main.go @@ -3,7 +3,6 @@ package main import ( "context" "encoding/json" - "fmt" "os" "time" @@ -37,15 +36,11 @@ func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRe // check JWT before touching anything else in the event; // NB we just log and accept the request (for now) - jwtHeaders := shared.GetEventHeader("X-Jwt-Authorization", event) - - if len(jwtHeaders) > 0 { - err = l.verifier.VerifyToken(jwtHeaders[0]) - if err != nil { - l.logger.Print(err) - } else { - l.logger.Print(fmt.Printf("Successfully parsed JWT; header was %s", jwtHeaders[0])) - } + err = l.verifier.VerifyHeader(event) + if err == nil { + l.logger.Print("Successfully parsed JWT from event header") + } else { + l.logger.Print(err) } err = json.Unmarshal([]byte(event.Body), &data) diff --git a/lambda/shared/event.go b/lambda/shared/event.go index 8132466c..95ecad6d 100644 --- a/lambda/shared/event.go +++ b/lambda/shared/event.go @@ -8,12 +8,13 @@ import ( func GetEventHeader(headerName string, event events.APIGatewayProxyRequest) []string { headerValues, ok := event.MultiValueHeaders[strings.Title(headerName)] + if !ok { headerValues, ok = event.MultiValueHeaders[strings.ToLower(headerName)] } if !ok { - return []string{} + headerValues = []string{} } return headerValues diff --git a/lambda/shared/jwt.go b/lambda/shared/jwt.go index 68f975c4..d1f726c8 100644 --- a/lambda/shared/jwt.go +++ b/lambda/shared/jwt.go @@ -7,6 +7,7 @@ import ( "regexp" "time" + "github.com/aws/aws-lambda-go/events" "github.com/golang-jwt/jwt/v5" ) @@ -106,8 +107,19 @@ func (v JWTVerifier) VerifyToken(tokenStr string) error { } if !parsedToken.Valid { - return fmt.Errorf("invalid JWT") + return fmt.Errorf("Invalid JWT") } return nil } + +// verify JWT from event header +func (v JWTVerifier) VerifyHeader(event events.APIGatewayProxyRequest) error { + jwtHeaders := GetEventHeader("X-Jwt-Authorization", event) + + if len(jwtHeaders) > 0 { + return v.VerifyToken(jwtHeaders[0]) + } + + return errors.New("No JWT authorization header present") +} From bea3c7fe6881ed15c0174184c0e09d5b2589bb0f Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Tue, 14 Nov 2023 12:08:58 +0000 Subject: [PATCH 12/19] Add JWT verification to get and update lambdas --- docker-compose.yml | 2 ++ lambda/create/main.go | 2 -- lambda/get/main.go | 19 +++++++++++++++---- lambda/update/main.go | 21 ++++++++++++++++----- 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 1fb0bbd1..bae43919 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -35,6 +35,7 @@ services: AWS_ACCESS_KEY_ID: X AWS_SECRET_ACCESS_KEY: X DDB_TABLE_NAME_DEEDS: deeds + JWT_SECRET_KEY: ${JWT_SECRET_KEY} volumes: - "./lambda/.aws-lambda-rie:/aws-lambda" entrypoint: /aws-lambda/aws-lambda-rie /var/task/main @@ -52,6 +53,7 @@ services: AWS_ACCESS_KEY_ID: X AWS_SECRET_ACCESS_KEY: X DDB_TABLE_NAME_DEEDS: deeds + JWT_SECRET_KEY: ${JWT_SECRET_KEY} volumes: - "./lambda/.aws-lambda-rie:/aws-lambda" entrypoint: /aws-lambda/aws-lambda-rie /var/task/main diff --git a/lambda/create/main.go b/lambda/create/main.go index b450ec76..6b255413 100644 --- a/lambda/create/main.go +++ b/lambda/create/main.go @@ -34,8 +34,6 @@ func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRe Body: "{\"code\":\"INTERNAL_SERVER_ERROR\",\"detail\":\"Internal server error\"}", } - // check JWT before touching anything else in the event; - // NB we just log and accept the request (for now) err = l.verifier.VerifyHeader(event) if err == nil { l.logger.Print("Successfully parsed JWT from event header") diff --git a/lambda/get/main.go b/lambda/get/main.go index 8a06327c..04fd5741 100644 --- a/lambda/get/main.go +++ b/lambda/get/main.go @@ -16,16 +16,26 @@ type Logger interface { } type Lambda struct { - store shared.Client - logger Logger + store shared.Client + verifier shared.JWTVerifier + logger Logger } func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) { + var err error + response := events.APIGatewayProxyResponse{ StatusCode: 500, Body: "{\"code\":\"INTERNAL_SERVER_ERROR\",\"detail\":\"Internal server error\"}", } + err = l.verifier.VerifyHeader(event) + if err == nil { + l.logger.Print("Successfully parsed JWT from event header") + } else { + l.logger.Print(err) + } + lpa, err := l.store.Get(ctx, event.PathParameters["uid"]) if err != nil { @@ -48,8 +58,9 @@ func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRe func main() { l := &Lambda{ - store: shared.NewDynamoDB(os.Getenv("DDB_TABLE_NAME_DEEDS")), - logger: logging.New(os.Stdout, "opg-data-lpa-store"), + store: shared.NewDynamoDB(os.Getenv("DDB_TABLE_NAME_DEEDS")), + verifier: shared.NewJWTVerifier(), + logger: logging.New(os.Stdout, "opg-data-lpa-store"), } lambda.Start(l.HandleEvent) diff --git a/lambda/update/main.go b/lambda/update/main.go index a5a7c280..5ff0ca4e 100644 --- a/lambda/update/main.go +++ b/lambda/update/main.go @@ -18,18 +18,28 @@ type Logger interface { } type Lambda struct { - store shared.Client - logger Logger + store shared.Client + verifier shared.JWTVerifier + logger Logger } func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) { var update shared.Update + var err error + response := events.APIGatewayProxyResponse{ StatusCode: 500, Body: "{\"code\":\"INTERNAL_SERVER_ERROR\",\"detail\":\"Internal server error\"}", } - err := json.Unmarshal([]byte(event.Body), &update) + err = l.verifier.VerifyHeader(event) + if err == nil { + l.logger.Print("Successfully parsed JWT from event header") + } else { + l.logger.Print(err) + } + + err = json.Unmarshal([]byte(event.Body), &update) if err != nil { l.logger.Print(err) return shared.ProblemInternalServerError.Respond() @@ -94,8 +104,9 @@ func applyUpdate(lpa *shared.Lpa, update shared.Update) error { func main() { l := &Lambda{ - store: shared.NewDynamoDB(os.Getenv("DDB_TABLE_NAME_DEEDS")), - logger: logging.New(os.Stdout, "opg-data-lpa-store"), + store: shared.NewDynamoDB(os.Getenv("DDB_TABLE_NAME_DEEDS")), + verifier: shared.NewJWTVerifier(), + logger: logging.New(os.Stdout, "opg-data-lpa-store"), } lambda.Start(l.HandleEvent) From 9ce63a44759acacbed470e4fed44d956b86c7cc5 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Tue, 14 Nov 2023 12:24:59 +0000 Subject: [PATCH 13/19] Move header-related code to VerifyHeader() --- lambda/shared/jwt.go | 13 +++++-------- lambda/shared/jwt_test.go | 4 ---- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/lambda/shared/jwt.go b/lambda/shared/jwt.go index d1f726c8..39450014 100644 --- a/lambda/shared/jwt.go +++ b/lambda/shared/jwt.go @@ -88,16 +88,10 @@ func NewJWTVerifier() JWTVerifier { } } -var bearerRegexp = regexp.MustCompile("^Bearer:[ ]+") - -// tokenStr may be just the JWT token, or can be prefixed with "^Bearer:[ ]+" -// (i.e. it can be the raw value from the Authorization header in the original request); -// any prefix is stripped before parsing the token +// tokenStr is the JWT token, minus any "Bearer: " prefix func (v JWTVerifier) VerifyToken(tokenStr string) error { lsc := lpaStoreClaims{} - tokenStr = bearerRegexp.ReplaceAllString(tokenStr, "") - parsedToken, err := jwt.ParseWithClaims(tokenStr, &lsc, func(token *jwt.Token) (interface{}, error) { return v.secretKey, nil }) @@ -113,12 +107,15 @@ func (v JWTVerifier) VerifyToken(tokenStr string) error { return nil } +var bearerRegexp = regexp.MustCompile("^Bearer:[ ]+") + // verify JWT from event header func (v JWTVerifier) VerifyHeader(event events.APIGatewayProxyRequest) error { jwtHeaders := GetEventHeader("X-Jwt-Authorization", event) if len(jwtHeaders) > 0 { - return v.VerifyToken(jwtHeaders[0]) + tokenStr := bearerRegexp.ReplaceAllString(jwtHeaders[0], "") + return v.VerifyToken(tokenStr) } return errors.New("No JWT authorization header present") diff --git a/lambda/shared/jwt_test.go b/lambda/shared/jwt_test.go index 6edb86e4..503896a2 100644 --- a/lambda/shared/jwt_test.go +++ b/lambda/shared/jwt_test.go @@ -1,7 +1,6 @@ package shared import ( - "fmt" "os" "testing" "time" @@ -123,9 +122,6 @@ func TestVerifyGoodJwt(t *testing.T) { err := verifier.VerifyToken(token) assert.Nil(t, err) - - err = verifier.VerifyToken(fmt.Sprintf("Bearer: %s", token)) - assert.Nil(t, err) } func TestNewJWTVerifier(t *testing.T) { From c54b37c09ba9f87ebbc216f0db0a376eedbdb855 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Tue, 14 Nov 2023 12:31:30 +0000 Subject: [PATCH 14/19] Add unit tests for VerifyHeader() --- lambda/shared/jwt_test.go | 57 +++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/lambda/shared/jwt_test.go b/lambda/shared/jwt_test.go index 503896a2..56c8ca87 100644 --- a/lambda/shared/jwt_test.go +++ b/lambda/shared/jwt_test.go @@ -1,10 +1,12 @@ package shared import ( + "fmt" "os" "testing" "time" + "github.com/aws/aws-lambda-go/events" "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/assert" ) @@ -15,16 +17,12 @@ var verifier = JWTVerifier{ secretKey: secretKey, } -func createToken(claims jwt.MapClaims) (string, error) { +func createToken(claims jwt.MapClaims) string { token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) - tokenString, err := token.SignedString(secretKey) + tokenString, _ := token.SignedString(secretKey) - if err != nil { - return "", err - } - - return tokenString, nil + return tokenString } func TestVerifyEmptyJwt(t *testing.T) { @@ -33,7 +31,7 @@ func TestVerifyEmptyJwt(t *testing.T) { } func TestVerifyExpInPast(t *testing.T) { - token, _ := createToken(jwt.MapClaims{ + token := createToken(jwt.MapClaims{ "exp": time.Now().Add(time.Hour * -24).Unix(), "iat": time.Now().Add(time.Hour * -24).Unix(), "iss": "opg.poas.makeregister", @@ -49,7 +47,7 @@ func TestVerifyExpInPast(t *testing.T) { } func TestVerifyIatInFuture(t *testing.T) { - token, _ := createToken(jwt.MapClaims{ + token := createToken(jwt.MapClaims{ "exp": time.Now().Add(time.Hour * 24).Unix(), "iat": time.Now().Add(time.Hour * 24).Unix(), "iss": "opg.poas.sirius", @@ -65,7 +63,7 @@ func TestVerifyIatInFuture(t *testing.T) { } func TestVerifyIssuer(t *testing.T) { - token, _ := createToken(jwt.MapClaims{ + token := createToken(jwt.MapClaims{ "exp": time.Now().Add(time.Hour * 24).Unix(), "iat": time.Now().Add(time.Hour * -24).Unix(), "iss": "daadsdaadsadsads", @@ -81,7 +79,7 @@ func TestVerifyIssuer(t *testing.T) { } func TestVerifyBadEmailForSiriusIssuer(t *testing.T) { - token, _ := createToken(jwt.MapClaims{ + token := createToken(jwt.MapClaims{ "exp": time.Now().Add(time.Hour * 24).Unix(), "iat": time.Now().Add(time.Hour * -24).Unix(), "iss": "opg.poas.sirius", @@ -97,7 +95,7 @@ func TestVerifyBadEmailForSiriusIssuer(t *testing.T) { } func TestVerifyBadUIDForMRLPAIssuer(t *testing.T) { - token, _ := createToken(jwt.MapClaims{ + token := createToken(jwt.MapClaims{ "exp": time.Now().Add(time.Hour * 24).Unix(), "iat": time.Now().Add(time.Hour * -24).Unix(), "iss": "opg.poas.makeregister", @@ -113,7 +111,7 @@ func TestVerifyBadUIDForMRLPAIssuer(t *testing.T) { } func TestVerifyGoodJwt(t *testing.T) { - token, _ := createToken(jwt.MapClaims{ + token := createToken(jwt.MapClaims{ "exp": time.Now().Add(time.Hour * 24).Unix(), "iat": time.Now().Add(time.Hour * -24).Unix(), "iss": "opg.poas.sirius", @@ -125,7 +123,7 @@ func TestVerifyGoodJwt(t *testing.T) { } func TestNewJWTVerifier(t *testing.T) { - token, _ := createToken(jwt.MapClaims{ + token := createToken(jwt.MapClaims{ "exp": time.Now().Add(time.Hour * 24).Unix(), "iat": time.Now().Add(time.Hour * -24).Unix(), "iss": "opg.poas.sirius", @@ -140,3 +138,34 @@ func TestNewJWTVerifier(t *testing.T) { assert.Nil(t, err) } +func TestVerifyHeaderNoJWTHeader(t *testing.T) { + event := events.APIGatewayProxyRequest{ + MultiValueHeaders: map[string][]string{}, + } + + err := verifier.VerifyHeader(event) + assert.NotNil(t, err) + if err != nil { + assert.Containsf(t, err.Error(), "No JWT authorization header present", "") + } +} + +func TestVerifyHeader(t *testing.T) { + token := createToken(jwt.MapClaims{ + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.sirius", + "sub": "someone@someplace.somewhere.com", + }) + + event := events.APIGatewayProxyRequest{ + MultiValueHeaders: map[string][]string{ + "X-Jwt-Authorization": []string{ + fmt.Sprintf("Bearer: %s", token), + }, + }, + } + + err := verifier.VerifyHeader(event) + assert.Nil(t, err) +} From 95ed0474ab8ea4d91a79a6851ccf12c32791a99a Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Tue, 14 Nov 2023 12:46:56 +0000 Subject: [PATCH 15/19] Tabs, not spaces --- api-test/main.go | 16 +++--- lambda/shared/jwt.go | 50 +++++++++---------- lambda/shared/jwt_test.go | 100 +++++++++++++++++++------------------- 3 files changed, 83 insertions(+), 83 deletions(-) diff --git a/api-test/main.go b/api-test/main.go index 91a41aad..5f248ceb 100644 --- a/api-test/main.go +++ b/api-test/main.go @@ -52,14 +52,14 @@ func main() { secretKey := []byte(*jwtSecret) claims := jwt.MapClaims{ - "exp": time.Now().Add(time.Hour * 24).Unix(), - "iat": time.Now().Add(time.Hour * -24).Unix(), - "iss": "opg.poas.sirius", - "sub": "someone@someplace.somewhere.com", - } - - token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) - tokenString, _ := token.SignedString(secretKey) + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.sirius", + "sub": "someone@someplace.somewhere.com", + } + + token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + tokenString, _ := token.SignedString(secretKey) req.Header.Add("X-Jwt-Authorization", fmt.Sprintf("Bearer: %s", tokenString)) } diff --git a/lambda/shared/jwt.go b/lambda/shared/jwt.go index 39450014..89dbfc10 100644 --- a/lambda/shared/jwt.go +++ b/lambda/shared/jwt.go @@ -63,19 +63,19 @@ func (l lpaStoreClaims) Validate() error { if iss == sirius { emailRegex := regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$") - if !emailRegex.MatchString(sub) { - return errors.New("Subject is not a valid email") - } - } - - if iss == mrlpa { - uidRegex := regexp.MustCompile("^.+$") - if !uidRegex.MatchString(sub) { - return errors.New("Subject is not a valid UID") - } - } - - return nil + if !emailRegex.MatchString(sub) { + return errors.New("Subject is not a valid email") + } + } + + if iss == mrlpa { + uidRegex := regexp.MustCompile("^.+$") + if !uidRegex.MatchString(sub) { + return errors.New("Subject is not a valid UID") + } + } + + return nil } type JWTVerifier struct { @@ -92,19 +92,19 @@ func NewJWTVerifier() JWTVerifier { func (v JWTVerifier) VerifyToken(tokenStr string) error { lsc := lpaStoreClaims{} - parsedToken, err := jwt.ParseWithClaims(tokenStr, &lsc, func(token *jwt.Token) (interface{}, error) { + parsedToken, err := jwt.ParseWithClaims(tokenStr, &lsc, func(token *jwt.Token) (interface{}, error) { return v.secretKey, nil - }) + }) - if err != nil { - return err - } + if err != nil { + return err + } - if !parsedToken.Valid { - return fmt.Errorf("Invalid JWT") - } + if !parsedToken.Valid { + return fmt.Errorf("Invalid JWT") + } - return nil + return nil } var bearerRegexp = regexp.MustCompile("^Bearer:[ ]+") @@ -113,10 +113,10 @@ var bearerRegexp = regexp.MustCompile("^Bearer:[ ]+") func (v JWTVerifier) VerifyHeader(event events.APIGatewayProxyRequest) error { jwtHeaders := GetEventHeader("X-Jwt-Authorization", event) - if len(jwtHeaders) > 0 { - tokenStr := bearerRegexp.ReplaceAllString(jwtHeaders[0], "") + if len(jwtHeaders) > 0 { + tokenStr := bearerRegexp.ReplaceAllString(jwtHeaders[0], "") return v.VerifyToken(tokenStr) - } + } return errors.New("No JWT authorization header present") } diff --git a/lambda/shared/jwt_test.go b/lambda/shared/jwt_test.go index 56c8ca87..a6fc4984 100644 --- a/lambda/shared/jwt_test.go +++ b/lambda/shared/jwt_test.go @@ -6,8 +6,8 @@ import ( "testing" "time" - "github.com/aws/aws-lambda-go/events" - "github.com/golang-jwt/jwt/v5" + "github.com/aws/aws-lambda-go/events" + "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/assert" ) @@ -18,11 +18,11 @@ var verifier = JWTVerifier{ } func createToken(claims jwt.MapClaims) string { - token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) + token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) - tokenString, _ := token.SignedString(secretKey) + tokenString, _ := token.SignedString(secretKey) - return tokenString + return tokenString } func TestVerifyEmptyJwt(t *testing.T) { @@ -32,11 +32,11 @@ func TestVerifyEmptyJwt(t *testing.T) { func TestVerifyExpInPast(t *testing.T) { token := createToken(jwt.MapClaims{ - "exp": time.Now().Add(time.Hour * -24).Unix(), - "iat": time.Now().Add(time.Hour * -24).Unix(), - "iss": "opg.poas.makeregister", - "sub": "M-3467-89QW-ERTY", - }) + "exp": time.Now().Add(time.Hour * -24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.makeregister", + "sub": "M-3467-89QW-ERTY", + }) err := verifier.VerifyToken(token) @@ -48,11 +48,11 @@ func TestVerifyExpInPast(t *testing.T) { func TestVerifyIatInFuture(t *testing.T) { token := createToken(jwt.MapClaims{ - "exp": time.Now().Add(time.Hour * 24).Unix(), - "iat": time.Now().Add(time.Hour * 24).Unix(), - "iss": "opg.poas.sirius", - "sub": "someone@someplace.somewhere.com", - }) + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * 24).Unix(), + "iss": "opg.poas.sirius", + "sub": "someone@someplace.somewhere.com", + }) err := verifier.VerifyToken(token) @@ -64,11 +64,11 @@ func TestVerifyIatInFuture(t *testing.T) { func TestVerifyIssuer(t *testing.T) { token := createToken(jwt.MapClaims{ - "exp": time.Now().Add(time.Hour * 24).Unix(), - "iat": time.Now().Add(time.Hour * -24).Unix(), - "iss": "daadsdaadsadsads", - "sub": "someone@someplace.somewhere.com", - }) + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "daadsdaadsadsads", + "sub": "someone@someplace.somewhere.com", + }) err := verifier.VerifyToken(token) @@ -80,11 +80,11 @@ func TestVerifyIssuer(t *testing.T) { func TestVerifyBadEmailForSiriusIssuer(t *testing.T) { token := createToken(jwt.MapClaims{ - "exp": time.Now().Add(time.Hour * 24).Unix(), - "iat": time.Now().Add(time.Hour * -24).Unix(), - "iss": "opg.poas.sirius", - "sub": "", - }) + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.sirius", + "sub": "", + }) err := verifier.VerifyToken(token) @@ -96,11 +96,11 @@ func TestVerifyBadEmailForSiriusIssuer(t *testing.T) { func TestVerifyBadUIDForMRLPAIssuer(t *testing.T) { token := createToken(jwt.MapClaims{ - "exp": time.Now().Add(time.Hour * 24).Unix(), - "iat": time.Now().Add(time.Hour * -24).Unix(), - "iss": "opg.poas.makeregister", - "sub": "", - }) + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.makeregister", + "sub": "", + }) err := verifier.VerifyToken(token) @@ -112,29 +112,29 @@ func TestVerifyBadUIDForMRLPAIssuer(t *testing.T) { func TestVerifyGoodJwt(t *testing.T) { token := createToken(jwt.MapClaims{ - "exp": time.Now().Add(time.Hour * 24).Unix(), - "iat": time.Now().Add(time.Hour * -24).Unix(), - "iss": "opg.poas.sirius", - "sub": "someone@someplace.somewhere.com", - }) + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.sirius", + "sub": "someone@someplace.somewhere.com", + }) - err := verifier.VerifyToken(token) + err := verifier.VerifyToken(token) assert.Nil(t, err) } func TestNewJWTVerifier(t *testing.T) { token := createToken(jwt.MapClaims{ - "exp": time.Now().Add(time.Hour * 24).Unix(), - "iat": time.Now().Add(time.Hour * -24).Unix(), - "iss": "opg.poas.sirius", - "sub": "someone@someplace.somewhere.com", - }) + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.sirius", + "sub": "someone@someplace.somewhere.com", + }) - os.Setenv("JWT_SECRET_KEY", string(secretKey)) - newVerifier := NewJWTVerifier() - os.Unsetenv("JWT_SECRET_KEY") + os.Setenv("JWT_SECRET_KEY", string(secretKey)) + newVerifier := NewJWTVerifier() + os.Unsetenv("JWT_SECRET_KEY") - err := newVerifier.VerifyToken(token) + err := newVerifier.VerifyToken(token) assert.Nil(t, err) } @@ -152,11 +152,11 @@ func TestVerifyHeaderNoJWTHeader(t *testing.T) { func TestVerifyHeader(t *testing.T) { token := createToken(jwt.MapClaims{ - "exp": time.Now().Add(time.Hour * 24).Unix(), - "iat": time.Now().Add(time.Hour * -24).Unix(), - "iss": "opg.poas.sirius", - "sub": "someone@someplace.somewhere.com", - }) + "exp": time.Now().Add(time.Hour * 24).Unix(), + "iat": time.Now().Add(time.Hour * -24).Unix(), + "iss": "opg.poas.sirius", + "sub": "someone@someplace.somewhere.com", + }) event := events.APIGatewayProxyRequest{ MultiValueHeaders: map[string][]string{ From 5b1b279176a40cbe1f9343c7cbff5a16e0e73d81 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Tue, 14 Nov 2023 12:48:59 +0000 Subject: [PATCH 16/19] Hard-code test JWT secret --- terraform/environment/region/main.tf | 1 + 1 file changed, 1 insertion(+) diff --git a/terraform/environment/region/main.tf b/terraform/environment/region/main.tf index c89196a0..458b2bc9 100644 --- a/terraform/environment/region/main.tf +++ b/terraform/environment/region/main.tf @@ -17,6 +17,7 @@ module "lambda" { environment_variables = { DDB_TABLE_NAME_DEEDS = var.dynamodb_name + JWT_SECRET_KEY = "secret" } providers = { From 6653e825248ebb69cb2302097f43baab6dbeb600 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Tue, 14 Nov 2023 15:51:30 +0000 Subject: [PATCH 17/19] Reject requests with a 401 if JWT cannot be verified --- lambda/create/main.go | 18 ++++++++---------- lambda/get/main.go | 14 ++++++-------- lambda/shared/jwt.go | 15 ++++++++++----- lambda/shared/jwt_test.go | 11 ++++------- lambda/shared/problem.go | 6 ++++++ lambda/update/main.go | 18 ++++++++---------- 6 files changed, 42 insertions(+), 40 deletions(-) diff --git a/lambda/create/main.go b/lambda/create/main.go index 6b255413..ae0c8057 100644 --- a/lambda/create/main.go +++ b/lambda/create/main.go @@ -26,22 +26,20 @@ type Lambda struct { } func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) { - var data shared.Lpa - var err error + if !l.verifier.VerifyHeader(event) { + l.logger.Print("Unable to verify JWT from header") + return shared.ProblemUnauthorisedRequest.Respond() + } + + l.logger.Print("Successfully parsed JWT from event header") response := events.APIGatewayProxyResponse{ StatusCode: 500, Body: "{\"code\":\"INTERNAL_SERVER_ERROR\",\"detail\":\"Internal server error\"}", } - err = l.verifier.VerifyHeader(event) - if err == nil { - l.logger.Print("Successfully parsed JWT from event header") - } else { - l.logger.Print(err) - } - - err = json.Unmarshal([]byte(event.Body), &data) + var data shared.Lpa + err := json.Unmarshal([]byte(event.Body), &data) if err != nil { l.logger.Print(err) return shared.ProblemInternalServerError.Respond() diff --git a/lambda/get/main.go b/lambda/get/main.go index 04fd5741..8e499524 100644 --- a/lambda/get/main.go +++ b/lambda/get/main.go @@ -22,20 +22,18 @@ type Lambda struct { } func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) { - var err error + if !l.verifier.VerifyHeader(event) { + l.logger.Print("Unable to verify JWT from header") + return shared.ProblemUnauthorisedRequest.Respond() + } + + l.logger.Print("Successfully parsed JWT from event header") response := events.APIGatewayProxyResponse{ StatusCode: 500, Body: "{\"code\":\"INTERNAL_SERVER_ERROR\",\"detail\":\"Internal server error\"}", } - err = l.verifier.VerifyHeader(event) - if err == nil { - l.logger.Print("Successfully parsed JWT from event header") - } else { - l.logger.Print(err) - } - lpa, err := l.store.Get(ctx, event.PathParameters["uid"]) if err != nil { diff --git a/lambda/shared/jwt.go b/lambda/shared/jwt.go index 89dbfc10..c309f39b 100644 --- a/lambda/shared/jwt.go +++ b/lambda/shared/jwt.go @@ -110,13 +110,18 @@ func (v JWTVerifier) VerifyToken(tokenStr string) error { var bearerRegexp = regexp.MustCompile("^Bearer:[ ]+") // verify JWT from event header -func (v JWTVerifier) VerifyHeader(event events.APIGatewayProxyRequest) error { +// returns true if verified, false otherwise +func (v JWTVerifier) VerifyHeader(event events.APIGatewayProxyRequest) bool { jwtHeaders := GetEventHeader("X-Jwt-Authorization", event) - if len(jwtHeaders) > 0 { - tokenStr := bearerRegexp.ReplaceAllString(jwtHeaders[0], "") - return v.VerifyToken(tokenStr) + if len(jwtHeaders) < 1 { + return false } - return errors.New("No JWT authorization header present") + tokenStr := bearerRegexp.ReplaceAllString(jwtHeaders[0], "") + if v.VerifyToken(tokenStr) != nil { + return false + } + + return true } diff --git a/lambda/shared/jwt_test.go b/lambda/shared/jwt_test.go index a6fc4984..fb90997f 100644 --- a/lambda/shared/jwt_test.go +++ b/lambda/shared/jwt_test.go @@ -143,11 +143,8 @@ func TestVerifyHeaderNoJWTHeader(t *testing.T) { MultiValueHeaders: map[string][]string{}, } - err := verifier.VerifyHeader(event) - assert.NotNil(t, err) - if err != nil { - assert.Containsf(t, err.Error(), "No JWT authorization header present", "") - } + verified := verifier.VerifyHeader(event) + assert.False(t, verified) } func TestVerifyHeader(t *testing.T) { @@ -166,6 +163,6 @@ func TestVerifyHeader(t *testing.T) { }, } - err := verifier.VerifyHeader(event) - assert.Nil(t, err) + verified := verifier.VerifyHeader(event) + assert.True(t, verified) } diff --git a/lambda/shared/problem.go b/lambda/shared/problem.go index 0a8eeccc..57fac580 100644 --- a/lambda/shared/problem.go +++ b/lambda/shared/problem.go @@ -42,6 +42,12 @@ var ProblemInvalidRequest Problem = Problem{ Detail: "Invalid request", } +var ProblemUnauthorisedRequest Problem = Problem{ + StatusCode: 401, + Code: "UNAUTHORISED", + Detail: "Invalid JWT", +} + func (problem Problem) Respond() (events.APIGatewayProxyResponse, error) { var errorString = "" for _, ve := range problem.Errors { diff --git a/lambda/update/main.go b/lambda/update/main.go index 5ff0ca4e..a2e935be 100644 --- a/lambda/update/main.go +++ b/lambda/update/main.go @@ -24,22 +24,20 @@ type Lambda struct { } func (l *Lambda) HandleEvent(ctx context.Context, event events.APIGatewayProxyRequest) (events.APIGatewayProxyResponse, error) { - var update shared.Update - var err error + if !l.verifier.VerifyHeader(event) { + l.logger.Print("Unable to verify JWT from header") + return shared.ProblemUnauthorisedRequest.Respond() + } + + l.logger.Print("Successfully parsed JWT from event header") response := events.APIGatewayProxyResponse{ StatusCode: 500, Body: "{\"code\":\"INTERNAL_SERVER_ERROR\",\"detail\":\"Internal server error\"}", } - err = l.verifier.VerifyHeader(event) - if err == nil { - l.logger.Print("Successfully parsed JWT from event header") - } else { - l.logger.Print(err) - } - - err = json.Unmarshal([]byte(event.Body), &update) + var update shared.Update + err := json.Unmarshal([]byte(event.Body), &update) if err != nil { l.logger.Print(err) return shared.ProblemInternalServerError.Respond() From 08b9129276ea32362cc880d95bb2601e3a330218 Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Tue, 14 Nov 2023 15:53:22 +0000 Subject: [PATCH 18/19] Add API tests for 401 from each lambda --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index f365e22b..daffe91e 100644 --- a/Makefile +++ b/Makefile @@ -21,6 +21,9 @@ test-api: $(shell go build -o ./api-test/tester ./api-test && chmod +x ./api-test/tester) $(eval LPA_UID := "$(shell ./api-test/tester UID)") + ./api-test/tester -expectedStatus=401 REQUEST PUT $(URL)/lpas/$(LPA_UID) '{"version":"1"}' && \ + ./api-test/tester -expectedStatus=401 REQUEST POST $(URL)/lpas/$(LPA_UID)/updates '{"type":"BUMP_VERSION","changes":[{"key":"/version","old":"1","new":"2"}]}' && \ + ./api-test/tester -expectedStatus=401 REQUEST GET $(URL)/lpas/$(LPA_UID) '' && \ ./api-test/tester -jwtSecret=$(JWT_SECRET_KEY) -expectedStatus=201 REQUEST PUT $(URL)/lpas/$(LPA_UID) '{"version":"1"}' && \ ./api-test/tester -jwtSecret=$(JWT_SECRET_KEY) -expectedStatus=400 REQUEST PUT $(URL)/lpas/$(LPA_UID) '{"version":"2"}' && \ ./api-test/tester -jwtSecret=$(JWT_SECRET_KEY) -expectedStatus=201 REQUEST POST $(URL)/lpas/$(LPA_UID)/updates '{"type":"BUMP_VERSION","changes":[{"key":"/version","old":"1","new":"2"}]}' && \ From 8f80c3bc385804f7f4758275dd0a6c7df1b2cc7c Mon Sep 17 00:00:00 2001 From: Elliot Smith Date: Tue, 14 Nov 2023 15:55:00 +0000 Subject: [PATCH 19/19] Show a bit more detail about API tests now there are more Makes it easier to figure out which test failed (if any). --- api-test/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api-test/main.go b/api-test/main.go index 5f248ceb..93b3459e 100644 --- a/api-test/main.go +++ b/api-test/main.go @@ -88,6 +88,6 @@ func main() { log.Printf("invalid status code %d; expected: %d", resp.StatusCode, *expectedStatusCode) log.Printf("error response: %s", buf.String()) } else { - log.Printf("Test passed - %d: %s", resp.StatusCode, buf.String()) + log.Printf("Test passed - %s to %s - %d: %s", method, url, resp.StatusCode, buf.String()) } }