From bd6b83236ab71043bcd6ccc37154fe909ade6d9c Mon Sep 17 00:00:00 2001 From: Corbadoman <100508310+corbadoman@users.noreply.github.com> Date: Mon, 30 Sep 2024 07:06:05 +0200 Subject: [PATCH 1/3] Optimized issue validation --- internal/services/session/session.go | 36 +++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/internal/services/session/session.go b/internal/services/session/session.go index aaebe81..88226f7 100644 --- a/internal/services/session/session.go +++ b/internal/services/session/session.go @@ -121,15 +121,12 @@ func (i *Impl) ValidateToken(shortSession string) (*entities.User, error) { } } - return nil, validationerror.New(err.Error(), code) + return nil, newValidationError(err.Error(), shortSession, code) } claims := token.Claims.(*entities.Claims) - if claims.Issuer != i.Config.JWTIssuer { - return nil, validationerror.New( - fmt.Sprintf("JWT issuer mismatch (configured: '%s', actual JWT: '%s')", i.Config.JWTIssuer, claims.Issuer), - validationerror.CodeJWTIssuerMismatch, - ) + if err := i.validateIssuer(claims.Issuer, shortSession); err != nil { + return nil, err } return &entities.User{ @@ -137,3 +134,30 @@ func (i *Impl) ValidateToken(shortSession string) (*entities.User, error) { FullName: claims.Name, }, nil } + +func (i *Impl) validateIssuer(jwtIssuer string, shortSession string) error { + // Compare to old Frontend API (without .cloud.) to make our Frontend API host name change downwards compatible + if jwtIssuer == fmt.Sprintf("https://%s.frontendapi.corbado.io", i.Config.ProjectID) { + return nil + } + + // Compare to new Frontend API (with .cloud.) + if jwtIssuer == fmt.Sprintf("https://%s.frontendapi.cloud.corbado.io", i.Config.ProjectID) { + return nil + } + + // Compare to configured issuer (from FrontendAPI), needed if you set a CNAME for example + if jwtIssuer != i.Config.JWTIssuer { + return newValidationError( + fmt.Sprintf("JWT issuer mismatch (configured trough FrontendAPI: '%s', JWT issuer: '%s')", i.Config.JWTIssuer, jwtIssuer), + shortSession, + validationerror.CodeJWTIssuerMismatch, + ) + } + + return nil +} + +func newValidationError(message string, jwt string, code validationerror.Code) error { + return validationerror.New(fmt.Sprintf("JWT validation failed: '%s' (JWT: '%s')", message, jwt), code) +} From b1d188d1e7194ecb9318a551e40b8d458753f128 Mon Sep 17 00:00:00 2001 From: Corbadoman <100508310+corbadoman@users.noreply.github.com> Date: Mon, 30 Sep 2024 09:04:44 +0200 Subject: [PATCH 2/3] Updated session unit tests --- tests/unit/session/session_test.go | 57 +++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/tests/unit/session/session_test.go b/tests/unit/session/session_test.go index e49badb..ce2c9dd 100644 --- a/tests/unit/session/session_test.go +++ b/tests/unit/session/session_test.go @@ -62,7 +62,7 @@ func generatePrivateKey(filename string) (*rsa.PrivateKey, error) { } // newSession mocks the JWKS endpoint and creates a new session service -func newSession() (*session.Impl, error) { +func newSession(issuer string) (*session.Impl, error) { workingDir, err := os.Getwd() if err != nil { return nil, err @@ -89,12 +89,9 @@ func newSession() (*session.Impl, error) { // Config config := &session.Config{ - ProjectID: "test-project-id", - JwksURI: "http://localhost:8081", - JWTIssuer: "https://auth.acme.com", - JWKSRefreshInterval: 0, - JWKSRefreshRateLimit: 0, - JWKSRefreshTimeout: 0, + ProjectID: "pro-1", + JwksURI: "http://localhost:8081", + JWTIssuer: issuer, } // Create a new JWKS instance using the mock JWKS server @@ -149,6 +146,7 @@ func TestValidateToken(t *testing.T) { tests := []struct { name string + issuer string shortSession string validationErrorCode validationerror.Code success bool @@ -160,43 +158,68 @@ func TestValidateToken(t *testing.T) { }, { name: "JWT with invalid format", + issuer: "https://pro-1.frontendapi.cloud.corbado.io", shortSession: "invalid", validationErrorCode: validationerror.CodeJWTInvalidData, success: false, }, { - name: "JWT with invalid signature", - // nolint:lll - shortSession: "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6ImtpZDEyMyJ9.eyJpc3MiOiJodHRwczovL2F1dGguYWNtZS5jb20iLCJpYXQiOjE3MjY0OTE4MDcsImV4cCI6MTcyNjQ5MTkwNywibmJmIjoxNzI2NDkxNzA3LCJzdWIiOiJ1c3ItMTIzNDU2Nzg5MCIsIm5hbWUiOiJuYW1lIiwiZW1haWwiOiJlbWFpbCIsInBob25lX251bWJlciI6InBob25lTnVtYmVyIiwib3JpZyI6Im9yaWcifQ.invalid", + name: "JWT with invalid signature", + issuer: "https://pro-1.frontendapi.cloud.corbado.io", + shortSession: "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6ImtpZDEyMyJ9.eyJpc3MiOiJodHRwczovL2F1dGguYWNtZS5jb20iLCJpYXQiOjE3MjY0OTE4MDcsImV4cCI6MTcyNjQ5MTkwNywibmJmIjoxNzI2NDkxNzA3LCJzdWIiOiJ1c3ItMTIzNDU2Nzg5MCIsIm5hbWUiOiJuYW1lIiwiZW1haWwiOiJlbWFpbCIsInBob25lX251bWJlciI6InBob25lTnVtYmVyIiwib3JpZyI6Im9yaWcifQ.invalid", // nolint:lll validationErrorCode: validationerror.CodeJWTInvalidSignature, success: false, }, { name: "JWT with invalid private key signed", - shortSession: generateJWT("https://auth.acme.com", time.Now().Add(100*time.Second).Unix(), time.Now().Unix(), invalidPrivateKey), + issuer: "https://pro-1.frontendapi.cloud.corbado.io", + shortSession: generateJWT("https://pro-1.frontendapi.cloud.corbado.io", time.Now().Add(100*time.Second).Unix(), time.Now().Unix(), invalidPrivateKey), validationErrorCode: validationerror.CodeJWTInvalidSignature, success: false, }, { name: "Not before (nbf) in future", - shortSession: generateJWT("https://auth.acme.com", time.Now().Add(100*time.Second).Unix(), time.Now().Add(100*time.Second).Unix(), validPrivateKey), + issuer: "https://pro-1.frontendapi.cloud.corbado.io", + shortSession: generateJWT("https://pro-1.frontendapi.cloud.corbado.io", time.Now().Add(100*time.Second).Unix(), time.Now().Add(100*time.Second).Unix(), validPrivateKey), validationErrorCode: validationerror.CodeJWTBefore, success: false, }, { name: "Expired (exp)", - shortSession: generateJWT("https://auth.acme.com", time.Now().Add(-100*time.Second).Unix(), time.Now().Add(-100*time.Second).Unix(), validPrivateKey), + issuer: "https://pro-1.frontendapi.cloud.corbado.io", + shortSession: generateJWT("https://pro-1.frontendapi.cloud.corbado.io", time.Now().Add(-100*time.Second).Unix(), time.Now().Add(-100*time.Second).Unix(), validPrivateKey), validationErrorCode: validationerror.CodeJWTExpired, success: false, }, { - name: "Invalid issuer (iss)", - shortSession: generateJWT("https://invalid.com", time.Now().Add(100*time.Second).Unix(), time.Now().Unix(), validPrivateKey), + name: "Invalid issuer 1 (iss)", + issuer: "https://pro-1.frontendapi.corbado.io", + shortSession: generateJWT("https://pro-2.frontendapi.cloud.corbado.io", time.Now().Add(100*time.Second).Unix(), time.Now().Unix(), validPrivateKey), validationErrorCode: validationerror.CodeJWTIssuerMismatch, success: false, }, { - name: "Success", + name: "Invalid issuer 1 (iss)", + issuer: "https://pro-1.frontendapi.cloud.corbado.io", + shortSession: generateJWT("https://pro-2.frontendapi.corbado.io", time.Now().Add(100*time.Second).Unix(), time.Now().Unix(), validPrivateKey), + validationErrorCode: validationerror.CodeJWTIssuerMismatch, + success: false, + }, + { + name: "Success with old Frontend API URL in JWT", + issuer: "https://pro-1.frontendapi.cloud.corbado.io", + shortSession: generateJWT("https://pro-1.frontendapi.corbado.io", time.Now().Add(100*time.Second).Unix(), time.Now().Unix(), validPrivateKey), + success: true, + }, + { + name: "Success with old Frontend API URL in config", + issuer: "https://pro-1.frontendapi.corbado.io", + shortSession: generateJWT("https://pro-1.frontendapi.cloud.corbado.io", time.Now().Add(100*time.Second).Unix(), time.Now().Unix(), validPrivateKey), + success: true, + }, + { + name: "Success with CNAME", + issuer: "https://auth.acme.com", shortSession: generateJWT("https://auth.acme.com", time.Now().Add(100*time.Second).Unix(), time.Now().Unix(), validPrivateKey), success: true, }, @@ -204,7 +227,7 @@ func TestValidateToken(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - sessionSvc, err := newSession() + sessionSvc, err := newSession(test.issuer) require.NoError(t, err) user, err := sessionSvc.ValidateToken(test.shortSession) From 4efd23284cfed86e9ff2f594628565793af24e05 Mon Sep 17 00:00:00 2001 From: Corbadoman <100508310+corbadoman@users.noreply.github.com> Date: Mon, 30 Sep 2024 10:07:35 +0200 Subject: [PATCH 3/3] Fixed linter error --- tests/unit/session/session_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/session/session_test.go b/tests/unit/session/session_test.go index ce2c9dd..e7e1378 100644 --- a/tests/unit/session/session_test.go +++ b/tests/unit/session/session_test.go @@ -137,6 +137,7 @@ func newSession(issuer string) (*session.Impl, error) { }, nil } +// nolint:funlen func TestValidateToken(t *testing.T) { validPrivateKey, err := generatePrivateKey("validPrivateKey.pem") require.NoError(t, err)