2e15f57 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Remove provider configuration from validation package

commit2e15f57b701a6ff8976f7dd68720bd44fd3df446 authorJoel Speed <joel.speed@hotmail.co.uk> dateTue 15 Feb 2022, 11:46 (5 years ago) parentd162b01

3 files changed+4−327

compare with master

unifiedsplit

pkg/validation/options.go−241 pkg/validation/options_test.go−86 pkg/validation/providers.go+4

pkg/validation/options.go−241

@@ -4,14 +4,11 @@ import (

44 "context"

55 "crypto/tls"

66 "fmt"

7− "io/ioutil"

87 "net/http"

98 "net/url"

10− "os"

119 "strings"

1210

1311 "github.com/coreos/go-oidc/v3/oidc"

14− "github.com/golang-jwt/jwt"

1512 "github.com/mbland/hmacauth"

1613 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

1714 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/ip"

@@ -19,7 +16,6 @@ import (

1916 internaloidc "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/oidc"

2017 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests"

2118 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util"

22− "github.com/oauth2-proxy/oauth2-proxy/v7/providers"

2319)

2420

2521// Validate checks that required options are set and validates those that they

@@ -61,100 +57,6 @@ func Validate(o *options.Options) error {

6157 "\n use email-domain=* to authorize all email addresses")

6258 }

6359

64− if o.Providers[0].OIDCConfig.IssuerURL != "" {

65−

66− ctx := context.Background()

67−

68− if o.Providers[0].OIDCConfig.InsecureSkipIssuerVerification && !o.Providers[0].OIDCConfig.SkipDiscovery {

69− // go-oidc doesn't let us pass bypass the issuer check this in the oidc.NewProvider call

70− // (which uses discovery to get the URLs), so we'll do a quick check ourselves and if

71− // we get the URLs, we'll just use the non-discovery path.

72−

73− logger.Printf("Performing OIDC Discovery...")

74−

75− requestURL := strings.TrimSuffix(o.Providers[0].OIDCConfig.IssuerURL, "/") + "/.well-known/openid-configuration"

76− body, err := requests.New(requestURL).

77− WithContext(ctx).

78− Do().

79− UnmarshalJSON()

80− if err != nil {

81− logger.Errorf("error: failed to discover OIDC configuration: %v", err)

82− } else {

83− // Prefer manually configured URLs. It's a bit unclear

84− // why you'd be doing discovery and also providing the URLs

85− // explicitly though...

86− if o.Providers[0].LoginURL == "" {

87− o.Providers[0].LoginURL = body.Get("authorization_endpoint").MustString()

88− }

89−

90− if o.Providers[0].RedeemURL == "" {

91− o.Providers[0].RedeemURL = body.Get("token_endpoint").MustString()

92− }

93−

94− if o.Providers[0].OIDCConfig.JwksURL == "" {

95− o.Providers[0].OIDCConfig.JwksURL = body.Get("jwks_uri").MustString()

96− }

97−

98− if o.Providers[0].ProfileURL == "" {

99− o.Providers[0].ProfileURL = body.Get("userinfo_endpoint").MustString()

100− }

101−

102− o.Providers[0].OIDCConfig.SkipDiscovery = true

103− }

104− }

105−

106− config := &oidc.Config{

107− ClientID: o.Providers[0].ClientID,

108− SkipIssuerCheck: o.Providers[0].OIDCConfig.InsecureSkipIssuerVerification,

109− SkipClientIDCheck: true, // client id check is done within oauth2-proxy: IDTokenVerifier.Verify

110− }

111−

112− verificationOptions := &internaloidc.IDTokenVerificationOptions{

113− AudienceClaims: o.Providers[0].OIDCConfig.AudienceClaims,

114− ClientID: o.Providers[0].ClientID,

115− ExtraAudiences: o.Providers[0].OIDCConfig.ExtraAudiences,

116− }

117−

118− // Construct a manual IDTokenVerifier from issuer URL & JWKS URI

119− // instead of metadata discovery if we enable -skip-oidc-discovery.

120− // In this case we need to make sure the required endpoints for

121− // the provider are configured.

122− if o.Providers[0].OIDCConfig.SkipDiscovery {

123− if o.Providers[0].LoginURL == "" {

124− msgs = append(msgs, "missing setting: login-url")

125− }

126− if o.Providers[0].RedeemURL == "" {

127− msgs = append(msgs, "missing setting: redeem-url")

128− }

129− if o.Providers[0].OIDCConfig.JwksURL == "" {

130− msgs = append(msgs, "missing setting: oidc-jwks-url")

131− }

132− keySet := oidc.NewRemoteKeySet(ctx, o.Providers[0].OIDCConfig.JwksURL)

133− o.SetOIDCVerifier(internaloidc.NewVerifier(

134− oidc.NewVerifier(o.Providers[0].OIDCConfig.IssuerURL, keySet, config), verificationOptions))

135− } else {

136− // Configure discoverable provider data.

137− provider, err := oidc.NewProvider(ctx, o.Providers[0].OIDCConfig.IssuerURL)

138− if err != nil {

139− return err

140− }

141−

142− o.SetOIDCVerifier(internaloidc.NewVerifier(provider.Verifier(config), verificationOptions))

143− o.Providers[0].LoginURL = provider.Endpoint().AuthURL

144− o.Providers[0].RedeemURL = provider.Endpoint().TokenURL

145− }

146− if o.Providers[0].Scope == "" {

147− o.Providers[0].Scope = "openid email profile"

148−

149− if len(o.Providers[0].AllowedGroups) > 0 {

150− o.Providers[0].Scope += " groups"

151− }

152− }

153− if o.Providers[0].OIDCConfig.UserIDClaim == "" {

154− o.Providers[0].OIDCConfig.UserIDClaim = "email"

155− }

156− }

