Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimizing changes with upstream #56

Merged
merged 10 commits into from
Dec 4, 2024
7 changes: 4 additions & 3 deletions pkg/apis/options/legacy_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,10 @@ type LegacyHeaders struct {
PassUserHeaders bool `flag:"pass-user-headers" cfg:"pass_user_headers"`
PassAuthorization bool `flag:"pass-authorization-header" cfg:"pass_authorization_header"`

SetBasicAuth bool `flag:"set-basic-auth" cfg:"set_basic_auth"`
SetXAuthRequest bool `flag:"set-xauthrequest" cfg:"set_xauthrequest"`
SetAuthorization bool `flag:"set-authorization-header" cfg:"set_authorization_header"`
SetBasicAuth bool `flag:"set-basic-auth" cfg:"set_basic_auth"`
SetXAuthRequest bool `flag:"set-xauthrequest" cfg:"set_xauthrequest"`
SetAuthorization bool `flag:"set-authorization-header" cfg:"set_authorization_header"`

SetIntrospectionValue bool `flag:"set-introspection-value" cfg:"set_introspection_value"`

PreferEmailToUser bool `flag:"prefer-email-to-user" cfg:"prefer_email_to_user"`
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/options/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ sub:
} else {
input = &TestOptions{}
}

err := LoadYAML(configFileName, input)
if in.expectedErr != nil {
Expect(err).To(MatchError(in.expectedErr.Error()))
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/options/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ type Provider struct {
AllowedGroups []string `json:"allowedGroups,omitempty"`
// The code challenge method
CodeChallengeMethod string `json:"code_challenge_method,omitempty"`

// URL to call to perform backend logout, `{id_token}` would be replaced by the actual `id_token` if available in the session
BackendLogoutURL string `json:"backendLogoutURL"`
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/apis/sessions/session_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ type SessionState struct {
IntrospectClaims string `msgpack:"ic,omitempty"`

// Internal helpers, not serialized
Clock clock.Clock `msgpack:"-"`
Lock Lock `msgpack:"-"`
SessionJustRefreshed bool `msgpack:"-"`
Clock clock.Clock `msgpack:"-"`
Lock Lock `msgpack:"-"`

SessionJustRefreshed bool `msgpack:"-"`
}

func (s *SessionState) ObtainLock(ctx context.Context, expiration time.Duration) error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/app/pagewriter/error.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no">
<title>{{.StatusCode}} {{.Title}}</title>
<link rel="stylesheet" href="{{.ProxyPrefix}}/static/css/bulma.min.css">
<link rel="stylesheet" href="{{.ProxyPrefix}}/static/css/all.min.css">
<link rel="stylesheet" href="{{.ProxyPrefix}}/static/css/bulma.min.css">
<link rel="stylesheet" href="{{.ProxyPrefix}}/static/css/all.min.css">

<script type="text/javascript">
document.addEventListener('DOMContentLoaded', function() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/app/pagewriter/sign_in.html
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
{{ else }}
{{.StatusCode}}: Invalid Username or Password
{{ end }}
</div>
</div>
{{ end }}

</div>
Expand Down
4 changes: 2 additions & 2 deletions pkg/cookies/cookies.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, opts *o
SameSite: ParseSameSite(opts.SameSite),
}

warnInvalidDomain(c, req)

if expiration != time.Duration(0) {
c.Expires = now.Add(expiration)
}

warnInvalidDomain(c, req)

return c
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/cookies/csrf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ var _ = Describe("CSRF Cookie Tests", func() {
Domains: []string{cookieDomain},
Path: cookiePath,
Expire: time.Hour,
CSRFExpire: time.Hour,
Secure: true,
HTTPOnly: true,
CSRFPerRequest: false,
CSRFExpire: time.Hour,
}

var err error
Expand Down
3 changes: 2 additions & 1 deletion pkg/encryption/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ func Validate(cookie *http.Cookie, seed string, expiration time.Duration) (value
// creation timestamp stored in the cookie falls within the
// window defined by (Now()-expiration, Now()].
t = time.Unix(int64(ts), 0)
if (expiration == time.Duration(0)) || t.After(time.Now().Add(expiration*-1)) && t.Before(time.Now().Add(time.Minute*5)) { // it's a valid cookie. now get the contents
if (expiration == time.Duration(0)) || (t.After(time.Now().Add(expiration*-1)) && t.Before(time.Now().Add(time.Minute*5))) {
// it's a valid cookie. now get the contents
rawValue, err := base64.URLEncoding.DecodeString(parts[0])
if err == nil {
value = rawValue
Expand Down
34 changes: 17 additions & 17 deletions pkg/encryption/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,23 +106,6 @@ func TestSignAndValidate(t *testing.T) {
assert.False(t, checkSignature(sha1sig, seed, key, "tampered", epoch))
}

func TestGenerateRandomASCIIString(t *testing.T) {
randomString, err := GenerateRandomASCIIString(96)
assert.NoError(t, err)

// Only 8-bit characters
assert.Equal(t, 96, len([]byte(randomString)))

// All non-ascii characters removed should still be the original string
removedChars := strings.Map(func(r rune) rune {
if r > unicode.MaxASCII {
return -1
}
return r
}, randomString)
assert.Equal(t, removedChars, randomString)
}

func TestValidate(t *testing.T) {
seed := "0123456789abcdef"
key := "cookie-name"
Expand All @@ -146,3 +129,20 @@ func TestValidate(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, validValue, expectedValue)
}

func TestGenerateRandomASCIIString(t *testing.T) {
randomString, err := GenerateRandomASCIIString(96)
assert.NoError(t, err)

// Only 8-bit characters
assert.Equal(t, 96, len([]byte(randomString)))

// All non-ascii characters removed should still be the original string
removedChars := strings.Map(func(r rune) rune {
if r > unicode.MaxASCII {
return -1
}
return r
}, randomString)
assert.Equal(t, removedChars, randomString)
}
18 changes: 10 additions & 8 deletions pkg/providers/oidc/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ type providerJSON struct {
// Endpoints represents the endpoints discovered as part of the OIDC discovery process
// that will be used by the authentication providers.
type Endpoints struct {
AuthURL string
TokenURL string
JWKsURL string
UserInfoURL string
AuthURL string
TokenURL string
JWKsURL string
UserInfoURL string

IntrospectEndpoint string
}

Expand Down Expand Up @@ -91,10 +92,11 @@ type discoveryProvider struct {
// Endpoints returns the discovered endpoints needed for an authentication provider.
func (p *discoveryProvider) Endpoints() Endpoints {
return Endpoints{
AuthURL: p.authURL,
TokenURL: p.tokenURL,
JWKsURL: p.jwksURL,
UserInfoURL: p.userInfoURL,
AuthURL: p.authURL,
TokenURL: p.tokenURL,
JWKsURL: p.jwksURL,
UserInfoURL: p.userInfoURL,

IntrospectEndpoint: p.introspectEndpoint,
}
}
Expand Down
1 change: 0 additions & 1 deletion pkg/sessions/persistence/ticket.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ func decodeTicket(encTicket string, cookieOpts *options.Cookie) (*ticket, error)
if errSecret != nil {
return nil, fmt.Errorf("failed to decode ticket: %v", errSecret)
}

return &ticket{
id: ticketID,
secret: secret,
Expand Down
1 change: 1 addition & 0 deletions pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ func GetCertPool(paths []string, useSystemPool bool) (*x509.CertPool, error) {
if len(paths) == 0 {
return nil, fmt.Errorf("invalid empty list of Root CAs file paths")
}

var pool *x509.CertPool
if useSystemPool {
rootPool, err := getSystemCertPool()
Expand Down
3 changes: 2 additions & 1 deletion pkg/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ mmu20Q5sWqggb0WNk1GoHIlpiObgY38KfnNSZ5Ra/QuXLF3QM2NdK5bT1iNFfPG3
fQklEouz/2lqZkRhsKSeXFKQ+h8GtNevKwFg6y3wWvH62WF5mTe9eyUE9VWl64y6
js5ESoVXA+e+QfsMsJrI5XfLV1O8ZxXKAVrYxBnC+WQbrNOjI7VBkjcn/QDmDjBw
sC1lo4YZwxEQ/bE0kEWI7PT/Skml4bTLw0jsgXNV9Nd8
-----END CERTIFICATE-----
-----END CERTIFICATE-----
`
cert1CertSubj = "CN=oauth2-proxy test cert1 ca"
cert1Cert = `-----BEGIN CERTIFICATE-----
Expand Down Expand Up @@ -217,6 +217,7 @@ func TestGetCertPool(t *testing.T) {

certFile1 := makeTestCertFile(t, root1Cert, tempDir)
certFile2 := makeTestCertFile(t, root2Cert, tempDir)

for _, tc := range tests {
// Append certs to "known" pool so we can compare them
assert.True(t, tc.pool.AppendCertsFromPEM([]byte(root1Cert)))
Expand Down
1 change: 0 additions & 1 deletion pkg/validation/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ func Validate(o *options.Options) error {

var redirectURL *url.URL
redirectURL, msgs = parseURL(o.RawRedirectURL, "redirect", msgs)

o.SetRedirectURL(redirectURL)
if o.RawRedirectURL == "" && !o.Cookie.Secure && !o.ReverseProxy {
logger.Print("WARNING: no explicit redirect URL: redirects will default to insecure HTTP")
Expand Down
1 change: 1 addition & 0 deletions pkg/validation/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func validateGoogleConfig(provider options.Provider) []string {
if !hasGoogleGroups && !hasAdminEmail && !hasSAJSON && !useADC {
return msgs
}

if !hasGoogleGroups {
msgs = append(msgs, "missing setting: google-group")
}
Expand Down
1 change: 1 addition & 0 deletions providers/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"golang.org/x/exp/slices"

"github.com/bitly/go-simplejson"

"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
Expand Down
5 changes: 4 additions & 1 deletion providers/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,14 @@ func (p *GitHubProvider) hasRepoAccess(ctx context.Context, accessToken string)
Pull bool `json:"pull"`
Push bool `json:"push"`
}

type repository struct {
Permissions permissions `json:"permissions"`
Private bool `json:"private"`
}

endpoint := p.makeGitHubAPIEndpoint("/repos/"+p.Repo, nil)

var repo repository
err := requests.New(endpoint.String()).
WithContext(ctx).
Expand Down Expand Up @@ -309,6 +312,7 @@ func (p *GitHubProvider) getEmail(ctx context.Context, s *sessions.SessionState)
}

endpoint := p.makeGitHubAPIEndpoint("/user/emails", nil)

err := requests.New(endpoint.String()).
WithContext(ctx).
WithHeaders(makeGitHubHeader(s.AccessToken)).
Expand Down Expand Up @@ -359,7 +363,6 @@ func (p *GitHubProvider) getUser(ctx context.Context, s *sessions.SessionState)
return nil
}

// isVerifiedUser
func (p *GitHubProvider) isVerifiedUser(username string) bool {
for _, u := range p.Users {
if username == u {
Expand Down
1 change: 1 addition & 0 deletions providers/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func NewGitLabProvider(p *ProviderData, opts options.Provider) (*GitLabProvider,
p.setProviderDefaults(providerDefaults{
name: gitlabProviderName,
})

if p.Scope == "" {
p.Scope = gitlabDefaultScope
}
Expand Down
1 change: 1 addition & 0 deletions providers/nextcloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ func NewNextcloudProvider(p *ProviderData) *NextcloudProvider {
p.setProviderDefaults(providerDefaults{
name: nextCloudProviderName,
})

p.getAuthorizationHeaderFunc = makeOIDCHeader
if p.EmailClaim == options.OIDCEmailClaim {
// This implies the email claim has not been overridden, we should set a default
Expand Down
1 change: 1 addition & 0 deletions providers/oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func TestOIDCProviderRedeem(t *testing.T) {
RefreshToken: refreshToken,
IDToken: idToken,
})

server, provider := newTestOIDCSetup(body, []byte(`{}`), []byte(`{}`))
defer server.Close()

Expand Down
1 change: 1 addition & 0 deletions providers/provider_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) {
for testName, tc := range testCases {
t.Run(testName, func(t *testing.T) {
g := NewWithT(t)

var (
profileURL *url.URL
profileURLCalled bool
Expand Down
13 changes: 8 additions & 5 deletions providers/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,12 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,
dst **url.URL
raw string
}{
"login": {dst: &p.LoginURL, raw: providerConfig.LoginURL},
"redeem": {dst: &p.RedeemURL, raw: providerConfig.RedeemURL},
"profile": {dst: &p.ProfileURL, raw: providerConfig.ProfileURL},
"validate": {dst: &p.ValidateURL, raw: providerConfig.ValidateURL},
"resource": {dst: &p.ProtectedResource, raw: providerConfig.ProtectedResource},
"login": {dst: &p.LoginURL, raw: providerConfig.LoginURL},
"redeem": {dst: &p.RedeemURL, raw: providerConfig.RedeemURL},
"profile": {dst: &p.ProfileURL, raw: providerConfig.ProfileURL},
"validate": {dst: &p.ValidateURL, raw: providerConfig.ValidateURL},
"resource": {dst: &p.ProtectedResource, raw: providerConfig.ProtectedResource},

"introspect": {dst: &p.IntrospectURL, raw: providerConfig.IntrospectURL},
} {
var err error
Expand Down Expand Up @@ -160,7 +161,9 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,
}

p.setAllowedGroups(providerConfig.AllowedGroups)

p.BackendLogoutURL = providerConfig.BackendLogoutURL

return p, nil
}

Expand Down
2 changes: 1 addition & 1 deletion providers/providers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ func TestScope(t *testing.T) {
}{
{
name: "oidc: with no scope provided",
configuredScope: "",
configuredType: "oidc",
configuredScope: "",
expectedScope: "openid email profile",
},
{
Expand Down
Loading