From cd26cbf150ef8b359ab964326f5ad092599748a7 Mon Sep 17 00:00:00 2001 From: Andrew Martinez Date: Fri, 24 Jan 2025 10:49:58 -0500 Subject: [PATCH] fixes #2689 form data doesn't respect query string authRequestID --- controller/oidc_auth/login.go | 3 +- controller/oidc_auth/parse.go | 9 +++-- go.sum | 4 -- tests/auth_oidc_test.go | 74 ++++++++++++++++++++++++++++++++++- 4 files changed, 81 insertions(+), 9 deletions(-) diff --git a/controller/oidc_auth/login.go b/controller/oidc_auth/login.go index 344e48950..f7053715b 100644 --- a/controller/oidc_auth/login.go +++ b/controller/oidc_auth/login.go @@ -22,7 +22,8 @@ import ( ) const ( - queryAuthRequestID = "authRequestID" + queryAuthRequestID = "authRequestID" + queryAuthRequestIdAlt = "id" //page specific resource keys pageLogin = "login" diff --git a/controller/oidc_auth/parse.go b/controller/oidc_auth/parse.go index dbba45b2b..36fd8c9c8 100644 --- a/controller/oidc_auth/parse.go +++ b/controller/oidc_auth/parse.go @@ -192,8 +192,6 @@ func parsePayload(r *http.Request, out AuthRequestIdHolder) error { return err } - return nil - } else if contentType == JsonContentType { body, err := io.ReadAll(r.Body) @@ -216,8 +214,13 @@ func parsePayload(r *http.Request, out AuthRequestIdHolder) error { } } + //prefer body, if not set use > query string queryAuthRequestID > query string queryAuthRequestIdAlt if out.GetAuthRequestId() == "" { - out.SetAuthRequestId(r.URL.Query().Get("id")) + if queryAuthRequestId := r.URL.Query().Get(queryAuthRequestID); queryAuthRequestId != "" { + out.SetAuthRequestId(queryAuthRequestId) + } else if queryId := r.URL.Query().Get(queryAuthRequestIdAlt); queryId != "" { + out.SetAuthRequestId(queryId) + } } return nil diff --git a/go.sum b/go.sum index 3e11361b3..632cf5bcd 100644 --- a/go.sum +++ b/go.sum @@ -595,10 +595,6 @@ github.com/openziti/cobra-to-md v1.0.1 h1:WRinNoIRmwWUSJm+pSNXMjOrtU48oxXDZgeCYQ github.com/openziti/cobra-to-md v1.0.1/go.mod h1:FjCpk/yzHF7/r28oSTNr5P57yN5VolpdAtS/g7KNi2c= github.com/openziti/dilithium v0.3.5 h1:+envGNzxc3OyVPiuvtxivQmCsOjdZjtOMLpQBeMz7eM= github.com/openziti/dilithium v0.3.5/go.mod h1:XONq1iK6te/WwNzkgZHfIDHordMPqb0hMwJ8bs9EfSk= -github.com/openziti/edge-api v0.26.36 h1:zy2DjmIz/B+WxPpIzhFOAxi/LhM/yeKa8s1Vz2h8cQk= -github.com/openziti/edge-api v0.26.36/go.mod h1:sYHVpm26Jr1u7VooNJzTb2b2nGSlmCHMnbGC8XfWSng= -github.com/openziti/edge-api v0.26.37 h1:MQGDQusrya6U5IEW/t6x6IRzBqzBVFF00YNcuPMNtnY= -github.com/openziti/edge-api v0.26.37/go.mod h1:sYHVpm26Jr1u7VooNJzTb2b2nGSlmCHMnbGC8XfWSng= github.com/openziti/edge-api v0.26.38 h1:3xDWC5SFn3qUVR428TIBpRc2lrjVV7Gz0Rx4pQx0JSg= github.com/openziti/edge-api v0.26.38/go.mod h1:sYHVpm26Jr1u7VooNJzTb2b2nGSlmCHMnbGC8XfWSng= github.com/openziti/foundation/v2 v2.0.56 h1:YXqBmkrN0fYr3TqIlWZSZGluE2QpJxlA29Z6okZyQ5I= diff --git a/tests/auth_oidc_test.go b/tests/auth_oidc_test.go index 82ac74b52..35341b693 100644 --- a/tests/auth_oidc_test.go +++ b/tests/auth_oidc_test.go @@ -165,7 +165,7 @@ func Test_Authenticate_OIDC_Auth(t *testing.T) { ctx.Req.Equal(http.StatusUnsupportedMediaType, resp.StatusCode()) }) - t.Run("updb", func(t *testing.T) { + t.Run("updb with auth request id in body", func(t *testing.T) { ctx.testContextChanged(t) client := resty.NewWithClient(ctx.NewHttpClient(ctx.NewTransport())) @@ -201,6 +201,78 @@ func Test_Authenticate_OIDC_Auth(t *testing.T) { ctx.Req.NotEmpty(outTokens.RefreshToken) }) + t.Run("updb with auth request id in query string", func(t *testing.T) { + ctx.testContextChanged(t) + + client := resty.NewWithClient(ctx.NewHttpClient(ctx.NewTransport())) + client.SetRedirectPolicy(resty.DomainCheckRedirectPolicy("127.0.0.1", "localhost")) + resp, err := client.R().Get(rpServer.LoginUri) + + ctx.Req.NoError(err) + ctx.Req.Equal(http.StatusOK, resp.StatusCode()) + + authRequestId := resp.Header().Get(oidc_auth.AuthRequestIdHeader) + ctx.Req.NotEmpty(authRequestId) + + opLoginUri := "https://" + resp.RawResponse.Request.URL.Host + "/oidc/login/username?authRequestID=" + authRequestId + + resp, err = client.R().SetFormData(map[string]string{"username": ctx.AdminAuthenticator.Username, "password": ctx.AdminAuthenticator.Password}).Post(opLoginUri) + + ctx.Req.NoError(err) + ctx.Req.Equal(http.StatusOK, resp.StatusCode()) + + var outTokens *oidc.Tokens[*oidc.IDTokenClaims] + + select { + case tokens := <-rpServer.TokenChan: + outTokens = tokens + case <-time.After(5 * time.Second): + ctx.Fail("no tokens received, hit timeout") + } + + ctx.Req.NotNil(outTokens) + ctx.Req.NotEmpty(outTokens.IDToken) + ctx.Req.NotEmpty(outTokens.IDTokenClaims) + ctx.Req.NotEmpty(outTokens.AccessToken) + ctx.Req.NotEmpty(outTokens.RefreshToken) + }) + + t.Run("updb with id in query string", func(t *testing.T) { + ctx.testContextChanged(t) + + client := resty.NewWithClient(ctx.NewHttpClient(ctx.NewTransport())) + client.SetRedirectPolicy(resty.DomainCheckRedirectPolicy("127.0.0.1", "localhost")) + resp, err := client.R().Get(rpServer.LoginUri) + + ctx.Req.NoError(err) + ctx.Req.Equal(http.StatusOK, resp.StatusCode()) + + authRequestId := resp.Header().Get(oidc_auth.AuthRequestIdHeader) + ctx.Req.NotEmpty(authRequestId) + + opLoginUri := "https://" + resp.RawResponse.Request.URL.Host + "/oidc/login/username?id=" + authRequestId + + resp, err = client.R().SetFormData(map[string]string{"username": ctx.AdminAuthenticator.Username, "password": ctx.AdminAuthenticator.Password}).Post(opLoginUri) + + ctx.Req.NoError(err) + ctx.Req.Equal(http.StatusOK, resp.StatusCode()) + + var outTokens *oidc.Tokens[*oidc.IDTokenClaims] + + select { + case tokens := <-rpServer.TokenChan: + outTokens = tokens + case <-time.After(5 * time.Second): + ctx.Fail("no tokens received, hit timeout") + } + + ctx.Req.NotNil(outTokens) + ctx.Req.NotEmpty(outTokens.IDToken) + ctx.Req.NotEmpty(outTokens.IDTokenClaims) + ctx.Req.NotEmpty(outTokens.AccessToken) + ctx.Req.NotEmpty(outTokens.RefreshToken) + }) + t.Run("cert", func(t *testing.T) { ctx.testContextChanged(t)