Skip to content

Commit

Permalink
VEGA-2105 Add JWT verification #minor (#54)
Browse files Browse the repository at this point in the history
* Install jwt library

* Add testify package

* Add tests for jwt verification

* Implement (most of the) jwt verification

* 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.

* Implement JWT subject verification based on issuer

* Add tests for constructing a JWTVerifier from env

* Implement JWTVerifier which can be constructed from env

* Incorporate verifier into create lambda

* 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

* Move header parsing out of lambda

Hopefully makes it slightly more reusable.

* Add JWT verification to get and update lambdas

* Move header-related code to VerifyHeader()

* Add unit tests for VerifyHeader()

* Tabs, not spaces

* Hard-code test JWT secret

* Reject requests with a 401 if JWT cannot be verified

* Add API tests for 401 from each lambda

* Show a bit more detail about API tests now there are more

Makes it easier to figure out which test failed (if any).

---------

Co-authored-by: Elliot Smith <[email protected]>
  • Loading branch information
townxelliot and Elliot Smith authored Nov 14, 2023
1 parent 425d275 commit c5f2b60
Show file tree
Hide file tree
Showing 13 changed files with 417 additions and 25 deletions.
12 changes: 8 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
@@ -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}'
Expand All @@ -20,10 +21,13 @@ 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 -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"}]}' && \
./api-test/tester -jwtSecret=$(JWT_SECRET_KEY) -expectedStatus=200 REQUEST GET $(URL)/lpas/$(LPA_UID) ''
.PHONY: test-api

create-tables:
Expand Down
34 changes: 27 additions & 7 deletions api-test/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <METHOD> <URL> <REQUEST BODY> to make a test request
// ./api-test/tester UID -> generate a UID
// ./api-test/tester -jwtSecret=secret -expectedStatus=200 REQUEST <METHOD> <URL> <REQUEST BODY>
// -> 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)
Expand All @@ -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])
Expand All @@ -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": "[email protected]",
}

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)
Expand All @@ -68,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())
}
}
3 changes: 3 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -34,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
Expand All @@ -51,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
Expand Down
19 changes: 14 additions & 5 deletions lambda/create/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,25 @@ 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 data shared.Lpa
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\"}",
}

var data shared.Lpa
err := json.Unmarshal([]byte(event.Body), &data)
if err != nil {
l.logger.Print(err)
Expand Down Expand Up @@ -86,8 +94,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)
Expand Down
17 changes: 13 additions & 4 deletions lambda/get/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,19 @@ 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) {
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\"}",
Expand Down Expand Up @@ -48,8 +56,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)
Expand Down
21 changes: 21 additions & 0 deletions lambda/shared/event.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
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 {
headerValues = []string{}
}

return headerValues
}
5 changes: 5 additions & 0 deletions lambda/shared/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +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
Expand All @@ -23,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
)
10 changes: 10 additions & 0 deletions lambda/shared/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ 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=
github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg=
github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
Expand All @@ -53,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=
Expand Down Expand Up @@ -112,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=
127 changes: 127 additions & 0 deletions lambda/shared/jwt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
package shared

import (
"errors"
"fmt"
"os"
"regexp"
"time"

"github.com/aws/aws-lambda-go/events"
"github.com/golang-jwt/jwt/v5"
)

const (
sirius string = "opg.poas.sirius"
mrlpa = "opg.poas.makeregister"
)

var validIssuers []string = []string{
sirius,
mrlpa,
}

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")
}

// 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
}

type JWTVerifier struct {
secretKey []byte
}

func NewJWTVerifier() JWTVerifier {
return JWTVerifier{
secretKey: []byte(os.Getenv("JWT_SECRET_KEY")),
}
}

// tokenStr is the JWT token, minus any "Bearer: " prefix
func (v JWTVerifier) VerifyToken(tokenStr string) error {
lsc := lpaStoreClaims{}

parsedToken, err := jwt.ParseWithClaims(tokenStr, &lsc, func(token *jwt.Token) (interface{}, error) {
return v.secretKey, nil
})

if err != nil {
return err
}

if !parsedToken.Valid {
return fmt.Errorf("Invalid JWT")
}

return nil
}

var bearerRegexp = regexp.MustCompile("^Bearer:[ ]+")

// verify JWT from event header
// returns true if verified, false otherwise
func (v JWTVerifier) VerifyHeader(event events.APIGatewayProxyRequest) bool {
jwtHeaders := GetEventHeader("X-Jwt-Authorization", event)

if len(jwtHeaders) < 1 {
return false
}

tokenStr := bearerRegexp.ReplaceAllString(jwtHeaders[0], "")
if v.VerifyToken(tokenStr) != nil {
return false
}

return true
}
Loading

0 comments on commit c5f2b60

Please sign in to comment.