From 2b3b68be149b557b097ad9218b629818c9cb6209 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Tue, 26 Mar 2024 17:21:32 +0100 Subject: [PATCH 01/28] Fixes 'superfluous response.WriteHeader' console message --- internal/httpserve/bankid.go | 4 ++-- internal/sse/sender.go | 17 ++++++++++------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index c902d45..fc88230 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -45,7 +45,7 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { return e.JSON(200, msg) } - sender, err := sse.NewSender(e.Response().Writer) + sender, err := sse.NewSender(e.Response()) if err != nil { fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) return e.JSON(500, "failed to setup response stream") @@ -127,7 +127,7 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { return e.JSON(200, msg) } - sender, err := sse.NewSender(e.Response().Writer) + sender, err := sse.NewSender(e.Response()) if err != nil { fmt.Printf("ERR: failed to setup sign response stream: %s\n", err.Error()) return e.JSON(500, "failed to setup response stream") diff --git a/internal/sse/sender.go b/internal/sse/sender.go index 59a9fc1..029725d 100644 --- a/internal/sse/sender.go +++ b/internal/sse/sender.go @@ -3,25 +3,28 @@ package sse import ( "errors" "fmt" + "github.com/labstack/echo/v4" "net/http" ) // Sender represents a http server-sent events sender used to send compatible SSE events. // See https://html.spec.whatwg.org/multipage/server-sent-events.html type Sender struct { - writer http.ResponseWriter - flusher http.Flusher + response *echo.Response + writer http.ResponseWriter + flusher http.Flusher } -func NewSender(writer http.ResponseWriter) (*Sender, error) { - flusher, ok := writer.(http.Flusher) +func NewSender(response *echo.Response) (*Sender, error) { + flusher, ok := response.Writer.(http.Flusher) if !ok { return nil, errors.New("failed to instantiate a http.Flusher from the response writer") } return &Sender{ - writer: writer, - flusher: flusher, + response: response, + writer: response.Writer, + flusher: flusher, }, nil } @@ -60,6 +63,6 @@ func (s *Sender) Send(event Event) error { } s.flusher.Flush() - + s.response.Committed = true return nil } From 8fd6c2688b337b71331e591d2b9eec77b0cb59de Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 27 Mar 2024 10:46:07 +0100 Subject: [PATCH 02/28] Refactor BankID loops waiting for status changes --- internal/httpserve/bankid.go | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index fc88230..b48c421 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -56,13 +56,6 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { interrupt := client.WatchForChange(e.Request().Context(), res.OrderRef) for i := 0; i < 30; i++ { - select { - case <-interrupt: - return e.JSON(http.StatusOK, bankid.Empty{}) - default: - break - } - msg := bankid.AuthSignAPIResponse{ OrderRef: res.OrderRef, URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", res.AutoStartToken), @@ -89,7 +82,11 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { } // Optimally subtract time that has elapsed, but no need to be that exact - time.Sleep(time.Second) + select { + case <-interrupt: + return e.JSON(http.StatusOK, bankid.Empty{}) + case <-time.After(time.Second): + } } return e.JSON(http.StatusOK, bankid.Empty{}) @@ -138,13 +135,6 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { interrupt := client.WatchForChange(e.Request().Context(), res.OrderRef) for i := 0; i < 30; i++ { - select { - case <-interrupt: - return e.JSON(http.StatusOK, bankid.Empty{}) - default: - break - } - msg := bankid.AuthSignAPIResponse{ OrderRef: res.OrderRef, URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", res.AutoStartToken), @@ -171,9 +161,12 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { } // Optimally subtract time that has elapsed, but no need to be that exact - time.Sleep(time.Second) + select { + case <-interrupt: + return e.JSON(http.StatusOK, bankid.Empty{}) + case <-time.After(time.Second): + } } - return e.JSON(http.StatusOK, bankid.Empty{}) }) From 70f8d47b18674b9846c66b3f7b2d1a08bdb32f07 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 27 Mar 2024 10:50:57 +0100 Subject: [PATCH 03/28] Minor changes * Move error check, to happen before the value returned is used * (Temporarily) comment unused struct variables (should be either removed or used) --- internal/eid/bankid/bankid.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/eid/bankid/bankid.go b/internal/eid/bankid/bankid.go index 8ce782c..8d16cad 100644 --- a/internal/eid/bankid/bankid.go +++ b/internal/eid/bankid/bankid.go @@ -25,9 +25,9 @@ func New(config ClientConfig) (client *BankID, err error) { pemClientCert: config.PemClientCert, pemRootCA: config.PemRootCA, - stop: make(chan struct{}), - infromAuth: make(chan string, 1), - infromSign: make(chan string, 1), + //stop: make(chan struct{}), + //infromAuth: make(chan string, 1), + //infromSign: make(chan string, 1), } p, _ := pem.Decode(client.pemClientCert) @@ -41,14 +41,14 @@ func New(config ClientConfig) (client *BankID, err error) { client.parsedClientCert = *cert client.httpClient, err = mtls.CreateHTTPClient(client.pemRootCA, client.pemClientCert, client.pemClientKey) + if err != nil { + return nil, err + } client.APIv51 = bankid_v51.NewEid(client.httpClient, config.BaseURL) client.APIv60 = bankid.NewAPI(client.httpClient, client.baseURL) - if err != nil { - return - } return } @@ -60,9 +60,9 @@ type BankID struct { httpClient *http.Client - stop chan struct{} - infromAuth chan string - infromSign chan string + //stop chan struct{} + //infromAuth chan string + //infromSign chan string pemRootCA []byte pemClientCert []byte From cd685f2422f161f0d04ec9b24edb8c2a9de95b89 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 27 Mar 2024 10:54:43 +0100 Subject: [PATCH 04/28] Exit process if we're unable to start the webserver --- cmd/twoferd/main.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/cmd/twoferd/main.go b/cmd/twoferd/main.go index 39dc520..3bb5844 100644 --- a/cmd/twoferd/main.go +++ b/cmd/twoferd/main.go @@ -89,14 +89,21 @@ func main() { } func startServer(e *echo.Echo) { + appCtx, appClose := context.WithCancel(context.Background()) go func() { - fmt.Println(e.Start(":8080")) + err := e.Start(":8080") + fmt.Println(err) + appClose() }() signalChannel := make(chan os.Signal, 1) signal.Notify(signalChannel, syscall.SIGTERM) - <-signalChannel + select { + case <-signalChannel: + case <-appCtx.Done(): + } + wg := sync.WaitGroup{} wg.Add(1) go func() { From a0c7ec5a8794557aa16fdd669d41f88dcc6a9cf8 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 27 Mar 2024 10:55:27 +0100 Subject: [PATCH 05/28] go mod tidy --- go.mod | 17 ++--------------- go.sum | 34 ---------------------------------- 2 files changed, 2 insertions(+), 49 deletions(-) diff --git a/go.mod b/go.mod index a0bf3d3..08babf3 100644 --- a/go.mod +++ b/go.mod @@ -4,49 +4,36 @@ go 1.19 require ( github.com/caarlos0/env/v6 v6.10.1 - github.com/davecgh/go-spew v1.1.1 github.com/go-webauthn/webauthn v0.8.2 - github.com/golang/protobuf v1.5.3 - github.com/gorilla/mux v1.8.0 + github.com/google/uuid v1.3.0 github.com/labstack/echo/v4 v4.10.2 - github.com/mdp/qrterminal/v3 v3.0.0 github.com/pquerna/otp v1.4.0 github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e github.com/stretchr/testify v1.8.2 - github.com/urfave/cli/v2 v2.20.2 golang.org/x/crypto v0.8.0 golang.org/x/net v0.9.0 - golang.org/x/oauth2 v0.7.0 - google.golang.org/grpc v1.52.0-dev ) require ( github.com/boombuler/barcode v1.0.1-0.20190219062509-6c824513bacc // indirect - github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/fxamacker/cbor/v2 v2.4.0 // indirect github.com/go-webauthn/revoke v0.1.9 // indirect github.com/golang-jwt/jwt v3.2.2+incompatible // indirect github.com/golang-jwt/jwt/v4 v4.5.0 // indirect - github.com/google/go-cmp v0.5.9 // indirect github.com/google/go-tpm v0.3.3 // indirect - github.com/google/uuid v1.3.0 // indirect github.com/kr/pretty v0.3.1 // indirect github.com/labstack/gommon v0.4.0 // indirect github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.18 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect github.com/valyala/fasttemplate v1.2.2 // indirect github.com/x448/float16 v0.8.4 // indirect - github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect golang.org/x/sys v0.7.0 // indirect golang.org/x/text v0.9.0 // indirect golang.org/x/time v0.3.0 // indirect - google.golang.org/genproto v0.0.0-20220822174746-9e6da59bd2fc // indirect - google.golang.org/protobuf v1.30.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - rsc.io/qr v0.2.0 // indirect ) diff --git a/go.sum b/go.sum index e1e2b2d..3dd7c7c 100644 --- a/go.sum +++ b/go.sum @@ -21,8 +21,6 @@ github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7 github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE= github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= -github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w= -github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= 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= @@ -61,18 +59,12 @@ github.com/golang/protobuf v1.4.0-rc.2/go.mod h1:LlEzMj4AhA7rCAGe4KMBDvJI+AwstrU github.com/golang/protobuf v1.4.0-rc.4.0.20200313231945-b860323f09d0/go.mod h1:WU3c8KckQ9AFe+yFwt9sWVRKCVIyN9cPHBJSNnbL67w= github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvqG2KuDX0= github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QDs8UjoX8= -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= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= -github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-tpm v0.1.2-0.20190725015402-ae6dd98980d4/go.mod h1:H9HbmUG2YgV/PHITkO7p6wxEEj/v5nlsVWIwumwH2NI= github.com/google/go-tpm v0.3.0/go.mod h1:iVLWvrPp/bHeEkxTFi9WG6K9w0iy2yIszHwZGHPbzAw= github.com/google/go-tpm v0.3.3 h1:P/ZFNBZYXRxc+z7i5uyd8VP7MaDteuLZInzrH2idRGo= @@ -81,8 +73,6 @@ github.com/google/go-tpm-tools v0.0.0-20190906225433-1614c142f845/go.mod h1:AVfH github.com/google/go-tpm-tools v0.2.0/go.mod h1:npUd03rQ60lxN7tzeBJreG38RvWwme2N1reF/eeiBk4= github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= -github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= github.com/grpc-ecosystem/go-grpc-middleware v1.0.0/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= @@ -108,19 +98,14 @@ github.com/labstack/echo/v4 v4.10.2/go.mod h1:OEyqf2//K1DFdE57vw2DRgWY0M7s65IVQO github.com/labstack/gommon v0.4.0 h1:y7cvthEAEbU0yHOf4axH8ZG2NH8knB9iNSoTO8dyIk8= github.com/labstack/gommon v0.4.0/go.mod h1:uW6kP17uPlLJsD3ijUYn3/M5bAxtlZhMI6m3MFxTMTM= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= -github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.11/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= -github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= github.com/mattn/go-isatty v0.0.18 h1:DOKFKCQ7FNG2L1rbrmstDN4QVRdS89Nkh85u68Uwp98= github.com/mattn/go-isatty v0.0.18/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= -github.com/mdp/qrterminal v1.0.1/go.mod h1:Z33WhxQe9B6CdW37HaVqcRKzP+kByF3q/qLxOGe12xQ= -github.com/mdp/qrterminal/v3 v3.0.0 h1:ywQqLRBXWTktytQNDKFjhAvoGkLVN3J2tAFZ0kMd9xQ= -github.com/mdp/qrterminal/v3 v3.0.0/go.mod h1:NJpfAs7OAm77Dy8EkWrtE4aq+cE6McoLXlBqXQEwvE0= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY= @@ -149,8 +134,6 @@ github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZV github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= -github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo= github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e h1:MRM5ITcdelLK2j1vwZ3Je0FKVCfqOLp5zO6trqMLYs0= @@ -179,8 +162,6 @@ github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= -github.com/urfave/cli/v2 v2.20.2 h1:dKA0LUjznZpwmmbrc0pOgcLTEilnHeM8Av9Yng77gHM= -github.com/urfave/cli/v2 v2.20.2/go.mod h1:1CNUng3PtjQMtRzJO4FMXBQvkGtuYRxxiR9xMa7jMwI= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/fasttemplate v1.2.1/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ= @@ -190,8 +171,6 @@ github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= -github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 h1:bAn7/zixMGCfxrRTfdpNzjtPYqr8smhKouy9mxVdGPU= -github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8= go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= @@ -215,8 +194,6 @@ golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR golang.org/x/net v0.9.0 h1:aWJ/m6xSmxWBx+V0XRHTlrYrPG56jKsLdTFmsSsCzOM= golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= -golang.org/x/oauth2 v0.7.0 h1:qe6s0zUXlPX80/dITx3440hWZ7GwMwgDDyrSGTPJG/g= -golang.org/x/oauth2 v0.7.0/go.mod h1:hPLQkd9LyjfXTiRohC/41GhcFqxisoUQ99sCUOHO9x4= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -227,7 +204,6 @@ golang.org/x/sys v0.0.0-20181107165924-66b7b1311ac8/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= -golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20210629170331-7dc0b73dc9fb/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -253,14 +229,10 @@ google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7 google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98Agz4BDEuKkezgsaosCRResVns1a3J2ZsMNc= google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo= -google.golang.org/genproto v0.0.0-20220822174746-9e6da59bd2fc h1:Nf+EdcTLHR8qDNN/KfkQL0u0ssxt9OhbaWCl5C0ucEI= -google.golang.org/genproto v0.0.0-20220822174746-9e6da59bd2fc/go.mod h1:dbqgFATTzChvnt+ujMdZwITVAJHFtfyN1qUhDqEiIlk= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.21.0/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= -google.golang.org/grpc v1.52.0-dev h1:yPVsJrs22LeKhr4nVwj9J0pePEBalDmPeVNDYdkTkjc= -google.golang.org/grpc v1.52.0-dev/go.mod h1:wgNDFcnuBGmxLKI/qn4T+m5BtEBYXJPvibbUPsAIPww= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= @@ -269,10 +241,6 @@ google.golang.org/protobuf v1.21.0/go.mod h1:47Nbq4nVaFHyn7ilMalzfO3qCViNmqZ2kzi google.golang.org/protobuf v1.22.0/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.23.1-0.20200526195155-81db48ad09cc/go.mod h1:EGpADcykh3NcUnDUJcl1+ZksZNG86OlYog2l/sGQquU= google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c= -google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= -google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= -google.golang.org/protobuf v1.30.0 h1:kPPoIgf3TsEvrm0PFe15JQ+570QVxYzEvvHqChK+cng= -google.golang.org/protobuf v1.30.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= @@ -288,5 +256,3 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= -rsc.io/qr v0.2.0 h1:6vBLea5/NRMVTz8V66gipeLycZMl/+UlFmk8DvqQ6WY= -rsc.io/qr v0.2.0/go.mod h1:IF+uZjkb9fqyeF/4tlBoynqmQxUoPfWEKh921coOuXs= From ee6e92cbcd482d0dc9b5daf6b95a431d7f330f3d Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 27 Mar 2024 13:11:43 +0100 Subject: [PATCH 06/28] Avoid error message in tests when server have been gracefully shut down --- test/main_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/main_test.go b/test/main_test.go index 4eaffaa..a784223 100644 --- a/test/main_test.go +++ b/test/main_test.go @@ -4,15 +4,16 @@ import ( "context" "errors" "fmt" + "log/slog" + "net/http" + "testing" + "time" + "github.com/labstack/echo/v4" "github.com/modfin/twofer/internal/bankid" "github.com/modfin/twofer/internal/httpserve" "github.com/modfin/twofer/test/fakes" "github.com/stretchr/testify/suite" - "log/slog" - "net/http" - "testing" - "time" ) type IntegrationTestSuite struct { @@ -47,7 +48,7 @@ func (s *IntegrationTestSuite) SetupSuite() { go func() { slog.Info("Starting twofer") err := s.twofer.Start(":8999") - if err != nil { + if !errors.Is(err, http.ErrServerClosed) { fmt.Println("Error starting twofer", err.Error()) } }() From b90ba5adb9a1d2c4ca3db30c76510397590eed96 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 27 Mar 2024 13:13:05 +0100 Subject: [PATCH 07/28] Avoid error message when server have been gracefully shut down --- cmd/twoferd/main.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/cmd/twoferd/main.go b/cmd/twoferd/main.go index 3bb5844..4bc1cdf 100644 --- a/cmd/twoferd/main.go +++ b/cmd/twoferd/main.go @@ -2,7 +2,16 @@ package main import ( "context" + "errors" "fmt" + "log" + "net/http" + "os" + "os/signal" + "sync" + "syscall" + "time" + "github.com/labstack/echo/v4/middleware" "github.com/modfin/twofer/internal/config" "github.com/modfin/twofer/internal/eid/bankid" @@ -11,12 +20,6 @@ import ( "github.com/modfin/twofer/internal/servotp" "github.com/modfin/twofer/internal/servpwd" "github.com/modfin/twofer/internal/servqr" - "log" - "os" - "os/signal" - "sync" - "syscall" - "time" "github.com/labstack/echo/v4" ) @@ -92,7 +95,9 @@ func startServer(e *echo.Echo) { appCtx, appClose := context.WithCancel(context.Background()) go func() { err := e.Start(":8080") - fmt.Println(err) + if !errors.Is(err, http.ErrServerClosed) { + fmt.Println(err) + } appClose() }() From 3220f48c3289194272ab29b0f98966f56b316703 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 27 Mar 2024 13:17:30 +0100 Subject: [PATCH 08/28] Q&D refactor to also send back status and hint changes over SSE --- internal/bankid/bankid_models.go | 8 ++ internal/bankid/endpoints.go | 73 ++++++++++---- internal/httpserve/bankid.go | 165 ++++++++++--------------------- internal/sse/sender.go | 30 ++++-- test/bankidv6_test.go | 146 +++++++++++++++++---------- 5 files changed, 229 insertions(+), 193 deletions(-) diff --git a/internal/bankid/bankid_models.go b/internal/bankid/bankid_models.go index 7f98bbc..c5d513b 100644 --- a/internal/bankid/bankid_models.go +++ b/internal/bankid/bankid_models.go @@ -80,6 +80,14 @@ type AuthSignResponse struct { QrStartSecret string `json:"qrStartSecret"` } +func (r *AuthSignResponse) APIResponse(qrCodeTime int) AuthSignAPIResponse { + return AuthSignAPIResponse{ + OrderRef: r.OrderRef, + URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", r.AutoStartToken), + QR: r.BuildQrCode(qrCodeTime), + } +} + // BuildQrCode builds a BankID compatible QR code // See https://www.bankid.com/utvecklare/guider/teknisk-integrationsguide/qrkoder func (r *AuthSignResponse) BuildQrCode(time int) string { diff --git a/internal/bankid/endpoints.go b/internal/bankid/endpoints.go index f198fb6..08c7f06 100644 --- a/internal/bankid/endpoints.go +++ b/internal/bankid/endpoints.go @@ -9,6 +9,9 @@ import ( "io" "net/http" "time" + + "github.com/labstack/echo/v4" + "github.com/modfin/twofer/internal/sse" ) const ( @@ -113,44 +116,78 @@ func (a *API) Change(ctx context.Context, r *ChangeRequest) (*CollectResponse, e } } -func (a *API) WatchForChange(ctx context.Context, orderRef string) chan WatchResponse { +func (a *API) WatchForChange(ctx echo.Context, orderRef string) (*sse.Sender, <-chan WatchResponse, error) { watch := make(chan WatchResponse) + sender, err := sse.NewSender(ctx.Response()) + if err != nil { + fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) + return nil, nil, err + } + + sender.Prepare() + + lastState, err := a.Collect(ctx.Request().Context(), &CollectRequest{OrderRef: orderRef}) + if err != nil { + return nil, nil, err + } + + var statusEvent struct { + Status Status + Hint HintCode + } + + updateStatus := func() { + statusEvent.Status = lastState.Status + statusEvent.Hint = lastState.HintCode + sender.Send("status", statusEvent) + + } + + updateStatus() go func() { changeRequest := &ChangeRequest{ OrderRef: orderRef, WaitUntilFinished: false, } + for { + check, err := a.Change(ctx.Request().Context(), changeRequest) + if err != nil { + if !errors.Is(err, context.Canceled) { + fmt.Println("ERR from change in bankid v6 auth/sign init: ", err) + watch <- WatchResponse{ + Cancelled: true, + Status: "", + } + close(watch) + return + } - check, err := a.Change(ctx, changeRequest) - if err != nil { - if !errors.Is(err, context.Canceled) { - fmt.Println("ERR from change in bankid v6 auth/sign init: ", err) watch <- WatchResponse{ Cancelled: true, - Status: "", + Status: err.Error(), } close(watch) return } - watch <- WatchResponse{ - Cancelled: true, - Status: err.Error(), + if check.Status != lastState.Status || check.HintCode != lastState.HintCode { + lastState = check + updateStatus() } - close(watch) - return - } - watch <- WatchResponse{ - Cancelled: false, - Status: string(check.Status), + if check.Status != Pending { + watch <- WatchResponse{ + Cancelled: check.Status == Failed, + Status: string(check.Status), + } + close(watch) + return + } } - close(watch) - return }() - return watch + return sender, watch, nil } func (a *API) Cancel(ctx context.Context, r *CancelRequest) error { diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index b48c421..d396984 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -3,239 +3,178 @@ package httpserve import ( "encoding/json" "fmt" - "github.com/labstack/echo/v4" - "github.com/modfin/twofer/internal/bankid" - "github.com/modfin/twofer/internal/sse" "io" "net/http" - "strconv" "time" + + "github.com/labstack/echo/v4" + "github.com/modfin/twofer/internal/bankid" ) func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { - e.POST("/bankid/v6/auth", func(e echo.Context) error { - b, err := io.ReadAll(e.Request().Body) + e.POST("/bankid/v6/auth", func(c echo.Context) error { + b, err := io.ReadAll(c.Request().Body) if err != nil { - return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) } var request bankid.AuthSignRequest err = json.Unmarshal(b, &request) if err != nil { fmt.Printf("ERR: failed to unmarshal auth request message: %s\n", err.Error()) - return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) } - res, err := client.Auth(e.Request().Context(), &request) + res, err := client.Auth(c.Request().Context(), &request) if err != nil { fmt.Printf("ERR: initiating auth request against bankid: %s\n", err.Error()) - return e.JSON(500, "failed to initiate auth against BankId") + return c.JSON(500, "failed to initiate auth against BankId") } // In the case a client wants to initiate a new request every second instead of relying on SSE // we respond with the first entry and then close the connection - response := e.QueryParam("type") + response := c.QueryParam("type") if response == "once" { - msg := bankid.AuthSignAPIResponse{ - OrderRef: res.OrderRef, - URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", res.AutoStartToken), - QR: res.BuildQrCode(0), - } - - return e.JSON(200, msg) + return c.JSON(200, res.APIResponse(0)) } - sender, err := sse.NewSender(e.Response()) + sender, interrupt, err := client.WatchForChange(c, res.OrderRef) if err != nil { - fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) - return e.JSON(500, "failed to setup response stream") + return c.JSON(500, "failed to setup response stream") } - sender.Prepare() - - interrupt := client.WatchForChange(e.Request().Context(), res.OrderRef) - for i := 0; i < 30; i++ { - msg := bankid.AuthSignAPIResponse{ - OrderRef: res.OrderRef, - URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", res.AutoStartToken), - QR: res.BuildQrCode(i), - } - - var bytes []byte - bytes, err = json.Marshal(msg) - if err != nil { - fmt.Printf("ERR: failed to build auth response message: %s\n", err.Error()) - return e.JSON(500, "failed to build response message") - } - - event := sse.Event{ - Id: strconv.Itoa(i), - Event: "message", - Data: string(bytes), - } - - err = sender.Send(event) + err = sender.Send("message", res.APIResponse(i)) // TODO: Should we change "message" to something else? if err != nil { fmt.Printf("ERR: failed to send auth response message: %s\n", err.Error()) - return e.JSON(500, "failed to send response message") + return c.JSON(500, "failed to send response message") } // Optimally subtract time that has elapsed, but no need to be that exact select { case <-interrupt: - return e.JSON(http.StatusOK, bankid.Empty{}) + return c.JSON(http.StatusOK, bankid.Empty{}) case <-time.After(time.Second): } } - return e.JSON(http.StatusOK, bankid.Empty{}) + return c.JSON(http.StatusOK, bankid.Empty{}) }) - e.POST("/bankid/v6/sign", func(e echo.Context) error { - b, err := io.ReadAll(e.Request().Body) + e.POST("/bankid/v6/sign", func(c echo.Context) error { + b, err := io.ReadAll(c.Request().Body) if err != nil { - return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) } var request bankid.AuthSignRequest err = json.Unmarshal(b, &request) if err != nil { fmt.Printf("ERR: failed to unmarshal sign request message: %s\n", err.Error()) - return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) } - res, err := client.Sign(e.Request().Context(), &request) + res, err := client.Sign(c.Request().Context(), &request) if err != nil { fmt.Printf("ERR: initiating sign request against bankid: %s\n", err.Error()) - return e.JSON(500, "failed to initiate sign against BankId") + return c.JSON(500, "failed to initiate sign against BankId") } // In the case a client wants to initiate a new request every second instead of relying on SSE // we respond with the first entry and then close the connection - response := e.QueryParam("type") + response := c.QueryParam("type") if response == "once" { - msg := bankid.AuthSignAPIResponse{ - OrderRef: res.OrderRef, - URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", res.AutoStartToken), - QR: res.BuildQrCode(0), - } - - return e.JSON(200, msg) + return c.JSON(200, res.APIResponse(0)) } - sender, err := sse.NewSender(e.Response()) + sender, interrupt, err := client.WatchForChange(c, res.OrderRef) if err != nil { - fmt.Printf("ERR: failed to setup sign response stream: %s\n", err.Error()) - return e.JSON(500, "failed to setup response stream") + return c.JSON(500, "failed to setup response stream") } - sender.Prepare() - - interrupt := client.WatchForChange(e.Request().Context(), res.OrderRef) - for i := 0; i < 30; i++ { - msg := bankid.AuthSignAPIResponse{ - OrderRef: res.OrderRef, - URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", res.AutoStartToken), - QR: res.BuildQrCode(i), - } - - var bytes []byte - bytes, err = json.Marshal(msg) - if err != nil { - fmt.Printf("ERR: failed to build sign response message: %s\n", err.Error()) - return e.JSON(500, "failed to build response message") - } - - event := sse.Event{ - Id: strconv.Itoa(i), - Event: "message", - Data: string(bytes), - } - - err = sender.Send(event) + err = sender.Send("message", res.APIResponse(i)) // TODO: Should we change "message" to something else? if err != nil { fmt.Printf("ERR: failed to send sign response message: %s\n", err.Error()) - return e.JSON(500, "failed to send response message") + return c.JSON(500, "failed to send response message") } // Optimally subtract time that has elapsed, but no need to be that exact select { case <-interrupt: - return e.JSON(http.StatusOK, bankid.Empty{}) + return c.JSON(http.StatusOK, bankid.Empty{}) case <-time.After(time.Second): } } - return e.JSON(http.StatusOK, bankid.Empty{}) + return c.JSON(http.StatusOK, bankid.Empty{}) }) - e.POST("/bankid/v6/change", func(e echo.Context) error { - b, err := io.ReadAll(e.Request().Body) + e.POST("/bankid/v6/change", func(c echo.Context) error { + b, err := io.ReadAll(c.Request().Body) if err != nil { fmt.Printf("ERR: failed to read change request message: %s\n", err.Error()) - return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) } var request bankid.ChangeRequest err = json.Unmarshal(b, &request) if err != nil { fmt.Printf("ERR: failed to unmarshal change request message: %s\n", err.Error()) - return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) } - res, err := client.Change(e.Request().Context(), &request) + res, err := client.Change(c.Request().Context(), &request) if err != nil { fmt.Printf("ERR: initiating change request against bankid: %s\n", err.Error()) - return e.JSON(500, "failed to start change request") + return c.JSON(500, "failed to start change request") } - return e.JSON(http.StatusOK, res) + return c.JSON(http.StatusOK, res) }) - e.POST("/bankid/v6/collect", func(e echo.Context) error { - b, err := io.ReadAll(e.Request().Body) + e.POST("/bankid/v6/collect", func(c echo.Context) error { + b, err := io.ReadAll(c.Request().Body) if err != nil { fmt.Printf("ERR: failed to read collect request message: %s\n", err.Error()) - return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) } var request bankid.CollectRequest err = json.Unmarshal(b, &request) if err != nil { fmt.Printf("ERR: failed to unmarshal collect request message: %s\n", err.Error()) - return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) } - res, err := client.Collect(e.Request().Context(), &request) + res, err := client.Collect(c.Request().Context(), &request) if err != nil { fmt.Printf("ERR: initiating collect request against bankid: %s\n", err.Error()) - return e.JSON(500, "failed to start collect against BankID") + return c.JSON(500, "failed to start collect against BankID") } - return e.JSON(http.StatusOK, res) + return c.JSON(http.StatusOK, res) }) - e.POST("/bankid/v6/cancel", func(e echo.Context) error { - b, err := io.ReadAll(e.Request().Body) + e.POST("/bankid/v6/cancel", func(c echo.Context) error { + b, err := io.ReadAll(c.Request().Body) if err != nil { fmt.Printf("ERR: failed to read cancel request message: %s\n", err.Error()) - return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) } var request bankid.CancelRequest err = json.Unmarshal(b, &request) if err != nil { fmt.Printf("ERR: failed to unmarshal cancel request message: %s\n", err.Error()) - return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) } - err = client.Cancel(e.Request().Context(), &request) + err = client.Cancel(c.Request().Context(), &request) if err != nil { fmt.Printf("ERR: initiating cancel request against bankid: %s\n", err.Error()) - return e.JSON(500, "failed to start cancel against BankID") + return c.JSON(500, "failed to start cancel against BankID") } - return e.NoContent(http.StatusNoContent) + return c.NoContent(http.StatusNoContent) }) } diff --git a/internal/sse/sender.go b/internal/sse/sender.go index 029725d..6df8492 100644 --- a/internal/sse/sender.go +++ b/internal/sse/sender.go @@ -1,15 +1,19 @@ package sse import ( + "encoding/json" "errors" "fmt" - "github.com/labstack/echo/v4" "net/http" + "sync/atomic" + + "github.com/labstack/echo/v4" ) // Sender represents a http server-sent events sender used to send compatible SSE events. // See https://html.spec.whatwg.org/multipage/server-sent-events.html type Sender struct { + eventID atomic.Uint32 response *echo.Response writer http.ResponseWriter flusher http.Flusher @@ -35,24 +39,30 @@ func (s *Sender) Prepare() { s.writer.Header().Set("Transfer-Encoding", "chunked") } -type Event struct { - Id string `json:"id"` - Event string `json:"event"` - Data string `json:"data"` -} +// type Event struct { +// Id string `json:"id"` +// Event string `json:"event"` +// Data string `json:"data"` +// } + +func (s *Sender) Send(event string, data any) error { + bytes, err := json.Marshal(data) + if err != nil { + fmt.Printf("ERR: failed to marshal event message: %v\n", err) + return err + } -func (s *Sender) Send(event Event) error { - _, err := fmt.Fprintf(s.writer, "id: %s\n", event.Id) + _, err = fmt.Fprintf(s.writer, "id: %d\n", s.eventID.Add(1)-1) if err != nil { return err } - _, err = fmt.Fprintf(s.writer, "event: %s\n", event.Event) + _, err = fmt.Fprintf(s.writer, "event: %s\n", event) if err != nil { return err } - _, err = fmt.Fprintf(s.writer, "data: %s\n", event.Data) + _, err = fmt.Fprintf(s.writer, "data: %s\n", bytes) if err != nil { return err } diff --git a/test/bankidv6_test.go b/test/bankidv6_test.go index e052687..8667896 100644 --- a/test/bankidv6_test.go +++ b/test/bankidv6_test.go @@ -7,12 +7,12 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" - "github.com/modfin/twofer/internal/bankid" - "github.com/modfin/twofer/internal/sse" "io" "net/http" "strconv" "strings" + + "github.com/modfin/twofer/internal/bankid" ) func (s *IntegrationTestSuite) TestAuthCallOnce() { @@ -82,8 +82,11 @@ func (s *IntegrationTestSuite) TestAuthCall() { // Default to splitting on each line scanner := bufio.NewScanner(resp.Body) defer resp.Body.Close() - for i := 0; i < 2; i++ { // Server should run for 30 seconds, but let's not wait that long for tests. If pattern holds for 2 messages it should be fine - msg := sse.Event{} + var eventTypes = [3]string{"status", "message", "message"} + var qrTime int + for i := 0; i < 3; i++ { // Server should run for 30 seconds, but let's not wait that long for tests. If pattern holds for 2 messages it should be fine + s.T().Logf("Iteration #%d", i) + // msg := sse.Event{} scanRes := scanner.Scan() if !scanRes { @@ -95,7 +98,7 @@ func (s *IntegrationTestSuite) TestAuthCall() { s.Equal("id", idSplit[0]) s.Equal(strconv.Itoa(i), idSplit[1]) - msg.Id = idSplit[1] + // msg.Id = idSplit[1] scanRes = scanner.Scan() if !scanRes { @@ -105,9 +108,9 @@ func (s *IntegrationTestSuite) TestAuthCall() { eventLine := scanner.Text() eventSplit := strings.Split(eventLine, ": ") s.Equal("event", eventSplit[0]) - s.Equal("message", eventSplit[1]) + s.Equal(eventTypes[i], eventSplit[1]) - msg.Event = eventSplit[1] + // msg.Event = eventSplit[1] scanRes = scanner.Scan() if !scanRes { @@ -118,7 +121,7 @@ func (s *IntegrationTestSuite) TestAuthCall() { dataSplit := strings.Split(dataLine, ": ") s.Equal("data", dataSplit[0]) - msg.Data = dataSplit[1] + // msg.Data = dataSplit[1] // SSE messages ends in extra empty line, so scan once more scanRes = scanner.Scan() @@ -126,26 +129,44 @@ func (s *IntegrationTestSuite) TestAuthCall() { s.FailNow("Server-Sent Events message stopped after data") } - var res bankid.AuthSignAPIResponse - err = json.Unmarshal([]byte(msg.Data), &res) - if err != nil { - s.NoError(err, "error unmarshaling SSE data for msg") + switch eventTypes[i] { + case "message": + var res bankid.AuthSignAPIResponse + err = json.Unmarshal([]byte(dataSplit[1]), &res) + if err != nil { + s.NoError(err, "error unmarshaling SSE data for msg") + } + + s.True(res.OrderRef != "") + + truth, ok := s.bankidv6.Orders[res.OrderRef] + s.True(ok, "no matching order in bankid fake") + + mac := hmac.New(sha256.New, []byte(truth.QrStartSecret)) + mac.Write([]byte(strconv.Itoa(qrTime))) + qrAuthCode := mac.Sum(nil) + + authCode := hex.EncodeToString(qrAuthCode) + qr := "bankid." + truth.QrStartToken + "." + strconv.Itoa(qrTime) + "." + authCode + s.Equal(qr, res.QR) + + s.Equal("bankid:///?autostarttoken="+truth.AutoStartToken+"&redirect=null", res.URI) + qrTime++ + case "status": + var res struct { + Status string + Hint string + } + err = json.Unmarshal([]byte(dataSplit[1]), &res) + if err != nil { + s.NoError(err, "error unmarshaling SSE data for msg") + } + + s.T().Logf("Status: %v", res) + s.True(res.Status != "", "Status is empty") + + s.True(res.Hint != "", "Hint is empty") } - - s.True(res.OrderRef != "") - - truth, ok := s.bankidv6.Orders[res.OrderRef] - s.True(ok, "no matching order in bankid fake") - - mac := hmac.New(sha256.New, []byte(truth.QrStartSecret)) - mac.Write([]byte(strconv.Itoa(i))) - qrAuthCode := mac.Sum(nil) - - authCode := hex.EncodeToString(qrAuthCode) - qr := "bankid." + truth.QrStartToken + "." + strconv.Itoa(i) + "." + authCode - s.Equal(qr, res.QR) - - s.Equal("bankid:///?autostarttoken="+truth.AutoStartToken+"&redirect=null", res.URI) } } @@ -362,8 +383,11 @@ func (s *IntegrationTestSuite) TestSignCall() { // Default to splitting on each line scanner := bufio.NewScanner(resp.Body) defer resp.Body.Close() - for i := 0; i < 2; i++ { // Server should run for 30 seconds, but let's not wait that long for tests. If pattern holds for 2 messages it should be fine - msg := sse.Event{} + var eventTypes = [3]string{"status", "message", "message"} + var qrTime int + for i := 0; i < 3; i++ { // Server should run for 30 seconds, but let's not wait that long for tests. If pattern holds for 2 messages it should be fine + s.T().Logf("Iteration #%d", i) + // msg := sse.Event{} scanRes := scanner.Scan() if !scanRes { @@ -375,7 +399,7 @@ func (s *IntegrationTestSuite) TestSignCall() { s.Equal("id", idSplit[0]) s.Equal(strconv.Itoa(i), idSplit[1]) - msg.Id = idSplit[1] + // msg.Id = idSplit[1] scanRes = scanner.Scan() if !scanRes { @@ -385,9 +409,9 @@ func (s *IntegrationTestSuite) TestSignCall() { eventLine := scanner.Text() eventSplit := strings.Split(eventLine, ": ") s.Equal("event", eventSplit[0]) - s.Equal("message", eventSplit[1]) + s.Equal(eventTypes[i], eventSplit[1]) - msg.Event = eventSplit[1] + // msg.Event = eventSplit[1] scanRes = scanner.Scan() if !scanRes { @@ -398,7 +422,7 @@ func (s *IntegrationTestSuite) TestSignCall() { dataSplit := strings.Split(dataLine, ": ") s.Equal("data", dataSplit[0]) - msg.Data = dataSplit[1] + // msg.Data = dataSplit[1] // SSE messages ends in extra empty line, so scan once more scanRes = scanner.Scan() @@ -406,26 +430,44 @@ func (s *IntegrationTestSuite) TestSignCall() { s.FailNow("Server-Sent Events message stopped after data") } - var res bankid.AuthSignAPIResponse - err = json.Unmarshal([]byte(msg.Data), &res) - if err != nil { - s.NoError(err, "error unmarshaling SSE data for msg") + switch eventTypes[i] { + case "message": + var res bankid.AuthSignAPIResponse + err = json.Unmarshal([]byte(dataSplit[1]), &res) + if err != nil { + s.NoError(err, "error unmarshaling SSE data for msg") + } + + s.True(res.OrderRef != "") + + truth, ok := s.bankidv6.Orders[res.OrderRef] + s.True(ok, "no matching order in bankid fake") + + mac := hmac.New(sha256.New, []byte(truth.QrStartSecret)) + mac.Write([]byte(strconv.Itoa(qrTime))) + qrAuthCode := mac.Sum(nil) + + signCode := hex.EncodeToString(qrAuthCode) + qr := "bankid." + truth.QrStartToken + "." + strconv.Itoa(qrTime) + "." + signCode + s.Equal(qr, res.QR) + + s.Equal("bankid:///?autostarttoken="+truth.AutoStartToken+"&redirect=null", res.URI) + qrTime++ + case "status": + var res struct { + Status string + Hint string + } + err = json.Unmarshal([]byte(dataSplit[1]), &res) + if err != nil { + s.NoError(err, "error unmarshaling SSE data for msg") + } + + s.T().Logf("Status: %v", res) + s.True(res.Status != "", "Status is empty") + + s.True(res.Hint != "", "Hint is empty") } - - s.True(res.OrderRef != "") - - truth, ok := s.bankidv6.Orders[res.OrderRef] - s.True(ok, "no matching order in bankid fake") - - mac := hmac.New(sha256.New, []byte(truth.QrStartSecret)) - mac.Write([]byte(strconv.Itoa(i))) - qrAuthCode := mac.Sum(nil) - - signCode := hex.EncodeToString(qrAuthCode) - qr := "bankid." + truth.QrStartToken + "." + strconv.Itoa(i) + "." + signCode - s.Equal(qr, res.QR) - - s.Equal("bankid:///?autostarttoken="+truth.AutoStartToken+"&redirect=null", res.URI) } } From 71f03a42a8fc1f891a249fb8295ea691e98cb0cd Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Thu, 11 Apr 2024 08:38:13 +0200 Subject: [PATCH 09/28] WIP: cleanup --- internal/bankid/bankid_models.go | 6 +- internal/bankid/endpoints.go | 95 +++++++++-------------- internal/httpserve/bankid.go | 129 ++++++++++++++++++++++--------- 3 files changed, 133 insertions(+), 97 deletions(-) diff --git a/internal/bankid/bankid_models.go b/internal/bankid/bankid_models.go index c5d513b..46aac18 100644 --- a/internal/bankid/bankid_models.go +++ b/internal/bankid/bankid_models.go @@ -175,9 +175,11 @@ type StepUp struct { MRTD bool `json:"mrtd"` } -type WatchResponse struct { +type WatchChanges struct { Cancelled bool - Status string + Status Status + Hint HintCode + Err error } type CancelRequest struct { diff --git a/internal/bankid/endpoints.go b/internal/bankid/endpoints.go index 08c7f06..e1dc2a1 100644 --- a/internal/bankid/endpoints.go +++ b/internal/bankid/endpoints.go @@ -4,14 +4,9 @@ import ( "bytes" "context" "encoding/json" - "errors" - "fmt" "io" "net/http" "time" - - "github.com/labstack/echo/v4" - "github.com/modfin/twofer/internal/sse" ) const ( @@ -116,78 +111,62 @@ func (a *API) Change(ctx context.Context, r *ChangeRequest) (*CollectResponse, e } } -func (a *API) WatchForChange(ctx echo.Context, orderRef string) (*sse.Sender, <-chan WatchResponse, error) { - watch := make(chan WatchResponse) - sender, err := sse.NewSender(ctx.Response()) - if err != nil { - fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) - return nil, nil, err +func (a *API) WatchForChange(ctx context.Context, orderRef string) (<-chan WatchChanges, error) { + collectRequest := &CollectRequest{OrderRef: orderRef} + if err := collectRequest.Validate(); err != nil { + return nil, err } - sender.Prepare() - - lastState, err := a.Collect(ctx.Request().Context(), &CollectRequest{OrderRef: orderRef}) + currentState, err := a.Collect(ctx, collectRequest) if err != nil { - return nil, nil, err - } - - var statusEvent struct { - Status Status - Hint HintCode + return nil, err } - updateStatus := func() { - statusEvent.Status = lastState.Status - statusEvent.Hint = lastState.HintCode - sender.Send("status", statusEvent) + watch := make(chan WatchChanges, 1) // Make it a buffered channel so that we can post initial state before we return + sendChange := func(change WatchChanges) { + select { + case watch <- change: + case <-time.After(time.Second): + } + } + updateStatus := func(state *CollectResponse) { + sendChange(WatchChanges{ + Cancelled: state.Status == Failed, + Status: state.Status, + Hint: state.HintCode, + }) } - updateStatus() - - go func() { - changeRequest := &ChangeRequest{ - OrderRef: orderRef, - WaitUntilFinished: false, - } + go func(lastState *CollectResponse) { + defer close(watch) for { - check, err := a.Change(ctx.Request().Context(), changeRequest) + select { + case <-ctx.Done(): + err = ctx.Err() + sendChange(WatchChanges{Cancelled: true, Err: ctx.Err()}) + return + case <-time.After(time.Second): + } + + resp, err := a.Collect(ctx, collectRequest) if err != nil { - if !errors.Is(err, context.Canceled) { - fmt.Println("ERR from change in bankid v6 auth/sign init: ", err) - watch <- WatchResponse{ - Cancelled: true, - Status: "", - } - close(watch) - return - } - - watch <- WatchResponse{ - Cancelled: true, - Status: err.Error(), - } - close(watch) + sendChange(WatchChanges{Cancelled: true, Err: err}) return } - if check.Status != lastState.Status || check.HintCode != lastState.HintCode { - lastState = check - updateStatus() + if resp.Status != lastState.Status || resp.HintCode != lastState.HintCode { + updateStatus(resp) + lastState = resp } - if check.Status != Pending { - watch <- WatchResponse{ - Cancelled: check.Status == Failed, - Status: string(check.Status), - } - close(watch) + if resp.Status != Pending { return } } - }() + }(currentState) - return sender, watch, nil + return watch, nil } func (a *API) Cancel(ctx context.Context, r *CancelRequest) error { diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index d396984..080b228 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -3,6 +3,7 @@ package httpserve import ( "encoding/json" "fmt" + "github.com/modfin/twofer/internal/sse" "io" "net/http" "time" @@ -11,8 +12,24 @@ import ( "github.com/modfin/twofer/internal/bankid" ) +type ( + statusUpdate struct { + Status bankid.Status + Hint bankid.HintCode + } +) + func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { - e.POST("/bankid/v6/auth", func(c echo.Context) error { + e.POST("/bankid/v6/auth", auth(client)) + e.POST("/bankid/v6/sign", sign(client)) + e.POST("/bankid/v6/change", change(client)) + e.POST("/bankid/v6/collect", collect(client)) + e.POST("/bankid/v6/cancel", cancel(client)) +} + +func auth(client *bankid.API) func(c echo.Context) error { + // TODO: Refactor auth and sign into single function that can handle both since the code if pretty much identical? + return func(c echo.Context) error { b, err := io.ReadAll(c.Request().Body) if err != nil { return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) @@ -38,30 +55,47 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { return c.JSON(200, res.APIResponse(0)) } - sender, interrupt, err := client.WatchForChange(c, res.OrderRef) + sender, err := sse.NewSender(c.Response()) if err != nil { - return c.JSON(500, "failed to setup response stream") + fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) + return err } - for i := 0; i < 30; i++ { - err = sender.Send("message", res.APIResponse(i)) // TODO: Should we change "message" to something else? - if err != nil { - fmt.Printf("ERR: failed to send auth response message: %s\n", err.Error()) - return c.JSON(500, "failed to send response message") - } + sender.Prepare() - // Optimally subtract time that has elapsed, but no need to be that exact + changes, err := client.WatchForChange(c.Request().Context(), res.OrderRef) + if err != nil { + return c.JSON(500, "failed to setup response stream") + } + + updateQR := time.NewTicker(time.Second) + qrCount := 1 + for { select { - case <-interrupt: - return c.JSON(http.StatusOK, bankid.Empty{}) - case <-time.After(time.Second): + case <-updateQR.C: + err = sender.Send("message", res.APIResponse(qrCount)) // TODO: Should we change "message" to something else? + if err != nil { + fmt.Printf("ERR: failed to send auth response message: %v\n", err) + return c.JSON(500, "failed to send response message") + } + case state, ok := <-changes: + if !ok { + return c.JSON(http.StatusOK, bankid.Empty{}) + } + // TODO: Stop updateQR timer when QR-code have been scanned + err = sender.Send("status", statusUpdate{Status: state.Status, Hint: state.Hint}) + if err != nil { + fmt.Printf("ERR: failed to send status update: %v\n", err) + return c.JSON(500, "failed to send response message") + } } } + } +} - return c.JSON(http.StatusOK, bankid.Empty{}) - }) - - e.POST("/bankid/v6/sign", func(c echo.Context) error { +func sign(client *bankid.API) func(c echo.Context) error { + // TODO: Refactor auth and sign into single function that can handle both since the code if pretty much identical? + return func(c echo.Context) error { b, err := io.ReadAll(c.Request().Body) if err != nil { return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) @@ -87,29 +121,46 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { return c.JSON(200, res.APIResponse(0)) } - sender, interrupt, err := client.WatchForChange(c, res.OrderRef) + sender, err := sse.NewSender(c.Response()) if err != nil { - return c.JSON(500, "failed to setup response stream") + fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) + return err } - for i := 0; i < 30; i++ { - err = sender.Send("message", res.APIResponse(i)) // TODO: Should we change "message" to something else? - if err != nil { - fmt.Printf("ERR: failed to send sign response message: %s\n", err.Error()) - return c.JSON(500, "failed to send response message") - } + sender.Prepare() + + changes, err := client.WatchForChange(c.Request().Context(), res.OrderRef) + if err != nil { + return c.JSON(500, "failed to setup response stream") + } - // Optimally subtract time that has elapsed, but no need to be that exact + updateQR := time.NewTicker(time.Second) + qrCount := 1 + for { select { - case <-interrupt: - return c.JSON(http.StatusOK, bankid.Empty{}) - case <-time.After(time.Second): + case <-updateQR.C: + err = sender.Send("message", res.APIResponse(qrCount)) // TODO: Should we change "message" to something else? + if err != nil { + fmt.Printf("ERR: failed to send sign response message: %v\n", err) + return c.JSON(500, "failed to send response message") + } + case state, ok := <-changes: + if !ok { + return c.JSON(http.StatusOK, bankid.Empty{}) + } + // TODO: Stop updateQR timer when QR-code have been scanned + err = sender.Send("status", statusUpdate{Status: state.Status, Hint: state.Hint}) + if err != nil { + fmt.Printf("ERR: failed to send status update: %v\n", err) + return c.JSON(500, "failed to send response message") + } } } - return c.JSON(http.StatusOK, bankid.Empty{}) - }) + } +} - e.POST("/bankid/v6/change", func(c echo.Context) error { +func change(client *bankid.API) func(c echo.Context) error { + return func(c echo.Context) error { b, err := io.ReadAll(c.Request().Body) if err != nil { fmt.Printf("ERR: failed to read change request message: %s\n", err.Error()) @@ -130,9 +181,11 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { } return c.JSON(http.StatusOK, res) - }) + } +} - e.POST("/bankid/v6/collect", func(c echo.Context) error { +func collect(client *bankid.API) func(c echo.Context) error { + return func(c echo.Context) error { b, err := io.ReadAll(c.Request().Body) if err != nil { fmt.Printf("ERR: failed to read collect request message: %s\n", err.Error()) @@ -153,9 +206,11 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { } return c.JSON(http.StatusOK, res) - }) + } +} - e.POST("/bankid/v6/cancel", func(c echo.Context) error { +func cancel(client *bankid.API) func(c echo.Context) error { + return func(c echo.Context) error { b, err := io.ReadAll(c.Request().Body) if err != nil { fmt.Printf("ERR: failed to read cancel request message: %s\n", err.Error()) @@ -176,5 +231,5 @@ func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { } return c.NoContent(http.StatusNoContent) - }) + } } From 0967ef2a6b484badf24cf1cbb67a8e5a69834c88 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Thu, 11 Apr 2024 08:41:16 +0200 Subject: [PATCH 10/28] WIP: Start on a 'public' sse package that can be used by a client --- sse/event.go | 8 +++ sse/reader.go | 106 +++++++++++++++++++++++++++++++++ sse/writer.go | 97 ++++++++++++++++++++++++++++++ sse/writer_test.go | 144 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 355 insertions(+) create mode 100644 sse/event.go create mode 100644 sse/reader.go create mode 100644 sse/writer.go create mode 100644 sse/writer_test.go diff --git a/sse/event.go b/sse/event.go new file mode 100644 index 0000000..3e735ae --- /dev/null +++ b/sse/event.go @@ -0,0 +1,8 @@ +package sse + +type Event struct { + Event string + Data string + ID string + Retry string +} diff --git a/sse/reader.go b/sse/reader.go new file mode 100644 index 0000000..3b36cf8 --- /dev/null +++ b/sse/reader.go @@ -0,0 +1,106 @@ +package sse + +import ( + "bufio" + "bytes" + "context" + "fmt" + "io" + "strings" +) + +type reader struct { + // buf *bufio.Scanner +} + +func NewReader(ctx context.Context, rc io.ReadCloser) <-chan Event { + eventChan := make(chan Event) + //r := reader{ + // buf: bufio.NewScanner(rc), + //} + go process(ctx, rc, eventChan) + return eventChan +} + +func scanLines(data []byte, atEOF bool) (advance int, token []byte, err error) { + if atEOF && len(data) == 0 { + return 0, nil, nil + } + + // Handle messages that use 'CRLF' or 'LF' line endings. + advance, token, err = bufio.ScanLines(data, atEOF) + if advance != 0 || len(data) == 0 { + return + } + + if i := bytes.IndexByte(data, '\r'); i >= 0 { + // We have a full CR-terminated line. + return i + 1, data[0:i], nil + } + // If we're at EOF, we have a final, non-terminated line. Return it. + if atEOF { + return len(data), data, nil + } + // Request more data. + return 0, nil, nil +} + +func process(ctx context.Context, rc io.ReadCloser, ec chan<- Event) { + defer rc.Close() + defer close(ec) + buf := bufio.NewScanner(rc) + buf.Split(scanLines) + var event Event + for { + select { + case <-ctx.Done(): + return + default: + // Continue to read data + } + fieldName, fieldData, eom, err := readField(buf) + if err != nil { + return + } + + if eom { + if event.Event == "" { + event.Event = "message" + } + ec <- event + event = Event{} + continue + } + + switch fieldName { + case "event": + event.Event += fieldData + case "data": + event.Data += fieldData + case "id": + event.ID += fieldData + case "retry": + event.Retry += fieldData + default: + fmt.Printf("ignored unknoen field %s: %s\n", fieldName, fieldData) + } + } +} + +func readField(s *bufio.Scanner) (string, string, bool, error) { + if !s.Scan() { + return "", "", false, s.Err() + } + + line := s.Text() + if line == "" { + return "", "", true, nil // End of event + } + + colonPos := strings.Index(line, ":") + if colonPos <= 1 { + return "", "", false, nil // Not a field, ignore + } + + return line[0:colonPos], strings.TrimSpace(line[colonPos+1:]), false, nil +} diff --git a/sse/writer.go b/sse/writer.go new file mode 100644 index 0000000..3654f91 --- /dev/null +++ b/sse/writer.go @@ -0,0 +1,97 @@ +package sse + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "net/http" + "strings" +) + +type Writer struct { + http.ResponseWriter + flush func() +} + +var ( + ErrInvalidWriter = errors.New("invalid writer") + ErrWrite = errors.New("write error") +) + +func NewWriter(w http.ResponseWriter) (*Writer, error) { + f, ok := w.(http.Flusher) + if !ok { + return nil, fmt.Errorf("%w: the provided writer don't support the http.Flusher interface", ErrInvalidWriter) + } + w.Header().Set("Content-Type", "text/event-stream") + w.Header().Set("Cache-Control", "no-cache") + w.Header().Set("Connection", "keep-alive") + w.Header().Set("Transfer-Encoding", "chunked") + return &Writer{ + ResponseWriter: w, + flush: f.Flush, + }, nil +} + +func writeField(buf *bytes.Buffer, field, data string) { + // Ignore empty and default values + if data == "" || (field == "event" && data == "message") { + return + } + + // If data has line breaks, the `field` must be prefixed to each line + hasLineBreaks := strings.ContainsAny(data, "\n\r") + if !hasLineBreaks { + buf.WriteString(field) + buf.WriteString(":") + buf.WriteString(data) + buf.WriteString("\n") + return + } + + // Split data and add each line as a separate field + start, dl := 0, len(data) + for i, r := range data { + if r == 13 || r == 10 { + writeField(buf, field, data[start:i]) + start = i + 1 + } + if i+1 == dl { + writeField(buf, field, data[start:i+1]) + } + } +} + +func (w *Writer) SendEvent(event, data, id, retry string) error { + //var buf strings.Builder + var buf bytes.Buffer + writeField(&buf, "event", event) + writeField(&buf, "data", data) + writeField(&buf, "id", id) + writeField(&buf, "retry", retry) + + if buf.Len() == 0 { + return nil + } + + buf.WriteString("\n") + bw, err := w.Write(buf.Bytes()) + if err != nil { + return err + } + if bw != buf.Len() { + return fmt.Errorf("%w: wrote %d bytes, expected %d", ErrWrite, bw, buf.Len()) + } + + w.flush() + return nil +} + +func (w *Writer) SendJSON(event string, data any) error { + b, err := json.Marshal(data) + if err != nil { + return err + } + return w.SendEvent(event, string(b), "", "") +} diff --git a/sse/writer_test.go b/sse/writer_test.go new file mode 100644 index 0000000..7ace087 --- /dev/null +++ b/sse/writer_test.go @@ -0,0 +1,144 @@ +package sse + +import ( + "bytes" + "net/http" + "testing" +) + +type ( + testWriter struct { + flushed bool + header http.Header + status int + written []byte + } +) + +func (w *testWriter) Flush() { + w.flushed = true +} + +func (w *testWriter) Header() http.Header { + if w.header == nil { + w.header = make(http.Header) + } + return w.header +} + +func (w *testWriter) Write(data []byte) (int, error) { + w.written = append(w.written, data...) + return len(data), nil +} + +func (w *testWriter) WriteHeader(statusCode int) { + w.status = statusCode +} + +func Test_writeField(t *testing.T) { + tests := []struct { + name string + field string + data string + want string + }{ + { + name: "vanilla", + field: "data", + data: "vanilla", + want: "data:vanilla\n", + }, + { + name: "multi-line", + field: "data", + data: "multi\nline", + want: "data:multi\ndata:line\n", + }, + { + name: "empty_data", + field: "data", + data: "", + want: "", + }, + { + name: "default_event_type", + field: "event", + data: "message", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + writeField(&buf, tt.field, tt.data) + got := string(buf.Bytes()) + if got != tt.want { + t.Errorf("got: '%s', want: '%s", got, tt.want) + } + }) + } +} + +func TestWriter_SendEvent(t *testing.T) { + tests := []struct { + name string + event string + data string + id string + retry string + wantErr bool + wantFlush bool + wantData string + }{ + { + name: "all fields", + event: "e1", + data: "d1", + id: "1", + retry: "1", + wantFlush: true, + wantData: "event:e1\ndata:d1\nid:1\nretry:1\n\n", + }, + { + name: "multiline_data", + data: "multi\nline\ndata", + wantFlush: true, + wantData: "data:multi\ndata:line\ndata:data\n\n", + }, + { + name: "only_default_event_type", // This should not send anything + event: "message", + wantFlush: false, + wantData: "", + }, + { + name: "only_non_default_event_type", + event: "ping", + wantFlush: true, + wantData: "event:ping\n\n", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tw := &testWriter{} + + w, err := NewWriter(tw) + if err != nil { + t.Fatalf("Failed to create new writer: %v", err) + } + + err = w.SendEvent(tt.event, tt.data, tt.id, tt.retry) + if (err != nil) != tt.wantErr { + t.Fatalf("SendEvent() error = %v, wantErr %v", err, tt.wantErr) + } + + if tw.flushed != tt.wantFlush { + t.Errorf("got flush: %v, want: %v", tw.flushed, tt.wantFlush) + } + + if string(tw.written) != tt.wantData { + t.Errorf("got data: '%s', want: '%s'", tw.written, tt.wantData) + } + }) + } +} From 52631f79f1ed6bfa725867a262adcb0fff542d9b Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Thu, 11 Apr 2024 19:06:30 +0200 Subject: [PATCH 11/28] cleanup public sse package --- sse/reader.go | 71 +++++++---- sse/reader_test.go | 288 +++++++++++++++++++++++++++++++++++++++++++++ sse/writer.go | 1 - 3 files changed, 333 insertions(+), 27 deletions(-) create mode 100644 sse/reader_test.go diff --git a/sse/reader.go b/sse/reader.go index 3b36cf8..aa1a33b 100644 --- a/sse/reader.go +++ b/sse/reader.go @@ -9,15 +9,8 @@ import ( "strings" ) -type reader struct { - // buf *bufio.Scanner -} - func NewReader(ctx context.Context, rc io.ReadCloser) <-chan Event { eventChan := make(chan Event) - //r := reader{ - // buf: bufio.NewScanner(rc), - //} go process(ctx, rc, eventChan) return eventChan } @@ -29,14 +22,24 @@ func scanLines(data []byte, atEOF bool) (advance int, token []byte, err error) { // Handle messages that use 'CRLF' or 'LF' line endings. advance, token, err = bufio.ScanLines(data, atEOF) + if advance > 0 && len(token) > 0 { + // Check if returned token contain CR + if i := bytes.IndexByte(token, '\r'); i >= 0 { + // We have a full CR-terminated line. + return i + 1, token[0:i], nil + } + } if advance != 0 || len(data) == 0 { return } + // Check if data contain CR + // (this can potentially break things if CRLF is used but `data` ends with the CR because we haven't read further from the reader yet) if i := bytes.IndexByte(data, '\r'); i >= 0 { // We have a full CR-terminated line. return i + 1, data[0:i], nil } + // If we're at EOF, we have a final, non-terminated line. Return it. if atEOF { return len(data), data, nil @@ -45,6 +48,27 @@ func scanLines(data []byte, atEOF bool) (advance int, token []byte, err error) { return 0, nil, nil } +func readField(s *bufio.Scanner) (string, string, bool, error) { + if !s.Scan() { + if s.Err() == nil { + return "", "", false, io.EOF + } + return "", "", false, s.Err() + } + + line := s.Text() + if line == "" { + return "", "", true, nil // End of event + } + + colonPos := strings.Index(line, ":") + if colonPos <= 1 || (len(line) >= 1 && line[0] == '#') { + return "", "", false, nil // Not a field, ignore + } + + return line[0:colonPos], strings.TrimSpace(line[colonPos+1:]), false, nil +} + func process(ctx context.Context, rc io.ReadCloser, ec chan<- Event) { defer rc.Close() defer close(ec) @@ -60,6 +84,7 @@ func process(ctx context.Context, rc io.ReadCloser, ec chan<- Event) { } fieldName, fieldData, eom, err := readField(buf) if err != nil { + fmt.Printf("failed to read field, error: %v\n", err) return } @@ -74,33 +99,27 @@ func process(ctx context.Context, rc io.ReadCloser, ec chan<- Event) { switch fieldName { case "event": + if event.Event != "" { + event.Event += "\n" + } event.Event += fieldData case "data": + if event.Data != "" { + event.Data += "\n" + } event.Data += fieldData case "id": + if event.ID != "" { + event.ID += "\n" + } event.ID += fieldData case "retry": + if event.Retry != "" { + event.Retry += "\n" + } event.Retry += fieldData default: - fmt.Printf("ignored unknoen field %s: %s\n", fieldName, fieldData) + fmt.Printf("ignored unknown field %s: %s\n", fieldName, fieldData) } } } - -func readField(s *bufio.Scanner) (string, string, bool, error) { - if !s.Scan() { - return "", "", false, s.Err() - } - - line := s.Text() - if line == "" { - return "", "", true, nil // End of event - } - - colonPos := strings.Index(line, ":") - if colonPos <= 1 { - return "", "", false, nil // Not a field, ignore - } - - return line[0:colonPos], strings.TrimSpace(line[colonPos+1:]), false, nil -} diff --git a/sse/reader_test.go b/sse/reader_test.go new file mode 100644 index 0000000..e904687 --- /dev/null +++ b/sse/reader_test.go @@ -0,0 +1,288 @@ +package sse + +import ( + "bufio" + "context" + "io" + "strings" + "testing" + "time" +) + +func Test_scanLines(t *testing.T) { + tests := []struct { + name string + data []byte + eof bool + wantAdvance int + wantToken []byte + wantErr bool + }{ + { + name: "lf_line", + data: []byte("token 1\ntoken 2\n"), + wantAdvance: 8, + wantToken: []byte("token 1"), + }, + { + name: "cr_line", + data: []byte("token 2\rtoken 3\r"), + wantAdvance: 8, + wantToken: []byte("token 2"), + }, + { + name: "crlf_line", + data: []byte("token 3\r\ntoken 4\r\n"), + wantAdvance: 9, + wantToken: []byte("token 3"), + }, + { + name: "mixed_line_line_endings_1", + data: []byte("token 4\ntoken 5\r\ntoken 6\r"), + wantAdvance: 8, + wantToken: []byte("token 4"), + }, + { + name: "mixed_line_line_endings_2", + data: []byte("token 5\r\ntoken 6\rtoken 7\n"), + wantAdvance: 9, + wantToken: []byte("token 5"), + }, + { + name: "mixed_line_line_endings_3", + data: []byte("token 6\rtoken 7\ntoken 8\r\n"), + wantAdvance: 8, + wantToken: []byte("token 6"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotAdvance, gotToken, err := scanLines(tt.data, tt.eof) + if (err != nil) != tt.wantErr { + t.Errorf("got error: %v, want: %v", err, tt.wantErr) + return + } + if gotAdvance != tt.wantAdvance { + t.Errorf("got advance: %v, want: %v", gotAdvance, tt.wantAdvance) + } + if string(gotToken) != string(tt.wantToken) { + t.Errorf("got token: %s, want %s", gotToken, tt.wantToken) + } + }) + } +} + +func Test_readField(t *testing.T) { + tests := []struct { + name string + buffer string + wantType string + wantData string + wantEOM bool + wantErr error + }{ + { + name: "event", + buffer: "event: test\n", + wantType: "event", + wantData: "test", + wantEOM: false, + }, + { + name: "comment", + buffer: "# comment\n", + wantType: "", + wantData: "", + wantEOM: false, + }, + { + name: "empty_line", + buffer: "\n", + wantType: "", + wantData: "", + wantEOM: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := bufio.NewScanner(strings.NewReader(tt.buffer)) + buf.Split(scanLines) + gotType, gotData, gotEOM, gotErr := readField(buf) + if gotErr != tt.wantErr { + t.Fatalf("got error: %v, want: %v", gotErr, tt.wantErr) + } + if gotType != tt.wantType { + t.Errorf("got type: %s, want %s", gotType, tt.wantType) + } + if gotData != tt.wantData { + t.Errorf("got data: %s, want %s", gotData, tt.wantData) + } + if gotEOM != tt.wantEOM { + t.Errorf("got End-of-Message: %v, want %v", gotEOM, tt.wantEOM) + } + }) + } +} + +func Test_process(t *testing.T) { + type send struct { + buffer string + wantEvents []Event + } + tests := []struct { + name string + sends []send + }{ + { + name: "single event", + sends: []send{ + { + buffer: "event:1\n\n", + wantEvents: []Event{{Event: "1"}}, + }, + }, + }, + { + name: "event_properties", + sends: []send{ + { + buffer: "event:2\n\ndata:2\n\nid:2\n\nretry:2\n\n", + wantEvents: []Event{{Event: "2"}, {Event: "message", Data: "2"}, {Event: "message", ID: "2"}, {Event: "message", Retry: "2"}}, + }, + }, + }, + { + name: "multi_send", + sends: []send{ + { + buffer: "event:31\n\n", + wantEvents: []Event{{Event: "31"}}, + }, + { + buffer: "event:32\n\ndata:32\n\n", + wantEvents: []Event{{Event: "32"}, {Event: "message", Data: "32"}}, + }, + { + buffer: "event:33\n\ndata:33\n\nid:33\n\n", + wantEvents: []Event{{Event: "33"}, {Event: "message", Data: "33"}, {Event: "message", ID: "33"}}, + }, + { + buffer: "event:34\n\ndata:34\n\nid:34\n\nretry:34\n\n", + wantEvents: []Event{{Event: "34"}, {Event: "message", Data: "34"}, {Event: "message", ID: "34"}, {Event: "message", Retry: "34"}}, + }, + }, + }, + { + name: "cr_line_breaks", + sends: []send{ + { + buffer: "event:41\r\revent:42\r\r", + wantEvents: []Event{{Event: "41"}, {Event: "42"}}, + }, + }, + }, + { + name: "crlf_line_breaks", + sends: []send{ + { + buffer: "event:51\r\n\r\nevent:52\r\n\r\n", + wantEvents: []Event{{Event: "51"}, {Event: "52"}}, + }, + }, + }, + { + name: "incomplete_message", + sends: []send{ + { + buffer: "no-line-break", + wantEvents: []Event{}, + }, + }, + }, + { + name: "message_without_supported_lines", + sends: []send{ + { + buffer: "# Comment\nignored_ data\n\n", + wantEvents: []Event{{Event: "message"}}, + }, + }, + }, + { + name: "multi_line_message", + sends: []send{ + { + buffer: "event: first line\nevent: second line\n\n", + wantEvents: []Event{{Event: "first line\nsecond line"}}, + }, + { + buffer: "data: first line\ndata: second line\n\n", + wantEvents: []Event{{Event: "message", Data: "first line\nsecond line"}}, + }, + { + buffer: "id: first line\nid: second line\n\n", + wantEvents: []Event{{Event: "message", ID: "first line\nsecond line"}}, + }, + { + buffer: "retry: first line\nretry: second line\n\n", + wantEvents: []Event{{Event: "message", Retry: "first line\nsecond line"}}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pr, pw := io.Pipe() + eventChan := make(chan Event) + var terminated bool + go func() { + t.Log("start async process...") + process(context.Background(), pr, eventChan) + terminated = true + t.Log("async process have terminated...") + }() + for i, s := range tt.sends { + _, err := pw.Write([]byte(s.buffer)) + if err != nil { + t.Fatalf("failed to write buffer #%d, error: %v", i, err) + } + var gotEvents []Event + done: + for { + select { + case e := <-eventChan: + gotEvents = append(gotEvents, e) + t.Logf("got event: %#v", e) + case <-time.After(time.Millisecond * 50): + break done + } + } + + if len(gotEvents) != len(s.wantEvents) { + t.Errorf("got %d events, want %d\nreceived events: %#v", len(gotEvents), len(s.wantEvents), gotEvents) + } else { + for i, we := range s.wantEvents { + ge := gotEvents[i] + if ge != we { + t.Errorf("got event #%d: %#v, want: %#v", i, ge, we) + } + } + } + } + + // Close pipe writer to indicate that there won't be any more data to read for the async process + err := pw.Close() + if err != nil { + t.Fatalf("failed to close pipe, error: %v", err) + } + + // wait a short time so that the async process have time to shutdown after we've closed the reader + time.Sleep(time.Millisecond * 100) + + // Check that the async process ended so that it's go-routine can exit cleanly + if !terminated { + t.Errorf("the async process didn't terminate after reader had been closed") + } + }) + } +} diff --git a/sse/writer.go b/sse/writer.go index 3654f91..a893f29 100644 --- a/sse/writer.go +++ b/sse/writer.go @@ -64,7 +64,6 @@ func writeField(buf *bytes.Buffer, field, data string) { } func (w *Writer) SendEvent(event, data, id, retry string) error { - //var buf strings.Builder var buf bytes.Buffer writeField(&buf, "event", event) writeField(&buf, "data", data) From 4314f9e6cd8e0757aa6f6296525b90fb0fb8506a Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Thu, 11 Apr 2024 19:14:23 +0200 Subject: [PATCH 12/28] Simplify shutdown process --- cmd/twoferd/main.go | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/cmd/twoferd/main.go b/cmd/twoferd/main.go index 4bc1cdf..7f0887e 100644 --- a/cmd/twoferd/main.go +++ b/cmd/twoferd/main.go @@ -8,7 +8,6 @@ import ( "net/http" "os" "os/signal" - "sync" "syscall" "time" @@ -106,23 +105,17 @@ func startServer(e *echo.Echo) { select { case <-signalChannel: + appClose() // Cancel 'app context' when we receive SIGTERM case <-appCtx.Done(): } - wg := sync.WaitGroup{} - wg.Add(1) - go func() { - timeout, cancel := context.WithTimeout(context.Background(), time.Second*30) - defer cancel() - - err := e.Shutdown(timeout) - if err != nil { - log.Fatalf("failure during Echo's shutdown: %v", err) - } - wg.Done() - }() + timeout, cancel := context.WithTimeout(context.Background(), time.Second*30) + defer cancel() - wg.Wait() + err := e.Shutdown(timeout) + if err != nil { + log.Fatalf("failure during Echo's shutdown: %v", err) + } } func startEid(e *echo.Echo) { From 8a1314baf7609a3cc8e77bf4f40d390e10726aa8 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Sun, 14 Apr 2024 16:53:50 +0200 Subject: [PATCH 13/28] Revert breaking API changes from old endpoints and move the new API changes to new endpoints --- api/models.go | 34 +++ internal/bankid/bankid_models.go | 14 +- internal/bankid/endpoints.go | 78 ++++-- internal/httpserve/bankid.go | 388 ++++++++++++++++++++++-------- internal/httpserve/bankid_test.go | 212 ++++++++++++++++ internal/sse/sender.go | 35 +-- sse/reader.go | 4 + test/bankidv6_test.go | 141 ++++------- 8 files changed, 665 insertions(+), 241 deletions(-) create mode 100644 api/models.go create mode 100644 internal/httpserve/bankid_test.go diff --git a/api/models.go b/api/models.go new file mode 100644 index 0000000..0875b99 --- /dev/null +++ b/api/models.go @@ -0,0 +1,34 @@ +package api + +type ( + BankIdV6Response struct { + OrderRef string `json:"orderRef"` + CollectError string `json:"error,omitempty"` + URI string `json:"uri,omitempty"` + QR string `json:"qr,omitempty"` + Status string `json:"status,omitempty"` + HintCode string `json:"hintCode,omitempty"` + CompletionData *BankIdV6CompletionData `json:"completionData,omitempty"` + } + BankIdV6CompletionData struct { + User BankIdV6User `json:"user,omitempty"` + Device BankIdV6Device `json:"device,omitempty"` + BankIdIssueDate string `json:"bankIdIssueDate,omitempty"` + StepUp BankIdV6StepUp `json:"stepUp,omitempty"` + Signature string `json:"signature,omitempty"` + OcspResponse string `json:"ocspResponse,omitempty"` + } + BankIdV6User struct { + PersonalNumber string `json:"personalNumber,omitempty"` + Name string `json:"name,omitempty"` + GivenName string `json:"givenName,omitempty"` + SurName string `json:"surName,omitempty"` + } + BankIdV6Device struct { + IpAddress string `json:"ipAddress,omitempty"` + UHI string `json:"uhi,omitempty"` + } + BankIdV6StepUp struct { + MRTD bool `json:"mrtd,omitempty"` + } +) diff --git a/internal/bankid/bankid_models.go b/internal/bankid/bankid_models.go index 46aac18..7f98bbc 100644 --- a/internal/bankid/bankid_models.go +++ b/internal/bankid/bankid_models.go @@ -80,14 +80,6 @@ type AuthSignResponse struct { QrStartSecret string `json:"qrStartSecret"` } -func (r *AuthSignResponse) APIResponse(qrCodeTime int) AuthSignAPIResponse { - return AuthSignAPIResponse{ - OrderRef: r.OrderRef, - URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", r.AutoStartToken), - QR: r.BuildQrCode(qrCodeTime), - } -} - // BuildQrCode builds a BankID compatible QR code // See https://www.bankid.com/utvecklare/guider/teknisk-integrationsguide/qrkoder func (r *AuthSignResponse) BuildQrCode(time int) string { @@ -175,11 +167,9 @@ type StepUp struct { MRTD bool `json:"mrtd"` } -type WatchChanges struct { +type WatchResponse struct { Cancelled bool - Status Status - Hint HintCode - Err error + Status string } type CancelRequest struct { diff --git a/internal/bankid/endpoints.go b/internal/bankid/endpoints.go index e1dc2a1..87ad22b 100644 --- a/internal/bankid/endpoints.go +++ b/internal/bankid/endpoints.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "encoding/json" + "errors" + "fmt" "io" "net/http" "time" @@ -111,52 +113,94 @@ func (a *API) Change(ctx context.Context, r *ChangeRequest) (*CollectResponse, e } } -func (a *API) WatchForChange(ctx context.Context, orderRef string) (<-chan WatchChanges, error) { +func (a *API) WatchForChange(ctx context.Context, orderRef string) <-chan WatchResponse { + watch := make(chan WatchResponse) + + go func() { + changeRequest := &ChangeRequest{ + OrderRef: orderRef, + WaitUntilFinished: false, + } + + check, err := a.Change(ctx, changeRequest) + if err != nil { + if !errors.Is(err, context.Canceled) { + fmt.Println("ERR from change in bankid v6 auth/sign init: ", err) + watch <- WatchResponse{ + Cancelled: true, + Status: "", + } + close(watch) + return + } + + watch <- WatchResponse{ + Cancelled: true, + Status: err.Error(), + } + close(watch) + return + } + + watch <- WatchResponse{ + Cancelled: false, + Status: string(check.Status), + } + close(watch) + return + }() + + return watch +} + +type Change struct { + CollectResponse + Err error +} + +func (a *API) WatchForChangeV2(ctx context.Context, orderRef string) (<-chan Change, error) { collectRequest := &CollectRequest{OrderRef: orderRef} - if err := collectRequest.Validate(); err != nil { - return nil, err - } currentState, err := a.Collect(ctx, collectRequest) if err != nil { return nil, err } - watch := make(chan WatchChanges, 1) // Make it a buffered channel so that we can post initial state before we return + watch := make(chan Change, 1) // Make it a buffered channel so that we can post initial state before we return - sendChange := func(change WatchChanges) { + sendError := func(err error) { select { - case watch <- change: + case watch <- Change{Err: err}: case <-time.After(time.Second): } } - updateStatus := func(state *CollectResponse) { - sendChange(WatchChanges{ - Cancelled: state.Status == Failed, - Status: state.Status, - Hint: state.HintCode, - }) + sendChange := func(change CollectResponse) { + select { + case watch <- Change{CollectResponse: change}: + case <-time.After(time.Second): + } } + sendChange(*currentState) + go func(lastState *CollectResponse) { defer close(watch) for { select { case <-ctx.Done(): - err = ctx.Err() - sendChange(WatchChanges{Cancelled: true, Err: ctx.Err()}) + sendError(ctx.Err()) return case <-time.After(time.Second): } resp, err := a.Collect(ctx, collectRequest) if err != nil { - sendChange(WatchChanges{Cancelled: true, Err: err}) + sendError(err) return } if resp.Status != lastState.Status || resp.HintCode != lastState.HintCode { - updateStatus(resp) + sendChange(*resp) lastState = resp } diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index 080b228..5f0d1e4 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -1,235 +1,423 @@ package httpserve import ( + "context" "encoding/json" "fmt" - "github.com/modfin/twofer/internal/sse" "io" "net/http" + "strconv" "time" + "github.com/modfin/twofer/api" + "github.com/modfin/twofer/internal/sse" + sse2 "github.com/modfin/twofer/sse" + "github.com/labstack/echo/v4" "github.com/modfin/twofer/internal/bankid" ) -type ( - statusUpdate struct { - Status bankid.Status - Hint bankid.HintCode - } -) +const qrCodeUpdatePeriod = time.Second func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { e.POST("/bankid/v6/auth", auth(client)) + e.POST("/bankid/v6/authv2", authSign(client.Auth, client.WatchForChangeV2, qrCodeUpdatePeriod)) e.POST("/bankid/v6/sign", sign(client)) + e.POST("/bankid/v6/signv2", authSign(client.Sign, client.WatchForChangeV2, qrCodeUpdatePeriod)) e.POST("/bankid/v6/change", change(client)) e.POST("/bankid/v6/collect", collect(client)) e.POST("/bankid/v6/cancel", cancel(client)) } -func auth(client *bankid.API) func(c echo.Context) error { +func auth(client *bankid.API) func(echo.Context) error { // TODO: Refactor auth and sign into single function that can handle both since the code if pretty much identical? - return func(c echo.Context) error { - b, err := io.ReadAll(c.Request().Body) + return func(e echo.Context) error { + b, err := io.ReadAll(e.Request().Body) if err != nil { - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) } var request bankid.AuthSignRequest err = json.Unmarshal(b, &request) if err != nil { fmt.Printf("ERR: failed to unmarshal auth request message: %s\n", err.Error()) - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) } - res, err := client.Auth(c.Request().Context(), &request) + res, err := client.Auth(e.Request().Context(), &request) if err != nil { fmt.Printf("ERR: initiating auth request against bankid: %s\n", err.Error()) - return c.JSON(500, "failed to initiate auth against BankId") + return e.JSON(500, "failed to initiate auth against BankId") } // In the case a client wants to initiate a new request every second instead of relying on SSE // we respond with the first entry and then close the connection - response := c.QueryParam("type") + response := e.QueryParam("type") if response == "once" { - return c.JSON(200, res.APIResponse(0)) + msg := bankid.AuthSignAPIResponse{ + OrderRef: res.OrderRef, + URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", res.AutoStartToken), + QR: res.BuildQrCode(0), + } + + return e.JSON(200, msg) } - sender, err := sse.NewSender(c.Response()) + sender, err := sse.NewSender(e.Response()) if err != nil { fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) - return err + return e.JSON(500, "failed to setup response stream") } sender.Prepare() - changes, err := client.WatchForChange(c.Request().Context(), res.OrderRef) - if err != nil { - return c.JSON(500, "failed to setup response stream") - } + interrupt := client.WatchForChange(e.Request().Context(), res.OrderRef) - updateQR := time.NewTicker(time.Second) - qrCount := 1 - for { + for i := 0; i < 30; i++ { select { - case <-updateQR.C: - err = sender.Send("message", res.APIResponse(qrCount)) // TODO: Should we change "message" to something else? - if err != nil { - fmt.Printf("ERR: failed to send auth response message: %v\n", err) - return c.JSON(500, "failed to send response message") - } - case state, ok := <-changes: - if !ok { - return c.JSON(http.StatusOK, bankid.Empty{}) - } - // TODO: Stop updateQR timer when QR-code have been scanned - err = sender.Send("status", statusUpdate{Status: state.Status, Hint: state.Hint}) - if err != nil { - fmt.Printf("ERR: failed to send status update: %v\n", err) - return c.JSON(500, "failed to send response message") - } + case <-interrupt: + return e.JSON(http.StatusOK, bankid.Empty{}) + default: + break + } + + msg := bankid.AuthSignAPIResponse{ + OrderRef: res.OrderRef, + URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", res.AutoStartToken), + QR: res.BuildQrCode(i), } + + var bytes []byte + bytes, err = json.Marshal(msg) + if err != nil { + fmt.Printf("ERR: failed to build auth response message: %s\n", err.Error()) + return e.JSON(500, "failed to build response message") + } + + event := sse.Event{ + Id: strconv.Itoa(i), + Event: "message", + Data: string(bytes), + } + + err = sender.Send(event) + if err != nil { + fmt.Printf("ERR: failed to send auth response message: %s\n", err.Error()) + return e.JSON(500, "failed to send response message") + } + + // Optimally subtract time that has elapsed, but no need to be that exact + time.Sleep(time.Second) } + + return e.JSON(http.StatusOK, bankid.Empty{}) } } -func sign(client *bankid.API) func(c echo.Context) error { +func sign(client *bankid.API) func(echo.Context) error { // TODO: Refactor auth and sign into single function that can handle both since the code if pretty much identical? - return func(c echo.Context) error { - b, err := io.ReadAll(c.Request().Body) + return func(e echo.Context) error { + b, err := io.ReadAll(e.Request().Body) if err != nil { - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) } var request bankid.AuthSignRequest err = json.Unmarshal(b, &request) if err != nil { fmt.Printf("ERR: failed to unmarshal sign request message: %s\n", err.Error()) - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) } - res, err := client.Sign(c.Request().Context(), &request) + res, err := client.Sign(e.Request().Context(), &request) if err != nil { fmt.Printf("ERR: initiating sign request against bankid: %s\n", err.Error()) - return c.JSON(500, "failed to initiate sign against BankId") + return e.JSON(500, "failed to initiate sign against BankId") } // In the case a client wants to initiate a new request every second instead of relying on SSE // we respond with the first entry and then close the connection - response := c.QueryParam("type") + response := e.QueryParam("type") if response == "once" { - return c.JSON(200, res.APIResponse(0)) + msg := bankid.AuthSignAPIResponse{ + OrderRef: res.OrderRef, + URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", res.AutoStartToken), + QR: res.BuildQrCode(0), + } + + return e.JSON(200, msg) } - sender, err := sse.NewSender(c.Response()) + sender, err := sse.NewSender(e.Response()) if err != nil { - fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) - return err + fmt.Printf("ERR: failed to setup sign response stream: %s\n", err.Error()) + return e.JSON(500, "failed to setup response stream") } sender.Prepare() - changes, err := client.WatchForChange(c.Request().Context(), res.OrderRef) - if err != nil { - return c.JSON(500, "failed to setup response stream") - } + interrupt := client.WatchForChange(e.Request().Context(), res.OrderRef) - updateQR := time.NewTicker(time.Second) - qrCount := 1 - for { + for i := 0; i < 30; i++ { select { - case <-updateQR.C: - err = sender.Send("message", res.APIResponse(qrCount)) // TODO: Should we change "message" to something else? - if err != nil { - fmt.Printf("ERR: failed to send sign response message: %v\n", err) - return c.JSON(500, "failed to send response message") - } - case state, ok := <-changes: - if !ok { - return c.JSON(http.StatusOK, bankid.Empty{}) - } - // TODO: Stop updateQR timer when QR-code have been scanned - err = sender.Send("status", statusUpdate{Status: state.Status, Hint: state.Hint}) - if err != nil { - fmt.Printf("ERR: failed to send status update: %v\n", err) - return c.JSON(500, "failed to send response message") - } + case <-interrupt: + return e.JSON(http.StatusOK, bankid.Empty{}) + default: + break + } + + msg := bankid.AuthSignAPIResponse{ + OrderRef: res.OrderRef, + URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", res.AutoStartToken), + QR: res.BuildQrCode(i), + } + + var bytes []byte + bytes, err = json.Marshal(msg) + if err != nil { + fmt.Printf("ERR: failed to build sign response message: %s\n", err.Error()) + return e.JSON(500, "failed to build response message") } + + event := sse.Event{ + Id: strconv.Itoa(i), + Event: "message", + Data: string(bytes), + } + + err = sender.Send(event) + if err != nil { + fmt.Printf("ERR: failed to send sign response message: %s\n", err.Error()) + return e.JSON(500, "failed to send response message") + } + + // Optimally subtract time that has elapsed, but no need to be that exact + time.Sleep(time.Second) } + + return e.JSON(http.StatusOK, bankid.Empty{}) } } -func change(client *bankid.API) func(c echo.Context) error { - return func(c echo.Context) error { - b, err := io.ReadAll(c.Request().Body) +func change(client *bankid.API) func(echo.Context) error { + return func(e echo.Context) error { + b, err := io.ReadAll(e.Request().Body) if err != nil { fmt.Printf("ERR: failed to read change request message: %s\n", err.Error()) - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) } var request bankid.ChangeRequest err = json.Unmarshal(b, &request) if err != nil { fmt.Printf("ERR: failed to unmarshal change request message: %s\n", err.Error()) - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) } - res, err := client.Change(c.Request().Context(), &request) + res, err := client.Change(e.Request().Context(), &request) if err != nil { fmt.Printf("ERR: initiating change request against bankid: %s\n", err.Error()) - return c.JSON(500, "failed to start change request") + return e.JSON(500, "failed to start change request") } - return c.JSON(http.StatusOK, res) + return e.JSON(http.StatusOK, res) } } -func collect(client *bankid.API) func(c echo.Context) error { - return func(c echo.Context) error { - b, err := io.ReadAll(c.Request().Body) +func collect(client *bankid.API) func(echo.Context) error { + return func(e echo.Context) error { + b, err := io.ReadAll(e.Request().Body) if err != nil { fmt.Printf("ERR: failed to read collect request message: %s\n", err.Error()) - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) } var request bankid.CollectRequest err = json.Unmarshal(b, &request) if err != nil { fmt.Printf("ERR: failed to unmarshal collect request message: %s\n", err.Error()) - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) } - res, err := client.Collect(c.Request().Context(), &request) + res, err := client.Collect(e.Request().Context(), &request) if err != nil { fmt.Printf("ERR: initiating collect request against bankid: %s\n", err.Error()) - return c.JSON(500, "failed to start collect against BankID") + return e.JSON(500, "failed to start collect against BankID") } - return c.JSON(http.StatusOK, res) + return e.JSON(http.StatusOK, res) } } -func cancel(client *bankid.API) func(c echo.Context) error { - return func(c echo.Context) error { - b, err := io.ReadAll(c.Request().Body) +func cancel(client *bankid.API) func(echo.Context) error { + return func(e echo.Context) error { + b, err := io.ReadAll(e.Request().Body) if err != nil { fmt.Printf("ERR: failed to read cancel request message: %s\n", err.Error()) - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) } var request bankid.CancelRequest err = json.Unmarshal(b, &request) if err != nil { fmt.Printf("ERR: failed to unmarshal cancel request message: %s\n", err.Error()) - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + return e.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) } - err = client.Cancel(c.Request().Context(), &request) + err = client.Cancel(e.Request().Context(), &request) if err != nil { fmt.Printf("ERR: initiating cancel request against bankid: %s\n", err.Error()) - return c.JSON(500, "failed to start cancel against BankID") + return e.JSON(500, "failed to start cancel against BankID") + } + + return e.NoContent(http.StatusNoContent) + } +} + +func createResponseFromAuthSign(r *bankid.AuthSignResponse, qrCodeTime int) api.BankIdV6Response { + return api.BankIdV6Response{ + OrderRef: r.OrderRef, + URI: fmt.Sprintf("bankid:///?autostarttoken=%s&redirect=null", r.AutoStartToken), + QR: r.BuildQrCode(qrCodeTime), + } +} + +func createResponseFromCollect(change bankid.Change) api.BankIdV6Response { + // Status == pending, only send status and hint updates + if change.Status == bankid.Pending { + return api.BankIdV6Response{ + OrderRef: change.OrderRef, + Status: string(change.Status), + HintCode: string(change.HintCode), + } + } + + // Status == (complete | failed), send complete message + return api.BankIdV6Response{ + OrderRef: change.OrderRef, + Status: string(change.Status), + HintCode: string(change.HintCode), + CompletionData: &api.BankIdV6CompletionData{ + User: api.BankIdV6User(change.CompletionData.User), + Device: api.BankIdV6Device(change.CompletionData.Device), + BankIdIssueDate: change.CompletionData.BankIdIssueDate, + StepUp: api.BankIdV6StepUp(change.CompletionData.StepUp), + Signature: change.CompletionData.Signature, + OcspResponse: change.CompletionData.OcspResponse, + }, + } +} + +func createResponseFromError(orderRef string, err error) api.BankIdV6Response { + return api.BankIdV6Response{ + OrderRef: orderRef, + CollectError: err.Error(), + } +} + +type ( + authSignFn func(context.Context, *bankid.AuthSignRequest) (*bankid.AuthSignResponse, error) + watchFn func(context.Context, string) (<-chan bankid.Change, error) +) + +const ( + qrCodeEvent = "qrcode" + statusEvent = "status" + errorEvent = "error" +) + +func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration) func(echo.Context) error { + return func(c echo.Context) error { + b, err := io.ReadAll(c.Request().Body) + if err != nil { + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + } + + var request bankid.AuthSignRequest + err = json.Unmarshal(b, &request) + if err != nil { + fmt.Printf("ERR: failed to unmarshal auth request message: %s\n", err.Error()) + return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + } + + res, err := authSign(c.Request().Context(), &request) + if err != nil { + fmt.Printf("ERR: initiating auth request against bankid: %s\n", err.Error()) + return c.JSON(500, "failed to initiate auth against BankId") } - return c.NoContent(http.StatusNoContent) + // In the case a client wants to initiate a new request every second instead of relying on SSE + // we respond with the first entry and then close the connection + response := c.QueryParam("type") + if response == "once" { + return c.JSON(200, createResponseFromAuthSign(res, 0)) + } + + stream, err := sse2.NewWriter(c.Response()) + if err != nil { + fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) + return err + } + + err = stream.SendJSON(qrCodeEvent, createResponseFromAuthSign(res, 0)) + if err != nil { + fmt.Printf("ERR: failed to write auth response to stream: %s\n", err.Error()) + return err + } + + changes, err := watch(c.Request().Context(), res.OrderRef) + if err != nil { + return c.JSON(500, "failed to setup response stream") + } + + // Stream new QR codes and status changes back to caller, while waiting + // for status to become != pending. If hintCode is 'userSign', the barcode + // have been read, and we'll stop sending new QR code strings to caller. + updateQR := time.NewTicker(qrPeriod) + qrCount := 1 + for { + select { + case <-updateQR.C: + err = stream.SendJSON(qrCodeEvent, createResponseFromAuthSign(res, qrCount)) + if err != nil { + fmt.Printf("ERR: failed to send updated QR code message: %v\n", err) + return echo.NewHTTPError(http.StatusInternalServerError, "failed to send updated QR code message") + } + qrCount++ + case state, ok := <-changes: + if !ok { + // channel were unexpectedly closed + fmt.Println("ERR: change channel were unexpectedly closed") + return echo.NewHTTPError(http.StatusInternalServerError, "change channel were unexpectedly closed") + } + + if state.Err != nil { + // Something failed, channel will close after this... + err = stream.SendJSON(errorEvent, createResponseFromError(res.OrderRef, state.Err)) + if err != nil { + fmt.Printf("ERR: failed to send status update: %v\n", err) + return echo.NewHTTPError(http.StatusInternalServerError, "failed to send error message") + } + return nil + } + + if state.HintCode == "userSign" { + // Stop updateQR timer when QR-code have been scanned + updateQR.Stop() + } + + // Stream latest status to caller + err = stream.SendJSON(statusEvent, createResponseFromCollect(state)) + if err != nil { + fmt.Printf("ERR: failed to send status update: %v\n", err) + return echo.NewHTTPError(http.StatusInternalServerError, "failed to send status update") + } + + // Check for completion + if state.Status != "" && state.Status != bankid.Pending { + return nil + } + } + } } } diff --git a/internal/httpserve/bankid_test.go b/internal/httpserve/bankid_test.go new file mode 100644 index 0000000..7a25917 --- /dev/null +++ b/internal/httpserve/bankid_test.go @@ -0,0 +1,212 @@ +package httpserve + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "reflect" + "testing" + "time" + + "github.com/labstack/echo/v4" + "github.com/modfin/twofer/internal/bankid" + "github.com/modfin/twofer/sse" +) + +const ( + qrTestPeriod = time.Millisecond * 100 + testIP = "127.0.0.1" + + // Test ID's taken from example in /auth API documentation + testAuthOrderRef = "131daac9-16c6-4618-beb0-365768f37288" + testAuthAutoStartToken = "7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6" + testAuthQrStartToken = "67df3917-fa0d-44e5-b327-edcc928297f8" + testAuthQrStartSecret = "d28db9a7-4cde-429e-a983-359be676944c" +) + +type ( + authSignTestFn func(*testing.T) authSignFn + watchTestFn func(*testing.T) watchFn + newContextFn func(r *http.Request, w http.ResponseWriter) echo.Context + authSignTest struct { + name string + request bankid.AuthSignRequest + params *url.Values + authSign authSignTestFn + watch watchTestFn + wantHTTPStatus int + wantEvents []sse.Event + } +) + +var ( + authResponseOK = bankid.AuthSignResponse{ + OrderRef: testAuthOrderRef, + AutoStartToken: testAuthAutoStartToken, + QrStartToken: testAuthQrStartToken, + QrStartSecret: testAuthQrStartSecret, + } + + pending_OutstandingTransaction = bankid.CollectResponse{OrderRef: testAuthOrderRef, Status: bankid.Pending, HintCode: bankid.OutstandingTransaction} + pending_UserSign = bankid.CollectResponse{OrderRef: testAuthOrderRef, Status: bankid.Pending, HintCode: bankid.UserSign} + complete_OK = bankid.CollectResponse{ + OrderRef: testAuthOrderRef, + Status: bankid.Complete, + // Data below taken from /collect API example + CompletionData: bankid.CompletionData{ + User: bankid.User{ + PersonalNumber: "190000000000", + Name: "Karl Karlsson", + GivenName: "Karl", + SurName: "Karlsson", + }, + Device: bankid.Device{ + IpAddress: testIP, + }, + BankIdIssueDate: "2020-02-01", + Signature: "", // OK since we don't try to decode it + OcspResponse: "", // OK since we don't try to decode it + }, + } +) + +func Test_authSign(t *testing.T) { + e := echo.New() + for _, tt := range []authSignTest{ + { + name: "happy_auth_flow", + request: bankid.AuthSignRequest{EndUserIp: testIP}, + authSign: authSignTestMock(authResponseOK, nil), + watch: watchMock([]bankid.CollectResponse{ + pending_OutstandingTransaction, + pending_UserSign, + complete_OK, + }, nil), + wantHTTPStatus: http.StatusOK, + wantEvents: []sse.Event{ + {Event: "qrcode", Data: `{"orderRef":"131daac9-16c6-4618-beb0-365768f37288","uri":"bankid:///?autostarttoken=7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6\u0026redirect=null","qr":"bankid.67df3917-fa0d-44e5-b327-edcc928297f8.0.dc69358e712458a66a7525beef148ae8526b1c71610eff2c16cdffb4cdac9bf8"}`}, + {Event: "qrcode", Data: `{"orderRef":"131daac9-16c6-4618-beb0-365768f37288","uri":"bankid:///?autostarttoken=7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6\u0026redirect=null","qr":"bankid.67df3917-fa0d-44e5-b327-edcc928297f8.1.949d559bf23403952a94d103e67743126381eda00f0b3cbddbf7c96b1adcbce2"}`}, + {Event: "qrcode", Data: `{"orderRef":"131daac9-16c6-4618-beb0-365768f37288","uri":"bankid:///?autostarttoken=7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6\u0026redirect=null","qr":"bankid.67df3917-fa0d-44e5-b327-edcc928297f8.2.a9e5ec59cb4eee4ef4117150abc58fad7a85439a6a96ccbecc3668b41795b3f3"}`}, + {Event: "status", Data: `{"orderRef":"131daac9-16c6-4618-beb0-365768f37288","status":"pending","hintCode":"outstandingTransaction"}`}, + {Event: "qrcode", Data: `{"orderRef":"131daac9-16c6-4618-beb0-365768f37288","uri":"bankid:///?autostarttoken=7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6\u0026redirect=null","qr":"bankid.67df3917-fa0d-44e5-b327-edcc928297f8.3.96077d77699971790b46ee1f04ff1e44fe96b0602c9c51e4ca9c6d031c7c3bb7"}`}, + {Event: "qrcode", Data: `{"orderRef":"131daac9-16c6-4618-beb0-365768f37288","uri":"bankid:///?autostarttoken=7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6\u0026redirect=null","qr":"bankid.67df3917-fa0d-44e5-b327-edcc928297f8.4.1d9a7e5dd98d08cb393f73c63ce032df0c9433512153ab9fb040b96cd45b1b11"}`}, + {Event: "status", Data: `{"orderRef":"131daac9-16c6-4618-beb0-365768f37288","status":"pending","hintCode":"userSign"}`}, + {Event: "status", Data: `{"orderRef":"131daac9-16c6-4618-beb0-365768f37288","status":"complete","completionData":{"user":{"personalNumber":"190000000000","name":"Karl Karlsson","givenName":"Karl","surName":"Karlsson"},"device":{"ipAddress":"127.0.0.1"},"bankIdIssueDate":"2020-02-01","stepUp":{},"signature":"\u003cbase64-encoded data\u003e","ocspResponse":"\u003cbase64-encoded data\u003e"}}`}, + }, + }, + } { + t.Run(tt.name, testAuthSign(tt, e.NewContext)) + } +} + +func testAuthSign(tt authSignTest, newContext newContextFn) func(t *testing.T) { + return func(t *testing.T) { + // t.Parallel() + + bodyData, err := json.Marshal(tt.request) + if err != nil { + t.Fatalf("ERROR: Failed to marshal request: %v", err) + } + + u, err := url.Parse("http://test.local/api/someurl") + if err != nil { + t.Fatalf("ERROR: Failed to parse URL: %v", err) + } + + if tt.params != nil { + u.RawQuery = tt.params.Encode() + } + + req := httptest.NewRequest("", u.String(), bytes.NewReader(bodyData)) + res := httptest.NewRecorder() + ctx := newContext(req, res) + + echoHandler := authSign(tt.authSign(t), tt.watch(t), qrTestPeriod) + err = echoHandler(ctx) + if err != nil { + t.Fatalf("got error: %v", err) + } + + response := res.Result() + defer func() { _ = response.Body.Close() }() + if response.StatusCode != tt.wantHTTPStatus { + t.Errorf("ERROR: got HTTP status code: %d, want: %d\n", response.StatusCode, tt.wantHTTPStatus) + } + + events := sse.NewReader(req.Context(), response.Body) + var cnt int + for e := range events { + // t.Logf("%v: got event: %v", time.Since(start), e) + if cnt < len(tt.wantEvents) && !reflect.DeepEqual(e, tt.wantEvents[cnt]) { + t.Errorf(" got event: %v\nwant event: %v", e, tt.wantEvents[cnt]) + } + cnt++ + } + if cnt != len(tt.wantEvents) { + t.Errorf("got %d events, want %d", cnt, len(tt.wantEvents)) + } + } +} + +func authSignTestMock(response bankid.AuthSignResponse, testErr error) authSignTestFn { + return func(t *testing.T) authSignFn { + return func(ctx context.Context, asr *bankid.AuthSignRequest) (*bankid.AuthSignResponse, error) { + if asr.EndUserIp != testIP { + t.Errorf("authFn got EndUserIp: '%s', want: '%s'", asr.EndUserIp, testIP) + } + + err := asr.ValidateAuthRequest() + if err != nil { + return nil, err + } + + return &response, testErr + } + } +} + +func watchMock(responses []bankid.CollectResponse, testErr error) watchTestFn { + return func(t *testing.T) watchFn { + return func(ctx context.Context, orderRef string) (<-chan bankid.Change, error) { + if orderRef != testAuthOrderRef { + t.Errorf("authwatch got orderRef: %s, want: %s", orderRef, testAuthOrderRef) + } + + collectRequest := &bankid.CollectRequest{OrderRef: orderRef} + err := collectRequest.Validate() + if err != nil { + return nil, err + } + + if testErr != nil { + return nil, testErr + } + + watch := make(chan bankid.Change, 1) // Make it a buffered channel so that we can post initial state before we return + + sendChange := func(change bankid.CollectResponse) { + select { + case watch <- bankid.Change{CollectResponse: change}: + case <-time.After(time.Second): + } + } + + go func() { + defer close(watch) + time.Sleep(time.Millisecond * 25) + for _, change := range responses { + time.Sleep(time.Millisecond * 200) + sendChange(change) + if change.Status != bankid.Pending { + return + } + } + }() + + return watch, nil + } + } +} diff --git a/internal/sse/sender.go b/internal/sse/sender.go index 6df8492..c603dc7 100644 --- a/internal/sse/sender.go +++ b/internal/sse/sender.go @@ -1,11 +1,9 @@ package sse import ( - "encoding/json" "errors" "fmt" "net/http" - "sync/atomic" "github.com/labstack/echo/v4" ) @@ -13,10 +11,9 @@ import ( // Sender represents a http server-sent events sender used to send compatible SSE events. // See https://html.spec.whatwg.org/multipage/server-sent-events.html type Sender struct { - eventID atomic.Uint32 response *echo.Response writer http.ResponseWriter - flusher http.Flusher + flush func() } func NewSender(response *echo.Response) (*Sender, error) { @@ -28,7 +25,7 @@ func NewSender(response *echo.Response) (*Sender, error) { return &Sender{ response: response, writer: response.Writer, - flusher: flusher, + flush: flusher.Flush, }, nil } @@ -39,30 +36,24 @@ func (s *Sender) Prepare() { s.writer.Header().Set("Transfer-Encoding", "chunked") } -// type Event struct { -// Id string `json:"id"` -// Event string `json:"event"` -// Data string `json:"data"` -// } - -func (s *Sender) Send(event string, data any) error { - bytes, err := json.Marshal(data) - if err != nil { - fmt.Printf("ERR: failed to marshal event message: %v\n", err) - return err - } +type Event struct { + Id string `json:"id"` + Event string `json:"event"` + Data string `json:"data"` +} - _, err = fmt.Fprintf(s.writer, "id: %d\n", s.eventID.Add(1)-1) +func (s *Sender) Send(event Event) error { + _, err := fmt.Fprintf(s.writer, "id: %s\n", event.Id) if err != nil { return err } - _, err = fmt.Fprintf(s.writer, "event: %s\n", event) + _, err = fmt.Fprintf(s.writer, "event: %s\n", event.Event) if err != nil { return err } - _, err = fmt.Fprintf(s.writer, "data: %s\n", bytes) + _, err = fmt.Fprintf(s.writer, "data: %s\n", event.Data) if err != nil { return err } @@ -72,7 +63,7 @@ func (s *Sender) Send(event string, data any) error { return err } - s.flusher.Flush() - s.response.Committed = true + s.flush() + s.response.Committed = true // Needed to fix 'superfluous response.WriteHeader' console messages return nil } diff --git a/sse/reader.go b/sse/reader.go index aa1a33b..75ee372 100644 --- a/sse/reader.go +++ b/sse/reader.go @@ -4,6 +4,7 @@ import ( "bufio" "bytes" "context" + "errors" "fmt" "io" "strings" @@ -83,6 +84,9 @@ func process(ctx context.Context, rc io.ReadCloser, ec chan<- Event) { // Continue to read data } fieldName, fieldData, eom, err := readField(buf) + if errors.Is(err, io.EOF) { + return + } if err != nil { fmt.Printf("failed to read field, error: %v\n", err) return diff --git a/test/bankidv6_test.go b/test/bankidv6_test.go index 8667896..4581ca0 100644 --- a/test/bankidv6_test.go +++ b/test/bankidv6_test.go @@ -13,6 +13,7 @@ import ( "strings" "github.com/modfin/twofer/internal/bankid" + "github.com/modfin/twofer/internal/sse" ) func (s *IntegrationTestSuite) TestAuthCallOnce() { @@ -82,11 +83,9 @@ func (s *IntegrationTestSuite) TestAuthCall() { // Default to splitting on each line scanner := bufio.NewScanner(resp.Body) defer resp.Body.Close() - var eventTypes = [3]string{"status", "message", "message"} - var qrTime int - for i := 0; i < 3; i++ { // Server should run for 30 seconds, but let's not wait that long for tests. If pattern holds for 2 messages it should be fine + for i := 0; i < 2; i++ { // Server should run for 30 seconds, but let's not wait that long for tests. If pattern holds for 2 messages it should be fine s.T().Logf("Iteration #%d", i) - // msg := sse.Event{} + msg := sse.Event{} scanRes := scanner.Scan() if !scanRes { @@ -98,7 +97,7 @@ func (s *IntegrationTestSuite) TestAuthCall() { s.Equal("id", idSplit[0]) s.Equal(strconv.Itoa(i), idSplit[1]) - // msg.Id = idSplit[1] + msg.Id = idSplit[1] scanRes = scanner.Scan() if !scanRes { @@ -108,9 +107,9 @@ func (s *IntegrationTestSuite) TestAuthCall() { eventLine := scanner.Text() eventSplit := strings.Split(eventLine, ": ") s.Equal("event", eventSplit[0]) - s.Equal(eventTypes[i], eventSplit[1]) + s.Equal("message", eventSplit[1]) - // msg.Event = eventSplit[1] + msg.Event = eventSplit[1] scanRes = scanner.Scan() if !scanRes { @@ -121,7 +120,7 @@ func (s *IntegrationTestSuite) TestAuthCall() { dataSplit := strings.Split(dataLine, ": ") s.Equal("data", dataSplit[0]) - // msg.Data = dataSplit[1] + msg.Data = dataSplit[1] // SSE messages ends in extra empty line, so scan once more scanRes = scanner.Scan() @@ -129,44 +128,26 @@ func (s *IntegrationTestSuite) TestAuthCall() { s.FailNow("Server-Sent Events message stopped after data") } - switch eventTypes[i] { - case "message": - var res bankid.AuthSignAPIResponse - err = json.Unmarshal([]byte(dataSplit[1]), &res) - if err != nil { - s.NoError(err, "error unmarshaling SSE data for msg") - } - - s.True(res.OrderRef != "") - - truth, ok := s.bankidv6.Orders[res.OrderRef] - s.True(ok, "no matching order in bankid fake") - - mac := hmac.New(sha256.New, []byte(truth.QrStartSecret)) - mac.Write([]byte(strconv.Itoa(qrTime))) - qrAuthCode := mac.Sum(nil) - - authCode := hex.EncodeToString(qrAuthCode) - qr := "bankid." + truth.QrStartToken + "." + strconv.Itoa(qrTime) + "." + authCode - s.Equal(qr, res.QR) - - s.Equal("bankid:///?autostarttoken="+truth.AutoStartToken+"&redirect=null", res.URI) - qrTime++ - case "status": - var res struct { - Status string - Hint string - } - err = json.Unmarshal([]byte(dataSplit[1]), &res) - if err != nil { - s.NoError(err, "error unmarshaling SSE data for msg") - } - - s.T().Logf("Status: %v", res) - s.True(res.Status != "", "Status is empty") - - s.True(res.Hint != "", "Hint is empty") + var res bankid.AuthSignAPIResponse + err = json.Unmarshal([]byte(msg.Data), &res) + if err != nil { + s.NoError(err, "error unmarshaling SSE data for msg") } + + s.True(res.OrderRef != "") + + truth, ok := s.bankidv6.Orders[res.OrderRef] + s.True(ok, "no matching order in bankid fake") + + mac := hmac.New(sha256.New, []byte(truth.QrStartSecret)) + mac.Write([]byte(strconv.Itoa(i))) + qrAuthCode := mac.Sum(nil) + + authCode := hex.EncodeToString(qrAuthCode) + qr := "bankid." + truth.QrStartToken + "." + strconv.Itoa(i) + "." + authCode + s.Equal(qr, res.QR) + + s.Equal("bankid:///?autostarttoken="+truth.AutoStartToken+"&redirect=null", res.URI) } } @@ -383,11 +364,9 @@ func (s *IntegrationTestSuite) TestSignCall() { // Default to splitting on each line scanner := bufio.NewScanner(resp.Body) defer resp.Body.Close() - var eventTypes = [3]string{"status", "message", "message"} - var qrTime int - for i := 0; i < 3; i++ { // Server should run for 30 seconds, but let's not wait that long for tests. If pattern holds for 2 messages it should be fine + for i := 0; i < 2; i++ { // Server should run for 30 seconds, but let's not wait that long for tests. If pattern holds for 2 messages it should be fine s.T().Logf("Iteration #%d", i) - // msg := sse.Event{} + msg := sse.Event{} scanRes := scanner.Scan() if !scanRes { @@ -399,7 +378,7 @@ func (s *IntegrationTestSuite) TestSignCall() { s.Equal("id", idSplit[0]) s.Equal(strconv.Itoa(i), idSplit[1]) - // msg.Id = idSplit[1] + msg.Id = idSplit[1] scanRes = scanner.Scan() if !scanRes { @@ -409,9 +388,9 @@ func (s *IntegrationTestSuite) TestSignCall() { eventLine := scanner.Text() eventSplit := strings.Split(eventLine, ": ") s.Equal("event", eventSplit[0]) - s.Equal(eventTypes[i], eventSplit[1]) + s.Equal("message", eventSplit[1]) - // msg.Event = eventSplit[1] + msg.Event = eventSplit[1] scanRes = scanner.Scan() if !scanRes { @@ -422,7 +401,7 @@ func (s *IntegrationTestSuite) TestSignCall() { dataSplit := strings.Split(dataLine, ": ") s.Equal("data", dataSplit[0]) - // msg.Data = dataSplit[1] + msg.Data = dataSplit[1] // SSE messages ends in extra empty line, so scan once more scanRes = scanner.Scan() @@ -430,44 +409,26 @@ func (s *IntegrationTestSuite) TestSignCall() { s.FailNow("Server-Sent Events message stopped after data") } - switch eventTypes[i] { - case "message": - var res bankid.AuthSignAPIResponse - err = json.Unmarshal([]byte(dataSplit[1]), &res) - if err != nil { - s.NoError(err, "error unmarshaling SSE data for msg") - } - - s.True(res.OrderRef != "") - - truth, ok := s.bankidv6.Orders[res.OrderRef] - s.True(ok, "no matching order in bankid fake") - - mac := hmac.New(sha256.New, []byte(truth.QrStartSecret)) - mac.Write([]byte(strconv.Itoa(qrTime))) - qrAuthCode := mac.Sum(nil) - - signCode := hex.EncodeToString(qrAuthCode) - qr := "bankid." + truth.QrStartToken + "." + strconv.Itoa(qrTime) + "." + signCode - s.Equal(qr, res.QR) - - s.Equal("bankid:///?autostarttoken="+truth.AutoStartToken+"&redirect=null", res.URI) - qrTime++ - case "status": - var res struct { - Status string - Hint string - } - err = json.Unmarshal([]byte(dataSplit[1]), &res) - if err != nil { - s.NoError(err, "error unmarshaling SSE data for msg") - } - - s.T().Logf("Status: %v", res) - s.True(res.Status != "", "Status is empty") - - s.True(res.Hint != "", "Hint is empty") + var res bankid.AuthSignAPIResponse + err = json.Unmarshal([]byte(msg.Data), &res) + if err != nil { + s.NoError(err, "error unmarshaling SSE data for msg") } + + s.True(res.OrderRef != "") + + truth, ok := s.bankidv6.Orders[res.OrderRef] + s.True(ok, "no matching order in bankid fake") + + mac := hmac.New(sha256.New, []byte(truth.QrStartSecret)) + mac.Write([]byte(strconv.Itoa(i))) + qrAuthCode := mac.Sum(nil) + + signCode := hex.EncodeToString(qrAuthCode) + qr := "bankid." + truth.QrStartToken + "." + strconv.Itoa(i) + "." + signCode + s.Equal(qr, res.QR) + + s.Equal("bankid:///?autostarttoken="+truth.AutoStartToken+"&redirect=null", res.URI) } } From 6da8894f10fe2c51d72190a78dab6c562780b93b Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Mon, 15 Apr 2024 17:21:26 +0200 Subject: [PATCH 14/28] Add NDJSON as a new default stream encoder for v2 endpoints Set STREAM_ENCODER environment variable to "SSE", to use SSE as stream encoder. --- cmd/twoferd/main.go | 17 ++++- internal/config/config.go | 2 + internal/httpserve/bankid.go | 28 ++++---- internal/httpserve/bankid_test.go | 111 +++++++++++++++++++++-------- stream/ndjson/ndjson.go | 10 +++ stream/ndjson/reader.go | 37 ++++++++++ stream/ndjson/reader_test.go | 78 ++++++++++++++++++++ stream/ndjson/writer.go | 52 ++++++++++++++ stream/ndjson/writer_test.go | 89 +++++++++++++++++++++++ {sse => stream/sse}/event.go | 0 {sse => stream/sse}/reader.go | 0 {sse => stream/sse}/reader_test.go | 0 {sse => stream/sse}/writer.go | 18 +++++ {sse => stream/sse}/writer_test.go | 0 stream/stream.go | 8 +++ test/main_test.go | 2 +- 16 files changed, 407 insertions(+), 45 deletions(-) create mode 100644 stream/ndjson/ndjson.go create mode 100644 stream/ndjson/reader.go create mode 100644 stream/ndjson/reader_test.go create mode 100644 stream/ndjson/writer.go create mode 100644 stream/ndjson/writer_test.go rename {sse => stream/sse}/event.go (100%) rename {sse => stream/sse}/reader.go (100%) rename {sse => stream/sse}/reader_test.go (100%) rename {sse => stream/sse}/writer.go (85%) rename {sse => stream/sse}/writer_test.go (100%) create mode 100644 stream/stream.go diff --git a/cmd/twoferd/main.go b/cmd/twoferd/main.go index 7f0887e..62f3ef6 100644 --- a/cmd/twoferd/main.go +++ b/cmd/twoferd/main.go @@ -11,7 +11,9 @@ import ( "syscall" "time" + "github.com/labstack/echo/v4" "github.com/labstack/echo/v4/middleware" + "github.com/modfin/twofer/internal/config" "github.com/modfin/twofer/internal/eid/bankid" "github.com/modfin/twofer/internal/httpserve" @@ -19,8 +21,8 @@ import ( "github.com/modfin/twofer/internal/servotp" "github.com/modfin/twofer/internal/servpwd" "github.com/modfin/twofer/internal/servqr" - - "github.com/labstack/echo/v4" + "github.com/modfin/twofer/stream/ndjson" + "github.com/modfin/twofer/stream/sse" ) func main() { @@ -154,6 +156,15 @@ func startEid(e *echo.Echo) { } else { fmt.Println(" - Adding BankId v6.0") fmt.Println(" - BankId Client Cert NotAfter:", bankid.ParsedClientCert().NotAfter) - httpserve.RegisterBankIDServer(e, bankid.APIv60) + httpserve.RegisterBankIDServer(e, bankid.APIv60, getStreamEncoder(config.Get().StreamEncoder)) + } +} + +func getStreamEncoder(encoder string) httpserve.NewStreamEncoder { + switch encoder { + case "SSE": + return sse.NewEncoder + default: + return ndjson.NewEncoder } } diff --git a/internal/config/config.go b/internal/config/config.go index a625687..ab1ce73 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -19,6 +19,8 @@ type Config struct { OTP OTP WebAuthn WebAuthn PWD PWD + + StreamEncoder string `env:"STREAM_ENCODER" envDefault:"NDJSON"` } func (c Config) EIDEnabled() bool { diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index 5f0d1e4..7e5a7dc 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -9,21 +9,23 @@ import ( "strconv" "time" - "github.com/modfin/twofer/api" - "github.com/modfin/twofer/internal/sse" - sse2 "github.com/modfin/twofer/sse" - "github.com/labstack/echo/v4" + + "github.com/modfin/twofer/api" "github.com/modfin/twofer/internal/bankid" + "github.com/modfin/twofer/internal/sse" + "github.com/modfin/twofer/stream" ) const qrCodeUpdatePeriod = time.Second -func RegisterBankIDServer(e *echo.Echo, client *bankid.API) { +type NewStreamEncoder func(http.ResponseWriter) (stream.Encoder, error) + +func RegisterBankIDServer(e *echo.Echo, client *bankid.API, newEncoder NewStreamEncoder) { e.POST("/bankid/v6/auth", auth(client)) - e.POST("/bankid/v6/authv2", authSign(client.Auth, client.WatchForChangeV2, qrCodeUpdatePeriod)) + e.POST("/bankid/v6/authv2", authSign(client.Auth, client.WatchForChangeV2, qrCodeUpdatePeriod, newEncoder)) e.POST("/bankid/v6/sign", sign(client)) - e.POST("/bankid/v6/signv2", authSign(client.Sign, client.WatchForChangeV2, qrCodeUpdatePeriod)) + e.POST("/bankid/v6/signv2", authSign(client.Sign, client.WatchForChangeV2, qrCodeUpdatePeriod, newEncoder)) e.POST("/bankid/v6/change", change(client)) e.POST("/bankid/v6/collect", collect(client)) e.POST("/bankid/v6/cancel", cancel(client)) @@ -326,7 +328,7 @@ const ( errorEvent = "error" ) -func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration) func(echo.Context) error { +func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration, newStreamEncoder NewStreamEncoder) func(echo.Context) error { return func(c echo.Context) error { b, err := io.ReadAll(c.Request().Body) if err != nil { @@ -353,13 +355,13 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration) func(e return c.JSON(200, createResponseFromAuthSign(res, 0)) } - stream, err := sse2.NewWriter(c.Response()) + stream, err := newStreamEncoder(c.Response()) if err != nil { fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) return err } - err = stream.SendJSON(qrCodeEvent, createResponseFromAuthSign(res, 0)) + err = stream(qrCodeEvent, createResponseFromAuthSign(res, 0)) if err != nil { fmt.Printf("ERR: failed to write auth response to stream: %s\n", err.Error()) return err @@ -378,7 +380,7 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration) func(e for { select { case <-updateQR.C: - err = stream.SendJSON(qrCodeEvent, createResponseFromAuthSign(res, qrCount)) + err = stream(qrCodeEvent, createResponseFromAuthSign(res, qrCount)) if err != nil { fmt.Printf("ERR: failed to send updated QR code message: %v\n", err) return echo.NewHTTPError(http.StatusInternalServerError, "failed to send updated QR code message") @@ -393,7 +395,7 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration) func(e if state.Err != nil { // Something failed, channel will close after this... - err = stream.SendJSON(errorEvent, createResponseFromError(res.OrderRef, state.Err)) + err = stream(errorEvent, createResponseFromError(res.OrderRef, state.Err)) if err != nil { fmt.Printf("ERR: failed to send status update: %v\n", err) return echo.NewHTTPError(http.StatusInternalServerError, "failed to send error message") @@ -407,7 +409,7 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration) func(e } // Stream latest status to caller - err = stream.SendJSON(statusEvent, createResponseFromCollect(state)) + err = stream(statusEvent, createResponseFromCollect(state)) if err != nil { fmt.Printf("ERR: failed to send status update: %v\n", err) return echo.NewHTTPError(http.StatusInternalServerError, "failed to send status update") diff --git a/internal/httpserve/bankid_test.go b/internal/httpserve/bankid_test.go index 7a25917..c8712a3 100644 --- a/internal/httpserve/bankid_test.go +++ b/internal/httpserve/bankid_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "io" "net/http" "net/http/httptest" "net/url" @@ -12,8 +13,11 @@ import ( "time" "github.com/labstack/echo/v4" + + "github.com/modfin/twofer/api" "github.com/modfin/twofer/internal/bankid" - "github.com/modfin/twofer/sse" + "github.com/modfin/twofer/stream/ndjson" + "github.com/modfin/twofer/stream/sse" ) const ( @@ -28,17 +32,21 @@ const ( ) type ( - authSignTestFn func(*testing.T) authSignFn - watchTestFn func(*testing.T) watchFn - newContextFn func(r *http.Request, w http.ResponseWriter) echo.Context - authSignTest struct { + authSignTestFn func(*testing.T) authSignFn + watchTestFn func(*testing.T) watchFn + newContextFn func(r *http.Request, w http.ResponseWriter) echo.Context + responseCheckFn func(*testing.T, context.Context, io.ReadCloser, authSignTest) + authSignTest struct { name string + encoder NewStreamEncoder + testDecoder responseCheckFn request bankid.AuthSignRequest params *url.Values authSign authSignTestFn watch watchTestFn wantHTTPStatus int wantEvents []sse.Event + wantResponses []api.BankIdV6Response } ) @@ -50,9 +58,9 @@ var ( QrStartSecret: testAuthQrStartSecret, } - pending_OutstandingTransaction = bankid.CollectResponse{OrderRef: testAuthOrderRef, Status: bankid.Pending, HintCode: bankid.OutstandingTransaction} - pending_UserSign = bankid.CollectResponse{OrderRef: testAuthOrderRef, Status: bankid.Pending, HintCode: bankid.UserSign} - complete_OK = bankid.CollectResponse{ + pendingOutstandingTransaction = bankid.CollectResponse{OrderRef: testAuthOrderRef, Status: bankid.Pending, HintCode: bankid.OutstandingTransaction} + pendingUserSign = bankid.CollectResponse{OrderRef: testAuthOrderRef, Status: bankid.Pending, HintCode: bankid.UserSign} + completeOK = bankid.CollectResponse{ OrderRef: testAuthOrderRef, Status: bankid.Complete, // Data below taken from /collect API example @@ -77,13 +85,15 @@ func Test_authSign(t *testing.T) { e := echo.New() for _, tt := range []authSignTest{ { - name: "happy_auth_flow", - request: bankid.AuthSignRequest{EndUserIp: testIP}, - authSign: authSignTestMock(authResponseOK, nil), + name: "happy_auth_flow_sse_stream", + encoder: sse.NewEncoder, + testDecoder: sseCheck, + request: bankid.AuthSignRequest{EndUserIp: testIP}, + authSign: authSignTestMock(authResponseOK, nil), watch: watchMock([]bankid.CollectResponse{ - pending_OutstandingTransaction, - pending_UserSign, - complete_OK, + pendingOutstandingTransaction, + pendingUserSign, + completeOK, }, nil), wantHTTPStatus: http.StatusOK, wantEvents: []sse.Event{ @@ -97,6 +107,34 @@ func Test_authSign(t *testing.T) { {Event: "status", Data: `{"orderRef":"131daac9-16c6-4618-beb0-365768f37288","status":"complete","completionData":{"user":{"personalNumber":"190000000000","name":"Karl Karlsson","givenName":"Karl","surName":"Karlsson"},"device":{"ipAddress":"127.0.0.1"},"bankIdIssueDate":"2020-02-01","stepUp":{},"signature":"\u003cbase64-encoded data\u003e","ocspResponse":"\u003cbase64-encoded data\u003e"}}`}, }, }, + { + name: "happy_auth_flow_ndjson_stream", + encoder: ndjson.NewEncoder, + testDecoder: ndjsonCheck, + request: bankid.AuthSignRequest{EndUserIp: testIP}, + authSign: authSignTestMock(authResponseOK, nil), + watch: watchMock([]bankid.CollectResponse{ + pendingOutstandingTransaction, + pendingUserSign, + completeOK, + }, nil), + wantHTTPStatus: http.StatusOK, + wantResponses: []api.BankIdV6Response{ + {OrderRef: "131daac9-16c6-4618-beb0-365768f37288", URI: "bankid:///?autostarttoken=7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6\u0026redirect=null", QR: "bankid.67df3917-fa0d-44e5-b327-edcc928297f8.0.dc69358e712458a66a7525beef148ae8526b1c71610eff2c16cdffb4cdac9bf8"}, + {OrderRef: "131daac9-16c6-4618-beb0-365768f37288", URI: "bankid:///?autostarttoken=7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6\u0026redirect=null", QR: "bankid.67df3917-fa0d-44e5-b327-edcc928297f8.1.949d559bf23403952a94d103e67743126381eda00f0b3cbddbf7c96b1adcbce2"}, + {OrderRef: "131daac9-16c6-4618-beb0-365768f37288", URI: "bankid:///?autostarttoken=7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6\u0026redirect=null", QR: "bankid.67df3917-fa0d-44e5-b327-edcc928297f8.2.a9e5ec59cb4eee4ef4117150abc58fad7a85439a6a96ccbecc3668b41795b3f3"}, + {OrderRef: "131daac9-16c6-4618-beb0-365768f37288", Status: "pending", HintCode: "outstandingTransaction"}, + {OrderRef: "131daac9-16c6-4618-beb0-365768f37288", URI: "bankid:///?autostarttoken=7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6\u0026redirect=null", QR: "bankid.67df3917-fa0d-44e5-b327-edcc928297f8.3.96077d77699971790b46ee1f04ff1e44fe96b0602c9c51e4ca9c6d031c7c3bb7"}, + {OrderRef: "131daac9-16c6-4618-beb0-365768f37288", URI: "bankid:///?autostarttoken=7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6\u0026redirect=null", QR: "bankid.67df3917-fa0d-44e5-b327-edcc928297f8.4.1d9a7e5dd98d08cb393f73c63ce032df0c9433512153ab9fb040b96cd45b1b11"}, + {OrderRef: "131daac9-16c6-4618-beb0-365768f37288", Status: "pending", HintCode: "userSign"}, + {OrderRef: "131daac9-16c6-4618-beb0-365768f37288", Status: "complete", CompletionData: &api.BankIdV6CompletionData{ + User: api.BankIdV6User{PersonalNumber: "190000000000", Name: "Karl Karlsson", GivenName: "Karl", SurName: "Karlsson"}, + Device: api.BankIdV6Device{IpAddress: "127.0.0.1"}, + BankIdIssueDate: "2020-02-01", + Signature: "\u003cbase64-encoded data\u003e", + OcspResponse: "\u003cbase64-encoded data\u003e"}}, + }, + }, } { t.Run(tt.name, testAuthSign(tt, e.NewContext)) } @@ -104,7 +142,7 @@ func Test_authSign(t *testing.T) { func testAuthSign(tt authSignTest, newContext newContextFn) func(t *testing.T) { return func(t *testing.T) { - // t.Parallel() + t.Parallel() bodyData, err := json.Marshal(tt.request) if err != nil { @@ -124,7 +162,7 @@ func testAuthSign(tt authSignTest, newContext newContextFn) func(t *testing.T) { res := httptest.NewRecorder() ctx := newContext(req, res) - echoHandler := authSign(tt.authSign(t), tt.watch(t), qrTestPeriod) + echoHandler := authSign(tt.authSign(t), tt.watch(t), qrTestPeriod, tt.encoder) err = echoHandler(ctx) if err != nil { t.Fatalf("got error: %v", err) @@ -136,18 +174,7 @@ func testAuthSign(tt authSignTest, newContext newContextFn) func(t *testing.T) { t.Errorf("ERROR: got HTTP status code: %d, want: %d\n", response.StatusCode, tt.wantHTTPStatus) } - events := sse.NewReader(req.Context(), response.Body) - var cnt int - for e := range events { - // t.Logf("%v: got event: %v", time.Since(start), e) - if cnt < len(tt.wantEvents) && !reflect.DeepEqual(e, tt.wantEvents[cnt]) { - t.Errorf(" got event: %v\nwant event: %v", e, tt.wantEvents[cnt]) - } - cnt++ - } - if cnt != len(tt.wantEvents) { - t.Errorf("got %d events, want %d", cnt, len(tt.wantEvents)) - } + tt.testDecoder(t, req.Context(), response.Body, tt) } } @@ -210,3 +237,31 @@ func watchMock(responses []bankid.CollectResponse, testErr error) watchTestFn { } } } + +func ndjsonCheck(t *testing.T, ctx context.Context, body io.ReadCloser, tt authSignTest) { + responseChan := ndjson.NewReader[api.BankIdV6Response](ctx, body) + var cnt int + for res := range responseChan { + //t.Logf("got response: %v", res) + if cnt < len(tt.wantEvents) && !reflect.DeepEqual(res, tt.wantResponses[cnt]) { + t.Errorf(" got event: %v\nwant event: %v", res, tt.wantResponses[cnt]) + } + cnt++ + } +} + +func sseCheck(t *testing.T, ctx context.Context, body io.ReadCloser, tt authSignTest) { + events := sse.NewReader(ctx, body) + var cnt int + for e := range events { + // t.Logf("%v: got event: %v", time.Since(start), e) + if cnt < len(tt.wantEvents) && !reflect.DeepEqual(e, tt.wantEvents[cnt]) { + t.Errorf(" got event: %v\nwant event: %v", e, tt.wantEvents[cnt]) + } + cnt++ + } + if cnt != len(tt.wantEvents) { + t.Errorf("got %d events, want %d", cnt, len(tt.wantEvents)) + } + +} diff --git a/stream/ndjson/ndjson.go b/stream/ndjson/ndjson.go new file mode 100644 index 0000000..b2d7b25 --- /dev/null +++ b/stream/ndjson/ndjson.go @@ -0,0 +1,10 @@ +/* +Package ndjson implement NDJSON (Newline Delimited JSON) stream readers and +writers. See link below for more information: +https://en.wikipedia.org/wiki/JSON_streaming#Newline-delimited_JSON + +As far as I could find there isn't a official MIME type registered for streaming +NDJSON, but `application/x-json-stream` seems to one unofficial that is used, and +that's the one that we're currently going with. +*/ +package ndjson diff --git a/stream/ndjson/reader.go b/stream/ndjson/reader.go new file mode 100644 index 0000000..cc45614 --- /dev/null +++ b/stream/ndjson/reader.go @@ -0,0 +1,37 @@ +package ndjson + +import ( + "context" + "encoding/json" + "errors" + "io" + "time" +) + +func NewReader[Response any](ctx context.Context, rc io.ReadCloser) <-chan Response { + eventChan := make(chan Response) + go func() { + defer close(eventChan) + dec := json.NewDecoder(rc) + for { + var data Response + err := dec.Decode(&data) + if errors.Is(err, io.EOF) { + return + } + if err != nil { + // TODO: Log error + return + } + + select { + case eventChan <- data: + case <-ctx.Done(): + return + case <-time.After(time.Second): + // TODO: Log that we have skipped to send 'data' (channel timeout) + } + } + }() + return eventChan +} diff --git a/stream/ndjson/reader_test.go b/stream/ndjson/reader_test.go new file mode 100644 index 0000000..62de5a3 --- /dev/null +++ b/stream/ndjson/reader_test.go @@ -0,0 +1,78 @@ +package ndjson + +import ( + "context" + "io" + "testing" + "time" +) + +func TestNewReader(t *testing.T) { + type testReadObj struct { + Data string `json:"data"` + } + type send struct { + buffer string + wantEvents []testReadObj + } + tests := []struct { + name string + sends []send + }{ + { + name: "single message", + sends: []send{ + {buffer: "{\"data\":\"test1-1\"}\n", wantEvents: []testReadObj{{"test1-1"}}}, + {buffer: "{\"data\":\"test1-2\"}\n", wantEvents: []testReadObj{{"test1-2"}}}, + {buffer: "{\"data\":\"test1-3\"}\n", wantEvents: []testReadObj{{"test1-3"}}}, + }, + }, + { + name: "concatenated messages", + sends: []send{ + {buffer: "{\"data\":\"test2-1\"}\n{\"data\":\"test2-2\"}\n{\"data\":\"test2-3\"}\n", wantEvents: []testReadObj{{"test2-1"}, {"test2-2"}, {"test2-3"}}}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pr, pw := io.Pipe() + chn := NewReader[testReadObj](context.Background(), pr) + + for i, s := range tt.sends { + if _, err := pw.Write([]byte(s.buffer)); err != nil { + t.Errorf("write buffer #%d ('%s') failed with error: %v", i, s.buffer, err) + } + + var gotEvents []testReadObj + done: + for { + select { + case e := <-chn: + gotEvents = append(gotEvents, e) + t.Logf("got event: %#v", e) + case <-time.After(time.Millisecond * 50): + break done + } + } + + if len(gotEvents) != len(s.wantEvents) { + t.Errorf("got %d events, want %d\nreceived events: %#v", len(gotEvents), len(s.wantEvents), gotEvents) + } else { + for i, we := range s.wantEvents { + ge := gotEvents[i] + if ge != we { + t.Errorf("got event #%d: %#v, want: %#v", i, ge, we) + } + } + } + } + + // Close pipe writer to indicate that there won't be any more data to read for the async process + err := pw.Close() + if err != nil { + t.Fatalf("failed to close pipe, error: %v", err) + } + }) + } +} diff --git a/stream/ndjson/writer.go b/stream/ndjson/writer.go new file mode 100644 index 0000000..c98c9f8 --- /dev/null +++ b/stream/ndjson/writer.go @@ -0,0 +1,52 @@ +package ndjson + +import ( + "encoding/json" + "net/http" + "sync" + + "github.com/modfin/twofer/stream" +) + +// Writer implement a NDJSON stream writer used to send streamed JSON objects over a HTTP stream. +type Writer struct { + mu sync.Mutex + encoder *json.Encoder + flush func() +} + +var _ stream.Writer = (*Writer)(nil) // Compile-time check that we implement stream.Writer interface + +func NewWriter(w http.ResponseWriter) (*Writer, error) { + flush := func() {} // NOP flusher, used if the provided writer don't support the http.Flusher interface + f, ok := w.(http.Flusher) + if ok { + // Replace NOP flusher since provided writer support the http.Flusher interface + flush = f.Flush + } + + w.Header().Set("Content-Type", "application/x-json-stream") + w.Header().Set("Cache-Control", "no-cache") + w.Header().Set("Connection", "keep-alive") + w.Header().Set("Transfer-Encoding", "chunked") + + return &Writer{encoder: json.NewEncoder(w), flush: flush}, nil +} + +func NewEncoder(w http.ResponseWriter) (stream.Encoder, error) { + enc, err := NewWriter(w) + if err != nil { + return nil, err + } + return enc.SendJSON, nil +} + +func (w *Writer) SendJSON(_ string, data any) error { + if data == nil { + return nil + } + w.mu.Lock() + defer w.mu.Unlock() + defer w.flush() + return w.encoder.Encode(data) +} diff --git a/stream/ndjson/writer_test.go b/stream/ndjson/writer_test.go new file mode 100644 index 0000000..8a17e07 --- /dev/null +++ b/stream/ndjson/writer_test.go @@ -0,0 +1,89 @@ +package ndjson + +import ( + "net/http" + "testing" +) + +type ( + testWriter struct { + flushed bool + header http.Header + status int + written []byte + } + testWriteObj struct { + Data string `json:"data,omitempty"` + } +) + +func (w *testWriter) Flush() { + w.flushed = true +} + +func (w *testWriter) Header() http.Header { + if w.header == nil { + w.header = make(http.Header) + } + return w.header +} + +func (w *testWriter) Write(data []byte) (int, error) { + w.written = append(w.written, data...) + return len(data), nil +} + +func (w *testWriter) WriteHeader(statusCode int) { + w.status = statusCode +} + +func TestWriter_SendEvent(t *testing.T) { + tests := []struct { + name string + data any + wantErr bool + wantFlush bool + wantData string + }{ + { + name: "all fields", + data: testWriteObj{Data: "test 1"}, + wantFlush: true, + wantData: `{"data":"test 1"}` + "\n", + }, + { + name: "empty object", + data: testWriteObj{}, + wantFlush: true, + wantData: `{}` + "\n", + }, + { + name: "nil object", // This should not send anything + wantFlush: false, + wantData: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tw := &testWriter{} + + w, err := NewWriter(tw) + if err != nil { + t.Fatalf("Failed to create new writer: %v", err) + } + + err = w.SendJSON("", tt.data) + if (err != nil) != tt.wantErr { + t.Fatalf("SendJSON() error = %v, wantErr %v", err, tt.wantErr) + } + + if tw.flushed != tt.wantFlush { + t.Errorf("got flush: %v, want: %v", tw.flushed, tt.wantFlush) + } + + if string(tw.written) != tt.wantData { + t.Errorf("got data: '%s', want: '%s'", tw.written, tt.wantData) + } + }) + } +} diff --git a/sse/event.go b/stream/sse/event.go similarity index 100% rename from sse/event.go rename to stream/sse/event.go diff --git a/sse/reader.go b/stream/sse/reader.go similarity index 100% rename from sse/reader.go rename to stream/sse/reader.go diff --git a/sse/reader_test.go b/stream/sse/reader_test.go similarity index 100% rename from sse/reader_test.go rename to stream/sse/reader_test.go diff --git a/sse/writer.go b/stream/sse/writer.go similarity index 85% rename from sse/writer.go rename to stream/sse/writer.go index a893f29..354e566 100644 --- a/sse/writer.go +++ b/stream/sse/writer.go @@ -7,9 +7,13 @@ import ( "fmt" "net/http" "strings" + "sync" + + "github.com/modfin/twofer/stream" ) type Writer struct { + mu sync.Mutex http.ResponseWriter flush func() } @@ -19,6 +23,8 @@ var ( ErrWrite = errors.New("write error") ) +var _ stream.Writer = (*Writer)(nil) // Compile-time check that we implement stream.Writer interface + func NewWriter(w http.ResponseWriter) (*Writer, error) { f, ok := w.(http.Flusher) if !ok { @@ -34,6 +40,15 @@ func NewWriter(w http.ResponseWriter) (*Writer, error) { }, nil } +func NewEncoder(w http.ResponseWriter) (stream.Encoder, error) { + enc, err := NewWriter(w) + if err != nil { + return nil, err + } + + return enc.SendJSON, nil +} + func writeField(buf *bytes.Buffer, field, data string) { // Ignore empty and default values if data == "" || (field == "event" && data == "message") { @@ -64,6 +79,9 @@ func writeField(buf *bytes.Buffer, field, data string) { } func (w *Writer) SendEvent(event, data, id, retry string) error { + w.mu.Lock() + defer w.mu.Unlock() + var buf bytes.Buffer writeField(&buf, "event", event) writeField(&buf, "data", data) diff --git a/sse/writer_test.go b/stream/sse/writer_test.go similarity index 100% rename from sse/writer_test.go rename to stream/sse/writer_test.go diff --git a/stream/stream.go b/stream/stream.go new file mode 100644 index 0000000..62fc1c7 --- /dev/null +++ b/stream/stream.go @@ -0,0 +1,8 @@ +package stream + +type ( + Writer interface { + SendJSON(string, any) error + } + Encoder func(string, any) error +) diff --git a/test/main_test.go b/test/main_test.go index a784223..13fcfc9 100644 --- a/test/main_test.go +++ b/test/main_test.go @@ -88,7 +88,7 @@ func InitApplication(bankIDV6URL string) (*echo.Echo, error) { client := &http.Client{} twoferBankIDAPI := bankid.NewAPI(client, bankIDV6URL) - httpserve.RegisterBankIDServer(e, twoferBankIDAPI) + httpserve.RegisterBankIDServer(e, twoferBankIDAPI, nil) return e, nil } From 068334db7a4d267e78c00f095412b269ae5bf3b8 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Mon, 15 Apr 2024 17:31:15 +0200 Subject: [PATCH 15/28] Update GO and Alpine versions --- cmd/twoferd/Dockerfile.build | 4 ++-- cmd/twoferd/Dockerfile.dev | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/twoferd/Dockerfile.build b/cmd/twoferd/Dockerfile.build index f5b22f5..4d0eefa 100644 --- a/cmd/twoferd/Dockerfile.build +++ b/cmd/twoferd/Dockerfile.build @@ -1,4 +1,4 @@ -FROM golang:1.20.3-alpine3.17 as builder +FROM golang:1.21.9-alpine3.19 as builder RUN apk add --no-cache git curl build-base bash shadow @@ -11,7 +11,7 @@ RUN ls RUN go build -o /twoferd ./cmd/twoferd/main.go -FROM alpine:3.17.3 +FROM alpine:3.19.1 RUN apk add --no-cache tzdata ca-certificates EXPOSE 8080 COPY --from=builder /twoferd / diff --git a/cmd/twoferd/Dockerfile.dev b/cmd/twoferd/Dockerfile.dev index c61c8eb..2208f23 100644 --- a/cmd/twoferd/Dockerfile.dev +++ b/cmd/twoferd/Dockerfile.dev @@ -1,4 +1,4 @@ -FROM golang:1.20.3 +FROM golang:1.21.9 RUN apt-get update && apt-get install -y inotify-hookable From e6249f8208e49c4f1087157c04538ee5617a4742 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Tue, 16 Apr 2024 09:27:24 +0200 Subject: [PATCH 16/28] Fixes correct marshaling of AuthSignRequest before it's sent to BankID --- internal/bankid/bankid_models.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/bankid/bankid_models.go b/internal/bankid/bankid_models.go index 7f98bbc..b3998ff 100644 --- a/internal/bankid/bankid_models.go +++ b/internal/bankid/bankid_models.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "encoding/base64" "encoding/hex" + "encoding/json" "errors" "fmt" "strconv" @@ -73,6 +74,19 @@ func (r *AuthSignRequest) ValidateSignRequest() error { return nil } +func (r *AuthSignRequest) MarshalJSON() ([]byte, error) { + type alias AuthSignRequest // Needed to avoid recursion + asr := alias(*r) + + if asr.UserVisibleData != "" { + asr.UserVisibleData = base64.StdEncoding.EncodeToString([]byte(asr.UserVisibleData)) + } + if asr.UserNonVisibleData != "" { + asr.UserNonVisibleData = base64.StdEncoding.EncodeToString([]byte(asr.UserNonVisibleData)) + } + return json.Marshal(asr) +} + type AuthSignResponse struct { OrderRef string `json:"orderRef"` AutoStartToken string `json:"autoStartToken"` From 45afc921d31072d24855dfb1b3abd8586619310e Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Tue, 16 Apr 2024 09:28:29 +0200 Subject: [PATCH 17/28] Switch docker image to be built from scratch --- cmd/twoferd/Dockerfile.build | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cmd/twoferd/Dockerfile.build b/cmd/twoferd/Dockerfile.build index 4d0eefa..6a59afa 100644 --- a/cmd/twoferd/Dockerfile.build +++ b/cmd/twoferd/Dockerfile.build @@ -1,6 +1,6 @@ FROM golang:1.21.9-alpine3.19 as builder -RUN apk add --no-cache git curl build-base bash shadow +#RUN apk add --no-cache git curl build-base bash shadow RUN mkdir -p /go/src/twofer WORKDIR /go/src/twofer @@ -9,10 +9,11 @@ COPY . /go/src/twofer RUN ls -RUN go build -o /twoferd ./cmd/twoferd/main.go +RUN CGO_ENABLE=0 go build -o /twoferd ./cmd/twoferd/main.go -FROM alpine:3.19.1 -RUN apk add --no-cache tzdata ca-certificates +#FROM alpine:3.19.1 +#RUN apk add --no-cache tzdata ca-certificates +FROM scratch EXPOSE 8080 COPY --from=builder /twoferd / -CMD /twoferd +CMD [ "/twoferd" ] From 1e3a41d1267f5634151293b9183c685281ae71a4 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Tue, 16 Apr 2024 10:01:55 +0200 Subject: [PATCH 18/28] Poll BankID every two seconds, as stated in their spec --- internal/bankid/endpoints.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/bankid/endpoints.go b/internal/bankid/endpoints.go index 87ad22b..c4171fa 100644 --- a/internal/bankid/endpoints.go +++ b/internal/bankid/endpoints.go @@ -184,13 +184,16 @@ func (a *API) WatchForChangeV2(ctx context.Context, orderRef string) (<-chan Cha sendChange(*currentState) go func(lastState *CollectResponse) { + // Poll BankID every two seconds (according to their spec) + pollTicker := time.NewTicker(time.Second * 2) + defer pollTicker.Stop() defer close(watch) for { select { case <-ctx.Done(): sendError(ctx.Err()) return - case <-time.After(time.Second): + case <-pollTicker.C: } resp, err := a.Collect(ctx, collectRequest) From a12fcfeda600e427b17e054b648104626126334e Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Tue, 16 Apr 2024 10:02:29 +0200 Subject: [PATCH 19/28] v2 error handling --- internal/httpserve/bankid.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index 7e5a7dc..ef98485 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -332,20 +332,21 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration, newStr return func(c echo.Context) error { b, err := io.ReadAll(c.Request().Body) if err != nil { - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload"}) + fmt.Printf("ERR: failed to read request body: %v\n", err) + return c.JSON(400, createResponseFromError("", err)) } var request bankid.AuthSignRequest err = json.Unmarshal(b, &request) if err != nil { - fmt.Printf("ERR: failed to unmarshal auth request message: %s\n", err.Error()) - return c.JSON(400, bankid.GenericResponse{Message: "invalid request payload content"}) + fmt.Printf("ERR: failed to unmarshal auth request message: %v\n", err) + return c.JSON(400, createResponseFromError("", err)) } res, err := authSign(c.Request().Context(), &request) if err != nil { - fmt.Printf("ERR: initiating auth request against bankid: %s\n", err.Error()) - return c.JSON(500, "failed to initiate auth against BankId") + fmt.Printf("ERR: initiating auth request against bankid: %v\n", err) + return c.JSON(400, createResponseFromError("", err)) } // In the case a client wants to initiate a new request every second instead of relying on SSE @@ -355,21 +356,22 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration, newStr return c.JSON(200, createResponseFromAuthSign(res, 0)) } - stream, err := newStreamEncoder(c.Response()) + send, err := newStreamEncoder(c.Response()) if err != nil { fmt.Printf("ERR: failed to setup auth response stream: %s\n", err.Error()) - return err + return c.JSON(400, createResponseFromError(res.OrderRef, err)) } - err = stream(qrCodeEvent, createResponseFromAuthSign(res, 0)) + err = send(qrCodeEvent, createResponseFromAuthSign(res, 0)) if err != nil { fmt.Printf("ERR: failed to write auth response to stream: %s\n", err.Error()) - return err + return c.JSON(400, createResponseFromError(res.OrderRef, err)) } changes, err := watch(c.Request().Context(), res.OrderRef) if err != nil { - return c.JSON(500, "failed to setup response stream") + fmt.Printf("ERR: failed to setup response stream: %s\n", err.Error()) + return c.JSON(400, createResponseFromError(res.OrderRef, err)) } // Stream new QR codes and status changes back to caller, while waiting @@ -380,7 +382,7 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration, newStr for { select { case <-updateQR.C: - err = stream(qrCodeEvent, createResponseFromAuthSign(res, qrCount)) + err = send(qrCodeEvent, createResponseFromAuthSign(res, qrCount)) if err != nil { fmt.Printf("ERR: failed to send updated QR code message: %v\n", err) return echo.NewHTTPError(http.StatusInternalServerError, "failed to send updated QR code message") @@ -395,7 +397,7 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration, newStr if state.Err != nil { // Something failed, channel will close after this... - err = stream(errorEvent, createResponseFromError(res.OrderRef, state.Err)) + err = send(errorEvent, createResponseFromError(res.OrderRef, state.Err)) if err != nil { fmt.Printf("ERR: failed to send status update: %v\n", err) return echo.NewHTTPError(http.StatusInternalServerError, "failed to send error message") @@ -409,7 +411,7 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration, newStr } // Stream latest status to caller - err = stream(statusEvent, createResponseFromCollect(state)) + err = send(statusEvent, createResponseFromCollect(state)) if err != nil { fmt.Printf("ERR: failed to send status update: %v\n", err) return echo.NewHTTPError(http.StatusInternalServerError, "failed to send status update") From c4a30d6a79b6f6d4198f34185ca4557851f244c3 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Tue, 16 Apr 2024 10:06:27 +0200 Subject: [PATCH 20/28] Upgrade golang.org/x/ dependencies that resolves a number of CVE's --- go.mod | 10 +++++----- go.sum | 18 ++++++++++-------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 08babf3..1bd1f0e 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/modfin/twofer -go 1.19 +go 1.21 require ( github.com/caarlos0/env/v6 v6.10.1 @@ -10,8 +10,8 @@ require ( github.com/pquerna/otp v1.4.0 github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e github.com/stretchr/testify v1.8.2 - golang.org/x/crypto v0.8.0 - golang.org/x/net v0.9.0 + golang.org/x/crypto v0.22.0 + golang.org/x/net v0.24.0 ) require ( @@ -31,8 +31,8 @@ require ( github.com/valyala/bytebufferpool v1.0.0 // indirect github.com/valyala/fasttemplate v1.2.2 // indirect github.com/x448/float16 v0.8.4 // indirect - golang.org/x/sys v0.7.0 // indirect - golang.org/x/text v0.9.0 // indirect + golang.org/x/sys v0.19.0 // indirect + golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.3.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index 3dd7c7c..510d1a0 100644 --- a/go.sum +++ b/go.sum @@ -178,8 +178,8 @@ go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/crypto v0.8.0 h1:pd9TJtTueMTVQXzk8E2XESSMQDj/U7OUu0PqJqPXQjQ= -golang.org/x/crypto v0.8.0/go.mod h1:mRqEX+O9/h5TFCrQhkgjo2yKi0yYA+9ecGkdQoHrywE= +golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30= +golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= @@ -191,8 +191,10 @@ golang.org/x/net v0.0.0-20181220203305-927f97764cc3/go.mod h1:mL1N/T3taQHkDXs73r golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= -golang.org/x/net v0.9.0 h1:aWJ/m6xSmxWBx+V0XRHTlrYrPG56jKsLdTFmsSsCzOM= -golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns= +golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= +golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= +golang.org/x/net v0.24.0 h1:1PcaxkF854Fu3+lvBIx5SYn9wRlBzzcnHZSiaFFAb0w= +golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -210,11 +212,11 @@ golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20211103235746-7861aae1554b/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU= -golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o= +golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.9.0 h1:2sjJmO8cDvYveuX97RDLsxlyUxLl+GHoLxBiRdHllBE= -golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= +golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= +golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4= golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= From ee9933af3f5db31aa90e61ba316de32eea6c0804 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Tue, 16 Apr 2024 10:08:25 +0200 Subject: [PATCH 21/28] Upgrade echo while we're here --- go.mod | 10 +++++----- go.sum | 35 ++++++++++------------------------- 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/go.mod b/go.mod index 1bd1f0e..ab075b5 100644 --- a/go.mod +++ b/go.mod @@ -6,10 +6,10 @@ require ( github.com/caarlos0/env/v6 v6.10.1 github.com/go-webauthn/webauthn v0.8.2 github.com/google/uuid v1.3.0 - github.com/labstack/echo/v4 v4.10.2 + github.com/labstack/echo/v4 v4.12.0 github.com/pquerna/otp v1.4.0 github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e - github.com/stretchr/testify v1.8.2 + github.com/stretchr/testify v1.8.4 golang.org/x/crypto v0.22.0 golang.org/x/net v0.24.0 ) @@ -23,9 +23,9 @@ require ( github.com/golang-jwt/jwt/v4 v4.5.0 // indirect github.com/google/go-tpm v0.3.3 // indirect github.com/kr/pretty v0.3.1 // indirect - github.com/labstack/gommon v0.4.0 // indirect + github.com/labstack/gommon v0.4.2 // indirect github.com/mattn/go-colorable v0.1.13 // indirect - github.com/mattn/go-isatty v0.0.18 // indirect + github.com/mattn/go-isatty v0.0.20 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect @@ -33,7 +33,7 @@ require ( github.com/x448/float16 v0.8.4 // indirect golang.org/x/sys v0.19.0 // indirect golang.org/x/text v0.14.0 // indirect - golang.org/x/time v0.3.0 // indirect + golang.org/x/time v0.5.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 510d1a0..412c64c 100644 --- a/go.sum +++ b/go.sum @@ -93,18 +93,16 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/labstack/echo/v4 v4.10.2 h1:n1jAhnq/elIFTHr1EYpiYtyKgx4RW9ccVgkqByZaN2M= -github.com/labstack/echo/v4 v4.10.2/go.mod h1:OEyqf2//K1DFdE57vw2DRgWY0M7s65IVQO2FzvI4J5k= -github.com/labstack/gommon v0.4.0 h1:y7cvthEAEbU0yHOf4axH8ZG2NH8knB9iNSoTO8dyIk8= -github.com/labstack/gommon v0.4.0/go.mod h1:uW6kP17uPlLJsD3ijUYn3/M5bAxtlZhMI6m3MFxTMTM= +github.com/labstack/echo/v4 v4.12.0 h1:IKpw49IMryVB2p1a4dzwlhP1O2Tf2E0Ir/450lH+kI0= +github.com/labstack/echo/v4 v4.12.0/go.mod h1:UP9Cr2DJXbOK3Kr9ONYzNowSh7HP0aG0ShAyycHSJvM= +github.com/labstack/gommon v0.4.2 h1:F8qTUNXgG1+6WQmqoUWnz8WiEU60mXVVw0P4ht1WRA0= +github.com/labstack/gommon v0.4.2/go.mod h1:QlUFxVM+SNXhDL/Z7YhocGIBYOiwB0mXm1+1bAPHPyU= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= -github.com/mattn/go-colorable v0.1.11/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= -github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= -github.com/mattn/go-isatty v0.0.18 h1:DOKFKCQ7FNG2L1rbrmstDN4QVRdS89Nkh85u68Uwp98= -github.com/mattn/go-isatty v0.0.18/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= +github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= @@ -150,21 +148,15 @@ github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DM github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/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/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -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.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= -github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +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/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= -github.com/valyala/fasttemplate v1.2.1/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ= github.com/valyala/fasttemplate v1.2.2 h1:lxLXG0uE3Qnshl9QyaK6XJxMXlQZELvChBOCmQD0Loo= github.com/valyala/fasttemplate v1.2.2/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= @@ -191,8 +183,6 @@ golang.org/x/net v0.0.0-20181220203305-927f97764cc3/go.mod h1:mL1N/T3taQHkDXs73r golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190522155817-f3200d17e092/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= -golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= -golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/net v0.24.0 h1:1PcaxkF854Fu3+lvBIx5SYn9wRlBzzcnHZSiaFFAb0w= golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -207,9 +197,6 @@ golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20210629170331-7dc0b73dc9fb/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20211103235746-7861aae1554b/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o= @@ -218,8 +205,8 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= -golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4= -golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= +golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk= +golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180221164845-07fd8470d635/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= @@ -252,8 +239,6 @@ gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/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.0-20210107192922-496545a6307b/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= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= From 70ae3aab32d293130ba19aeb02ec52bcabd15555 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 17 Apr 2024 15:40:12 +0200 Subject: [PATCH 22/28] Increase gracefult shutdown period A BankID auth order is only valid for 30 seconds, unless the QR-code is scanned, then the order is valid for 180 seconds. --- cmd/twoferd/main.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/twoferd/main.go b/cmd/twoferd/main.go index 62f3ef6..ea393bf 100644 --- a/cmd/twoferd/main.go +++ b/cmd/twoferd/main.go @@ -25,6 +25,8 @@ import ( "github.com/modfin/twofer/stream/sse" ) +const shutdownGracePeriod = time.Second * 200 // A BankID auth order is only valid for 30 seconds, unless it's scanned, then it's valid for 180 seconds. + func main() { cfg := config.Get() e := echo.New() @@ -111,7 +113,7 @@ func startServer(e *echo.Echo) { case <-appCtx.Done(): } - timeout, cancel := context.WithTimeout(context.Background(), time.Second*30) + timeout, cancel := context.WithTimeout(context.Background(), shutdownGracePeriod) defer cancel() err := e.Shutdown(timeout) From 45299d392450ab5d8149000f84b62c28cf95174d Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 17 Apr 2024 16:25:59 +0200 Subject: [PATCH 23/28] Send status=error when collect fail --- api/models.go | 7 +++++++ internal/httpserve/bankid.go | 1 + 2 files changed, 8 insertions(+) diff --git a/api/models.go b/api/models.go index 0875b99..ce6fb2b 100644 --- a/api/models.go +++ b/api/models.go @@ -32,3 +32,10 @@ type ( MRTD bool `json:"mrtd,omitempty"` } ) + +const ( + StatusPending = "pending" + StatusComplete = "complete" + StatusFailed = "failed" + StatusError = "error" +) diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index ef98485..148b9ef 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -313,6 +313,7 @@ func createResponseFromCollect(change bankid.Change) api.BankIdV6Response { func createResponseFromError(orderRef string, err error) api.BankIdV6Response { return api.BankIdV6Response{ OrderRef: orderRef, + Status: api.StatusError, CollectError: err.Error(), } } From 1315d7c30de2e4f8a07f721ec942f1344b26d004 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 17 Apr 2024 16:48:33 +0200 Subject: [PATCH 24/28] Add two logging TODO's --- internal/bankid/endpoints.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/bankid/endpoints.go b/internal/bankid/endpoints.go index c4171fa..3e96b71 100644 --- a/internal/bankid/endpoints.go +++ b/internal/bankid/endpoints.go @@ -172,12 +172,14 @@ func (a *API) WatchForChangeV2(ctx context.Context, orderRef string) (<-chan Cha select { case watch <- Change{Err: err}: case <-time.After(time.Second): + // TODO: Log that we have skipped to send 'data' (channel timeout) } } sendChange := func(change CollectResponse) { select { case watch <- Change{CollectResponse: change}: case <-time.After(time.Second): + // TODO: Log that we have skipped to send 'data' (channel timeout) } } From 6d90c650d6db2e26d5d2c2739732cf7c8619cb7b Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Wed, 17 Apr 2024 17:23:42 +0200 Subject: [PATCH 25/28] In case of a BankIdError, send back ErrorCode as a separate field to caller --- api/models.go | 1 + internal/httpserve/bankid.go | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/api/models.go b/api/models.go index ce6fb2b..a865b97 100644 --- a/api/models.go +++ b/api/models.go @@ -4,6 +4,7 @@ type ( BankIdV6Response struct { OrderRef string `json:"orderRef"` CollectError string `json:"error,omitempty"` + ErrorCode string `json:"errorCode,omitempty"` URI string `json:"uri,omitempty"` QR string `json:"qr,omitempty"` Status string `json:"status,omitempty"` diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index 148b9ef..17d6c5c 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -3,6 +3,7 @@ package httpserve import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -311,6 +312,15 @@ func createResponseFromCollect(change bankid.Change) api.BankIdV6Response { } func createResponseFromError(orderRef string, err error) api.BankIdV6Response { + var bie bankid.BankIdError + if errors.As(err, &bie) { + return api.BankIdV6Response{ + OrderRef: orderRef, + Status: api.StatusError, + CollectError: bie.Details, + ErrorCode: bie.ErrorCode, + } + } return api.BankIdV6Response{ OrderRef: orderRef, Status: api.StatusError, From 9069b1c985699d1882e730b089e90a48d33c6e94 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Thu, 18 Apr 2024 08:08:36 +0200 Subject: [PATCH 26/28] Rename 'CollectError' to 'ErrorText' in the public API --- api/models.go | 2 +- internal/httpserve/bankid.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/models.go b/api/models.go index a865b97..e4b4311 100644 --- a/api/models.go +++ b/api/models.go @@ -3,8 +3,8 @@ package api type ( BankIdV6Response struct { OrderRef string `json:"orderRef"` - CollectError string `json:"error,omitempty"` ErrorCode string `json:"errorCode,omitempty"` + ErrorText string `json:"errorText,omitempty"` URI string `json:"uri,omitempty"` QR string `json:"qr,omitempty"` Status string `json:"status,omitempty"` diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index 17d6c5c..8d4c8db 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -317,14 +317,14 @@ func createResponseFromError(orderRef string, err error) api.BankIdV6Response { return api.BankIdV6Response{ OrderRef: orderRef, Status: api.StatusError, - CollectError: bie.Details, ErrorCode: bie.ErrorCode, + ErrorText: bie.Details, } } return api.BankIdV6Response{ OrderRef: orderRef, Status: api.StatusError, - CollectError: err.Error(), + ErrorText: err.Error(), } } From 4b48dcbaeec040073682f6fb168e67df087cd018 Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Thu, 18 Apr 2024 18:05:44 +0200 Subject: [PATCH 27/28] Add prestophook command --- cmd/twoferd/main.go | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/cmd/twoferd/main.go b/cmd/twoferd/main.go index ea393bf..4ec224d 100644 --- a/cmd/twoferd/main.go +++ b/cmd/twoferd/main.go @@ -28,6 +28,11 @@ import ( const shutdownGracePeriod = time.Second * 200 // A BankID auth order is only valid for 30 seconds, unless it's scanned, then it's valid for 180 seconds. func main() { + if len(os.Args) > 1 && os.Args[1] == "prestophook" { + prestophook() + return + } + cfg := config.Get() e := echo.New() e.Use(middleware.Logger()) @@ -105,14 +110,17 @@ func startServer(e *echo.Echo) { }() signalChannel := make(chan os.Signal, 1) - signal.Notify(signalChannel, syscall.SIGTERM) + signal.Notify(signalChannel, syscall.SIGTERM, syscall.SIGINT) select { - case <-signalChannel: + case s := <-signalChannel: + fmt.Printf("twoferd received signal: %v\n", s) appClose() // Cancel 'app context' when we receive SIGTERM case <-appCtx.Done(): } + fmt.Println("Graceful shutdown initiated...") + timeout, cancel := context.WithTimeout(context.Background(), shutdownGracePeriod) defer cancel() @@ -120,6 +128,8 @@ func startServer(e *echo.Echo) { if err != nil { log.Fatalf("failure during Echo's shutdown: %v", err) } + + fmt.Println("twoferd stopped") } func startEid(e *echo.Echo) { @@ -170,3 +180,20 @@ func getStreamEncoder(encoder string) httpserve.NewStreamEncoder { return ndjson.NewEncoder } } + +func prestophook() { + fmt.Println("twofer - prestophook") + + // Give K8S time to remove POD from service before stutdown is started + time.Sleep(time.Second) + + // TODO: Check that twofer is actually PID 1 before we try to send signal? + err := syscall.Kill(1, syscall.SIGINT) + if err != nil { + fmt.Printf("prestophook: SIGINT error: %v", err) + return + } + + // Wait for graceful shutdown period to end. When PID 1 have exited, we'll be terminated as well + time.Sleep(shutdownGracePeriod) +} From 6d3441a99c94b83970407dc18226183feefc3fea Mon Sep 17 00:00:00 2001 From: Lars-Erik Svensson Date: Tue, 23 Apr 2024 06:40:37 +0200 Subject: [PATCH 28/28] Add a little logging --- internal/bankid/endpoints.go | 5 +++-- internal/httpserve/bankid.go | 16 +++++++++------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/internal/bankid/endpoints.go b/internal/bankid/endpoints.go index 3e96b71..206afb5 100644 --- a/internal/bankid/endpoints.go +++ b/internal/bankid/endpoints.go @@ -172,14 +172,14 @@ func (a *API) WatchForChangeV2(ctx context.Context, orderRef string) (<-chan Cha select { case watch <- Change{Err: err}: case <-time.After(time.Second): - // TODO: Log that we have skipped to send 'data' (channel timeout) + fmt.Printf("WatchForChangeV2 send timeout, failed to send: %v", err) } } sendChange := func(change CollectResponse) { select { case watch <- Change{CollectResponse: change}: case <-time.After(time.Second): - // TODO: Log that we have skipped to send 'data' (channel timeout) + fmt.Printf("WatchForChangeV2 send timeout, failed to send: %v", change) } } @@ -251,6 +251,7 @@ func post[Request any, Response any](ctx context.Context, client *http.Client, r } if res.StatusCode != 200 { + fmt.Printf("%s returned status code %d with data: %s\n", url, res.StatusCode, body) var bidError BankIdError err = json.Unmarshal(body, &bidError) if err == nil { diff --git a/internal/httpserve/bankid.go b/internal/httpserve/bankid.go index 8d4c8db..ef0387c 100644 --- a/internal/httpserve/bankid.go +++ b/internal/httpserve/bankid.go @@ -315,16 +315,16 @@ func createResponseFromError(orderRef string, err error) api.BankIdV6Response { var bie bankid.BankIdError if errors.As(err, &bie) { return api.BankIdV6Response{ - OrderRef: orderRef, - Status: api.StatusError, - ErrorCode: bie.ErrorCode, - ErrorText: bie.Details, + OrderRef: orderRef, + Status: api.StatusError, + ErrorCode: bie.ErrorCode, + ErrorText: bie.Details, } } return api.BankIdV6Response{ - OrderRef: orderRef, - Status: api.StatusError, - ErrorText: err.Error(), + OrderRef: orderRef, + Status: api.StatusError, + ErrorText: err.Error(), } } @@ -408,6 +408,7 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration, newStr if state.Err != nil { // Something failed, channel will close after this... + fmt.Printf("ERR: collect returned error: %v\n", state.Err) err = send(errorEvent, createResponseFromError(res.OrderRef, state.Err)) if err != nil { fmt.Printf("ERR: failed to send status update: %v\n", err) @@ -430,6 +431,7 @@ func authSign(authSign authSignFn, watch watchFn, qrPeriod time.Duration, newStr // Check for completion if state.Status != "" && state.Status != bankid.Pending { + fmt.Printf("reached status: %v\n", state.Status) return nil } }