code review fixes

This commit is contained in:
Krzysztof Dryś 2024-03-05 08:59:45 +01:00
parent e6b1c17809
commit 949b7594d8
11 changed files with 136 additions and 116 deletions

View File

@ -1,9 +1,12 @@
package config
import "time"
type PassConfig struct {
Addr string `envconfig:"PASS_ADDR" default:":8082"`
KMSKeyID string `envconfig:"KMS_KEY_ID" default:"alias/pass_service_signing_key"`
AWSEndpoint string `envconfig:"AWS_ENDPOINT" default:""`
AWSRegion string `envconfig:"AWS_REGION" default:"us-east-2"`
FakeMobilePush bool `envconfig:"FAKE_MOBILE_PUSH" default:"false"`
Addr string `envconfig:"PASS_ADDR" default:":8082"`
KMSKeyID string `envconfig:"KMS_KEY_ID" default:"alias/pass_service_signing_key"`
AWSEndpoint string `envconfig:"AWS_ENDPOINT" default:""`
AWSRegion string `envconfig:"AWS_REGION" default:"us-east-2"`
FakeMobilePush bool `envconfig:"FAKE_MOBILE_PUSH" default:"false"`
PairingRequestTokenValidityDuration time.Duration `envconfig:"PAIRING_REQUEST_TOKEN_VALIDITY_DURATION" default:"8765h"` // 1 year
}

View File