157−

15860 if o.SkipJwtBearerTokens {

15961 // Configure extra issuers

16062 if len(o.ExtraJwtIssuers) > 0 {

@@ -182,7 +84,6 @@ func Validate(o *options.Options) error {

18284 }

18385

18486 msgs = append(msgs, validateUpstreams(o.UpstreamServers)...)

185− msgs = parseProviderInfo(o, msgs)

18687

18788 if o.ReverseProxy {

18889 parser, err := ip.GetRealClientIPParser(o.RealClientIPHeader)

@@ -207,148 +108,6 @@ func Validate(o *options.Options) error {

207108 return nil

208109}

209110

210−func parseProviderInfo(o *options.Options, msgs []string) []string {

211− p := &providers.ProviderData{

212− Scope: o.Providers[0].Scope,

213− ClientID: o.Providers[0].ClientID,

214− ClientSecret: o.Providers[0].ClientSecret,

215− ClientSecretFile: o.Providers[0].ClientSecretFile,

216− Prompt: o.Providers[0].Prompt,

217− ApprovalPrompt: o.Providers[0].ApprovalPrompt,

218− AcrValues: o.Providers[0].AcrValues,

219− }

220− p.LoginURL, msgs = parseURL(o.Providers[0].LoginURL, "login", msgs)

221− p.RedeemURL, msgs = parseURL(o.Providers[0].RedeemURL, "redeem", msgs)

222− p.ProfileURL, msgs = parseURL(o.Providers[0].ProfileURL, "profile", msgs)

223− p.ValidateURL, msgs = parseURL(o.Providers[0].ValidateURL, "validate", msgs)

224− p.ProtectedResource, msgs = parseURL(o.Providers[0].ProtectedResource, "resource", msgs)

225−

226− // Make the OIDC options available to all providers that support it

227− p.AllowUnverifiedEmail = o.Providers[0].OIDCConfig.InsecureAllowUnverifiedEmail

228− p.EmailClaim = o.Providers[0].OIDCConfig.EmailClaim

229− p.GroupsClaim = o.Providers[0].OIDCConfig.GroupsClaim

230− p.Verifier = o.GetOIDCVerifier()

231−

232− // TODO (@NickMeves) - Remove This

233− // Backwards Compatibility for Deprecated UserIDClaim option

234− if o.Providers[0].OIDCConfig.EmailClaim == providers.OIDCEmailClaim &&

235− o.Providers[0].OIDCConfig.UserIDClaim != providers.OIDCEmailClaim {

236− p.EmailClaim = o.Providers[0].OIDCConfig.UserIDClaim

237− }

238−

239− p.SetAllowedGroups(o.Providers[0].AllowedGroups)

240−

241− provider := providers.New(o.Providers[0].Type, p)

242− if provider == nil {

243− msgs = append(msgs, fmt.Sprintf("invalid setting: provider '%s' is not available", o.Providers[0].Type))

244− return msgs

245− }

246− o.SetProvider(provider)

247−

248− switch p := o.GetProvider().(type) {

249− case *providers.AzureProvider:

250− p.Configure(o.Providers[0].AzureConfig.Tenant)

251− case *providers.ADFSProvider:

252− p.Configure(o.Providers[0].ADFSConfig.SkipScope)

253− case *providers.GitHubProvider:

254− p.SetOrgTeam(o.Providers[0].GitHubConfig.Org, o.Providers[0].GitHubConfig.Team)

255− p.SetRepo(o.Providers[0].GitHubConfig.Repo, o.Providers[0].GitHubConfig.Token)

256− p.SetUsers(o.Providers[0].GitHubConfig.Users)

257− case *providers.KeycloakProvider:

258− // Backwards compatibility with `--keycloak-group` option

259− if len(o.Providers[0].KeycloakConfig.Groups) > 0 {

260− p.SetAllowedGroups(o.Providers[0].KeycloakConfig.Groups)

261− }

262− case *providers.KeycloakOIDCProvider:

263− if p.Verifier == nil {

264− msgs = append(msgs, "keycloak-oidc provider requires an oidc issuer URL")

265− }

266− p.AddAllowedRoles(o.Providers[0].KeycloakConfig.Roles)

267− case *providers.GoogleProvider:

268− if o.Providers[0].GoogleConfig.ServiceAccountJSON != "" {

269− file, err := os.Open(o.Providers[0].GoogleConfig.ServiceAccountJSON)

270− if err != nil {

271− msgs = append(msgs, "invalid Google credentials file: "+o.Providers[0].GoogleConfig.ServiceAccountJSON)

272− } else {

273− groups := o.Providers[0].AllowedGroups

274− // Backwards compatibility with `--google-group` option

275− if len(o.Providers[0].GoogleConfig.Groups) > 0 {

276− groups = o.Providers[0].GoogleConfig.Groups

277− p.SetAllowedGroups(groups)

278− }

279− p.SetGroupRestriction(groups, o.Providers[0].GoogleConfig.AdminEmail, file)

280− }

281− }

282− case *providers.BitbucketProvider:

283− p.SetTeam(o.Providers[0].BitbucketConfig.Team)

284− p.SetRepository(o.Providers[0].BitbucketConfig.Repository)

285− case *providers.OIDCProvider:

286− p.SkipNonce = o.Providers[0].OIDCConfig.InsecureSkipNonce

287− if p.Verifier == nil {

288− msgs = append(msgs, "oidc provider requires an oidc issuer URL")

289− }

290− case *providers.GitLabProvider:

291− p.SetAllowedGroups(o.Providers[0].GitLabConfig.Group)

292− err := p.SetAllowedProjects(o.Providers[0].GitLabConfig.Projects)

293− if err != nil {

294− msgs = append(msgs, "failed to setup gitlab project access level")

295− }

296−

297− if p.Verifier == nil {

298− // Initialize with default verifier for gitlab.com

299− ctx := context.Background()

300−

301− provider, err := oidc.NewProvider(ctx, "https://gitlab.com")

302− if err != nil {

303− msgs = append(msgs, "failed to initialize oidc provider for gitlab.com")

304− } else {

305− verificationOptions := &internaloidc.IDTokenVerificationOptions{

306− AudienceClaims: o.Providers[0].OIDCConfig.AudienceClaims,

307− ClientID: o.Providers[0].ClientID,

308− ExtraAudiences: o.Providers[0].OIDCConfig.ExtraAudiences,

309− }

310− p.Verifier = internaloidc.NewVerifier(provider.Verifier(&oidc.Config{

311− ClientID: o.Providers[0].ClientID,

312− }), verificationOptions)

313−

314− p.LoginURL, msgs = parseURL(provider.Endpoint().AuthURL, "login", msgs)

315− p.RedeemURL, msgs = parseURL(provider.Endpoint().TokenURL, "redeem", msgs)

316− }

317− }

318− case *providers.LoginGovProvider:

319− p.PubJWKURL, msgs = parseURL(o.Providers[0].LoginGovConfig.PubJWKURL, "pubjwk", msgs)

320−

321− // JWT key can be supplied via env variable or file in the filesystem, but not both.

322− switch {

323− case o.Providers[0].LoginGovConfig.JWTKey != "" && o.Providers[0].LoginGovConfig.JWTKeyFile != "":

324− msgs = append(msgs, "cannot set both jwt-key and jwt-key-file options")

325− case o.Providers[0].LoginGovConfig.JWTKey == "" && o.Providers[0].LoginGovConfig.JWTKeyFile == "":

326− msgs = append(msgs, "login.gov provider requires a private key for signing JWTs")

327− case o.Providers[0].LoginGovConfig.JWTKey != "":

328− // The JWT Key is in the commandline argument

329− signKey, err := jwt.ParseRSAPrivateKeyFromPEM([]byte(o.Providers[0].LoginGovConfig.JWTKey))

330− if err != nil {

331− msgs = append(msgs, "could not parse RSA Private Key PEM")

332− } else {

333− p.JWTKey = signKey

334− }

335− case o.Providers[0].LoginGovConfig.JWTKeyFile != "":

336− // The JWT key is in the filesystem

337− keyData, err := ioutil.ReadFile(o.Providers[0].LoginGovConfig.JWTKeyFile)

338− if err != nil {

339− msgs = append(msgs, "could not read key file: "+o.Providers[0].LoginGovConfig.JWTKeyFile)

340− }

341− signKey, err := jwt.ParseRSAPrivateKeyFromPEM(keyData)

342− if err != nil {

343− msgs = append(msgs, "could not parse private key from PEM file:"+o.Providers[0].LoginGovConfig.JWTKeyFile)

344− } else {

345− p.JWTKey = signKey

346− }

347− }

348− }

349− return msgs

350−}

351−

352111func parseSignatureKey(o *options.Options, msgs []string) []string {

353112 if o.SignatureKey == "" {

354113 return msgs

pkg/validation/options_test.go−86

@@ -56,63 +56,6 @@ func TestNewOptions(t *testing.T) {

5656 assert.Equal(t, expected, err.Error())

5757}

5858

59−func TestClientSecretFileOptionFails(t *testing.T) {

60− o := options.NewOptions()

61− o.Cookie.Secret = cookieSecret

62− o.Providers[0].ID = providerID

63− o.Providers[0].ClientID = clientID

64− o.Providers[0].ClientSecretFile = clientSecret

65− o.EmailDomains = []string{"*"}

66− err := Validate(o)

67− assert.NotEqual(t, nil, err)

68−

69− p := o.GetProvider().Data()

70− assert.Equal(t, clientSecret, p.ClientSecretFile)

71− assert.Equal(t, "", p.ClientSecret)

72−

73− s, err := p.GetClientSecret()

74− assert.NotEqual(t, nil, err)

75− assert.Equal(t, "", s)

76−}

77−

78−func TestClientSecretFileOption(t *testing.T) {

79− var err error

80− f, err := ioutil.TempFile("", "client_secret_temp_file_")

81− if err != nil {

82− t.Fatalf("failed to create temp file: %v", err)

83− }

84− _, err = f.WriteString("testcase")

85− if err != nil {

86− t.Fatalf("failed to write to temp file: %v", err)

87− }

88− if err := f.Close(); err != nil {

89− t.Fatalf("failed to close temp file: %v", err)

90− }

91− clientSecretFileName := f.Name()

92− defer func(t *testing.T) {

93− if err := os.Remove(clientSecretFileName); err != nil {

94− t.Fatalf("failed to delete temp file: %v", err)

95− }

96− }(t)

97−

98− o := options.NewOptions()

99− o.Cookie.Secret = cookieSecret

100− o.Providers[0].ID = providerID

101− o.Providers[0].ClientID = clientID

102− o.Providers[0].ClientSecretFile = clientSecretFileName

103− o.EmailDomains = []string{"*"}

104− err = Validate(o)

105− assert.Equal(t, nil, err)

106−

107− p := o.GetProvider().Data()

108− assert.Equal(t, clientSecretFileName, p.ClientSecretFile)

109− assert.Equal(t, "", p.ClientSecret)

110−

111− s, err := p.GetClientSecret()

112− assert.Equal(t, nil, err)

113− assert.Equal(t, "testcase", s)

114−}

115−

11659func TestGoogleGroupOptions(t *testing.T) {

11760 o := testOptions()

11861 o.Providers[0].GoogleConfig.Groups = []string{"googlegroup"}

@@ -155,18 +98,6 @@ func TestRedirectURL(t *testing.T) {

15598 assert.Equal(t, expected, o.GetRedirectURL())

15699}

157100

158−func TestDefaultProviderApiSettings(t *testing.T) {

159− o := testOptions()

160− assert.Equal(t, nil, Validate(o))

161− p := o.GetProvider().Data()

162− assert.Equal(t, "https://accounts.google.com/o/oauth2/auth?access_type=offline",

163− p.LoginURL.String())

164− assert.Equal(t, "https://www.googleapis.com/oauth2/v3/token",

165− p.RedeemURL.String())

166− assert.Equal(t, "", p.ProfileURL.String())

167− assert.Equal(t, "profile email", p.Scope)

168−}

169−

170101func TestCookieRefreshMustBeLessThanCookieExpire(t *testing.T) {

171102 o := testOptions()

172103 assert.Equal(t, nil, Validate(o))

@@ -228,23 +159,6 @@ func TestValidateSignatureKeyUnsupportedAlgorithm(t *testing.T) {

228159 " unsupported signature hash algorithm: "+o.SignatureKey)

229160}

230161

231−func TestSkipOIDCDiscovery(t *testing.T) {

232− o := testOptions()

233− o.Providers[0].Type = "oidc"

234− o.Providers[0].OIDCConfig.IssuerURL = "https://login.microsoftonline.com/fabrikamb2c.onmicrosoft.com/v2.0/"

235− o.Providers[0].OIDCConfig.SkipDiscovery = true

236−

237− err := Validate(o)

238− assert.Equal(t, "invalid configuration:\n"+

239− " missing setting: login-url\n missing setting: redeem-url\n missing setting: oidc-jwks-url", err.Error())

240−

241− o.Providers[0].LoginURL = "https://login.microsoftonline.com/fabrikamb2c.onmicrosoft.com/oauth2/v2.0/authorize?p=b2c_1_sign_in"

242− o.Providers[0].RedeemURL = "https://login.microsoftonline.com/fabrikamb2c.onmicrosoft.com/oauth2/v2.0/token?p=b2c_1_sign_in"

243− o.Providers[0].OIDCConfig.JwksURL = "https://login.microsoftonline.com/fabrikamb2c.onmicrosoft.com/discovery/v2.0/keys"

244−

245− assert.Equal(t, nil, Validate(o))

246−}

247−

248162func TestGCPHealthcheck(t *testing.T) {

249163 o := testOptions()

250164 o.GCPHealthChecks = true

pkg/validation/providers.go+4

@@ -3,6 +3,7 @@ package validation

33import (

44 "fmt"

55 "io/ioutil"

6+ "os"

78 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"

89)

@@ -77,7 +78,10 @@ func validateGoogleConfig(provider options.Provider) []string {

7778 }

7879 if provider.GoogleConfig.ServiceAccountJSON == "" {

7980 msgs = append(msgs, "missing setting: google-service-account-json")

81+ } else if _, err := os.Stat(provider.GoogleConfig.ServiceAccountJSON); err != nil {

82+ msgs = append(msgs, fmt.Sprintf("invalid Google credentials file: %s", provider.GoogleConfig.ServiceAccountJSON))

8083 }

8184 }

85+

8286 return msgs

8387}