mirror of
https://github.com/sourcegraph/sourcegraph.git
synced 2026-02-06 19:51:50 +00:00
Remove OverrideAuthMiddleware because it's unused (#43308)
I'm pretty sure we don't need this anymore and when in doubt I'd like to remove _overrides_ to our authentication. The instance at `sourcegraph.sgdev.org` that's mentioned in `package.json` doesn't exist anymore and I can't find other references to the env var or header anymore.
This commit is contained in:
Thorsten Ball
GitHub
parent
2cb1454aee
commit
cf7e5ea45d
@ -15,7 +15,6 @@
|
||||
"test:regression:integrations": "yarn task:mocha ./src/regression/integrations.test.ts",
|
||||
"test:regression:onboarding": "yarn task:mocha ./src/regression/onboarding.test.ts",
|
||||
"test:regression:search": "yarn task:mocha ./src/regression/search.test.ts",
|
||||
"test-e2e-sgdev": "yarn run -T cross-env SOURCEGRAPH_BASE_URL=https://sourcegraph.sgdev.org OVERRIDE_AUTH_SECRET=${SGDEV_OVERRIDE_AUTH_SECRET} yarn task:mocha ./src/end-to-end/end-to-end.test.ts",
|
||||
"storybook": "STORIES_GLOB='client/web/src/**/*.story.tsx' yarn workspace @sourcegraph/storybook run start",
|
||||
"serve:dev": "yarn run -T ts-node-transpile-only --project ./dev/tsconfig.json ./dev/server/development.server.ts",
|
||||
"serve:prod": "yarn run -T ts-node-transpile-only --project ./dev/tsconfig.json ./dev/server/production.server.ts",
|
||||
|
||||
@ -1,88 +0,0 @@
|
||||
package auth
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
|
||||
"github.com/inconshreveable/log15"
|
||||
|
||||
"github.com/sourcegraph/sourcegraph/cmd/frontend/auth"
|
||||
"github.com/sourcegraph/sourcegraph/cmd/frontend/internal/session"
|
||||
"github.com/sourcegraph/sourcegraph/internal/actor"
|
||||
"github.com/sourcegraph/sourcegraph/internal/database"
|
||||
"github.com/sourcegraph/sourcegraph/internal/env"
|
||||
"github.com/sourcegraph/sourcegraph/internal/extsvc"
|
||||
)
|
||||
|
||||
const (
|
||||
overrideSecretHeader = "X-Override-Auth-Secret"
|
||||
overrideUsernameHeader = "X-Override-Auth-Username"
|
||||
|
||||
defaultUsername = "override-auth-user"
|
||||
)
|
||||
|
||||
// OverrideAuthMiddleware is middleware that causes a new authenticated session (associated with a
|
||||
// new user named "anon-user") to be started if the client provides a secret header value specified
|
||||
// in site config.
|
||||
//
|
||||
// It is used to enable our e2e tests to authenticate to https://sourcegraph.sgdev.org without
|
||||
// needing to give them Google Workspace access.
|
||||
func OverrideAuthMiddleware(db database.DB, next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
secret := envOverrideAuthSecret
|
||||
// Accept both old header (X-Oidc-Override, deprecated) and new overrideSecretHeader for now.
|
||||
if secret != "" && (r.Header.Get("X-Oidc-Override") == secret || r.Header.Get(overrideSecretHeader) == secret) {
|
||||
username := r.Header.Get(overrideUsernameHeader)
|
||||
if username == "" {
|
||||
username = defaultUsername
|
||||
}
|
||||
|
||||
userID, safeErrMsg, err := auth.GetAndSaveUser(r.Context(), db, auth.GetAndSaveUserOp{
|
||||
UserProps: database.NewUser{
|
||||
Username: username,
|
||||
Email: username + "+override@example.com",
|
||||
EmailIsVerified: true,
|
||||
},
|
||||
ExternalAccount: extsvc.AccountSpec{
|
||||
ServiceType: "override",
|
||||
AccountID: username,
|
||||
},
|
||||
CreateIfNotExist: true,
|
||||
})
|
||||
if err != nil {
|
||||
log15.Error("Error getting/creating auth-override user.", "error", err, "userErr", safeErrMsg)
|
||||
http.Error(w, safeErrMsg, http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
// Make the user a site admin because that is more useful for e2e tests and local dev
|
||||
// scripting (which are the use cases of this override auth provider).
|
||||
if err := db.Users().SetIsSiteAdmin(r.Context(), userID, true); err != nil {
|
||||
log15.Error("Error setting auth-override user as site admin.", "error", err)
|
||||
http.Error(w, "", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
user, err := db.Users().GetByID(r.Context(), userID)
|
||||
if err != nil {
|
||||
log15.Error("Error retrieving user from database.", "error", err)
|
||||
http.Error(w, "", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
a := actor.FromUser(userID)
|
||||
if err := session.SetActor(w, r, a, 0, user.CreatedAt); err != nil {
|
||||
log15.Error("Error starting auth-override session.", "error", err)
|
||||
http.Error(w, "error starting auth-override session", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
r = r.WithContext(actor.WithActor(r.Context(), a))
|
||||
}
|
||||
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
|
||||
// envOverrideAuthSecret (the env var OVERRIDE_AUTH_SECRET) is the preferred source of the secret
|
||||
// for overriding auth.
|
||||
var envOverrideAuthSecret = env.Get("OVERRIDE_AUTH_SECRET", "", "X-Override-Auth-Secret HTTP request header value used to authenticate site-admin-authed sessions (use X-Override-Auth-Username header to set username)")
|
||||
@ -1,152 +0,0 @@
|
||||
package auth
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/sourcegraph/sourcegraph/cmd/frontend/auth"
|
||||
"github.com/sourcegraph/sourcegraph/cmd/frontend/internal/session"
|
||||
"github.com/sourcegraph/sourcegraph/internal/actor"
|
||||
"github.com/sourcegraph/sourcegraph/internal/database"
|
||||
"github.com/sourcegraph/sourcegraph/internal/types"
|
||||
"github.com/sourcegraph/sourcegraph/lib/errors"
|
||||
)
|
||||
|
||||
func TestOverrideAuthMiddleware(t *testing.T) {
|
||||
cleanup := session.ResetMockSessionStore(t)
|
||||
defer cleanup()
|
||||
|
||||
newHandler := func(db database.DB) http.Handler {
|
||||
return OverrideAuthMiddleware(db, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
actor := actor.FromContext(r.Context())
|
||||
if actor.IsAuthenticated() {
|
||||
fmt.Fprintf(w, "user %v", actor.UID)
|
||||
} else {
|
||||
fmt.Fprint(w, "no user")
|
||||
}
|
||||
}))
|
||||
}
|
||||
|
||||
const overrideSecret = "s"
|
||||
|
||||
t.Run("disabled, not sent", func(t *testing.T) {
|
||||
rr := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest("GET", "/", nil)
|
||||
handler := newHandler(database.NewMockDB())
|
||||
handler.ServeHTTP(rr, req)
|
||||
if got, want := rr.Body.String(), "no user"; got != want {
|
||||
t.Errorf("got %q, want %q", got, want)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("not sent", func(t *testing.T) {
|
||||
envOverrideAuthSecret = overrideSecret
|
||||
defer func() { envOverrideAuthSecret = "" }()
|
||||
rr := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest("GET", "/", nil)
|
||||
handler := newHandler(database.NewMockDB())
|
||||
handler.ServeHTTP(rr, req)
|
||||
if got, want := rr.Body.String(), "no user"; got != want {
|
||||
t.Errorf("got %q, want %q", got, want)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("not sent, actor present", func(t *testing.T) {
|
||||
envOverrideAuthSecret = overrideSecret
|
||||
defer func() { envOverrideAuthSecret = "" }()
|
||||
rr := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest("GET", "/", nil)
|
||||
req = req.WithContext(actor.WithActor(context.Background(), &actor.Actor{UID: 2}))
|
||||
handler := newHandler(database.NewMockDB())
|
||||
handler.ServeHTTP(rr, req)
|
||||
if got, want := rr.Body.String(), "user 2"; got != want {
|
||||
t.Errorf("got %q, want %q", got, want)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("sent, actor not set", func(t *testing.T) {
|
||||
envOverrideAuthSecret = overrideSecret
|
||||
defer func() { envOverrideAuthSecret = "" }()
|
||||
rr := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest("GET", "/", nil)
|
||||
req.Header.Set(overrideSecretHeader, overrideSecret)
|
||||
var calledMock bool
|
||||
auth.MockGetAndSaveUser = func(ctx context.Context, op auth.GetAndSaveUserOp) (userID int32, safeErrMsg string, err error) {
|
||||
calledMock = true
|
||||
if want := defaultUsername; op.UserProps.Username != want {
|
||||
t.Errorf("got %q, want %q", op.UserProps.Username, want)
|
||||
}
|
||||
return 1, "", nil
|
||||
}
|
||||
defer func() { auth.MockGetAndSaveUser = nil }()
|
||||
|
||||
users := database.NewMockUserStore()
|
||||
users.SetIsSiteAdminFunc.SetDefaultReturn(nil)
|
||||
users.GetByIDFunc.SetDefaultHook(func(_ context.Context, id int32) (*types.User, error) {
|
||||
return &types.User{ID: id, CreatedAt: time.Now()}, nil
|
||||
})
|
||||
db := database.NewMockDB()
|
||||
db.UsersFunc.SetDefaultReturn(users)
|
||||
|
||||
handler := newHandler(db)
|
||||
handler.ServeHTTP(rr, req)
|
||||
if got, want := rr.Body.String(), "user 1"; got != want {
|
||||
t.Errorf("got %q, want %q", got, want)
|
||||
}
|
||||
if !calledMock {
|
||||
t.Error("!calledMock")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("sent, actor already set", func(t *testing.T) {
|
||||
envOverrideAuthSecret = overrideSecret
|
||||
defer func() { envOverrideAuthSecret = "" }()
|
||||
rr := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest("GET", "/", nil)
|
||||
req.Header.Set(overrideSecretHeader, overrideSecret)
|
||||
req = req.WithContext(actor.WithActor(context.Background(), &actor.Actor{UID: 123}))
|
||||
var calledMock bool
|
||||
auth.MockGetAndSaveUser = func(ctx context.Context, op auth.GetAndSaveUserOp) (userID int32, safeErrMsg string, err error) {
|
||||
calledMock = true
|
||||
if op.ExternalAccount.ServiceType == "override" && op.ExternalAccount.AccountID == defaultUsername {
|
||||
return 1, "", nil
|
||||
}
|
||||
return 0, "safeErr", errors.New("x")
|
||||
}
|
||||
defer func() { auth.MockGetAndSaveUser = nil }()
|
||||
|
||||
users := database.NewMockUserStore()
|
||||
users.SetIsSiteAdminFunc.SetDefaultReturn(nil)
|
||||
users.GetByIDFunc.SetDefaultHook(func(_ context.Context, id int32) (*types.User, error) {
|
||||
return &types.User{ID: id, CreatedAt: time.Now()}, nil
|
||||
})
|
||||
db := database.NewMockDB()
|
||||
db.UsersFunc.SetDefaultReturn(users)
|
||||
|
||||
handler := newHandler(db)
|
||||
handler.ServeHTTP(rr, req)
|
||||
if got, want := rr.Body.String(), "user 1"; got != want {
|
||||
t.Errorf("got %q, want %q", got, want)
|
||||
}
|
||||
if !calledMock {
|
||||
t.Error("!calledMock")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("sent, wrong secret", func(t *testing.T) {
|
||||
envOverrideAuthSecret = overrideSecret
|
||||
defer func() { envOverrideAuthSecret = "" }()
|
||||
rr := httptest.NewRecorder()
|
||||
req, _ := http.NewRequest("GET", "/", nil)
|
||||
req.Header.Set(overrideSecretHeader, "bad")
|
||||
handler := newHandler(database.NewMockDB())
|
||||
handler.ServeHTTP(rr, req)
|
||||
if got, want := rr.Body.String(), "no user"; got != want {
|
||||
t.Errorf("got %q, want %q", got, want)
|
||||
}
|
||||
})
|
||||
}
|
||||
@ -108,16 +108,12 @@ func newExternalHTTPHandler(
|
||||
// Wrap in middleware, first line is last to run.
|
||||
//
|
||||
// 🚨 SECURITY: Auth middleware that must run before other auth middlewares.
|
||||
// OverrideAuthMiddleware allows us to inject an authentication token via an
|
||||
// environment variable, for testing. This is true only when a site-config
|
||||
// change is explicitly made, to enable this token.
|
||||
h = middleware.Trace(h)
|
||||
h = gcontext.ClearHandler(h)
|
||||
h = healthCheckMiddleware(h)
|
||||
h = middleware.BlackHole(h)
|
||||
h = middleware.SourcegraphComGoGetHandler(h)
|
||||
h = internalauth.ForbidAllRequestsMiddleware(h)
|
||||
h = internalauth.OverrideAuthMiddleware(db, h)
|
||||
h = tracepkg.HTTPMiddleware(logger, h, conf.DefaultClient())
|
||||
h = instrumentation.HTTPMiddleware("external", h)
|
||||
|
||||
|
||||
@ -66,7 +66,6 @@ env:
|
||||
{ "Name": "zoekt-web-0", "Host": "127.0.0.1:3070", "DefaultPath": "/debug/requests/" },
|
||||
{ "Name": "zoekt-web-1", "Host": "127.0.0.1:3071", "DefaultPath": "/debug/requests/" }
|
||||
]
|
||||
OVERRIDE_AUTH_SECRET: sSsNGlI8fBDftBz0LDQNXEnP6lrWdt9g0fK6hoFvGQ
|
||||
# Settings/config
|
||||
SITE_CONFIG_FILE: ./dev/site-config.json
|
||||
SITE_CONFIG_ALLOW_EDITS: true
|
||||
|
||||
Loading…
Reference in New Issue
Block a user