Add access request enabled check in backend redirect (#62472)

* add accessrequest check in backend

* update tests

* remove debug

* remove unused test

* update comment
This commit is contained in:
Gabe Torres 2024-05-07 09:58:41 -07:00 committed by GitHub
parent 992930f830
commit 99f49a46c9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 105 additions and 31 deletions

View File

@ -230,23 +230,6 @@ describe('SignInPage', () => {
expect(render('/sign-in', { authenticatedUser: mockUser }).asFragment()).toMatchSnapshot()
})
it('renders redirect when there is only 1 auth provider', () => {
const withGitHubProvider: SourcegraphContext['authProviders'] = [
{
serviceType: 'github',
displayName: 'GitHub',
isBuiltin: false,
authenticationURL: 'http://localhost/.auth/gitlab/login?pc=f00bar&returnTo=%2Fsearch',
serviceID: 'https://github.com',
clientID: '1234',
noSignIn: false,
requiredForAuthz: false,
},
]
expect(render('/sign-in', { authProviders: withGitHubProvider }).asFragment()).toMatchSnapshot()
})
it('does not render redirect when there is only 1 auth provider with request access enabled', () => {
const withGitHubProvider: SourcegraphContext['authProviders'] = [
{

View File

@ -87,7 +87,7 @@ export const SignInPage: React.FunctionComponent<React.PropsWithChildren<SignInP
const thirdPartyAuthProviders = nonBuiltinAuthProviders.filter(provider => shouldShowProvider(provider))
// If there is only one auth provider that is going to be displayed, and request access is disabled, we want to redirect to the auth provider directly.
if (thirdPartyAuthProviders.length === 1 && !builtInAuthProvider && !isRequestAccessAllowed) {
if (context.sourcegraphDotComMode && thirdPartyAuthProviders.length === 1) {
// Add '?returnTo=' + encodeURIComponent(returnTo) to thirdPartyAuthProviders[0].authenticationURL in a safe way.
const redirectUrl = new URL(thirdPartyAuthProviders[0].authenticationURL, window.location.href)
if (returnTo) {

View File

@ -3,6 +3,7 @@ package bitbucketcloudoauth
import (
"bytes"
"context"
"github.com/sourcegraph/sourcegraph/internal/conf"
"io"
"net/http"
"net/http/httptest"
@ -62,14 +63,24 @@ func TestMiddleware(t *testing.T) {
return respRecorder.Result()
}
t.Run("unauthenticated homepage visit, sign-out cookie present -> sg sign-in", func(t *testing.T) {
t.Run("unauthenticated homepage visit, sign-out cookie present, access requests enabled -> sg sign-in", func(t *testing.T) {
cookie := &http.Cookie{Name: auth.SignOutCookie, Value: "true"}
resp := doRequest("GET", "http://example.com/", "", "", []*http.Cookie{cookie}, false)
if want := http.StatusOK; resp.StatusCode != want {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)
}
})
t.Run("unauthenticated homepage visit, no sign-out cookie -> bitbucket cloud oauth flow", func(t *testing.T) {
t.Run("unauthenticated homepage visit, no sign-out cookie, access requests disabled -> bitbucket cloud oauth flow", func(t *testing.T) {
falseVal := false
conf.Mock(&conf.Unified{
SiteConfiguration: schema.SiteConfiguration{
AuthAccessRequest: &schema.AuthAccessRequest{
Enabled: &falseVal,
},
},
})
t.Cleanup(func() { conf.Mock(nil) })
resp := doRequest("GET", "http://example.com/", "", "", nil, false)
if want := http.StatusFound; resp.StatusCode != want {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)
@ -85,7 +96,17 @@ func TestMiddleware(t *testing.T) {
t.Errorf("got return-to URL %v, want %v", got, want)
}
})
t.Run("unauthenticated subpage visit -> bitbucket cloud oauth flow", func(t *testing.T) {
t.Run("unauthenticated subpage visit, access requests disabled -> bitbucket cloud oauth flow", func(t *testing.T) {
falseVal := false
conf.Mock(&conf.Unified{
SiteConfiguration: schema.SiteConfiguration{
AuthAccessRequest: &schema.AuthAccessRequest{
Enabled: &falseVal,
},
},
})
t.Cleanup(func() { conf.Mock(nil) })
resp := doRequest("GET", "http://example.com/page", "", "", nil, false)
if want := http.StatusFound; resp.StatusCode != want {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)

View File

@ -3,6 +3,7 @@ package githuboauth
import (
"bytes"
"context"
"github.com/sourcegraph/sourcegraph/internal/conf"
"io"
"net/http"
"net/http/httptest"
@ -64,7 +65,24 @@ func TestMiddleware(t *testing.T) {
return respRecorder.Result()
}
t.Run("unauthenticated homepage visit, sign-out cookie present -> sg sign-in", func(t *testing.T) {
t.Run("unauthenticated homepage visit, sign-out cookie present, access requests enabled -> sg sign-in", func(t *testing.T) {
cookie := &http.Cookie{Name: auth.SignOutCookie, Value: "true"}
resp := doRequest("GET", "http://example.com/", "", "", []*http.Cookie{cookie}, false)
if want := http.StatusOK; resp.StatusCode != want {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)
}
})
t.Run("unauthenticated homepage visit, sign-out cookie present, access requests disabled -> sg sign-in", func(t *testing.T) {
falseVal := false
conf.Mock(&conf.Unified{
SiteConfiguration: schema.SiteConfiguration{
AuthAccessRequest: &schema.AuthAccessRequest{
Enabled: &falseVal,
},
},
})
t.Cleanup(func() { conf.Mock(nil) })
cookie := &http.Cookie{Name: auth.SignOutCookie, Value: "true"}
resp := doRequest("GET", "http://example.com/", "", "", []*http.Cookie{cookie}, false)
@ -72,7 +90,17 @@ func TestMiddleware(t *testing.T) {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)
}
})
t.Run("unauthenticated homepage visit, no sign-out cookie -> github oauth flow", func(t *testing.T) {
t.Run("unauthenticated homepage visit, no sign-out cookie, access requests disabled -> github oauth flow", func(t *testing.T) {
falseVal := false
conf.Mock(&conf.Unified{
SiteConfiguration: schema.SiteConfiguration{
AuthAccessRequest: &schema.AuthAccessRequest{
Enabled: &falseVal,
},
},
})
t.Cleanup(func() { conf.Mock(nil) })
resp := doRequest("GET", "http://example.com/", "", "", nil, false)
if want := http.StatusFound; resp.StatusCode != want {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)
@ -88,7 +116,22 @@ func TestMiddleware(t *testing.T) {
t.Errorf("got return-to URL %v, want %v", got, want)
}
})
t.Run("unauthenticated subpage visit -> github oauth flow", func(t *testing.T) {
t.Run("unauthenticated homepage visit, no sign-out cookie, access requests enabled -> sg sign-in", func(t *testing.T) {
resp := doRequest("GET", "http://example.com/", "", "", nil, false)
if want := http.StatusOK; resp.StatusCode != want {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)
}
})
t.Run("unauthenticated subpage visit, access requests disabled -> github oauth flow", func(t *testing.T) {
falseVal := false
conf.Mock(&conf.Unified{
SiteConfiguration: schema.SiteConfiguration{
AuthAccessRequest: &schema.AuthAccessRequest{
Enabled: &falseVal,
},
},
})
t.Cleanup(func() { conf.Mock(nil) })
resp := doRequest("GET", "http://example.com/page", "", "", nil, false)
if want := http.StatusFound; resp.StatusCode != want {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)
@ -105,6 +148,12 @@ func TestMiddleware(t *testing.T) {
t.Errorf("got return-to URL %v, want %v", got, want)
}
})
t.Run("unauthenticated subpage visit, access requests enabled -> sg sign-in", func(t *testing.T) {
resp := doRequest("GET", "http://example.com/page", "", "", nil, false)
if want := http.StatusOK; resp.StatusCode != want {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)
}
})
// Add 2 GitHub auth providers
providers.MockProviders = []providers.Provider{mockGHE.Provider, mockGitHubCom.Provider}

View File

@ -3,6 +3,7 @@ package gitlaboauth
import (
"bytes"
"context"
"github.com/sourcegraph/sourcegraph/internal/conf"
"io"
"net/http"
"net/http/httptest"
@ -67,7 +68,17 @@ func TestMiddleware(t *testing.T) {
authedHandler.ServeHTTP(respRecorder, req)
return respRecorder.Result()
}
t.Run("unauthenticated homepage visit, no sign-out cookie -> gitlab oauth flow", func(t *testing.T) {
t.Run("unauthenticated homepage visit, no sign-out cookie, access requests disabled -> gitlab oauth flow", func(t *testing.T) {
falseVal := false
conf.Mock(&conf.Unified{
SiteConfiguration: schema.SiteConfiguration{
AuthAccessRequest: &schema.AuthAccessRequest{
Enabled: &falseVal,
},
},
})
t.Cleanup(func() { conf.Mock(nil) })
resp := doRequest("GET", "http://example.com/", "", "", nil, false)
if want := http.StatusFound; resp.StatusCode != want {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)
@ -83,7 +94,7 @@ func TestMiddleware(t *testing.T) {
t.Errorf("got return-to URL %v, want %v", got, want)
}
})
t.Run("unauthenticated homepage visit, sign-out cookie present -> sg sign-in", func(t *testing.T) {
t.Run("unauthenticated homepage visit, sign-out cookie present, access requests enabled -> sg sign-in", func(t *testing.T) {
cookie := &http.Cookie{Name: auth.SignOutCookie, Value: "true"}
resp := doRequest("GET", "http://example.com/", "", "", []*http.Cookie{cookie}, false)
@ -91,7 +102,17 @@ func TestMiddleware(t *testing.T) {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)
}
})
t.Run("unauthenticated subpage visit -> gitlab oauth flow", func(t *testing.T) {
t.Run("unauthenticated subpage visit, access requests disabled -> gitlab oauth flow", func(t *testing.T) {
falseVal := false
conf.Mock(&conf.Unified{
SiteConfiguration: schema.SiteConfiguration{
AuthAccessRequest: &schema.AuthAccessRequest{
Enabled: &falseVal,
},
},
})
t.Cleanup(func() { conf.Mock(nil) })
resp := doRequest("GET", "http://example.com/page", "", "", nil, false)
if want := http.StatusFound; resp.StatusCode != want {
t.Errorf("got response code %v, want %v", resp.StatusCode, want)

View File

@ -19,6 +19,7 @@ import (
"github.com/sourcegraph/sourcegraph/cmd/frontend/auth"
"github.com/sourcegraph/sourcegraph/internal/actor"
"github.com/sourcegraph/sourcegraph/internal/auth/providers"
"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/env"
"github.com/sourcegraph/sourcegraph/internal/httpcli"
@ -56,11 +57,11 @@ func NewMiddleware(db database.DB, serviceType, authPrefix string, isAPIHandler
}
// If there is only one auth provider configured, the single auth provider is a OAuth
// instance, it's an app request, and the sign-out cookie is not present, redirect to sign-in immediately.
// instance, it's an app request, the sign-out cookie is not present, and access requests are disabled, redirect to sign-in immediately.
//
// For sign-out requests (signout cookie is present), the user will be redirected to the SG login page.
// For sign-out requests (sign-out cookie is present), the user will be redirected to the SG login page.
pc := getExactlyOneOAuthProvider()
if pc != nil && !isAPIHandler && pc.AuthPrefix == authPrefix && !auth.HasSignOutCookie(r) && isHuman(r) {
if pc != nil && !isAPIHandler && pc.AuthPrefix == authPrefix && !auth.HasSignOutCookie(r) && isHuman(r) && !conf.IsAccessRequestEnabled() {
span.AddEvent("redirect to signin")
v := make(url.Values)
v.Set("redirect", auth.SafeRedirectURL(r.URL.String()))
@ -70,7 +71,6 @@ func NewMiddleware(db database.DB, serviceType, authPrefix string, isAPIHandler
return
}
span.AddEvent("proceeding to next")
span.End()
next.ServeHTTP(w, r)