@ -45,7 +45,7 @@ type proxy struct {
conn *websocket.Conn
}
func StartProxy(wsConn *websocket.Conn, send, read chan []byte) {
func startProxy(wsConn *websocket.Conn, send, read chan []byte) {
proxy := &proxy{
send: send,
read: read,
@ -71,10 +71,10 @@ func StartProxy(wsConn *websocket.Conn, send, read chan []byte) {
})
}
// readPump pumps messages from the websocket connection to send.
// readPump pumps messages from the websocket proxy to send.
//
// The application runs readPump in a per-connection goroutine. The application
// ensures that there is at most one reader on a connection by executing all
// The application runs readPump in a per-proxy goroutine. The application
// ensures that there is at most one reader on a proxy by executing all
// reads from this goroutine.
func (p *proxy) readPump() {
defer func() {
@ -95,7 +95,7 @@ func (p *proxy) readPump() {
if websocket.IsUnexpectedCloseError(err, acceptedCloseStatus...) {
logging.WithFields(logging.Fields{
"reason": err.Error(),
}).Error("Websocket connection closed unexpected")
}).Error("Websocket proxy closed unexpected")
} else {
logging.WithFields(logging.Fields{
"reason": err.Error(),
@ -108,10 +108,10 @@ func (p *proxy) readPump() {
}
}
// writePump pumps messages from the read chan to the websocket connection.
// writePump pumps messages from the read chan to the websocket proxy.
//
// A goroutine running writePump is started for each connection. The
// application ensures that there is at most one writer to a connection by
// A goroutine running writePump is started for each proxy. The
// application ensures that there is at most one writer to a proxy by
// executing all writes from this goroutine.
func (p *proxy) writePump() {
ticker := time.NewTicker(pingPeriod)

View File

@ -1,35 +1,10 @@
package connection
import (
"fmt"
"net/http"
"sync"
"time"
"github.com/twofas/2fas-server/internal/common/logging"
)
// Proxy between Browser Extension and Mobile.
type Proxy struct {
proxyPool *proxyPool
idLabel string
}
func NewProxy(idLabel string) *Proxy {
proxyPool := &proxyPool{proxies: map[string]*proxyPair{}}
go func() {
ticker := time.NewTicker(30 * time.Second)
for {
<-ticker.C
proxyPool.deleteExpiresPairs()
}
}()
return &Proxy{
proxyPool: proxyPool,
idLabel: idLabel,
}
}
type proxyPool struct {
mu sync.Mutex
proxies map[string]*proxyPair
@ -73,31 +48,3 @@ func initProxyPair() *proxyPair {
expiresAt: time.Now().Add(proxyTimeout),
}
}
func (p *Proxy) ServeExtensionProxyToMobileWS(w http.ResponseWriter, r *http.Request, id string) error {
log := logging.WithField(p.idLabel, id)
conn, err := Upgrade(w, r)
if err != nil {
return fmt.Errorf("failed to upgrade connection: %w", err)
}
log.Infof("Starting ServeExtensionProxyToMobileWS")
proxyPair := p.proxyPool.getOrCreateProxyPair(id)
StartProxy(conn, proxyPair.toMobileDataCh, proxyPair.toExtensionDataCh)
return nil
}
func (p *Proxy) ServeMobileProxyToExtensionWS(w http.ResponseWriter, r *http.Request, id string) error {
conn, err := Upgrade(w, r)
if err != nil {
return fmt.Errorf("failed to upgrade connection: %w", err)
}
logging.Infof("Starting ServeMobileProxyToExtensionWS for dev: %v", id)
proxyPair := p.proxyPool.getOrCreateProxyPair(id)
StartProxy(conn, proxyPair.toExtensionDataCh, proxyPair.toMobileDataCh)
return nil
}

View File

@ -0,0 +1,58 @@
package connection
import (
"fmt"
"net/http"
"time"
"github.com/twofas/2fas-server/internal/common/logging"
)
// ProxyServer manages proxy connections between Browser Extension and Mobile.
type ProxyServer struct {
proxyPool *proxyPool
idLabel string
}
func NewProxyServer(idLabel string) *ProxyServer {
proxyPool := &proxyPool{proxies: map[string]*proxyPair{}}
go func() {
ticker := time.NewTicker(30 * time.Second)
for {
<-ticker.C
proxyPool.deleteExpiresPairs()
}
}()
return &ProxyServer{
proxyPool: proxyPool,
idLabel: idLabel,
}
}
func (p *ProxyServer) ServeExtensionProxyToMobileWS(w http.ResponseWriter, r *http.Request, id string) error {
log := logging.WithField(p.idLabel, id)
conn, err := Upgrade(w, r)
if err != nil {
return fmt.Errorf("failed to upgrade proxy: %w", err)
}
log.Infof("Starting ServeExtensionProxyToMobileWS")
proxyPair := p.proxyPool.getOrCreateProxyPair(id)
startProxy(conn, proxyPair.toMobileDataCh, proxyPair.toExtensionDataCh)
return nil
}
func (p *ProxyServer) ServeMobileProxyToExtensionWS(w http.ResponseWriter, r *http.Request, id string) error {
conn, err := Upgrade(w, r)
if err != nil {
return fmt.Errorf("failed to upgrade proxy: %w", err)
}
logging.Infof("Starting ServeMobileProxyToExtensionWS for dev: %v", id)
proxyPair := p.proxyPool.getOrCreateProxyPair(id)
startProxy(conn, proxyPair.toExtensionDataCh, proxyPair.toMobileDataCh)
return nil
}

View File

@ -98,7 +98,7 @@ func Upgrade(w http.ResponseWriter, req *http.Request) (*websocket.Conn, error)
}
conn, err := upgrader2pass.Upgrade(w, req, nil)
if err != nil {
return nil, fmt.Errorf("failed to upgrade connection: %w", err)
return nil, fmt.Errorf("failed to upgrade proxy: %w", err)
}
return conn, nil
}

View File

@ -39,7 +39,7 @@ func ExtensionWaitForConnWSHandler(pairingApp *Pairing) gin.HandlerFunc {
token, err := connection.TokenFromWSProtocol(gCtx.Request)
if err != nil {
logging.Errorf("Failed to get token from request: %v", err)
gCtx.Status(http.StatusForbidden)
gCtx.Status(http.StatusUnauthorized)
return
}
@ -58,7 +58,7 @@ func ExtensionWaitForConnWSHandler(pairingApp *Pairing) gin.HandlerFunc {
}
}
func ExtensionProxyWSHandler(pairingApp *Pairing, proxyApp *connection.Proxy) gin.HandlerFunc {
func ExtensionProxyWSHandler(pairingApp *Pairing, proxyApp *connection.ProxyServer) gin.HandlerFunc {
return func(gCtx *gin.Context) {
token, err := connection.TokenFromWSProtocol(gCtx.Request)
if err != nil {
@ -125,7 +125,7 @@ func MobileConfirmHandler(pairingApp *Pairing) gin.HandlerFunc {
}
}
func MobileProxyWSHandler(pairingApp *Pairing, proxy *connection.Proxy) gin.HandlerFunc {
func MobileProxyWSHandler(pairingApp *Pairing, proxy *connection.ProxyServer) gin.HandlerFunc {
return func(gCtx *gin.Context) {
token, err := connection.TokenFromWSProtocol(gCtx.Request)
if err != nil {

View File

@ -15,8 +15,9 @@ import (
)
type Pairing struct {
store store
signSvc *sign.Service
store store
signSvc *sign.Service
pairingRequestTokenValidityDuration time.Duration
}
type store interface {
@ -26,10 +27,11 @@ type store interface {
SetPairingInfo(ctx context.Context, extensionID string, pi PairingInfo) error
}
func NewPairingApp(signService *sign.Service) *Pairing {
func NewApp(signService *sign.Service, pairingRequestTokenValidityDuration time.Duration) *Pairing {
return &Pairing{
store: NewMemoryStore(),
signSvc: signService,
store: NewMemoryStore(),
signSvc: signService,
pairingRequestTokenValidityDuration: pairingRequestTokenValidityDuration,
}
}
@ -142,15 +144,19 @@ func (p *Pairing) sendTokenAndCloseConn(extID string, pairingInfo PairingInfo, c
ConnectionType: sign.ConnectionTypeBrowserExtensionProxy,
})
if err != nil {
return fmt.Errorf("Failed to generate ext proxy token: %v", err)
return fmt.Errorf("failed to generate ext proxy token: %v", err)
}
var syncToken string
if pairingInfo.Device.FCMToken != "" {
syncToken, err = p.signSvc.SignAndEncode(sign.Message{
ConnectionID: pairingInfo.Device.FCMToken,
ExpiresAt: time.Now().AddDate(1, 0, 0),
ExpiresAt: time.Now().Add(p.pairingRequestTokenValidityDuration),
ConnectionType: sign.ConnectionTypeBrowserExtensionSyncRequest,
})
if err != nil {
return fmt.Errorf("failed to generate proxy sync request token: %v", err)
}
}
if err := conn.WriteJSON(WaitForConnectionResponse{

View File

@ -51,11 +51,11 @@ func NewServer(cfg config.PassConfig) *Server {
log.Fatal(err)
}
pairingApp := pairing.NewPairingApp(signSvc)
proxyPairingApp := connection.NewProxy("fcm_token")
pairingApp := pairing.NewApp(signSvc, cfg.PairingRequestTokenValidityDuration)
proxyPairingApp := connection.NewProxyServer("device_id")
syncApp := sync.NewPairingApp(signSvc, cfg.FakeMobilePush)
proxySyncApp := connection.NewProxy("device_id")
syncApp := sync.NewApp(signSvc, cfg.FakeMobilePush)
proxySyncApp := connection.NewProxyServer("fcm_token")
router := gin.New()
router.Use(recovery.RecoveryMiddleware())
@ -68,11 +68,14 @@ func NewServer(cfg config.PassConfig) *Server {
context.Status(200)
})
router.POST("/browser_extension/configure", pairing.ExtensionConfigureHandler(pairingApp))
// Deprecated paths start here.
router.GET("/browser_extension/wait_for_connection", pairing.ExtensionWaitForConnWSHandler(pairingApp))
router.GET("/browser_extension/proxy_to_mobile", pairing.ExtensionProxyWSHandler(pairingApp, proxyPairingApp))
router.POST("/mobile/confirm", pairing.MobileConfirmHandler(pairingApp))
router.GET("/mobile/proxy_to_browser_extension", pairing.MobileProxyWSHandler(pairingApp, proxyPairingApp))
// Deprecated paths end here.
router.POST("/browser_extension/configure", pairing.ExtensionConfigureHandler(pairingApp))
router.GET("/browser_extension/pairing/wait", pairing.ExtensionWaitForConnWSHandler(pairingApp))
router.GET("/browser_extension/pairing/proxy", pairing.ExtensionProxyWSHandler(pairingApp, proxyPairingApp))

View File

@ -8,8 +8,8 @@ import (
)
// VerifyExtRequestSyncToken verifies sync request token and returns fcm_token.
func (p *Syncing) VerifyExtRequestSyncToken(ctx context.Context, proxyToken string) (string, error) {
fcmToken, err := p.signSvc.CanI(proxyToken, sign.ConnectionTypeBrowserExtensionSyncRequest)
func (s *Syncing) VerifyExtRequestSyncToken(ctx context.Context, proxyToken string) (string, error) {
fcmToken, err := s.signSvc.CanI(proxyToken, sign.ConnectionTypeBrowserExtensionSyncRequest)
if err != nil {
return "", fmt.Errorf("failed to check token signature: %w", err)
}
@ -17,8 +17,8 @@ func (p *Syncing) VerifyExtRequestSyncToken(ctx context.Context, proxyToken stri
}
// VerifyExtSyncToken verifies sync token and returns fcm_token.
func (p *Syncing) VerifyExtSyncToken(ctx context.Context, proxyToken string) (string, error) {
fcmToken, err := p.signSvc.CanI(proxyToken, sign.ConnectionTypeBrowserExtensionSync)
func (s *Syncing) VerifyExtSyncToken(ctx context.Context, proxyToken string) (string, error) {
fcmToken, err := s.signSvc.CanI(proxyToken, sign.ConnectionTypeBrowserExtensionSync)
if err != nil {
return "", fmt.Errorf("failed to check token signature: %w", err)
}
@ -26,8 +26,8 @@ func (p *Syncing) VerifyExtSyncToken(ctx context.Context, proxyToken string) (st
}
// VerifyMobileSyncConfirmToken verifies mobile token and returns connection id.
func (p *Syncing) VerifyMobileSyncConfirmToken(ctx context.Context, proxyToken string) (string, error) {
extensionID, err := p.signSvc.CanI(proxyToken, sign.ConnectionTypeMobileSyncConfirm)
func (s *Syncing) VerifyMobileSyncConfirmToken(ctx context.Context, proxyToken string) (string, error) {
extensionID, err := s.signSvc.CanI(proxyToken, sign.ConnectionTypeMobileSyncConfirm)
if err != nil {
return "", fmt.Errorf("failed to check token signature: %w", err)
}
@ -35,8 +35,8 @@ func (p *Syncing) VerifyMobileSyncConfirmToken(ctx context.Context, proxyToken s
}
// VerifyMobileSyncProxyToken verifies mobile token and returns connection id.
func (p *Syncing) VerifyMobileSyncProxyToken(ctx context.Context, proxyToken string) (string, error) {
extensionID, err := p.signSvc.CanI(proxyToken, sign.ConnectionTypeMobileSyncProxy)
func (s *Syncing) VerifyMobileSyncProxyToken(ctx context.Context, proxyToken string) (string, error) {
extensionID, err := s.signSvc.CanI(proxyToken, sign.ConnectionTypeMobileSyncProxy)
if err != nil {
return "", fmt.Errorf("failed to check token signature: %w", err)
}

View File

@ -34,7 +34,7 @@ func ExtensionRequestSync(syncingApp *Syncing) gin.HandlerFunc {
}
}
func ExtensionProxyWSHandler(syncingApp *Syncing, proxy *connection.Proxy) gin.HandlerFunc {
func ExtensionProxyWSHandler(syncingApp *Syncing, proxy *connection.ProxyServer) gin.HandlerFunc {
return func(gCtx *gin.Context) {
token, err := connection.TokenFromWSProtocol(gCtx.Request)
if err != nil {
@ -78,6 +78,7 @@ func MobileConfirmHandler(syncApp *Syncing) gin.HandlerFunc {
if err != nil {
if errors.Is(err, noSyncRequestErr) {
gCtx.String(http.StatusBadRequest, "no sync request was created for this token")
return
}
logging.Errorf("Failed to ConfirmPairing: %v", err)
gCtx.Status(http.StatusInternalServerError)
@ -87,7 +88,7 @@ func MobileConfirmHandler(syncApp *Syncing) gin.HandlerFunc {
}
}
func MobileProxyWSHandler(syncingApp *Syncing, proxy *connection.Proxy) gin.HandlerFunc {
func MobileProxyWSHandler(syncingApp *Syncing, proxy *connection.ProxyServer) gin.HandlerFunc {
return func(gCtx *gin.Context) {
token, err := connection.TokenFromWSProtocol(gCtx.Request)
if err != nil {

View File

@ -26,7 +26,7 @@ type store interface {
IsSyncCofirmed(fmtToken string) bool
}
func NewPairingApp(signService *sign.Service, fakeMobilePush bool) *Syncing {
func NewApp(signService *sign.Service, fakeMobilePush bool) *Syncing {
return &Syncing{
store: NewMemoryStore(),
signSvc: signService,
@ -34,7 +34,7 @@ func NewPairingApp(signService *sign.Service, fakeMobilePush bool) *Syncing {
}
const (
pairingTokenValidityDuration = 3 * time.Minute
syncTokenValidityDuration = 3 * time.Minute
)
type ConfigureBrowserExtensionRequest struct {
@ -60,7 +60,7 @@ type MobileSyncPayload struct {
MobileSyncToken string `json:"mobile_sync_token"`
}
func (p *Syncing) ServeSyncingRequestWS(w http.ResponseWriter, r *http.Request, fcmToken string) error {
func (s *Syncing) ServeSyncingRequestWS(w http.ResponseWriter, r *http.Request, fcmToken string) error {
log := logging.WithField("fcm_token", fcmToken)
conn, err := connection.Upgrade(w, r)
if err != nil {
@ -69,10 +69,10 @@ func (p *Syncing) ServeSyncingRequestWS(w http.ResponseWriter, r *http.Request,
defer conn.Close()
log.Infof("Starting sync request WS for %q", fcmToken)
p.requestSync(r.Context(), fcmToken)
s.requestSync(r.Context(), fcmToken)
if syncDone := p.isSyncConfirmed(r.Context(), fcmToken); syncDone {
if err := p.sendTokenAndCloseConn(fcmToken, conn); err != nil {
if syncDone := s.isSyncConfirmed(r.Context(), fcmToken); syncDone {
if err := s.sendTokenAndCloseConn(fcmToken, conn); err != nil {
log.Errorf("Failed to send token: %v", err)
}
log.Infof("Paring ws finished")
@ -92,8 +92,8 @@ func (p *Syncing) ServeSyncingRequestWS(w http.ResponseWriter, r *http.Request,
log.Info("Closing paring ws after timeout")
return nil
case <-connectedCheckTicker.C:
if syncConfirmed := p.isSyncConfirmed(r.Context(), fcmToken); syncConfirmed {
if err := p.sendTokenAndCloseConn(fcmToken, conn); err != nil {
if syncConfirmed := s.isSyncConfirmed(r.Context(), fcmToken); syncConfirmed {
if err := s.sendTokenAndCloseConn(fcmToken, conn); err != nil {
log.Errorf("Failed to send token: %v", err)
return nil
}
@ -104,18 +104,18 @@ func (p *Syncing) ServeSyncingRequestWS(w http.ResponseWriter, r *http.Request,
}
}
func (p *Syncing) isSyncConfirmed(ctx context.Context, fcmToken string) bool {
return p.store.IsSyncCofirmed(fcmToken)
func (s *Syncing) isSyncConfirmed(ctx context.Context, fcmToken string) bool {
return s.store.IsSyncCofirmed(fcmToken)
}
func (p *Syncing) requestSync(ctx context.Context, fcmToken string) {
p.store.RequestSync(fcmToken)
func (s *Syncing) requestSync(ctx context.Context, fcmToken string) {
s.store.RequestSync(fcmToken)
}
func (p *Syncing) sendTokenAndCloseConn(fcmToken string, conn *websocket.Conn) error {
extProxyToken, err := p.signSvc.SignAndEncode(sign.Message{
func (s *Syncing) sendTokenAndCloseConn(fcmToken string, conn *websocket.Conn) error {
extProxyToken, err := s.signSvc.SignAndEncode(sign.Message{
ConnectionID: fcmToken,
ExpiresAt: time.Now().Add(pairingTokenValidityDuration),
ExpiresAt: time.Now().Add(syncTokenValidityDuration),
ConnectionType: sign.ConnectionTypeBrowserExtensionSync,
})
if err != nil {
@ -131,10 +131,10 @@ func (p *Syncing) sendTokenAndCloseConn(fcmToken string, conn *websocket.Conn) e
return conn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""))
}
func (p *Syncing) sendMobileToken(fcmToken string, resp http.ResponseWriter) error {
extProxyToken, err := p.signSvc.SignAndEncode(sign.Message{
func (s *Syncing) sendMobileToken(fcmToken string, resp http.ResponseWriter) error {
extProxyToken, err := s.signSvc.SignAndEncode(sign.Message{
ConnectionID: fcmToken,
ExpiresAt: time.Now().Add(pairingTokenValidityDuration),
ExpiresAt: time.Now().Add(syncTokenValidityDuration),
ConnectionType: sign.ConnectionTypeMobileSyncConfirm,
})
if err != nil {
@ -147,9 +147,11 @@ func (p *Syncing) sendMobileToken(fcmToken string, resp http.ResponseWriter) err
MobileSyncConfirmToken: extProxyToken,
})
if err != nil {
return fmt.Errorf("failed to write to extension: %v", err)
return fmt.Errorf("failed to marshal the response: %v", err)
}
if _, err := resp.Write(bb); err != nil {
return fmt.Errorf("failed to write the response: %v", err)
}
resp.Write(bb)
return nil
}
@ -159,18 +161,18 @@ type ConfirmSyncResponse struct {
var noSyncRequestErr = errors.New("sync request was not created")
func (p *Syncing) confirmSync(ctx context.Context, fcmToken string) (ConfirmSyncResponse, error) {
func (s *Syncing) confirmSync(ctx context.Context, fcmToken string) (ConfirmSyncResponse, error) {
logging.Infof("Starting sync confirm for %q", fcmToken)
mobileProxyToken, err := p.signSvc.SignAndEncode(sign.Message{
mobileProxyToken, err := s.signSvc.SignAndEncode(sign.Message{
ConnectionID: fcmToken,
ExpiresAt: time.Now().Add(pairingTokenValidityDuration),
ExpiresAt: time.Now().Add(syncTokenValidityDuration),
ConnectionType: sign.ConnectionTypeMobileSyncConfirm,
})
if err != nil {
return ConfirmSyncResponse{}, fmt.Errorf("failed to generate ext proxy token: %v", err)
}
if ok := p.store.ConfirmSync(fcmToken); !ok {
if ok := s.store.ConfirmSync(fcmToken); !ok {
return ConfirmSyncResponse{}, noSyncRequestErr
}