mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-02 19:49:07 +02:00
[management,proxy] Rename the OIDC session code query parameter (#7981)
This commit is contained in:
@@ -125,9 +125,9 @@ func (h *AuthCallbackHandler) handleCallback(w http.ResponseWriter, r *http.Requ
|
||||
http.Error(w, "Failed to create session", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
query.Set("session_code", code)
|
||||
query.Set(auth.SessionCodeQueryParam, code)
|
||||
} else {
|
||||
query.Set("session_token", sessionToken)
|
||||
query.Set(auth.SessionTokenQueryParam, sessionToken)
|
||||
}
|
||||
redirectURL.RawQuery = query.Encode()
|
||||
|
||||
|
||||
@@ -532,8 +532,8 @@ func TestAuthCallback_UserAllowedToLogin(t *testing.T) {
|
||||
wantParam string
|
||||
absentParam string
|
||||
}{
|
||||
{name: "legacy proxy", manager: testSessionCodeManager{}, wantParam: "session_token", absentParam: "session_code"},
|
||||
{name: "compatible proxy", manager: testSessionCodeManager{supported: true}, wantParam: "session_code", absentParam: "session_token"},
|
||||
{name: "legacy proxy", manager: testSessionCodeManager{}, wantParam: "session_token", absentParam: "nb_session_code"},
|
||||
{name: "compatible proxy", manager: testSessionCodeManager{supported: true}, wantParam: "nb_session_code", absentParam: "session_token"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
@@ -555,8 +555,8 @@ func TestAuthCallback_UserAllowedToLogin(t *testing.T) {
|
||||
require.Empty(t, location.Query().Get(tt.absentParam))
|
||||
require.Empty(t, location.Query().Get("error"))
|
||||
|
||||
if tt.wantParam == "session_code" {
|
||||
code := location.Query().Get("session_code")
|
||||
if tt.wantParam == "nb_session_code" {
|
||||
code := location.Query().Get("nb_session_code")
|
||||
response, err := setup.proxyService.ValidateSession(context.Background(), &proto.ValidateSessionRequest{
|
||||
Domain: location.Hostname(),
|
||||
SessionCode: code,
|
||||
|
||||
@@ -30,6 +30,14 @@ const (
|
||||
SessionJWTIssuer = "netbird-management"
|
||||
)
|
||||
|
||||
// Query parameters management uses to hand the OIDC session to the proxy. The
|
||||
// proxy strips them before forwarding, so they must not collide with names the
|
||||
// proxied service uses itself.
|
||||
const (
|
||||
SessionCodeQueryParam = "nb_session_code"
|
||||
SessionTokenQueryParam = "session_token"
|
||||
)
|
||||
|
||||
// HeaderUserID is the synthetic user id recorded for header-authenticated
|
||||
// requests. Header auth validates a per-service secret and resolves no user
|
||||
// record, so proxy access logs and management-minted session tokens both
|
||||
|
||||
@@ -583,7 +583,7 @@ func (mw *Middleware) authenticateWithSchemes(w http.ResponseWriter, r *http.Req
|
||||
// handleAuthenticatedToken validates the token, handles denied access, and on
|
||||
// success sets a session cookie and redirects to the original URL.
|
||||
func (mw *Middleware) handleAuthenticatedToken(w http.ResponseWriter, r *http.Request, host, token string, config DomainConfig, scheme Scheme) {
|
||||
isCode := scheme.Type() == auth.MethodOIDC && r.URL.Query().Get("session_code") != ""
|
||||
isCode := scheme.Type() == auth.MethodOIDC && r.URL.Query().Get(auth.SessionCodeQueryParam) != ""
|
||||
result, err := mw.validateSessionToken(r.Context(), host, token, isCode, config.SessionPublicKey, scheme.Type())
|
||||
if err != nil {
|
||||
if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil {
|
||||
@@ -661,7 +661,7 @@ func wasCredentialSubmitted(r *http.Request, method auth.Method) bool {
|
||||
case auth.MethodPassword:
|
||||
return credentialFormValue(r, passwordFormId) != ""
|
||||
case auth.MethodOIDC:
|
||||
return r.URL.Query().Get("session_token") != "" || r.URL.Query().Get("session_code") != ""
|
||||
return r.URL.Query().Get(auth.SessionTokenQueryParam) != "" || r.URL.Query().Get(auth.SessionCodeQueryParam) != ""
|
||||
}
|
||||
return false
|
||||
}
|
||||
@@ -806,11 +806,11 @@ func sessionGroupsAllowed(allowed map[string]struct{}, method auth.Method, group
|
||||
// or history.
|
||||
func stripSessionTokenParam(u *url.URL) string {
|
||||
q := u.Query()
|
||||
if !q.Has("session_token") && !q.Has("session_code") {
|
||||
if !q.Has(auth.SessionTokenQueryParam) && !q.Has(auth.SessionCodeQueryParam) {
|
||||
return u.RequestURI()
|
||||
}
|
||||
q.Del("session_token")
|
||||
q.Del("session_code")
|
||||
q.Del(auth.SessionTokenQueryParam)
|
||||
q.Del(auth.SessionCodeQueryParam)
|
||||
clean := *u
|
||||
clean.RawQuery = q.Encode()
|
||||
return clean.RequestURI()
|
||||
|
||||
@@ -786,9 +786,15 @@ func TestWasCredentialSubmitted(t *testing.T) {
|
||||
{
|
||||
name: "OIDC code in query",
|
||||
method: auth.MethodOIDC,
|
||||
query: url.Values{"session_code": {"abc123"}},
|
||||
query: url.Values{"nb_session_code": {"abc123"}},
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "OIDC backend session_code in query",
|
||||
method: auth.MethodOIDC,
|
||||
query: url.Values{"session_code": {"abc123"}},
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "OIDC token not in query",
|
||||
method: auth.MethodOIDC,
|
||||
@@ -1585,8 +1591,9 @@ func TestStripSessionTokenParam(t *testing.T) {
|
||||
want string
|
||||
}{
|
||||
{"strips session_token", "https://ex.com/p?a=1&session_token=tok", "/p?a=1"},
|
||||
{"strips session_code", "https://ex.com/p?a=1&session_code=code", "/p?a=1"},
|
||||
{"strips both", "https://ex.com/p?session_token=tok&session_code=code&a=1", "/p?a=1"},
|
||||
{"strips nb_session_code", "https://ex.com/p?a=1&nb_session_code=code", "/p?a=1"},
|
||||
{"strips both", "https://ex.com/p?session_token=tok&nb_session_code=code&a=1", "/p?a=1"},
|
||||
{"keeps backend session_code", "https://ex.com/p?a=1&session_code=backend", "/p?a=1&session_code=backend"},
|
||||
{"no-op when absent", "https://ex.com/p?a=1", "/p?a=1"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
|
||||
@@ -43,12 +43,12 @@ func (o OIDC) Authenticate(r *http.Request) (string, string, error) {
|
||||
// Check for the session credential returned by the OIDC callback. The management
|
||||
// server passes it in the URL because it cannot set a cookie for the proxy's
|
||||
// domain (cookies are domain-scoped per RFC 6265). The current flow uses a
|
||||
// single-use session_code to keep the durable token out of the URL.
|
||||
// single-use session code to keep the durable token out of the URL.
|
||||
// session_token remains supported for backward compatibility.
|
||||
if code := r.URL.Query().Get("session_code"); code != "" {
|
||||
if code := r.URL.Query().Get(auth.SessionCodeQueryParam); code != "" {
|
||||
return code, "", nil
|
||||
}
|
||||
if token := r.URL.Query().Get("session_token"); token != "" {
|
||||
if token := r.URL.Query().Get(auth.SessionTokenQueryParam); token != "" {
|
||||
return token, "", nil
|
||||
}
|
||||
|
||||
|
||||
@@ -725,9 +725,9 @@ func stripSessionCookie(r *httputil.ProxyRequest) {
|
||||
// from the outgoing URL to prevent credential leakage to backends.
|
||||
func stripSessionTokenQuery(r *httputil.ProxyRequest) {
|
||||
q := r.Out.URL.Query()
|
||||
if q.Has("session_token") || q.Has("session_code") {
|
||||
q.Del("session_token")
|
||||
q.Del("session_code")
|
||||
if q.Has(auth.SessionTokenQueryParam) || q.Has(auth.SessionCodeQueryParam) {
|
||||
q.Del(auth.SessionTokenQueryParam)
|
||||
q.Del(auth.SessionCodeQueryParam)
|
||||
r.Out.URL.RawQuery = q.Encode()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -236,6 +236,17 @@ func TestRewriteFunc_SessionTokenQueryStripping(t *testing.T) {
|
||||
"other query parameters must be preserved")
|
||||
})
|
||||
|
||||
t.Run("strips nb_session_code query parameter", func(t *testing.T) {
|
||||
pr := newProxyRequest(t, "http://example.com/callback?nb_session_code=code123&other=keep", "1.2.3.4:5000")
|
||||
|
||||
rewrite(pr)
|
||||
|
||||
assert.Empty(t, pr.Out.URL.Query().Get("nb_session_code"),
|
||||
"OIDC session code must be stripped from backend request")
|
||||
assert.Equal(t, "keep", pr.Out.URL.Query().Get("other"),
|
||||
"other query parameters must be preserved")
|
||||
})
|
||||
|
||||
t.Run("preserves query when no session_token present", func(t *testing.T) {
|
||||
pr := newProxyRequest(t, "http://example.com/api?foo=bar&baz=qux", "1.2.3.4:5000")
|
||||
|
||||
|
||||
+4
-2
@@ -10,6 +10,8 @@ import (
|
||||
"net/url"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
|
||||
"github.com/netbirdio/netbird/proxy/auth"
|
||||
)
|
||||
|
||||
// PathPrefix is the unique URL prefix for serving the proxy's own web assets.
|
||||
@@ -180,8 +182,8 @@ func ServeAccessDeniedPage(w http.ResponseWriter, r *http.Request, code int, tit
|
||||
// stripAuthParams returns the request URI with auth-related query parameters removed.
|
||||
func stripAuthParams(u *url.URL) string {
|
||||
q := u.Query()
|
||||
q.Del("session_token")
|
||||
q.Del("session_code")
|
||||
q.Del(auth.SessionTokenQueryParam)
|
||||
q.Del(auth.SessionCodeQueryParam)
|
||||
q.Del("error")
|
||||
q.Del("error_description")
|
||||
clean := *u
|
||||
|
||||
Reference in New Issue
Block a user