Skip to content

Commit dbd4ade

Browse files
authored
fix(server): ignore Basic auth header when AUTH_ENABLED=true (#9147)
OAuth2ProxyAuthentication turns any "Authorization: Basic user:pass" header into a user without checking the password. That was fine while nginx in config-ui verified the credentials against ADMIN_USER/ADMIN_PASS before proxying, and the lake only kept the username for audit fields. #8854 put auth.RequireAuth behind it. With AUTH_ENABLED=true that gate now accepts the unverified Basic user, so any username:password pair reaches every non-public endpoint (blueprints, connections, pipelines) on a lake that is exposed directly. #8880 closed the same hole for X-Forwarded-User with FORWARDED_USER_SECRET but left the Basic branch untouched. Only consult the Basic header when AUTH_ENABLED is off, which is the legacy nginx mode. With AUTH_ENABLED the lake authenticates callers itself: session cookie, API key, or forwarded headers with the shared secret, exactly what auth.go already logs as the supported set. Upstream: not yet submitted.
1 parent e744aef commit dbd4ade

3 files changed

Lines changed: 73 additions & 3 deletions

File tree

‎backend/server/api/middlewares.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,15 +93,22 @@ func getBasicAuthUserInfo(c *gin.Context, basicRes context.BasicRes) (*common.Us
9393

9494
func OAuth2ProxyAuthentication(basicRes context.BasicRes) gin.HandlerFunc {
9595
logger := basicRes.GetLogger()
96-
forwardedUserSecret := strings.TrimSpace(basicRes.GetConfigReader().GetString("FORWARDED_USER_SECRET"))
96+
cfg := basicRes.GetConfigReader()
97+
forwardedUserSecret := strings.TrimSpace(cfg.GetString("FORWARDED_USER_SECRET"))
98+
// A Basic header is only an identity nginx (config-ui) has already verified
99+
// against ADMIN_USER/ADMIN_PASS; the lake never checks the password. With
100+
// AUTH_ENABLED the lake is the authenticator (session cookie, API key, or
101+
// forwarded headers with the shared secret), so an unverified Basic header
102+
// must not become a user, or any username:password passes RequireAuth.
103+
trustBasicAuth := !(cfg.IsSet("AUTH_ENABLED") && cfg.GetBool("AUTH_ENABLED"))
97104
return func(c *gin.Context) {
98105
_, exist := c.Get(common.USER)
99106
if !exist {
100107
user, err := getOAuthUserInfo(c, forwardedUserSecret)
101108
if err != nil {
102109
logger.Warn(err, "rejected forwarded user headers")
103110
}
104-
if user == nil || user.Name == "" {
111+
if (user == nil || user.Name == "") && trustBasicAuth {
105112
// fetch with basic auth header
106113
user, err = getBasicAuthUserInfo(c, basicRes)
107114
if err != nil {
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package api
19+
20+
import (
21+
"encoding/base64"
22+
"testing"
23+
24+
"github.com/spf13/viper"
25+
)
26+
27+
func basicAuthHeader(username, password string) map[string]string {
28+
return map[string]string{
29+
"Authorization": "Basic " + base64.StdEncoding.EncodeToString([]byte(username+":"+password)),
30+
}
31+
}
32+
33+
func TestOAuth2ProxyAuthenticationIgnoresBasicAuthWhenAuthEnabled(t *testing.T) {
34+
cfg := viper.New()
35+
cfg.Set("AUTH_ENABLED", true)
36+
router := newProxyAuthRouterWithConfig(cfg)
37+
body := performProxyAuthRequest(t, router, basicAuthHeader("anyone", "anything"))
38+
if body.Authenticated {
39+
t.Fatalf("expected Basic auth header to be ignored with AUTH_ENABLED=true, got %+v", body)
40+
}
41+
}
42+
43+
func TestOAuth2ProxyAuthenticationTrustsBasicAuthWhenAuthDisabled(t *testing.T) {
44+
cases := map[string]func(*viper.Viper){
45+
"AUTH_ENABLED unset": func(*viper.Viper) {},
46+
"AUTH_ENABLED explicit": func(cfg *viper.Viper) { cfg.Set("AUTH_ENABLED", false) },
47+
}
48+
for name, configure := range cases {
49+
t.Run(name, func(t *testing.T) {
50+
cfg := viper.New()
51+
configure(cfg)
52+
router := newProxyAuthRouterWithConfig(cfg)
53+
body := performProxyAuthRequest(t, router, basicAuthHeader("admin", "secret"))
54+
if !body.Authenticated || body.Name != "admin" {
55+
t.Fatalf("expected Basic auth user to be kept without AUTH_ENABLED, got %+v", body)
56+
}
57+
})
58+
}
59+
}

‎backend/server/api/middlewares_forwardsecret_test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,13 @@ type proxyAuthResponse struct {
5757
}
5858

5959
func newProxyAuthRouter(secret string) *gin.Engine {
60-
gin.SetMode(gin.TestMode)
6160
cfg := viper.New()
6261
cfg.Set("FORWARDED_USER_SECRET", secret)
62+
return newProxyAuthRouterWithConfig(cfg)
63+
}
64+
65+
func newProxyAuthRouterWithConfig(cfg *viper.Viper) *gin.Engine {
66+
gin.SetMode(gin.TestMode)
6367
basicRes := &proxyAuthTestBasicRes{
6468
cfg: cfg,
6569
logger: logruslog.Global,

0 commit comments

Comments
 (0)